[PATCH 09/16] hw/9pfs: Disable unsupported flags and features for Windows
From: Guohuai Shi Some flags and features are not supported on Windows, like mknod, readlink, file mode, etc. Update the codes for Windows. Signed-off-by: Guohuai Shi Signed-off-by: Bin Meng --- hw/9pfs/9p-util.h | 6 +++- hw/9pfs/9p.c | 90 ++- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h index 82b2d0c3e4..3d154e9103 100644 --- a/hw/9pfs/9p-util.h +++ b/hw/9pfs/9p-util.h @@ -53,8 +53,10 @@ static inline uint64_t makedev_dotl(uint32_t dev_major, uint32_t dev_minor) */ static inline uint64_t host_dev_to_dotl_dev(dev_t dev) { -#ifdef CONFIG_LINUX +#if defined(CONFIG_LINUX) return dev; +#elif defined(CONFIG_WIN32) +return 0; #else return makedev_dotl(major(dev), minor(dev)); #endif @@ -260,7 +262,9 @@ static inline struct dirent *qemu_dirent_dup(struct dirent *dent) #if defined CONFIG_DARWIN && defined CONFIG_PTHREAD_FCHDIR_NP int pthread_fchdir_np(int fd) __attribute__((weak_import)); #endif +#ifndef CONFIG_WIN32 int qemu_mknodat(P9_FILE_ID dirfd, const char *filename, mode_t mode, dev_t dev); +#endif #endif diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 6c4af86240..771aab34ac 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -39,6 +39,11 @@ #include "qemu/xxhash.h" #include +#ifdef CONFIG_WIN32 +#define UTIME_NOW ((1l << 30) - 1l) +#define UTIME_OMIT ((1l << 30) - 2l) +#endif + int open_fd_hw; int total_open_fd; static int open_fd_rc; @@ -132,13 +137,17 @@ static int dotl_to_open_flags(int flags) DotlOpenflagMap dotl_oflag_map[] = { { P9_DOTL_CREATE, O_CREAT }, { P9_DOTL_EXCL, O_EXCL }, +#ifndef CONFIG_WIN32 { P9_DOTL_NOCTTY , O_NOCTTY }, +#endif { P9_DOTL_TRUNC, O_TRUNC }, { P9_DOTL_APPEND, O_APPEND }, +#ifndef CONFIG_WIN32 { P9_DOTL_NONBLOCK, O_NONBLOCK } , { P9_DOTL_DSYNC, O_DSYNC }, { P9_DOTL_FASYNC, FASYNC }, -#ifndef CONFIG_DARWIN +#endif +#ifdef CONFIG_LINUX { P9_DOTL_NOATIME, O_NOATIME }, /* * On Darwin, we could map to F_NOCACHE, which is @@ -151,8 +160,10 @@ static int dotl_to_open_flags(int flags) #endif { P9_DOTL_LARGEFILE, O_LARGEFILE }, { P9_DOTL_DIRECTORY, O_DIRECTORY }, +#ifndef CONFIG_WIN32 { P9_DOTL_NOFOLLOW, O_NOFOLLOW }, { P9_DOTL_SYNC, O_SYNC }, +#endif }; for (i = 0; i < ARRAY_SIZE(dotl_oflag_map); i++) { @@ -179,8 +190,11 @@ static int get_dotl_openflags(V9fsState *s, int oflags) * Filter the client open flags */ flags = dotl_to_open_flags(oflags); -flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT); -#ifndef CONFIG_DARWIN +flags &= ~(O_CREAT); +#ifndef CONFIG_WIN32 +flags &= ~(O_NOCTTY | O_ASYNC); +#endif +#ifdef CONFIG_LINUX /* * Ignore direct disk access hint until the server supports it. */ @@ -986,9 +1000,11 @@ static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp) if (S_ISDIR(stbuf->st_mode)) { qidp->type |= P9_QID_TYPE_DIR; } +#ifndef CONFIG_WIN32 if (S_ISLNK(stbuf->st_mode)) { qidp->type |= P9_QID_TYPE_SYMLINK; } +#endif return 0; } @@ -1097,6 +1113,7 @@ static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) ret |= S_IFDIR; } +#ifndef CONFIG_WIN32 if (mode & P9_STAT_MODE_SYMLINK) { ret |= S_IFLNK; } @@ -1106,6 +1123,7 @@ static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) if (mode & P9_STAT_MODE_NAMED_PIPE) { ret |= S_IFIFO; } +#endif if (mode & P9_STAT_MODE_DEVICE) { if (extension->size && extension->data[0] == 'c') { ret |= S_IFCHR; @@ -1118,6 +1136,7 @@ static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) ret |= S_IFREG; } +#ifndef CONFIG_WIN32 if (mode & P9_STAT_MODE_SETUID) { ret |= S_ISUID; } @@ -1127,6 +1146,7 @@ static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) if (mode & P9_STAT_MODE_SETVTX) { ret |= S_ISVTX; } +#endif return ret; } @@ -1182,6 +1202,7 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf) mode |= P9_STAT_MODE_DIR; } +#ifndef CONFIG_WIN32 if (S_ISLNK(stbuf->st_mode)) { mode |= P9_STAT_MODE_SYMLINK; } @@ -1193,11 +1214,13 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf) if (S_ISFIFO(stbuf->st_mode)) { mode |= P9_STAT_MODE_NAMED_PIPE; } +#endif if (S_ISBLK(stbuf->st_mode) || S_ISCHR(stbuf->st_mode)) { mode |= P9_STAT_MODE_DEVICE; } +#ifndef CONFIG_WIN32 if (stbuf->st_mode & S_ISUID) { mode |= P9_STAT_MODE_SETUID; } @@ -1209,6 +1232,7 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf) if (stbuf->st_mode & S_ISVTX) { mode |= P9_STAT_MODE_SETVTX; } +#endif return mode; } @@ -1247,9 +1271
Re: [PATCH] include/hw/scsi/scsi.h: Remove unused scsi_legacy_handle_cmdline() prototype
Le 13/10/2022 à 15:05, Peter Maydell a écrit : In commit 1454509726719e0933c80 we removed the function scsi_legacy_handle_cmdline() and all of its callers, but forgot to delete the prototype from the header function. Delete the prototype too. Signed-off-by: Peter Maydell --- include/hw/scsi/scsi.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index 001103488c2..9ad86b25bb4 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -187,7 +187,6 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk, BlockdevOnError werror, const char *serial, Error **errp); void scsi_bus_legacy_handle_cmdline(SCSIBus *bus); -void scsi_legacy_handle_cmdline(void); SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d, uint32_t tag, uint32_t lun, void *hba_private); Applied to my trivial-patches branch. Thanks, Laurent
[PATCH v2 09/43] hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
Suggested-by: Mark Cave-Ayland Signed-off-by: Bernhard Beschow --- hw/i386/pc_piix.c | 3 ++- hw/i386/pc_q35.c | 13 +++-- hw/isa/piix4.c| 2 +- hw/usb/hcd-uhci.c | 16 hw/usb/hcd-uhci.h | 9 + 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index e26509a935..caa983d76e 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -50,6 +50,7 @@ #include "exec/memory.h" #include "hw/acpi/acpi.h" #include "hw/acpi/piix4.h" +#include "hw/usb/hcd-uhci.h" #include "qapi/error.h" #include "qemu/error-report.h" #include "sysemu/xen.h" @@ -291,7 +292,7 @@ static void pc_init1(MachineState *machine, #endif if (pcmc->pci_enabled && machine_usb(machine)) { -pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); +pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI); } if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) { diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index a496bd6e74..fa24b5ef66 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -48,6 +48,7 @@ #include "hw/ide/pci.h" #include "hw/ide/ahci.h" #include "hw/usb.h" +#include "hw/usb/hcd-uhci.h" #include "qapi/error.h" #include "qemu/error-report.h" #include "sysemu/numa.h" @@ -65,15 +66,15 @@ struct ehci_companions { }; static const struct ehci_companions ich9_1d[] = { -{ .name = "ich9-usb-uhci1", .func = 0, .port = 0 }, -{ .name = "ich9-usb-uhci2", .func = 1, .port = 2 }, -{ .name = "ich9-usb-uhci3", .func = 2, .port = 4 }, +{ .name = TYPE_ICH9_USB_UHCI1, .func = 0, .port = 0 }, +{ .name = TYPE_ICH9_USB_UHCI2, .func = 1, .port = 2 }, +{ .name = TYPE_ICH9_USB_UHCI3, .func = 2, .port = 4 }, }; static const struct ehci_companions ich9_1a[] = { -{ .name = "ich9-usb-uhci4", .func = 0, .port = 0 }, -{ .name = "ich9-usb-uhci5", .func = 1, .port = 2 }, -{ .name = "ich9-usb-uhci6", .func = 2, .port = 4 }, +{ .name = TYPE_ICH9_USB_UHCI4, .func = 0, .port = 0 }, +{ .name = TYPE_ICH9_USB_UHCI5, .func = 1, .port = 2 }, +{ .name = TYPE_ICH9_USB_UHCI6, .func = 2, .port = 4 }, }; static int ehci_create_ich9_with_companions(PCIBus *bus, int slot) diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c index e05e65d3bc..83b50c3a9b 100644 --- a/hw/isa/piix4.c +++ b/hw/isa/piix4.c @@ -279,7 +279,7 @@ static void piix4_init(Object *obj) object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC); object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE); -object_initialize_child(obj, "uhci", &s->uhci, "piix4-usb-uhci"); +object_initialize_child(obj, "uhci", &s->uhci, TYPE_PIIX4_USB_UHCI); object_initialize_child(obj, "pm", &s->pm, TYPE_PIIX4_PM); qdev_prop_set_uint32(DEVICE(&s->pm), "smb_io_base", 0x1100); diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index d1b5657d72..0ec4cfaa52 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -1292,56 +1292,56 @@ void uhci_data_class_init(ObjectClass *klass, void *data) static UHCIInfo uhci_info[] = { { -.name = "piix3-usb-uhci", +.name = TYPE_PIIX3_USB_UHCI, .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82371SB_2, .revision = 0x01, .irq_pin = 3, .unplug= true, },{ -.name = "piix4-usb-uhci", +.name = TYPE_PIIX4_USB_UHCI, .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82371AB_2, .revision = 0x01, .irq_pin = 3, .unplug= true, },{ -.name = "ich9-usb-uhci1", /* 00:1d.0 */ +.name = TYPE_ICH9_USB_UHCI1, /* 00:1d.0 */ .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1, .revision = 0x03, .irq_pin = 0, .unplug= false, },{ -.name = "ich9-usb-uhci2", /* 00:1d.1 */ +.name = TYPE_ICH9_USB_UHCI2, /* 00:1d.1 */ .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2, .revision = 0x03, .irq_pin = 1, .unplug= false, },{ -.name = "ich9-usb-uhci3", /* 00:1d.2 */ +.name = TYPE_ICH9_USB_UHCI3, /* 00:1d.2 */ .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3, .revision = 0x03, .irq_pin = 2, .unplug= false, },{ -.name = "ich9-usb-uhci4", /* 00:1a.0 */ +.name = TYPE_ICH9_USB_UHCI4, /* 00:1a.0 */ .vendor_id = PCI_VENDOR_ID_INTEL, .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4, .revision = 0x03, .irq_pin = 0, .unplug= false, },{ -.name = "ich9-usb-uhci5", /* 00:1a.1 */ +.name = TYPE_ICH9_USB_UHCI5, /* 00:1a.1 */ .vendor_id =
[Bug 1994002] [NEW] [SRU] migration was active, but no RAM info was set
Public bug reported: While live-migrating many instances concurrently, libvirt sometimes return internal error: migration was active, but no RAM info was set: ~~~ 2022-03-30 06:08:37.197 7 WARNING nova.virt.libvirt.driver [req-5c3296cf-88ee-4af6-ae6a-ddba99935e23 - - - - -] [instance: af339c99-1182-4489-b15c-21e52f50f724] Error monitoring migration: internal error: migration was active, but no RAM info was set: libvirt.libvirtError: internal error: migration was active, but no RAM info was set ~~~ >From upstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=2074205 [Impact] * Effects of this bug are mostly observed in large scale clusters with a lot of live migration activity. * Has second order effects for consumers of migration monitor such as libvirt and openstack. [Test Case] Steps to Reproduce: 1. live evacuate a compute 2. live migration of one or more instances fails with the above error N.B Due to the nature of this bug it is difficult consistently reproduce. [Where problems could occur] * In the event of a regression the migration monitor may report an inconsistent state. ** Affects: cloud-archive Importance: Undecided Status: New ** Affects: cloud-archive/ussuri Importance: Undecided Assignee: Brett Milford (brettmilford) Status: New ** Affects: qemu Importance: Undecided Status: New ** Affects: qemu (Ubuntu) Importance: Undecided Status: New ** Affects: qemu (Ubuntu Focal) Importance: Undecided Assignee: Brett Milford (brettmilford) Status: New ** Affects: qemu (Ubuntu Jammy) Importance: Undecided Assignee: Brett Milford (brettmilford) Status: New ** Affects: qemu (Ubuntu Kinetic) Importance: Undecided Status: New ** Also affects: qemu (Ubuntu Kinetic) Importance: Undecided Status: New ** Also affects: qemu (Ubuntu Jammy) Importance: Undecided Status: New ** Also affects: qemu (Ubuntu Focal) Importance: Undecided Status: New ** Also affects: qemu Importance: Undecided Status: New ** Also affects: cloud-archive Importance: Undecided Status: New ** Also affects: cloud-archive/ussuri Importance: Undecided Status: New ** Changed in: cloud-archive/ussuri Assignee: (unassigned) => Brett Milford (brettmilford) ** Changed in: qemu (Ubuntu Focal) Assignee: (unassigned) => Brett Milford (brettmilford) ** Changed in: qemu (Ubuntu Jammy) Assignee: (unassigned) => Brett Milford (brettmilford) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1994002 Title: [SRU] migration was active, but no RAM info was set Status in Ubuntu Cloud Archive: New Status in Ubuntu Cloud Archive ussuri series: New Status in QEMU: New Status in qemu package in Ubuntu: New Status in qemu source package in Focal: New Status in qemu source package in Jammy: New Status in qemu source package in Kinetic: New Bug description: While live-migrating many instances concurrently, libvirt sometimes return internal error: migration was active, but no RAM info was set: ~~~ 2022-03-30 06:08:37.197 7 WARNING nova.virt.libvirt.driver [req-5c3296cf-88ee-4af6-ae6a-ddba99935e23 - - - - -] [instance: af339c99-1182-4489-b15c-21e52f50f724] Error monitoring migration: internal error: migration was active, but no RAM info was set: libvirt.libvirtError: internal error: migration was active, but no RAM info was set ~~~ From upstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=2074205 [Impact] * Effects of this bug are mostly observed in large scale clusters with a lot of live migration activity. * Has second order effects for consumers of migration monitor such as libvirt and openstack. [Test Case] Steps to Reproduce: 1. live evacuate a compute 2. live migration of one or more instances fails with the above error N.B Due to the nature of this bug it is difficult consistently reproduce. [Where problems could occur] * In the event of a regression the migration monitor may report an inconsistent state. To manage notifications about this bug go to: https://bugs.launchpad.net/cloud-archive/+bug/1994002/+subscriptions
Re: [PATCH v3 2/4] hw/audio: fix tab indentation
Am 21.10.22 um 18:59 schrieb Amarjargal Gundjalam: The TABs should be replaced with spaces, to make sure that we have a consistent coding style with an indentation of 4 spaces everywhere. Resolves:https://gitlab.com/qemu-project/qemu/-/issues/370 Reviewed-by: Daniel P. Berrangé Signed-off-by: Amarjargal Gundjalam --- hw/audio/fmopl.c | 1664 ++--- hw/audio/fmopl.h | 138 +-- hw/audio/intel-hda-defs.h | 1008 +++--- hw/audio/wm8750.c | 270 +++--- 4 files changed, 1540 insertions(+), 1540 deletions(-) Hi Amarjargal, I had a look at hw/audio/fmopl.c and I think the result doesn't look right. A few comments are no longer correctly aligned. I guess you just replaced all TABs with four spaces. But this is not how TABs work. For reference: I used the vim command :%s/^I/ /g and the result is identical to your file. The commands :se ts=4 expandtab :retab would have been a much better starting point for the last few manual changes. Here is another example. For the file hw/audio/wm8750.c I would have started with the following vim commands :se ts=8 expandtab :retab With best regards, Volker
[PATCH 2/2] accel/kvm: introduce begin/commit listener callbacks
These callback make sure that all vcpus are blocked before performing memslot updates, and resumed once we are finished. They rely on kvm support for KVM_KICK_ALL_RUNNING_VCPUS and KVM_RESUME_ALL_KICKED_VCPUS ioctls to respectively pause and resume all vcpus that are in KVM_RUN state. Signed-off-by: Emanuele Giuseppe Esposito --- accel/kvm/kvm-all.c | 50 + 1 file changed, 50 insertions(+) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 645f0a249a..bd0dfa8613 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -178,6 +178,8 @@ bool kvm_has_guest_debug; int kvm_sstep_flags; static bool kvm_immediate_exit; static hwaddr kvm_max_slot_size = ~0; +static QemuEvent mem_transaction_proceed; + static const KVMCapabilityInfo kvm_required_capabilites[] = { KVM_CAP_INFO(USER_MEMORY), @@ -1523,6 +1525,38 @@ static void kvm_region_del(MemoryListener *listener, memory_region_unref(section->mr); } +static void kvm_begin(MemoryListener *listener) +{ +KVMState *s = kvm_state; + +/* + * Make sure BQL is taken so cpus in kvm_cpu_exec that just exited from + * KVM_RUN do not continue, since many run->exit_reason take it anyways. + */ +assert(qemu_mutex_iothread_locked()); + +/* + * Stop incoming cpus that want to execute KVM_RUN from running. + * Makes cpus calling qemu_event_wait() in kvm_cpu_exec() block. + */ +qemu_event_reset(&mem_transaction_proceed); + +/* Ask KVM to stop all vcpus that are currently running KVM_RUN */ +kvm_vm_ioctl(s, KVM_KICK_ALL_RUNNING_VCPUS); +} + +static void kvm_commit(MemoryListener *listener) +{ +KVMState *s = kvm_state; +assert(qemu_mutex_iothread_locked()); + +/* Ask KVM to resume all vcpus that are currently blocked in KVM_RUN */ +kvm_vm_ioctl(s, KVM_RESUME_ALL_KICKED_VCPUS); + +/* Resume cpus waiting in qemu_event_wait() in kvm_cpu_exec() */ +qemu_event_set(&mem_transaction_proceed); +} + static void kvm_log_sync(MemoryListener *listener, MemoryRegionSection *section) { @@ -1668,6 +1702,8 @@ void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml, kml->listener.region_del = kvm_region_del; kml->listener.log_start = kvm_log_start; kml->listener.log_stop = kvm_log_stop; +kml->listener.begin = kvm_begin; +kml->listener.commit = kvm_commit; kml->listener.priority = 10; kml->listener.name = name; @@ -2611,6 +2647,7 @@ static int kvm_init(MachineState *ms) } kvm_state = s; +qemu_event_init(&mem_transaction_proceed, false); ret = kvm_arch_init(ms, s); if (ret < 0) { @@ -2875,6 +2912,19 @@ int kvm_cpu_exec(CPUState *cpu) } qemu_mutex_unlock_iothread(); + +/* + * Wait that a running memory transaction (memslot update) is concluded. + * + * If the event state is EV_SET, it means kvm_commit() has already finished + * and called qemu_event_set(), therefore cpu can execute. + * + * If it's EV_FREE, it means kvm_begin() has already called + * qemu_event_reset(), therefore a memory transaction is happening and the + * cpu must wait. + */ +qemu_event_wait(&mem_transaction_proceed); + cpu_exec_start(cpu); do { -- 2.31.1
Re: [PULL 0/2] M68k for 7.2 patches
Le 22/10/2022 à 11:17, Laurent Vivier a écrit : The following changes since commit 0529245488865038344d64fff7ee05864d3d17f6: Merge tag 'pull-target-arm-20221020' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-10-20 14:36:12 -0400) are available in the Git repository at: https://github.com/vivier/qemu-m68k.git tags/m68k-for-7.2-pull-request for you to fetch changes up to d3c7b59be912d257ae7773eb3f1127f81a710a4d: m68k: write bootinfo as rom section and re-randomize on reboot (2022-10-22 09:58:24 +0200) Pull request m68k branch 20221022 Update rng seed boot parameter Jason A. Donenfeld (2): m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED m68k: write bootinfo as rom section and re-randomize on reboot Please, cancel this pull request, there is a new version of PATCH 2. Thanks, Laurent
Re: [PATCH] tests/qtest/ac97-test: add up-/downsampling tests
Hi On Mon, Oct 24, 2022 at 9:28 AM Volker Rümelin wrote: > Test if the audio subsystem can handle extreme up- and down- > sampling ratios like 44100/1 and 1/44100. For some time these > used to trigger QEMU aborts. The test was taken from > https://gitlab.com/qemu-project/qemu/-/issues/71 where it was > used to demonstrate a very different issue. > > Suggested-by: Marc-André Lureau > Signed-off-by: Volker Rümelin > Thanks for working on this It seems to show something different though: " A bug was just triggered in audio_calloc Save all your work and restart without audio I am sorry " AUD_open_out() is called with audsettings: {freq = 1, nchannels = 2, fmt = AUDIO_FORMAT_S16, endianness = 0} And that's it. Any idea? --- > tests/qtest/ac97-test.c | 40 +++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/ac97-test.c b/tests/qtest/ac97-test.c > index 74103efdfa..ce25f1d588 100644 > --- a/tests/qtest/ac97-test.c > +++ b/tests/qtest/ac97-test.c > @@ -42,16 +42,54 @@ static void *ac97_create(void *pci_bus, > QGuestAllocator *alloc, void *addr) > return &ac97->obj; > } > > +/* > + * This is rather a test of the audio subsystem and not an AC97 test. > Test if > + * the audio subsystem can handle a 44100/1 upsample ratio. With an audio > + * mixing buffer size of 1024 audio frames, the audio subsystem should > either > + * generate 1024 output frames from 0.0232 input frames or silently give > up. > + */ > +static void ac97_playback_upsample(void *obj, void *data, QGuestAllocator > *alloc) > +{ > +QAC97 *ac97 = obj; > +QPCIDevice *dev = &ac97->dev; > +QPCIBar bar0; > + > +qpci_device_enable(dev); > +bar0 = qpci_iomap(dev, 0, NULL); > +qpci_io_writew(dev, bar0, 0x2c, 0x1); > +} > + > +/* > + * This test is similar to the playback upsample test. This time the audio > + * subsystem should either generate 0.0232 audio frames from 1024 input > frames > + * or silently give up. > + */ > +static void ac97_record_downsample(void *obj, void *data, QGuestAllocator > *alloc) > +{ > +QAC97 *ac97 = obj; > +QPCIDevice *dev = &ac97->dev; > +QPCIBar bar0; > + > +qpci_device_enable(dev); > +bar0 = qpci_iomap(dev, 0, NULL); > +qpci_io_writew(dev, bar0, 0x32, 0x1); > +} > + > static void ac97_register_nodes(void) > { > QOSGraphEdgeOptions opts = { > -.extra_device_opts = "addr=04.0", > +.extra_device_opts = "addr=04.0,audiodev=snd0", > +.after_cmd_line = "-audiodev none,id=snd0" > + ",out.frequency=44100,in.frequency=44100", > }; > add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) }); > > qos_node_create_driver("AC97", ac97_create); > qos_node_produces("AC97", "pci-device"); > qos_node_consumes("AC97", "pci-bus", &opts); > + > +qos_add_test("playback_upsample", "AC97", ac97_playback_upsample, > NULL); > +qos_add_test("record_downsample", "AC97", ac97_record_downsample, > NULL); > } > > libqos_init(ac97_register_nodes); > -- > 2.35.3 > > > -- Marc-André Lureau
[PULL 0/2] M68k for 7.2 patches
The following changes since commit 0529245488865038344d64fff7ee05864d3d17f6: Merge tag 'pull-target-arm-20221020' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-10-20 14:36:12 -0400) are available in the Git repository at: https://github.com/vivier/qemu-m68k.git tags/m68k-for-7.2-pull-request for you to fetch changes up to 281ac13ecedf8bfe1b83e566f39cb5683e553cb6: m68k: write bootinfo as rom section and re-randomize on reboot (2022-10-24 10:47:14 +0200) Pull request m68k branch 20221024 Update rng seed boot parameter Jason A. Donenfeld (2): m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED m68k: write bootinfo as rom section and re-randomize on reboot hw/m68k/bootinfo.h| 48 ++-- .../standard-headers/asm-m68k/bootinfo-virt.h | 4 +- include/standard-headers/asm-m68k/bootinfo.h | 8 +- hw/m68k/q800.c| 76 ++- hw/m68k/virt.c| 57 +- 5 files changed, 130 insertions(+), 63 deletions(-) -- 2.37.3
Re: [PATCH v2 14/43] hw/intc/i8259: Introduce i8259 proxy "isa-pic"
Hi Bernhard, On 22/10/22 17:04, Bernhard Beschow wrote: Having an i8259 proxy allows for ISA PICs to be created and wired up in southbridges. This is especially interesting for PIIX3 for two reasons: First, the southbridge doesn't need to care about the virtualization technology used (KVM, TCG, Xen) due to in-IRQs (where devices get attached) and out-IRQs (which will trigger the IRQs of the respective virtzalization technology) are separated. Second, since the in-IRQs are populated with fully initialized qemu_irq's, they can already be wired up inside PIIX3. Signed-off-by: Bernhard Beschow --- hw/intc/i8259.c | 27 +++ include/hw/intc/i8259.h | 14 ++ 2 files changed, 41 insertions(+) +static void isapic_set_irq(void *opaque, int irq, int level) +{ +ISAPICState *s = opaque; + +qemu_set_irq(s->out_irqs[irq], level); +} + +static void isapic_init(Object *obj) +{ +ISAPICState *s = ISA_PIC(obj); + +qdev_init_gpio_in(DEVICE(s), isapic_set_irq, ISA_NUM_IRQS); +qdev_init_gpio_out(DEVICE(s), s->out_irqs, ISA_NUM_IRQS); + +for (int i = 0; i < ISA_NUM_IRQS; ++i) { +s->in_irqs[i] = qdev_get_gpio_in(DEVICE(s), i); +} +} + +static const TypeInfo isapic_info = { +.name = TYPE_ISA_PIC, +.parent= TYPE_ISA_DEVICE, +.instance_size = sizeof(ISAPICState), +.instance_init = isapic_init, +}; --- a/include/hw/intc/i8259.h +++ b/include/hw/intc/i8259.h @@ -1,6 +1,20 @@ #ifndef HW_I8259_H #define HW_I8259_H +#include "qom/object.h" +#include "hw/isa/isa.h" +#include "qemu/typedefs.h" + +#define TYPE_ISA_PIC "isa-pic" +OBJECT_DECLARE_SIMPLE_TYPE(ISAPICState, ISA_PIC) + +struct ISAPICState { +ISADevice parent_obj; + +qemu_irq in_irqs[ISA_NUM_IRQS]; +qemu_irq out_irqs[ISA_NUM_IRQS]; +}; There is nothing I8259 / ISA specific in your model. What about adding a generic qdev proxy-irq (having a configurable number of IRQs to proxy)? See for example hw/core/split-irq.c. Regards, Phil.
Re: [PATCH v2 0/4] Add a new backend for cryptodev
On 2022/10/8 16:50, Lei He wrote: v1 --> v2: - Fix compile errors when neither 'nettle' nor 'gcrypt' are enabled. - Trivial changes to error codes when neither 'nettle' nor 'gcrypt' are enabled. Hi, lei: Daniel has reviewed the crypto part of this patch(thanks a lot for Daniel), can you help to have a glance at rest codes? Thanks. Best regards, Lei He -- helei.si...@bytedance.com
[PATCH v3] target/i386: Fix calculation of LOCK NEG eflags
In sequence: --- lock negl -0x14(%rbp) pushf pop%rax --- %rax will obtain the wrong value becasue the "lock neg" calculates the wrong eflags. The "s->T0" is updated by the wrong value. You can use this to do some test: --- #include int main() { __volatile__ unsigned test = 0x2363a; __volatile__ char cond = 0; asm( "lock negl %0 \n\t" "sets %1" : "=m"(test), "=r"(cond) : :); assert(cond & 1); return 0; } --- Reported-by: Jinyang Shen Co-Developed-by: Xuehai Chen Signed-off-by: Xuehai Chen Signed-off-by: Qi Hu --- V2 -> V3: Fix typo "caculation". V1 -> V2: Following Richard's suggestion, just change mov to neg instead of using local_tmp. --- target/i386/tcg/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index e19d5c1c64..cec2182080 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -3299,7 +3299,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) tcg_temp_free(t2); tcg_temp_free(a0); -tcg_gen_mov_tl(s->T0, t0); +tcg_gen_neg_tl(s->T0, t0); tcg_temp_free(t0); } else { tcg_gen_neg_tl(s->T0, s->T0); -- 2.38.0
[PULL 1/2] m68k: rework BI_VIRT_RNG_SEED as BI_RNG_SEED
From: "Jason A. Donenfeld" Following a change on the kernel side (see link), pass BI_RNG_SEED instead of BI_VIRT_RNG_SEED. This should have no impact on compatibility, as there will simply be no effect if it's an old kernel, which is how things have always been. We then use this as an opportunity to add this to q800, since now we can, which is a nice improvement. Cc: Geert Uytterhoeven Cc: Laurent Vivier Link: https://lore.kernel.org/lkml/20220923170340.4099226-3-ja...@zx2c4.com/ Signed-off-by: Jason A. Donenfeld Message-Id: <20220926113900.1256630-1-ja...@zx2c4.com> [lv: s/^I/ /g] Signed-off-by: Laurent Vivier --- include/standard-headers/asm-m68k/bootinfo-virt.h | 4 +++- include/standard-headers/asm-m68k/bootinfo.h | 8 +++- hw/m68k/q800.c| 7 +++ hw/m68k/virt.c| 8 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/include/standard-headers/asm-m68k/bootinfo-virt.h b/include/standard-headers/asm-m68k/bootinfo-virt.h index 1b1ffd4705d6..75ac6bbd7d73 100644 --- a/include/standard-headers/asm-m68k/bootinfo-virt.h +++ b/include/standard-headers/asm-m68k/bootinfo-virt.h @@ -12,7 +12,9 @@ #define BI_VIRT_GF_TTY_BASE0x8003 #define BI_VIRT_VIRTIO_BASE0x8004 #define BI_VIRT_CTRL_BASE 0x8005 -#define BI_VIRT_RNG_SEED 0x8006 + +/* No longer used -- replaced with BI_RNG_SEED -- but don't reuse this index: + * #define BI_VIRT_RNG_SEED0x8006 */ #define VIRT_BOOTI_VERSION MK_BI_VERSION(2, 0) diff --git a/include/standard-headers/asm-m68k/bootinfo.h b/include/standard-headers/asm-m68k/bootinfo.h index 7b790e8ec8d6..b7a8dd2514fe 100644 --- a/include/standard-headers/asm-m68k/bootinfo.h +++ b/include/standard-headers/asm-m68k/bootinfo.h @@ -57,7 +57,13 @@ struct mem_info { /* (struct mem_info) */ #define BI_COMMAND_LINE0x0007 /* kernel command line parameters */ /* (string) */ - +/* + * A random seed used to initialize the RNG. Record format: + * + * - length [ 2 bytes, 16-bit big endian ] + * - seed data[ `length` bytes, padded to preserve 4-byte struct alignment ] + */ +#define BI_RNG_SEED0x0008 /* * Linux/m68k Architectures (BI_MACHTYPE) diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c index 101ab0f803f6..a4590c2cb0b1 100644 --- a/hw/m68k/q800.c +++ b/hw/m68k/q800.c @@ -23,6 +23,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" #include "qemu/datadir.h" +#include "qemu/guest-random.h" #include "sysemu/sysemu.h" #include "cpu.h" #include "hw/boards.h" @@ -385,6 +386,7 @@ static void q800_init(MachineState *machine) NubusBus *nubus; DeviceState *glue; DriveInfo *dinfo; +uint8_t rng_seed[32]; linux_boot = (kernel_filename != NULL); @@ -634,6 +636,11 @@ static void q800_init(MachineState *machine) kernel_cmdline); } +/* Pass seed to RNG. */ +qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); +BOOTINFODATA(cs->as, parameters_base, BI_RNG_SEED, + rng_seed, sizeof(rng_seed)); + /* load initrd */ if (initrd_filename) { initrd_size = get_image_size(initrd_filename); diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c index 2f3ffc0de677..f7b903ea1b62 100644 --- a/hw/m68k/virt.c +++ b/hw/m68k/virt.c @@ -248,10 +248,10 @@ static void virt_init(MachineState *machine) kernel_cmdline); } - /* Pass seed to RNG. */ - qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); - BOOTINFODATA(cs->as, parameters_base, BI_VIRT_RNG_SEED, -rng_seed, sizeof(rng_seed)); +/* Pass seed to RNG. */ +qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); +BOOTINFODATA(cs->as, parameters_base, BI_RNG_SEED, + rng_seed, sizeof(rng_seed)); /* load initrd */ if (initrd_filename) { -- 2.37.3
Re: [RFC PATCH v2 2/8] vdpa: Save emulated features list in vhost_vdpa
On Mon, Oct 24, 2022 at 4:14 AM Jason Wang wrote: > > On Fri, Oct 21, 2022 at 4:56 PM Eugenio Perez Martin > wrote: > > > > On Fri, Oct 21, 2022 at 4:57 AM Jason Wang wrote: > > > > > > On Thu, Oct 20, 2022 at 2:34 PM Eugenio Perez Martin > > > wrote: > > > > > > > > On Thu, Oct 20, 2022 at 6:23 AM Jason Wang wrote: > > > > > > > > > > On Wed, Oct 19, 2022 at 8:52 PM Eugenio Pérez > > > > > wrote: > > > > > > > > > > > > At this moment only _F_LOG is added there. > > > > > > > > > > > > However future patches add features that depend on the kind of > > > > > > device. > > > > > > In particular, only net devices can add VIRTIO_F_GUEST_ANNOUNCE. So > > > > > > let's allow vhost_vdpa creator to set custom emulated device > > > > > > features. > > > > > > > > > > > > Signed-off-by: Eugenio Pérez > > > > > > --- > > > > > > include/hw/virtio/vhost-vdpa.h | 2 ++ > > > > > > hw/virtio/vhost-vdpa.c | 8 > > > > > > net/vhost-vdpa.c | 4 > > > > > > 3 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/include/hw/virtio/vhost-vdpa.h > > > > > > b/include/hw/virtio/vhost-vdpa.h > > > > > > index d85643..50083e1e3b 100644 > > > > > > --- a/include/hw/virtio/vhost-vdpa.h > > > > > > +++ b/include/hw/virtio/vhost-vdpa.h > > > > > > @@ -31,6 +31,8 @@ typedef struct vhost_vdpa { > > > > > > bool iotlb_batch_begin_sent; > > > > > > MemoryListener listener; > > > > > > struct vhost_vdpa_iova_range iova_range; > > > > > > +/* VirtIO device features that can be emulated by qemu */ > > > > > > +uint64_t added_features; > > > > > > > > > > Any reason we need a per vhost_vdpa storage for this? Or is there a > > > > > chance that this field could be different among the devices? > > > > > > > > > > > > > Yes, one device could support SVQ and the other one could not support > > > > it because of different feature sets for example. > > > > > > Right, but for those devices that don't support SVQ, we don't even > > > need mediation for feature like F_LOG and _F_STATUS? > > > > > > > No, and we cannot offer it to the guest either. > > Just to make sure we are on the same page, what I meant is, consider > in the future SVQ get the support of all features, so we can remove > this field? This is because _F_STATUS can be mediated unconditionally > anyhow. > For _F_STATUS that is right. But we cannot handle full _F_GUEST_ANNOUNCE since control SVQ (will) needs features from the device that cannot be emulated, like ASID. I think your point is "Since qemu cannot migrate these devices it will never set VIRTIO_NET_S_ANNOUNCE, so the guest will never send VIRTIO_NET_CTRL_ANNOUNCE messages". And I think that is totally right, but I still feel it is weird to expose it if we cannot handle it. Maybe a good first step is to move added_features to vhost_net, or maybe to convert it to "bool guest_announce_emulated" or something similar? This way hw/virtio/vhost-vdpa is totally unaware of this and changes are more self contained. Thanks! > Thanks > > > > > > Thanks > > > > > > > > > > > Thanks! > > > > > > > > > Thanks > > > > > > > > > > > uint64_t acked_features; > > > > > > bool shadow_vqs_enabled; > > > > > > /* IOVA mapping used by the Shadow Virtqueue */ > > > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > > > > > index 7468e44b87..ddb5e29288 100644 > > > > > > --- a/hw/virtio/vhost-vdpa.c > > > > > > +++ b/hw/virtio/vhost-vdpa.c > > > > > > @@ -660,8 +660,8 @@ static int vhost_vdpa_set_features(struct > > > > > > vhost_dev *dev, > > > > > > > > > > > > v->acked_features = features; > > > > > > > > > > > > -/* We must not ack _F_LOG if SVQ is enabled */ > > > > > > -features &= ~BIT_ULL(VHOST_F_LOG_ALL); > > > > > > +/* Do not ack features emulated by qemu */ > > > > > > +features &= ~v->added_features; > > > > > > } > > > > > > > > > > > > trace_vhost_vdpa_set_features(dev, features); > > > > > > @@ -1244,8 +1244,8 @@ static int vhost_vdpa_get_features(struct > > > > > > vhost_dev *dev, > > > > > > int ret = vhost_vdpa_get_dev_features(dev, features); > > > > > > > > > > > > if (ret == 0 && v->shadow_vqs_enabled) { > > > > > > -/* Add SVQ logging capabilities */ > > > > > > -*features |= BIT_ULL(VHOST_F_LOG_ALL); > > > > > > +/* Add emulated capabilities */ > > > > > > +*features |= v->added_features; > > > > > > } > > > > > > > > > > > > return ret; > > > > > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > > > > > > index eebf29f5c1..3803452800 100644 > > > > > > --- a/net/vhost-vdpa.c > > > > > > +++ b/net/vhost-vdpa.c > > > > > > @@ -599,6 +599,10 @@ static NetClientState > > > > > > *net_vhost_vdpa_init(NetClientState *peer, > > > > > > s->vhost_vdpa.index = queue_pair_index; > > > > > > s->vhost_vdpa.shadow_vqs_enabled = svq; > > > > > > s->vhost_vdpa.iova_tree =
Re: [PATCH] treewide: Remove the unnecessary space before semicolon
Hllo Bin Meng, I ACK the change for SJA1000. On Monday 24 of October 2022 09:28:02 Bin Meng wrote: > %s/return ;/return; > > Signed-off-by: Bin Meng > --- > > include/hw/elf_ops.h | 2 +- > hw/9pfs/9p.c | 2 +- > hw/dma/pl330.c | 2 +- > hw/net/can/can_sja1000.c | 2 +- Acked-by: Pave Pisa > hw/timer/renesas_cmt.c | 2 +- > hw/timer/renesas_tmr.c | 8 > hw/virtio/virtio-pci.c | 2 +- > target/riscv/vector_helper.c | 2 +- > target/rx/op_helper.c| 4 ++-- > ui/vnc-jobs.c| 2 +- > ui/vnc.c | 2 +- > 11 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c > index e0f76d3eb3..73201f9139 100644 > --- a/hw/net/can/can_sja1000.c > +++ b/hw/net/can/can_sja1000.c > @@ -431,7 +431,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr addr, > uint64_t val, (unsigned long long)val, (unsigned int)addr); > > if (addr > CAN_SJA_MEM_SIZE) { > -return ; > +return; > } > > if (s->clock & 0x80) { /* PeliCAN Mode */ -- Pavel Pisa phone: +420 603531357 e-mail: p...@cmp.felk.cvut.cz Department of Control Engineering FEE CVUT Karlovo namesti 13, 121 35, Prague 2 university: http://control.fel.cvut.cz/ personal: http://cmp.felk.cvut.cz/~pisa projects: https://www.openhub.net/accounts/ppisa CAN related:http://canbus.pages.fel.cvut.cz/ RISC-V education: https://comparch.edu.cvut.cz/ Open Technologies Research Education and Exchange Services https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home
[PULL 2/2] m68k: write bootinfo as rom section and re-randomize on reboot
From: "Jason A. Donenfeld" Rather than poking directly into RAM, add the bootinfo block as a proper ROM, so that it's restored when rebooting the system. This way, if the guest corrupts any of the bootinfo items, but then tries to reboot, it'll still be restored back to normal as expected. Then, since the RNG seed needs to be fresh on each boot, regenerate the RNG seed in the ROM when reseting the CPU. Cc: Geert Uytterhoeven Cc: Laurent Vivier Signed-off-by: Jason A. Donenfeld Message-Id: <20221023191340.36238-1-ja...@zx2c4.com> Signed-off-by: Laurent Vivier --- hw/m68k/bootinfo.h | 48 +++ hw/m68k/q800.c | 71 +- hw/m68k/virt.c | 51 +++-- 3 files changed, 111 insertions(+), 59 deletions(-) diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h index 897162b8189c..a3d37e3c8094 100644 --- a/hw/m68k/bootinfo.h +++ b/hw/m68k/bootinfo.h @@ -12,66 +12,66 @@ #ifndef HW_M68K_BOOTINFO_H #define HW_M68K_BOOTINFO_H -#define BOOTINFO0(as, base, id) \ +#define BOOTINFO0(base, id) \ do { \ -stw_phys(as, base, id); \ +stw_p(base, id); \ base += 2; \ -stw_phys(as, base, sizeof(struct bi_record)); \ +stw_p(base, sizeof(struct bi_record)); \ base += 2; \ } while (0) -#define BOOTINFO1(as, base, id, value) \ +#define BOOTINFO1(base, id, value) \ do { \ -stw_phys(as, base, id); \ +stw_p(base, id); \ base += 2; \ -stw_phys(as, base, sizeof(struct bi_record) + 4); \ +stw_p(base, sizeof(struct bi_record) + 4); \ base += 2; \ -stl_phys(as, base, value); \ +stl_p(base, value); \ base += 4; \ } while (0) -#define BOOTINFO2(as, base, id, value1, value2) \ +#define BOOTINFO2(base, id, value1, value2) \ do { \ -stw_phys(as, base, id); \ +stw_p(base, id); \ base += 2; \ -stw_phys(as, base, sizeof(struct bi_record) + 8); \ +stw_p(base, sizeof(struct bi_record) + 8); \ base += 2; \ -stl_phys(as, base, value1); \ +stl_p(base, value1); \ base += 4; \ -stl_phys(as, base, value2); \ +stl_p(base, value2); \ base += 4; \ } while (0) -#define BOOTINFOSTR(as, base, id, string) \ +#define BOOTINFOSTR(base, id, string) \ do { \ int i; \ -stw_phys(as, base, id); \ +stw_p(base, id); \ base += 2; \ -stw_phys(as, base, \ +stw_p(base, \ (sizeof(struct bi_record) + strlen(string) + \ 1 /* null termination */ + 3 /* padding */) & ~3); \ base += 2; \ for (i = 0; string[i]; i++) { \ -stb_phys(as, base++, string[i]); \ +stb_p(base++, string[i]); \ } \ -stb_phys(as, base++, 0); \ -base = (base + 3) & ~3; \ +stb_p(base++, 0); \ +base = QEMU_ALIGN_PTR_UP(base, 4); \ } while (0) -#define BOOTINFODATA(as, base, id, data, len) \ +#define BOOTINFODATA(base, id, data, len) \ do { \ int i; \ -stw_phys(as, base, id); \ +stw_p(base, id); \ base += 2; \ -stw_phys(as, base, \ +stw_p(base, \ (sizeof(struct bi_record) + len + \ 2 /* length field */ + 3 /* padding */) & ~3); \ base += 2; \ -stw_phys(as, base, len); \ +stw_p(base, len); \ base += 2; \ for (i = 0; i < len; ++i) { \ -stb_phys(as, base++, data[i]); \ +stb_p(base++, data[i]); \ } \ -base = (base + 3) & ~3; \ +base = QEMU_ALIGN_PTR_UP(base, 4); \ } while (0) #endif diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c index a4590c2cb0b1..e09e244ddc1d 100644 --- a/hw/m68k/q800.c +++ b/hw/m68k/q800.c @@ -321,11 +321,22 @@ static const TypeInfo glue_info = { }, }; +typedef struct { +M68kCPU *cpu; +struct bi_record *rng_seed; +} ResetInfo; + static void main_cpu_reset(void *opaque) { -M68kCPU *cpu = opaque; +ResetInfo *reset_info = opaque; +M68kCPU *cpu = reset_info->cpu; CPUState *cs = CPU(cpu); +if (reset_info->rng_seed) { +qemu_guest_getrandom_nofail((void *)reset_info->rng_seed->data + 2, +be16_to_cpu(*(uint16_t *)reset_info->rng_seed->data)); +} + cpu_reset(cs); cpu->env.aregs[7] = ldl_phys(cs->as, 0); cpu->env.pc = ldl_phys(cs->as, 4); @@ -386,6 +397,7 @@ static void q800_init(MachineState *machine) NubusBus *nubus; DeviceState *glue; DriveInfo *dinfo; +ResetInfo *reset_info; uint8_t rng_seed[32]; linux_boot = (kernel_filename != NULL); @@ -396,9 +408,12 @@ static void q800_init(MachineState *machine) exit(1); } +reset_info = g_new0(ResetInfo, 1); + /* init CPUs */ cpu = M68K_CPU(cpu_create(machine->cpu_type)); -qemu_register_reset(main
Re: [PATCH] avocado: use sha1 for fc31 imgs to avoid first time re-download
On Sat, Oct 22, 2022 at 02:03:50PM -0300, Daniel Henrique Barboza wrote: > 'make check-avocado' will download any images that aren't present in the > cache via 'get-vm-images' in tests/Makefile.include. The target that > downloads fedora 31 images, get-vm-image-fedora-31, will use 'avocado > vmimage get --distro=fedora --distro-version=31 --arch=(...)' to > download the image for each arch. Note that this command does not > support any argument to set the hash algorithm used and, based on the > avocado source code [1], DEFAULT_HASH_ALGORITHM is set to "sha1". The > sha1 hash is stored in a Fedora-Cloud-Base-31-1.9.{ARCH}.qcow2-CHECKSUM > in the cache. > For now, in QEMU, let's use sha1 for all Fedora 31 images. This will > immediately spares us at least one extra download for each Fedora 31 > image that we're doing in all our CI runs. > > [1] https://github.com/avocado-framework/avocado.git @ 942a5d6972906 > [2] https://github.com/avocado-framework/avocado/issues/5496 Can we just ask Avocado maintainers to fix this problem on their side to allow use of a modern hash alg as a priority item. We've already had this problem in QEMU for over a year AFAICT, so doesn't seem like we need to urgently do a workaround on QEMU side, so we can get Avocado devs to commit to fixing it in the next month. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v10 1/9] s390x/cpu topology: core_id sets s390x CPU topology
On 10/19/22 17:39, Pierre Morel wrote: On 10/18/22 18:43, Cédric Le Goater wrote: Hello Pierre, On 10/12/22 18:20, Pierre Morel wrote: In the S390x CPU topology the core_id specifies the CPU address and the position of the core withing the topology. Let's build the topology based on the core_id. s390x/cpu topology: core_id sets s390x CPU topology In the S390x CPU topology the core_id specifies the CPU address and the position of the cpu withing the topology. Let's build the topology based on the core_id. The commit log is doubled. Yes, thanks. Signed-off-by: Pierre Morel --- include/hw/s390x/cpu-topology.h | 45 +++ hw/s390x/cpu-topology.c | 132 hw/s390x/s390-virtio-ccw.c | 21 + hw/s390x/meson.build | 1 + 4 files changed, 199 insertions(+) create mode 100644 include/hw/s390x/cpu-topology.h create mode 100644 hw/s390x/cpu-topology.c diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h new file mode 100644 index 00..66c171d0bc --- /dev/null +++ b/include/hw/s390x/cpu-topology.h @@ -0,0 +1,45 @@ +/* + * CPU Topology + * + * Copyright 2022 IBM Corp. + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at + * your option) any later version. See the COPYING file in the top-level + * directory. + */ +#ifndef HW_S390X_CPU_TOPOLOGY_H +#define HW_S390X_CPU_TOPOLOGY_H + +#include "hw/qdev-core.h" +#include "qom/object.h" + +typedef struct S390TopoContainer { + int active_count; +} S390TopoContainer; This structure does not seem very useful. + +#define S390_TOPOLOGY_CPU_IFL 0x03 +#define S390_TOPOLOGY_MAX_ORIGIN ((63 + S390_MAX_CPUS) / 64) +typedef struct S390TopoTLE { The 'Topo' is redundant as TLE stands for 'topology-list entry'. This is minor. + uint64_t mask[S390_TOPOLOGY_MAX_ORIGIN]; +} S390TopoTLE; + +struct S390Topology { + SysBusDevice parent_obj; + int cpus; + S390TopoContainer *socket; + S390TopoTLE *tle; + MachineState *ms; hmm, it would be cleaner to introduce the fields and properties needed by the S390Topology model and avoid dragging the machine object pointer. AFAICT, these properties would be : "nr-cpus" "max-cpus" "nr-sockets" OK +}; + +#define TYPE_S390_CPU_TOPOLOGY "s390-topology" +OBJECT_DECLARE_SIMPLE_TYPE(S390Topology, S390_CPU_TOPOLOGY) + +S390Topology *s390_get_topology(void); +void s390_topology_new_cpu(int core_id); + +static inline bool s390_has_topology(void) +{ + return false; +} + +#endif diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c new file mode 100644 index 00..42b22a1831 --- /dev/null +++ b/hw/s390x/cpu-topology.c @@ -0,0 +1,132 @@ +/* + * CPU Topology + * + * Copyright IBM Corp. 2022 The Copyright tag is different in the .h file. OK, I change this to be like in the header file it seems to be the most used format. + * Author(s): Pierre Morel + + * This work is licensed under the terms of the GNU GPL, version 2 or (at + * your option) any later version. See the COPYING file in the top-level + * directory. + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "qemu/error-report.h" +#include "hw/sysbus.h" +#include "hw/qdev-properties.h" +#include "hw/boards.h" +#include "qemu/typedefs.h" +#include "target/s390x/cpu.h" +#include "hw/s390x/s390-virtio-ccw.h" +#include "hw/s390x/cpu-topology.h" + +S390Topology *s390_get_topology(void) +{ + static S390Topology *s390Topology; + + if (!s390Topology) { + s390Topology = S390_CPU_TOPOLOGY( + object_resolve_path(TYPE_S390_CPU_TOPOLOGY, NULL)); + } + + return s390Topology; I am not convinced this routine is useful. The s390Topology pointer could be stored under the machine state I think. It wouldn't be a problem when CPUs are hot plugged since we have access to the machine in the hot plug handler. OK, I add a pointer to the machine state that will be initialised during s390_init_topology() LGTM. For the stsi call, 'struct ArchCPU' probably lacks a back pointer to the machine objects with which CPU interact. These are typically interrupt controllers or this new s390Topology model. You could add the pointer there or, better, under a generic 'void *opaque' attribute. That said, what you did works fine. The modeling could be cleaner. Yes. I think you are right and I add a opaque pointer to the topology. As an example, you could look at PPC where the PowerPCCPU CPU model is shared between two differents machine, a baremetal one PowerNV and the para-virtual one pSeries/sPAPR. Look for : pnv_cpu_state(PowerPCCPU *cpu) spapr_cpu_state(PowerPCCPU *cpu) the machine CPU state is stored under an opaque cpu->machine_data which is specific to each machine. It doesn't have to be as complex on s390 since we only have one type of z-machine. An opaque is a good idea still. Thanks, C.
[PATCH 2/2] hw/ide/piix: Ignore writes of hardwired PCI command register bits
One method to enable PCI bus mastering for IDE controllers, often used by x86 firmware, is to write 0x7 to the PCI command register. Neither the PIIX 3/4 specifications nor actual PIIX 3 hardware (a Tyan S1686D system) permit setting the Memory Space Enable (MSE) bit, thus the command register would be left in an unspecified state without this patch. * hw/core/machine.c Facilitate migration by not masking writes to the PIIX 3/4 PCICMD register for machine states of QEMU versions prior to 7.2. * hw/ide/piix.c a) Add a reference to the PIIX 4 data sheet. b) Mask the MSE bit using the QEMU PCI device wmask field. c) Define a new boolean property, x-filter-pcicmd, to control whether the write mask is enabled and enable it by default for both the PIIX 3 and PIIX 4 IDE controllers. * include/hw/ide/pci.h Add the boolean filter_pcicmd field to the PCIIDEState structure, because the PIIX IDE controllers do not define their own state. * tests/qtest/ide-test.c Use the command_disabled field of the QPCIDevice testing model to indicate that PCI_COMMAND_MEMORY is hardwired within PIIX 3/4 IDE controllers. Signed-off-by: Lev Kujawski --- hw/core/machine.c | 5 - hw/ide/piix.c | 24 include/hw/ide/pci.h | 1 + tests/qtest/ide-test.c | 1 + 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index aa520e74a8..8e8e69c081 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -40,7 +40,10 @@ #include "hw/virtio/virtio-pci.h" #include "qom/object_interfaces.h" -GlobalProperty hw_compat_7_1[] = {}; +GlobalProperty hw_compat_7_1[] = { +{ "piix3-ide", "x-filter-pcicmd", "false" }, +{ "piix4-ide", "x-filter-pcicmd", "false" }, +}; const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1); GlobalProperty hw_compat_7_0[] = { diff --git a/hw/ide/piix.c b/hw/ide/piix.c index de1f4f0efb..a3af32e126 100644 --- a/hw/ide/piix.c +++ b/hw/ide/piix.c @@ -25,6 +25,8 @@ * References: * [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR, * 290550-002, Intel Corporation, April 1997. + * [2] 82371AB PCI-TO-ISA / IDE XCELERATOR (PIIX4), 290562-001, + * Intel Corporation, April 1997. */ #include "qemu/osdep.h" @@ -160,6 +162,21 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp) uint8_t *pci_conf = dev->config; int rc; +/* + * Mask all IDE PCI command register bits except for Bus Master + * Function Enable (bit 2) and I/O Space Enable (bit 0), as the + * remainder are hardwired to 0 [1, p.48] [2, p.89-90]. + * + * NOTE: According to the PIIX3 datasheet [1], the Memory Space + * Enable (MSE, bit 1) is hardwired to 1, but this is contradicted + * by actual PIIX3 hardware, the datasheet itself (viz., Default + * Value: h), and the PIIX4 datasheet [2]. + */ +if (d->filter_pcicmd) { +pci_set_word(dev->wmask + PCI_COMMAND, + PCI_COMMAND_MASTER | PCI_COMMAND_IO); +} + pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode bmdma_setup_bar(d); @@ -185,6 +202,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev) } } +static Property pci_piix_ide_properties[] = { +DEFINE_PROP_BOOL("x-filter-pcicmd", PCIIDEState, filter_pcicmd, TRUE), +DEFINE_PROP_END_OF_LIST(), +}; + /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */ static void piix3_ide_class_init(ObjectClass *klass, void *data) { @@ -198,6 +220,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data) k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1; k->class_id = PCI_CLASS_STORAGE_IDE; set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); +device_class_set_props(dc, pci_piix_ide_properties); dc->hotpluggable = false; } @@ -220,6 +243,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data) k->device_id = PCI_DEVICE_ID_INTEL_82371AB; k->class_id = PCI_CLASS_STORAGE_IDE; set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); +device_class_set_props(dc, pci_piix_ide_properties); dc->hotpluggable = false; } diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h index d8384e1c42..5424b00a9e 100644 --- a/include/hw/ide/pci.h +++ b/include/hw/ide/pci.h @@ -53,6 +53,7 @@ struct PCIIDEState { MemoryRegion bmdma_bar; MemoryRegion cmd_bar[2]; MemoryRegion data_bar[2]; +bool filter_pcicmd; /* used only for piix3/4 */ }; static inline IDEState *bmdma_active_if(BMDMAState *bmdma) diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c index dbe1563b23..d5cec7c14c 100644 --- a/tests/qtest/ide-test.c +++ b/tests/qtest/ide-test.c @@ -174,6 +174,7 @@ static QPCIDevice *get_pci_device(QTestState *qts, QPCIBar *bmdma_bar, *ide_bar = qpci_legacy_iomap(dev, IDE_BASE); +dev->command_disabled = PCI_COMMAND_MEMORY; qpci_device_enable(dev); return dev; -- 2.34.1
[Bug 1994002] Re: [SRU] migration was active, but no RAM info was set
The attachment "lp1994002-qemu-ussuri.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team. [This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.] ** Tags added: patch -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1994002 Title: [SRU] migration was active, but no RAM info was set Status in Ubuntu Cloud Archive: New Status in Ubuntu Cloud Archive ussuri series: New Status in QEMU: New Status in qemu package in Ubuntu: New Status in qemu source package in Focal: New Status in qemu source package in Jammy: New Status in qemu source package in Kinetic: New Bug description: While live-migrating many instances concurrently, libvirt sometimes return internal error: migration was active, but no RAM info was set: ~~~ 2022-03-30 06:08:37.197 7 WARNING nova.virt.libvirt.driver [req-5c3296cf-88ee-4af6-ae6a-ddba99935e23 - - - - -] [instance: af339c99-1182-4489-b15c-21e52f50f724] Error monitoring migration: internal error: migration was active, but no RAM info was set: libvirt.libvirtError: internal error: migration was active, but no RAM info was set ~~~ From upstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=2074205 [Impact] * Effects of this bug are mostly observed in large scale clusters with a lot of live migration activity. * Has second order effects for consumers of migration monitor such as libvirt and openstack. [Test Case] Steps to Reproduce: 1. live evacuate a compute 2. live migration of one or more instances fails with the above error N.B Due to the nature of this bug it is difficult consistently reproduce. [Where problems could occur] * In the event of a regression the migration monitor may report an inconsistent state. To manage notifications about this bug go to: https://bugs.launchpad.net/cloud-archive/+bug/1994002/+subscriptions
Re: [PATCH v3 2/2] hw/ide/piix: Ignore writes of hardwired PCI command register bits
> I guess this cna work but what I had in mind is much > simpler. Add an internal property (name starting with "x-") > enabling the buggy behaviour and set it in hw compat array. > If set - do not touch the wmask register. > > post load hooks are harder to reason about. Thanks again for the review and clarification, please find attached an updated patch. My only concern with the internal property approach is a potential proliferation of similar boolean values if someone else encounters an incompatibility. I have not conducted a thorough audit of all the PIIX 3/4 IDE registers for hardwired bits (only what I encountered testing proprietary firmware - PCICMD), and I do not have access to my PIIX 3 system at the moment. Kind regards, Lev Kujawski Lev Kujawski (2): qpci_device_enable: Allow for command bits hardwired to 0 hw/ide/piix: Ignore writes of hardwired PCI command register bits hw/core/machine.c| 5 - hw/ide/piix.c| 24 include/hw/ide/pci.h | 1 + tests/qtest/ide-test.c | 1 + tests/qtest/libqos/pci.c | 13 +++-- tests/qtest/libqos/pci.h | 1 + 6 files changed, 38 insertions(+), 7 deletions(-) -- 2.34.1
Re: [PATCH] treewide: Remove the unnecessary space before semicolon
On Mon, Oct 24, 2022 at 03:28:02PM +0800, Bin Meng wrote: > %s/return ;/return; > > Signed-off-by: Bin Meng ack for virtio trivial tree pls > --- > > include/hw/elf_ops.h | 2 +- > hw/9pfs/9p.c | 2 +- > hw/dma/pl330.c | 2 +- > hw/net/can/can_sja1000.c | 2 +- > hw/timer/renesas_cmt.c | 2 +- > hw/timer/renesas_tmr.c | 8 > hw/virtio/virtio-pci.c | 2 +- > target/riscv/vector_helper.c | 2 +- > target/rx/op_helper.c| 4 ++-- > ui/vnc-jobs.c| 2 +- > ui/vnc.c | 2 +- > 11 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index 7c3b1d0f6c..fbe0b1e956 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -117,7 +117,7 @@ static void glue(load_symbols, SZ)(struct elfhdr *ehdr, > int fd, int must_swab, > shdr_table = load_at(fd, ehdr->e_shoff, > sizeof(struct elf_shdr) * ehdr->e_shnum); > if (!shdr_table) { > -return ; > +return; > } > > if (must_swab) { > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index aebadeaa03..76c591a01b 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -1786,7 +1786,7 @@ static void coroutine_fn v9fs_walk(void *opaque) > err = pdu_unmarshal(pdu, offset, "ddw", &fid, &newfid, &nwnames); > if (err < 0) { > pdu_complete(pdu, err); > -return ; > +return; > } > offset += err; > > diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c > index 08e5938ec7..e5d521c329 100644 > --- a/hw/dma/pl330.c > +++ b/hw/dma/pl330.c > @@ -1328,7 +1328,7 @@ static void pl330_debug_exec(PL330State *s) > } > if (!insn) { > pl330_fault(ch, PL330_FAULT_UNDEF_INSTR | PL330_FAULT_DBG_INSTR); > -return ; > +return; > } > ch->stall = 0; > insn->exec(ch, opcode, args, insn->size - 1); > diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c > index e0f76d3eb3..73201f9139 100644 > --- a/hw/net/can/can_sja1000.c > +++ b/hw/net/can/can_sja1000.c > @@ -431,7 +431,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr addr, > uint64_t val, > (unsigned long long)val, (unsigned int)addr); > > if (addr > CAN_SJA_MEM_SIZE) { > -return ; > +return; > } > > if (s->clock & 0x80) { /* PeliCAN Mode */ > diff --git a/hw/timer/renesas_cmt.c b/hw/timer/renesas_cmt.c > index 2e0fd21a36..69eabc678a 100644 > --- a/hw/timer/renesas_cmt.c > +++ b/hw/timer/renesas_cmt.c > @@ -57,7 +57,7 @@ static void update_events(RCMTState *cmt, int ch) > > if ((cmt->cmstr & (1 << ch)) == 0) { > /* count disable, so not happened next event. */ > -return ; > +return; > } > next_time = cmt->cmcor[ch] - cmt->cmcnt[ch]; > next_time *= NANOSECONDS_PER_SECOND; > diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c > index d96002e1ee..c15f654738 100644 > --- a/hw/timer/renesas_tmr.c > +++ b/hw/timer/renesas_tmr.c > @@ -67,18 +67,18 @@ static void update_events(RTMRState *tmr, int ch) > int i, event; > > if (tmr->tccr[ch] == 0) { > -return ; > +return; > } > if (FIELD_EX8(tmr->tccr[ch], TCCR, CSS) == 0) { > /* external clock mode */ > /* event not happened */ > -return ; > +return; > } > if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CSS_CASCADING) { > /* cascading mode */ > if (ch == 1) { > tmr->next[ch] = none; > -return ; > +return; > } > diff[cmia] = concat_reg(tmr->tcora) - concat_reg(tmr->tcnt); > diff[cmib] = concat_reg(tmr->tcorb) - concat_reg(tmr->tcnt); > @@ -384,7 +384,7 @@ static void timer_events(RTMRState *tmr, int ch) > tmr->tcorb[ch]) & 0xff; > } else { > if (ch == 1) { > -return ; > +return; > } > tcnt = issue_event(tmr, ch, 16, > concat_reg(tmr->tcnt), > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index e7d80242b7..34db51e241 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1675,7 +1675,7 @@ static void virtio_pci_device_plugged(DeviceState *d, > Error **errp) > if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { > error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by" > " neither legacy nor transitional device"); > -return ; > +return; > } > /* > * Legacy and transitional devices use specific subsystem IDs. > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c > index b94f809eb3..0020b9a95d 100644 > --- a/target/riscv/vector_helper.c > +++ b/target/riscv/vector_helper.c > @@ -211,7 +211,7 @@ static void vext_set_elems_1s(void *base,
Re: [PATCH] accel/tcg/tcg-accel-ops-rr: fix trivial typo
On 10/21/22 19:36, Matheus Tavares Bernardino wrote: > Signed-off-by: Matheus Tavares Bernardino > --- > accel/tcg/tcg-accel-ops-rr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c > index cc8adc2380..cc912df108 100644 > --- a/accel/tcg/tcg-accel-ops-rr.c > +++ b/accel/tcg/tcg-accel-ops-rr.c > @@ -51,7 +51,7 @@ void rr_kick_vcpu_thread(CPUState *unused) > * > * The kick timer is responsible for moving single threaded vCPU > * emulation on to the next vCPU. If more than one vCPU is running a > - * timer event with force a cpu->exit so the next vCPU can get > + * timer event we force a cpu->exit so the next vCPU can get > * scheduled. > * > * The timer is removed if all vCPUs are idle and restarted again once Reviewed-by: Claudio Fontana
Re: [PATCH v3 1/2] qpci_device_enable: Allow for command bits hardwired to 0
Michael S. Tsirkin writes: > On Sun, Sep 25, 2022 at 09:37:58AM +, Lev Kujawski wrote: >> Devices like the PIIX3/4 IDE controller do not support certain modes >> of operation, such as memory space accesses, and indicate this lack of >> support by hardwiring the applicable bits to zero. Extend the QEMU >> PCI device testing framework to accommodate such devices. >> >> * tests/qtest/libqos/pci.h: Add the command_disabled word to indicate >> bits hardwired to 0. >> * tests/qtest/libqos/pci.c: Verify that hardwired bits are actually >> hardwired. >> >> Signed-off-by: Lev Kujawski >> --- >> tests/qtest/libqos/pci.c | 13 +++-- >> tests/qtest/libqos/pci.h | 1 + >> 2 files changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c >> index b23d72346b..4f3d28d8d9 100644 >> --- a/tests/qtest/libqos/pci.c >> +++ b/tests/qtest/libqos/pci.c >> @@ -220,18 +220,19 @@ int qpci_secondary_buses_init(QPCIBus *bus) >> >> void qpci_device_enable(QPCIDevice *dev) >> { >> -uint16_t cmd; >> +const uint16_t enable_bits = >> +PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; >> +uint16_t cmd, new_cmd; >> >> /* FIXME -- does this need to be a bus callout? */ >> cmd = qpci_config_readw(dev, PCI_COMMAND); >> -cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; >> +cmd |= enable_bits; >> qpci_config_writew(dev, PCI_COMMAND, cmd); >> >> /* Verify the bits are now set. */ >> -cmd = qpci_config_readw(dev, PCI_COMMAND); >> -g_assert_cmphex(cmd & PCI_COMMAND_IO, ==, PCI_COMMAND_IO); >> -g_assert_cmphex(cmd & PCI_COMMAND_MEMORY, ==, PCI_COMMAND_MEMORY); >> -g_assert_cmphex(cmd & PCI_COMMAND_MASTER, ==, PCI_COMMAND_MASTER); >> +new_cmd = qpci_config_readw(dev, PCI_COMMAND); >> +new_cmd &= enable_bits; >> +g_assert_cmphex(new_cmd, ==, enable_bits & ~dev->command_disabled); >> } >> >> /** >> diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h >> index 8389614523..eaedb98588 100644 >> --- a/tests/qtest/libqos/pci.h >> +++ b/tests/qtest/libqos/pci.h >> @@ -68,6 +68,7 @@ struct QPCIDevice >> bool msix_enabled; >> QPCIBar msix_table_bar, msix_pba_bar; >> uint64_t msix_table_off, msix_pba_off; >> +uint16_t command_disabled; > > > Can we get this from device's wmask? > I have not seen any way to pass the wmask from the underlying PCI device without violating the abstraction of the driver testing framework. Another approach might be to omit the verification of the PCI command bits in the assumption that some filtering mechanism like wmask is active, but I think the advantage of this patch is that it makes the expected (albeit abnormal) behavior explicit in the device test. Kind regards, Lev Kujawski
[PATCH 1/2] qpci_device_enable: Allow for command bits hardwired to 0
Devices like the PIIX3/4 IDE controller do not support certain modes of operation, such as memory space accesses, and indicate this lack of support by hardwiring the applicable bits to zero. Extend the QEMU PCI device testing framework to accommodate such devices. * tests/qtest/libqos/pci.h: Add the command_disabled word to indicate bits hardwired to 0. * tests/qtest/libqos/pci.c: Verify that hardwired bits are actually hardwired. Signed-off-by: Lev Kujawski --- tests/qtest/libqos/pci.c | 13 +++-- tests/qtest/libqos/pci.h | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c index b23d72346b..4f3d28d8d9 100644 --- a/tests/qtest/libqos/pci.c +++ b/tests/qtest/libqos/pci.c @@ -220,18 +220,19 @@ int qpci_secondary_buses_init(QPCIBus *bus) void qpci_device_enable(QPCIDevice *dev) { -uint16_t cmd; +const uint16_t enable_bits = +PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; +uint16_t cmd, new_cmd; /* FIXME -- does this need to be a bus callout? */ cmd = qpci_config_readw(dev, PCI_COMMAND); -cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; +cmd |= enable_bits; qpci_config_writew(dev, PCI_COMMAND, cmd); /* Verify the bits are now set. */ -cmd = qpci_config_readw(dev, PCI_COMMAND); -g_assert_cmphex(cmd & PCI_COMMAND_IO, ==, PCI_COMMAND_IO); -g_assert_cmphex(cmd & PCI_COMMAND_MEMORY, ==, PCI_COMMAND_MEMORY); -g_assert_cmphex(cmd & PCI_COMMAND_MASTER, ==, PCI_COMMAND_MASTER); +new_cmd = qpci_config_readw(dev, PCI_COMMAND); +new_cmd &= enable_bits; +g_assert_cmphex(new_cmd, ==, enable_bits & ~dev->command_disabled); } /** diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h index 8389614523..eaedb98588 100644 --- a/tests/qtest/libqos/pci.h +++ b/tests/qtest/libqos/pci.h @@ -68,6 +68,7 @@ struct QPCIDevice bool msix_enabled; QPCIBar msix_table_bar, msix_pba_bar; uint64_t msix_table_off, msix_pba_off; +uint16_t command_disabled; }; struct QPCIAddress { -- 2.34.1
Re: [PATCH v2] target/i386: Fix caculation of LOCK NEG eflags
On 2022/10/24 05:02, Philippe Mathieu-Daudé wrote: Typo "calculation" in subject. Thanks for the reminder. It's my fault. I will send V3 to fix this typo. Qi On 22/10/22 08:12, Qi Hu wrote: In sequence: --- lock negl -0x14(%rbp) pushf pop %rax --- %rax will obtain the wrong value becasue the "lock neg" caculates the wrong eflags. The "s->T0" is updated by the wrong value. You can use this to do some test: --- #include int main() { __volatile__ unsigned test = 0x2363a; __volatile__ char cond = 0; asm( "lock negl %0 \n\t" "sets %1" : "=m"(test), "=r"(cond) : :); assert(cond & 1); return 0; } --- Reported-by: Jinyang Shen Co-Developed-by: Xuehai Chen Signed-off-by: Xuehai Chen Signed-off-by: Qi Hu --- V1 -> V2: Following Richard's suggestion, just change mov to neg instead of using local_tmp. --- target/i386/tcg/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: [PATCH] ui: remove useless typecasts
On 10/22/22 16:12, Volker Rümelin wrote: > Commit 8f9abdf586 ("chardev: src buffer const for write functions") > changed the type of the second parameter of qemu_chr_be_write() > from uint8_t * to const uint8_t *. Remove the now useless type > casts from qemu_chr_be_write() function calls in ui/console.c and > ui/gtk.c. > > Cc: qemu-triv...@nongnu.org > Signed-off-by: Volker Rümelin Reviewed-by: Claudio Fontana > --- > ui/console.c | 2 +- > ui/gtk.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ui/console.c b/ui/console.c > index 49da6a91df..65c117874c 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -1297,7 +1297,7 @@ static void kbd_send_chars(QemuConsole *s) > uint32_t size; > > buf = fifo8_pop_buf(&s->out_fifo, MIN(len, avail), &size); > -qemu_chr_be_write(s->chr, (uint8_t *)buf, size); > +qemu_chr_be_write(s->chr, buf, size); > len = qemu_chr_be_can_write(s->chr); > avail -= size; > } > diff --git a/ui/gtk.c b/ui/gtk.c > index 92daaa6a6e..7ec21f7798 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -1763,7 +1763,7 @@ static void gd_vc_send_chars(VirtualConsole *vc) > uint32_t size; > > buf = fifo8_pop_buf(&vc->vte.out_fifo, MIN(len, avail), &size); > -qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size); > +qemu_chr_be_write(vc->vte.chr, buf, size); > len = qemu_chr_be_can_write(vc->vte.chr); > avail -= size; > }
Re: [PATCH v2 2/2] util/log: Always send errors to logfile when daemonized
Paolo Bonzini writes: >> If we want to connect stdout/err to something when daemonized >> then lets either have a dedicated option for that, or simply >> tell apps not to use -daemonize and to take care of daemonzing >> themselves, thus having full control over stdout/err. The latter >> is what libvirt uses, because we actually want stderr/out on a >> pipe, not a file, in order to enforce rollover. > > I would gladly get rid of -daemonize, unfortunately it has many users. > Adding further complication to it is not beautiful, but overall I > think Greg's patch does make sense. In particular I would continue > the refactoring by moving > > > /* > * If per-thread, filename contains a single %d that should be > * converted. > */ > if (per_thread) { > fname = g_strdup_printf(filename, getpid()); > } else { > fname = g_strdup(filename); > } > > return fopen(fname, log_append ? "a" : "w"); > > to a new function that can be used in both qemu_log_trylock() and > qemu_set_log_internal(). (In fact this refactoring is a bugfix > because per-thread log files do not currently obey log_append). What is the use case for log_append. AFAICT it only ever applied if you did a dynamic set_log. Was it ever really used or should it be dropped as an excessive complication? >From my point of view appending to an existing per-thread log is just going to cause confusion. > > Paolo -- Alex Bennée
[PATCH] treewide: Remove the unnecessary space before semicolon
%s/return ;/return; Signed-off-by: Bin Meng --- include/hw/elf_ops.h | 2 +- hw/9pfs/9p.c | 2 +- hw/dma/pl330.c | 2 +- hw/net/can/can_sja1000.c | 2 +- hw/timer/renesas_cmt.c | 2 +- hw/timer/renesas_tmr.c | 8 hw/virtio/virtio-pci.c | 2 +- target/riscv/vector_helper.c | 2 +- target/rx/op_helper.c| 4 ++-- ui/vnc-jobs.c| 2 +- ui/vnc.c | 2 +- 11 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index 7c3b1d0f6c..fbe0b1e956 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -117,7 +117,7 @@ static void glue(load_symbols, SZ)(struct elfhdr *ehdr, int fd, int must_swab, shdr_table = load_at(fd, ehdr->e_shoff, sizeof(struct elf_shdr) * ehdr->e_shnum); if (!shdr_table) { -return ; +return; } if (must_swab) { diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index aebadeaa03..76c591a01b 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1786,7 +1786,7 @@ static void coroutine_fn v9fs_walk(void *opaque) err = pdu_unmarshal(pdu, offset, "ddw", &fid, &newfid, &nwnames); if (err < 0) { pdu_complete(pdu, err); -return ; +return; } offset += err; diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c index 08e5938ec7..e5d521c329 100644 --- a/hw/dma/pl330.c +++ b/hw/dma/pl330.c @@ -1328,7 +1328,7 @@ static void pl330_debug_exec(PL330State *s) } if (!insn) { pl330_fault(ch, PL330_FAULT_UNDEF_INSTR | PL330_FAULT_DBG_INSTR); -return ; +return; } ch->stall = 0; insn->exec(ch, opcode, args, insn->size - 1); diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c index e0f76d3eb3..73201f9139 100644 --- a/hw/net/can/can_sja1000.c +++ b/hw/net/can/can_sja1000.c @@ -431,7 +431,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr addr, uint64_t val, (unsigned long long)val, (unsigned int)addr); if (addr > CAN_SJA_MEM_SIZE) { -return ; +return; } if (s->clock & 0x80) { /* PeliCAN Mode */ diff --git a/hw/timer/renesas_cmt.c b/hw/timer/renesas_cmt.c index 2e0fd21a36..69eabc678a 100644 --- a/hw/timer/renesas_cmt.c +++ b/hw/timer/renesas_cmt.c @@ -57,7 +57,7 @@ static void update_events(RCMTState *cmt, int ch) if ((cmt->cmstr & (1 << ch)) == 0) { /* count disable, so not happened next event. */ -return ; +return; } next_time = cmt->cmcor[ch] - cmt->cmcnt[ch]; next_time *= NANOSECONDS_PER_SECOND; diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c index d96002e1ee..c15f654738 100644 --- a/hw/timer/renesas_tmr.c +++ b/hw/timer/renesas_tmr.c @@ -67,18 +67,18 @@ static void update_events(RTMRState *tmr, int ch) int i, event; if (tmr->tccr[ch] == 0) { -return ; +return; } if (FIELD_EX8(tmr->tccr[ch], TCCR, CSS) == 0) { /* external clock mode */ /* event not happened */ -return ; +return; } if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CSS_CASCADING) { /* cascading mode */ if (ch == 1) { tmr->next[ch] = none; -return ; +return; } diff[cmia] = concat_reg(tmr->tcora) - concat_reg(tmr->tcnt); diff[cmib] = concat_reg(tmr->tcorb) - concat_reg(tmr->tcnt); @@ -384,7 +384,7 @@ static void timer_events(RTMRState *tmr, int ch) tmr->tcorb[ch]) & 0xff; } else { if (ch == 1) { -return ; +return; } tcnt = issue_event(tmr, ch, 16, concat_reg(tmr->tcnt), diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index e7d80242b7..34db51e241 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1675,7 +1675,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by" " neither legacy nor transitional device"); -return ; +return; } /* * Legacy and transitional devices use specific subsystem IDs. diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index b94f809eb3..0020b9a95d 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -211,7 +211,7 @@ static void vext_set_elems_1s(void *base, uint32_t is_agnostic, uint32_t cnt, return; } if (tot - cnt == 0) { -return ; +return; } memset(base + cnt, -1, tot - cnt); } diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c index 9ca32dcc82..acce650185 100644 --- a/target/rx/op_helper.c +++ b/target/rx/op_helper.c @@ -286,7 +286,7 @@ void helper_suntil(CPURXSta
Re: [PATCH] ui/gtk: prevent ui lock up when dpy_gl_update called again before current draw event occurs
On Sat, Oct 22, 2022 at 3:31 AM Dongwon Kim wrote: > A warning, "qemu: warning: console: no gl-unblock within" followed by > guest scanout lockup can happen if dpy_gl_update is called in a row > and the second call is made before gd_draw_event scheduled by the first > call is taking place. This is because draw call returns without > decrementing > gl_block ref count if the dmabuf was already submitted as shown below. > > (gd_gl_area_draw/gd_egl_draw) > > if (dmabuf) { > if (!dmabuf->draw_submitted) { > return; > } else { > dmabuf->draw_submitted = false; > } > } > > So it should not schedule any redundant draw event in case draw_submitted > is > already set in gd_egl_fluch/gd_gl_area_scanout_flush. > > Cc: Gerd Hoffmann > Cc: Vivek Kasireddy > Signed-off-by: Dongwon Kim > Reviewed-by: Marc-André Lureau > --- > ui/gtk-egl.c | 2 +- > ui/gtk-gl-area.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c > index 35f917ceb1..e84431790c 100644 > --- a/ui/gtk-egl.c > +++ b/ui/gtk-egl.c > @@ -341,7 +341,7 @@ void gd_egl_flush(DisplayChangeListener *dcl, > VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); > GtkWidget *area = vc->gfx.drawing_area; > > -if (vc->gfx.guest_fb.dmabuf) { > +if (vc->gfx.guest_fb.dmabuf && > !vc->gfx.guest_fb.dmabuf->draw_submitted) { > graphic_hw_gl_block(vc->gfx.dcl.con, true); > vc->gfx.guest_fb.dmabuf->draw_submitted = true; > gtk_widget_queue_draw_area(area, x, y, w, h); > diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c > index 682638a197..7696df1f6b 100644 > --- a/ui/gtk-gl-area.c > +++ b/ui/gtk-gl-area.c > @@ -278,7 +278,7 @@ void gd_gl_area_scanout_flush(DisplayChangeListener > *dcl, > { > VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl); > > -if (vc->gfx.guest_fb.dmabuf) { > +if (vc->gfx.guest_fb.dmabuf && > !vc->gfx.guest_fb.dmabuf->draw_submitted) { > graphic_hw_gl_block(vc->gfx.dcl.con, true); > vc->gfx.guest_fb.dmabuf->draw_submitted = true; > } > -- > 2.30.2 > > > -- Marc-André Lureau
[Bug 1994002] Re: [SRU] migration was active, but no RAM info was set
** Patch added: "lp1994002-qemu-ussuri.debdiff" https://bugs.launchpad.net/cloud-archive/+bug/1994002/+attachment/5626374/+files/lp1994002-qemu-ussuri.debdiff -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1994002 Title: [SRU] migration was active, but no RAM info was set Status in Ubuntu Cloud Archive: New Status in Ubuntu Cloud Archive ussuri series: New Status in QEMU: New Status in qemu package in Ubuntu: New Status in qemu source package in Focal: New Status in qemu source package in Jammy: New Status in qemu source package in Kinetic: New Bug description: While live-migrating many instances concurrently, libvirt sometimes return internal error: migration was active, but no RAM info was set: ~~~ 2022-03-30 06:08:37.197 7 WARNING nova.virt.libvirt.driver [req-5c3296cf-88ee-4af6-ae6a-ddba99935e23 - - - - -] [instance: af339c99-1182-4489-b15c-21e52f50f724] Error monitoring migration: internal error: migration was active, but no RAM info was set: libvirt.libvirtError: internal error: migration was active, but no RAM info was set ~~~ From upstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=2074205 [Impact] * Effects of this bug are mostly observed in large scale clusters with a lot of live migration activity. * Has second order effects for consumers of migration monitor such as libvirt and openstack. [Test Case] Steps to Reproduce: 1. live evacuate a compute 2. live migration of one or more instances fails with the above error N.B Due to the nature of this bug it is difficult consistently reproduce. [Where problems could occur] * In the event of a regression the migration monitor may report an inconsistent state. To manage notifications about this bug go to: https://bugs.launchpad.net/cloud-archive/+bug/1994002/+subscriptions
GTK clipboard implementation causing regression, falling through the cracks?
Hi all, the GTK clipboard implementation seems to be causing some stability issues (guest CPUs stuck), Gerd can you take a look? https://gitlab.com/qemu-project/qemu/-/issues/1150 Thanks, Claudio
Re: [PATCH v14 17/17] net: stream: add QAPI events to report connection state
Laurent Vivier writes: > The netdev reports NETDEV_STREAM_CONNECTED event when the backend > is connected, and NETDEV_STREAM_DISCONNECTED when it is disconnected. > > The NETDEV_STREAM_CONNECTED event includes the destination address. > > This allows a system manager like libvirt to detect when the server > fails. > > For instance with passt: > > { 'execute': 'qmp_capabilities' } > { "return": { } } > { "timestamp": { "seconds": 1666341395, "microseconds": 505347 }, > "event": "NETDEV_STREAM_CONNECTED", > "data": { "netdev-id": "netdev0", > "addr": { "path": "/tmp/passt_1.socket", "type": "unix" } } } > > [killing passt here] > > { "timestamp": { "seconds": 1666341430, "microseconds": 968694 }, > "event": "NETDEV_STREAM_DISCONNECTED", > "data": { "netdev-id": "netdev0" } } > > Signed-off-by: Laurent Vivier > Acked-by: Michael S. Tsirkin QAPI schema Acked-by: Markus Armbruster
Re: [PATCH] treewide: Remove the unnecessary space before semicolon
On Mon, 24 Oct 2022 at 08:28, Bin Meng wrote: > > %s/return ;/return; > > Signed-off-by: Bin Meng > --- Reviewed-by: Peter Maydell thanks -- PMM
[PULL 12/23] tests/9p: simplify callers of treaddir()
Now as treaddir() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <7cec6f2c7011a481806c34908893b7282702a7a6.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index e5c174c218..99e24fce0b 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -120,12 +120,12 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) /* * submit count = msize - 11, because 11 is the header size of Rreaddir */ -req = treaddir({ +treaddir({ .client = v9p, .fid = 1, .offset = 0, .count = P9_MAX_SIZE - 11, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rreaddir(req, &count, &nentries, &entries); +.rreaddir = { +.count = &count, .nentries = &nentries, .entries = &entries +} +}); /* * Assuming msize (P9_MAX_SIZE) is large enough so we can retrieve all @@ -190,12 +190,13 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) npartialentries = 0; partialentries = NULL; -req = treaddir({ +treaddir({ .client = v9p, .fid = fid, .offset = offset, .count = count, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rreaddir(req, &count, &npartialentries, &partialentries); +.rreaddir = { +.count = &count, .nentries = &npartialentries, +.entries = &partialentries +} +}); if (npartialentries > 0 && partialentries) { if (!entries) { entries = partialentries; -- 2.30.2
[PULL 21/23] tests/9p: merge v9fs_tlink() and do_hardlink()
As with previous patches, unify those 2 functions into a single function v9fs_tlink() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 43 ++- tests/qtest/libqos/virtio-9p-client.h | 31 +-- tests/qtest/virtio-9p-test.c | 26 ++-- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 89eaf50355..a2770719b9 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -950,23 +950,50 @@ void v9fs_rsymlink(P9Req *req, v9fs_qid *qid) } /* size[4] Tlink tag[2] dfid[4] fid[4] name[s] */ -P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, - const char *name, uint16_t tag) +TlinkRes v9fs_tlink(TlinkOpt opt) { P9Req *req; +uint32_t err; + +g_assert(opt.client); +/* expecting either hi-level atPath or low-level dfid, but not both */ +g_assert(!opt.atPath || !opt.dfid); +/* expecting either hi-level toPath or low-level fid, but not both */ +g_assert(!opt.toPath || !opt.fid); + +if (opt.atPath) { +opt.dfid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; +} +if (opt.toPath) { +opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.toPath }).newfid; +} uint32_t body_size = 4 + 4; -uint16_t string_size = v9fs_string_size(name); +uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; -req = v9fs_req_init(v9p, body_size, P9_TLINK, tag); -v9fs_uint32_write(req, dfid); -v9fs_uint32_write(req, fid); -v9fs_string_write(req, name); +req = v9fs_req_init(opt.client, body_size, P9_TLINK, opt.tag); +v9fs_uint32_write(req, opt.dfid); +v9fs_uint32_write(req, opt.fid); +v9fs_string_write(req, opt.name); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rlink(req); +} +req = NULL; /* request was freed */ +} + +return (TlinkRes) { .req = req }; } /* size[4] Rlink tag[2] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index b905a54966..49ffd0fc51 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -387,6 +387,34 @@ typedef struct TsymlinkRes { P9Req *req; } TsymlinkRes; +/* options for 'Tlink' 9p request */ +typedef struct TlinkOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* low-level variant of directory where hard link shall be created */ +uint32_t dfid; +/* high-level variant of directory where hard link shall be created */ +const char *atPath; +/* low-level variant of target referenced by new hard link */ +uint32_t fid; +/* high-level variant of target referenced by new hard link */ +const char *toPath; +/* name of hard link (required) */ +const char *name; +/* only send Tlink request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TlinkOpt; + +/* result of 'Tlink' 9p request */ +typedef struct TlinkRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TlinkRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -432,8 +460,7 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt); void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit); TsymlinkRes v9fs_tsymlink(TsymlinkOpt); void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); -P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, - const char *name, uint16_t tag); +TlinkRes v9fs_tlink(TlinkOpt); void v9fs_rlink(P9Req *req); P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, uint32_t flags, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index c7213d6caf..185eaf8b1e 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -27,6 +27,7 @@ #define tmkdir(...) v9fs_tmkdir((TMkdirOpt) __VA_ARGS__) #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__) #define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__)
[PULL 03/23] 9pfs: use GHashTable for fid table
From: Linus Heckemann The previous implementation would iterate over the fid table for lookup operations, resulting in an operation with O(n) complexity on the number of open files and poor cache locality -- for every open, stat, read, write, etc operation. This change uses a hashtable for this instead, significantly improving the performance of the 9p filesystem. The runtime of NixOS's simple installer test, which copies ~122k files totalling ~1.8GiB from 9p, decreased by a factor of about 10. Signed-off-by: Linus Heckemann Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Greg Kurz [CS: - Retain BUG_ON(f->clunked) in get_fid(). - Add TODO comment in clunk_fid(). ] Message-Id: <20221004104121.713689-1-...@sphalerite.org> [CS: - Drop unnecessary goto and out: label. ] Signed-off-by: Christian Schoenebeck --- hw/9pfs/9p.c | 196 +-- hw/9pfs/9p.h | 2 +- 2 files changed, 113 insertions(+), 85 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index aebadeaa03..9bf13133e5 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -256,7 +256,8 @@ static size_t v9fs_string_size(V9fsString *str) } /* - * returns 0 if fid got re-opened, 1 if not, < 0 on error */ + * returns 0 if fid got re-opened, 1 if not, < 0 on error + */ static int coroutine_fn v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f) { int err = 1; @@ -282,33 +283,32 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid) V9fsFidState *f; V9fsState *s = pdu->s; -QSIMPLEQ_FOREACH(f, &s->fid_list, next) { +f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid)); +if (f) { BUG_ON(f->clunked); -if (f->fid == fid) { -/* - * Update the fid ref upfront so that - * we don't get reclaimed when we yield - * in open later. - */ -f->ref++; -/* - * check whether we need to reopen the - * file. We might have closed the fd - * while trying to free up some file - * descriptors. - */ -err = v9fs_reopen_fid(pdu, f); -if (err < 0) { -f->ref--; -return NULL; -} -/* - * Mark the fid as referenced so that the LRU - * reclaim won't close the file descriptor - */ -f->flags |= FID_REFERENCED; -return f; +/* + * Update the fid ref upfront so that + * we don't get reclaimed when we yield + * in open later. + */ +f->ref++; +/* + * check whether we need to reopen the + * file. We might have closed the fd + * while trying to free up some file + * descriptors. + */ +err = v9fs_reopen_fid(pdu, f); +if (err < 0) { +f->ref--; +return NULL; } +/* + * Mark the fid as referenced so that the LRU + * reclaim won't close the file descriptor + */ +f->flags |= FID_REFERENCED; +return f; } return NULL; } @@ -317,12 +317,11 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) { V9fsFidState *f; -QSIMPLEQ_FOREACH(f, &s->fid_list, next) { +f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid)); +if (f) { /* If fid is already there return NULL */ BUG_ON(f->clunked); -if (f->fid == fid) { -return NULL; -} +return NULL; } f = g_new0(V9fsFidState, 1); f->fid = fid; @@ -333,7 +332,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) * reclaim won't close the file descriptor */ f->flags |= FID_REFERENCED; -QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next); +g_hash_table_insert(s->fids, GINT_TO_POINTER(fid), f); v9fs_readdir_init(s->proto_version, &f->fs.dir); v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir); @@ -424,12 +423,12 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid) { V9fsFidState *fidp; -QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) { -if (fidp->fid == fid) { -QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next); -fidp->clunked = true; -return fidp; -} +/* TODO: Use g_hash_table_steal_extended() instead? */ +fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid)); +if (fidp) { +g_hash_table_remove(s->fids, GINT_TO_POINTER(fid)); +fidp->clunked = true; +return fidp; } return NULL; } @@ -439,10 +438,15 @@ void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu) int reclaim_count = 0; V9fsState *s = pdu->s; V9fsFidState *f; +GHashTableIter iter; +gpointer fid; + +g_hash_table_iter_init(&iter, s->fids); + QSLIST_HEAD(, V9fsFidState) reclaim_list = QSLIST_HEAD_INITIALIZER(reclaim_list); -QSIMPLE
Re: [PATCH v2 7/8] hw/arm/virt: Fix devicetree warnings about the virtio-iommu node
On Fri, 21 Oct 2022 at 15:33, Jean-Philippe Brucker wrote: > > On Tue, Sep 27, 2022 at 04:35:25PM +0200, Eric Auger wrote: > > >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > >> index 2de16f6324..5e16d54bbb 100644 > > >> --- a/hw/arm/virt.c > > >> +++ b/hw/arm/virt.c > > >> @@ -1372,14 +1372,15 @@ static void create_smmu(const VirtMachineState > > >> *vms, > > >> > > >> static void create_virtio_iommu_dt_bindings(VirtMachineState *vms) > > >> { > > >> -const char compat[] = "virtio,pci-iommu"; > > >> +const char compat[] = "virtio,pci-iommu\0pci1af4,1057"; > > >> uint16_t bdf = vms->virtio_iommu_bdf; > > > > > > PCI_DEVICE_ID_VIRTIO_IOMMU is listed in include/hw/pci/pci.h > > > as 0x1014, so where does 1057 come from? (This is a hex value, > > > right?) > > the virtio spec states: > > The PCI Device ID is calculated by adding 0x1040 to the Virtio Device ID > > (this IOMMU device ID is 0d23 = 0x17 for the virtio-iommu device, also > > found in include/uapi/linux/virtio_ids.h) so 0x1057 above looks correct > > > > note that in docs/specs/pci-ids.txt there are a bunch of other device > > ids not documented (virtio-mem, pmem) > > > > What I don't get anymore is the device id in qemu include/hw/pci/pci.h > > Yes 0x1057 is the right device ID, and it matches what the > virtio-iommu-pci device gets in hw/virtio/virtio-pci.c:1691. > The wrong 0x1014 value set by hw/virtio/virtio-iommu-pci.c:78 gets > overwritten since virtio-iommu is modern only. I can send a patch to > remove it. > > Peter, do you mind taking this patch as well, or should I resend it? Sure, I've applied this one to target-arm.next. -- PMM
[PULL 04/23] tests/9p: merge *walk*() functions
Introduce declarative function calls. There are currently 4 different functions for sending a 9p 'Twalk' request: v9fs_twalk(), do_walk(), do_walk_rqids() and do_walk_expect_error(). They are all doing the same thing, just in a slightly different way and with slightly different function arguments. Merge those 4 functions into a single function by using a struct for function call arguments and use designated initializers when calling this function to turn usage into a declarative approach, which is better readable and easier to maintain. Also move private functions genfid(), split() and split_free() from virtio-9p-test.c to virtio-9p-client.c. Based-on: Signed-off-by: Christian Schoenebeck Message-Id: <607969dbfbc63c1be008df9131133711b046e979.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 114 ++-- tests/qtest/libqos/virtio-9p-client.h | 37 - tests/qtest/virtio-9p-test.c | 187 +- 3 files changed, 198 insertions(+), 140 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index f5c35fd722..a95bbad9c8 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -23,6 +23,65 @@ void v9fs_set_allocator(QGuestAllocator *t_alloc) alloc = t_alloc; } +/* + * Used to auto generate new fids. Start with arbitrary high value to avoid + * collision with hard coded fids in basic test code. + */ +static uint32_t fid_generator = 1000; + +static uint32_t genfid(void) +{ +return fid_generator++; +} + +/** + * Splits the @a in string by @a delim into individual (non empty) strings + * and outputs them to @a out. The output array @a out is NULL terminated. + * + * Output array @a out must be freed by calling split_free(). + * + * @returns number of individual elements in output array @a out (without the + * final NULL terminating element) + */ +static int split(const char *in, const char *delim, char ***out) +{ +int n = 0, i = 0; +char *tmp, *p; + +tmp = g_strdup(in); +for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) { +if (strlen(p) > 0) { +++n; +} +} +g_free(tmp); + +*out = g_new0(char *, n + 1); /* last element NULL delimiter */ + +tmp = g_strdup(in); +for (p = strtok(tmp, delim); p != NULL; p = strtok(NULL, delim)) { +if (strlen(p) > 0) { +(*out)[i++] = g_strdup(p); +} +} +g_free(tmp); + +return n; +} + +static void split_free(char ***out) +{ +int i; +if (!*out) { +return; +} +for (i = 0; (*out)[i]; ++i) { +g_free((*out)[i]); +} +g_free(*out); +*out = NULL; +} + void v9fs_memwrite(P9Req *req, const void *addr, size_t len) { qtest_memwrite(req->qts, req->t_msg + req->t_off, addr, len); @@ -294,28 +353,61 @@ void v9fs_rattach(P9Req *req, v9fs_qid *qid) } /* size[4] Twalk tag[2] fid[4] newfid[4] nwname[2] nwname*(wname[s]) */ -P9Req *v9fs_twalk(QVirtio9P *v9p, uint32_t fid, uint32_t newfid, - uint16_t nwname, char *const wnames[], uint16_t tag) +TWalkRes v9fs_twalk(TWalkOpt opt) { P9Req *req; int i; uint32_t body_size = 4 + 4 + 2; +uint32_t err; +char **wnames = NULL; -for (i = 0; i < nwname; i++) { -uint16_t wname_size = v9fs_string_size(wnames[i]); +g_assert(opt.client); +/* expecting either high- or low-level path, both not both */ +g_assert(!opt.path || !(opt.nwname || opt.wnames)); +/* expecting either Rwalk or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !(opt.rwalk.nwqid || opt.rwalk.wqid)); + +if (!opt.newfid) { +opt.newfid = genfid(); +} + +if (opt.path) { +opt.nwname = split(opt.path, "/", &wnames); +opt.wnames = wnames; +} + +for (i = 0; i < opt.nwname; i++) { +uint16_t wname_size = v9fs_string_size(opt.wnames[i]); g_assert_cmpint(body_size, <=, UINT32_MAX - wname_size); body_size += wname_size; } -req = v9fs_req_init(v9p, body_size, P9_TWALK, tag); -v9fs_uint32_write(req, fid); -v9fs_uint32_write(req, newfid); -v9fs_uint16_write(req, nwname); -for (i = 0; i < nwname; i++) { -v9fs_string_write(req, wnames[i]); +req = v9fs_req_init(opt.client, body_size, P9_TWALK, opt.tag); +v9fs_uint32_write(req, opt.fid); +v9fs_uint32_write(req, opt.newfid); +v9fs_uint16_write(req, opt.nwname); +for (i = 0; i < opt.nwname; i++) { +v9fs_string_write(req, opt.wnames[i]); } v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rwalk(req, opt.rwalk.nwqid, opt.rwalk.wqid); +} +req = NULL; /* request
[PULL 14/23] tests/9p: simplify callers of tlopen()
Now as tlopen() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/virtio-9p-test.c | 43 +--- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 0455c3a094..60a030b877 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -105,7 +105,6 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_qid qid; uint32_t count, nentries; struct V9fsDirent *entries = NULL; -P9Req *req; tattach({ .client = v9p }); twalk({ @@ -114,11 +113,9 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) }); g_assert_cmpint(nqid, ==, 1); -req = tlopen({ -.client = v9p, .fid = 1, .flags = O_DIRECTORY, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlopen(req, &qid, NULL); +tlopen({ +.client = v9p, .fid = 1, .flags = O_DIRECTORY, .rlopen.qid = &qid +}); /* * submit count = msize - 11, because 11 is the header size of Rreaddir @@ -163,7 +160,6 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) v9fs_qid qid; uint32_t nentries, npartialentries; struct V9fsDirent *entries, *tail, *partialentries; -P9Req *req; int fid; uint64_t offset; @@ -181,11 +177,9 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) }); g_assert_cmpint(nqid, ==, 1); -req = tlopen({ -.client = v9p, .fid = fid, .flags = O_DIRECTORY, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlopen(req, &qid, NULL); +tlopen({ +.client = v9p, .fid = fid, .flags = O_DIRECTORY, .rlopen.qid = &qid +}); /* * send as many Treaddir requests as required to get all directory @@ -363,18 +357,13 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_LOPEN_FILE) }; -P9Req *req; tattach({ .client = v9p }); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); -req = tlopen({ -.client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlopen(req, NULL, NULL); +tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); g_free(wnames[0]); } @@ -394,11 +383,7 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); -req = tlopen({ -.client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlopen(req, NULL, NULL); +tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); req = v9fs_twrite(v9p, 1, 0, write_count, buf, 0); v9fs_req_wait_for_reply(req, NULL); @@ -422,11 +407,7 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); -req = tlopen({ -.client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlopen(req, NULL, NULL); +tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); /* This will cause the 9p server to try to write data to the backend, * until the write request gets cancelled. @@ -461,11 +442,7 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames }); -req = tlopen({ -.client = v9p, .fid = 1, .flags = O_WRONLY, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlopen(req, NULL, NULL); +tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); /* This will cause the write request to complete right away, before it * could be actually cancelled. -- 2.30.2
[PULL 22/23] tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
As with previous patches, unify those 2 functions into a single function v9fs_tunlinkat() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <1dea593edd464908d92501933c068388c01f1744.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 37 +-- tests/qtest/libqos/virtio-9p-client.h | 29 +++-- tests/qtest/virtio-9p-test.c | 26 ++- 3 files changed, 64 insertions(+), 28 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index a2770719b9..e017e030ec 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -1004,23 +1004,44 @@ void v9fs_rlink(P9Req *req) } /* size[4] Tunlinkat tag[2] dirfd[4] name[s] flags[4] */ -P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, - uint32_t flags, uint16_t tag) +TunlinkatRes v9fs_tunlinkat(TunlinkatOpt opt) { P9Req *req; +uint32_t err; + +g_assert(opt.client); +/* expecting either hi-level atPath or low-level dirfd, but not both */ +g_assert(!opt.atPath || !opt.dirfd); + +if (opt.atPath) { +opt.dirfd = v9fs_twalk((TWalkOpt) { .client = opt.client, +.path = opt.atPath }).newfid; +} uint32_t body_size = 4 + 4; -uint16_t string_size = v9fs_string_size(name); +uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; -req = v9fs_req_init(v9p, body_size, P9_TUNLINKAT, tag); -v9fs_uint32_write(req, dirfd); -v9fs_string_write(req, name); -v9fs_uint32_write(req, flags); +req = v9fs_req_init(opt.client, body_size, P9_TUNLINKAT, opt.tag); +v9fs_uint32_write(req, opt.dirfd); +v9fs_string_write(req, opt.name); +v9fs_uint32_write(req, opt.flags); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_runlinkat(req); +} +req = NULL; /* request was freed */ +} + +return (TunlinkatRes) { .req = req }; } /* size[4] Runlinkat tag[2] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 49ffd0fc51..78228eb97d 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -415,6 +415,32 @@ typedef struct TlinkRes { P9Req *req; } TlinkRes; +/* options for 'Tunlinkat' 9p request */ +typedef struct TunlinkatOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* low-level variant of directory where name shall be unlinked */ +uint32_t dirfd; +/* high-level variant of directory where name shall be unlinked */ +const char *atPath; +/* name of directory entry to be unlinked (required) */ +const char *name; +/* Linux unlinkat(2) flags */ +uint32_t flags; +/* only send Tunlinkat request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TunlinkatOpt; + +/* result of 'Tunlinkat' 9p request */ +typedef struct TunlinkatRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TunlinkatRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -462,8 +488,7 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt); void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); TlinkRes v9fs_tlink(TlinkOpt); void v9fs_rlink(P9Req *req); -P9Req *v9fs_tunlinkat(QVirtio9P *v9p, uint32_t dirfd, const char *name, - uint32_t flags, uint16_t tag); +TunlinkatRes v9fs_tunlinkat(TunlinkatOpt); void v9fs_runlinkat(P9Req *req); #endif diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 185eaf8b1e..65e69491e5 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -28,6 +28,7 @@ #define tlcreate(...) v9fs_tlcreate((TlcreateOpt) __VA_ARGS__) #define tsymlink(...) v9fs_tsymlink((TsymlinkOpt) __VA_ARGS__) #define tlink(...) v9fs_tlink((TlinkOpt) __VA_ARGS__) +#define tunlinkat(...) v9fs_tunlinkat((TunlinkatOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -481,20 +482,6 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) g_free(wnames[0]); } -static void do_unlinkat(QVirtio9P *v9p, const char *atpath, const char *rpath, -
[PULL 08/23] tests/9p: simplify callers of tattach()
Now as tattach() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <9b50e5b89a0072e84a9191d18c19a53546a28bba.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 271c42f6f9..46bb189b81 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -302,11 +302,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) struct v9fs_attr attr; tversion({ .client = v9p }); -req = tattach({ -.client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rattach(req, &root_qid); +tattach({ +.client = v9p, .fid = 0, .n_uname = getuid(), +.rattach.qid = &root_qid +}); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL, @@ -330,14 +329,12 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) char *wnames[] = { g_strdup("..") }; v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; -P9Req *req; tversion({ .client = v9p }); -req = tattach((TAttachOpt) { -.client = v9p, .fid = 0, .n_uname = getuid(), .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rattach(req, &root_qid); +tattach({ +.client = v9p, .fid = 0, .n_uname = getuid(), +.rattach.qid = &root_qid +}); twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, -- 2.30.2
[PULL 06/23] tests/9p: merge v9fs_tversion() and do_version()
As with previous patches, unify functions v9fs_tversion() and do_version() into a single function v9fs_tversion() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <2d253491aaffd267ec295f056dda47456692cd0c.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 47 +++ tests/qtest/libqos/virtio-9p-client.h | 25 -- tests/qtest/virtio-9p-test.c | 23 +++-- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index a95bbad9c8..e8364f8d64 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -291,21 +291,54 @@ void v9fs_rlerror(P9Req *req, uint32_t *err) } /* size[4] Tversion tag[2] msize[4] version[s] */ -P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, - uint16_t tag) +TVersionRes v9fs_tversion(TVersionOpt opt) { P9Req *req; +uint32_t err; uint32_t body_size = 4; -uint16_t string_size = v9fs_string_size(version); +uint16_t string_size; +uint16_t server_len; +g_autofree char *server_version = NULL; +g_assert(opt.client); + +if (!opt.msize) { +opt.msize = P9_MAX_SIZE; +} + +if (!opt.tag) { +opt.tag = P9_NOTAG; +} + +if (!opt.version) { +opt.version = "9P2000.L"; +} + +string_size = v9fs_string_size(opt.version); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; -req = v9fs_req_init(v9p, body_size, P9_TVERSION, tag); +req = v9fs_req_init(opt.client, body_size, P9_TVERSION, opt.tag); -v9fs_uint32_write(req, msize); -v9fs_string_write(req, version); +v9fs_uint32_write(req, opt.msize); +v9fs_string_write(req, opt.version); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rversion(req, &server_len, &server_version); +g_assert_cmpmem(server_version, server_len, +opt.version, strlen(opt.version)); +} +req = NULL; /* request was freed */ +} + +return (TVersionRes) { +.req = req, +}; } /* size[4] Rversion tag[2] msize[4] version[s] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 8c6abbb173..fcde849b5d 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -106,6 +106,28 @@ typedef struct TWalkRes { P9Req *req; } TWalkRes; +/* options for 'Tversion' 9p request */ +typedef struct TVersionOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* maximum message size that can be handled by client (optional) */ +uint32_t msize; +/* protocol version (optional) */ +const char *version; +/* only send Tversion request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TVersionOpt; + +/* result of 'Tversion' 9p request */ +typedef struct TVersionRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TVersionRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -127,8 +149,7 @@ void v9fs_req_wait_for_reply(P9Req *req, uint32_t *len); void v9fs_req_recv(P9Req *req, uint8_t id); void v9fs_req_free(P9Req *req); void v9fs_rlerror(P9Req *req, uint32_t *err); -P9Req *v9fs_tversion(QVirtio9P *v9p, uint32_t msize, const char *version, - uint16_t tag); +TVersionRes v9fs_tversion(TVersionOpt); void v9fs_rversion(P9Req *req, uint16_t *len, char **version); P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 3c326451b1..f2907c8026 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -17,6 +17,7 @@ #include "libqos/virtio-9p-client.h" #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) +#define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -41,31 +42,17 @@ static inline bool is_same_qid(v9fs_qid a, v9fs_qid b) return a[0] == b[0] && memcmp(&a[5], &b[5], 8) == 0; } -static void do_version(QVirtio9P *v9p) -{ -const char *version = "9P2000.L"; -uint16_t server_len; -g_auto
[PULL 00/23] 9p queue 2022-10-24
The following changes since commit 0529245488865038344d64fff7ee05864d3d17f6: Merge tag 'pull-target-arm-20221020' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-10-20 14:36:12 -0400) are available in the Git repository at: https://github.com/cschoenebeck/qemu.git tags/pull-9p-20221024 for you to fetch changes up to 3ce77865bf813f313cf79c00fd951bfc95a50165: tests/9p: remove unnecessary g_strdup() calls (2022-10-24 12:24:32 +0200) 9pfs: performance, Windows host prep, tests restructure * Highlight of this PR is Linus Heckemann's GHashTable patch which brings massive general performance improvements of 9p server somewhere between factor 6 .. 12. * Bin Meng's g_mkdir patch is a preparatory patch for upcoming Windows host support of 9p server. * The rest of the patches in this PR are 9p test code restructuring and refactoring changes to improve readability and to ease maintenance of 9p test code on the long-term. Bin Meng (1): fsdev/virtfs-proxy-helper: Use g_mkdir() Christian Schoenebeck (21): tests/9p: split virtio-9p-test.c into tests and 9p client part tests/9p: merge *walk*() functions tests/9p: simplify callers of twalk() tests/9p: merge v9fs_tversion() and do_version() tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid() tests/9p: simplify callers of tattach() tests/9p: convert v9fs_tgetattr() to declarative arguments tests/9p: simplify callers of tgetattr() tests/9p: convert v9fs_treaddir() to declarative arguments tests/9p: simplify callers of treaddir() tests/9p: convert v9fs_tlopen() to declarative arguments tests/9p: simplify callers of tlopen() tests/9p: convert v9fs_twrite() to declarative arguments tests/9p: simplify callers of twrite() tests/9p: convert v9fs_tflush() to declarative arguments tests/9p: merge v9fs_tmkdir() and do_mkdir() tests/9p: merge v9fs_tlcreate() and do_lcreate() tests/9p: merge v9fs_tsymlink() and do_symlink() tests/9p: merge v9fs_tlink() and do_hardlink() tests/9p: merge v9fs_tunlinkat() and do_unlinkat() tests/9p: remove unnecessary g_strdup() calls Linus Heckemann (1): 9pfs: use GHashTable for fid table fsdev/virtfs-proxy-helper.c |3 +- hw/9pfs/9p.c | 196 ++--- hw/9pfs/9p.h |2 +- tests/qtest/libqos/meson.build|1 + tests/qtest/libqos/virtio-9p-client.c | 1049 ++ tests/qtest/libqos/virtio-9p-client.h | 494 + tests/qtest/virtio-9p-test.c | 1299 ++--- 7 files changed, 1867 insertions(+), 1177 deletions(-) create mode 100644 tests/qtest/libqos/virtio-9p-client.c create mode 100644 tests/qtest/libqos/virtio-9p-client.h
[PULL 13/23] tests/9p: convert v9fs_tlopen() to declarative arguments
Use declarative function arguments for function v9fs_tlopen(). Signed-off-by: Christian Schoenebeck Message-Id: <765ab515353c56f88f0a163631f626a44e9565d6.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 28 +++-- tests/qtest/libqos/virtio-9p-client.h | 30 +-- tests/qtest/virtio-9p-test.c | 25 -- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 047c8993b6..15fde54d63 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -643,16 +643,32 @@ void v9fs_free_dirents(struct V9fsDirent *e) } /* size[4] Tlopen tag[2] fid[4] flags[4] */ -P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, - uint16_t tag) +TLOpenRes v9fs_tlopen(TLOpenOpt opt) { P9Req *req; +uint32_t err; -req = v9fs_req_init(v9p, 4 + 4, P9_TLOPEN, tag); -v9fs_uint32_write(req, fid); -v9fs_uint32_write(req, flags); +g_assert(opt.client); +/* expecting either Rlopen or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !(opt.rlopen.qid || opt.rlopen.iounit)); + +req = v9fs_req_init(opt.client, 4 + 4, P9_TLOPEN, opt.tag); +v9fs_uint32_write(req, opt.fid); +v9fs_uint32_write(req, opt.flags); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rlopen(req, opt.rlopen.qid, opt.rlopen.iounit); +} +req = NULL; /* request was freed */ +} + +return (TLOpenRes) { .req = req }; } /* size[4] Rlopen tag[2] qid[13] iounit[4] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 2bf649085f..3b70aef51e 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -212,6 +212,33 @@ typedef struct TReadDirRes { P9Req *req; } TReadDirRes; +/* options for 'Tlopen' 9p request */ +typedef struct TLOpenOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* file ID of file / directory to be opened (required) */ +uint32_t fid; +/* Linux open(2) flags such as O_RDONLY, O_RDWR, O_WRONLY (optional) */ +uint32_t flags; +/* data being received from 9p server as 'Rlopen' response (optional) */ +struct { +v9fs_qid *qid; +uint32_t *iounit; +} rlopen; +/* only send Tlopen request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TLOpenOpt; + +/* result of 'Tlopen' 9p request */ +typedef struct TLOpenRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TLOpenRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -245,8 +272,7 @@ TReadDirRes v9fs_treaddir(TReadDirOpt); void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, struct V9fsDirent **entries); void v9fs_free_dirents(struct V9fsDirent *e); -P9Req *v9fs_tlopen(QVirtio9P *v9p, uint32_t fid, uint32_t flags, - uint16_t tag); +TLOpenRes v9fs_tlopen(TLOpenOpt); void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, uint32_t count, const void *data, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 99e24fce0b..0455c3a094 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -21,6 +21,7 @@ #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) +#define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -113,7 +114,9 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) }); g_assert_cmpint(nqid, ==, 1); -req = v9fs_tlopen(v9p, 1, O_DIRECTORY, 0); +req = tlopen({ +.client = v9p, .fid = 1, .flags = O_DIRECTORY, .requestOnly = true +}).req; v9fs_req_wait_for_reply(req, NULL); v9fs_rlopen(req, &qid, NULL); @@ -178,7 +181,9 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) }); g_assert_cmpint(nqid, ==, 1); -req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0); +req = tlopen({ +.client = v9p, .fid =
[PULL 02/23] tests/9p: split virtio-9p-test.c into tests and 9p client part
This patch is pure refactoring, it does not change behaviour. virtio-9p-test.c grew to 1657 lines. Let's split this file up between actual 9p test cases vs. 9p test client, to make it easier to concentrate on the actual 9p tests. Move the 9p test client code to a new unit virtio-9p-client.c, which are basically all functions and types prefixed with v9fs_* already. Note that some client wrapper functions (do_*) are preserved in virtio-9p-test.c, simply because these wrapper functions are going to be wiped with subsequent patches anyway. As the global QGuestAllocator variable is moved to virtio-9p-client.c, add a new function v9fs_set_allocator() to be used by virtio-9p-test.c instead of fiddling with a global variable across units and libraries. Signed-off-by: Christian Schoenebeck Reviewed-by: Greg Kurz Message-Id: --- tests/qtest/libqos/meson.build| 1 + tests/qtest/libqos/virtio-9p-client.c | 684 +++ tests/qtest/libqos/virtio-9p-client.h | 138 + tests/qtest/virtio-9p-test.c | 770 +- 4 files changed, 849 insertions(+), 744 deletions(-) create mode 100644 tests/qtest/libqos/virtio-9p-client.c create mode 100644 tests/qtest/libqos/virtio-9p-client.h diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build index a5b6d5197a..113c80b4e4 100644 --- a/tests/qtest/libqos/meson.build +++ b/tests/qtest/libqos/meson.build @@ -34,6 +34,7 @@ libqos_srcs = files( 'tpci200.c', 'virtio.c', 'virtio-9p.c', +'virtio-9p-client.c', 'virtio-balloon.c', 'virtio-blk.c', 'vhost-user-blk.c', diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c new file mode 100644 index 00..f5c35fd722 --- /dev/null +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -0,0 +1,684 @@ +/* + * 9P network client for VirtIO 9P test cases (based on QTest) + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +/* + * Not so fast! You might want to read the 9p developer docs first: + * https://wiki.qemu.org/Documentation/9p + */ + +#include "qemu/osdep.h" +#include "virtio-9p-client.h" + +#define QVIRTIO_9P_TIMEOUT_US (10 * 1000 * 1000) +static QGuestAllocator *alloc; + +void v9fs_set_allocator(QGuestAllocator *t_alloc) +{ +alloc = t_alloc; +} + +void v9fs_memwrite(P9Req *req, const void *addr, size_t len) +{ +qtest_memwrite(req->qts, req->t_msg + req->t_off, addr, len); +req->t_off += len; +} + +void v9fs_memskip(P9Req *req, size_t len) +{ +req->r_off += len; +} + +void v9fs_memread(P9Req *req, void *addr, size_t len) +{ +qtest_memread(req->qts, req->r_msg + req->r_off, addr, len); +req->r_off += len; +} + +void v9fs_uint8_read(P9Req *req, uint8_t *val) +{ +v9fs_memread(req, val, 1); +} + +void v9fs_uint16_write(P9Req *req, uint16_t val) +{ +uint16_t le_val = cpu_to_le16(val); + +v9fs_memwrite(req, &le_val, 2); +} + +void v9fs_uint16_read(P9Req *req, uint16_t *val) +{ +v9fs_memread(req, val, 2); +le16_to_cpus(val); +} + +void v9fs_uint32_write(P9Req *req, uint32_t val) +{ +uint32_t le_val = cpu_to_le32(val); + +v9fs_memwrite(req, &le_val, 4); +} + +void v9fs_uint64_write(P9Req *req, uint64_t val) +{ +uint64_t le_val = cpu_to_le64(val); + +v9fs_memwrite(req, &le_val, 8); +} + +void v9fs_uint32_read(P9Req *req, uint32_t *val) +{ +v9fs_memread(req, val, 4); +le32_to_cpus(val); +} + +void v9fs_uint64_read(P9Req *req, uint64_t *val) +{ +v9fs_memread(req, val, 8); +le64_to_cpus(val); +} + +/* len[2] string[len] */ +uint16_t v9fs_string_size(const char *string) +{ +size_t len = strlen(string); + +g_assert_cmpint(len, <=, UINT16_MAX - 2); + +return 2 + len; +} + +void v9fs_string_write(P9Req *req, const char *string) +{ +int len = strlen(string); + +g_assert_cmpint(len, <=, UINT16_MAX); + +v9fs_uint16_write(req, (uint16_t) len); +v9fs_memwrite(req, string, len); +} + +void v9fs_string_read(P9Req *req, uint16_t *len, char **string) +{ +uint16_t local_len; + +v9fs_uint16_read(req, &local_len); +if (len) { +*len = local_len; +} +if (string) { +*string = g_malloc(local_len + 1); +v9fs_memread(req, *string, local_len); +(*string)[local_len] = 0; +} else { +v9fs_memskip(req, local_len); +} +} + +typedef struct { +uint32_t size; +uint8_t id; +uint16_t tag; +} QEMU_PACKED P9Hdr; + +P9Req *v9fs_req_init(QVirtio9P *v9p, uint32_t size, uint8_t id, + uint16_t tag) +{ +P9Req *req = g_new0(P9Req, 1); +uint32_t total_size = 7; /* 9P header has well-known size of 7 bytes */ +P9Hdr hdr = { +.id = id, +.tag = cpu_to_le16(tag) +}; + +g_assert_cmpint(total_size, <=, UINT32_MAX - size); +total_size += size
[PULL 05/23] tests/9p: simplify callers of twalk()
Now as twalk() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <8b9d3c656ad43b6c953d6bdacd8d9f4c8e599b2a.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 92 +--- 1 file changed, 32 insertions(+), 60 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index cf5d6146ad..3c326451b1 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -90,19 +90,17 @@ static void fs_walk(void *obj, void *data, QGuestAllocator *t_alloc) uint16_t nwqid; g_autofree v9fs_qid *wqid = NULL; int i; -P9Req *req; for (i = 0; i < P9_MAXWELEM; i++) { wnames[i] = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i); } do_attach(v9p); -req = twalk({ +twalk({ .client = v9p, .fid = 0, .newfid = 1, -.nwname = P9_MAXWELEM, .wnames = wnames, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwalk(req, &nwqid, &wqid); +.nwname = P9_MAXWELEM, .wnames = wnames, +.rwalk = { .nwqid = &nwqid, .wqid = &wqid } +}); g_assert_cmpint(nwqid, ==, P9_MAXWELEM); @@ -134,12 +132,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; do_attach(v9p); -req = twalk({ +twalk({ .client = v9p, .fid = 0, .newfid = 1, -.nwname = 1, .wnames = wnames, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwalk(req, &nqid, NULL); +.nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid +}); g_assert_cmpint(nqid, ==, 1); req = v9fs_tlopen(v9p, 1, O_DIRECTORY, 0); @@ -198,12 +194,10 @@ static void do_readdir_split(QVirtio9P *v9p, uint32_t count) nentries = 0; tail = NULL; -req = twalk({ +twalk({ .client = v9p, .fid = 0, .newfid = fid, -.nwname = 1, .wnames = wnames, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwalk(req, &nqid, NULL); +.nwname = 1, .wnames = wnames, .rwalk.nwqid = &nqid +}); g_assert_cmpint(nqid, ==, 1); req = v9fs_tlopen(v9p, fid, O_DIRECTORY, 0); @@ -266,18 +260,12 @@ static void fs_walk_no_slash(void *obj, void *data, QGuestAllocator *t_alloc) QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); char *wnames[] = { g_strdup(" /") }; -P9Req *req; -uint32_t err; do_attach(v9p); -req = twalk({ +twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlerror(req, &err); - -g_assert_cmpint(err, ==, ENOENT); +.expectErr = ENOENT +}); g_free(wnames[0]); } @@ -312,7 +300,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, do_attach_rqid(v9p, &root_qid); fid = twalk({ .client = v9p, .path = path, -.rwalk.nwqid = &nwqid, .rwalk.wqid = &wqid +.rwalk = { .nwqid = &nwqid, .wqid = &wqid } }).newfid; /* * The 9p2000 protocol spec says: "nwqid is therefore either nwname or the @@ -345,12 +333,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); -req = twalk({ +twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 0, .wnames = NULL, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwalk(req, NULL, &wqid); +.rwalk.wqid = &wqid +}); /* special case: no QID is returned if nwname=0 was sent */ g_assert(wqid == NULL); @@ -376,12 +362,10 @@ static void fs_walk_dotdot(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_req_wait_for_reply(req, NULL); v9fs_rattach(req, &root_qid); -req = twalk({ +twalk({ .client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwalk(req, NULL, &wqid); /* We now we'll get one qid */ +.rwalk.wqid = &wqid /* We now we'll get one qid */ +}); g_assert_cmpmem(&root_qid, 13, wqid[0], 13); @@ -396,12 +380,9 @@ static void fs_lopen(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req; do_attach(v9p); -req = twalk({ -.client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwalk(req, NULL, NULL); +twalk({ +.client = v9p, .fid = 0, .newfid = 1, .nwname = 1, .wnames = wnames +}); req = v9fs_tlopen(v9p, 1, O_WRONLY, 0); v9fs_req_wait_for_reply(req, NULL); @@ -421,12 +402,9 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) P9Req *req;
Re: [PATCH v13 17/17] net: stream: add QAPI events to report connection state
Markus Armbruster writes: > Cc: Stefano Brivio > > Laurent Vivier writes: > >> On 10/21/22 07:48, Markus Armbruster wrote: >>> Laurent Vivier writes: >>> The netdev reports NETDEV_STREAM_CONNECTED event when the backend is connected, and NETDEV_STREAM_DISCONNECTED when it is disconnected. >>> >>> Use cases? >> >> This is asked by Stefano Brivio to allow libvirt to detect if connection to >> passt is lost and to restart passt. [...] >>> Could similar event signalling be useful for other kinds of netdev >>> backends? >> >> I was wondering, but it becomes more complicated to be generic. > > Making something complicated and generic where a simpler special > solution would do is the worst. > > Not quite as bad (but still plenty bad) is making a few special > solutions first, then replace them all with a generic solution. > > I believe we should have a good, hard think on possible applications of > a generic solution now. > > There is no need to hold back this series for that. > > If we conclude a generic solution is called for, we better replace this > special solution before it becomes ABI. Either by replacing it before > we release it, or by keeping it unstable until we replace it. Stefano, any thoughts on this?
[PULL 19/23] tests/9p: merge v9fs_tlcreate() and do_lcreate()
As with previous patches, unify those 2 functions into a single function v9fs_tlcreate() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <4c01b2caa5f5b54a2020fc92701deadd2abf0571.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 45 +-- tests/qtest/libqos/virtio-9p-client.h | 39 +-- tests/qtest/virtio-9p-test.c | 30 +- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index c374ba2048..5c805a133c 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -827,11 +827,26 @@ void v9fs_rmkdir(P9Req *req, v9fs_qid *qid) } /* size[4] Tlcreate tag[2] fid[4] name[s] flags[4] mode[4] gid[4] */ -P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, - uint32_t flags, uint32_t mode, uint32_t gid, - uint16_t tag) +TlcreateRes v9fs_tlcreate(TlcreateOpt opt) { P9Req *req; +uint32_t err; +g_autofree char *name = g_strdup(opt.name); + +g_assert(opt.client); +/* expecting either hi-level atPath or low-level fid, but not both */ +g_assert(!opt.atPath || !opt.fid); +/* expecting either Rlcreate or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !(opt.rlcreate.qid || opt.rlcreate.iounit)); + +if (opt.atPath) { +opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; +} + +if (!opt.mode) { +opt.mode = 0750; +} uint32_t body_size = 4 + 4 + 4 + 4; uint16_t string_size = v9fs_string_size(name); @@ -839,14 +854,26 @@ P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; -req = v9fs_req_init(v9p, body_size, P9_TLCREATE, tag); -v9fs_uint32_write(req, fid); +req = v9fs_req_init(opt.client, body_size, P9_TLCREATE, opt.tag); +v9fs_uint32_write(req, opt.fid); v9fs_string_write(req, name); -v9fs_uint32_write(req, flags); -v9fs_uint32_write(req, mode); -v9fs_uint32_write(req, gid); +v9fs_uint32_write(req, opt.flags); +v9fs_uint32_write(req, opt.mode); +v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rlcreate(req, opt.rlcreate.qid, opt.rlcreate.iounit); +} +req = NULL; /* request was freed */ +} + +return (TlcreateRes) { .req = req }; } /* size[4] Rlcreate tag[2] qid[13] iounit[4] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index ae44f95a4d..8916b1c7aa 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -320,6 +320,41 @@ typedef struct TMkdirRes { P9Req *req; } TMkdirRes; +/* options for 'Tlcreate' 9p request */ +typedef struct TlcreateOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* low-level variant of directory where new file shall be created */ +uint32_t fid; +/* high-level variant of directory where new file shall be created */ +const char *atPath; +/* name of new file (required) */ +const char *name; +/* Linux kernel intent bits */ +uint32_t flags; +/* Linux create(2) mode bits */ +uint32_t mode; +/* effective group ID of caller */ +uint32_t gid; +/* data being received from 9p server as 'Rlcreate' response (optional) */ +struct { +v9fs_qid *qid; +uint32_t *iounit; +} rlcreate; +/* only send Tlcreate request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TlcreateOpt; + +/* result of 'Tlcreate' 9p request */ +typedef struct TlcreateRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TlcreateRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -361,9 +396,7 @@ TFlushRes v9fs_tflush(TFlushOpt); void v9fs_rflush(P9Req *req); TMkdirRes v9fs_tmkdir(TMkdirOpt); void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); -P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, - uint32_t flags, uint32_t mode, uint32_t gid, - uint16_t tag); +TlcreateRes v9fs_tlcreate(Tlc
[PULL 20/23] tests/9p: merge v9fs_tsymlink() and do_symlink()
As with previous patches, unify those 2 functions into a single function v9fs_tsymlink() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: <563f3ad04fe596ce0ae1e2654d1d08237f18c830.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 37 ++- tests/qtest/libqos/virtio-9p-client.h | 35 +++-- tests/qtest/virtio-9p-test.c | 27 +++ 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 5c805a133c..89eaf50355 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -892,10 +892,23 @@ void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit) } /* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4] */ -P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, - const char *symtgt, uint32_t gid, uint16_t tag) +TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt) { P9Req *req; +uint32_t err; +g_autofree char *name = g_strdup(opt.name); +g_autofree char *symtgt = g_strdup(opt.symtgt); + +g_assert(opt.client); +/* expecting either hi-level atPath or low-level fid, but not both */ +g_assert(!opt.atPath || !opt.fid); +/* expecting either Rsymlink or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !opt.rsymlink.qid); + +if (opt.atPath) { +opt.fid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; +} uint32_t body_size = 4 + 4; uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt); @@ -903,13 +916,25 @@ P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; -req = v9fs_req_init(v9p, body_size, P9_TSYMLINK, tag); -v9fs_uint32_write(req, fid); +req = v9fs_req_init(opt.client, body_size, P9_TSYMLINK, opt.tag); +v9fs_uint32_write(req, opt.fid); v9fs_string_write(req, name); v9fs_string_write(req, symtgt); -v9fs_uint32_write(req, gid); +v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rsymlink(req, opt.rsymlink.qid); +} +req = NULL; /* request was freed */ +} + +return (TsymlinkRes) { .req = req }; } /* size[4] Rsymlink tag[2] qid[13] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 8916b1c7aa..b905a54966 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -355,6 +355,38 @@ typedef struct TlcreateRes { P9Req *req; } TlcreateRes; +/* options for 'Tsymlink' 9p request */ +typedef struct TsymlinkOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* low-level variant of directory where symlink shall be created */ +uint32_t fid; +/* high-level variant of directory where symlink shall be created */ +const char *atPath; +/* name of symlink (required) */ +const char *name; +/* where symlink will point to (required) */ +const char *symtgt; +/* effective group ID of caller */ +uint32_t gid; +/* data being received from 9p server as 'Rsymlink' response (optional) */ +struct { +v9fs_qid *qid; +} rsymlink; +/* only send Tsymlink request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TsymlinkOpt; + +/* result of 'Tsymlink' 9p request */ +typedef struct TsymlinkRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TsymlinkRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -398,8 +430,7 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt); void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); TlcreateRes v9fs_tlcreate(TlcreateOpt); void v9fs_rlcreate(P9Req *req, v9fs_qid *qid, uint32_t *iounit); -P9Req *v9fs_tsymlink(QVirtio9P *v9p, uint32_t fid, const char *name, - const char *symtgt, uint32_t gid, uint16_t tag); +TsymlinkRes v9fs_tsymlink(TsymlinkOpt); void v9fs_rsymlink(P9Req *req, v9fs_qid *qid); P9Req *v9fs_tlink(QVirtio9P *v9p, uint32_t dfid, uint32_t fid, const char *name, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qt
[PULL 07/23] tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
As with previous patches, unify those 3 functions into a single function v9fs_tattach() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 40 ++--- tests/qtest/libqos/virtio-9p-client.h | 30 - tests/qtest/virtio-9p-test.c | 62 +++ 3 files changed, 88 insertions(+), 44 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index e8364f8d64..5e6bd6120c 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -359,20 +359,48 @@ void v9fs_rversion(P9Req *req, uint16_t *len, char **version) } /* size[4] Tattach tag[2] fid[4] afid[4] uname[s] aname[s] n_uname[4] */ -P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, -uint16_t tag) +TAttachRes v9fs_tattach(TAttachOpt opt) { +uint32_t err; const char *uname = ""; /* ignored by QEMU */ const char *aname = ""; /* ignored by QEMU */ -P9Req *req = v9fs_req_init(v9p, 4 + 4 + 2 + 2 + 4, P9_TATTACH, tag); -v9fs_uint32_write(req, fid); +g_assert(opt.client); +/* expecting either Rattach or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !opt.rattach.qid); + +if (!opt.requestOnly) { +v9fs_tversion((TVersionOpt) { .client = opt.client }); +} + +if (!opt.n_uname) { +opt.n_uname = getuid(); +} + +P9Req *req = v9fs_req_init(opt.client, 4 + 4 + 2 + 2 + 4, P9_TATTACH, + opt.tag); + +v9fs_uint32_write(req, opt.fid); v9fs_uint32_write(req, P9_NOFID); v9fs_string_write(req, uname); v9fs_string_write(req, aname); -v9fs_uint32_write(req, n_uname); +v9fs_uint32_write(req, opt.n_uname); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rattach(req, opt.rattach.qid); +} +req = NULL; /* request was freed */ +} + +return (TAttachRes) { +.req = req, +}; } /* size[4] Rattach tag[2] qid[13] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index fcde849b5d..64b97b229b 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -128,6 +128,33 @@ typedef struct TVersionRes { P9Req *req; } TVersionRes; +/* options for 'Tattach' 9p request */ +typedef struct TAttachOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* file ID to be associated with root of file tree (optional) */ +uint32_t fid; +/* numerical uid of user being introduced to server (optional) */ +uint32_t n_uname; +/* data being received from 9p server as 'Rattach' response (optional) */ +struct { +/* server's idea of the root of the file tree */ +v9fs_qid *qid; +} rattach; +/* only send Tattach request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TAttachOpt; + +/* result of 'Tattach' 9p request */ +typedef struct TAttachRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TAttachRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -151,8 +178,7 @@ void v9fs_req_free(P9Req *req); void v9fs_rlerror(P9Req *req, uint32_t *err); TVersionRes v9fs_tversion(TVersionOpt); void v9fs_rversion(P9Req *req, uint16_t *len, char **version); -P9Req *v9fs_tattach(QVirtio9P *v9p, uint32_t fid, uint32_t n_uname, -uint16_t tag); +TAttachRes v9fs_tattach(TAttachOpt); void v9fs_rattach(P9Req *req, v9fs_qid *qid); TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index f2907c8026..271c42f6f9 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -18,6 +18,7 @@ #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) +#define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -48,25 +49,10 @@ static void fs_version(void *obj, void *data, QGuestAllocator *t_alloc) tversion({ .client = obj }); } -static void do_attach_rqid(QVirtio9P *v9p, v9fs_qid *qid) -{ -P9Req *req; - -tversion({ .client = v9p }); -
[PULL 15/23] tests/9p: convert v9fs_twrite() to declarative arguments
Use declarative function arguments for function v9fs_twrite(). Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 38 --- tests/qtest/libqos/virtio-9p-client.h | 31 -- tests/qtest/virtio-9p-test.c | 18 ++--- 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 15fde54d63..9ae347fad5 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -687,21 +687,39 @@ void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit) } /* size[4] Twrite tag[2] fid[4] offset[8] count[4] data[count] */ -P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, const void *data, uint16_t tag) +TWriteRes v9fs_twrite(TWriteOpt opt) { P9Req *req; +uint32_t err; uint32_t body_size = 4 + 8 + 4; +uint32_t written = 0; -g_assert_cmpint(body_size, <=, UINT32_MAX - count); -body_size += count; -req = v9fs_req_init(v9p, body_size, P9_TWRITE, tag); -v9fs_uint32_write(req, fid); -v9fs_uint64_write(req, offset); -v9fs_uint32_write(req, count); -v9fs_memwrite(req, data, count); +g_assert(opt.client); + +g_assert_cmpint(body_size, <=, UINT32_MAX - opt.count); +body_size += opt.count; +req = v9fs_req_init(opt.client, body_size, P9_TWRITE, opt.tag); +v9fs_uint32_write(req, opt.fid); +v9fs_uint64_write(req, opt.offset); +v9fs_uint32_write(req, opt.count); +v9fs_memwrite(req, opt.data, opt.count); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rwrite(req, &written); +} +req = NULL; /* request was freed */ +} + +return (TWriteRes) { +.req = req, +.count = written +}; } /* size[4] Rwrite tag[2] count[4] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 3b70aef51e..dda371c054 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -239,6 +239,34 @@ typedef struct TLOpenRes { P9Req *req; } TLOpenRes; +/* options for 'Twrite' 9p request */ +typedef struct TWriteOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* file ID of file to write to (required) */ +uint32_t fid; +/* start position of write from beginning of file (optional) */ +uint64_t offset; +/* how many bytes to write */ +uint32_t count; +/* data to be written */ +const void *data; +/* only send Twrite request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TWriteOpt; + +/* result of 'Twrite' 9p request */ +typedef struct TWriteRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +/* amount of bytes written */ +uint32_t count; +} TWriteRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -274,8 +302,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, void v9fs_free_dirents(struct V9fsDirent *e); TLOpenRes v9fs_tlopen(TLOpenOpt); void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); -P9Req *v9fs_twrite(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, const void *data, uint16_t tag); +TWriteRes v9fs_twrite(TWriteOpt); void v9fs_rwrite(P9Req *req, uint32_t *count); P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag); void v9fs_rflush(P9Req *req); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 60a030b877..a5b9284acb 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -22,6 +22,7 @@ #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) +#define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -385,7 +386,10 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); -req = v9fs_twrite(v9p, 1, 0, write_count, buf, 0); +req = twrite({ +.client = v9p, .fid = 1, .offset = 0, .count = write_count, +.data = buf, .requestOnly = true +}).req; v9fs_req_wait_for
Re: [PATCH v7 3/5] module: add Error arguments to module_load and module_load_qom
On 9/30/22 13:50, Markus Armbruster wrote: > Claudio Fontana writes: > >> On 9/28/22 13:31, Markus Armbruster wrote: >>> Claudio Fontana writes: >>> improve error handling during module load, by changing: bool module_load(const char *prefix, const char *lib_name); void module_load_qom(const char *type); to: int module_load(const char *prefix, const char *name, Error **errp); int module_load_qom(const char *type, Error **errp); where the return value is: -1 on module load error, and errp is set with the error 0 on module or one of its dependencies are not installed 1 on module load success 2 on module load success (module already loaded or built-in) >>> >>> Two changes, if I understand things correctly: >>> >>> 1. Convert to Error API from fprintf(stderr, ...) >>> >>> 2. Return a more useful value >>> >>> Right? >> >> Yes. >> >>> >>> Do you add any new errors here that weren't reported before? Just >> >> Yes. > > Thanks! > >>> trying to calibrate my expectations before I dig into the actual patch. >>> module_load_qom_one has been introduced in: commit 28457744c345 ("module: qom module support"), which built on top of module_load_one, but discarded the bool return value. Restore it. Adapt all callers to emit errors, or ignore them, or fail hard, as appropriate in each context. Some memory leaks also fixed as part of the module_load changes. > > [...] > audio: when attempting to load an audio module, report module load errors. block: when attempting to load a block module, report module load errors. console: when attempting to load a display module, report module load errors. qdev: when creating a new qdev Device object (DeviceState), report load errors. If a module cannot be loaded to create that device, now abort execution. qom/object.c: when initializing a QOM object, or looking up class_by_name, report module load errors. qtest: when processing the "module_load" qtest command, report errors in the load of the module. >>> >>> This looks like a list of behavioral changes. Appreciated! It's a bit >>> terse, though. I might come back to this and suggest improvement. But >>> first, I need to understand the patch. >>> Signed-off-by: Claudio Fontana --- audio/audio.c | 16 ++-- block.c | 20 +++- block/dmg.c | 14 ++- hw/core/qdev.c| 17 +++- include/qemu/module.h | 37 +++- qom/object.c | 18 +++- softmmu/qtest.c | 8 +- ui/console.c | 18 +++- util/module.c | 211 +++--- 9 files changed, 235 insertions(+), 124 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 0a682336a0..ea51793843 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -72,20 +72,24 @@ void audio_driver_register(audio_driver *drv) audio_driver *audio_driver_lookup(const char *name) { struct audio_driver *d; +Error *local_err = NULL; +int rv; QLIST_FOREACH(d, &audio_drivers, next) { if (strcmp(name, d->name) == 0) { return d; } } - -audio_module_load(name); -QLIST_FOREACH(d, &audio_drivers, next) { -if (strcmp(name, d->name) == 0) { -return d; +rv = audio_module_load(name, &local_err); +if (rv > 0) { +QLIST_FOREACH(d, &audio_drivers, next) { +if (strcmp(name, d->name) == 0) { +return d; +} } +} else if (rv < 0) { +error_report_err(local_err); } - return NULL; } >>> >>> Before: audio_module_load() reports to stderr, but the caller can't >> >> before: reports _some_ errors to stderr > > Thanks for the reminder. > >>> know. So, error or no error, search the driver registry for the one we >>> want. Return it if found, else fail. >>> >>> After: if audio_module_load() fails, report to stderr or current >>> monitor, and fail. If it could find no module or loaded one, search the >>> driver registry. Return it if found, else fail. >>> >>> What if audio_module_load() fails, but a search for the driver succeeds? >>> Before the patch, we succeed. >> >> audio_module_load() is the only way that audio_drivers can be updated and >> the search would return a different result. > > Not true. > > @audio_driver gets built with audio_driver_register(). Audio drivers > call it via type_init(). For instance: > > static void register_audio_none(void) > { > audio_driver_register(&no_audio_driver); > } > type_init(register_audio_none); > > My build
[PULL 09/23] tests/9p: convert v9fs_tgetattr() to declarative arguments
Use declarative function arguments for function v9fs_tgetattr(). Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 32 ++- tests/qtest/libqos/virtio-9p-client.h | 30 +++-- tests/qtest/virtio-9p-test.c | 11 +++-- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 5e6bd6120c..29916a23b5 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -489,16 +489,36 @@ void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid) } /* size[4] Tgetattr tag[2] fid[4] request_mask[8] */ -P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, - uint16_t tag) +TGetAttrRes v9fs_tgetattr(TGetAttrOpt opt) { P9Req *req; +uint32_t err; -req = v9fs_req_init(v9p, 4 + 8, P9_TGETATTR, tag); -v9fs_uint32_write(req, fid); -v9fs_uint64_write(req, request_mask); +g_assert(opt.client); +/* expecting either Rgetattr or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !opt.rgetattr.attr); + +if (!opt.request_mask) { +opt.request_mask = P9_GETATTR_ALL; +} + +req = v9fs_req_init(opt.client, 4 + 8, P9_TGETATTR, opt.tag); +v9fs_uint32_write(req, opt.fid); +v9fs_uint64_write(req, opt.request_mask); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rgetattr(req, opt.rgetattr.attr); +} +req = NULL; /* request was freed */ +} + +return (TGetAttrRes) { .req = req }; } /* diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index 64b97b229b..f7b1bfc79a 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -63,6 +63,7 @@ typedef struct v9fs_attr { } v9fs_attr; #define P9_GETATTR_BASIC0x07ffULL /* Mask for fields up to BLOCKS */ +#define P9_GETATTR_ALL 0x3fffULL /* Mask for ALL fields */ struct V9fsDirent { v9fs_qid qid; @@ -155,6 +156,32 @@ typedef struct TAttachRes { P9Req *req; } TAttachRes; +/* options for 'Tgetattr' 9p request */ +typedef struct TGetAttrOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* file ID of file/dir whose attributes shall be retrieved (required) */ +uint32_t fid; +/* bitmask indicating attribute fields to be retrieved (optional) */ +uint64_t request_mask; +/* data being received from 9p server as 'Rgetattr' response (optional) */ +struct { +v9fs_attr *attr; +} rgetattr; +/* only send Tgetattr request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TGetAttrOpt; + +/* result of 'Tgetattr' 9p request */ +typedef struct TGetAttrRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TGetAttrRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -182,8 +209,7 @@ TAttachRes v9fs_tattach(TAttachOpt); void v9fs_rattach(P9Req *req, v9fs_qid *qid); TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); -P9Req *v9fs_tgetattr(QVirtio9P *v9p, uint32_t fid, uint64_t request_mask, - uint16_t tag); +TGetAttrRes v9fs_tgetattr(TGetAttrOpt); void v9fs_rgetattr(P9Req *req, v9fs_attr *attr); P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, uint32_t count, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 46bb189b81..9c1219db33 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -19,6 +19,7 @@ #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__) #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) +#define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -285,7 +286,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0])); /* expect fid being unaffected by walk above */ -req = v9fs_tgetattr(v9p, fid, P9_GETATTR_BASIC, 0); +req = tgetattr({ +.client = v9p, .fid = fid, .request_mask = P9_GETATTR_BASIC, +.requestOnly = true +}).req; v
Re: [PATCH v7 for-7.2 00/15] block: cleanup backing and file handling
Am 26.07.2022 um 22:11 hat Vladimir Sementsov-Ogievskiy geschrieben: > Hi all! > > That's the first part of > "[PATCH v5 00/45] Transactional block-graph modifying API", > updated and is fully reviewed by Hanna. > > v7: add r-bs and rebase on master Thanks, applied to the block branch. Kevin
Re: [PATCH v2] hw/mem/nvdimm: fix error message for 'unarmed' flag
On 23.10.22 21:58, Julia Suvorova wrote: In the ACPI specification [1], the 'unarmed' bit is set when a device cannot accept a persistent write. This means that when a memdev is read-only, the 'unarmed' flag must be turned on. The logic is correct, just changing the error message. [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3 Fixes: dbd730e859 ("nvdimm: check -object memory-backend-file, readonly=on option") Signed-off-by: Julia Suvorova Reviewed-by: Stefan Hajnoczi Reviewed-by: Pankaj Gupta Reviewed-by: Philippe Mathieu-Daudé Acked-by: David Hildenbrand --- v2: * enquote 'on' [Philippe] hw/mem/nvdimm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index 7c7d81..31080c22c9 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -149,7 +149,7 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp) if (!nvdimm->unarmed && memory_region_is_rom(mr)) { HostMemoryBackend *hostmem = dimm->hostmem; -error_setg(errp, "'unarmed' property must be off since memdev %s " +error_setg(errp, "'unarmed' property must be 'on' since memdev %s " "is read-only", object_get_canonical_path_component(OBJECT(hostmem))); return; Thanks, queued to https://github.com/davidhildenbrand/qemu.git mem-next -- Thanks, David / dhildenb
[PULL 18/23] tests/9p: merge v9fs_tmkdir() and do_mkdir()
As with previous patches, unify those 2 functions into a single function v9fs_tmkdir() by using a declarative function arguments approach. Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 42 ++- tests/qtest/libqos/virtio-9p-client.h | 36 +-- tests/qtest/virtio-9p-test.c | 30 ++- 3 files changed, 78 insertions(+), 30 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 3be0ffc7da..c374ba2048 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -766,10 +766,26 @@ void v9fs_rflush(P9Req *req) } /* size[4] Tmkdir tag[2] dfid[4] name[s] mode[4] gid[4] */ -P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, - uint32_t mode, uint32_t gid, uint16_t tag) +TMkdirRes v9fs_tmkdir(TMkdirOpt opt) { P9Req *req; +uint32_t err; +g_autofree char *name = g_strdup(opt.name); + +g_assert(opt.client); +/* expecting either hi-level atPath or low-level dfid, but not both */ +g_assert(!opt.atPath || !opt.dfid); +/* expecting either Rmkdir or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !opt.rmkdir.qid); + +if (opt.atPath) { +opt.dfid = v9fs_twalk((TWalkOpt) { .client = opt.client, + .path = opt.atPath }).newfid; +} + +if (!opt.mode) { +opt.mode = 0750; +} uint32_t body_size = 4 + 4 + 4; uint16_t string_size = v9fs_string_size(name); @@ -777,13 +793,25 @@ P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; -req = v9fs_req_init(v9p, body_size, P9_TMKDIR, tag); -v9fs_uint32_write(req, dfid); +req = v9fs_req_init(opt.client, body_size, P9_TMKDIR, opt.tag); +v9fs_uint32_write(req, opt.dfid); v9fs_string_write(req, name); -v9fs_uint32_write(req, mode); -v9fs_uint32_write(req, gid); +v9fs_uint32_write(req, opt.mode); +v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rmkdir(req, opt.rmkdir.qid); +} +req = NULL; /* request was freed */ +} + +return (TMkdirRes) { .req = req }; } /* size[4] Rmkdir tag[2] qid[13] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index b22b54c720..ae44f95a4d 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -287,6 +287,39 @@ typedef struct TFlushRes { P9Req *req; } TFlushRes; +/* options for 'Tmkdir' 9p request */ +typedef struct TMkdirOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* low level variant of directory where new one shall be created */ +uint32_t dfid; +/* high-level variant of directory where new one shall be created */ +const char *atPath; +/* New directory's name (required) */ +const char *name; +/* Linux mkdir(2) mode bits (optional) */ +uint32_t mode; +/* effective group ID of caller */ +uint32_t gid; +/* data being received from 9p server as 'Rmkdir' response (optional) */ +struct { +/* QID of newly created directory */ +v9fs_qid *qid; +} rmkdir; +/* only send Tmkdir request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TMkdirOpt; + +/* result of 'TMkdir' 9p request */ +typedef struct TMkdirRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TMkdirRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -326,8 +359,7 @@ TWriteRes v9fs_twrite(TWriteOpt); void v9fs_rwrite(P9Req *req, uint32_t *count); TFlushRes v9fs_tflush(TFlushOpt); void v9fs_rflush(P9Req *req); -P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, - uint32_t mode, uint32_t gid, uint16_t tag); +TMkdirRes v9fs_tmkdir(TMkdirOpt); void v9fs_rmkdir(P9Req *req, v9fs_qid *qid); P9Req *v9fs_tlcreate(QVirtio9P *v9p, uint32_t fid, const char *name, uint32_t flags, uint32_t mode, uint32_t gid, diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 5544998bac..6d75afee87 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -24,6 +24,7
[PULL 10/23] tests/9p: simplify callers of tgetattr()
Now as tgetattr() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <60c6a083f320b86f3172951445df7bbc895932e2.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 22 +++--- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 9c1219db33..ae1220d0cb 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -264,8 +264,7 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, v9fs_set_allocator(t_alloc); v9fs_qid root_qid; uint16_t nwqid; -uint32_t fid, err; -P9Req *req; +uint32_t fid; g_autofree v9fs_qid *wqid = NULL; g_autofree char *path = g_strdup_printf( QTEST_V9FS_SYNTH_WALK_FILE "/non-existent", 0 @@ -286,14 +285,10 @@ static void fs_walk_2nd_nonexistent(void *obj, void *data, g_assert(wqid && wqid[0] && !is_same_qid(root_qid, wqid[0])); /* expect fid being unaffected by walk above */ -req = tgetattr({ +tgetattr({ .client = v9p, .fid = fid, .request_mask = P9_GETATTR_BASIC, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rlerror(req, &err); - -g_assert_cmpint(err, ==, ENOENT); +.expectErr = ENOENT +}); } static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) @@ -302,7 +297,6 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) v9fs_set_allocator(t_alloc); v9fs_qid root_qid; g_autofree v9fs_qid *wqid = NULL; -P9Req *req; struct v9fs_attr attr; tversion({ .client = v9p }); @@ -319,12 +313,10 @@ static void fs_walk_none(void *obj, void *data, QGuestAllocator *t_alloc) /* special case: no QID is returned if nwname=0 was sent */ g_assert(wqid == NULL); -req = tgetattr({ +tgetattr({ .client = v9p, .fid = 1, .request_mask = P9_GETATTR_BASIC, -.requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rgetattr(req, &attr); +.rgetattr.attr = &attr +}); g_assert(is_same_qid(root_qid, attr.qid)); } -- 2.30.2
Re: [PULL 00/23] 9p queue 2022-10-24
On Monday, October 24, 2022 12:54:23 PM CEST Christian Schoenebeck wrote: > The following changes since commit 0529245488865038344d64fff7ee05864d3d17f6: > > Merge tag 'pull-target-arm-20221020' of > https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-10-20 > 14:36:12 -0400) > > are available in the Git repository at: > > https://github.com/cschoenebeck/qemu.git tags/pull-9p-20221024 > > for you to fetch changes up to 3ce77865bf813f313cf79c00fd951bfc95a50165: > > tests/9p: remove unnecessary g_strdup() calls (2022-10-24 12:24:32 +0200) > > > 9pfs: performance, Windows host prep, tests restructure > > * Highlight of this PR is Linus Heckemann's GHashTable patch which > brings massive general performance improvements of 9p server > somewhere between factor 6 .. 12. > > * Bin Meng's g_mkdir patch is a preparatory patch for upcoming > Windows host support of 9p server. > > * The rest of the patches in this PR are 9p test code restructuring > and refactoring changes to improve readability and to ease > maintenance of 9p test code on the long-term. Unfortunately I haven't found any reviewer of my patches 04 .. 23. :/ I decided to queue them anyway, as they are just restructuring and refactoring of 9p test code: https://lore.kernel.org/all/cover.1664917004.git.qemu_...@crudebyte.com If anybody finds some time at least for a glimpse on them, very much appreciated! Best regards, Christian Schoenebeck > > Bin Meng (1): > fsdev/virtfs-proxy-helper: Use g_mkdir() > > Christian Schoenebeck (21): > tests/9p: split virtio-9p-test.c into tests and 9p client part > tests/9p: merge *walk*() functions > tests/9p: simplify callers of twalk() > tests/9p: merge v9fs_tversion() and do_version() > tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid() > tests/9p: simplify callers of tattach() > tests/9p: convert v9fs_tgetattr() to declarative arguments > tests/9p: simplify callers of tgetattr() > tests/9p: convert v9fs_treaddir() to declarative arguments > tests/9p: simplify callers of treaddir() > tests/9p: convert v9fs_tlopen() to declarative arguments > tests/9p: simplify callers of tlopen() > tests/9p: convert v9fs_twrite() to declarative arguments > tests/9p: simplify callers of twrite() > tests/9p: convert v9fs_tflush() to declarative arguments > tests/9p: merge v9fs_tmkdir() and do_mkdir() > tests/9p: merge v9fs_tlcreate() and do_lcreate() > tests/9p: merge v9fs_tsymlink() and do_symlink() > tests/9p: merge v9fs_tlink() and do_hardlink() > tests/9p: merge v9fs_tunlinkat() and do_unlinkat() > tests/9p: remove unnecessary g_strdup() calls > > Linus Heckemann (1): > 9pfs: use GHashTable for fid table > > fsdev/virtfs-proxy-helper.c |3 +- > hw/9pfs/9p.c | 196 ++--- > hw/9pfs/9p.h |2 +- > tests/qtest/libqos/meson.build|1 + > tests/qtest/libqos/virtio-9p-client.c | 1049 ++ > tests/qtest/libqos/virtio-9p-client.h | 494 + > tests/qtest/virtio-9p-test.c | 1299 > ++--- > 7 files changed, 1867 insertions(+), 1177 deletions(-) > create mode 100644 tests/qtest/libqos/virtio-9p-client.c > create mode 100644 tests/qtest/libqos/virtio-9p-client.h > >
Re: [PATCH] treewide: Remove the unnecessary space before semicolon
On Monday, October 24, 2022 9:28:02 AM CEST Bin Meng wrote: > %s/return ;/return; > > Signed-off-by: Bin Meng > --- Reviewed-by: Christian Schoenebeck > > include/hw/elf_ops.h | 2 +- > hw/9pfs/9p.c | 2 +- > hw/dma/pl330.c | 2 +- > hw/net/can/can_sja1000.c | 2 +- > hw/timer/renesas_cmt.c | 2 +- > hw/timer/renesas_tmr.c | 8 > hw/virtio/virtio-pci.c | 2 +- > target/riscv/vector_helper.c | 2 +- > target/rx/op_helper.c| 4 ++-- > ui/vnc-jobs.c| 2 +- > ui/vnc.c | 2 +- > 11 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index 7c3b1d0f6c..fbe0b1e956 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -117,7 +117,7 @@ static void glue(load_symbols, SZ)(struct elfhdr *ehdr, > int fd, int must_swab, > shdr_table = load_at(fd, ehdr->e_shoff, > sizeof(struct elf_shdr) * ehdr->e_shnum); > if (!shdr_table) { > -return ; > +return; > } > > if (must_swab) { > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index aebadeaa03..76c591a01b 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -1786,7 +1786,7 @@ static void coroutine_fn v9fs_walk(void *opaque) > err = pdu_unmarshal(pdu, offset, "ddw", &fid, &newfid, &nwnames); > if (err < 0) { > pdu_complete(pdu, err); > -return ; > +return; > } > offset += err; > > diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c > index 08e5938ec7..e5d521c329 100644 > --- a/hw/dma/pl330.c > +++ b/hw/dma/pl330.c > @@ -1328,7 +1328,7 @@ static void pl330_debug_exec(PL330State *s) > } > if (!insn) { > pl330_fault(ch, PL330_FAULT_UNDEF_INSTR | PL330_FAULT_DBG_INSTR); > -return ; > +return; > } > ch->stall = 0; > insn->exec(ch, opcode, args, insn->size - 1); > diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c > index e0f76d3eb3..73201f9139 100644 > --- a/hw/net/can/can_sja1000.c > +++ b/hw/net/can/can_sja1000.c > @@ -431,7 +431,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr addr, > uint64_t val, > (unsigned long long)val, (unsigned int)addr); > > if (addr > CAN_SJA_MEM_SIZE) { > -return ; > +return; > } > > if (s->clock & 0x80) { /* PeliCAN Mode */ > diff --git a/hw/timer/renesas_cmt.c b/hw/timer/renesas_cmt.c > index 2e0fd21a36..69eabc678a 100644 > --- a/hw/timer/renesas_cmt.c > +++ b/hw/timer/renesas_cmt.c > @@ -57,7 +57,7 @@ static void update_events(RCMTState *cmt, int ch) > > if ((cmt->cmstr & (1 << ch)) == 0) { > /* count disable, so not happened next event. */ > -return ; > +return; > } > next_time = cmt->cmcor[ch] - cmt->cmcnt[ch]; > next_time *= NANOSECONDS_PER_SECOND; > diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c > index d96002e1ee..c15f654738 100644 > --- a/hw/timer/renesas_tmr.c > +++ b/hw/timer/renesas_tmr.c > @@ -67,18 +67,18 @@ static void update_events(RTMRState *tmr, int ch) > int i, event; > > if (tmr->tccr[ch] == 0) { > -return ; > +return; > } > if (FIELD_EX8(tmr->tccr[ch], TCCR, CSS) == 0) { > /* external clock mode */ > /* event not happened */ > -return ; > +return; > } > if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CSS_CASCADING) { > /* cascading mode */ > if (ch == 1) { > tmr->next[ch] = none; > -return ; > +return; > } > diff[cmia] = concat_reg(tmr->tcora) - concat_reg(tmr->tcnt); > diff[cmib] = concat_reg(tmr->tcorb) - concat_reg(tmr->tcnt); > @@ -384,7 +384,7 @@ static void timer_events(RTMRState *tmr, int ch) > tmr->tcorb[ch]) & 0xff; > } else { > if (ch == 1) { > -return ; > +return; > } > tcnt = issue_event(tmr, ch, 16, > concat_reg(tmr->tcnt), > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index e7d80242b7..34db51e241 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1675,7 +1675,7 @@ static void virtio_pci_device_plugged(DeviceState *d, > Error **errp) > if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { > error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by" > " neither legacy nor transitional device"); > -return ; > +return; > } > /* > * Legacy and transitional devices use specific subsystem IDs. > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c > index b94f809eb3..0020b9a95d 100644 > --- a/target/riscv/vector_helper.c > +++ b/target/riscv/vector_helper.c > @@ -211,7 +211,7 @@ static void vext_set_elems_1s(void *b
[PULL 17/23] tests/9p: convert v9fs_tflush() to declarative arguments
Use declarative function arguments for function v9fs_tflush(). Signed-off-by: Christian Schoenebeck Message-Id: <91b7b154298c500d100b05137146c2905c3acdec.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 23 +++ tests/qtest/libqos/virtio-9p-client.h | 22 +- tests/qtest/virtio-9p-test.c | 9 +++-- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 9ae347fad5..3be0ffc7da 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -733,14 +733,29 @@ void v9fs_rwrite(P9Req *req, uint32_t *count) } /* size[4] Tflush tag[2] oldtag[2] */ -P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag) +TFlushRes v9fs_tflush(TFlushOpt opt) { P9Req *req; +uint32_t err; -req = v9fs_req_init(v9p, 2, P9_TFLUSH, tag); -v9fs_uint32_write(req, oldtag); +g_assert(opt.client); + +req = v9fs_req_init(opt.client, 2, P9_TFLUSH, opt.tag); +v9fs_uint32_write(req, opt.oldtag); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rflush(req); +} +req = NULL; /* request was freed */ +} + +return (TFlushRes) { .req = req }; } /* size[4] Rflush tag[2] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index dda371c054..b22b54c720 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -267,6 +267,26 @@ typedef struct TWriteRes { uint32_t count; } TWriteRes; +/* options for 'Tflush' 9p request */ +typedef struct TFlushOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* message to flush (required) */ +uint16_t oldtag; +/* only send Tflush request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TFlushOpt; + +/* result of 'Tflush' 9p request */ +typedef struct TFlushRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TFlushRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -304,7 +324,7 @@ TLOpenRes v9fs_tlopen(TLOpenOpt); void v9fs_rlopen(P9Req *req, v9fs_qid *qid, uint32_t *iounit); TWriteRes v9fs_twrite(TWriteOpt); void v9fs_rwrite(P9Req *req, uint32_t *count); -P9Req *v9fs_tflush(QVirtio9P *v9p, uint16_t oldtag, uint16_t tag); +TFlushRes v9fs_tflush(TFlushOpt); void v9fs_rflush(P9Req *req); P9Req *v9fs_tmkdir(QVirtio9P *v9p, uint32_t dfid, const char *name, uint32_t mode, uint32_t gid, uint16_t tag); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 5ad7bebec7..5544998bac 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -23,6 +23,7 @@ #define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) #define tlopen(...) v9fs_tlopen((TLOpenOpt) __VA_ARGS__) #define twrite(...) v9fs_twrite((TWriteOpt) __VA_ARGS__) +#define tflush(...) v9fs_tflush((TFlushOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -420,7 +421,9 @@ static void fs_flush_success(void *obj, void *data, QGuestAllocator *t_alloc) .requestOnly = true }).req; -flush_req = v9fs_tflush(v9p, req->tag, 1); +flush_req = tflush({ +.client = v9p, .oldtag = req->tag, .tag = 1, .requestOnly = true +}).req; /* The write request is supposed to be flushed: the server should just * mark the write request as used and reply to the flush request. @@ -459,7 +462,9 @@ static void fs_flush_ignored(void *obj, void *data, QGuestAllocator *t_alloc) .requestOnly = true }).req; -flush_req = v9fs_tflush(v9p, req->tag, 1); +flush_req = tflush({ +.client = v9p, .oldtag = req->tag, .tag = 1, .requestOnly = true +}).req; /* The write request is supposed to complete. The server should * reply to the write request and the flush request. -- 2.30.2
Re: [PATCH] accel/tcg/tcg-accel-ops-rr: fix trivial typo
Le 21/10/2022 à 19:36, Matheus Tavares Bernardino a écrit : Signed-off-by: Matheus Tavares Bernardino --- accel/tcg/tcg-accel-ops-rr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c index cc8adc2380..cc912df108 100644 --- a/accel/tcg/tcg-accel-ops-rr.c +++ b/accel/tcg/tcg-accel-ops-rr.c @@ -51,7 +51,7 @@ void rr_kick_vcpu_thread(CPUState *unused) * * The kick timer is responsible for moving single threaded vCPU * emulation on to the next vCPU. If more than one vCPU is running a - * timer event with force a cpu->exit so the next vCPU can get + * timer event we force a cpu->exit so the next vCPU can get * scheduled. * * The timer is removed if all vCPUs are idle and restarted again once Applied to my trivial-patches branch. Thanks, Laurent
[PULL 01/23] fsdev/virtfs-proxy-helper: Use g_mkdir()
From: Bin Meng Use g_mkdir() to create a directory on all platforms. Signed-off-by: Bin Meng Reviewed-by: Christian Schoenebeck Message-Id: <20220927110632.1973965-27-bmeng...@gmail.com> Signed-off-by: Christian Schoenebeck --- fsdev/virtfs-proxy-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index 2dde27922f..5cafcd7703 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -10,6 +10,7 @@ */ #include "qemu/osdep.h" +#include #include #include #include @@ -639,7 +640,7 @@ static int do_create_others(int type, struct iovec *iovec) if (retval < 0) { goto err_out; } -retval = mkdir(path.data, mode); +retval = g_mkdir(path.data, mode); break; case T_SYMLINK: retval = proxy_unmarshal(iovec, offset, "ss", &oldpath, &path); -- 2.30.2
[PULL 23/23] tests/9p: remove unnecessary g_strdup() calls
This is a leftover from before the recent function merge and refactoring patches: As these functions do not return control to the caller in between, it is not necessary to duplicate strings passed to them. Signed-off-by: Christian Schoenebeck Message-Id: <0f80141cde3904ed0591354059da49d1d60bcdbc.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/libqos/virtio-9p-client.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index e017e030ec..e4a368e036 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -770,7 +770,6 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt opt) { P9Req *req; uint32_t err; -g_autofree char *name = g_strdup(opt.name); g_assert(opt.client); /* expecting either hi-level atPath or low-level dfid, but not both */ @@ -788,14 +787,14 @@ TMkdirRes v9fs_tmkdir(TMkdirOpt opt) } uint32_t body_size = 4 + 4 + 4; -uint16_t string_size = v9fs_string_size(name); +uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; req = v9fs_req_init(opt.client, body_size, P9_TMKDIR, opt.tag); v9fs_uint32_write(req, opt.dfid); -v9fs_string_write(req, name); +v9fs_string_write(req, opt.name); v9fs_uint32_write(req, opt.mode); v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); @@ -831,7 +830,6 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt opt) { P9Req *req; uint32_t err; -g_autofree char *name = g_strdup(opt.name); g_assert(opt.client); /* expecting either hi-level atPath or low-level fid, but not both */ @@ -849,14 +847,14 @@ TlcreateRes v9fs_tlcreate(TlcreateOpt opt) } uint32_t body_size = 4 + 4 + 4 + 4; -uint16_t string_size = v9fs_string_size(name); +uint16_t string_size = v9fs_string_size(opt.name); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; req = v9fs_req_init(opt.client, body_size, P9_TLCREATE, opt.tag); v9fs_uint32_write(req, opt.fid); -v9fs_string_write(req, name); +v9fs_string_write(req, opt.name); v9fs_uint32_write(req, opt.flags); v9fs_uint32_write(req, opt.mode); v9fs_uint32_write(req, opt.gid); @@ -896,8 +894,6 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt) { P9Req *req; uint32_t err; -g_autofree char *name = g_strdup(opt.name); -g_autofree char *symtgt = g_strdup(opt.symtgt); g_assert(opt.client); /* expecting either hi-level atPath or low-level fid, but not both */ @@ -911,15 +907,16 @@ TsymlinkRes v9fs_tsymlink(TsymlinkOpt opt) } uint32_t body_size = 4 + 4; -uint16_t string_size = v9fs_string_size(name) + v9fs_string_size(symtgt); +uint16_t string_size = v9fs_string_size(opt.name) + + v9fs_string_size(opt.symtgt); g_assert_cmpint(body_size, <=, UINT32_MAX - string_size); body_size += string_size; req = v9fs_req_init(opt.client, body_size, P9_TSYMLINK, opt.tag); v9fs_uint32_write(req, opt.fid); -v9fs_string_write(req, name); -v9fs_string_write(req, symtgt); +v9fs_string_write(req, opt.name); +v9fs_string_write(req, opt.symtgt); v9fs_uint32_write(req, opt.gid); v9fs_req_send(req); -- 2.30.2
Re: [PATCH] treewide: Remove the unnecessary space before semicolon
Le 24/10/2022 à 09:28, Bin Meng a écrit : %s/return ;/return; Signed-off-by: Bin Meng --- include/hw/elf_ops.h | 2 +- hw/9pfs/9p.c | 2 +- hw/dma/pl330.c | 2 +- hw/net/can/can_sja1000.c | 2 +- hw/timer/renesas_cmt.c | 2 +- hw/timer/renesas_tmr.c | 8 hw/virtio/virtio-pci.c | 2 +- target/riscv/vector_helper.c | 2 +- target/rx/op_helper.c| 4 ++-- ui/vnc-jobs.c| 2 +- ui/vnc.c | 2 +- 11 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index 7c3b1d0f6c..fbe0b1e956 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -117,7 +117,7 @@ static void glue(load_symbols, SZ)(struct elfhdr *ehdr, int fd, int must_swab, shdr_table = load_at(fd, ehdr->e_shoff, sizeof(struct elf_shdr) * ehdr->e_shnum); if (!shdr_table) { -return ; +return; } if (must_swab) { diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index aebadeaa03..76c591a01b 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1786,7 +1786,7 @@ static void coroutine_fn v9fs_walk(void *opaque) err = pdu_unmarshal(pdu, offset, "ddw", &fid, &newfid, &nwnames); if (err < 0) { pdu_complete(pdu, err); -return ; +return; } offset += err; diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c index 08e5938ec7..e5d521c329 100644 --- a/hw/dma/pl330.c +++ b/hw/dma/pl330.c @@ -1328,7 +1328,7 @@ static void pl330_debug_exec(PL330State *s) } if (!insn) { pl330_fault(ch, PL330_FAULT_UNDEF_INSTR | PL330_FAULT_DBG_INSTR); -return ; +return; } ch->stall = 0; insn->exec(ch, opcode, args, insn->size - 1); diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c index e0f76d3eb3..73201f9139 100644 --- a/hw/net/can/can_sja1000.c +++ b/hw/net/can/can_sja1000.c @@ -431,7 +431,7 @@ void can_sja_mem_write(CanSJA1000State *s, hwaddr addr, uint64_t val, (unsigned long long)val, (unsigned int)addr); if (addr > CAN_SJA_MEM_SIZE) { -return ; +return; } if (s->clock & 0x80) { /* PeliCAN Mode */ diff --git a/hw/timer/renesas_cmt.c b/hw/timer/renesas_cmt.c index 2e0fd21a36..69eabc678a 100644 --- a/hw/timer/renesas_cmt.c +++ b/hw/timer/renesas_cmt.c @@ -57,7 +57,7 @@ static void update_events(RCMTState *cmt, int ch) if ((cmt->cmstr & (1 << ch)) == 0) { /* count disable, so not happened next event. */ -return ; +return; } next_time = cmt->cmcor[ch] - cmt->cmcnt[ch]; next_time *= NANOSECONDS_PER_SECOND; diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c index d96002e1ee..c15f654738 100644 --- a/hw/timer/renesas_tmr.c +++ b/hw/timer/renesas_tmr.c @@ -67,18 +67,18 @@ static void update_events(RTMRState *tmr, int ch) int i, event; if (tmr->tccr[ch] == 0) { -return ; +return; } if (FIELD_EX8(tmr->tccr[ch], TCCR, CSS) == 0) { /* external clock mode */ /* event not happened */ -return ; +return; } if (FIELD_EX8(tmr->tccr[0], TCCR, CSS) == CSS_CASCADING) { /* cascading mode */ if (ch == 1) { tmr->next[ch] = none; -return ; +return; } diff[cmia] = concat_reg(tmr->tcora) - concat_reg(tmr->tcnt); diff[cmib] = concat_reg(tmr->tcorb) - concat_reg(tmr->tcnt); @@ -384,7 +384,7 @@ static void timer_events(RTMRState *tmr, int ch) tmr->tcorb[ch]) & 0xff; } else { if (ch == 1) { -return ; +return; } tcnt = issue_event(tmr, ch, 16, concat_reg(tmr->tcnt), diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index e7d80242b7..34db51e241 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1675,7 +1675,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by" " neither legacy nor transitional device"); -return ; +return; } /* * Legacy and transitional devices use specific subsystem IDs. diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index b94f809eb3..0020b9a95d 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -211,7 +211,7 @@ static void vext_set_elems_1s(void *base, uint32_t is_agnostic, uint32_t cnt, return; } if (tot - cnt == 0) { -return ; +return; } memset(base + cnt, -1, tot - cnt); } diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c index 9ca32dcc
[PULL 11/23] tests/9p: convert v9fs_treaddir() to declarative arguments
Use declarative function arguments for function v9fs_treaddir(). Signed-off-by: Christian Schoenebeck Message-Id: --- tests/qtest/libqos/virtio-9p-client.c | 32 -- tests/qtest/libqos/virtio-9p-client.h | 33 +-- tests/qtest/virtio-9p-test.c | 11 +++-- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c index 29916a23b5..047c8993b6 100644 --- a/tests/qtest/libqos/virtio-9p-client.c +++ b/tests/qtest/libqos/virtio-9p-client.c @@ -557,17 +557,35 @@ void v9fs_rgetattr(P9Req *req, v9fs_attr *attr) } /* size[4] Treaddir tag[2] fid[4] offset[8] count[4] */ -P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, uint16_t tag) +TReadDirRes v9fs_treaddir(TReadDirOpt opt) { P9Req *req; +uint32_t err; -req = v9fs_req_init(v9p, 4 + 8 + 4, P9_TREADDIR, tag); -v9fs_uint32_write(req, fid); -v9fs_uint64_write(req, offset); -v9fs_uint32_write(req, count); +g_assert(opt.client); +/* expecting either Rreaddir or Rlerror, but obviously not both */ +g_assert(!opt.expectErr || !(opt.rreaddir.count || + opt.rreaddir.nentries || opt.rreaddir.entries)); + +req = v9fs_req_init(opt.client, 4 + 8 + 4, P9_TREADDIR, opt.tag); +v9fs_uint32_write(req, opt.fid); +v9fs_uint64_write(req, opt.offset); +v9fs_uint32_write(req, opt.count); v9fs_req_send(req); -return req; + +if (!opt.requestOnly) { +v9fs_req_wait_for_reply(req, NULL); +if (opt.expectErr) { +v9fs_rlerror(req, &err); +g_assert_cmpint(err, ==, opt.expectErr); +} else { +v9fs_rreaddir(req, opt.rreaddir.count, opt.rreaddir.nentries, + opt.rreaddir.entries); +} +req = NULL; /* request was freed */ +} + +return (TReadDirRes) { .req = req }; } /* size[4] Rreaddir tag[2] count[4] data[count] */ diff --git a/tests/qtest/libqos/virtio-9p-client.h b/tests/qtest/libqos/virtio-9p-client.h index f7b1bfc79a..2bf649085f 100644 --- a/tests/qtest/libqos/virtio-9p-client.h +++ b/tests/qtest/libqos/virtio-9p-client.h @@ -182,6 +182,36 @@ typedef struct TGetAttrRes { P9Req *req; } TGetAttrRes; +/* options for 'Treaddir' 9p request */ +typedef struct TReadDirOpt { +/* 9P client being used (mandatory) */ +QVirtio9P *client; +/* user supplied tag number being returned with response (optional) */ +uint16_t tag; +/* file ID of directory whose entries shall be retrieved (required) */ +uint32_t fid; +/* offset in entries stream, i.e. for multiple requests (optional) */ +uint64_t offset; +/* maximum bytes to be returned by server (required) */ +uint32_t count; +/* data being received from 9p server as 'Rreaddir' response (optional) */ +struct { +uint32_t *count; +uint32_t *nentries; +struct V9fsDirent **entries; +} rreaddir; +/* only send Treaddir request but not wait for a reply? (optional) */ +bool requestOnly; +/* do we expect an Rlerror response, if yes which error code? (optional) */ +uint32_t expectErr; +} TReadDirOpt; + +/* result of 'Treaddir' 9p request */ +typedef struct TReadDirRes { +/* if requestOnly was set: request object for further processing */ +P9Req *req; +} TReadDirRes; + void v9fs_set_allocator(QGuestAllocator *t_alloc); void v9fs_memwrite(P9Req *req, const void *addr, size_t len); void v9fs_memskip(P9Req *req, size_t len); @@ -211,8 +241,7 @@ TWalkRes v9fs_twalk(TWalkOpt opt); void v9fs_rwalk(P9Req *req, uint16_t *nwqid, v9fs_qid **wqid); TGetAttrRes v9fs_tgetattr(TGetAttrOpt); void v9fs_rgetattr(P9Req *req, v9fs_attr *attr); -P9Req *v9fs_treaddir(QVirtio9P *v9p, uint32_t fid, uint64_t offset, - uint32_t count, uint16_t tag); +TReadDirRes v9fs_treaddir(TReadDirOpt); void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries, struct V9fsDirent **entries); void v9fs_free_dirents(struct V9fsDirent *e); diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index ae1220d0cb..e5c174c218 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -20,6 +20,7 @@ #define tversion(...) v9fs_tversion((TVersionOpt) __VA_ARGS__) #define tattach(...) v9fs_tattach((TAttachOpt) __VA_ARGS__) #define tgetattr(...) v9fs_tgetattr((TGetAttrOpt) __VA_ARGS__) +#define treaddir(...) v9fs_treaddir((TReadDirOpt) __VA_ARGS__) static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc) { @@ -119,7 +120,10 @@ static void fs_readdir(void *obj, void *data, QGuestAllocator *t_alloc) /* * submit count = msize - 11, because 11 is the header size of Rreaddir */ -req = v9fs_treaddir(v9p, 1, 0, P9_MAX_SIZE - 11, 0); +req = treaddir({ +.client = v9p, .fid = 1, .offset =
Re: [PATCH v6 09/14] target/arm: Don't shift attrs in get_phys_addr_lpae
On 24/10/22 07:18, Richard Henderson wrote: Leave the upper and lower attributes in the place they originate from in the descriptor. Shifting them around is confusing, since one cannot read the bit numbers out of the manual. Also, new attributes have been added which would alter the shifts. Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/ptw.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2] nbd/client: Use smarter assert
On 10/17/22 22:12, Eric Blake wrote: Assigning strlen() to a uint32_t and then asserting that it isn't too large doesn't catch the case of an input string 4G in length. Thankfully, the incoming strings can never be that large: if the export name or query is reflecting a string the client got from the server, we already guarantee that we dropped the NBD connection if the server sent more than 32M in a single reply to our NBD_OPT_* request; if the export name is coming from qemu, nbd_receive_negotiate() asserted that strlen(info->name) <= NBD_MAX_STRING_SIZE; and similarly, a query string via x->dirty_bitmap coming from the user was bounds-checked in either qemu-nbd or by the limitations of QMP. Still, it doesn't hurt to be more explicit in how we write our assertions to not have to analyze whether inadvertent wraparound is possible. Fixes: 93676c88 ("nbd: Don't send oversize strings", v4.2.0) Reported-by: Dr. David Alan Gilbert Signed-off-by: Eric Blake --- v2: update subject line and commit message to reflect file being touched; adjust a second nearby assertion with the same issue nbd/client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 30d5383cb1..90a6b7b38b 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -658,11 +658,11 @@ static int nbd_send_meta_query(QIOChannel *ioc, uint32_t opt, char *p; data_len = sizeof(export_len) + export_len + sizeof(queries); -assert(export_len <= NBD_MAX_STRING_SIZE); +assert(strlen(export) <= NBD_MAX_STRING_SIZE); if (query) { query_len = strlen(query); data_len += sizeof(query_len) + query_len; -assert(query_len <= NBD_MAX_STRING_SIZE); +assert(strlen(query) <= NBD_MAX_STRING_SIZE); } else { assert(opt == NBD_OPT_LIST_META_CONTEXT); } I'm a bit late, and this should work as is. Still, for me it's a bit strange: you point to the fact that we probably overflow uint32_t variable. But we keep this fact hidden in the code. So, everyone who read should guess "aha, this extra strlen here is because the previous one may overflow the variable)". Could we use strnlen() instead of strlen()? That would be also more effective. -- Best regards, Vladimir
Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading
(Cc'ing Markus for a QMP related question.) On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld wrote: > > Snapshot loading only expects to call deterministic handlers, not > non-deterministic ones. So introduce a way of registering handlers that > won't be called when reseting for snapshots. > > Signed-off-by: Jason A. Donenfeld > diff --git a/migration/savevm.c b/migration/savevm.c > index 48e85c052c..a0cdb714f7 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char > *vmstate, > goto err_drain; > } > > -qemu_system_reset(SHUTDOWN_CAUSE_NONE); > +qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD); > mis->from_src_file = f; > > if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) { > diff --git a/qapi/run-state.json b/qapi/run-state.json > index 9273ea6516..74ed0ac93c 100644 > --- a/qapi/run-state.json > +++ b/qapi/run-state.json > @@ -86,12 +86,14 @@ > # ignores --no-reboot. This is useful for sanitizing > # hypercalls on s390 that are used during kexec/kdump/boot > # > +# @snapshot-load: A snapshot is being loaded by the record & replay > subsystem. > +# > ## > { 'enum': 'ShutdownCause', ># Beware, shutdown_caused_by_guest() depends on enumeration order >'data': [ 'none', 'host-error', 'host-qmp-quit', 'host-qmp-system-reset', > 'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset', > -'guest-panic', 'subsystem-reset'] } > +'guest-panic', 'subsystem-reset', 'snapshot-load'] } Markus: do we need to mark new enum values with some kind of "since n.n" version info ? > ## > # @StatusInfo: > diff --git a/softmmu/runstate.c b/softmmu/runstate.c > index 1e68680b9d..03e1ee3a8a 100644 > --- a/softmmu/runstate.c > +++ b/softmmu/runstate.c > @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason) > cpu_synchronize_all_states(); > > if (mc && mc->reset) { > -mc->reset(current_machine); > +mc->reset(current_machine, reason); > } else { > -qemu_devices_reset(); > +qemu_devices_reset(reason); > } > if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) { > qapi_event_send_reset(shutdown_caused_by_guest(reason), reason); This change means that resets on snapshot-load, which previously did not result in sending a QMP event, will now start to do so with this new ShutdownCause type. I don't think we want that change in behaviour. (Also, as per the 'Beware' comment in run-state.json, we will claim this to be a 'caused by guest' reset, which doesn't seem right. If we suppress the sending the event that is moot, though.) Markus: if we add a new value to the ShutdownCause enumeration, how annoying is it if we decide we don't want it later? I guess we can just leave it in the enum unused... (In this case we're using it for purely internal purposes and it won't ever actually wind up in any QMP events.) thanks -- PMM
Re: [PATCH v13 17/17] net: stream: add QAPI events to report connection state
On Mon, 24 Oct 2022 13:00:09 +0200 Markus Armbruster wrote: > Markus Armbruster writes: > > > Cc: Stefano Brivio > > > > Laurent Vivier writes: > > > >> On 10/21/22 07:48, Markus Armbruster wrote: > >>> Laurent Vivier writes: > >>> > The netdev reports NETDEV_STREAM_CONNECTED event when the backend > is connected, and NETDEV_STREAM_DISCONNECTED when it is disconnected. > >>> > >>> Use cases? > >> > >> This is asked by Stefano Brivio to allow libvirt to detect if connection > >> to passt is lost and to restart passt. > > [...] > > >>> Could similar event signalling be useful for other kinds of netdev > >>> backends? > >> > >> I was wondering, but it becomes more complicated to be generic. > > > > Making something complicated and generic where a simpler special > > solution would do is the worst. > > > > Not quite as bad (but still plenty bad) is making a few special > > solutions first, then replace them all with a generic solution. > > > > I believe we should have a good, hard think on possible applications of > > a generic solution now. > > > > There is no need to hold back this series for that. > > > > If we conclude a generic solution is called for, we better replace this > > special solution before it becomes ABI. Either by replacing it before > > we release it, or by keeping it unstable until we replace it. > > Stefano, any thoughts on this? Actually, to me, it already looks as generic as it can be: stream back-ends are the only ones connecting and disconnecting. I quickly tried to think about possible, similar events for other back-ends: - user: handled by libslirp, there's no connection, and probably not much we can or want to export from libslirp itself - tap, bridge: the closest equivalent would be interfaces changing states, but that's something that's also externally observable with a netlink socket, in case one needs to know. And in any case, it's logically very different from a connection or disconnection. If we want events for that, they should have different names - vhost-user, vde: we could implement something similar if the need arises, but it should logically have a different name - l2tpv3: stateless, same as datagram-oriented socket. No states, no events to report, I guess. All in all, to me, NETDEV_STREAM_{,DIS}CONNECTED events here don't look very "special" or hackish. -- Stefano
[PULL 16/23] tests/9p: simplify callers of twrite()
Now as twrite() is using a declarative approach, simplify the code of callers of this function. Signed-off-by: Christian Schoenebeck Message-Id: <7f280ec6a1f9d8afed46567a796562c4dc28afa9.1664917004.git.qemu_...@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index a5b9284acb..5ad7bebec7 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -377,7 +377,6 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) char *wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) }; g_autofree char *buf = g_malloc0(write_count); uint32_t count; -P9Req *req; tattach({ .client = v9p }); twalk({ @@ -386,12 +385,10 @@ static void fs_write(void *obj, void *data, QGuestAllocator *t_alloc) tlopen({ .client = v9p, .fid = 1, .flags = O_WRONLY }); -req = twrite({ +count = twrite({ .client = v9p, .fid = 1, .offset = 0, .count = write_count, -.data = buf, .requestOnly = true -}).req; -v9fs_req_wait_for_reply(req, NULL); -v9fs_rwrite(req, &count); +.data = buf +}).count; g_assert_cmpint(count, ==, write_count); g_free(wnames[0]); -- 2.30.2
Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading
On 10/24/22, Peter Maydell wrote: > (Cc'ing Markus for a QMP related question.) > > On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld wrote: >> >> Snapshot loading only expects to call deterministic handlers, not >> non-deterministic ones. So introduce a way of registering handlers that >> won't be called when reseting for snapshots. >> >> Signed-off-by: Jason A. Donenfeld > >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 48e85c052c..a0cdb714f7 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char >> *vmstate, >> goto err_drain; >> } >> >> -qemu_system_reset(SHUTDOWN_CAUSE_NONE); >> +qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD); >> mis->from_src_file = f; >> >> if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) { >> diff --git a/qapi/run-state.json b/qapi/run-state.json >> index 9273ea6516..74ed0ac93c 100644 >> --- a/qapi/run-state.json >> +++ b/qapi/run-state.json >> @@ -86,12 +86,14 @@ >> # ignores --no-reboot. This is useful for sanitizing >> # hypercalls on s390 that are used during >> kexec/kdump/boot >> # >> +# @snapshot-load: A snapshot is being loaded by the record & replay >> subsystem. >> +# >> ## >> { 'enum': 'ShutdownCause', >># Beware, shutdown_caused_by_guest() depends on enumeration order >>'data': [ 'none', 'host-error', 'host-qmp-quit', >> 'host-qmp-system-reset', >> 'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset', >> -'guest-panic', 'subsystem-reset'] } >> +'guest-panic', 'subsystem-reset', 'snapshot-load'] } > > Markus: do we need to mark new enum values with some kind of "since n.n" > version info ? > >> ## >> # @StatusInfo: >> diff --git a/softmmu/runstate.c b/softmmu/runstate.c >> index 1e68680b9d..03e1ee3a8a 100644 >> --- a/softmmu/runstate.c >> +++ b/softmmu/runstate.c >> @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason) >> cpu_synchronize_all_states(); >> >> if (mc && mc->reset) { >> -mc->reset(current_machine); >> +mc->reset(current_machine, reason); >> } else { >> -qemu_devices_reset(); >> +qemu_devices_reset(reason); >> } >> if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) { >> qapi_event_send_reset(shutdown_caused_by_guest(reason), reason); > > This change means that resets on snapshot-load, which previously > did not result in sending a QMP event, will now start to do so > with this new ShutdownCause type. I don't think we want that > change in behaviour. Good point. I'll exclude that case and send a v+1. Jason > > (Also, as per the 'Beware' comment in run-state.json, we will > claim this to be a 'caused by guest' reset, which doesn't seem > right. If we suppress the sending the event that is moot, though.) > > Markus: if we add a new value to the ShutdownCause enumeration, > how annoying is it if we decide we don't want it later? I guess > we can just leave it in the enum unused... (In this case we're > using it for purely internal purposes and it won't ever actually > wind up in any QMP events.) > > thanks > -- PMM > -- Jason A. Donenfeld Deep Space Explorer fr: +33 6 51 90 82 66 us: +1 513 476 1200 www.jasondonenfeld.com www.zx2c4.com zx2c4.com/keys/AB9942E6D4A4CFC3412620A749FC7012A5DE03AE.asc
Re: [PATCH] ui: remove useless typecasts
Le 22/10/2022 à 16:12, Volker Rümelin a écrit : Commit 8f9abdf586 ("chardev: src buffer const for write functions") changed the type of the second parameter of qemu_chr_be_write() from uint8_t * to const uint8_t *. Remove the now useless type casts from qemu_chr_be_write() function calls in ui/console.c and ui/gtk.c. Cc: qemu-triv...@nongnu.org Signed-off-by: Volker Rümelin --- ui/console.c | 2 +- ui/gtk.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/console.c b/ui/console.c index 49da6a91df..65c117874c 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1297,7 +1297,7 @@ static void kbd_send_chars(QemuConsole *s) uint32_t size; buf = fifo8_pop_buf(&s->out_fifo, MIN(len, avail), &size); -qemu_chr_be_write(s->chr, (uint8_t *)buf, size); +qemu_chr_be_write(s->chr, buf, size); len = qemu_chr_be_can_write(s->chr); avail -= size; } diff --git a/ui/gtk.c b/ui/gtk.c index 92daaa6a6e..7ec21f7798 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1763,7 +1763,7 @@ static void gd_vc_send_chars(VirtualConsole *vc) uint32_t size; buf = fifo8_pop_buf(&vc->vte.out_fifo, MIN(len, avail), &size); -qemu_chr_be_write(vc->vte.chr, (uint8_t *)buf, size); +qemu_chr_be_write(vc->vte.chr, buf, size); len = qemu_chr_be_can_write(vc->vte.chr); avail -= size; } Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH v13 17/17] net: stream: add QAPI events to report connection state
Stefano Brivio writes: > On Mon, 24 Oct 2022 13:00:09 +0200 > Markus Armbruster wrote: > >> Markus Armbruster writes: >> >> > Cc: Stefano Brivio >> > >> > Laurent Vivier writes: >> > >> >> On 10/21/22 07:48, Markus Armbruster wrote: >> >>> Laurent Vivier writes: >> >>> >> The netdev reports NETDEV_STREAM_CONNECTED event when the backend >> is connected, and NETDEV_STREAM_DISCONNECTED when it is disconnected. >> >>> >> >>> Use cases? >> >> >> >> This is asked by Stefano Brivio to allow libvirt to detect if connection >> >> to passt is lost and to restart passt. >> >> [...] >> >> >>> Could similar event signalling be useful for other kinds of netdev >> >>> backends? >> >> >> >> I was wondering, but it becomes more complicated to be generic. >> > >> > Making something complicated and generic where a simpler special >> > solution would do is the worst. >> > >> > Not quite as bad (but still plenty bad) is making a few special >> > solutions first, then replace them all with a generic solution. >> > >> > I believe we should have a good, hard think on possible applications of >> > a generic solution now. >> > >> > There is no need to hold back this series for that. >> > >> > If we conclude a generic solution is called for, we better replace this >> > special solution before it becomes ABI. Either by replacing it before >> > we release it, or by keeping it unstable until we replace it. >> >> Stefano, any thoughts on this? > > Actually, to me, it already looks as generic as it can be: stream > back-ends are the only ones connecting and disconnecting. > > I quickly tried to think about possible, similar events for other > back-ends: > > - user: handled by libslirp, there's no connection, and probably not > much we can or want to export from libslirp itself > > - tap, bridge: the closest equivalent would be interfaces changing > states, but that's something that's also externally observable with a > netlink socket, in case one needs to know. And in any case, it's > logically very different from a connection or disconnection. If we > want events for that, they should have different names > > - vhost-user, vde: we could implement something similar if the need > arises, but it should logically have a different name > > - l2tpv3: stateless, same as datagram-oriented socket. No states, no > events to report, I guess. > > All in all, to me, NETDEV_STREAM_{,DIS}CONNECTED events here don't look > very "special" or hackish. Thanks!
Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading
Peter Maydell writes: > (Cc'ing Markus for a QMP related question.) > > On Fri, 14 Oct 2022 at 03:17, Jason A. Donenfeld wrote: >> >> Snapshot loading only expects to call deterministic handlers, not >> non-deterministic ones. So introduce a way of registering handlers that >> won't be called when reseting for snapshots. >> >> Signed-off-by: Jason A. Donenfeld > >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 48e85c052c..a0cdb714f7 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -3058,7 +3058,7 @@ bool load_snapshot(const char *name, const char >> *vmstate, >> goto err_drain; >> } >> >> -qemu_system_reset(SHUTDOWN_CAUSE_NONE); >> +qemu_system_reset(SHUTDOWN_CAUSE_SNAPSHOT_LOAD); >> mis->from_src_file = f; >> >> if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) { >> diff --git a/qapi/run-state.json b/qapi/run-state.json >> index 9273ea6516..74ed0ac93c 100644 >> --- a/qapi/run-state.json >> +++ b/qapi/run-state.json >> @@ -86,12 +86,14 @@ >> # ignores --no-reboot. This is useful for sanitizing >> # hypercalls on s390 that are used during kexec/kdump/boot >> # >> +# @snapshot-load: A snapshot is being loaded by the record & replay >> subsystem. >> +# >> ## >> { 'enum': 'ShutdownCause', >># Beware, shutdown_caused_by_guest() depends on enumeration order >>'data': [ 'none', 'host-error', 'host-qmp-quit', 'host-qmp-system-reset', >> 'host-signal', 'host-ui', 'guest-shutdown', 'guest-reset', >> -'guest-panic', 'subsystem-reset'] } >> +'guest-panic', 'subsystem-reset', 'snapshot-load'] } > > Markus: do we need to mark new enum values with some kind of "since n.n" > version info ? We do! Commonly like # @snapshot-load: A snapshot is being loaded by the record & replay # subsystem (since 7.2) >> ## >> # @StatusInfo: >> diff --git a/softmmu/runstate.c b/softmmu/runstate.c >> index 1e68680b9d..03e1ee3a8a 100644 >> --- a/softmmu/runstate.c >> +++ b/softmmu/runstate.c >> @@ -441,9 +441,9 @@ void qemu_system_reset(ShutdownCause reason) >> cpu_synchronize_all_states(); >> >> if (mc && mc->reset) { >> -mc->reset(current_machine); >> +mc->reset(current_machine, reason); >> } else { >> -qemu_devices_reset(); >> +qemu_devices_reset(reason); >> } >> if (reason && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) { >> qapi_event_send_reset(shutdown_caused_by_guest(reason), reason); > > This change means that resets on snapshot-load, which previously > did not result in sending a QMP event, will now start to do so > with this new ShutdownCause type. I don't think we want that > change in behaviour. > > (Also, as per the 'Beware' comment in run-state.json, we will > claim this to be a 'caused by guest' reset, which doesn't seem > right. If we suppress the sending the event that is moot, though.) > > Markus: if we add a new value to the ShutdownCause enumeration, > how annoying is it if we decide we don't want it later? I guess > we can just leave it in the enum unused... (In this case we're > using it for purely internal purposes and it won't ever actually > wind up in any QMP events.) Deleting enumeration values is a compatibility issue only if the value is usable in QMP input. "Purely internal" means it cannot occur in QMP output, and any attempt to use it in input fails. Aside: feels a bit fragile. Having an enumeration type where some values are like this is mildly ugly, because introspection still shows the value. If we care about fragile or mildly ugly, we can mark such values with a special feature flag, and have the QAPI generator keep them internal (input, output, introspection). Does this answer your question?
[PATCH v2] kset: fix memory leak when kset_register() returns error
Inject fault while loading module, kset_register() may fail. If it fails, the name allocated by kobject_set_name() which is called before kset_register() is leaked, because refcount of kobject is hold in kset_init(). As a kset may be embedded in a larger structure which needs be freed in release() function or error path in callers, we can not call kset_put() in kset_register(), or it will cause double free, so just call kfree_const() to free the name and set it to NULL. With this fix, the callers don't need to care about the name freeing and call an extra kset_put() if kset_register() fails. Suggested-by: Luben Tuikov Signed-off-by: Yang Yingliang --- v1 -> v2: Free name inside of kset_register() instead of calling kset_put() in drivers. --- lib/kobject.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/kobject.c b/lib/kobject.c index a0b2dbfcfa23..3409a89c81e5 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -834,6 +834,9 @@ EXPORT_SYMBOL_GPL(kobj_sysfs_ops); /** * kset_register() - Initialize and add a kset. * @k: kset. + * + * NOTE: On error, the kset.kobj.name allocated by() kobj_set_name() + * which is called before kset_register() in caller need be freed. */ int kset_register(struct kset *k) { @@ -844,8 +847,11 @@ int kset_register(struct kset *k) kset_init(k); err = kobject_add_internal(&k->kobj); - if (err) + if (err) { + kfree_const(k->kobj.name); + k->kobj.name = NULL; return err; + } kobject_uevent(&k->kobj, KOBJ_ADD); return 0; } -- 2.25.1
Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading
On Mon, 24 Oct 2022 at 13:28, Markus Armbruster wrote: > > Peter Maydell writes: > > Markus: if we add a new value to the ShutdownCause enumeration, > > how annoying is it if we decide we don't want it later? I guess > > we can just leave it in the enum unused... (In this case we're > > using it for purely internal purposes and it won't ever actually > > wind up in any QMP events.) > > Deleting enumeration values is a compatibility issue only if the value > is usable in QMP input. > > "Purely internal" means it cannot occur in QMP output, and any attempt > to use it in input fails. Aside: feels a bit fragile. In this case there are as far as I can see no QMP input commands which use the enum at all -- it's only used in events, which are always output, I think. thanks -- PMM
Re: [PATCH qemu.git] target/imx: reload cmp timer outside of the reload ptimer transaction
On Mon, 24 Oct 2022 at 13:37, Peter Maydell wrote: > > On Fri, 21 Oct 2022 at 20:18, ~axelheider wrote: > > > > From: Axel Heider > > > > Signed-off-by: Axel Heider > > --- > > See https://gitlab.com/qemu-project/qemu/-/issues/1263 > > When running the seL4 tests > > (https://docs.sel4.systems/projects/sel4test), on the sabrelight > > platform the timer test fails (and thus it's disabled by default). > > Investigation has shown that the arm/imx6 EPIT timer interrupt does not > > fire properly, instead of a second in can take up to a minute to finally > > see the interrupt. Oh yes, this sort of information should ideally be in the commit message proper -- the commit message is the place to explain what you're changing and why. You can make the gitlab issue auto-close by putting a line Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1263 into the commit message. thanks -- PMM
Re: [PATCH qemu.git] target/imx: reload cmp timer outside of the reload ptimer transaction
On Fri, 21 Oct 2022 at 20:18, ~axelheider wrote: > > From: Axel Heider > > Signed-off-by: Axel Heider > --- > See https://gitlab.com/qemu-project/qemu/-/issues/1263 > When running the seL4 tests > (https://docs.sel4.systems/projects/sel4test), on the sabrelight > platform the timer test fails (and thus it's disabled by default). > Investigation has shown that the arm/imx6 EPIT timer interrupt does not > fire properly, instead of a second in can take up to a minute to finally > see the interrupt. > > hw/timer/imx_epit.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c > index 2bf8c754b2..0b13c1eab0 100644 > --- a/hw/timer/imx_epit.c > +++ b/hw/timer/imx_epit.c > @@ -276,9 +276,12 @@ static void imx_epit_write(void *opaque, hwaddr offset, > uint64_t value, > ptimer_set_count(s->timer_reload, s->lr); > } > > +// commit s->timer_reload before imx_epit_reload_compare_timer > +// as timer_reload is read in imx_epit_reload_compare_timer QEMU coding style requires /* ... */ for comments, not // (with the /* and */ on lines of their own if it's a multiline comment. > +ptimer_transaction_commit(s->timer_reload); > + > imx_epit_reload_compare_timer(s); > ptimer_transaction_commit(s->timer_cmp); > -ptimer_transaction_commit(s->timer_reload); > break; Yes, I see what's happening here. It's OK to commit the timer_reload timer transaction first because that timer doesn't care about the timer_cmp timer. Do we also need to change the other places that call imx_epit_reload_compare_timer() in the handling of CR register writes ? (Those are a little more tricky.) thanks -- PMM
Re: [PATCH v3 1/8] reset: allow registering handlers that aren't called by snapshot loading
Peter Maydell writes: > On Mon, 24 Oct 2022 at 13:28, Markus Armbruster wrote: >> >> Peter Maydell writes: >> > Markus: if we add a new value to the ShutdownCause enumeration, >> > how annoying is it if we decide we don't want it later? I guess >> > we can just leave it in the enum unused... (In this case we're >> > using it for purely internal purposes and it won't ever actually >> > wind up in any QMP events.) >> >> Deleting enumeration values is a compatibility issue only if the value >> is usable in QMP input. >> >> "Purely internal" means it cannot occur in QMP output, and any attempt >> to use it in input fails. Aside: feels a bit fragile. > > In this case there are as far as I can see no QMP input commands > which use the enum at all -- it's only used in events, which are > always output, I think. They are. Ascertaining "not used in QMP input" is pretty easy, and "not used in CLI" isn't hard. My point is that uses could creep in later. This is what makes "purely internal" fragile.
Re: [PATCH 5/9] target/s390x: Use Int128 for return from TRE
On 21/10/22 09:30, Richard Henderson wrote: Signed-off-by: Richard Henderson --- target/s390x/helper.h | 2 +- target/s390x/tcg/mem_helper.c | 7 +++ target/s390x/tcg/translate.c | 7 +-- 3 files changed, 9 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 3/9] target/s390x: Use Int128 for return from CLST
On 21/10/22 09:30, Richard Henderson wrote: Signed-off-by: Richard Henderson --- target/s390x/helper.h | 2 +- target/s390x/tcg/mem_helper.c | 11 --- target/s390x/tcg/translate.c | 8 ++-- 3 files changed, 11 insertions(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 7/9] tests/tcg/s390x: Add long-double.c
On 21/10/22 09:30, Richard Henderson wrote: Signed-off-by: Richard Henderson --- tests/tcg/s390x/long-double.c | 24 tests/tcg/s390x/Makefile.target | 1 + 2 files changed, 25 insertions(+) create mode 100644 tests/tcg/s390x/long-double.c Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 16/26] MAINTAINERS: add features_to_c.sh to gdbstub files
On 20/10/22 13:51, Alex Bennée wrote: Signed-off-by: Alex Bennée --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Philippe Mathieu-Daudé
[PATCH 26/29] target/openrisc: Always exit after mtspr npc
We have called cpu_restore_state asserting will_exit. Do not go back on that promise. This affects icount. Signed-off-by: Richard Henderson --- target/openrisc/sys_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index 09b3c97d7c..a3508e421d 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -51,8 +51,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb) if (env->pc != rb) { env->pc = rb; env->dflag = 0; -cpu_loop_exit(cs); } +cpu_loop_exit(cs); break; case TO_SPR(0, 17): /* SR */ -- 2.34.1
[PATCH 19/29] target/sh4: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/sh4/cpu.c | 16 target/sh4/translate.c | 10 -- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c index 56c50530da..453268392b 100644 --- a/target/sh4/cpu.c +++ b/target/sh4/cpu.c @@ -50,6 +50,21 @@ static void superh_cpu_synchronize_from_tb(CPUState *cs, cpu->env.flags = tb->flags; } +static void superh_restore_state_to_opc(CPUState *cs, +const TranslationBlock *tb, +const uint64_t *data) +{ +SuperHCPU *cpu = SUPERH_CPU(cs); + +cpu->env.pc = data[0]; +cpu->env.flags = data[1]; +/* + * Theoretically delayed_pc should also be restored. In practice the + * branch instruction is re-executed after exception, so the delayed + * branch target will be recomputed. + */ +} + #ifndef CONFIG_USER_ONLY static bool superh_io_recompile_replay_branch(CPUState *cs, const TranslationBlock *tb) @@ -243,6 +258,7 @@ static const struct SysemuCPUOps sh4_sysemu_ops = { static const struct TCGCPUOps superh_tcg_ops = { .initialize = sh4_translate_init, .synchronize_from_tb = superh_cpu_synchronize_from_tb, +.restore_state_to_opc = superh_restore_state_to_opc, #ifndef CONFIG_USER_ONLY .tlb_fill = superh_cpu_tlb_fill, diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 26231b2a5a..7db3468b01 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -2381,13 +2381,3 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns, translator_loop(cs, tb, max_insns, pc, host_pc, &sh4_tr_ops, &ctx.base); } - -void restore_state_to_opc(CPUSH4State *env, TranslationBlock *tb, - target_ulong *data) -{ -env->pc = data[0]; -env->flags = data[1]; -/* Theoretically delayed_pc should also be restored. In practice the - branch instruction is re-executed after exception, so the delayed - branch target will be recomputed. */ -} -- 2.34.1
[PATCH 07/29] target/hppa: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/hppa/cpu.c | 19 +++ target/hppa/translate.c | 13 - 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c index e677ca09d4..55c190280e 100644 --- a/target/hppa/cpu.c +++ b/target/hppa/cpu.c @@ -68,6 +68,24 @@ static void hppa_cpu_synchronize_from_tb(CPUState *cs, cpu->env.psw_n = (tb->flags & PSW_N) != 0; } +static void hppa_restore_state_to_opc(CPUState *cs, + const TranslationBlock *tb, + const uint64_t *data) +{ +HPPACPU *cpu = HPPA_CPU(cs); + +cpu->env.iaoq_f = data[0]; +if (data[1] != (target_ureg)-1) { +cpu->env.iaoq_b = data[1]; +} +/* + * Since we were executing the instruction at IAOQ_F, and took some + * sort of action that provoked the cpu_restore_state, we can infer + * that the instruction was not nullified. + */ +cpu->env.psw_n = 0; +} + static bool hppa_cpu_has_work(CPUState *cs) { return cs->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI); @@ -153,6 +171,7 @@ static const struct SysemuCPUOps hppa_sysemu_ops = { static const struct TCGCPUOps hppa_tcg_ops = { .initialize = hppa_translate_init, .synchronize_from_tb = hppa_cpu_synchronize_from_tb, +.restore_state_to_opc = hppa_restore_state_to_opc, #ifndef CONFIG_USER_ONLY .tlb_fill = hppa_cpu_tlb_fill, diff --git a/target/hppa/translate.c b/target/hppa/translate.c index 8b861957e0..1af77473da 100644 --- a/target/hppa/translate.c +++ b/target/hppa/translate.c @@ -4346,16 +4346,3 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns, DisasContext ctx; translator_loop(cs, tb, max_insns, pc, host_pc, &hppa_tr_ops, &ctx.base); } - -void restore_state_to_opc(CPUHPPAState *env, TranslationBlock *tb, - target_ulong *data) -{ -env->iaoq_f = data[0]; -if (data[1] != (target_ureg)-1) { -env->iaoq_b = data[1]; -} -/* Since we were executing the instruction at IAOQ_F, and took some - sort of action that provoked the cpu_restore_state, we can infer - that the instruction was not nullified. */ -env->psw_n = 0; -} -- 2.34.1
[PATCH 00/29] tcg: Fix x86 TARGET_TB_PCREL (#1269)
As per #1269, this affects NetBSD installer boot. The problem is that one of the x86 acpi callbacks modifies env->eip during an mmio store, which means that the tracking that translate.c does is thrown out of whack. Introduce a method to extract unwind data without the writeback to env. This isn't a perfect abstraction, but I couldn't think of anything better. There's a couple of lines of code duplication, but probably less than any abstration that we might put on top Move restore_state_to_opc to a tcg_ops hook. Remove the last use of cpu_restore_state with will_exit=false from openrisc, which was on shaky ground already with similar modifications to translate.c variables. Remove the will_exit/reset_icount parameters, which are now always true. r~ Richard Henderson (29): accel/tcg: Add restore_state_to_opc to TCGCPUOps target/alpha: Convert to tcg_ops restore_state_to_opc target/arm: Convert to tcg_ops restore_state_to_opc target/avr: Convert to tcg_ops restore_state_to_opc target/cris: Convert to tcg_ops restore_state_to_opc target/hexagon: Convert to tcg_ops restore_state_to_opc target/hppa: Convert to tcg_ops restore_state_to_opc target/i386: Convert to tcg_ops restore_state_to_opc target/loongarch: Convert to tcg_ops restore_state_to_opc target/m68k: Convert to tcg_ops restore_state_to_opc target/microblaze: Convert to tcg_ops restore_state_to_opc target/mips: Convert to tcg_ops restore_state_to_opc target/nios2: Convert to tcg_ops restore_state_to_opc target/openrisc: Convert to tcg_ops restore_state_to_opc target/ppc: Convert to tcg_ops restore_state_to_opc target/riscv: Convert to tcg_ops restore_state_to_opc target/rx: Convert to tcg_ops restore_state_to_opc target/s390x: Convert to tcg_ops restore_state_to_opc target/sh4: Convert to tcg_ops restore_state_to_opc target/sparc: Convert to tcg_ops restore_state_to_opc target/tricore: Convert to tcg_ops restore_state_to_opc target/xtensa: Convert to tcg_ops restore_state_to_opc accel/tcg: Remove restore_state_to_opc function accel/tcg: Introduce cpu_unwind_state_data target/i386: Use cpu_unwind_state_data for tpr access target/openrisc: Always exit after mtspr npc target/openrisc: Use cpu_unwind_state_data for mfspr accel/tcg: Remove will_exit argument from cpu_restore_state accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb include/exec/exec-all.h | 23 +--- include/hw/core/tcg-cpu-ops.h | 11 target/mips/tcg/tcg-internal.h | 3 + target/s390x/s390x-internal.h | 4 +- target/sparc/cpu.h | 3 + accel/tcg/cpu-exec-common.c | 2 +- accel/tcg/translate-all.c | 87 ++--- target/alpha/cpu.c | 9 +++ target/alpha/helper.c | 2 +- target/alpha/mem_helper.c | 2 +- target/alpha/translate.c| 6 -- target/arm/cpu.c| 26 + target/arm/op_helper.c | 2 +- target/arm/tlb_helper.c | 8 +-- target/arm/translate.c | 22 target/avr/cpu.c| 11 target/avr/translate.c | 6 -- target/cris/cpu.c | 11 target/cris/helper.c| 2 +- target/cris/translate.c | 6 -- target/hexagon/cpu.c| 9 ++- target/hppa/cpu.c | 19 +++ target/hppa/translate.c | 13 - target/i386/helper.c| 21 ++- target/i386/tcg/sysemu/svm_helper.c | 2 +- target/i386/tcg/tcg-cpu.c | 19 +++ target/i386/tcg/translate.c | 15 - target/loongarch/cpu.c | 11 target/loongarch/translate.c| 6 -- target/m68k/cpu.c | 14 + target/m68k/op_helper.c | 4 +- target/m68k/translate.c | 10 target/microblaze/cpu.c | 11 target/microblaze/helper.c | 2 +- target/microblaze/translate.c | 7 --- target/mips/cpu.c | 1 + target/mips/tcg/translate.c | 8 ++- target/nios2/cpu.c | 11 target/nios2/op_helper.c| 2 +- target/nios2/translate.c| 6 -- target/openrisc/cpu.c | 13 + target/openrisc/sys_helper.c| 17 -- target/openrisc/translate.c | 10 target/ppc/cpu_init.c | 10 target/ppc/excp_helper.c| 2 +- target/ppc/translate.c | 6 -- target/riscv/cpu.c | 9 ++- target/rx/cpu.c | 10 target/rx/translate.c | 6 -- target/s390x/cpu.c | 1 + target/s390x/tcg/excp_helper.c | 2 +- target/s390x/tcg/translate.c| 7 ++- target/sh4/cpu.c| 16 ++ target/sh4/translate.c | 10 target/sparc/cpu.c | 1 + target/s
[PATCH 12/29] target/mips: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/mips/tcg/tcg-internal.h | 3 +++ target/mips/cpu.c | 1 + target/mips/tcg/translate.c| 8 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h index 1d27fa2ff9..aef032c48d 100644 --- a/target/mips/tcg/tcg-internal.h +++ b/target/mips/tcg/tcg-internal.h @@ -21,6 +21,9 @@ void mips_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb); G_NORETURN void mips_cpu_do_unaligned_access(CPUState *cpu, vaddr addr, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); +void mips_restore_state_to_opc(CPUState *cs, + const TranslationBlock *tb, + const uint64_t *data); const char *mips_exception_name(int32_t exception); diff --git a/target/mips/cpu.c b/target/mips/cpu.c index da58eb8892..e997c1b9cb 100644 --- a/target/mips/cpu.c +++ b/target/mips/cpu.c @@ -538,6 +538,7 @@ static const struct SysemuCPUOps mips_sysemu_ops = { static const struct TCGCPUOps mips_tcg_ops = { .initialize = mips_tcg_init, .synchronize_from_tb = mips_cpu_synchronize_from_tb, +.restore_state_to_opc = mips_restore_state_to_opc, #if !defined(CONFIG_USER_ONLY) .tlb_fill = mips_cpu_tlb_fill, diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c index c3f92ea652..2f2d707a12 100644 --- a/target/mips/tcg/translate.c +++ b/target/mips/tcg/translate.c @@ -16229,9 +16229,13 @@ void mips_tcg_init(void) } } -void restore_state_to_opc(CPUMIPSState *env, TranslationBlock *tb, - target_ulong *data) +void mips_restore_state_to_opc(CPUState *cs, + const TranslationBlock *tb, + const uint64_t *data) { +MIPSCPU *cpu = MIPS_CPU(cs); +CPUMIPSState *env = &cpu->env; + env->active_tc.PC = data[0]; env->hflags &= ~MIPS_HFLAG_BMASK; env->hflags |= data[1]; -- 2.34.1
[PATCH 16/29] target/riscv: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/riscv/cpu.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index e6d9c706bb..d14e95c9dc 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -503,10 +503,14 @@ static bool riscv_cpu_has_work(CPUState *cs) #endif } -void restore_state_to_opc(CPURISCVState *env, TranslationBlock *tb, - target_ulong *data) +static void riscv_restore_state_to_opc(CPUState *cs, + const TranslationBlock *tb, + const uint64_t *data) { +RISCVCPU *cpu = RISCV_CPU(cs); +CPURISCVState *env = &cpu->env; RISCVMXL xl = FIELD_EX32(tb->flags, TB_FLAGS, XL); + if (xl == MXL_RV32) { env->pc = (int32_t)data[0]; } else { @@ -1138,6 +1142,7 @@ static const struct SysemuCPUOps riscv_sysemu_ops = { static const struct TCGCPUOps riscv_tcg_ops = { .initialize = riscv_translate_init, .synchronize_from_tb = riscv_cpu_synchronize_from_tb, +.restore_state_to_opc = riscv_restore_state_to_opc, #ifndef CONFIG_USER_ONLY .tlb_fill = riscv_cpu_tlb_fill, -- 2.34.1
[PATCH 17/29] target/rx: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/rx/cpu.c | 10 ++ target/rx/translate.c | 6 -- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/target/rx/cpu.c b/target/rx/cpu.c index 2f28099723..9003c6e9fe 100644 --- a/target/rx/cpu.c +++ b/target/rx/cpu.c @@ -47,6 +47,15 @@ static void rx_cpu_synchronize_from_tb(CPUState *cs, cpu->env.pc = tb_pc(tb); } +static void rx_restore_state_to_opc(CPUState *cs, +const TranslationBlock *tb, +const uint64_t *data) +{ +RXCPU *cpu = RX_CPU(cs); + +cpu->env.pc = data[0]; +} + static bool rx_cpu_has_work(CPUState *cs) { return cs->interrupt_request & @@ -192,6 +201,7 @@ static const struct SysemuCPUOps rx_sysemu_ops = { static const struct TCGCPUOps rx_tcg_ops = { .initialize = rx_translate_init, .synchronize_from_tb = rx_cpu_synchronize_from_tb, +.restore_state_to_opc = rx_restore_state_to_opc, .tlb_fill = rx_cpu_tlb_fill, #ifndef CONFIG_USER_ONLY diff --git a/target/rx/translate.c b/target/rx/translate.c index ea5653bc95..87a3f54adb 100644 --- a/target/rx/translate.c +++ b/target/rx/translate.c @@ -2371,12 +2371,6 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns, translator_loop(cs, tb, max_insns, pc, host_pc, &rx_tr_ops, &dc.base); } -void restore_state_to_opc(CPURXState *env, TranslationBlock *tb, - target_ulong *data) -{ -env->pc = data[0]; -} - #define ALLOC_REGISTER(sym, name) \ cpu_##sym = tcg_global_mem_new_i32(cpu_env, \ offsetof(CPURXState, sym), name) -- 2.34.1
Re: [PATCH 2/9] target/s390x: Use a single return for helper_divs64/u64
On 21/10/22 09:29, Richard Henderson wrote: Pack the quotient and remainder into a single Int128. Use the divu128 primitive to remove the cpu_abort on 32-bit hosts. This is the first use of Int128 as a return value. Signed-off-by: Richard Henderson --- target/s390x/helper.h | 4 ++-- target/s390x/tcg/int_helper.c | 38 +-- target/s390x/tcg/translate.c | 14 + 3 files changed, 21 insertions(+), 35 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[PATCH 24/29] accel/tcg: Introduce cpu_unwind_state_data
Add a way to examine the unwind data without actually restoring the data back into env. Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 13 accel/tcg/translate-all.c | 68 ++- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 300832bd0b..d49cf113dd 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -39,6 +39,19 @@ typedef ram_addr_t tb_page_addr_t; #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT #endif +/** + * cpu_unwind_state_data: + * @cpu: the vCPU state is to be restore to + * @host_pc: the host PC the fault occurred at + * @data: output data + * + * Attempt to load the the unwind state for a host pc occurring in + * translated code. If the searched_pc is not in translated code, + * the function returns false; otherwise @data is loaded. + * This is the same unwind info as given to restore_state_to_opc. + */ +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); + /** * cpu_restore_state: * @cpu: the vCPU state is to be restore to diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index e4386b3198..c772e3769c 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -320,29 +320,20 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) return p - block; } -/* The cpu state corresponding to 'searched_pc' is restored. - * When reset_icount is true, current TB will be interrupted and - * icount should be recalculated. - */ -static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t searched_pc, bool reset_icount) +static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, + uint64_t *data) { -uint64_t data[TARGET_INSN_START_WORDS]; -uintptr_t host_pc = (uintptr_t)tb->tc.ptr; +uintptr_t iter_pc = (uintptr_t)tb->tc.ptr; const uint8_t *p = tb->tc.ptr + tb->tc.size; int i, j, num_insns = tb->icount; -#ifdef CONFIG_PROFILER -TCGProfile *prof = &tcg_ctx->prof; -int64_t ti = profile_getclock(); -#endif -searched_pc -= GETPC_ADJ; +host_pc -= GETPC_ADJ; -if (searched_pc < host_pc) { +if (host_pc < iter_pc) { return -1; } -memset(data, 0, sizeof(data)); +memset(data, 0, sizeof(uint64_t) * TARGET_INSN_START_WORDS); if (!TARGET_TB_PCREL) { data[0] = tb_pc(tb); } @@ -353,19 +344,40 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, for (j = 0; j < TARGET_INSN_START_WORDS; ++j) { data[j] += decode_sleb128(&p); } -host_pc += decode_sleb128(&p); -if (host_pc > searched_pc) { -goto found; +iter_pc += decode_sleb128(&p); +if (iter_pc > host_pc) { +return num_insns - i; } } return -1; +} + +/* + * The cpu state corresponding to 'host_pc' is restored. + * When reset_icount is true, current TB will be interrupted and + * icount should be recalculated. + */ +static void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, + uintptr_t host_pc, bool reset_icount) +{ +uint64_t data[TARGET_INSN_START_WORDS]; +#ifdef CONFIG_PROFILER +TCGProfile *prof = &tcg_ctx->prof; +int64_t ti = profile_getclock(); +#endif +int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); + +if (insns_left < 0) { +return; +} - found: if (reset_icount && (tb_cflags(tb) & CF_USE_ICOUNT)) { assert(icount_enabled()); -/* Reset the cycle counter to the start of the block - and shift if to the number of actually executed instructions */ -cpu_neg(cpu)->icount_decr.u16.low += num_insns - i; +/* + * Reset the cycle counter to the start of the block and + * shift if to the number of actually executed instructions. + */ +cpu_neg(cpu)->icount_decr.u16.low += insns_left; } cpu->cc->tcg_ops->restore_state_to_opc(cpu, tb, data); @@ -375,7 +387,6 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, prof->restore_time + profile_getclock() - ti); qatomic_set(&prof->restore_count, prof->restore_count + 1); #endif -return 0; } bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) @@ -408,6 +419,17 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) return false; } +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data) +{ +if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { +TranslationBlock *tb = tcg_tb_lookup(host_pc); +if (tb) { +return cpu_unwind_data_from_tb(tb, host_pc, data) >= 0; +} +} +return false; +} + void page_init(void) { page
[PATCH 21/29] target/tricore: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/tricore/cpu.c | 11 +++ target/tricore/translate.c | 6 -- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c index ab7a1e3a6d..2c54a2825f 100644 --- a/target/tricore/cpu.c +++ b/target/tricore/cpu.c @@ -58,6 +58,16 @@ static void tricore_cpu_synchronize_from_tb(CPUState *cs, env->PC = tb_pc(tb); } +static void tricore_restore_state_to_opc(CPUState *cs, + const TranslationBlock *tb, + const uint64_t *data) +{ +TriCoreCPU *cpu = TRICORE_CPU(cs); +CPUTriCoreState *env = &cpu->env; + +env->PC = data[0]; +} + static void tricore_cpu_reset(DeviceState *dev) { CPUState *s = CPU(dev); @@ -161,6 +171,7 @@ static const struct SysemuCPUOps tricore_sysemu_ops = { static const struct TCGCPUOps tricore_tcg_ops = { .initialize = tricore_tcg_init, .synchronize_from_tb = tricore_cpu_synchronize_from_tb, +.restore_state_to_opc = tricore_restore_state_to_opc, .tlb_fill = tricore_cpu_tlb_fill, }; diff --git a/target/tricore/translate.c b/target/tricore/translate.c index a0558ead71..c5b7bfbf20 100644 --- a/target/tricore/translate.c +++ b/target/tricore/translate.c @@ -8886,12 +8886,6 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns, &tricore_tr_ops, &ctx.base); } -void -restore_state_to_opc(CPUTriCoreState *env, TranslationBlock *tb, - target_ulong *data) -{ -env->PC = data[0]; -} /* * * Initialization -- 2.34.1
[PATCH 23/29] accel/tcg: Remove restore_state_to_opc function
All targets have been updated. Use the tcg_ops target hook exclusively, which allows the compat code to be removed. Signed-off-by: Richard Henderson --- include/exec/exec-all.h | 3 --- accel/tcg/translate-all.c | 16 ++-- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index a772e8cbdc..300832bd0b 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -39,9 +39,6 @@ typedef ram_addr_t tb_page_addr_t; #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT #endif -void restore_state_to_opc(CPUArchState *env, TranslationBlock *tb, - target_ulong *data) __attribute__((weak)); - /** * cpu_restore_state: * @cpu: the vCPU state is to be restore to diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 19cd23e9a0..e4386b3198 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -327,7 +327,7 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, uintptr_t searched_pc, bool reset_icount) { -target_ulong data[TARGET_INSN_START_WORDS]; +uint64_t data[TARGET_INSN_START_WORDS]; uintptr_t host_pc = (uintptr_t)tb->tc.ptr; const uint8_t *p = tb->tc.ptr + tb->tc.size; int i, j, num_insns = tb->icount; @@ -368,19 +368,7 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, cpu_neg(cpu)->icount_decr.u16.low += num_insns - i; } -{ -const struct TCGCPUOps *ops = cpu->cc->tcg_ops; -__typeof(ops->restore_state_to_opc) restore = ops->restore_state_to_opc; -if (restore) { -uint64_t d64[TARGET_INSN_START_WORDS]; -for (i = 0; i < TARGET_INSN_START_WORDS; ++i) { -d64[i] = data[i]; -} -restore(cpu, tb, d64); -} else { -restore_state_to_opc(cpu->env_ptr, tb, data); -} -} +cpu->cc->tcg_ops->restore_state_to_opc(cpu, tb, data); #ifdef CONFIG_PROFILER qatomic_set(&prof->restore_time, -- 2.34.1
[Bug 1994002] Re: [SRU] migration was active, but no RAM info was set
If you need something from upstream QEMU, please use the new bug tracker here: https://gitlab.com/qemu-project/qemu/-/issues ** No longer affects: qemu -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1994002 Title: [SRU] migration was active, but no RAM info was set Status in Ubuntu Cloud Archive: New Status in Ubuntu Cloud Archive ussuri series: New Status in qemu package in Ubuntu: New Status in qemu source package in Focal: New Status in qemu source package in Jammy: New Status in qemu source package in Kinetic: New Bug description: While live-migrating many instances concurrently, libvirt sometimes return internal error: migration was active, but no RAM info was set: ~~~ 2022-03-30 06:08:37.197 7 WARNING nova.virt.libvirt.driver [req-5c3296cf-88ee-4af6-ae6a-ddba99935e23 - - - - -] [instance: af339c99-1182-4489-b15c-21e52f50f724] Error monitoring migration: internal error: migration was active, but no RAM info was set: libvirt.libvirtError: internal error: migration was active, but no RAM info was set ~~~ From upstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=2074205 [Impact] * Effects of this bug are mostly observed in large scale clusters with a lot of live migration activity. * Has second order effects for consumers of migration monitor such as libvirt and openstack. [Test Case] Steps to Reproduce: 1. live evacuate a compute 2. live migration of one or more instances fails with the above error N.B Due to the nature of this bug it is difficult consistently reproduce. [Where problems could occur] * In the event of a regression the migration monitor may report an inconsistent state. To manage notifications about this bug go to: https://bugs.launchpad.net/cloud-archive/+bug/1994002/+subscriptions
[PATCH 29/29] accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb
The value passed is always true. Signed-off-by: Richard Henderson --- accel/tcg/translate-all.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index dd439b5e3c..9e7dd41795 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -353,12 +353,11 @@ static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, } /* - * The cpu state corresponding to 'host_pc' is restored. - * When reset_icount is true, current TB will be interrupted and - * icount should be recalculated. + * The cpu state corresponding to 'host_pc' is restored in + * preparation for exiting the TB. */ static void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t host_pc, bool reset_icount) + uintptr_t host_pc) { uint64_t data[TARGET_INSN_START_WORDS]; #ifdef CONFIG_PROFILER @@ -371,7 +370,7 @@ static void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, return; } -if (reset_icount && (tb_cflags(tb) & CF_USE_ICOUNT)) { +if (tb_cflags(tb) & CF_USE_ICOUNT) { assert(icount_enabled()); /* * Reset the cycle counter to the start of the block and @@ -404,7 +403,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc) if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { TranslationBlock *tb = tcg_tb_lookup(host_pc); if (tb) { -cpu_restore_state_from_tb(cpu, tb, host_pc, true); +cpu_restore_state_from_tb(cpu, tb, host_pc); return true; } } @@ -1715,7 +1714,7 @@ tb_invalidate_phys_page_range__locked(struct page_collection *pages, * restore the CPU state. */ current_tb_modified = true; -cpu_restore_state_from_tb(cpu, current_tb, retaddr, true); +cpu_restore_state_from_tb(cpu, current_tb, retaddr); cpu_get_tb_cpu_state(env, ¤t_pc, ¤t_cs_base, ¤t_flags); } @@ -1874,7 +1873,7 @@ static bool tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc) restore the CPU state */ current_tb_modified = 1; -cpu_restore_state_from_tb(cpu, current_tb, pc, true); +cpu_restore_state_from_tb(cpu, current_tb, pc); cpu_get_tb_cpu_state(env, ¤t_pc, ¤t_cs_base, ¤t_flags); } @@ -1904,7 +1903,7 @@ void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr) tb = tcg_tb_lookup(retaddr); if (tb) { /* We can use retranslation to find the PC. */ -cpu_restore_state_from_tb(cpu, tb, retaddr, true); +cpu_restore_state_from_tb(cpu, tb, retaddr); tb_phys_invalidate(tb, -1); } else { /* The exception probably happened in a helper. The CPU state should @@ -1940,7 +1939,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) cpu_abort(cpu, "cpu_io_recompile: could not find TB for pc=%p", (void *)retaddr); } -cpu_restore_state_from_tb(cpu, tb, retaddr, true); +cpu_restore_state_from_tb(cpu, tb, retaddr); /* * Some guests must re-execute the branch when re-executing a delay -- 2.34.1
[PATCH] tests/tcg/nios2: Tweak 10m50-ghrd.ld
More closely follow the default linker script for nios2. This magically fixes a problem resolving .got relocs from the toolchain's libgcc.a. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1258 Signed-off-by: Richard Henderson --- tests/tcg/nios2/10m50-ghrd.ld | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/tcg/nios2/10m50-ghrd.ld b/tests/tcg/nios2/10m50-ghrd.ld index 7db0d59ad7..71cdda450c 100644 --- a/tests/tcg/nios2/10m50-ghrd.ld +++ b/tests/tcg/nios2/10m50-ghrd.ld @@ -44,11 +44,15 @@ SECTIONS .data : ALIGN(4) { *(.shdata) *(.data .data.* .gnu.linkonce.d.*) -. = ALIGN(4); -_gp = ABSOLUTE(. + 0x8000); -*(.got.plt) *(.got) -*(.lit8) -*(.lit4) +} >ram :RAM + +HIDDEN (_gp = ALIGN(16) + 0x7ff0); +PROVIDE_HIDDEN (gp = _gp); +.got : ALIGN(4) { +*(.got.plt) *(.igot.plt) *(.got) *(.igot) +} >ram :RAM + +.sdata : ALIGN(4) { *(.sdata .sdata.* .gnu.linkonce.s.*) } >ram :RAM -- 2.34.1
[PATCH 02/29] target/alpha: Convert to tcg_ops restore_state_to_opc
Signed-off-by: Richard Henderson --- target/alpha/cpu.c | 9 + target/alpha/translate.c | 6 -- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c index 979a629d59..270ae787b1 100644 --- a/target/alpha/cpu.c +++ b/target/alpha/cpu.c @@ -40,6 +40,14 @@ static vaddr alpha_cpu_get_pc(CPUState *cs) return cpu->env.pc; } +static void alpha_restore_state_to_opc(CPUState *cs, + const TranslationBlock *tb, + const uint64_t *data) +{ +AlphaCPU *cpu = ALPHA_CPU(cs); + +cpu->env.pc = data[0]; +} static bool alpha_cpu_has_work(CPUState *cs) { @@ -226,6 +234,7 @@ static const struct SysemuCPUOps alpha_sysemu_ops = { static const struct TCGCPUOps alpha_tcg_ops = { .initialize = alpha_translate_init, +.restore_state_to_opc = alpha_restore_state_to_opc, #ifdef CONFIG_USER_ONLY .record_sigsegv = alpha_cpu_record_sigsegv, diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 6766350f56..f9bcdeb717 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -3049,9 +3049,3 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns, DisasContext dc; translator_loop(cpu, tb, max_insns, pc, host_pc, &alpha_tr_ops, &dc.base); } - -void restore_state_to_opc(CPUAlphaState *env, TranslationBlock *tb, - target_ulong *data) -{ -env->pc = data[0]; -} -- 2.34.1
[PATCH 25/29] target/i386: Use cpu_unwind_state_data for tpr access
Avoid cpu_restore_state, and modifying env->eip out from underneath the translator with TARGET_TB_PCREL. There is some slight duplication from x86_restore_state_to_opc, but it's just a few lines. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1269 Signed-off-by: Richard Henderson --- target/i386/helper.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/target/i386/helper.c b/target/i386/helper.c index b62a1e48e2..2cd1756f1a 100644 --- a/target/i386/helper.c +++ b/target/i386/helper.c @@ -509,6 +509,23 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank, } } +static target_ulong get_memio_eip(CPUX86State *env) +{ +uint64_t data[TARGET_INSN_START_WORDS]; +CPUState *cs = env_cpu(env); + +if (!cpu_unwind_state_data(cs, cs->mem_io_pc, data)) { +return env->eip; +} + +/* Per x86_restore_state_to_opc. */ +if (TARGET_TB_PCREL) { +return (env->eip & TARGET_PAGE_MASK) | data[0]; +} else { +return data[0] - env->segs[R_CS].base; +} +} + void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) { X86CPU *cpu = env_archcpu(env); @@ -519,9 +536,9 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) cpu_interrupt(cs, CPU_INTERRUPT_TPR); } else if (tcg_enabled()) { -cpu_restore_state(cs, cs->mem_io_pc, false); +target_ulong eip = get_memio_eip(env); -apic_handle_tpr_access_report(cpu->apic_state, env->eip, access); +apic_handle_tpr_access_report(cpu->apic_state, eip, access); } } #endif /* !CONFIG_USER_ONLY */ -- 2.34.1