Re: [PATCH v2] block/snapshot: Fix compiler warning with -Wshadow=local
On 10/24/23 13:51, Thomas Huth wrote: On 24/10/2023 12.37, Markus Armbruster wrote: Markus Armbruster writes: Thomas Huth writes: No need to declare a new variable in the the inner code block here, we can re-use the "ret" variable that has been declared at the beginning of the function. With this change, the code can now be successfully compiled with -Wshadow=local again. Fixes: a32e781838 ("Mark bdrv_snapshot_fallback() and callers GRAPH_RDLOCK") Signed-off-by: Thomas Huth --- v2: Assign "ret" only in one spot block/snapshot.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/block/snapshot.c b/block/snapshot.c index 6e16eb803a..55974273ae 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -629,7 +629,6 @@ int bdrv_all_goto_snapshot(const char *name, while (iterbdrvs) { BlockDriverState *bs = iterbdrvs->data; AioContext *ctx = bdrv_get_aio_context(bs); - int ret = 0; bool all_snapshots_includes_bs; aio_context_acquire(ctx); @@ -637,9 +636,8 @@ int bdrv_all_goto_snapshot(const char *name, all_snapshots_includes_bs = bdrv_all_snapshots_includes_bs(bs); bdrv_graph_rdunlock_main_loop(); - if (devices || all_snapshots_includes_bs) { - ret = bdrv_snapshot_goto(bs, name, errp); - } + ret = (devices || all_snapshots_includes_bs) ? + bdrv_snapshot_goto(bs, name, errp) : 0; aio_context_release(ctx); if (ret < 0) { bdrv_graph_rdlock_main_loop(); Better. Unconditional assignment to @ret right before it's checked is how we should use @ret. Reviewed-by: Markus Armbruster And queued. Thanks! Mind if I drop the Fixes: tag? Nothing broken until we enable -Wshadow=local... Fine for me! This looks like the last remaining warning. Are we going to activate -Wshadow=local next ? Thanks, C.
Re: [PATCH v6 1/6] virtio: split into vhost-user-base and vhost-user-device
On Mon, Nov 06, 2023 at 07:15:10PM +, Alex Bennée wrote: > Lets keep a cleaner split between the base class and the derived > vhost-user-device which we can use for generic vhost-user stubs. This > includes an update to introduce the vq_size property so the number of > entries in a virtq can be defined. > > Signed-off-by: Alex Bennée > Message-Id: <20231009095937.195728-2-alex.ben...@linaro.org> Please do not put Message-Id tags in patches you post. These are used to map patches to email - this patch was In-Reply-To: <20231106191515.2801863-2-alex.ben...@linaro.org> > --- > v5 > - s/parent/parent_obj/ > - remove left over vhost-user-device.h > - use DEFINE_TYPES > v6 > - rebase and set .abstrace = true for vhost-user-device > --- > ...{vhost-user-device.h => vhost-user-base.h} | 21 +- > hw/virtio/vhost-user-base.c | 345 ++ > hw/virtio/vhost-user-device-pci.c | 10 +- > hw/virtio/vhost-user-device.c | 337 + > hw/virtio/meson.build | 1 + > 5 files changed, 372 insertions(+), 342 deletions(-) > rename include/hw/virtio/{vhost-user-device.h => vhost-user-base.h} (71%) > create mode 100644 hw/virtio/vhost-user-base.c > > diff --git a/include/hw/virtio/vhost-user-device.h > b/include/hw/virtio/vhost-user-base.h > similarity index 71% > rename from include/hw/virtio/vhost-user-device.h > rename to include/hw/virtio/vhost-user-base.h > index 3ddf88a146..51d0968b89 100644 > --- a/include/hw/virtio/vhost-user-device.h > +++ b/include/hw/virtio/vhost-user-base.h > @@ -6,8 +6,8 @@ > * SPDX-License-Identifier: GPL-2.0-or-later > */ > > -#ifndef QEMU_VHOST_USER_DEVICE_H > -#define QEMU_VHOST_USER_DEVICE_H > +#ifndef QEMU_VHOST_USER_BASE_H > +#define QEMU_VHOST_USER_BASE_H > > #include "hw/virtio/vhost.h" > #include "hw/virtio/vhost-user.h" > @@ -17,11 +17,13 @@ > OBJECT_DECLARE_TYPE(VHostUserBase, VHostUserBaseClass, VHOST_USER_BASE) > > struct VHostUserBase { > -VirtIODevice parent; > +VirtIODevice parent_obj; > + > /* Properties */ > CharBackend chardev; > uint16_t virtio_id; > uint32_t num_vqs; > +uint32_t vq_size; /* can't exceed VIRTIO_QUEUE_MAX */ > uint32_t config_size; > /* State tracking */ > VhostUserState vhost_user; > @@ -31,16 +33,17 @@ struct VHostUserBase { > bool connected; > }; > > -/* needed so we can use the base realize after specialisation > - tweaks */ > +/* > + * Needed so we can use the base realize after specialisation > + * tweaks > + */ > struct VHostUserBaseClass { > -/*< private >*/ > VirtioDeviceClass parent_class; > -/*< public >*/ > + > DeviceRealize parent_realize; > }; > > -/* shared for the benefit of the derived pci class */ > + > #define TYPE_VHOST_USER_DEVICE "vhost-user-device" > > -#endif /* QEMU_VHOST_USER_DEVICE_H */ > +#endif /* QEMU_VHOST_USER_BASE_H */ > diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c > new file mode 100644 > index 00..ebb4795ebf > --- /dev/null > +++ b/hw/virtio/vhost-user-base.c > @@ -0,0 +1,345 @@ > +/* > + * Base vhost-user-base implementation. This can be used to derive a > + * more fully specified vhost-user backend either generically (see > + * vhost-user-device) or via a specific stub for a device which > + * encapsulates some fixed parameters. > + * > + * Copyright (c) 2023 Linaro Ltd > + * Author: Alex Bennée > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "qapi/error.h" > +#include "hw/qdev-properties.h" > +#include "hw/virtio/virtio-bus.h" > +#include "hw/virtio/vhost-user-base.h" > +#include "qemu/error-report.h" > + > +static void vub_start(VirtIODevice *vdev) > +{ > +BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); > +VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > +VHostUserBase *vub = VHOST_USER_BASE(vdev); > +int ret, i; > + > +if (!k->set_guest_notifiers) { > +error_report("binding does not support guest notifiers"); > +return; > +} > + > +ret = vhost_dev_enable_notifiers(>vhost_dev, vdev); > +if (ret < 0) { > +error_report("Error enabling host notifiers: %d", -ret); > +return; > +} > + > +ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true); > +if (ret < 0) { > +error_report("Error binding guest notifier: %d", -ret); > +goto err_host_notifiers; > +} > + > +vub->vhost_dev.acked_features = vdev->guest_features; > + > +ret = vhost_dev_start(>vhost_dev, vdev, true); > +if (ret < 0) { > +error_report("Error starting vhost-user-base: %d", -ret); > +goto err_guest_notifiers; > +} > + > +/* > + * guest_notifier_mask/pending not used yet, so just unmask > + * everything here. virtio-pci will do the right thing by > + * enabling/disabling irqfd. > + */ > +
Re: [PATCH v6 1/6] virtio: split into vhost-user-base and vhost-user-device
On Mon, Nov 06, 2023 at 07:15:10PM +, Alex Bennée wrote: > Lets keep a cleaner split between the base class and the derived > vhost-user-device which we can use for generic vhost-user stubs. This > includes an update to introduce the vq_size property so the number of > entries in a virtq can be defined. > > Signed-off-by: Alex Bennée > Message-Id: <20231009095937.195728-2-alex.ben...@linaro.org> > > --- > v5 > - s/parent/parent_obj/ > - remove left over vhost-user-device.h > - use DEFINE_TYPES > v6 > - rebase and set .abstrace = true for vhost-user-device abstract :) ... > +static const TypeInfo vub_types[] = { > +{ > +.name = TYPE_VHOST_USER_BASE, > +.parent = TYPE_VIRTIO_DEVICE, > +.instance_size = sizeof(VHostUserBase), > +.class_init = vub_class_init, > +.class_size = sizeof(VHostUserBaseClass), > +.abstract = true I suspect it's this change that breaks CI: https://gitlab.com/mstredhat/qemu/-/jobs/5472898562 WARNING: qemu received signal 6; command: "./qemu-system-ppc -display none -vga none -chardev socket,id=mon,fd=3 -mon chardev=mon,mode=control -S -machine g3beige,accel=tcg -device vhost-user-device-pci" CRITICAL: failed: binary=./qemu-system-ppc accel=tcg machine=g3beige device=vhost-user-device-pci CRITICAL: cmdline: ./qemu-system-ppc -S -machine g3beige,accel=tcg -device vhost-user-device-pci CRITICAL: log: ** CRITICAL: log: ERROR:../qom/object.c:524:object_initialize_with_type: assertion failed: (type->abstract == false) CRITICAL: log: Bail out! ERROR:../qom/object.c:524:object_initialize_with_type: assertion failed: (type->abstract == false) CRITICAL: exit code: -6 146/395 qemu:qtest+qtest-mips / qtest-mips/device-introspect-test ERROR 1.27s killed by signal 6 SIGABRT 153/395 qemu:qtest+qtest-xtensa / qtest-xtensa/device-introspect-test ERROR 1.56s killed by signal 6 SIGABRT 171/395 qemu:qtest+qtest-riscv32 / qtest-riscv32/device-introspect-test ERROR 1.40s killed by signal 6 SIGABRT 184/395 qemu:qtest+qtest-ppc / qtest-ppc/device-introspect-test ERROR 1.51s killed by signal 6 SIGABRT 195/395 qemu:qtest+qtest-sparc64 / qtest-sparc64/device-introspect-test ERROR 1.35s killed by signal 6 SIGABRT I dropped this .abstract = true line for now and the part about it in docs. Pls send patch on top once pull is merged. -- MST
Re: [PATCH v2 0/2] virtio-blk: add iothread-vq-mapping parameter
On Thu, Nov 02, 2023 at 03:10:52PM +0100, Kevin Wolf wrote: > Am 18.09.2023 um 18:16 hat Stefan Hajnoczi geschrieben: > > virtio-blk and virtio-scsi devices need a way to specify the mapping between > > IOThreads and virtqueues. At the moment all virtqueues are assigned to a > > single > > IOThread or the main loop. This single thread can be a CPU bottleneck, so > > it is > > necessary to allow finer-grained assignment to spread the load. With this > > series applied, "pidstat -t 1" shows that guests with -smp 2 or higher are > > able > > to exploit multiple IOThreads. > > > > This series introduces command-line syntax for the new iothread-vq-mapping > > property is as follows: > > > > --device > > '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread0","vqs":[0,1,2]},...]},...' > > > > IOThreads are specified by name and virtqueues are specified by 0-based > > index. > > > > It will be common to simply assign virtqueues round-robin across a set > > of IOThreads. A convenient syntax that does not require specifying > > individual virtqueue indices is available: > > > > --device > > '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread0"},{"iothread":"iothread1"},...]},...' > > > > There is no way to reassign virtqueues at runtime and I expect that to be a > > very rare requirement. > > > > Note that JSON --device syntax is required for the iothread-vq-mapping > > parameter because it's non-scalar. > > > > Based-on: 20230912231037.826804-1-stefa...@redhat.com ("[PATCH v3 0/5] > > block-backend: process I/O in the current AioContext") > > Does this strictly depend on patch 5/5 of that series, or would it just > be a missed opportunity for optimisation by unnecessarily running some > requests from a different thread? I looked at the issue with PATCH 5/5 more and didn't find a minimal solution that I can implement today for soft freeze. There are too much inconsistency between blk_remove_bs() in whether or not the AioContext is acquired: block/block-backend.c:blk_remove_bs(blk); <- blk_unref (can't tell if AioContext is acquired) block/block-backend.c:blk_remove_bs(blk); (acquired) block/monitor/block-hmp-cmds.c:blk_remove_bs(blk); (acquired) block/qapi-sysemu.c:blk_remove_bs(blk); (acquired) block/qapi-sysemu.c:blk_remove_bs(blk); (not acquired) qemu-nbd.c:blk_remove_bs(blk); (not acquired) tests/unit/test-block-iothread.c:blk_remove_bs(blk); (acquired) tests/unit/test-blockjob.c:blk_remove_bs(blk); (sometimes acquired, sometimes not) They usually get away with it because BDRV_WAIT_WHILE() only unlocks the AioContext when the BlockDriverState's AioContext is not the current thread's home context. This means main loop code works when the AioContext is not acquired as long as the BDS AioContext is the main loop AioContext. The solution I have confidence in is to stop using the AioContext lock, but it will take more time to refactor the SCSI layer (the last real user of the AioContext). I'm afraid iothread-vq-mapping can't make it into QEMU 8.2. Stefan signature.asc Description: PGP signature
Re: [PULL 0/3] Block patches
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PULL 0/7] xenfv-stable queue
Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any user-visible changes. signature.asc Description: PGP signature
Re: [PATCH v2 0/2] virtio-blk: add iothread-vq-mapping parameter
On Thu, Nov 02, 2023 at 03:10:52PM +0100, Kevin Wolf wrote: > Am 18.09.2023 um 18:16 hat Stefan Hajnoczi geschrieben: > > virtio-blk and virtio-scsi devices need a way to specify the mapping between > > IOThreads and virtqueues. At the moment all virtqueues are assigned to a > > single > > IOThread or the main loop. This single thread can be a CPU bottleneck, so > > it is > > necessary to allow finer-grained assignment to spread the load. With this > > series applied, "pidstat -t 1" shows that guests with -smp 2 or higher are > > able > > to exploit multiple IOThreads. > > > > This series introduces command-line syntax for the new iothread-vq-mapping > > property is as follows: > > > > --device > > '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread0","vqs":[0,1,2]},...]},...' > > > > IOThreads are specified by name and virtqueues are specified by 0-based > > index. > > > > It will be common to simply assign virtqueues round-robin across a set > > of IOThreads. A convenient syntax that does not require specifying > > individual virtqueue indices is available: > > > > --device > > '{"driver":"virtio-blk-pci","iothread-vq-mapping":[{"iothread":"iothread0"},{"iothread":"iothread1"},...]},...' > > > > There is no way to reassign virtqueues at runtime and I expect that to be a > > very rare requirement. > > > > Note that JSON --device syntax is required for the iothread-vq-mapping > > parameter because it's non-scalar. > > > > Based-on: 20230912231037.826804-1-stefa...@redhat.com ("[PATCH v3 0/5] > > block-backend: process I/O in the current AioContext") > > Does this strictly depend on patch 5/5 of that series, or would it just > be a missed opportunity for optimisation by unnecessarily running some > requests from a different thread? "[PATCH v3 5/5] block-coroutine-wrapper: use qemu_get_current_aio_context()" is necessary so that virtio_blk_sect_range_ok -> blk_get_geometry -> blk_nb_sectors -> bdrv_refresh_total_sectors -> bdrv_poll_co can be called without holding the AioContext lock. That case only happens when the BlockDriverState is a file-posix host CD-ROM or a file-win32 host_device. Most users will never hit this problem, but it would be unsafe to proceed merging code without this patch. > I suspect it does depend on the other virtio-blk series, though: > > [PATCH 0/4] virtio-blk: prepare for the multi-queue block layer > https://patchew.org/QEMU/20230914140101.1065008-1-stefa...@redhat.com/ > > Is this right? Yes, it depends on "[PATCH 0/4] virtio-blk: prepare for the multi-queue block layer" so that every AioContext is able to handle Linux AIO or io_uring I/O and to stop using the AioContext lock in the virtio-blk I/O code path. Stefan > > Given that soft freeze is early next week, maybe we should try to merge > just the bare minimum of strictly necessary dependencies. > > Kevin > signature.asc Description: PGP signature
Re: [PULL 00/60] Misc HW/UI patches for 2023-11-06
On Mon, 6 Nov 2023 at 19:03, Philippe Mathieu-Daudé wrote: > > The following changes since commit d762bf97931b58839316b68a570eecc6143c9e3e: > > Merge tag 'pull-target-arm-20231102' of > https://git.linaro.org/people/pmaydell/qemu-arm into staging (2023-11-03 > 10:04:12 +0800) > > are available in the Git repository at: > > https://github.com/philmd/qemu.git tags/misc-cpus-20231106 > > for you to fetch changes up to a81b438ac3933419910cbdf2e2e8d87681de611e: > > ui/sdl2: use correct key names in win title on mac (2023-11-06 11:07:32 > +0100) > > Few checkpatch warnings in target/i386/hvf/x86_emu.c are deliberately ignored. > > Misc hardware patch queue > > HW emulation: > - PMBus fixes and tests (Titus) > - IDE fixes and tests (Fiona) > - New ADM1266 sensor (Titus) > - Better error propagation in PCI-ISA i82378 (Philippe) > > Topology: > - Fix CPUState::nr_cores calculation (Zhuocheng Ding and Zhao Liu) > > Monitor: > - Synchronize CPU state in 'info lapic' (Dongli Zhang) > > QOM: > - Have 'cpu-qom.h' target-agnostic (Philippe) > - Call object_class_is_abstract once in cpu_class_by_name (Philippe) > > UI: > - Use correct key names in titles on MacOS / SDL2 (Adrian) > > MIPS: > - Fix MSA BZ/BNZ and TX79 LQ/SQ opcodes (Philippe) > > Nios2: > - Create IRQs *after* vCPU is realized (Philippe) > > PPC: > - Restrict KVM objects to system emulation (Philippe) > > X86: > - HVF & KVM cleanups (Philippe) > > Various targets: > - Use env_archcpu() to optimize (Philippe) > > Misc: > - Few global variable shadowing removed (Philippe) > - Introduce cpu_exec_reset_hold and factor tcg_cpu_reset_hold out (Philippe) > - Remove few more 'softmmu' mentions (Philippe) > - Fix and cleanup in vl.c (Akihiko & Marc-André) > - MAINTAINERS updates (Thomas, Daniel) > > > > Adrian Wowk (1): > ui/sdl2: use correct key names in win title on mac > > Akihiko Odaki (1): > vl: Free machine list > > Daniel P. Berrangé (1): > MAINTAINERS: update libvirt devel mailing list address > > Dongli Zhang (1): > target/i386/monitor: synchronize cpu state for lapic info > > Fiona Ebner (2): > hw/ide: reset: cancel async DMA operation before resetting state > tests/qtest: ahci-test: add test exposing reset issue with pending > callback > > Marc-André Lureau (1): > vl: constify default_list > > Philippe Mathieu-Daudé (39): > tests/vm/ubuntu.aarch64: Correct comment about TCG specific delay > tests/unit/test-seccomp: Remove mentions of softmmu in test names > accel/tcg: Declare tcg_flush_jmp_cache() in 'exec/tb-flush.h' > accel: Introduce cpu_exec_reset_hold() > accel/tcg: Factor tcg_cpu_reset_hold() out > target: Unify QOM style > target: Mention 'cpu-qom.h' is target agnostic > target/arm: Move internal declarations from 'cpu-qom.h' to 'cpu.h' > target/ppc: Remove CPU_RESOLVING_TYPE from 'cpu-qom.h' > target/riscv: Remove CPU_RESOLVING_TYPE from 'cpu-qom.h' > target: Declare FOO_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h' > target/hexagon: Declare QOM definitions in 'cpu-qom.h' > target/loongarch: Declare QOM definitions in 'cpu-qom.h' > target/nios2: Declare QOM definitions in 'cpu-qom.h' > target/openrisc: Declare QOM definitions in 'cpu-qom.h' > target/riscv: Move TYPE_RISCV_CPU_BASE definition to 'cpu.h' > target/ppc: Use env_archcpu() in helper_book3s_msgsndp() > target/riscv: Use env_archcpu() in [check_]nanbox() > target/s390x: Use env_archcpu() in handle_diag_308() > target/xtensa: Use env_archcpu() in update_c[compare|count]() > target/i386/hvf: Use x86_cpu in simulate_[rdmsr|wrmsr]() > target/i386/hvf: Use env_archcpu() in simulate_[rdmsr/wrmsr]() > target/i386/hvf: Use CPUState typedef > target/i386/hvf: Rename 'CPUState *cpu' variable as 'cs' > target/i386/hvf: Rename 'X86CPU *x86_cpu' variable as 'cpu' > target/i386/kvm: Correct comment in kvm_cpu_realize() > target/mips: Fix MSA BZ/BNZ opcodes displacement > target/mips: Fix TX79 LQ/SQ opcodes > sysemu/kvm: Restrict kvmppc_get_radix_page_info() to ppc targets > hw/ppc/e500: Restrict ppce500_init_mpic_kvm() to KVM > target/ppc: Restrict KVM objects to system emulation > target/ppc: Prohibit target specific KVM prototypes on user emulation > target/nios2: Create IRQs *after* accelerator vCPU is realized > target/alpha: Tidy up alpha_cpu_class_by_name() > hw/cpu: Call object_class_is_abstract() once in cpu_class_by_name() > exec/cpu: Have cpu_exec_realize() return a boolean > hw/cpu: Clean up global variable
Re: [PATCH v2 1/3] ide/pci.c: introduce pci_ide_update_mode() function
On 06/11/2023 14:12, Kevin Wolf wrote: Hi Kevin, Thanks for taking the time to review this. I'll reply inline below. Am 25.10.2023 um 00:40 hat Mark Cave-Ayland geschrieben: This function reads the value of the PCI_CLASS_PROG register for PCI IDE controllers and configures the PCI BARs and/or IDE ioports accordingly. In the case where we switch to legacy mode, the PCI BARs are set to return zero (as suggested in the "PCI IDE Controller" specification), the legacy IDE ioports are enabled, and the PCI interrupt pin cleared to indicate legacy IRQ routing. Conversely when we switch to native mode, the legacy IDE ioports are disabled and the PCI interrupt pin set to indicate native IRQ routing. The contents of the PCI BARs are unspecified, but this is not an issue since if a PCI IDE controller has been switched to native mode then its BARs will need to be programmed. Signed-off-by: Mark Cave-Ayland Tested-by: BALATON Zoltan Tested-by: Bernhard Beschow --- hw/ide/pci.c | 90 include/hw/ide/pci.h | 1 + 2 files changed, 91 insertions(+) diff --git a/hw/ide/pci.c b/hw/ide/pci.c index a25b352537..5be643b460 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -104,6 +104,96 @@ const MemoryRegionOps pci_ide_data_le_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; +static const MemoryRegionPortio ide_portio_list[] = { +{ 0, 8, 1, .read = ide_ioport_read, .write = ide_ioport_write }, +{ 0, 1, 2, .read = ide_data_readw, .write = ide_data_writew }, +{ 0, 1, 4, .read = ide_data_readl, .write = ide_data_writel }, +PORTIO_END_OF_LIST(), +}; + +static const MemoryRegionPortio ide_portio2_list[] = { +{ 0, 1, 1, .read = ide_status_read, .write = ide_ctrl_write }, +PORTIO_END_OF_LIST(), +}; This is duplicated from hw/ide/ioport.c. I think it would be better to use the arrays already defined there, ideally by calling ioport.c functions to setup and release the I/O ports. The tricky part here is that hw/ide/ioport.c is defined for CONFIG_ISA, and so if we did that then all PCI IDE controllers would become dependent upon ISA too, regardless of whether they implement compatibility mode or not. What do you think is the best solution here? Perhaps moving ide_init_ioport() to a more ISA-specific place? I know that both myself and Phil have considered whether ide_init_ioport() should be replaced by something else further down the line. +void pci_ide_update_mode(PCIIDEState *s) +{ +PCIDevice *d = PCI_DEVICE(s); +uint8_t mode = d->config[PCI_CLASS_PROG]; + +switch (mode & 0xf) { +case 0xa: +/* Both channels legacy mode */ Why is it ok to handle only the case where both channels are set to the same mode? The spec describes mixed-mode setups, too, and doesn't seem to allow ignoring a mode change if it's only for one of the channels. Certainly that can be done: only both channels were implemented initially because that was the test case immediately available using the VIA. I can have a look at implementing both channels separately in v2. + +/* Zero BARs */ +pci_set_long(d->config + PCI_BASE_ADDRESS_0, 0x0); +pci_set_long(d->config + PCI_BASE_ADDRESS_1, 0x0); +pci_set_long(d->config + PCI_BASE_ADDRESS_2, 0x0); +pci_set_long(d->config + PCI_BASE_ADDRESS_3, 0x0); Here I'm not sure what the spec really implies. Disabling the BAR (i.e. making it read-only and returning 0) while in compatibility mode doesn't necessarily mean that the value of the register is lost. In other words, are we sure that a driver can't expect that the old value is still there when it re-enables native mode? The specification is definitely a bit vague on the details here. In the testing I've done with the VIA, there is only ever a switch from native to legacy mode, and not the other way around. I can see the logic that once you've gone from the native to legacy mode, the memory allocated for the BARs is already reserved by the OS so in theory it could be possible to switch back to native mode again... and that would work if the BARs are preserved. Would it happen in practice? I'm not really sure, but I can try to implement this if you think it makes sense to take a safer approach. +/* Clear interrupt pin */ +pci_config_set_interrupt_pin(d->config, 0); Unlike for the BARs, I don't see anything in the spec that allows disabling this byte. I doubt it hurts in practice, but did you see any drivers requiring this? According to the spec, we just must not use the PCI interrupt in compatbility mode, but the registers stay accessible. The PCI config dumps taken from a real VIA indicate that this byte is cleared in legacy mode, and that appears to make sense here. If you imagine an early PCI IDE controller, it will always start up in legacy mode and so you don't want to indicate to the guest OS that PCI IRQ routing is required unless it has been switched to
Re: [PATCH v5 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
"Michael S. Tsirkin" writes: > On Mon, Nov 06, 2023 at 05:30:39PM +, Alex Bennée wrote: >> "Michael S. Tsirkin" writes: >> >> > On Thu, Oct 19, 2023 at 10:56:07AM +0100, Alex Bennée wrote: >> >> Now the new base class supports config handling we can take advantage >> >> and make vhost-user-gpio a much simpler boilerplate wrapper. Also as >> >> this doesn't require any target specific hacks we only need to build >> >> the stubs once. >> >> >> >> Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org> >> >> Acked-by: Mark Cave-Ayland >> >> Acked-by: Viresh Kumar >> >> Signed-off-by: Alex Bennée >> >> Message-Id: <20231009095937.195728-4-alex.ben...@linaro.org> >> >> --- >> >> -case CHR_EVENT_OPENED: >> >> -if (vu_gpio_connect(dev, _err) < 0) { >> >> -qemu_chr_fe_disconnect(>chardev); >> >> -return; >> >> -} >> >> -break; >> >> -case CHR_EVENT_CLOSED: >> >> -/* defer close until later to avoid circular close */ >> >> -vhost_user_async_close(dev, >chardev, >vhost_dev, >> >> - vu_gpio_disconnect); >> > >> > Hmm. Looking at this, it seems that the base device will handle close >> > synchronously. No? Why isn't this a problem? >> >> I suspect it was a copy and paste from another vhost-user impl. But >> testing has shown it works ok. > > Can you rebase on latest master then please? There have been > changes exactly in this area. Sent v6 to the list: https://patchew.org/QEMU/20231106191515.2801863-1-alex.ben...@linaro.org/ -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH v6 1/6] virtio: split into vhost-user-base and vhost-user-device
Lets keep a cleaner split between the base class and the derived vhost-user-device which we can use for generic vhost-user stubs. This includes an update to introduce the vq_size property so the number of entries in a virtq can be defined. Signed-off-by: Alex Bennée Message-Id: <20231009095937.195728-2-alex.ben...@linaro.org> --- v5 - s/parent/parent_obj/ - remove left over vhost-user-device.h - use DEFINE_TYPES v6 - rebase and set .abstrace = true for vhost-user-device --- ...{vhost-user-device.h => vhost-user-base.h} | 21 +- hw/virtio/vhost-user-base.c | 345 ++ hw/virtio/vhost-user-device-pci.c | 10 +- hw/virtio/vhost-user-device.c | 337 + hw/virtio/meson.build | 1 + 5 files changed, 372 insertions(+), 342 deletions(-) rename include/hw/virtio/{vhost-user-device.h => vhost-user-base.h} (71%) create mode 100644 hw/virtio/vhost-user-base.c diff --git a/include/hw/virtio/vhost-user-device.h b/include/hw/virtio/vhost-user-base.h similarity index 71% rename from include/hw/virtio/vhost-user-device.h rename to include/hw/virtio/vhost-user-base.h index 3ddf88a146..51d0968b89 100644 --- a/include/hw/virtio/vhost-user-device.h +++ b/include/hw/virtio/vhost-user-base.h @@ -6,8 +6,8 @@ * SPDX-License-Identifier: GPL-2.0-or-later */ -#ifndef QEMU_VHOST_USER_DEVICE_H -#define QEMU_VHOST_USER_DEVICE_H +#ifndef QEMU_VHOST_USER_BASE_H +#define QEMU_VHOST_USER_BASE_H #include "hw/virtio/vhost.h" #include "hw/virtio/vhost-user.h" @@ -17,11 +17,13 @@ OBJECT_DECLARE_TYPE(VHostUserBase, VHostUserBaseClass, VHOST_USER_BASE) struct VHostUserBase { -VirtIODevice parent; +VirtIODevice parent_obj; + /* Properties */ CharBackend chardev; uint16_t virtio_id; uint32_t num_vqs; +uint32_t vq_size; /* can't exceed VIRTIO_QUEUE_MAX */ uint32_t config_size; /* State tracking */ VhostUserState vhost_user; @@ -31,16 +33,17 @@ struct VHostUserBase { bool connected; }; -/* needed so we can use the base realize after specialisation - tweaks */ +/* + * Needed so we can use the base realize after specialisation + * tweaks + */ struct VHostUserBaseClass { -/*< private >*/ VirtioDeviceClass parent_class; -/*< public >*/ + DeviceRealize parent_realize; }; -/* shared for the benefit of the derived pci class */ + #define TYPE_VHOST_USER_DEVICE "vhost-user-device" -#endif /* QEMU_VHOST_USER_DEVICE_H */ +#endif /* QEMU_VHOST_USER_BASE_H */ diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c new file mode 100644 index 00..ebb4795ebf --- /dev/null +++ b/hw/virtio/vhost-user-base.c @@ -0,0 +1,345 @@ +/* + * Base vhost-user-base implementation. This can be used to derive a + * more fully specified vhost-user backend either generically (see + * vhost-user-device) or via a specific stub for a device which + * encapsulates some fixed parameters. + * + * Copyright (c) 2023 Linaro Ltd + * Author: Alex Bennée + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "hw/qdev-properties.h" +#include "hw/virtio/virtio-bus.h" +#include "hw/virtio/vhost-user-base.h" +#include "qemu/error-report.h" + +static void vub_start(VirtIODevice *vdev) +{ +BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); +VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); +VHostUserBase *vub = VHOST_USER_BASE(vdev); +int ret, i; + +if (!k->set_guest_notifiers) { +error_report("binding does not support guest notifiers"); +return; +} + +ret = vhost_dev_enable_notifiers(>vhost_dev, vdev); +if (ret < 0) { +error_report("Error enabling host notifiers: %d", -ret); +return; +} + +ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true); +if (ret < 0) { +error_report("Error binding guest notifier: %d", -ret); +goto err_host_notifiers; +} + +vub->vhost_dev.acked_features = vdev->guest_features; + +ret = vhost_dev_start(>vhost_dev, vdev, true); +if (ret < 0) { +error_report("Error starting vhost-user-base: %d", -ret); +goto err_guest_notifiers; +} + +/* + * guest_notifier_mask/pending not used yet, so just unmask + * everything here. virtio-pci will do the right thing by + * enabling/disabling irqfd. + */ +for (i = 0; i < vub->vhost_dev.nvqs; i++) { +vhost_virtqueue_mask(>vhost_dev, vdev, i, false); +} + +return; + +err_guest_notifiers: +k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false); +err_host_notifiers: +vhost_dev_disable_notifiers(>vhost_dev, vdev); +} + +static void vub_stop(VirtIODevice *vdev) +{ +VHostUserBase *vub = VHOST_USER_BASE(vdev); +BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); +VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); +int ret; + +
[PATCH v6 0/6] virtio: cleanup vhost-user-generic and reduce c
A lot of our vhost-user stubs are large chunks of boilerplate that do (mostly) the same thing. This series continues the cleanups by splitting the vhost-user-base and vhost-user-generic implementations. After adding a new vq_size property the rng, gpio and i2c vhost-user devices become simple specialisations of the common base defining the ID, number of queues and potentially the config handling. I've also added Manos' vhost-user-sound while I was at it. Changes --- v6 - re-base to current master - make vhost-user-device abstract - mention abstractness in docs v5 - addressing comments and tags - improved the docs v4 - dropped the F_TRANSPORT work for another series - added vhost-user-sound Alex Bennée (5): virtio: split into vhost-user-base and vhost-user-device hw/virtio: derive vhost-user-rng from vhost-user-base hw/virtio: derive vhost-user-gpio from vhost-user-base hw/virtio: derive vhost-user-i2c from vhost-user-base docs/system: add a basic enumeration of vhost-user devices Manos Pitsidianakis (1): hw/virtio: add vhost-user-snd and virtio-snd-pci devices docs/system/devices/vhost-user-rng.rst| 2 + docs/system/devices/vhost-user.rst| 65 ++- ...{vhost-user-device.h => vhost-user-base.h} | 21 +- include/hw/virtio/vhost-user-gpio.h | 23 +- include/hw/virtio/vhost-user-i2c.h| 14 +- include/hw/virtio/vhost-user-rng.h| 11 +- include/hw/virtio/vhost-user-snd.h| 26 ++ hw/virtio/vhost-user-base.c | 345 +++ hw/virtio/vhost-user-device-pci.c | 10 +- hw/virtio/vhost-user-device.c | 337 +-- hw/virtio/vhost-user-gpio.c | 406 +- hw/virtio/vhost-user-i2c.c| 272 +--- hw/virtio/vhost-user-rng.c| 278 +--- hw/virtio/vhost-user-snd-pci.c| 75 hw/virtio/vhost-user-snd.c| 67 +++ hw/virtio/Kconfig | 5 + hw/virtio/meson.build | 23 +- 17 files changed, 690 insertions(+), 1290 deletions(-) rename include/hw/virtio/{vhost-user-device.h => vhost-user-base.h} (71%) create mode 100644 include/hw/virtio/vhost-user-snd.h create mode 100644 hw/virtio/vhost-user-base.c create mode 100644 hw/virtio/vhost-user-snd-pci.c create mode 100644 hw/virtio/vhost-user-snd.c -- 2.39.2
[PATCH v6 4/6] hw/virtio: derive vhost-user-i2c from vhost-user-base
Now we can take advantage of the new base class and make vhost-user-i2c a much simpler boilerplate wrapper. Also as this doesn't require any target specific hacks we only need to build the stubs once. Message-Id: <20230418162140.373219-13-alex.ben...@linaro.org> Acked-by: Mark Cave-Ayland Acked-by: Viresh Kumar Signed-off-by: Alex Bennée Message-Id: <20231009095937.195728-5-alex.ben...@linaro.org> --- include/hw/virtio/vhost-user-i2c.h | 14 +- hw/virtio/vhost-user-i2c.c | 272 ++--- hw/virtio/meson.build | 5 +- 3 files changed, 23 insertions(+), 268 deletions(-) diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h index 0f7acd40e3..a8fcb108db 100644 --- a/include/hw/virtio/vhost-user-i2c.h +++ b/include/hw/virtio/vhost-user-i2c.h @@ -9,23 +9,17 @@ #ifndef QEMU_VHOST_USER_I2C_H #define QEMU_VHOST_USER_I2C_H +#include "hw/virtio/virtio.h" #include "hw/virtio/vhost.h" #include "hw/virtio/vhost-user.h" +#include "hw/virtio/vhost-user-base.h" #define TYPE_VHOST_USER_I2C "vhost-user-i2c-device" + OBJECT_DECLARE_SIMPLE_TYPE(VHostUserI2C, VHOST_USER_I2C) struct VHostUserI2C { -VirtIODevice parent; -CharBackend chardev; -struct vhost_virtqueue *vhost_vq; -struct vhost_dev vhost_dev; -VhostUserState vhost_user; -VirtQueue *vq; -bool connected; +VHostUserBase parent; }; -/* Virtio Feature bits */ -#define VIRTIO_I2C_F_ZERO_LENGTH_REQUEST 0 - #endif /* QEMU_VHOST_USER_I2C_H */ diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c index 4eef3f0633..a464f5e039 100644 --- a/hw/virtio/vhost-user-i2c.c +++ b/hw/virtio/vhost-user-i2c.c @@ -14,253 +14,22 @@ #include "qemu/error-report.h" #include "standard-headers/linux/virtio_ids.h" -static const int feature_bits[] = { -VIRTIO_I2C_F_ZERO_LENGTH_REQUEST, -VIRTIO_F_RING_RESET, -VHOST_INVALID_FEATURE_BIT +static Property vi2c_properties[] = { +DEFINE_PROP_CHR("chardev", VHostUserBase, chardev), +DEFINE_PROP_END_OF_LIST(), }; -static void vu_i2c_start(VirtIODevice *vdev) -{ -BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); -VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); -VHostUserI2C *i2c = VHOST_USER_I2C(vdev); -int ret, i; - -if (!k->set_guest_notifiers) { -error_report("binding does not support guest notifiers"); -return; -} - -ret = vhost_dev_enable_notifiers(>vhost_dev, vdev); -if (ret < 0) { -error_report("Error enabling host notifiers: %d", -ret); -return; -} - -ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, true); -if (ret < 0) { -error_report("Error binding guest notifier: %d", -ret); -goto err_host_notifiers; -} - -i2c->vhost_dev.acked_features = vdev->guest_features; - -ret = vhost_dev_start(>vhost_dev, vdev, true); -if (ret < 0) { -error_report("Error starting vhost-user-i2c: %d", -ret); -goto err_guest_notifiers; -} - -/* - * guest_notifier_mask/pending not used yet, so just unmask - * everything here. virtio-pci will do the right thing by - * enabling/disabling irqfd. - */ -for (i = 0; i < i2c->vhost_dev.nvqs; i++) { -vhost_virtqueue_mask(>vhost_dev, vdev, i, false); -} - -return; - -err_guest_notifiers: -k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false); -err_host_notifiers: -vhost_dev_disable_notifiers(>vhost_dev, vdev); -} - -static void vu_i2c_stop(VirtIODevice *vdev) -{ -VHostUserI2C *i2c = VHOST_USER_I2C(vdev); -BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); -VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); -int ret; - -if (!k->set_guest_notifiers) { -return; -} - -vhost_dev_stop(>vhost_dev, vdev, true); - -ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false); -if (ret < 0) { -error_report("vhost guest notifier cleanup failed: %d", ret); -return; -} - -vhost_dev_disable_notifiers(>vhost_dev, vdev); -} - -static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status) -{ -VHostUserI2C *i2c = VHOST_USER_I2C(vdev); -bool should_start = virtio_device_should_start(vdev, status); - -if (vhost_dev_is_started(>vhost_dev) == should_start) { -return; -} - -if (should_start) { -vu_i2c_start(vdev); -} else { -vu_i2c_stop(vdev); -} -} - -static uint64_t vu_i2c_get_features(VirtIODevice *vdev, -uint64_t requested_features, Error **errp) -{ -VHostUserI2C *i2c = VHOST_USER_I2C(vdev); - -virtio_add_feature(_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST); -return vhost_get_features(>vhost_dev, feature_bits, requested_features); -} - -static void vu_i2c_handle_output(VirtIODevice *vdev, VirtQueue *vq) -{ -/* - * Not normally called; it's the daemon that handles the queue; -
[PATCH v6 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base
Now we can take advantage of our new base class and make vhost-user-rng a much simpler boilerplate wrapper. Also as this doesn't require any target specific hacks we only need to build the stubs once. Acked-by: Mark Cave-Ayland Signed-off-by: Alex Bennée Message-Id: <20231009095937.195728-3-alex.ben...@linaro.org> --- v5 - don't remove the in-QEMU RNG emulation! --- include/hw/virtio/vhost-user-rng.h | 11 +- hw/virtio/vhost-user-rng.c | 278 +++-- hw/virtio/meson.build | 9 +- 3 files changed, 31 insertions(+), 267 deletions(-) diff --git a/include/hw/virtio/vhost-user-rng.h b/include/hw/virtio/vhost-user-rng.h index ddd9f01eea..6cffe28807 100644 --- a/include/hw/virtio/vhost-user-rng.h +++ b/include/hw/virtio/vhost-user-rng.h @@ -12,21 +12,14 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/vhost.h" #include "hw/virtio/vhost-user.h" -#include "chardev/char-fe.h" +#include "hw/virtio/vhost-user-base.h" #define TYPE_VHOST_USER_RNG "vhost-user-rng" OBJECT_DECLARE_SIMPLE_TYPE(VHostUserRNG, VHOST_USER_RNG) struct VHostUserRNG { /*< private >*/ -VirtIODevice parent; -CharBackend chardev; -struct vhost_virtqueue *vhost_vq; -struct vhost_dev vhost_dev; -VhostUserState vhost_user; -VirtQueue *req_vq; -bool connected; - +VHostUserBase parent; /*< public >*/ }; diff --git a/hw/virtio/vhost-user-rng.c b/hw/virtio/vhost-user-rng.c index efc54cd3fb..01879c863d 100644 --- a/hw/virtio/vhost-user-rng.c +++ b/hw/virtio/vhost-user-rng.c @@ -3,7 +3,7 @@ * * Copyright (c) 2021 Mathieu Poirier * - * Implementation seriously tailored on vhost-user-i2c.c + * Simple wrapper of the generic vhost-user-device. * * SPDX-License-Identifier: GPL-2.0-or-later */ @@ -13,281 +13,47 @@ #include "hw/qdev-properties.h" #include "hw/virtio/virtio-bus.h" #include "hw/virtio/vhost-user-rng.h" -#include "qemu/error-report.h" #include "standard-headers/linux/virtio_ids.h" -static const int feature_bits[] = { -VIRTIO_F_RING_RESET, -VHOST_INVALID_FEATURE_BIT -}; - -static void vu_rng_start(VirtIODevice *vdev) -{ -VHostUserRNG *rng = VHOST_USER_RNG(vdev); -BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); -VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); -int ret; -int i; - -if (!k->set_guest_notifiers) { -error_report("binding does not support guest notifiers"); -return; -} - -ret = vhost_dev_enable_notifiers(>vhost_dev, vdev); -if (ret < 0) { -error_report("Error enabling host notifiers: %d", -ret); -return; -} - -ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, true); -if (ret < 0) { -error_report("Error binding guest notifier: %d", -ret); -goto err_host_notifiers; -} - -rng->vhost_dev.acked_features = vdev->guest_features; -ret = vhost_dev_start(>vhost_dev, vdev, true); -if (ret < 0) { -error_report("Error starting vhost-user-rng: %d", -ret); -goto err_guest_notifiers; -} - -/* - * guest_notifier_mask/pending not used yet, so just unmask - * everything here. virtio-pci will do the right thing by - * enabling/disabling irqfd. - */ -for (i = 0; i < rng->vhost_dev.nvqs; i++) { -vhost_virtqueue_mask(>vhost_dev, vdev, i, false); -} - -return; - -err_guest_notifiers: -k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false); -err_host_notifiers: -vhost_dev_disable_notifiers(>vhost_dev, vdev); -} - -static void vu_rng_stop(VirtIODevice *vdev) -{ -VHostUserRNG *rng = VHOST_USER_RNG(vdev); -BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); -VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); -int ret; - -if (!k->set_guest_notifiers) { -return; -} - -vhost_dev_stop(>vhost_dev, vdev, true); - -ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false); -if (ret < 0) { -error_report("vhost guest notifier cleanup failed: %d", ret); -return; -} - -vhost_dev_disable_notifiers(>vhost_dev, vdev); -} - -static void vu_rng_set_status(VirtIODevice *vdev, uint8_t status) -{ -VHostUserRNG *rng = VHOST_USER_RNG(vdev); -bool should_start = virtio_device_should_start(vdev, status); - -if (vhost_dev_is_started(>vhost_dev) == should_start) { -return; -} - -if (should_start) { -vu_rng_start(vdev); -} else { -vu_rng_stop(vdev); -} -} - -static uint64_t vu_rng_get_features(VirtIODevice *vdev, -uint64_t requested_features, Error **errp) -{ -VHostUserRNG *rng = VHOST_USER_RNG(vdev); - -return vhost_get_features(>vhost_dev, feature_bits, - requested_features); -} - -static void vu_rng_handle_output(VirtIODevice *vdev, VirtQueue *vq) -{ -/* - * Not normally called; it's the daemon that handles the queue; - * however
[PATCH v6 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
Now the new base class supports config handling we can take advantage and make vhost-user-gpio a much simpler boilerplate wrapper. Also as this doesn't require any target specific hacks we only need to build the stubs once. Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org> Acked-by: Mark Cave-Ayland Acked-by: Viresh Kumar Signed-off-by: Alex Bennée Message-Id: <20231009095937.195728-4-alex.ben...@linaro.org> --- include/hw/virtio/vhost-user-gpio.h | 23 +- hw/virtio/vhost-user-gpio.c | 406 ++-- hw/virtio/meson.build | 5 +- 3 files changed, 22 insertions(+), 412 deletions(-) diff --git a/include/hw/virtio/vhost-user-gpio.h b/include/hw/virtio/vhost-user-gpio.h index a9d3f9b049..5201d5f072 100644 --- a/include/hw/virtio/vhost-user-gpio.h +++ b/include/hw/virtio/vhost-user-gpio.h @@ -12,33 +12,14 @@ #include "hw/virtio/virtio.h" #include "hw/virtio/vhost.h" #include "hw/virtio/vhost-user.h" -#include "standard-headers/linux/virtio_gpio.h" -#include "chardev/char-fe.h" +#include "hw/virtio/vhost-user-base.h" #define TYPE_VHOST_USER_GPIO "vhost-user-gpio-device" OBJECT_DECLARE_SIMPLE_TYPE(VHostUserGPIO, VHOST_USER_GPIO); struct VHostUserGPIO { /*< private >*/ -VirtIODevice parent_obj; -CharBackend chardev; -struct virtio_gpio_config config; -struct vhost_virtqueue *vhost_vqs; -struct vhost_dev vhost_dev; -VhostUserState vhost_user; -VirtQueue *command_vq; -VirtQueue *interrupt_vq; -/** - * There are at least two steps of initialization of the - * vhost-user device. The first is a "connect" step and - * second is a "start" step. Make a separation between - * those initialization phases by using two fields. - * - * @connected: see vu_gpio_connect()/vu_gpio_disconnect() - * @started_vu: see vu_gpio_start()/vu_gpio_stop() - */ -bool connected; -bool started_vu; +VHostUserBase parent; /*< public >*/ }; diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c index aff2d7eff6..9f37c25415 100644 --- a/hw/virtio/vhost-user-gpio.c +++ b/hw/virtio/vhost-user-gpio.c @@ -11,387 +11,25 @@ #include "hw/qdev-properties.h" #include "hw/virtio/virtio-bus.h" #include "hw/virtio/vhost-user-gpio.h" -#include "qemu/error-report.h" #include "standard-headers/linux/virtio_ids.h" -#include "trace.h" +#include "standard-headers/linux/virtio_gpio.h" -#define VHOST_NVQS 2 - -/* Features required from VirtIO */ -static const int feature_bits[] = { -VIRTIO_F_VERSION_1, -VIRTIO_F_NOTIFY_ON_EMPTY, -VIRTIO_RING_F_INDIRECT_DESC, -VIRTIO_RING_F_EVENT_IDX, -VIRTIO_GPIO_F_IRQ, -VIRTIO_F_RING_RESET, -VHOST_INVALID_FEATURE_BIT -}; - -static void vu_gpio_get_config(VirtIODevice *vdev, uint8_t *config) -{ -VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev); - -memcpy(config, >config, sizeof(gpio->config)); -} - -static int vu_gpio_config_notifier(struct vhost_dev *dev) -{ -VHostUserGPIO *gpio = VHOST_USER_GPIO(dev->vdev); - -memcpy(dev->vdev->config, >config, sizeof(gpio->config)); -virtio_notify_config(dev->vdev); - -return 0; -} - -const VhostDevConfigOps gpio_ops = { -.vhost_dev_config_notifier = vu_gpio_config_notifier, +static Property vgpio_properties[] = { +DEFINE_PROP_CHR("chardev", VHostUserBase, chardev), +DEFINE_PROP_END_OF_LIST(), }; -static int vu_gpio_start(VirtIODevice *vdev) -{ -BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); -VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); -VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev); -struct vhost_dev *vhost_dev = >vhost_dev; -int ret, i; - -if (!k->set_guest_notifiers) { -error_report("binding does not support guest notifiers"); -return -ENOSYS; -} - -ret = vhost_dev_enable_notifiers(vhost_dev, vdev); -if (ret < 0) { -error_report("Error enabling host notifiers: %d", ret); -return ret; -} - -ret = k->set_guest_notifiers(qbus->parent, vhost_dev->nvqs, true); -if (ret < 0) { -error_report("Error binding guest notifier: %d", ret); -goto err_host_notifiers; -} - -/* - * Before we start up we need to ensure we have the final feature - * set needed for the vhost configuration. The backend may also - * apply backend_features when the feature set is sent. - */ -vhost_ack_features(>vhost_dev, feature_bits, vdev->guest_features); - -ret = vhost_dev_start(>vhost_dev, vdev, false); -if (ret < 0) { -error_report("Error starting vhost-user-gpio: %d", ret); -goto err_guest_notifiers; -} -gpio->started_vu = true; - -/* - * guest_notifier_mask/pending not used yet, so just unmask - * everything here. virtio-pci will do the right thing by - * enabling/disabling irqfd. - */ -for (i = 0; i < gpio->vhost_dev.nvqs; i++) { -vhost_virtqueue_mask(>vhost_dev, vdev, i, false); -
[PATCH v6 6/6] docs/system: add a basic enumeration of vhost-user devices
Make it clear the vhost-user-device is intended for expert use only. Signed-off-by: Alex Bennée Message-Id: <20231009095937.195728-7-alex.ben...@linaro.org> --- v5 - split vhost-user-device out of the table - sort the table alphabetically - add sound and scmi devices v6 - add note re vhost-user-device --- docs/system/devices/vhost-user-rng.rst | 2 + docs/system/devices/vhost-user.rst | 65 +- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/docs/system/devices/vhost-user-rng.rst b/docs/system/devices/vhost-user-rng.rst index a145d4105c..ead1405326 100644 --- a/docs/system/devices/vhost-user-rng.rst +++ b/docs/system/devices/vhost-user-rng.rst @@ -1,3 +1,5 @@ +.. _vhost_user_rng: + QEMU vhost-user-rng - RNG emulation === diff --git a/docs/system/devices/vhost-user.rst b/docs/system/devices/vhost-user.rst index a80e95a48a..74c8b2f8be 100644 --- a/docs/system/devices/vhost-user.rst +++ b/docs/system/devices/vhost-user.rst @@ -8,13 +8,76 @@ outside of QEMU itself. To do this there are a number of things required. vhost-user device -=== += These are simple stub devices that ensure the VirtIO device is visible to the guest. The code is mostly boilerplate although each device has a ``chardev`` option which specifies the ID of the ``--chardev`` device that connects via a socket to the vhost-user *daemon*. +Each device will have an virtio-mmio and virtio-pci variant. See your +platform details for what sort of virtio bus to use. + +.. list-table:: vhost-user devices + :widths: 20 20 60 + :header-rows: 1 + + * - Device +- Type +- Notes + * - vhost-user-blk +- Block storage +- See contrib/vhost-user-blk + * - vhost-user-fs +- File based storage driver +- See https://gitlab.com/virtio-fs/virtiofsd + * - vhost-user-gpio +- Proxy gpio pins to host +- See https://github.com/rust-vmm/vhost-device + * - vhost-user-gpu +- GPU driver +- See contrib/vhost-user-gpu + * - vhost-user-i2c +- Proxy i2c devices to host +- See https://github.com/rust-vmm/vhost-device + * - vhost-user-input +- Generic input driver +- See contrib/vhost-user-input + * - vhost-user-rng +- Entropy driver +- :ref:`vhost_user_rng` + * - vhost-user-scmi +- System Control and Management Interface +- See https://github.com/rust-vmm/vhost-device + * - vhost-user-snd +- Audio device +- See https://github.com/rust-vmm/vhost-device/staging + * - vhost-user-scsi +- SCSI based storage +- See contrib/vhost-user-scsi + * - vhost-user-vsock +- Socket based communication +- See https://github.com/rust-vmm/vhost-device + +The referenced *daemons* are not exhaustive, any conforming backend +implementing the device and using the vhost-user protocol should work. + +vhost-user-device +^ + +The vhost-user-device is a generic development device intended for +expert use while developing new backends. The user needs to specify +all the required parameters including: + + - Device ``virtio-id`` + - The ``num_vqs`` it needs and their ``vq_size`` + - The ``config_size`` if needed + +.. note:: + To prevent user confusion you cannot currently instantiate + vhost-user-device without first patching out ``.abstract = true`` in + the ``vhost-user-device.c`` file and rebuilding. + vhost-user daemon = -- 2.39.2
[PATCH v6 5/6] hw/virtio: add vhost-user-snd and virtio-snd-pci devices
From: Manos Pitsidianakis Tested with rust-vmm vhost-user-sound daemon: RUST_LOG=trace cargo run --bin vhost-user-sound -- --socket /tmp/snd.sock --backend null Invocation: qemu-system-x86_64 \ -qmp unix:./qmp-sock,server,wait=off \ -m 4096 \ -numa node,memdev=mem \ -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on \ -D qemu.log \ -d guest_errors,trace:\*snd\*,trace:\*sound\*,trace:\*vhost\* \ -chardev socket,id=vsnd,path=/tmp/snd.sock \ -device vhost-user-snd-pci,chardev=vsnd,id=snd \ /path/to/disk [AJB: imported from https://github.com/epilys/qemu-virtio-snd/commit/54ae1cdd15fef2d88e9e387a175f099a38c636f4.patch] Signed-off-by: Alex Bennée Message-Id: <20231009095937.195728-6-alex.ben...@linaro.org> --- include/hw/virtio/vhost-user-snd.h | 26 +++ hw/virtio/vhost-user-snd-pci.c | 75 ++ hw/virtio/vhost-user-snd.c | 67 ++ hw/virtio/Kconfig | 5 ++ hw/virtio/meson.build | 3 ++ 5 files changed, 176 insertions(+) create mode 100644 include/hw/virtio/vhost-user-snd.h create mode 100644 hw/virtio/vhost-user-snd-pci.c create mode 100644 hw/virtio/vhost-user-snd.c diff --git a/include/hw/virtio/vhost-user-snd.h b/include/hw/virtio/vhost-user-snd.h new file mode 100644 index 00..a1627003a0 --- /dev/null +++ b/include/hw/virtio/vhost-user-snd.h @@ -0,0 +1,26 @@ +/* + * Vhost-user Sound virtio device + * + * Copyright (c) 2021 Mathieu Poirier + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef QEMU_VHOST_USER_SND_H +#define QEMU_VHOST_USER_SND_H + +#include "hw/virtio/virtio.h" +#include "hw/virtio/vhost.h" +#include "hw/virtio/vhost-user.h" +#include "hw/virtio/vhost-user-base.h" + +#define TYPE_VHOST_USER_SND "vhost-user-snd" +OBJECT_DECLARE_SIMPLE_TYPE(VHostUserSound, VHOST_USER_SND) + +struct VHostUserSound { +/*< private >*/ +VHostUserBase parent; +/*< public >*/ +}; + +#endif /* QEMU_VHOST_USER_SND_H */ diff --git a/hw/virtio/vhost-user-snd-pci.c b/hw/virtio/vhost-user-snd-pci.c new file mode 100644 index 00..d61cfdae63 --- /dev/null +++ b/hw/virtio/vhost-user-snd-pci.c @@ -0,0 +1,75 @@ +/* + * Vhost-user Sound virtio device PCI glue + * + * Copyright (c) 2023 Manos Pitsidianakis + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "hw/qdev-properties.h" +#include "hw/virtio/vhost-user-snd.h" +#include "hw/virtio/virtio-pci.h" + +struct VHostUserSoundPCI { +VirtIOPCIProxy parent_obj; +VHostUserSound vdev; +}; + +typedef struct VHostUserSoundPCI VHostUserSoundPCI; + +#define TYPE_VHOST_USER_SND_PCI "vhost-user-snd-pci-base" + +DECLARE_INSTANCE_CHECKER(VHostUserSoundPCI, VHOST_USER_SND_PCI, + TYPE_VHOST_USER_SND_PCI) + +static Property vhost_user_snd_pci_properties[] = { +DEFINE_PROP_END_OF_LIST(), +}; + +static void vhost_user_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) +{ +VHostUserSoundPCI *dev = VHOST_USER_SND_PCI(vpci_dev); +DeviceState *vdev = DEVICE(>vdev); + +vpci_dev->nvectors = 1; + +qdev_realize(vdev, BUS(_dev->bus), errp); +} + +static void vhost_user_snd_pci_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass); +PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass); +k->realize = vhost_user_snd_pci_realize; +set_bit(DEVICE_CATEGORY_SOUND, dc->categories); +device_class_set_props(dc, vhost_user_snd_pci_properties); +pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; +pcidev_k->device_id = 0; /* Set by virtio-pci based on virtio id */ +pcidev_k->revision = 0x00; +pcidev_k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO; +} + +static void vhost_user_snd_pci_instance_init(Object *obj) +{ +VHostUserSoundPCI *dev = VHOST_USER_SND_PCI(obj); + +virtio_instance_init_common(obj, >vdev, sizeof(dev->vdev), +TYPE_VHOST_USER_SND); +} + +static const VirtioPCIDeviceTypeInfo vhost_user_snd_pci_info = { +.base_name = TYPE_VHOST_USER_SND_PCI, +.non_transitional_name = "vhost-user-snd-pci", +.instance_size = sizeof(VHostUserSoundPCI), +.instance_init = vhost_user_snd_pci_instance_init, +.class_init = vhost_user_snd_pci_class_init, +}; + +static void vhost_user_snd_pci_register(void) +{ +virtio_pci_types_register(_user_snd_pci_info); +} + +type_init(vhost_user_snd_pci_register); diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c new file mode 100644 index 00..9a217543f8 --- /dev/null +++ b/hw/virtio/vhost-user-snd.c @@ -0,0 +1,67 @@ +/* + * Vhost-user snd virtio device + * + * Copyright (c) 2023 Manos Pitsidianakis + * + * Simple wrapper of the generic vhost-user-device. + * + * SPDX-License-Identifier: GPL-2.0-or-later +
Re: [PATCH v5 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
On Mon, Nov 06, 2023 at 05:30:39PM +, Alex Bennée wrote: > "Michael S. Tsirkin" writes: > > > On Thu, Oct 19, 2023 at 10:56:07AM +0100, Alex Bennée wrote: > >> Now the new base class supports config handling we can take advantage > >> and make vhost-user-gpio a much simpler boilerplate wrapper. Also as > >> this doesn't require any target specific hacks we only need to build > >> the stubs once. > >> > >> Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org> > >> Acked-by: Mark Cave-Ayland > >> Acked-by: Viresh Kumar > >> Signed-off-by: Alex Bennée > >> Message-Id: <20231009095937.195728-4-alex.ben...@linaro.org> > >> --- > >> -case CHR_EVENT_OPENED: > >> -if (vu_gpio_connect(dev, _err) < 0) { > >> -qemu_chr_fe_disconnect(>chardev); > >> -return; > >> -} > >> -break; > >> -case CHR_EVENT_CLOSED: > >> -/* defer close until later to avoid circular close */ > >> -vhost_user_async_close(dev, >chardev, >vhost_dev, > >> - vu_gpio_disconnect); > > > > Hmm. Looking at this, it seems that the base device will handle close > > synchronously. No? Why isn't this a problem? > > I suspect it was a copy and paste from another vhost-user impl. But > testing has shown it works ok. Can you rebase on latest master then please? There have been changes exactly in this area. -- MST
Re: [PATCH v5 1/6] virtio: split into vhost-user-base and vhost-user-device
On Mon, Nov 06, 2023 at 05:40:10PM +, Alex Bennée wrote: > "Michael S. Tsirkin" writes: > > > On Thu, Oct 19, 2023 at 10:56:05AM +0100, Alex Bennée wrote: > >> Lets keep a cleaner split between the base class and the derived > >> vhost-user-device which we can use for generic vhost-user stubs. This > >> includes an update to introduce the vq_size property so the number of > >> entries in a virtq can be defined. > >> > >> Signed-off-by: Alex Bennée > >> Message-Id: <20231009095937.195728-2-alex.ben...@linaro.org> > > > > I applied but I think for this release we are better off just > > preventing users from instanciating these on command line - > > developers can always hack code to drop this. > > I guess if a user doesn't read the documentation and makes up some > random numbers for the properties they deserve what they get? Problem is users tend to create weird workflows a la https://xkcd.com/1172/ > > Can you post a patch please? > > Is: > > /* comment out if you want to use backends qemu doesn't know about */ > .abstract = true, > > really worth it? > For now I'd say so, yes. > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro
Re: [PATCH v3 0/6] migration: check required entries and sections are loaded
On Mon, Nov 06, 2023 at 03:35:54PM +0400, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > Hi, > > Surprisingly, the migration code doesn't check that required migration entries > and subsections are loaded. Either optional or required sections are both > ignored when missing. According to the documentation a "newer QEMU that knows > about a subsection can (with care) load a stream from an older QEMU that > didn't > send the subsection". I propose this behaviour to be limited to "optional" > sections only. > > This series has a few preliminary fixes, add new checks that entries are > loaded once and required ones have been loaded, add some tests and > documentation update. > > thanks I think this kind of thing is better deferred to the next release - unless you have something specific in mind this fixes? > v3: > - rebased, drop RFC status > - switch from tracepoint + returning an error to report for missing >subsections, as we worry about potential regressions > - add r-b tags > > v2: > - add "migration: rename vmstate_save_needed->vmstate_section_needed" > - add "migration: set file error on subsection loading" > - add subsection tests > - update the documentation > > Marc-André Lureau (6): > block/fdc: 'phase' is not needed on load > virtio: make endian_needed() work during loading > migration: check required subsections are loaded, once > migration: check required entries are loaded, once > test-vmstate: add some subsection tests > docs/migration: reflect the changes about needed subsections > > docs/devel/migration.rst | 17 +++--- > hw/block/fdc.c| 5 ++ > hw/virtio/virtio.c| 6 +- > migration/savevm.c| 43 ++ > migration/vmstate.c | 40 - > tests/unit/test-vmstate.c | 116 ++ > 6 files changed, 215 insertions(+), 12 deletions(-) > > -- > 2.41.0
Re: [PATCH v5 1/6] virtio: split into vhost-user-base and vhost-user-device
"Michael S. Tsirkin" writes: > On Thu, Oct 19, 2023 at 10:56:05AM +0100, Alex Bennée wrote: >> Lets keep a cleaner split between the base class and the derived >> vhost-user-device which we can use for generic vhost-user stubs. This >> includes an update to introduce the vq_size property so the number of >> entries in a virtq can be defined. >> >> Signed-off-by: Alex Bennée >> Message-Id: <20231009095937.195728-2-alex.ben...@linaro.org> > > I applied but I think for this release we are better off just > preventing users from instanciating these on command line - > developers can always hack code to drop this. I guess if a user doesn't read the documentation and makes up some random numbers for the properties they deserve what they get? > Can you post a patch please? Is: /* comment out if you want to use backends qemu doesn't know about */ .abstract = true, really worth it? -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v5 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
"Michael S. Tsirkin" writes: > On Thu, Oct 19, 2023 at 10:56:07AM +0100, Alex Bennée wrote: >> Now the new base class supports config handling we can take advantage >> and make vhost-user-gpio a much simpler boilerplate wrapper. Also as >> this doesn't require any target specific hacks we only need to build >> the stubs once. >> >> Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org> >> Acked-by: Mark Cave-Ayland >> Acked-by: Viresh Kumar >> Signed-off-by: Alex Bennée >> Message-Id: <20231009095937.195728-4-alex.ben...@linaro.org> >> --- >> -case CHR_EVENT_OPENED: >> -if (vu_gpio_connect(dev, _err) < 0) { >> -qemu_chr_fe_disconnect(>chardev); >> -return; >> -} >> -break; >> -case CHR_EVENT_CLOSED: >> -/* defer close until later to avoid circular close */ >> -vhost_user_async_close(dev, >chardev, >vhost_dev, >> - vu_gpio_disconnect); > > Hmm. Looking at this, it seems that the base device will handle close > synchronously. No? Why isn't this a problem? I suspect it was a copy and paste from another vhost-user impl. But testing has shown it works ok. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v5 1/6] virtio: split into vhost-user-base and vhost-user-device
On Thu, Oct 19, 2023 at 10:56:05AM +0100, Alex Bennée wrote: > Lets keep a cleaner split between the base class and the derived > vhost-user-device which we can use for generic vhost-user stubs. This > includes an update to introduce the vq_size property so the number of > entries in a virtq can be defined. > > Signed-off-by: Alex Bennée > Message-Id: <20231009095937.195728-2-alex.ben...@linaro.org> I applied but I think for this release we are better off just preventing users from instanciating these on command line - developers can always hack code to drop this. Can you post a patch please? > --- > v5 > - s/parent/parent_obj/ > - remove left over vhost-user-device.h > - use DEFINE_TYPES > --- > ...{vhost-user-device.h => vhost-user-base.h} | 21 +- > hw/virtio/vhost-user-base.c | 345 ++ > hw/virtio/vhost-user-device-pci.c | 10 +- > hw/virtio/vhost-user-device.c | 335 + > hw/virtio/meson.build | 1 + > 5 files changed, 370 insertions(+), 342 deletions(-) > rename include/hw/virtio/{vhost-user-device.h => vhost-user-base.h} (71%) > create mode 100644 hw/virtio/vhost-user-base.c > > diff --git a/include/hw/virtio/vhost-user-device.h > b/include/hw/virtio/vhost-user-base.h > similarity index 71% > rename from include/hw/virtio/vhost-user-device.h > rename to include/hw/virtio/vhost-user-base.h > index 3ddf88a146..51d0968b89 100644 > --- a/include/hw/virtio/vhost-user-device.h > +++ b/include/hw/virtio/vhost-user-base.h > @@ -6,8 +6,8 @@ > * SPDX-License-Identifier: GPL-2.0-or-later > */ > > -#ifndef QEMU_VHOST_USER_DEVICE_H > -#define QEMU_VHOST_USER_DEVICE_H > +#ifndef QEMU_VHOST_USER_BASE_H > +#define QEMU_VHOST_USER_BASE_H > > #include "hw/virtio/vhost.h" > #include "hw/virtio/vhost-user.h" > @@ -17,11 +17,13 @@ > OBJECT_DECLARE_TYPE(VHostUserBase, VHostUserBaseClass, VHOST_USER_BASE) > > struct VHostUserBase { > -VirtIODevice parent; > +VirtIODevice parent_obj; > + > /* Properties */ > CharBackend chardev; > uint16_t virtio_id; > uint32_t num_vqs; > +uint32_t vq_size; /* can't exceed VIRTIO_QUEUE_MAX */ > uint32_t config_size; > /* State tracking */ > VhostUserState vhost_user; > @@ -31,16 +33,17 @@ struct VHostUserBase { > bool connected; > }; > > -/* needed so we can use the base realize after specialisation > - tweaks */ > +/* > + * Needed so we can use the base realize after specialisation > + * tweaks > + */ > struct VHostUserBaseClass { > -/*< private >*/ > VirtioDeviceClass parent_class; > -/*< public >*/ > + > DeviceRealize parent_realize; > }; > > -/* shared for the benefit of the derived pci class */ > + > #define TYPE_VHOST_USER_DEVICE "vhost-user-device" > > -#endif /* QEMU_VHOST_USER_DEVICE_H */ > +#endif /* QEMU_VHOST_USER_BASE_H */ > diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c > new file mode 100644 > index 00..ebb4795ebf > --- /dev/null > +++ b/hw/virtio/vhost-user-base.c > @@ -0,0 +1,345 @@ > +/* > + * Base vhost-user-base implementation. This can be used to derive a > + * more fully specified vhost-user backend either generically (see > + * vhost-user-device) or via a specific stub for a device which > + * encapsulates some fixed parameters. > + * > + * Copyright (c) 2023 Linaro Ltd > + * Author: Alex Bennée > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "qapi/error.h" > +#include "hw/qdev-properties.h" > +#include "hw/virtio/virtio-bus.h" > +#include "hw/virtio/vhost-user-base.h" > +#include "qemu/error-report.h" > + > +static void vub_start(VirtIODevice *vdev) > +{ > +BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); > +VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > +VHostUserBase *vub = VHOST_USER_BASE(vdev); > +int ret, i; > + > +if (!k->set_guest_notifiers) { > +error_report("binding does not support guest notifiers"); > +return; > +} > + > +ret = vhost_dev_enable_notifiers(>vhost_dev, vdev); > +if (ret < 0) { > +error_report("Error enabling host notifiers: %d", -ret); > +return; > +} > + > +ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true); > +if (ret < 0) { > +error_report("Error binding guest notifier: %d", -ret); > +goto err_host_notifiers; > +} > + > +vub->vhost_dev.acked_features = vdev->guest_features; > + > +ret = vhost_dev_start(>vhost_dev, vdev, true); > +if (ret < 0) { > +error_report("Error starting vhost-user-base: %d", -ret); > +goto err_guest_notifiers; > +} > + > +/* > + * guest_notifier_mask/pending not used yet, so just unmask > + * everything here. virtio-pci will do the right thing by > + * enabling/disabling irqfd. > + */ > +for (i = 0; i < vub->vhost_dev.nvqs; i++)
Re: [PATCH v5 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
On Thu, Oct 19, 2023 at 10:56:07AM +0100, Alex Bennée wrote: > Now the new base class supports config handling we can take advantage > and make vhost-user-gpio a much simpler boilerplate wrapper. Also as > this doesn't require any target specific hacks we only need to build > the stubs once. > > Message-Id: <20230418162140.373219-12-alex.ben...@linaro.org> > Acked-by: Mark Cave-Ayland > Acked-by: Viresh Kumar > Signed-off-by: Alex Bennée > Message-Id: <20231009095937.195728-4-alex.ben...@linaro.org> > --- > include/hw/virtio/vhost-user-gpio.h | 23 +- > hw/virtio/vhost-user-gpio.c | 407 ++-- > hw/virtio/meson.build | 5 +- > 3 files changed, 22 insertions(+), 413 deletions(-) > > diff --git a/include/hw/virtio/vhost-user-gpio.h > b/include/hw/virtio/vhost-user-gpio.h > index a9d3f9b049..5201d5f072 100644 > --- a/include/hw/virtio/vhost-user-gpio.h > +++ b/include/hw/virtio/vhost-user-gpio.h > @@ -12,33 +12,14 @@ > #include "hw/virtio/virtio.h" > #include "hw/virtio/vhost.h" > #include "hw/virtio/vhost-user.h" > -#include "standard-headers/linux/virtio_gpio.h" > -#include "chardev/char-fe.h" > +#include "hw/virtio/vhost-user-base.h" > > #define TYPE_VHOST_USER_GPIO "vhost-user-gpio-device" > OBJECT_DECLARE_SIMPLE_TYPE(VHostUserGPIO, VHOST_USER_GPIO); > > struct VHostUserGPIO { > /*< private >*/ > -VirtIODevice parent_obj; > -CharBackend chardev; > -struct virtio_gpio_config config; > -struct vhost_virtqueue *vhost_vqs; > -struct vhost_dev vhost_dev; > -VhostUserState vhost_user; > -VirtQueue *command_vq; > -VirtQueue *interrupt_vq; > -/** > - * There are at least two steps of initialization of the > - * vhost-user device. The first is a "connect" step and > - * second is a "start" step. Make a separation between > - * those initialization phases by using two fields. > - * > - * @connected: see vu_gpio_connect()/vu_gpio_disconnect() > - * @started_vu: see vu_gpio_start()/vu_gpio_stop() > - */ > -bool connected; > -bool started_vu; > +VHostUserBase parent; > /*< public >*/ > }; > > diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c > index 3d7fae3984..9f37c25415 100644 > --- a/hw/virtio/vhost-user-gpio.c > +++ b/hw/virtio/vhost-user-gpio.c > @@ -11,388 +11,25 @@ > #include "hw/qdev-properties.h" > #include "hw/virtio/virtio-bus.h" > #include "hw/virtio/vhost-user-gpio.h" > -#include "qemu/error-report.h" > #include "standard-headers/linux/virtio_ids.h" > -#include "trace.h" > +#include "standard-headers/linux/virtio_gpio.h" > > -#define REALIZE_CONNECTION_RETRIES 3 > -#define VHOST_NVQS 2 > - > -/* Features required from VirtIO */ > -static const int feature_bits[] = { > -VIRTIO_F_VERSION_1, > -VIRTIO_F_NOTIFY_ON_EMPTY, > -VIRTIO_RING_F_INDIRECT_DESC, > -VIRTIO_RING_F_EVENT_IDX, > -VIRTIO_GPIO_F_IRQ, > -VIRTIO_F_RING_RESET, > -VHOST_INVALID_FEATURE_BIT > -}; > - > -static void vu_gpio_get_config(VirtIODevice *vdev, uint8_t *config) > -{ > -VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev); > - > -memcpy(config, >config, sizeof(gpio->config)); > -} > - > -static int vu_gpio_config_notifier(struct vhost_dev *dev) > -{ > -VHostUserGPIO *gpio = VHOST_USER_GPIO(dev->vdev); > - > -memcpy(dev->vdev->config, >config, sizeof(gpio->config)); > -virtio_notify_config(dev->vdev); > - > -return 0; > -} > - > -const VhostDevConfigOps gpio_ops = { > -.vhost_dev_config_notifier = vu_gpio_config_notifier, > +static Property vgpio_properties[] = { > +DEFINE_PROP_CHR("chardev", VHostUserBase, chardev), > +DEFINE_PROP_END_OF_LIST(), > }; > > -static int vu_gpio_start(VirtIODevice *vdev) > -{ > -BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); > -VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > -VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev); > -struct vhost_dev *vhost_dev = >vhost_dev; > -int ret, i; > - > -if (!k->set_guest_notifiers) { > -error_report("binding does not support guest notifiers"); > -return -ENOSYS; > -} > - > -ret = vhost_dev_enable_notifiers(vhost_dev, vdev); > -if (ret < 0) { > -error_report("Error enabling host notifiers: %d", ret); > -return ret; > -} > - > -ret = k->set_guest_notifiers(qbus->parent, vhost_dev->nvqs, true); > -if (ret < 0) { > -error_report("Error binding guest notifier: %d", ret); > -goto err_host_notifiers; > -} > - > -/* > - * Before we start up we need to ensure we have the final feature > - * set needed for the vhost configuration. The backend may also > - * apply backend_features when the feature set is sent. > - */ > -vhost_ack_features(>vhost_dev, feature_bits, vdev->guest_features); > - > -ret = vhost_dev_start(>vhost_dev, vdev, false); > -if (ret < 0) { > -error_report("Error starting
[PULL 0/3] Block patches
The following changes since commit 3e01f1147a16ca566694b97eafc941d62fa1e8d8: Merge tag 'pull-sp-20231105' of https://gitlab.com/rth7680/qemu into staging (2023-11-06 09:34:22 +0800) are available in the Git repository at: https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-11-06 for you to fetch changes up to ad4feaca61d76fecad784e6d5e7bae40d0411c46: file-posix: fix over-writing of returning zone_append offset (2023-11-06 16:15:07 +0100) Block patches: - One patch to make qcow2's discard-no-unref option do better what it is supposed to do (i.e. prevent fragmentation) - Two fixes for zoned requests Jean-Louis Dupond (1): qcow2: keep reference on zeroize with discard-no-unref enabled Naohiro Aota (1): file-posix: fix over-writing of returning zone_append offset Sam Li (1): block/file-posix: fix update_zones_wp() caller qapi/block-core.json | 24 ++-- block/file-posix.c| 23 --- block/qcow2-cluster.c | 22 ++ qemu-options.hx | 10 +++--- 4 files changed, 51 insertions(+), 28 deletions(-) -- 2.41.0
[PULL 2/3] block/file-posix: fix update_zones_wp() caller
From: Sam Li When the zoned request fail, it needs to update only the wp of the target zones for not disrupting the in-flight writes on these other zones. The wp is updated successfully after the request completes. Fixed the callers with right offset and nr_zones. Signed-off-by: Sam Li Message-Id: <20230825040556.4217-1-faithilike...@gmail.com> Reviewed-by: Stefan Hajnoczi [hreitz: Rebased and fixed comment spelling] Signed-off-by: Hanna Czenczek --- block/file-posix.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index 50e2b20d5c..3c0ce9c258 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -2523,7 +2523,10 @@ out: } } } else { -update_zones_wp(bs, s->fd, 0, 1); +/* + * write and append write are not allowed to cross zone boundaries + */ +update_zones_wp(bs, s->fd, offset, 1); } qemu_co_mutex_unlock(>colock); @@ -3470,7 +3473,7 @@ static int coroutine_fn raw_co_zone_mgmt(BlockDriverState *bs, BlockZoneOp op, len >> BDRV_SECTOR_BITS); ret = raw_thread_pool_submit(handle_aiocb_zone_mgmt, ); if (ret != 0) { -update_zones_wp(bs, s->fd, offset, i); +update_zones_wp(bs, s->fd, offset, nrz); error_report("ioctl %s failed %d", op_name, ret); return ret; } -- 2.41.0
[PULL 1/3] qcow2: keep reference on zeroize with discard-no-unref enabled
From: Jean-Louis Dupond When the discard-no-unref flag is enabled, we keep the reference for normal discard requests. But when a discard is executed on a snapshot/qcow2 image with backing, the discards are saved as zero clusters in the snapshot image. When committing the snapshot to the backing file, not discard_in_l2_slice is called but zero_in_l2_slice. Which did not had any logic to keep the reference when discard-no-unref is enabled. Therefor we add logic in the zero_in_l2_slice call to keep the reference on commit. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1621 Signed-off-by: Jean-Louis Dupond Message-Id: <20231003125236.216473-2-jean-lo...@dupond.be> [hreitz: Made the documentation change more verbose, as discussed on-list] Signed-off-by: Hanna Czenczek --- qapi/block-core.json | 24 ++-- block/qcow2-cluster.c | 22 ++ qemu-options.hx | 10 +++--- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 99961256f2..ca390c5700 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3528,16 +3528,20 @@ # @pass-discard-other: whether discard requests for the data source # should be issued on other occasions where a cluster gets freed # -# @discard-no-unref: when enabled, discards from the guest will not -# cause cluster allocations to be relinquished. This prevents -# qcow2 fragmentation that would be caused by such discards. -# Besides potential performance degradation, such fragmentation -# can lead to increased allocation of clusters past the end of the -# image file, resulting in image files whose file length can grow -# much larger than their guest disk size would suggest. If image -# file length is of concern (e.g. when storing qcow2 images -# directly on block devices), you should consider enabling this -# option. (since 8.1) +# @discard-no-unref: when enabled, data clusters will remain +# preallocated when they are no longer used, e.g. because they are +# discarded or converted to zero clusters. As usual, whether the +# old data is discarded or kept on the protocol level (i.e. in the +# image file) depends on the setting of the pass-discard-request +# option. Keeping the clusters preallocated prevents qcow2 +# fragmentation that would otherwise be caused by freeing and +# re-allocating them later. Besides potential performance +# degradation, such fragmentation can lead to increased allocation +# of clusters past the end of the image file, resulting in image +# files whose file length can grow much larger than their guest disk +# size would suggest. If image file length is of concern (e.g. when +# storing qcow2 images directly on block devices), you should +# consider enabling this option. (since 8.1) # # @overlap-check: which overlap checks to perform for writes to the # image, defaults to 'cached' (since 2.2) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 904f00d1b3..5af439bd11 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1983,7 +1983,7 @@ discard_in_l2_slice(BlockDriverState *bs, uint64_t offset, uint64_t nb_clusters, /* If we keep the reference, pass on the discard still */ bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK, s->cluster_size); - } +} } qcow2_cache_put(s->l2_table_cache, (void **) _slice); @@ -2061,9 +2061,15 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset, QCow2ClusterType type = qcow2_get_cluster_type(bs, old_l2_entry); bool unmap = (type == QCOW2_CLUSTER_COMPRESSED) || ((flags & BDRV_REQ_MAY_UNMAP) && qcow2_cluster_is_allocated(type)); -uint64_t new_l2_entry = unmap ? 0 : old_l2_entry; +bool keep_reference = +(s->discard_no_unref && type != QCOW2_CLUSTER_COMPRESSED); +uint64_t new_l2_entry = old_l2_entry; uint64_t new_l2_bitmap = old_l2_bitmap; +if (unmap && !keep_reference) { +new_l2_entry = 0; +} + if (has_subclusters(s)) { new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES; } else { @@ -2081,9 +2087,17 @@ zero_in_l2_slice(BlockDriverState *bs, uint64_t offset, set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap); } -/* Then decrease the refcount */ if (unmap) { -qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST); +if (!keep_reference) { +/* Then decrease the refcount */ +qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST); +} else if (s->discard_passthrough[QCOW2_DISCARD_REQUEST] && + (type == QCOW2_CLUSTER_NORMAL || +type == QCOW2_CLUSTER_ZERO_ALLOC)) { +/* If
[PULL 3/3] file-posix: fix over-writing of returning zone_append offset
From: Naohiro Aota raw_co_zone_append() sets "s->offset" where "BDRVRawState *s". This pointer is used later at raw_co_prw() to save the block address where the data is written. When multiple IOs are on-going at the same time, a later IO's raw_co_zone_append() call over-writes a former IO's offset address before raw_co_prw() completes. As a result, the former zone append IO returns the initial value (= the start address of the writing zone), instead of the proper address. Fix the issue by passing the offset pointer to raw_co_prw() instead of passing it through s->offset. Also, remove "offset" from BDRVRawState as there is no usage anymore. Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices") Signed-off-by: Naohiro Aota Message-Id: <20231030073853.2601162-1-naohiro.a...@wdc.com> Reviewed-by: Sam Li Reviewed-by: Stefan Hajnoczi Signed-off-by: Hanna Czenczek --- block/file-posix.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index 3c0ce9c258..b862406c71 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -160,7 +160,6 @@ typedef struct BDRVRawState { bool has_write_zeroes:1; bool use_linux_aio:1; bool use_linux_io_uring:1; -int64_t *offset; /* offset of zone append operation */ int page_cache_inconsistent; /* errno from fdatasync failure */ bool has_fallocate; bool needs_alignment; @@ -2445,12 +2444,13 @@ static bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov) return true; } -static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, +static int coroutine_fn raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr, uint64_t bytes, QEMUIOVector *qiov, int type) { BDRVRawState *s = bs->opaque; RawPosixAIOData acb; int ret; +uint64_t offset = *offset_ptr; if (fd_open(bs) < 0) return -EIO; @@ -2513,8 +2513,8 @@ out: uint64_t *wp = >wp[offset / bs->bl.zone_size]; if (!BDRV_ZT_IS_CONV(*wp)) { if (type & QEMU_AIO_ZONE_APPEND) { -*s->offset = *wp; -trace_zbd_zone_append_complete(bs, *s->offset +*offset_ptr = *wp; +trace_zbd_zone_append_complete(bs, *offset_ptr >> BDRV_SECTOR_BITS); } /* Advance the wp if needed */ @@ -2539,14 +2539,14 @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) { -return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ); +return raw_co_prw(bs, , bytes, qiov, QEMU_AIO_READ); } static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) { -return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE); +return raw_co_prw(bs, , bytes, qiov, QEMU_AIO_WRITE); } static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) @@ -3509,8 +3509,6 @@ static int coroutine_fn raw_co_zone_append(BlockDriverState *bs, int64_t zone_size_mask = bs->bl.zone_size - 1; int64_t iov_len = 0; int64_t len = 0; -BDRVRawState *s = bs->opaque; -s->offset = offset; if (*offset & zone_size_mask) { error_report("sector offset %" PRId64 " is not aligned to zone size " @@ -3531,7 +3529,7 @@ static int coroutine_fn raw_co_zone_append(BlockDriverState *bs, } trace_zbd_zone_append(bs, *offset >> BDRV_SECTOR_BITS); -return raw_co_prw(bs, *offset, len, qiov, QEMU_AIO_ZONE_APPEND); +return raw_co_prw(bs, offset, len, qiov, QEMU_AIO_ZONE_APPEND); } #endif -- 2.41.0
Re: [PATCH v2 3/3] hw/ide/via: implement legacy/native mode switching
On Mon, 6 Nov 2023, Kevin Wolf wrote: Am 25.10.2023 um 00:40 hat Mark Cave-Ayland geschrieben: Allow the VIA IDE controller to switch between both legacy and native modes by calling pci_ide_update_mode() to reconfigure the device whenever PCI_CLASS_PROG is updated. This patch moves the initial setting of PCI_CLASS_PROG from via_ide_realize() to via_ide_reset(), and removes the direct setting of PCI_INTERRUPT_PIN during PCI bus reset since this is now managed by pci_ide_update_mode(). This ensures that the device configuration is always consistent with respect to the currently selected mode. Signed-off-by: Mark Cave-Ayland Tested-by: BALATON Zoltan Tested-by: Bernhard Beschow As I already noted in patch 1, the interrupt handling seems to be wrong here, it continues to use the ISA IRQ in via_ide_set_irq() even after switching to native mode. That's a peculiarity of this via-ide device. It always uses 14/15 legacy interrupts even in native mode and guests expect that so using native interrupts would break pegasos2 guests. This was discussed and tested extensively before. Regards, BALATON Zoltan Other than this, the patch looks good to me. Kevin
Re: [PATCH v4 17/17] docs: update Xen-on-KVM documentation
On 06/11/2023 14:35, David Woodhouse wrote: From: David Woodhouse Add notes about console and network support, and how to launch PV guests. Clean up the disk configuration examples now that that's simpler, and remove the comment about IDE unplug on q35/AHCI now that it's fixed. Update the -initrd option documentation to explain how to quote commas in module command lines, and reference it when documenting PV guests. Also update stale avocado test filename in MAINTAINERS. Signed-off-by: David Woodhouse --- MAINTAINERS | 2 +- docs/system/i386/xen.rst | 107 +-- qemu-options.hx | 14 +++-- 3 files changed, 91 insertions(+), 32 deletions(-) Reviewed-by: Paul Durrant
Re: [PATCH v4 16/17] doc/sphinx/hxtool.py: add optional label argument to SRST directive
On 06/11/2023 14:35, David Woodhouse wrote: From: David Woodhouse We can't just embed labels directly into files like qemu-options.hx which are included from multiple top-level RST files, because Sphinx sees the labels as duplicate: https://github.com/sphinx-doc/sphinx/issues/9707 So add an 'emitrefs' option to the Sphinx hxtool-doc directive, which is set only in invocation.rst and not from the HTML rendition of the man page. Along with an argument to the SRST directive which causes a label of the form '.. _LABEL-reference-label:' to be emitted when the emitrefs option is set. Signed-off-by: David Woodhouse --- docs/sphinx/hxtool.py | 18 +- docs/system/invocation.rst | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) Reviewed-by: Paul Durrant
Re: [PATCH v4 13/17] hw/i386/pc: support '-nic' for xen-net-device
On 06/11/2023 14:35, David Woodhouse wrote: From: David Woodhouse The default NIC creation seems a bit hackish to me. I don't understand why each platform has to call pci_nic_init_nofail() from a point in the code where it actually has a pointer to the PCI bus, and then we have the special cases for things like ne2k_isa. If qmp_device_add() can *find* the appropriate bus and instantiate the device on it, why can't we just do that from generic code for creating the default NICs too? But that isn't a yak I want to shave today. Add a xenbus field to the PCMachineState so that it can make its way from pc_basic_device_init() to pc_nic_init() and be handled as a special case like ne2k_isa is. Now we can launch emulated Xen guests with '-nic user'. Signed-off-by: David Woodhouse --- hw/i386/pc.c | 11 --- hw/i386/pc_piix.c| 2 +- hw/i386/pc_q35.c | 2 +- hw/xen/xen-bus.c | 4 +++- include/hw/i386/pc.h | 4 +++- include/hw/xen/xen-bus.h | 2 +- 6 files changed, 17 insertions(+), 8 deletions(-) Reviewed-by: Paul Durrant
Re: [PATCH v4 06/17] hw/xen: automatically assign device index to block devices
On 06/11/2023 14:34, David Woodhouse wrote: From: David Woodhouse There's no need to force the user to assign a vdev. We can automatically assign one, starting at xvda and searching until we find the first disk name that's unused. This means we can now allow '-drive if=xen,file=xxx' to work without an explicit separate -driver argument, just like if=virtio. Rip out the legacy handling from the xenpv machine, which was scribbling over any disks configured by the toolstack, and didn't work with anything but raw images. Signed-off-by: David Woodhouse Acked-by: Kevin Wolf --- blockdev.c | 15 +++- hw/block/xen-block.c| 118 ++-- hw/xen/xen_devconfig.c | 28 --- hw/xenpv/xen_machine_pv.c | 9 --- include/hw/xen/xen-legacy-backend.h | 1 - 5 files changed, 125 insertions(+), 46 deletions(-) Reviewed-by: Paul Durrant
Re: [PATCH] file-posix: fix over-writing of returning zone_append offset
On 30.10.23 08:38, Naohiro Aota wrote: raw_co_zone_append() sets "s->offset" where "BDRVRawState *s". This pointer is used later at raw_co_prw() to save the block address where the data is written. When multiple IOs are on-going at the same time, a later IO's raw_co_zone_append() call over-writes a former IO's offset address before raw_co_prw() completes. As a result, the former zone append IO returns the initial value (= the start address of the writing zone), instead of the proper address. Fix the issue by passing the offset pointer to raw_co_prw() instead of passing it through s->offset. Also, remove "offset" from BDRVRawState as there is no usage anymore. Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices") Signed-off-by: Naohiro Aota --- block/file-posix.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) Thanks, applied to my block branch: https://gitlab.com/hreitz/qemu/-/commits/block Hanna
Re: [PATCH v2 09/10] block: Convert qmp_query_block() to coroutine_fn
On 09.06.23 22:19, Fabiano Rosas wrote: This is another caller of bdrv_get_allocated_file_size() that needs to be converted to a coroutine because that function will be made asynchronous when called (indirectly) from the QMP dispatcher. This QMP command is a candidate because it calls bdrv_do_query_node_info(), which in turn calls bdrv_get_allocated_file_size(). We've determined bdrv_do_query_node_info() to be coroutine-safe (see previous commits), so we can just put this QMP command in a coroutine. Since qmp_query_block() now expects to run in a coroutine, its callers need to be converted as well. Convert hmp_info_block(), which calls only coroutine-safe code, including qmp_query_named_block_nodes() which has been converted to coroutine in the previous patches. Now that all callers of bdrv_[co_]block_device_info() are using the coroutine version, a few things happen: - we can return to using bdrv_block_device_info() without a wrapper; - bdrv_get_allocated_file_size() can stop being mixed; - bdrv_co_get_allocated_file_size() needs to be put under the graph lock because it is being called wthout the wrapper; - bdrv_do_query_node_info() doesn't need to acquire the AioContext because it doesn't call aio_poll anymore; Signed-off-by: Fabiano Rosas --- block.c| 2 +- block/monitor/block-hmp-cmds.c | 2 +- block/qapi.c | 18 +- hmp-commands-info.hx | 1 + include/block/block-hmp-cmds.h | 2 +- include/block/block-io.h | 2 +- include/block/qapi.h | 12 qapi/block-core.json | 2 +- 8 files changed, 19 insertions(+), 22 deletions(-) After this series has been sent, we got some usages of GRAPH_RDLOCK_GUARD_MAINLOOP() that no longer fit with this patch – I’ve also mentioned one case on patch 7, not yet realizing that this was a new thing. Those must now be fixed (e.g. in qmp_query_block(), or in bdrv_snapshot_list()), or they’ll crash. Hanna
Re: [PATCH v2 10/10] block: Add a thread-pool version of fstat
On 09.06.23 22:19, Fabiano Rosas wrote: From: João Silva The fstat call can take a long time to finish when running over NFS. Add a version of it that runs in the thread pool. Adapt one of its users, raw_co_get_allocated_file size to use the new version. That function is called via QMP under the qemu_global_mutex so it has a large chance of blocking VCPU threads in case it takes too long to finish. Reviewed-by: Claudio Fontana Signed-off-by: João Silva Signed-off-by: Fabiano Rosas --- block/file-posix.c | 40 +--- include/block/raw-aio.h | 4 +++- 2 files changed, 40 insertions(+), 4 deletions(-) Reviewed-by: Hanna Czenczek
Re: [PATCH v2 05/10] block: Convert bdrv_query_block_graph_info to coroutine
On 09.06.23 22:19, Fabiano Rosas wrote: We're converting callers of bdrv_get_allocated_file_size() to run in coroutines because that function will be made asynchronous when called (indirectly) from the QMP dispatcher. This function is a candidate because it calls bdrv_do_query_node_info(), which in turn calls bdrv_get_allocated_file_size(). All the functions called from bdrv_do_query_node_info() onwards are coroutine-safe, either have a coroutine version themselves[1] or are mostly simple code/string manipulation[2]. 1) bdrv_getlength(), bdrv_get_allocated_file_size(), bdrv_get_info(), bdrv_get_specific_info(); 2) bdrv_refresh_filename(), bdrv_get_format_name(), bdrv_get_full_backing_filename(), bdrv_query_snapshot_info_list(); Signed-off-by: Fabiano Rosas --- block/qapi.c | 12 +++- include/block/qapi.h | 6 +- qemu-img.c | 2 -- 3 files changed, 12 insertions(+), 8 deletions(-) Reviewed-by: Hanna Czenczek
Re: [PATCH v2 04/10] block: Temporarily mark bdrv_co_get_allocated_file_size as mixed
On 09.06.23 22:19, Fabiano Rosas wrote: Some callers of this function are about to be converted to run in coroutines, so allow it to be executed both inside and outside a coroutine while we convert all the callers. This will be reverted once all callers of bdrv_do_query_node_info run in a coroutine. Signed-off-by: Fabiano Rosas Reviewed-by: Eric Blake --- include/block/block-io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Hanna Czenczek
Re: [PATCH v2 03/10] block: Allow the wrapper script to see functions declared in qapi.h
On 09.06.23 22:19, Fabiano Rosas wrote: The following patches will add co_wrapper annotations to functions declared in qapi.h. Add that header to the set of files used by block-coroutine-wrapper.py. Signed-off-by: Fabiano Rosas --- block/meson.build | 1 + scripts/block-coroutine-wrapper.py | 1 + 2 files changed, 2 insertions(+) Reviewed-by: Hanna Czenczek
Re: [PATCH v2 09/10] block: Convert qmp_query_block() to coroutine_fn
On 09.06.23 22:19, Fabiano Rosas wrote: This is another caller of bdrv_get_allocated_file_size() that needs to be converted to a coroutine because that function will be made asynchronous when called (indirectly) from the QMP dispatcher. This QMP command is a candidate because it calls bdrv_do_query_node_info(), which in turn calls bdrv_get_allocated_file_size(). We've determined bdrv_do_query_node_info() to be coroutine-safe (see previous commits), so we can just put this QMP command in a coroutine. Since qmp_query_block() now expects to run in a coroutine, its callers need to be converted as well. Convert hmp_info_block(), which calls only coroutine-safe code, including qmp_query_named_block_nodes() which has been converted to coroutine in the previous patches. Now that all callers of bdrv_[co_]block_device_info() are using the coroutine version, a few things happen: - we can return to using bdrv_block_device_info() without a wrapper; - bdrv_get_allocated_file_size() can stop being mixed; - bdrv_co_get_allocated_file_size() needs to be put under the graph lock because it is being called wthout the wrapper; But bdrv_do_query_node_info() is marked GRAPH_RDLOCK, so the whole function must not be called without holding the lock. I don’t understand why we need to explicitly take it another time. - bdrv_do_query_node_info() doesn't need to acquire the AioContext because it doesn't call aio_poll anymore; In the past (very likely outdated, but still mentioning it) you’d need to take the AioContext just in general when operating on a block device that might be processed in a different AioContext than the main one, and the current code runs in the main context, i.e. which is the situation we have here. Speaking of contexts, I wonder how the threading is actually supposed to work. I assume QMP coroutines run in the main thread, so now we run bdrv_co_get_allocated_file_size() in the main thread – is that correct, or do we need to use bdrv_co_enter() like qmp_block_resize() does? And so, if we run it in the main thread, is it OK not to acquire the AioContext around it to prevent interference from a potential I/O thread? Signed-off-by: Fabiano Rosas --- block.c| 2 +- block/monitor/block-hmp-cmds.c | 2 +- block/qapi.c | 18 +- hmp-commands-info.hx | 1 + include/block/block-hmp-cmds.h | 2 +- include/block/block-io.h | 2 +- include/block/qapi.h | 12 qapi/block-core.json | 2 +- 8 files changed, 19 insertions(+), 22 deletions(-) This patch implicitly assumes that quite a lot of functions (at least bdrv_query_info(), bdrv_query_image_info(), bdrv_do_query_node_info()) are now run in coroutine context. This assumption must be formalized by annotating them all with coroutine_fn, and ideally adding a _co_ into their name. Also, those functions should be checked whether they call coroutine wrappers, and made to use the native coroutine version now if so. (At least I’d find that nicer, FWIW.) I’ve seen at least bdrv_getlength() in bdrv_do_query_node_info(), which could be a bdrv_co_getlength(). diff --git a/block.c b/block.c index abed744b60..f94cee8930 100644 --- a/block.c +++ b/block.c @@ -6148,7 +6148,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(bool flat, list = NULL; QTAILQ_FOREACH(bs, _bdrv_states, node_list) { -BlockDeviceInfo *info = bdrv_co_block_device_info(NULL, bs, flat, errp); +BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, flat, errp); if (!info) { qapi_free_BlockDeviceInfoList(list); return NULL; diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index 26116fe831..1049f9b006 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -742,7 +742,7 @@ static void print_block_info(Monitor *mon, BlockInfo *info, } } -void hmp_info_block(Monitor *mon, const QDict *qdict) +void coroutine_fn hmp_info_block(Monitor *mon, const QDict *qdict) { BlockInfoList *block_list, *info; BlockDeviceInfoList *blockdev_list, *blockdev; diff --git a/block/qapi.c b/block/qapi.c index 20660e15d6..3b4bc0b782 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -41,10 +41,10 @@ #include "qemu/qemu-print.h" #include "sysemu/block-backend.h" -BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk, -BlockDriverState *bs, -bool flat, -Error **errp) +BlockDeviceInfo *coroutine_fn bdrv_block_device_info(BlockBackend *blk, + BlockDriverState *bs, + bool flat, +
[PATCH v4 15/17] xen-platform: unplug AHCI disks
From: David Woodhouse To support Xen guests using the Q35 chipset, the unplug protocol needs to also remove AHCI disks. Make pci_xen_ide_unplug() more generic, iterating over the children of the PCI device and destroying the "ide-hd" devices. That works the same for both AHCI and IDE, as does the detection of the primary disk as unit 0 on the bus named "ide.0". Then pci_xen_ide_unplug() can be used for both AHCI and IDE devices. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/i386/xen/xen_platform.c | 68 +- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c index e2dd1b536a..ef7d3fc05f 100644 --- a/hw/i386/xen/xen_platform.c +++ b/hw/i386/xen/xen_platform.c @@ -169,39 +169,60 @@ static void pci_unplug_nics(PCIBus *bus) * * [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc */ -static void pci_xen_ide_unplug(PCIDevice *d, bool aux) +struct ide_unplug_state { +bool aux; +int nr_unplugged; +}; + +static int ide_dev_unplug(DeviceState *dev, void *_st) { -DeviceState *dev = DEVICE(d); -PCIIDEState *pci_ide; -int i; +struct ide_unplug_state *st = _st; IDEDevice *idedev; IDEBus *idebus; BlockBackend *blk; +int unit; + +idedev = IDE_DEVICE(object_dynamic_cast(OBJECT(dev), "ide-hd")); +if (!idedev) { +return 0; +} -pci_ide = PCI_IDE(dev); +idebus = IDE_BUS(qdev_get_parent_bus(dev)); -for (i = aux ? 1 : 0; i < 4; i++) { -idebus = _ide->bus[i / 2]; -blk = idebus->ifs[i % 2].blk; +unit = (idedev == idebus->slave); +assert(unit || idedev == idebus->master); -if (blk && idebus->ifs[i % 2].drive_kind != IDE_CD) { -if (!(i % 2)) { -idedev = idebus->master; -} else { -idedev = idebus->slave; -} +if (st->aux && !unit && !strcmp(BUS(idebus)->name, "ide.0")) { +return 0; +} -blk_drain(blk); -blk_flush(blk); +blk = idebus->ifs[unit].blk; +if (blk) { +blk_drain(blk); +blk_flush(blk); -blk_detach_dev(blk, DEVICE(idedev)); -idebus->ifs[i % 2].blk = NULL; -idedev->conf.blk = NULL; -monitor_remove_blk(blk); -blk_unref(blk); -} +blk_detach_dev(blk, DEVICE(idedev)); +idebus->ifs[unit].blk = NULL; +idedev->conf.blk = NULL; +monitor_remove_blk(blk); +blk_unref(blk); +} + +object_unparent(OBJECT(dev)); +st->nr_unplugged++; + +return 0; +} + +static void pci_xen_ide_unplug(PCIDevice *d, bool aux) +{ +struct ide_unplug_state st = { aux, 0 }; +DeviceState *dev = DEVICE(d); + +qdev_walk_children(dev, NULL, NULL, ide_dev_unplug, NULL, ); +if (st.nr_unplugged) { +pci_device_reset(d); } -pci_device_reset(d); } static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque) @@ -216,6 +237,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque) switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) { case PCI_CLASS_STORAGE_IDE: +case PCI_CLASS_STORAGE_SATA: pci_xen_ide_unplug(d, aux); break; -- 2.34.1
[PATCH v4 07/17] hw/xen: add get_frontend_path() method to XenDeviceClass
From: David Woodhouse The primary Xen console is special. The guest's side is set up for it by the toolstack automatically and not by the standard PV init sequence. Accordingly, its *frontend* doesn't appear in …/device/console/0 either; instead it appears under …/console in the guest's XenStore node. To allow the Xen console driver to override the frontend path for the primary console, add a method to the XenDeviceClass which can be used instead of the standard xen_device_get_frontend_path() Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/xen/xen-bus.c | 11 ++- include/hw/xen/xen-bus.h | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c index ece8ec40cd..12ff782005 100644 --- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -711,8 +711,17 @@ static void xen_device_frontend_create(XenDevice *xendev, Error **errp) { ERRP_GUARD(); XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev))); +XenDeviceClass *xendev_class = XEN_DEVICE_GET_CLASS(xendev); -xendev->frontend_path = xen_device_get_frontend_path(xendev); +if (xendev_class->get_frontend_path) { +xendev->frontend_path = xendev_class->get_frontend_path(xendev, errp); +if (!xendev->frontend_path) { +error_prepend(errp, "failed to create frontend: "); +return; +} +} else { +xendev->frontend_path = xen_device_get_frontend_path(xendev); +} /* * The frontend area may have already been created by a legacy diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h index f435898164..eb440880b5 100644 --- a/include/hw/xen/xen-bus.h +++ b/include/hw/xen/xen-bus.h @@ -33,6 +33,7 @@ struct XenDevice { }; typedef struct XenDevice XenDevice; +typedef char *(*XenDeviceGetFrontendPath)(XenDevice *xendev, Error **errp); typedef char *(*XenDeviceGetName)(XenDevice *xendev, Error **errp); typedef void (*XenDeviceRealize)(XenDevice *xendev, Error **errp); typedef void (*XenDeviceFrontendChanged)(XenDevice *xendev, @@ -46,6 +47,7 @@ struct XenDeviceClass { /*< public >*/ const char *backend; const char *device; +XenDeviceGetFrontendPath get_frontend_path; XenDeviceGetName get_name; XenDeviceRealize realize; XenDeviceFrontendChanged frontend_changed; -- 2.34.1
Re: [PATCH v2 3/3] hw/ide/via: implement legacy/native mode switching
Am 25.10.2023 um 00:40 hat Mark Cave-Ayland geschrieben: > Allow the VIA IDE controller to switch between both legacy and native modes by > calling pci_ide_update_mode() to reconfigure the device whenever > PCI_CLASS_PROG > is updated. > > This patch moves the initial setting of PCI_CLASS_PROG from via_ide_realize() > to > via_ide_reset(), and removes the direct setting of PCI_INTERRUPT_PIN during > PCI > bus reset since this is now managed by pci_ide_update_mode(). This ensures > that > the device configuration is always consistent with respect to the currently > selected mode. > > Signed-off-by: Mark Cave-Ayland > Tested-by: BALATON Zoltan > Tested-by: Bernhard Beschow As I already noted in patch 1, the interrupt handling seems to be wrong here, it continues to use the ISA IRQ in via_ide_set_irq() even after switching to native mode. Other than this, the patch looks good to me. Kevin
[PATCH v4 17/17] docs: update Xen-on-KVM documentation
From: David Woodhouse Add notes about console and network support, and how to launch PV guests. Clean up the disk configuration examples now that that's simpler, and remove the comment about IDE unplug on q35/AHCI now that it's fixed. Update the -initrd option documentation to explain how to quote commas in module command lines, and reference it when documenting PV guests. Also update stale avocado test filename in MAINTAINERS. Signed-off-by: David Woodhouse --- MAINTAINERS | 2 +- docs/system/i386/xen.rst | 107 +-- qemu-options.hx | 14 +++-- 3 files changed, 91 insertions(+), 32 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 8e8a7d5be5..3252be2696 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -490,7 +490,7 @@ S: Supported F: include/sysemu/kvm_xen.h F: target/i386/kvm/xen* F: hw/i386/kvm/xen* -F: tests/avocado/xen_guest.py +F: tests/avocado/kvm_xen_guest.py Guest CPU Cores (other accelerators) diff --git a/docs/system/i386/xen.rst b/docs/system/i386/xen.rst index f06765e88c..f3bd90d4d6 100644 --- a/docs/system/i386/xen.rst +++ b/docs/system/i386/xen.rst @@ -15,46 +15,24 @@ Setup - Xen mode is enabled by setting the ``xen-version`` property of the KVM -accelerator, for example for Xen 4.10: +accelerator, for example for Xen 4.17: .. parsed-literal:: - |qemu_system| --accel kvm,xen-version=0x4000a,kernel-irqchip=split + |qemu_system| --accel kvm,xen-version=0x40011,kernel-irqchip=split Additionally, virtual APIC support can be advertised to the guest through the ``xen-vapic`` CPU flag: .. parsed-literal:: - |qemu_system| --accel kvm,xen-version=0x4000a,kernel-irqchip=split --cpu host,+xen_vapic + |qemu_system| --accel kvm,xen-version=0x40011,kernel-irqchip=split --cpu host,+xen-vapic When Xen support is enabled, QEMU changes hypervisor identification (CPUID 0x4000..0x400A) to Xen. The KVM identification and features are not advertised to a Xen guest. If Hyper-V is also enabled, the Xen identification moves to leaves 0x4100..0x410A. -The Xen platform device is enabled automatically for a Xen guest. This allows -a guest to unplug all emulated devices, in order to use Xen PV block and network -drivers instead. Under Xen, the boot disk is typically available both via IDE -emulation, and as a PV block device. Guest bootloaders typically use IDE to load -the guest kernel, which then unplugs the IDE and continues with the Xen PV block -device. - -This configuration can be achieved as follows - -.. parsed-literal:: - - |qemu_system| -M pc --accel kvm,xen-version=0x4000a,kernel-irqchip=split \\ - -drive file=${GUEST_IMAGE},if=none,id=disk,file.locking=off -device xen-disk,drive=disk,vdev=xvda \\ - -drive file=${GUEST_IMAGE},index=2,media=disk,file.locking=off,if=ide - -It is necessary to use the pc machine type, as the q35 machine uses AHCI instead -of legacy IDE, and AHCI disks are not unplugged through the Xen PV unplug -mechanism. - -VirtIO devices can also be used; Linux guests may need to be dissuaded from -umplugging them by adding 'xen_emul_unplug=never' on their command line. - Properties -- @@ -63,7 +41,10 @@ The following properties exist on the KVM accelerator object: ``xen-version`` This property contains the Xen version in ``XENVER_version`` form, with the major version in the top 16 bits and the minor version in the low 16 bits. - Setting this property enables the Xen guest support. + Setting this property enables the Xen guest support. If Xen version 4.5 or + greater is specified, the HVM leaf in Xen CPUID is populated. Xen version + 4.6 enables the vCPU ID in CPUID, and version 4.17 advertises vCPU upcall + vector support to the guest. ``xen-evtchn-max-pirq`` Xen PIRQs represent an emulated physical interrupt, either GSI or MSI, which @@ -83,8 +64,78 @@ The following properties exist on the KVM accelerator object: through simultaneous grants. For guests with large numbers of PV devices and high throughput, it may be desirable to increase this value. -OS requirements +Xen paravirtual devices +--- + +The Xen PCI platform device is enabled automatically for a Xen guest. This +allows a guest to unplug all emulated devices, in order to use paravirtual +block and network drivers instead. + +Those paravirtual Xen block, network (and console) devices can be created +through the command line, and/or hot-plugged. + +To provide a Xen console device, define a character device and then a device +of type ``xen-console`` to connect to it. For the Xen console equivalent of +the handy ``-serial mon:stdio`` option, for example: + +.. parsed-literal:: + -chardev stdio,mux=on,id=char0,signal=off -mon char0 \\ + -device xen-console,chardev=char0 + +The Xen network device is ``xen-net-device``, which becomes the default NIC +model for emulated Xen
[PATCH v4 08/17] hw/xen: do not repeatedly try to create a failing backend device
From: David Woodhouse If xen_backend_device_create() fails to instantiate a device, the XenBus code will just keep trying over and over again each time the bus is re-enumerated, as long as the backend appears online and in XenbusStateInitialising. The only thing which prevents the XenBus code from recreating duplicates of devices which already exist, is the fact that xen_device_realize() sets the backend state to XenbusStateInitWait. If the attempt to create the device doesn't get *that* far, that's when it will keep getting retried. My first thought was to handle errors by setting the backend state to XenbusStateClosed, but that doesn't work for XenConsole which wants to *ignore* any device of type != "ioemu" completely. So, make xen_backend_device_create() *keep* the XenBackendInstance for a failed device, and provide a new xen_backend_exists() function to allow xen_bus_type_enumerate() to check whether one already exists before creating a new one. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/xen/xen-backend.c | 27 +-- hw/xen/xen-bus.c | 3 ++- include/hw/xen/xen-backend.h | 1 + 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/hw/xen/xen-backend.c b/hw/xen/xen-backend.c index 5b0fb76eae..b9bf70a9f5 100644 --- a/hw/xen/xen-backend.c +++ b/hw/xen/xen-backend.c @@ -101,6 +101,24 @@ static XenBackendInstance *xen_backend_list_find(XenDevice *xendev) return NULL; } +bool xen_backend_exists(const char *type, const char *name) +{ +const XenBackendImpl *impl = xen_backend_table_lookup(type); +XenBackendInstance *backend; + +if (!impl) { +return false; +} + +QLIST_FOREACH(backend, _list, entry) { +if (backend->impl == impl && !strcmp(backend->name, name)) { +return true; +} +} + +return false; +} + static void xen_backend_list_remove(XenBackendInstance *backend) { QLIST_REMOVE(backend, entry); @@ -122,11 +140,6 @@ void xen_backend_device_create(XenBus *xenbus, const char *type, backend->name = g_strdup(name); impl->create(backend, opts, errp); -if (*errp) { -g_free(backend->name); -g_free(backend); -return; -} backend->impl = impl; xen_backend_list_add(backend); @@ -165,7 +178,9 @@ bool xen_backend_try_device_destroy(XenDevice *xendev, Error **errp) } impl = backend->impl; -impl->destroy(backend, errp); +if (backend->xendev) { +impl->destroy(backend, errp); +} xen_backend_list_remove(backend); g_free(backend->name); diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c index 12ff782005..3ffd1a5333 100644 --- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -209,7 +209,8 @@ static void xen_bus_type_enumerate(XenBus *xenbus, const char *type) NULL, "%u", ) != 1) online = 0; -if (online && state == XenbusStateInitialising) { +if (online && state == XenbusStateInitialising && +!xen_backend_exists(type, backend[i])) { Error *local_err = NULL; xen_bus_backend_create(xenbus, type, backend[i], backend_path, diff --git a/include/hw/xen/xen-backend.h b/include/hw/xen/xen-backend.h index aac2fd454d..0f01631ae7 100644 --- a/include/hw/xen/xen-backend.h +++ b/include/hw/xen/xen-backend.h @@ -33,6 +33,7 @@ XenDevice *xen_backend_get_device(XenBackendInstance *backend); void xen_backend_register(const XenBackendInfo *info); const char **xen_backend_get_types(unsigned int *nr); +bool xen_backend_exists(const char *type, const char *name); void xen_backend_device_create(XenBus *xenbus, const char *type, const char *name, QDict *opts, Error **errp); bool xen_backend_try_device_destroy(XenDevice *xendev, Error **errp); -- 2.34.1
[PATCH v4 14/17] net: do not delete nics in net_cleanup()
From: David Woodhouse In net_cleanup() we only need to delete the netdevs, as those may have state which outlives Qemu when it exits, and thus may actually need to be cleaned up on exit. The nics, on the other hand, are owned by the device which created them. Most devices don't bother to clean up on exit because they don't have any state which will outlive Qemu... but XenBus devices do need to clean up their nodes in XenStore, and do have an exit handler to delete them. When the XenBus exit handler destroys the xen-net-device, it attempts to delete its nic after net_cleanup() had already done so. And crashes. Fix this by only deleting netdevs as we walk the list. As the comment notes, we can't use QTAILQ_FOREACH_SAFE() as each deletion may remove *multiple* entries, including the "safely" saved 'next' pointer. But we can store the *previous* entry, since nics are safe. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- net/net.c | 28 ++-- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/net/net.c b/net/net.c index c0c0cbe99e..bbe33da176 100644 --- a/net/net.c +++ b/net/net.c @@ -1499,18 +1499,34 @@ static void net_vm_change_state_handler(void *opaque, bool running, void net_cleanup(void) { -NetClientState *nc; +NetClientState *nc, **p = _FIRST(_clients); /*cleanup colo compare module for COLO*/ colo_compare_cleanup(); -/* We may del multiple entries during qemu_del_net_client(), - * so QTAILQ_FOREACH_SAFE() is also not safe here. +/* + * Walk the net_clients list and remove the netdevs but *not* any + * NET_CLIENT_DRIVER_NIC entries. The latter are owned by the device + * model which created them, and in some cases (e.g. xen-net-device) + * the device itself may do cleanup at exit and will be upset if we + * just delete its NIC from underneath it. + * + * Since qemu_del_net_client() may delete multiple entries, using + * QTAILQ_FOREACH_SAFE() is not safe here. The only safe pointer + * to keep as a bookmark is a NET_CLIENT_DRIVER_NIC entry, so keep + * 'p' pointing to either the head of the list, or the 'next' field + * of the latest NET_CLIENT_DRIVER_NIC, and operate on *p as we walk + * the list. + * + * The 'nc' variable isn't part of the list traversal; it's purely + * for convenience as too much '(*p)->' has a tendency to make the + * readers' eyes bleed. */ -while (!QTAILQ_EMPTY(_clients)) { -nc = QTAILQ_FIRST(_clients); +while (*p) { +nc = *p; if (nc->info->type == NET_CLIENT_DRIVER_NIC) { -qemu_del_nic(qemu_get_nic(nc)); +/* Skip NET_CLIENT_DRIVER_NIC entries */ +p = _NEXT(nc, next); } else { qemu_del_net_client(nc); } -- 2.34.1
[PATCH v4 16/17] doc/sphinx/hxtool.py: add optional label argument to SRST directive
From: David Woodhouse We can't just embed labels directly into files like qemu-options.hx which are included from multiple top-level RST files, because Sphinx sees the labels as duplicate: https://github.com/sphinx-doc/sphinx/issues/9707 So add an 'emitrefs' option to the Sphinx hxtool-doc directive, which is set only in invocation.rst and not from the HTML rendition of the man page. Along with an argument to the SRST directive which causes a label of the form '.. _LABEL-reference-label:' to be emitted when the emitrefs option is set. Signed-off-by: David Woodhouse --- docs/sphinx/hxtool.py | 18 +- docs/system/invocation.rst | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/sphinx/hxtool.py b/docs/sphinx/hxtool.py index 9f6b9d87dc..bfb0929573 100644 --- a/docs/sphinx/hxtool.py +++ b/docs/sphinx/hxtool.py @@ -78,18 +78,28 @@ def parse_archheading(file, lnum, line): serror(file, lnum, "Invalid ARCHHEADING line") return match.group(1) +def parse_srst(file, lnum, line): +"""Handle an SRST directive""" +# The input should be "SRST(label)". +match = re.match(r'SRST\((.*?)\)', line) +if match is None: +serror(file, lnum, "Invalid SRST line") +return match.group(1) + class HxtoolDocDirective(Directive): """Extract rST fragments from the specified .hx file""" required_argument = 1 optional_arguments = 1 option_spec = { -'hxfile': directives.unchanged_required +'hxfile': directives.unchanged_required, +'emitrefs': directives.flag } has_content = False def run(self): env = self.state.document.settings.env hxfile = env.config.hxtool_srctree + '/' + self.arguments[0] +emitrefs = "emitrefs" in self.options # Tell sphinx of the dependency env.note_dependency(os.path.abspath(hxfile)) @@ -113,6 +123,12 @@ def run(self): serror(hxfile, lnum, 'expected ERST, found SRST') else: state = HxState.RST +if emitrefs and line != "SRST": +label = parse_srst(hxfile, lnum, line) +if label != "": +rstlist.append("", hxfile, lnum - 1) +refline = ".. _" + label + "-reference-label:" +rstlist.append(refline, hxfile, lnum - 1) elif directive == 'ERST': if state == HxState.CTEXT: serror(hxfile, lnum, 'expected SRST, found ERST') diff --git a/docs/system/invocation.rst b/docs/system/invocation.rst index 4ba38fc23d..ef75dad2e2 100644 --- a/docs/system/invocation.rst +++ b/docs/system/invocation.rst @@ -11,6 +11,7 @@ disk_image is a raw hard disk image for IDE hard disk 0. Some targets do not need a disk image. .. hxtool-doc:: qemu-options.hx +:emitrefs: Device URL Syntax ~ -- 2.34.1
[PATCH v4 12/17] hw/xen: update Xen PV NIC to XenDevice model
From: David Woodhouse This allows us to use Xen PV networking with emulated Xen guests, and to add them on the command line or hotplug. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/net/meson.build| 2 +- hw/net/trace-events | 11 + hw/net/xen_nic.c | 484 +- hw/xenpv/xen_machine_pv.c | 1 - 4 files changed, 381 insertions(+), 117 deletions(-) diff --git a/hw/net/meson.build b/hw/net/meson.build index 2632634df3..f64651c467 100644 --- a/hw/net/meson.build +++ b/hw/net/meson.build @@ -1,5 +1,5 @@ system_ss.add(when: 'CONFIG_DP8393X', if_true: files('dp8393x.c')) -system_ss.add(when: 'CONFIG_XEN', if_true: files('xen_nic.c')) +system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen_nic.c')) system_ss.add(when: 'CONFIG_NE2000_COMMON', if_true: files('ne2000.c')) # PCI network cards diff --git a/hw/net/trace-events b/hw/net/trace-events index 3abfd65e5b..3097742cc0 100644 --- a/hw/net/trace-events +++ b/hw/net/trace-events @@ -482,3 +482,14 @@ dp8393x_receive_oversize(int size) "oversize packet, pkt_size is %d" dp8393x_receive_not_netcard(void) "packet not for netcard" dp8393x_receive_packet(int crba) "Receive packet at 0x%"PRIx32 dp8393x_receive_write_status(int crba) "Write status at 0x%"PRIx32 + +# xen_nic.c +xen_netdev_realize(int dev, const char *info, const char *peer) "vif%u info '%s' peer '%s'" +xen_netdev_unrealize(int dev) "vif%u" +xen_netdev_create(int dev) "vif%u" +xen_netdev_destroy(int dev) "vif%u" +xen_netdev_disconnect(int dev) "vif%u" +xen_netdev_connect(int dev, unsigned int tx, unsigned int rx, int port) "vif%u tx %u rx %u port %u" +xen_netdev_frontend_changed(const char *dev, int state) "vif%s state %d" +xen_netdev_tx(int dev, int ref, int off, int len, unsigned int flags, const char *c, const char *d, const char *m, const char *e) "vif%u ref %u off %u len %u flags 0x%x%s%s%s%s" +xen_netdev_rx(int dev, int idx, int status, int flags) "vif%u idx %d status %d flags 0x%x" diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c index 9bbf6599fc..af4ba3f1e6 100644 --- a/hw/net/xen_nic.c +++ b/hw/net/xen_nic.c @@ -20,6 +20,13 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" +#include "qemu/cutils.h" +#include "qemu/log.h" +#include "qemu/qemu-print.h" +#include "qapi/qmp/qdict.h" +#include "qapi/error.h" + #include #include #include @@ -27,18 +34,26 @@ #include "net/net.h" #include "net/checksum.h" #include "net/util.h" -#include "hw/xen/xen-legacy-backend.h" + +#include "hw/xen/xen-backend.h" +#include "hw/xen/xen-bus-helper.h" +#include "hw/qdev-properties.h" +#include "hw/qdev-properties-system.h" #include "hw/xen/interface/io/netif.h" +#include "hw/xen/interface/io/xs_wire.h" + +#include "trace.h" /* - */ struct XenNetDev { -struct XenLegacyDevice xendev; /* must be first */ -char *mac; +struct XenDevice xendev; /* must be first */ +XenEventChannel *event_channel; +int dev; int tx_work; -int tx_ring_ref; -int rx_ring_ref; +unsigned int tx_ring_ref; +unsigned int rx_ring_ref; struct netif_tx_sring *txs; struct netif_rx_sring *rxs; netif_tx_back_ring_t tx_ring; @@ -47,6 +62,11 @@ struct XenNetDev { NICState *nic; }; +typedef struct XenNetDev XenNetDev; + +#define TYPE_XEN_NET_DEVICE "xen-net-device" +OBJECT_DECLARE_SIMPLE_TYPE(XenNetDev, XEN_NET_DEVICE) + /* - */ static void net_tx_response(struct XenNetDev *netdev, netif_tx_request_t *txp, int8_t st) @@ -68,7 +88,8 @@ static void net_tx_response(struct XenNetDev *netdev, netif_tx_request_t *txp, i netdev->tx_ring.rsp_prod_pvt = ++i; RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(>tx_ring, notify); if (notify) { -xen_pv_send_notify(>xendev); +xen_device_notify_event_channel(XEN_DEVICE(netdev), +netdev->event_channel, NULL); } if (i == netdev->tx_ring.req_cons) { @@ -104,13 +125,16 @@ static void net_tx_error(struct XenNetDev *netdev, netif_tx_request_t *txp, RING #endif } -static void net_tx_packets(struct XenNetDev *netdev) +static bool net_tx_packets(struct XenNetDev *netdev) { +bool done_something = false; netif_tx_request_t txreq; RING_IDX rc, rp; void *page; void *tmpbuf = NULL; +assert(qemu_mutex_iothread_locked()); + for (;;) { rc = netdev->tx_ring.req_cons; rp = netdev->tx_ring.sring->req_prod; @@ -122,49 +146,52 @@ static void net_tx_packets(struct XenNetDev *netdev) } memcpy(, RING_GET_REQUEST(>tx_ring, rc), sizeof(txreq)); netdev->tx_ring.req_cons = ++rc; +done_something = true; #if 1
[PATCH v4 05/17] hw/xen: populate store frontend nodes with XenStore PFN/port
From: David Woodhouse This is kind of redundant since without being able to get these through some other method (HVMOP_get_param) the guest wouldn't be able to access XenStore in order to find them. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/i386/kvm/xen_xenstore.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c index 831da535fc..b7c0407765 100644 --- a/hw/i386/kvm/xen_xenstore.c +++ b/hw/i386/kvm/xen_xenstore.c @@ -1434,6 +1434,7 @@ static void alloc_guest_port(XenXenstoreState *s) int xen_xenstore_reset(void) { XenXenstoreState *s = xen_xenstore_singleton; +GList *perms; int err; if (!s) { @@ -1461,6 +1462,16 @@ int xen_xenstore_reset(void) } s->be_port = err; +/* Create frontend store nodes */ +perms = g_list_append(NULL, xs_perm_as_string(XS_PERM_NONE, DOMID_QEMU)); +perms = g_list_append(perms, xs_perm_as_string(XS_PERM_READ, xen_domid)); + +relpath_printf(s, perms, "store/port", "%u", s->guest_port); +relpath_printf(s, perms, "store/ring-ref", "%lu", + XEN_SPECIAL_PFN(XENSTORE)); + +g_list_free_full(perms, g_free); + /* * We don't actually access the guest's page through the grant, because * this isn't real Xen, and we can just use the page we gave it in the -- 2.34.1
[PATCH v4 02/17] hw/xen: Clean up event channel 'type_val' handling to use union
From: David Woodhouse A previous implementation of this stuff used a 64-bit field for all of the port information (vcpu/type/type_val) and did atomic exchanges on them. When I implemented that in Qemu I regretted my life choices and just kept it simple with locking instead. So there's no need for the XenEvtchnPort to be so simplistic. We can use a union for the pirq/virq/interdomain information, which lets us keep a separate bit for the 'remote domain' in interdomain ports. A single bit is enough since the only possible targets are loopback or qemu itself. So now we can ditch PORT_INFO_TYPEVAL_REMOTE_QEMU and the horrid manual masking, although the in-memory representation is identical so there's no change in the saved state ABI. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/i386/kvm/xen_evtchn.c | 151 ++- 1 file changed, 70 insertions(+), 81 deletions(-) diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index b2b4be9983..02b8cbf8df 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -58,7 +58,15 @@ OBJECT_DECLARE_SIMPLE_TYPE(XenEvtchnState, XEN_EVTCHN) typedef struct XenEvtchnPort { uint32_t vcpu; /* Xen/ACPI vcpu_id */ uint16_t type; /* EVTCHNSTAT_ */ -uint16_t type_val; /* pirq# / virq# / remote port according to type */ +union { +uint16_t val; /* raw value for serialization etc. */ +uint16_t pirq; +uint16_t virq; +struct { +uint16_t port:15; +uint16_t to_qemu:1; /* Only two targets; qemu or loopback */ +} interdomain; +} u; } XenEvtchnPort; /* 32-bit compatibility definitions, also used natively in 32-bit build */ @@ -105,14 +113,6 @@ struct xenevtchn_handle { int fd; }; -/* - * For unbound/interdomain ports there are only two possible remote - * domains; self and QEMU. Use a single high bit in type_val for that, - * and the low bits for the remote port number (or 0 for unbound). - */ -#define PORT_INFO_TYPEVAL_REMOTE_QEMU 0x8000 -#define PORT_INFO_TYPEVAL_REMOTE_PORT_MASK 0x7FFF - /* * These 'emuirq' values are used by Xen in the LM stream... and yes, I am * insane enough to think about guest-transparent live migration from actual @@ -210,16 +210,16 @@ static int xen_evtchn_post_load(void *opaque, int version_id) XenEvtchnPort *p = >port_table[i]; if (p->type == EVTCHNSTAT_pirq) { -assert(p->type_val); -assert(p->type_val < s->nr_pirqs); +assert(p->u.pirq); +assert(p->u.pirq < s->nr_pirqs); /* * Set the gsi to IRQ_UNBOUND; it may be changed to an actual * GSI# below, or to IRQ_MSI_EMU when the MSI table snooping * catches up with it. */ -s->pirq[p->type_val].gsi = IRQ_UNBOUND; -s->pirq[p->type_val].port = i; +s->pirq[p->u.pirq].gsi = IRQ_UNBOUND; +s->pirq[p->u.pirq].port = i; } } /* Rebuild s->pirq[].gsi mapping */ @@ -243,7 +243,7 @@ static const VMStateDescription xen_evtchn_port_vmstate = { .fields = (VMStateField[]) { VMSTATE_UINT32(vcpu, XenEvtchnPort), VMSTATE_UINT16(type, XenEvtchnPort), -VMSTATE_UINT16(type_val, XenEvtchnPort), +VMSTATE_UINT16(u.val, XenEvtchnPort), VMSTATE_END_OF_LIST() } }; @@ -605,14 +605,13 @@ static void unbind_backend_ports(XenEvtchnState *s) for (i = 1; i < s->nr_ports; i++) { p = >port_table[i]; -if (p->type == EVTCHNSTAT_interdomain && -(p->type_val & PORT_INFO_TYPEVAL_REMOTE_QEMU)) { -evtchn_port_t be_port = p->type_val & PORT_INFO_TYPEVAL_REMOTE_PORT_MASK; +if (p->type == EVTCHNSTAT_interdomain && p->u.interdomain.to_qemu) { +evtchn_port_t be_port = p->u.interdomain.port; if (s->be_handles[be_port]) { /* This part will be overwritten on the load anyway. */ p->type = EVTCHNSTAT_unbound; -p->type_val = PORT_INFO_TYPEVAL_REMOTE_QEMU; +p->u.interdomain.port = 0; /* Leave the backend port open and unbound too. */ if (kvm_xen_has_cap(EVTCHN_SEND)) { @@ -650,30 +649,22 @@ int xen_evtchn_status_op(struct evtchn_status *status) switch (p->type) { case EVTCHNSTAT_unbound: -if (p->type_val & PORT_INFO_TYPEVAL_REMOTE_QEMU) { -status->u.unbound.dom = DOMID_QEMU; -} else { -status->u.unbound.dom = xen_domid; -} +status->u.unbound.dom = p->u.interdomain.to_qemu ? DOMID_QEMU + : xen_domid; break; case EVTCHNSTAT_interdomain: -if (p->type_val & PORT_INFO_TYPEVAL_REMOTE_QEMU) { -status->u.interdomain.dom = DOMID_QEMU; -} else { -
[PATCH v4 11/17] hw/xen: only remove peers of PCI NICs on unplug
From: David Woodhouse When the Xen guest asks to unplug *emulated* NICs, it's kind of unhelpful also to unplug the peer of the *Xen* PV NIC. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/i386/xen/xen_platform.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c index 17457ff3de..e2dd1b536a 100644 --- a/hw/i386/xen/xen_platform.c +++ b/hw/i386/xen/xen_platform.c @@ -140,9 +140,14 @@ static void unplug_nic(PCIBus *b, PCIDevice *d, void *o) /* Remove the peer of the NIC device. Normally, this would be a tap device. */ static void del_nic_peer(NICState *nic, void *opaque) { -NetClientState *nc; +NetClientState *nc = qemu_get_queue(nic); +ObjectClass *klass = module_object_class_by_name(nc->model); + +/* Only delete peers of PCI NICs that we're about to delete */ +if (!klass || !object_class_dynamic_cast(klass, TYPE_PCI_DEVICE)) { +return; +} -nc = qemu_get_queue(nic); if (nc->peer) qemu_del_net_client(nc->peer); } -- 2.34.1
[PATCH v4 09/17] hw/xen: update Xen console to XenDevice model
From: David Woodhouse This allows (non-primary) console devices to be created on the command line and hotplugged. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/char/trace-events| 8 + hw/char/xen_console.c | 532 +++- hw/xen/xen-legacy-backend.c | 1 - 3 files changed, 411 insertions(+), 130 deletions(-) diff --git a/hw/char/trace-events b/hw/char/trace-events index babf4d35ea..7a398c82a5 100644 --- a/hw/char/trace-events +++ b/hw/char/trace-events @@ -105,3 +105,11 @@ cadence_uart_baudrate(unsigned baudrate) "baudrate %u" # sh_serial.c sh_serial_read(char *id, unsigned size, uint64_t offs, uint64_t val) " %s size %d offs 0x%02" PRIx64 " -> 0x%02" PRIx64 sh_serial_write(char *id, unsigned size, uint64_t offs, uint64_t val) "%s size %d offs 0x%02" PRIx64 " <- 0x%02" PRIx64 + +# xen_console.c +xen_console_connect(unsigned int idx, unsigned int ring_ref, unsigned int port, unsigned int limit) "idx %u ring_ref %u port %u limit %u" +xen_console_disconnect(unsigned int idx) "idx %u" +xen_console_unrealize(unsigned int idx) "idx %u" +xen_console_realize(unsigned int idx, const char *chrdev) "idx %u chrdev %s" +xen_console_device_create(unsigned int idx) "idx %u" +xen_console_device_destroy(unsigned int idx) "idx %u" diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c index 810dae3f44..4a419dc287 100644 --- a/hw/char/xen_console.c +++ b/hw/char/xen_console.c @@ -20,15 +20,20 @@ */ #include "qemu/osdep.h" +#include "qemu/cutils.h" #include #include #include "qapi/error.h" #include "sysemu/sysemu.h" #include "chardev/char-fe.h" -#include "hw/xen/xen-legacy-backend.h" - +#include "hw/xen/xen-backend.h" +#include "hw/xen/xen-bus-helper.h" +#include "hw/qdev-properties.h" +#include "hw/qdev-properties-system.h" #include "hw/xen/interface/io/console.h" +#include "hw/xen/interface/io/xs_wire.h" +#include "trace.h" struct buffer { uint8_t *data; @@ -39,16 +44,22 @@ struct buffer { }; struct XenConsole { -struct XenLegacyDevice xendev; /* must be first */ +struct XenDevice xendev; /* must be first */ +XenEventChannel *event_channel; +int dev; struct buffer buffer; -char console[XEN_BUFSIZE]; -int ring_ref; +char *fe_path; +unsigned int ring_ref; void *sring; CharBackend chr; int backlog; }; +typedef struct XenConsole XenConsole; + +#define TYPE_XEN_CONSOLE_DEVICE "xen-console" +OBJECT_DECLARE_SIMPLE_TYPE(XenConsole, XEN_CONSOLE_DEVICE) -static void buffer_append(struct XenConsole *con) +static bool buffer_append(XenConsole *con) { struct buffer *buffer = >buffer; XENCONS_RING_IDX cons, prod, size; @@ -60,7 +71,7 @@ static void buffer_append(struct XenConsole *con) size = prod - cons; if ((size == 0) || (size > sizeof(intf->out))) -return; +return false; if ((buffer->capacity - buffer->size) < size) { buffer->capacity += (size + 1024); @@ -73,7 +84,7 @@ static void buffer_append(struct XenConsole *con) xen_mb(); intf->out_cons = cons; -xen_pv_send_notify(>xendev); +xen_device_notify_event_channel(XEN_DEVICE(con), con->event_channel, NULL); if (buffer->max_capacity && buffer->size > buffer->max_capacity) { @@ -89,6 +100,7 @@ static void buffer_append(struct XenConsole *con) if (buffer->consumed > buffer->max_capacity - over) buffer->consumed = buffer->max_capacity - over; } +return true; } static void buffer_advance(struct buffer *buffer, size_t len) @@ -100,7 +112,7 @@ static void buffer_advance(struct buffer *buffer, size_t len) } } -static int ring_free_bytes(struct XenConsole *con) +static int ring_free_bytes(XenConsole *con) { struct xencons_interface *intf = con->sring; XENCONS_RING_IDX cons, prod, space; @@ -118,13 +130,13 @@ static int ring_free_bytes(struct XenConsole *con) static int xencons_can_receive(void *opaque) { -struct XenConsole *con = opaque; +XenConsole *con = opaque; return ring_free_bytes(con); } static void xencons_receive(void *opaque, const uint8_t *buf, int len) { -struct XenConsole *con = opaque; +XenConsole *con = opaque; struct xencons_interface *intf = con->sring; XENCONS_RING_IDX prod; int i, max; @@ -141,10 +153,10 @@ static void xencons_receive(void *opaque, const uint8_t *buf, int len) } xen_wmb(); intf->in_prod = prod; -xen_pv_send_notify(>xendev); +xen_device_notify_event_channel(XEN_DEVICE(con), con->event_channel, NULL); } -static void xencons_send(struct XenConsole *con) +static bool xencons_send(XenConsole *con) { ssize_t len, size; @@ -159,174 +171,436 @@ static void xencons_send(struct XenConsole *con) if (len < 1) { if (!con->backlog) { con->backlog = 1; -
[PATCH v4 13/17] hw/i386/pc: support '-nic' for xen-net-device
From: David Woodhouse The default NIC creation seems a bit hackish to me. I don't understand why each platform has to call pci_nic_init_nofail() from a point in the code where it actually has a pointer to the PCI bus, and then we have the special cases for things like ne2k_isa. If qmp_device_add() can *find* the appropriate bus and instantiate the device on it, why can't we just do that from generic code for creating the default NICs too? But that isn't a yak I want to shave today. Add a xenbus field to the PCMachineState so that it can make its way from pc_basic_device_init() to pc_nic_init() and be handled as a special case like ne2k_isa is. Now we can launch emulated Xen guests with '-nic user'. Signed-off-by: David Woodhouse --- hw/i386/pc.c | 11 --- hw/i386/pc_piix.c| 2 +- hw/i386/pc_q35.c | 2 +- hw/xen/xen-bus.c | 4 +++- include/hw/i386/pc.h | 4 +++- include/hw/xen/xen-bus.h | 2 +- 6 files changed, 17 insertions(+), 8 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 6031234a73..c2bc3fa52d 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1259,7 +1259,7 @@ void pc_basic_device_init(struct PCMachineState *pcms, if (pcms->bus) { pci_create_simple(pcms->bus, -1, "xen-platform"); } -xen_bus_init(); +pcms->xenbus = xen_bus_init(); xen_be_init(); } #endif @@ -1287,7 +1287,8 @@ void pc_basic_device_init(struct PCMachineState *pcms, pcms->vmport != ON_OFF_AUTO_ON); } -void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus) +void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus, + BusState *xen_bus) { MachineClass *mc = MACHINE_CLASS(pcmc); int i; @@ -1297,7 +1298,11 @@ void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus) NICInfo *nd = _table[i]; const char *model = nd->model ? nd->model : mc->default_nic; -if (g_str_equal(model, "ne2k_isa")) { +if (xen_bus && (!nd->model || g_str_equal(model, "xen-net-device"))) { +DeviceState *dev = qdev_new("xen-net-device"); +qdev_set_nic_properties(dev, nd); +qdev_realize_and_unref(dev, xen_bus, _fatal); +} else if (g_str_equal(model, "ne2k_isa")) { pc_init_ne2k_isa(isa_bus, nd); } else { pci_nic_init_nofail(nd, pci_bus, model, NULL); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 26e161beb9..eace854335 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -342,7 +342,7 @@ static void pc_init1(MachineState *machine, pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true, 0x4); -pc_nic_init(pcmc, isa_bus, pci_bus); +pc_nic_init(pcmc, isa_bus, pci_bus, pcms->xenbus); if (pcmc->pci_enabled) { pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 597943ff1b..4f3e5412f6 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -340,7 +340,7 @@ static void pc_q35_init(MachineState *machine) /* the rest devices to which pci devfn is automatically assigned */ pc_vga_init(isa_bus, host_bus); -pc_nic_init(pcmc, isa_bus, host_bus); +pc_nic_init(pcmc, isa_bus, host_bus, pcms->xenbus); if (machine->nvdimms_state->is_enabled) { nvdimm_init_acpi_state(machine->nvdimms_state, system_io, diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c index cc6f1b362f..4973e7d9c9 100644 --- a/hw/xen/xen-bus.c +++ b/hw/xen/xen-bus.c @@ -1133,11 +1133,13 @@ static void xen_register_types(void) type_init(xen_register_types) -void xen_bus_init(void) +BusState *xen_bus_init(void) { DeviceState *dev = qdev_new(TYPE_XEN_BRIDGE); BusState *bus = qbus_new(TYPE_XEN_BUS, dev, NULL); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal); qbus_set_bus_hotplug_handler(bus); + +return bus; } diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 29a9724524..a10ceeabbf 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -33,6 +33,7 @@ typedef struct PCMachineState { /* Pointers to devices and objects: */ PCIBus *bus; +BusState *xenbus; I2CBus *smbus; PFlashCFI01 *flash[2]; ISADevice *pcspk; @@ -184,7 +185,8 @@ void pc_basic_device_init(struct PCMachineState *pcms, void pc_cmos_init(PCMachineState *pcms, BusState *ide0, BusState *ide1, ISADevice *s); -void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus); +void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus, + BusState *xen_bus); void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs); diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h index 38d40afa37..334ddd1ff6 100644 --- a/include/hw/xen/xen-bus.h +++ b/include/hw/xen/xen-bus.h @@ -75,7 +75,7 @@
[PATCH v4 10/17] hw/xen: add support for Xen primary console in emulated mode
From: David Woodhouse The primary console is special because the toolstack maps a page into the guest for its ring, and also allocates the guest-side event channel. The guest's grant table is even primed to export that page using a known grant ref#. Add support for all that in emulated mode, so that we can have a primary console. For reasons unclear, the backends running under real Xen don't just use a mapping of the well-known GNTTAB_RESERVED_CONSOLE grant ref (which would also be in the ring-ref node in XenStore). Instead, the toolstack sets the ring-ref node of the primary console to the GFN of the guest page. The backend is expected to handle that special case and map it with foreignmem operations instead. We don't have an implementation of foreignmem ops for emulated Xen mode, so just make it map GNTTAB_RESERVED_CONSOLE instead. This would probably work for real Xen too, but we can't work out how to make real Xen create a primary console of type "ioemu" to make QEMU drive it, so we can't test that; might as well leave it as it is for now under Xen. Now at last we can boot the Xen PV shim and run PV kernels in QEMU. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- hw/char/xen_console.c | 78 hw/i386/kvm/meson.build | 1 + hw/i386/kvm/trace-events | 2 + hw/i386/kvm/xen-stubs.c | 8 ++ hw/i386/kvm/xen_gnttab.c | 7 +- hw/i386/kvm/xen_primary_console.c | 193 ++ hw/i386/kvm/xen_primary_console.h | 23 hw/i386/kvm/xen_xenstore.c| 10 ++ hw/xen/xen-bus.c | 5 + include/hw/xen/xen-bus.h | 1 + target/i386/kvm/xen-emu.c | 23 +++- 11 files changed, 328 insertions(+), 23 deletions(-) create mode 100644 hw/i386/kvm/xen_primary_console.c create mode 100644 hw/i386/kvm/xen_primary_console.h diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c index 4a419dc287..5cbee2f184 100644 --- a/hw/char/xen_console.c +++ b/hw/char/xen_console.c @@ -33,6 +33,8 @@ #include "hw/qdev-properties-system.h" #include "hw/xen/interface/io/console.h" #include "hw/xen/interface/io/xs_wire.h" +#include "hw/xen/interface/grant_table.h" +#include "hw/i386/kvm/xen_primary_console.h" #include "trace.h" struct buffer { @@ -230,24 +232,47 @@ static bool xen_console_connect(XenDevice *xendev, Error **errp) return false; } -if (!con->dev) { -xen_pfn_t mfn = (xen_pfn_t)con->ring_ref; -con->sring = qemu_xen_foreignmem_map(xendev->frontend_id, NULL, - PROT_READ | PROT_WRITE, - 1, , NULL); -if (!con->sring) { -error_setg(errp, "failed to map console page"); -return false; +switch (con->dev) { +case 0: +/* + * The primary console is special. For real Xen the ring-ref is + * actually a GFN which needs to be mapped as foreignmem. + */ +if (xen_mode != XEN_EMULATE) { +xen_pfn_t mfn = (xen_pfn_t)con->ring_ref; +con->sring = qemu_xen_foreignmem_map(xendev->frontend_id, NULL, + PROT_READ | PROT_WRITE, + 1, , NULL); +if (!con->sring) { +error_setg(errp, "failed to map console page"); +return false; +} +break; } -} else { + +/* + * For Xen emulation, we still follow the convention of ring-ref + * holding the GFN, but we map the fixed GNTTAB_RESERVED_CONSOLE + * grant ref because there is no implementation of foreignmem + * operations for emulated mode. The emulation code which handles + * the guest-side page and event channel also needs to be informed + * of the backend event channel port, in order to reconnect to it + * after a soft reset. + */ +xen_primary_console_set_be_port( +xen_event_channel_get_local_port(con->event_channel)); +con->ring_ref = GNTTAB_RESERVED_CONSOLE; +/* fallthrough */ +default: con->sring = xen_device_map_grant_refs(xendev, >ring_ref, 1, PROT_READ | PROT_WRITE, errp); if (!con->sring) { -error_prepend(errp, "failed to map grant ref: "); +error_prepend(errp, "failed to map console grant ref: "); return false; } +break; } trace_xen_console_connect(con->dev, con->ring_ref, port, @@ -272,10 +297,14 @@ static void xen_console_disconnect(XenDevice *xendev, Error **errp) xen_device_unbind_event_channel(xendev, con->event_channel, errp); con->event_channel =
[PATCH v4 06/17] hw/xen: automatically assign device index to block devices
From: David Woodhouse There's no need to force the user to assign a vdev. We can automatically assign one, starting at xvda and searching until we find the first disk name that's unused. This means we can now allow '-drive if=xen,file=xxx' to work without an explicit separate -driver argument, just like if=virtio. Rip out the legacy handling from the xenpv machine, which was scribbling over any disks configured by the toolstack, and didn't work with anything but raw images. Signed-off-by: David Woodhouse Acked-by: Kevin Wolf --- blockdev.c | 15 +++- hw/block/xen-block.c| 118 ++-- hw/xen/xen_devconfig.c | 28 --- hw/xenpv/xen_machine_pv.c | 9 --- include/hw/xen/xen-legacy-backend.h | 1 - 5 files changed, 125 insertions(+), 46 deletions(-) diff --git a/blockdev.c b/blockdev.c index 1517dc6210..e9b7e38dc4 100644 --- a/blockdev.c +++ b/blockdev.c @@ -255,13 +255,13 @@ void drive_check_orphaned(void) * Ignore default drives, because we create certain default * drives unconditionally, then leave them unclaimed. Not the * users fault. - * Ignore IF_VIRTIO, because it gets desugared into -device, - * so we can leave failing to -device. + * Ignore IF_VIRTIO or IF_XEN, because it gets desugared into + * -device, so we can leave failing to -device. * Ignore IF_NONE, because leaving unclaimed IF_NONE remains * available for device_add is a feature. */ if (dinfo->is_default || dinfo->type == IF_VIRTIO -|| dinfo->type == IF_NONE) { +|| dinfo->type == IF_XEN || dinfo->type == IF_NONE) { continue; } if (!blk_get_attached_dev(blk)) { @@ -977,6 +977,15 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type, qemu_opt_set(devopts, "driver", "virtio-blk", _abort); qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"), _abort); +} else if (type == IF_XEN) { +QemuOpts *devopts; +devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, + _abort); +qemu_opt_set(devopts, "driver", + (media == MEDIA_CDROM) ? "xen-cdrom" : "xen-disk", + _abort); +qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"), + _abort); } filename = qemu_opt_get(legacy_opts, "file"); diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c index bfa53960c3..6d64ede94f 100644 --- a/hw/block/xen-block.c +++ b/hw/block/xen-block.c @@ -27,13 +27,119 @@ #include "sysemu/block-backend.h" #include "sysemu/iothread.h" #include "dataplane/xen-block.h" +#include "hw/xen/interface/io/xs_wire.h" #include "trace.h" +#define XVDA_MAJOR 202 +#define XVDQ_MAJOR (1 << 20) +#define XVDBGQCV_MAJOR ((1 << 21) - 1) +#define HDA_MAJOR 3 +#define HDC_MAJOR 22 +#define SDA_MAJOR 8 + + +static int vdev_to_diskno(unsigned int vdev_nr) +{ +switch (vdev_nr >> 8) { +case XVDA_MAJOR: +case SDA_MAJOR: +return (vdev_nr >> 4) & 0x15; + +case HDA_MAJOR: +return (vdev_nr >> 6) & 1; + +case HDC_MAJOR: +return ((vdev_nr >> 6) & 1) + 2; + +case XVDQ_MAJOR ... XVDBGQCV_MAJOR: +return (vdev_nr >> 8) & 0xf; + +default: +return -1; +} +} + +#define MAX_AUTO_VDEV 4096 + +/* + * Find a free device name in the xvda → xvdfan range and set it in + * blockdev->props.vdev. Our definition of "free" is that there must + * be no other disk or partition with the same disk number. + * + * You are technically permitted to have all of hda, hda1, sda, sda1, + * xvda and xvda1 as *separate* PV block devices with separate backing + * stores. That doesn't make it a good idea. This code will skip xvda + * if *any* of those "conflicting" devices already exists. + * + * The limit of xvdfan (disk 4095) is fairly arbitrary just to avoid a + * stupidly sized bitmap, but Linux as of v6.6 doesn't support anything + * higher than that anyway. + */ +static bool xen_block_find_free_vdev(XenBlockDevice *blockdev, Error **errp) +{ +XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(blockdev))); +unsigned long used_devs[BITS_TO_LONGS(MAX_AUTO_VDEV)]; +XenBlockVdev *vdev = >props.vdev; +char fe_path[XENSTORE_ABS_PATH_MAX + 1]; +char **existing_frontends; +unsigned int nr_existing = 0; +unsigned int vdev_nr; +int i, disk = 0; + +snprintf(fe_path, sizeof(fe_path), "/local/domain/%u/device/vbd", + blockdev->xendev.frontend_id); + +existing_frontends = qemu_xen_xs_directory(xenbus->xsh, XBT_NULL, fe_path, + _existing); +if (!existing_frontends && errno != ENOENT) { +error_setg_errno(errp, errno, "cannot read %s", fe_path); +return false; +} + +
[PATCH v4 04/17] i386/xen: advertise XEN_HVM_CPUID_UPCALL_VECTOR in CPUID
From: David Woodhouse This will allow Linux guests (since v6.0) to use the per-vCPU upcall vector delivered as MSI through the local APIC. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- target/i386/kvm/kvm.c | 4 1 file changed, 4 insertions(+) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 770e81d56e..11b8177eff 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -1837,6 +1837,10 @@ int kvm_arch_init_vcpu(CPUState *cs) c->eax |= XEN_HVM_CPUID_VCPU_ID_PRESENT; c->ebx = cs->cpu_index; } + +if (cs->kvm_state->xen_version >= XEN_VERSION(4, 17)) { +c->eax |= XEN_HVM_CPUID_UPCALL_VECTOR; +} } r = kvm_xen_init_vcpu(cs); -- 2.34.1
[PATCH v4 01/17] i386/xen: Ignore VCPU_SSHOTTMR_future flag in set_singleshot_timer()
From: David Woodhouse Upstream Xen now ignores this flag¹, since the only guest kernel ever to use it was buggy. ¹ https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=19c6cbd909 Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant --- target/i386/kvm/xen-emu.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c index 75b2c557b9..1dc9ab0d91 100644 --- a/target/i386/kvm/xen-emu.c +++ b/target/i386/kvm/xen-emu.c @@ -1077,17 +1077,13 @@ static int vcpuop_stop_periodic_timer(CPUState *target) * Must always be called with xen_timers_lock held. */ static int do_set_singleshot_timer(CPUState *cs, uint64_t timeout_abs, - bool future, bool linux_wa) + bool linux_wa) { CPUX86State *env = _CPU(cs)->env; int64_t now = kvm_get_current_ns(); int64_t qemu_now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); int64_t delta = timeout_abs - now; -if (future && timeout_abs < now) { -return -ETIME; -} - if (linux_wa && unlikely((int64_t)timeout_abs < 0 || (delta > 0 && (uint32_t)(delta >> 50) != 0))) { /* @@ -1129,9 +1125,13 @@ static int vcpuop_set_singleshot_timer(CPUState *cs, uint64_t arg) } QEMU_LOCK_GUARD(_CPU(cs)->env.xen_timers_lock); -return do_set_singleshot_timer(cs, sst.timeout_abs_ns, - !!(sst.flags & VCPU_SSHOTTMR_future), - false); + +/* + * We ignore the VCPU_SSHOTTMR_future flag, just as Xen now does. + * The only guest that ever used it, got it wrong. + * https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=19c6cbd909 + */ +return do_set_singleshot_timer(cs, sst.timeout_abs_ns, false); } static int vcpuop_stop_singleshot_timer(CPUState *cs) @@ -1156,7 +1156,7 @@ static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, err = vcpuop_stop_singleshot_timer(CPU(cpu)); } else { QEMU_LOCK_GUARD(_CPU(cpu)->env.xen_timers_lock); -err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); +err = do_set_singleshot_timer(CPU(cpu), timeout, true); } exit->u.hcall.result = err; return true; @@ -1844,7 +1844,7 @@ int kvm_put_xen_state(CPUState *cs) QEMU_LOCK_GUARD(>xen_timers_lock); if (env->xen_singleshot_timer_ns) { ret = do_set_singleshot_timer(cs, env->xen_singleshot_timer_ns, -false, false); + false); if (ret < 0) { return ret; } -- 2.34.1
[PATCH v4 03/17] include: update Xen public headers to Xen 4.17.2 release
From: David Woodhouse ... in order to advertise the XEN_HVM_CPUID_UPCALL_VECTOR feature, which will come in a subsequent commit. Signed-off-by: David Woodhouse Acked-by: Paul Durrant --- hw/i386/kvm/xen_xenstore.c| 2 +- include/hw/xen/interface/arch-arm.h | 37 +++--- include/hw/xen/interface/arch-x86/cpuid.h | 31 +--- .../hw/xen/interface/arch-x86/xen-x86_32.h| 19 +-- .../hw/xen/interface/arch-x86/xen-x86_64.h| 19 +-- include/hw/xen/interface/arch-x86/xen.h | 26 ++ include/hw/xen/interface/event_channel.h | 19 +-- include/hw/xen/interface/features.h | 19 +-- include/hw/xen/interface/grant_table.h| 19 +-- include/hw/xen/interface/hvm/hvm_op.h | 19 +-- include/hw/xen/interface/hvm/params.h | 19 +-- include/hw/xen/interface/io/blkif.h | 27 -- include/hw/xen/interface/io/console.h | 19 +-- include/hw/xen/interface/io/fbif.h| 19 +-- include/hw/xen/interface/io/kbdif.h | 19 +-- include/hw/xen/interface/io/netif.h | 25 +++--- include/hw/xen/interface/io/protocols.h | 19 +-- include/hw/xen/interface/io/ring.h| 49 ++- include/hw/xen/interface/io/usbif.h | 19 +-- include/hw/xen/interface/io/xenbus.h | 19 +-- include/hw/xen/interface/io/xs_wire.h | 36 ++ include/hw/xen/interface/memory.h | 30 +--- include/hw/xen/interface/physdev.h| 23 ++--- include/hw/xen/interface/sched.h | 19 +-- include/hw/xen/interface/trace.h | 19 +-- include/hw/xen/interface/vcpu.h | 19 +-- include/hw/xen/interface/version.h| 19 +-- include/hw/xen/interface/xen-compat.h | 19 +-- include/hw/xen/interface/xen.h| 19 +-- 29 files changed, 124 insertions(+), 523 deletions(-) diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c index 8e716a7009..831da535fc 100644 --- a/hw/i386/kvm/xen_xenstore.c +++ b/hw/i386/kvm/xen_xenstore.c @@ -331,7 +331,7 @@ static void xs_error(XenXenstoreState *s, unsigned int id, const char *errstr = NULL; for (unsigned int i = 0; i < ARRAY_SIZE(xsd_errors); i++) { -struct xsd_errors *xsd_error = _errors[i]; +const struct xsd_errors *xsd_error = _errors[i]; if (xsd_error->errnum == errnum) { errstr = xsd_error->errstring; diff --git a/include/hw/xen/interface/arch-arm.h b/include/hw/xen/interface/arch-arm.h index 94b31511dd..1528ced509 100644 --- a/include/hw/xen/interface/arch-arm.h +++ b/include/hw/xen/interface/arch-arm.h @@ -1,26 +1,9 @@ +/* SPDX-License-Identifier: MIT */ /** * arch-arm.h * * Guest OS interface to ARM Xen. * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to - * deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or - * sell copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - * * Copyright 2011 (C) Citrix Systems */ @@ -361,6 +344,7 @@ typedef uint64_t xen_callback_t; #define PSR_DBG_MASK(1<<9)/* arm64: Debug Exception mask */ #define PSR_IT_MASK (0x0600fc00) /* Thumb If-Then Mask */ #define PSR_JAZELLE (1<<24) /* Jazelle Mode */ +#define PSR_Z (1<<30) /* Zero condition flag */ /* 32 bit modes */ #define PSR_MODE_USR 0x10 @@ -383,7 +367,15 @@ typedef uint64_t xen_callback_t; #define PSR_MODE_EL1t 0x04 #define PSR_MODE_EL0t 0x00 -#define PSR_GUEST32_INIT (PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_SVC) +/* + * We set PSR_Z to be able to boot Linux kernel versions with an invalid + * encoding of the first 8 NOP instructions. See commit a92882a4d270 in + * Linux. + * + * Note that PSR_Z is also set by U-Boot and QEMU -kernel when loading + * zImage kernels on aarch32. + */ +#define PSR_GUEST32_INIT
[PATCH v4 00/17] Get Xen PV shim running in QEMU, add net and console
The Xen PV shim requires a PV console; add that. Also update the Xen PV network support to the new XenDevice model so that it can be used with emulated Xen guests. Fix up the Xen block support to allow it to be used with '-device file=IMAGE,if=xen'. Update the documentation to reflect all of these, taking the opportunity to simplify what it says about q35, by making unplug work for AHCI. Ignore the VCPU_SSHOTTMR_future timer flag, and advertise the 'fixed' per-vCPU upcall vector support, as newer upstream Xen do. Fix a bug where net_cleanup() would remove the NIC from underneath the emulated network devices, which doesn't work well when network devices have their own destructors (as the Xen PV one has to, in order to clean up the XenStore nodes). v4: • Drop the fixes from the start of the series, which have been sent as a separate pull request (cc: qemu-stable): https://lore.kernel.org/qemu-devel/20231106103955.200867-1-dw...@infradead.org/ • Go back to the original, more hackish, version of making `-nic` work for Xen network. The better fix for that is not going to get through review before the soft freeze. • Fix the documentation so the docs can reference the '-initrd' command line option with newer Sphinx. • Improve the duplicate detection for Xen block devices to match all partitions and even the same disk number on different majors. David Woodhouse (17): i386/xen: Ignore VCPU_SSHOTTMR_future flag in set_singleshot_timer() hw/xen: Clean up event channel 'type_val' handling to use union include: update Xen public headers to Xen 4.17.2 release i386/xen: advertise XEN_HVM_CPUID_UPCALL_VECTOR in CPUID hw/xen: populate store frontend nodes with XenStore PFN/port hw/xen: automatically assign device index to block devices hw/xen: add get_frontend_path() method to XenDeviceClass hw/xen: do not repeatedly try to create a failing backend device hw/xen: update Xen console to XenDevice model hw/xen: add support for Xen primary console in emulated mode hw/xen: only remove peers of PCI NICs on unplug hw/xen: update Xen PV NIC to XenDevice model hw/i386/pc: support '-nic' for xen-net-device net: do not delete nics in net_cleanup() xen-platform: unplug AHCI disks doc/sphinx/hxtool.py: add optional label argument to SRST directive docs: update Xen-on-KVM documentation MAINTAINERS| 2 +- blockdev.c | 15 +- docs/sphinx/hxtool.py | 18 +- docs/system/i386/xen.rst | 107 +++-- docs/system/invocation.rst | 1 + hw/block/xen-block.c | 118 - hw/char/trace-events | 8 + hw/char/xen_console.c | 572 +++-- hw/i386/kvm/meson.build| 1 + hw/i386/kvm/trace-events | 2 + hw/i386/kvm/xen-stubs.c| 8 + hw/i386/kvm/xen_evtchn.c | 151 +++ hw/i386/kvm/xen_gnttab.c | 7 +- hw/i386/kvm/xen_primary_console.c | 193 + hw/i386/kvm/xen_primary_console.h | 23 + hw/i386/kvm/xen_xenstore.c | 23 +- hw/i386/pc.c | 11 +- hw/i386/pc_piix.c | 2 +- hw/i386/pc_q35.c | 2 +- hw/i386/xen/xen_platform.c | 77 ++-- hw/net/meson.build | 2 +- hw/net/trace-events| 11 + hw/net/xen_nic.c | 484 - hw/xen/xen-backend.c | 27 +- hw/xen/xen-bus.c | 23 +- hw/xen/xen-legacy-backend.c| 1 - hw/xen/xen_devconfig.c | 28 -- hw/xenpv/xen_machine_pv.c | 10 - include/hw/i386/pc.h | 4 +- include/hw/xen/interface/arch-arm.h| 37 +- include/hw/xen/interface/arch-x86/cpuid.h | 31 +- include/hw/xen/interface/arch-x86/xen-x86_32.h | 19 +- include/hw/xen/interface/arch-x86/xen-x86_64.h | 19 +- include/hw/xen/interface/arch-x86/xen.h| 26 +- include/hw/xen/interface/event_channel.h | 19 +- include/hw/xen/interface/features.h| 19 +- include/hw/xen/interface/grant_table.h | 19 +- include/hw/xen/interface/hvm/hvm_op.h | 19 +- include/hw/xen/interface/hvm/params.h | 19 +- include/hw/xen/interface/io/blkif.h| 27 +- include/hw/xen/interface/io/console.h | 19 +- include/hw/xen/interface/io/fbif.h | 19 +- include/hw/xen/interface/io/kbdif.h| 19 +- include/hw/xen/interface/io/netif.h| 25 +-
Re: [PATCH v2 1/3] ide/pci.c: introduce pci_ide_update_mode() function
Am 25.10.2023 um 00:40 hat Mark Cave-Ayland geschrieben: > This function reads the value of the PCI_CLASS_PROG register for PCI IDE > controllers and configures the PCI BARs and/or IDE ioports accordingly. > > In the case where we switch to legacy mode, the PCI BARs are set to return > zero > (as suggested in the "PCI IDE Controller" specification), the legacy IDE > ioports > are enabled, and the PCI interrupt pin cleared to indicate legacy IRQ routing. > > Conversely when we switch to native mode, the legacy IDE ioports are disabled > and the PCI interrupt pin set to indicate native IRQ routing. The contents of > the PCI BARs are unspecified, but this is not an issue since if a PCI IDE > controller has been switched to native mode then its BARs will need to be > programmed. > > Signed-off-by: Mark Cave-Ayland > Tested-by: BALATON Zoltan > Tested-by: Bernhard Beschow > --- > hw/ide/pci.c | 90 > include/hw/ide/pci.h | 1 + > 2 files changed, 91 insertions(+) > > diff --git a/hw/ide/pci.c b/hw/ide/pci.c > index a25b352537..5be643b460 100644 > --- a/hw/ide/pci.c > +++ b/hw/ide/pci.c > @@ -104,6 +104,96 @@ const MemoryRegionOps pci_ide_data_le_ops = { > .endianness = DEVICE_LITTLE_ENDIAN, > }; > > +static const MemoryRegionPortio ide_portio_list[] = { > +{ 0, 8, 1, .read = ide_ioport_read, .write = ide_ioport_write }, > +{ 0, 1, 2, .read = ide_data_readw, .write = ide_data_writew }, > +{ 0, 1, 4, .read = ide_data_readl, .write = ide_data_writel }, > +PORTIO_END_OF_LIST(), > +}; > + > +static const MemoryRegionPortio ide_portio2_list[] = { > +{ 0, 1, 1, .read = ide_status_read, .write = ide_ctrl_write }, > +PORTIO_END_OF_LIST(), > +}; This is duplicated from hw/ide/ioport.c. I think it would be better to use the arrays already defined there, ideally by calling ioport.c functions to setup and release the I/O ports. > +void pci_ide_update_mode(PCIIDEState *s) > +{ > +PCIDevice *d = PCI_DEVICE(s); > +uint8_t mode = d->config[PCI_CLASS_PROG]; > + > +switch (mode & 0xf) { > +case 0xa: > +/* Both channels legacy mode */ Why is it ok to handle only the case where both channels are set to the same mode? The spec describes mixed-mode setups, too, and doesn't seem to allow ignoring a mode change if it's only for one of the channels. > + > +/* Zero BARs */ > +pci_set_long(d->config + PCI_BASE_ADDRESS_0, 0x0); > +pci_set_long(d->config + PCI_BASE_ADDRESS_1, 0x0); > +pci_set_long(d->config + PCI_BASE_ADDRESS_2, 0x0); > +pci_set_long(d->config + PCI_BASE_ADDRESS_3, 0x0); Here I'm not sure what the spec really implies. Disabling the BAR (i.e. making it read-only and returning 0) while in compatibility mode doesn't necessarily mean that the value of the register is lost. In other words, are we sure that a driver can't expect that the old value is still there when it re-enables native mode? > +/* Clear interrupt pin */ > +pci_config_set_interrupt_pin(d->config, 0); Unlike for the BARs, I don't see anything in the spec that allows disabling this byte. I doubt it hurts in practice, but did you see any drivers requiring this? According to the spec, we just must not use the PCI interrupt in compatbility mode, but the registers stay accessible. As far as I can see, the whole PCI interrupt configuration is currently unused anyway, and nothing in this series seems to change it. So won't we incorrectly continue to use the legacy interrupt even in native mode? (Actually, cmd646 seems to get it wrong the other way around and uses the PCI interrupt even in compatibility mode.) I think this means that BMDMAState needs to have two irq lines (a legacy and a PCI one) and select the right one in bmdma_irq() depending on which mode we're in currently. > +/* Add legacy IDE ports */ > +if (!s->bus[0].portio_list.owner) { > +portio_list_init(>bus[0].portio_list, OBJECT(d), > + ide_portio_list, >bus[0], "ide"); > +portio_list_add(>bus[0].portio_list, > +pci_address_space_io(d), 0x1f0); > +} > + > +if (!s->bus[0].portio2_list.owner) { > +portio_list_init(>bus[0].portio2_list, OBJECT(d), > + ide_portio2_list, >bus[0], "ide"); > +portio_list_add(>bus[0].portio2_list, > +pci_address_space_io(d), 0x3f6); > +} > + > +if (!s->bus[1].portio_list.owner) { > +portio_list_init(>bus[1].portio_list, OBJECT(d), > +ide_portio_list, >bus[1], "ide"); > +portio_list_add(>bus[1].portio_list, > +pci_address_space_io(d), 0x170); > +} > + > +if (!s->bus[1].portio2_list.owner) { > +portio_list_init(>bus[1].portio2_list, OBJECT(d), > +
Re: [PULL 38/40] migration: Implement MigrateChannelList to qmp migration flow.
Peter Maydell wrote: > On Thu, 2 Nov 2023 at 11:46, Juan Quintela wrote: >> >> From: Het Gala >> >> Integrate MigrateChannelList with all transport backends >> (socket, exec and rdma) for both src and dest migration >> endpoints for qmp migration. >> >> For current series, limit the size of MigrateChannelList >> to single element (single interface) as runtime check. >> >> Suggested-by: Aravind Retnakaran >> Signed-off-by: Het Gala >> Signed-off-by: Fabiano Rosas >> Reviewed-by: Juan Quintela >> Signed-off-by: Juan Quintela >> Message-ID: <20231023182053.8711-13-faro...@suse.de> > > Hi; after this migration pull there seem to be a lot of > new Coverity issues in migration code. Could somebody > take a look at them, please? Hi I received this, looking into it. Later, Juan. > > Here's one in particular (CID 1523826, 1523828): > >> @@ -1927,35 +1933,38 @@ void qmp_migrate(const char *uri, bool has_channels, >> bool resume_requested; >> Error *local_err = NULL; >> MigrationState *s = migrate_get_current(); >> -g_autoptr(MigrationAddress) channel = NULL; >> +MigrationChannel *channel = NULL; >> +MigrationAddress *addr = NULL; > > 'channel' in this function used to be auto-freed, but now it is not... > >> >> /* >> * Having preliminary checks for uri and channel >> */ >> -if (has_channels) { >> -error_setg(errp, "'channels' argument should not be set yet."); >> -return; >> -} >> - >> if (uri && has_channels) { >> error_setg(errp, "'uri' and 'channels' arguments are mutually " >> "exclusive; exactly one of the two should be present in " >> "'migrate' qmp command "); >> return; >> -} >> - >> -if (!uri && !has_channels) { >> +} else if (channels) { >> +/* To verify that Migrate channel list has only item */ >> +if (channels->next) { >> +error_setg(errp, "Channel list has more than one entries"); >> +return; >> +} >> +channel = channels->value; >> +} else if (uri) { >> +/* caller uses the old URI syntax */ >> +if (!migrate_uri_parse(uri, , errp)) { >> +return; >> +} > > ...and here migrate_uri_parse() allocates memory which is > returned into 'channel'... > >> +} else { >> error_setg(errp, "neither 'uri' or 'channels' argument are " >> "specified in 'migrate' qmp command "); >> return; >> } >> - >> -if (!migrate_uri_parse(uri, , errp)) { >> -return; >> -} >> +addr = channel->addr; >> >> /* transport mechanism not suitable for migration? */ >> -if (!migration_channels_and_transport_compatible(channel, errp)) { >> +if (!migration_channels_and_transport_compatible(addr, errp)) { >> return; >> } >> >> @@ -1972,8 +1981,8 @@ void qmp_migrate(const char *uri, bool has_channels, >> } >> } >> >> -if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { >> -SocketAddress *saddr = >u.socket; >> +if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { >> +SocketAddress *saddr = >u.socket; >> if (saddr->type == SOCKET_ADDRESS_TYPE_INET || >> saddr->type == SOCKET_ADDRESS_TYPE_UNIX || >> saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) { >> @@ -1982,13 +1991,13 @@ void qmp_migrate(const char *uri, bool has_channels, >> fd_start_outgoing_migration(s, saddr->u.fd.str, _err); >> } >> #ifdef CONFIG_RDMA >> -} else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) { >> -rdma_start_outgoing_migration(s, >u.rdma, _err); >> +} else if (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) { >> +rdma_start_outgoing_migration(s, >u.rdma, _err); >> #endif >> -} else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) { >> -exec_start_outgoing_migration(s, channel->u.exec.args, _err); >> -} else if (channel->transport == MIGRATION_ADDRESS_TYPE_FILE) { >> -file_start_outgoing_migration(s, >u.file, _err); >> +} else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) { >> +exec_start_outgoing_migration(s, addr->u.exec.args, _err); >> +} else if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) { >> +file_start_outgoing_migration(s, >u.file, _err); >> } else { >> error_setg(_err, QERR_INVALID_PARAMETER_VALUE, "uri", >> "a valid migration protocol"); > > ...which is leaked because we don't add any code for freeing > channel to compensate for it no longer being autofreed. > > thanks > -- PMM
Re: [PULL 38/40] migration: Implement MigrateChannelList to qmp migration flow.
On Thu, 2 Nov 2023 at 11:46, Juan Quintela wrote: > > From: Het Gala > > Integrate MigrateChannelList with all transport backends > (socket, exec and rdma) for both src and dest migration > endpoints for qmp migration. > > For current series, limit the size of MigrateChannelList > to single element (single interface) as runtime check. > > Suggested-by: Aravind Retnakaran > Signed-off-by: Het Gala > Signed-off-by: Fabiano Rosas > Reviewed-by: Juan Quintela > Signed-off-by: Juan Quintela > Message-ID: <20231023182053.8711-13-faro...@suse.de> Hi; after this migration pull there seem to be a lot of new Coverity issues in migration code. Could somebody take a look at them, please? Here's one in particular (CID 1523826, 1523828): > @@ -1927,35 +1933,38 @@ void qmp_migrate(const char *uri, bool has_channels, > bool resume_requested; > Error *local_err = NULL; > MigrationState *s = migrate_get_current(); > -g_autoptr(MigrationAddress) channel = NULL; > +MigrationChannel *channel = NULL; > +MigrationAddress *addr = NULL; 'channel' in this function used to be auto-freed, but now it is not... > > /* > * Having preliminary checks for uri and channel > */ > -if (has_channels) { > -error_setg(errp, "'channels' argument should not be set yet."); > -return; > -} > - > if (uri && has_channels) { > error_setg(errp, "'uri' and 'channels' arguments are mutually " > "exclusive; exactly one of the two should be present in " > "'migrate' qmp command "); > return; > -} > - > -if (!uri && !has_channels) { > +} else if (channels) { > +/* To verify that Migrate channel list has only item */ > +if (channels->next) { > +error_setg(errp, "Channel list has more than one entries"); > +return; > +} > +channel = channels->value; > +} else if (uri) { > +/* caller uses the old URI syntax */ > +if (!migrate_uri_parse(uri, , errp)) { > +return; > +} ...and here migrate_uri_parse() allocates memory which is returned into 'channel'... > +} else { > error_setg(errp, "neither 'uri' or 'channels' argument are " > "specified in 'migrate' qmp command "); > return; > } > - > -if (!migrate_uri_parse(uri, , errp)) { > -return; > -} > +addr = channel->addr; > > /* transport mechanism not suitable for migration? */ > -if (!migration_channels_and_transport_compatible(channel, errp)) { > +if (!migration_channels_and_transport_compatible(addr, errp)) { > return; > } > > @@ -1972,8 +1981,8 @@ void qmp_migrate(const char *uri, bool has_channels, > } > } > > -if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { > -SocketAddress *saddr = >u.socket; > +if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { > +SocketAddress *saddr = >u.socket; > if (saddr->type == SOCKET_ADDRESS_TYPE_INET || > saddr->type == SOCKET_ADDRESS_TYPE_UNIX || > saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) { > @@ -1982,13 +1991,13 @@ void qmp_migrate(const char *uri, bool has_channels, > fd_start_outgoing_migration(s, saddr->u.fd.str, _err); > } > #ifdef CONFIG_RDMA > -} else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) { > -rdma_start_outgoing_migration(s, >u.rdma, _err); > +} else if (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) { > +rdma_start_outgoing_migration(s, >u.rdma, _err); > #endif > -} else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) { > -exec_start_outgoing_migration(s, channel->u.exec.args, _err); > -} else if (channel->transport == MIGRATION_ADDRESS_TYPE_FILE) { > -file_start_outgoing_migration(s, >u.file, _err); > +} else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) { > +exec_start_outgoing_migration(s, addr->u.exec.args, _err); > +} else if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) { > +file_start_outgoing_migration(s, >u.file, _err); > } else { > error_setg(_err, QERR_INVALID_PARAMETER_VALUE, "uri", > "a valid migration protocol"); ...which is leaked because we don't add any code for freeing channel to compensate for it no longer being autofreed. thanks -- PMM
Re: [PATCH v2 08/10] block: Don't query all block devices at hmp_nbd_server_start
On 09.06.23 22:19, Fabiano Rosas wrote: We're currently doing a full query-block just to enumerate the devices for qmp_nbd_server_add and then discarding the BlockInfoList afterwards. Alter hmp_nbd_server_start to instead iterate explicitly over the block_backends list. This allows the removal of the dependency on qmp_query_block from hmp_nbd_server_start. This is desirable because we're about to move qmp_query_block into a coroutine and don't need to change the NBD code at the same time. Signed-off-by: Fabiano Rosas --- block/monitor/block-hmp-cmds.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index ca2599de44..26116fe831 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -394,7 +394,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) bool writable = qdict_get_try_bool(qdict, "writable", false); bool all = qdict_get_try_bool(qdict, "all", false); Error *local_err = NULL; -BlockInfoList *block_list, *info; +BlockBackend *blk; SocketAddress *addr; NbdServerAddOptions export; @@ -419,18 +419,24 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) return; } -/* Then try adding all block devices. If one fails, close all and +/* + * Then try adding all block devices. If one fails, close all and * exit. */ -block_list = qmp_query_block(NULL); +for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) { +BlockDriverState *bs = blk_bs(blk); -for (info = block_list; info; info = info->next) { -if (!info->value->inserted) { +if (!*blk_name(blk) && !blk_get_attached_dev(blk)) { I’d like a comment here that historically, we’ve used qmp_query_block() here, and this is the same condition that it uses. (Otherwise, it’s hard to see why it matters whether a device is attached or not.) +continue; +} + +bs = bdrv_skip_implicit_filters(bs); +if (!bs || !bs->drv) { Same here. Just checking blk_is_inserted() would make more sense in this place, but if you want to absolutely keep behavior unchanged, then there should be a comment here why this check is done (because bdrv_query_info() does it to determine whether info->inserted should be set, which was historically used to determine whether this BlockBackend can be exported). continue; } export = (NbdServerAddOptions) { -.device = info->value->device, +.device = g_strdup(blk_name(blk)), Do we need to g_strdup() here? We didn’t before, so I think this will leak export.device. I know bdrv_query_info() uses g_strdup(), but that was released by the qapi_free_BlockInfoList() below, which is now removed without replacement. (On that note, it also looks like qmp_nbd_server_add() can leak arg->name (i.e. device.name) if it is not set by the caller. It also uses g_strdup() there, but never frees it. It does free the export_opts it creates, and `arg` is put into it, but as a deep copy, so anything in `arg` is leaked.) Hanna .has_writable = true, .writable = writable, }; @@ -443,8 +449,6 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) } } -qapi_free_BlockInfoList(block_list); - exit: hmp_handle_error(mon, local_err); }
Re: [PATCH v2 06/10] block: Convert bdrv_block_device_info into co_wrapper
On 09.06.23 22:19, Fabiano Rosas wrote: We're converting callers of bdrv_get_allocated_file_size() to run in coroutines because that function will be made asynchronous when called (indirectly) from the QMP dispatcher. This function is a candidate because it calls bdrv_query_image_info() -> bdrv_do_query_node_info() -> bdrv_get_allocated_file_size(). It is safe to turn this is a coroutine because the code it calls is made up of either simple accessors and string manipulation functions [1] or it has already been determined to be safe [2]. 1) bdrv_refresh_filename(), bdrv_is_read_only(), blk_enable_write_cache(), bdrv_cow_bs(), blk_get_public(), throttle_group_get_name(), bdrv_write_threshold_get(), bdrv_query_dirty_bitmaps(), throttle_group_get_config(), bdrv_filter_or_cow_bs(), bdrv_skip_implicit_filters() 2) bdrv_do_query_node_info() (see previous commit); Signed-off-by: Fabiano Rosas --- block/qapi.c | 8 include/block/qapi.h | 12 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index a2e71edaff..20660e15d6 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -41,10 +41,10 @@ #include "qemu/qemu-print.h" #include "sysemu/block-backend.h" -BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, -BlockDriverState *bs, -bool flat, -Error **errp) +BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk, +BlockDriverState *bs, +bool flat, +Error **errp) { ImageInfo **p_image_info; ImageInfo *backing_info; diff --git a/include/block/qapi.h b/include/block/qapi.h index 7035bcd1ae..5cb0202791 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -30,10 +30,14 @@ #include "block/snapshot.h" #include "qapi/qapi-types-block-core.h" -BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, -BlockDriverState *bs, -bool flat, -Error **errp); +BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk, +BlockDriverState *bs, +bool flat, +Error **errp); +BlockDeviceInfo *co_wrapper bdrv_block_device_info(BlockBackend *blk, + BlockDriverState *bs, + bool flat, + Error **errp); bdrv_co_block_device_info() is now marked as GRAPH_RDLOCK, so should this use a co_wrapper_bdrv_rdlock instead? Hanna int bdrv_query_snapshot_info_list(BlockDriverState *bs, SnapshotInfoList **p_list, Error **errp);
Re: [PATCH v2 07/10] block: Convert qmp_query_named_block_nodes to coroutine
On 09.06.23 22:19, Fabiano Rosas wrote: From: Lin Ma We're converting callers of bdrv_get_allocated_file_size() to run in coroutines because that function will be made asynchronous when called (indirectly) from the QMP dispatcher. This QMP command is a candidate because it indirectly calls bdrv_get_allocated_file_size() through bdrv_block_device_info() -> bdrv_query_image_info() -> bdrv_query_image_info(). The previous patches have determined that bdrv_query_image_info() and bdrv_do_query_node_info() are coroutine-safe so we can just make the QMP command run in a coroutine. Signed-off-by: Lin Ma Signed-off-by: Fabiano Rosas --- block.c | 2 +- blockdev.c | 6 +++--- qapi/block-core.json | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) (I see patch 9 does something with HMP code, but) hmp_info_block() calls qmp_query_named_block_nodes(), and I don’t think it may call such a coroutine_fn directly. diff --git a/block.c b/block.c index f94cee8930..abed744b60 100644 --- a/block.c +++ b/block.c @@ -6148,7 +6148,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(bool flat, list = NULL; QTAILQ_FOREACH(bs, _bdrv_states, node_list) { -BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, flat, errp); +BlockDeviceInfo *info = bdrv_co_block_device_info(NULL, bs, flat, errp); As far as I understand, only functions marked as coroutine_fn may call other coroutine_fn. Regardless of whether bdrv_named_nodes_list() is only called by another coroutine_fn, we still have to mark it as coroutine_fn, too (and probably rename it to bdrv_co_named_nodes_list()). Also, this function (bdrv_named_nodes_list()) uses GRAPH_RDLOCK_GUARD_MAINLOOP(). Is that the correct thing to use in a coroutine context? Hanna if (!info) { qapi_free_BlockDeviceInfoList(list); return NULL; diff --git a/blockdev.c b/blockdev.c index e6eba61484..8b5f7d06c8 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2818,9 +2818,9 @@ void qmp_drive_backup(DriveBackup *backup, Error **errp) blockdev_do_action(, errp); } -BlockDeviceInfoList *qmp_query_named_block_nodes(bool has_flat, - bool flat, - Error **errp) +BlockDeviceInfoList *coroutine_fn qmp_query_named_block_nodes(bool has_flat, + bool flat, + Error **errp) { bool return_flat = has_flat && flat; diff --git a/qapi/block-core.json b/qapi/block-core.json index 5dd5f7e4b0..9d4c92f2c9 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1972,7 +1972,8 @@ { 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ], 'data': { '*flat': 'bool' }, - 'allow-preconfig': true } + 'allow-preconfig': true, + 'coroutine': true} ## # @XDbgBlockGraphNodeType:
[PATCH v3 2/6] virtio: make endian_needed() work during loading
From: Marc-André Lureau There is no simple way to distinguish when the callback is used for load or save, AFAICT. Signed-off-by: Marc-André Lureau --- hw/virtio/virtio.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index e5105571cf..22aeb88235 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2517,7 +2517,11 @@ static bool virtio_device_endian_needed(void *opaque) { VirtIODevice *vdev = opaque; -assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); +/* On load, endian is UNKNOWN. The section might be loaded as required. */ +if (vdev->device_endian == VIRTIO_DEVICE_ENDIAN_UNKNOWN) { +return false; +} + if (!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { return vdev->device_endian != virtio_default_endian(); } -- 2.41.0
[PATCH v3 5/6] test-vmstate: add some subsection tests
From: Marc-André Lureau Check subsection support, and optional handling. Signed-off-by: Marc-André Lureau Reviewed-by: Juan Quintela --- tests/unit/test-vmstate.c | 116 ++ 1 file changed, 116 insertions(+) diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c index 0b7d5ecd68..d60457486c 100644 --- a/tests/unit/test-vmstate.c +++ b/tests/unit/test-vmstate.c @@ -1479,6 +1479,118 @@ static void test_tmp_struct(void) g_assert_cmpint(obj.f, ==, 8); /* From the child->parent */ } +static bool sub_optional_needed = true; + +static bool sub_optional_needed_cb(void *opaque) +{ +return sub_optional_needed; +} + +static const VMStateDescription vmstate_sub_optional_a = { +.name = "sub/optional/a", +.version_id = 1, +.minimum_version_id = 1, +.needed = sub_optional_needed_cb, +.fields = (VMStateField[]) { +VMSTATE_END_OF_LIST() +} +}; + +static const VMStateDescription vmstate_sub_optional = { +.name = "sub/optional", +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_END_OF_LIST() +}, +.subsections = (const VMStateDescription * []) { +_sub_optional_a, +} +}; + +static uint8_t wire_sub_optional[] = { +QEMU_VM_SUBSECTION, +14, +'s', 'u', 'b', '/', 'o', 'p', 't', 'i', 'o', 'n', 'a', 'l', '/', 'a', +0x0, 0x0, 0x0, 1, +QEMU_VM_EOF, +}; + +static uint8_t wire_sub_optional_missing[] = { +QEMU_VM_EOF, +}; + +static void test_sub_optional_needed(void) +{ +sub_optional_needed = true; +save_vmstate(_sub_optional, NULL); + +compare_vmstate(wire_sub_optional, sizeof(wire_sub_optional)); + +SUCCESS(load_vmstate_one(_sub_optional, NULL, + 1, wire_sub_optional, + sizeof(wire_sub_optional))); + +/* this will print an error, but succeed nonetheless */ +load_vmstate_one(_sub_optional, NULL, + 1, wire_sub_optional_missing, + sizeof(wire_sub_optional_missing)); +} + +static void test_sub_optional_missing(void) +{ +sub_optional_needed = false; +save_vmstate(_sub_optional, NULL); + +compare_vmstate(wire_sub_optional_missing, sizeof(wire_sub_optional_missing)); + +SUCCESS(load_vmstate_one(_sub_optional, NULL, + 1, wire_sub_optional, + sizeof(wire_sub_optional))); + +SUCCESS(load_vmstate_one(_sub_optional, NULL, + 1, wire_sub_optional_missing, + sizeof(wire_sub_optional_missing))); +} + +static uint8_t wire_sub_dup[] = { +QEMU_VM_SUBSECTION, +14, +'s', 'u', 'b', '/', 'o', 'p', 't', 'i', 'o', 'n', 'a', 'l', '/', 'a', +0x0, 0x0, 0x0, 1, +QEMU_VM_SUBSECTION, +14, +'s', 'u', 'b', '/', 'o', 'p', 't', 'i', 'o', 'n', 'a', 'l', '/', 'a', +0x0, 0x0, 0x0, 1, +QEMU_VM_EOF, +}; + +static void test_sub_optional_dup(void) +{ +sub_optional_needed = false; + +FAILURE(load_vmstate_one(_sub_optional, NULL, + 1, wire_sub_dup, + sizeof(wire_sub_dup))); +} + +static uint8_t wire_sub_unknown[] = { +QEMU_VM_SUBSECTION, +14, +'s', 'u', 'b', '/', 'o', 'p', 't', 'i', 'o', 'n', 'a', 'l', '/', 'b', +0x0, 0x0, 0x0, 1, +QEMU_VM_EOF, +}; + +static void test_sub_optional_unknown(void) +{ +sub_optional_needed = false; + +FAILURE(load_vmstate_one(_sub_optional, NULL, + 1, wire_sub_unknown, + sizeof(wire_sub_unknown))); +} + int main(int argc, char **argv) { g_autofree char *temp_file = g_strdup_printf("%s/vmst.test.XX", @@ -1519,6 +1631,10 @@ int main(int argc, char **argv) g_test_add_func("/vmstate/qlist/save/saveqlist", test_save_qlist); g_test_add_func("/vmstate/qlist/load/loadqlist", test_load_qlist); g_test_add_func("/vmstate/tmp_struct", test_tmp_struct); +g_test_add_func("/vmstate/subsection/needed", test_sub_optional_needed); +g_test_add_func("/vmstate/subsection/missing", test_sub_optional_missing); +g_test_add_func("/vmstate/subsection/dup", test_sub_optional_dup); +g_test_add_func("/vmstate/subsection/unknown", test_sub_optional_unknown); g_test_run(); close(temp_fd); -- 2.41.0
[PATCH v3 6/6] docs/migration: reflect the changes about needed subsections
From: Marc-André Lureau Signed-off-by: Marc-André Lureau Reviewed-by: Juan Quintela --- docs/devel/migration.rst | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index 240eb16d90..22875ac40c 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -246,17 +246,16 @@ a newer form of device, or adding that state that you previously forgot to migrate. This is best solved using a subsection. A subsection is "like" a device vmstate, but with a particularity, it -has a Boolean function that tells if that values are needed to be sent -or not. If this functions returns false, the subsection is not sent. -Subsections have a unique name, that is looked for on the receiving -side. +has a Boolean function that tells if that values are needed or not. If +this functions returns false, the subsection is not sent. Subsections +have a unique name, that is looked for on the receiving side. On the receiving side, if we found a subsection for a device that we -don't understand, we just fail the migration. If we understand all -the subsections, then we load the state with success. There's no check -that a subsection is loaded, so a newer QEMU that knows about a subsection -can (with care) load a stream from an older QEMU that didn't send -the subsection. +don't understand, we just fail the migration. If we understand all the +subsections, then we load the state with success. There's no check +that an optional subsection is loaded, so a newer QEMU that knows +about a subsection can (with care) load a stream from an older QEMU +that didn't send the subsection. If the new data is only needed in a rare case, then the subsection can be made conditional on that case and the migration will still -- 2.41.0
[PATCH v3 0/6] migration: check required entries and sections are loaded
From: Marc-André Lureau Hi, Surprisingly, the migration code doesn't check that required migration entries and subsections are loaded. Either optional or required sections are both ignored when missing. According to the documentation a "newer QEMU that knows about a subsection can (with care) load a stream from an older QEMU that didn't send the subsection". I propose this behaviour to be limited to "optional" sections only. This series has a few preliminary fixes, add new checks that entries are loaded once and required ones have been loaded, add some tests and documentation update. thanks v3: - rebased, drop RFC status - switch from tracepoint + returning an error to report for missing subsections, as we worry about potential regressions - add r-b tags v2: - add "migration: rename vmstate_save_needed->vmstate_section_needed" - add "migration: set file error on subsection loading" - add subsection tests - update the documentation Marc-André Lureau (6): block/fdc: 'phase' is not needed on load virtio: make endian_needed() work during loading migration: check required subsections are loaded, once migration: check required entries are loaded, once test-vmstate: add some subsection tests docs/migration: reflect the changes about needed subsections docs/devel/migration.rst | 17 +++--- hw/block/fdc.c| 5 ++ hw/virtio/virtio.c| 6 +- migration/savevm.c| 43 ++ migration/vmstate.c | 40 - tests/unit/test-vmstate.c | 116 ++ 6 files changed, 215 insertions(+), 12 deletions(-) -- 2.41.0
[PULL 41/60] hw/loader: Clean up global variable shadowing in rom_add_file()
Fix: hw/core/loader.c:1073:27: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] bool option_rom, MemoryRegion *mr, ^ include/sysemu/sysemu.h:57:22: note: previous declaration is here extern QEMUOptionRom option_rom[MAX_OPTION_ROMS]; ^ Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Ani Sinha Message-Id: <20231010115048.11856-3-phi...@linaro.org> --- include/hw/loader.h | 2 +- hw/core/loader.c| 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/hw/loader.h b/include/hw/loader.h index c4c14170ea..8685e27334 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -272,7 +272,7 @@ void pstrcpy_targphys(const char *name, ssize_t rom_add_file(const char *file, const char *fw_dir, hwaddr addr, int32_t bootindex, - bool option_rom, MemoryRegion *mr, AddressSpace *as); + bool has_option_rom, MemoryRegion *mr, AddressSpace *as); MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, size_t max_len, hwaddr addr, const char *fw_file_name, diff --git a/hw/core/loader.c b/hw/core/loader.c index 4dd5a71fb7..7f0cbfb214 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -1070,7 +1070,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name, bool ro) ssize_t rom_add_file(const char *file, const char *fw_dir, hwaddr addr, int32_t bootindex, - bool option_rom, MemoryRegion *mr, + bool has_option_rom, MemoryRegion *mr, AddressSpace *as) { MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); @@ -1139,7 +1139,7 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, basename); snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); -if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) { +if ((!has_option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) { data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true); } else { data = rom->data; -- 2.41.0
[PATCH v3 3/6] migration: check required subsections are loaded, once
From: Marc-André Lureau Check that required subsections have been loaded. Signed-off-by: Marc-André Lureau Reviewed-by: Juan Quintela --- migration/vmstate.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/migration/vmstate.c b/migration/vmstate.c index b7723a4187..9d7a58d26b 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -452,22 +452,51 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, } static const VMStateDescription * -vmstate_get_subsection(const VMStateDescription **sub, char *idstr) +vmstate_get_subsection(const VMStateDescription **sub, char *idstr, bool *visited) { +size_t i = 0; + while (sub && *sub) { if (strcmp(idstr, (*sub)->name) == 0) { +if (visited[i]) { +return NULL; +} +visited[i] = true; return *sub; } +i++; sub++; } return NULL; } +static size_t +vmstate_get_n_subsections(const VMStateDescription **sub) +{ +size_t n = 0; + +if (!sub) { +return 0; +} + +while (sub[n]) { +n++; +} + +return n; +} + static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, void *opaque) { +size_t i, n; +g_autofree bool *visited = NULL; + trace_vmstate_subsection_load(vmsd->name); +n = vmstate_get_n_subsections(vmsd->subsections); +visited = g_new0(bool, n); + while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) { char idstr[256], *idstr_ret; int ret; @@ -493,7 +522,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, /* it doesn't have a valid subsection name */ return 0; } -sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr); +sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr, visited); if (sub_vmsd == NULL) { trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(lookup)"); return -ENOENT; @@ -510,6 +539,13 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, } } +for (i = 0; i < n; i++) { +if (!visited[i] && vmstate_section_needed(vmsd->subsections[i], opaque)) { +error_report("Subsection %s %s missing", + vmsd->name, vmsd->subsections[i]->name); +} +} + trace_vmstate_subsection_load_good(vmsd->name); return 0; } -- 2.41.0
[PULL 11/60] target/ppc: Remove CPU_RESOLVING_TYPE from 'cpu-qom.h'
CPU_RESOLVING_TYPE is a per-target definition, and is irrelevant for other targets. Move it to "cpu.h". "target/ppc/cpu-qom.h" is supposed to be target agnostic (include-able by any target). Add such mention in the header. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20231013140116.255-5-phi...@linaro.org> --- target/ppc/cpu-qom.h | 3 +-- target/ppc/cpu.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index be33786bd8..41df51269b 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -1,5 +1,5 @@ /* - * QEMU PowerPC CPU + * QEMU PowerPC CPU QOM header (target agnostic) * * Copyright (c) 2012 SUSE LINUX Products GmbH * @@ -33,7 +33,6 @@ OBJECT_DECLARE_CPU_TYPE(PowerPCCPU, PowerPCCPUClass, POWERPC_CPU) #define POWERPC_CPU_TYPE_SUFFIX "-" TYPE_POWERPC_CPU #define POWERPC_CPU_TYPE_NAME(model) model POWERPC_CPU_TYPE_SUFFIX -#define CPU_RESOLVING_TYPE TYPE_POWERPC_CPU #define TYPE_HOST_POWERPC_CPU POWERPC_CPU_TYPE_NAME("host") diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 24dd6b1b0a..02619e5d54 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -27,6 +27,8 @@ #include "qom/object.h" #include "hw/registerfields.h" +#define CPU_RESOLVING_TYPE TYPE_POWERPC_CPU + #define TCG_GUEST_DEFAULT_MO 0 #define TARGET_PAGE_BITS_64K 16 -- 2.41.0
[PATCH v3 1/6] block/fdc: 'phase' is not needed on load
From: Marc-André Lureau It is reconstructed during fdc_post_load() Signed-off-by: Marc-André Lureau --- hw/block/fdc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index d7cc4d3ec1..fc71660ba0 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -1005,6 +1005,11 @@ static bool fdc_phase_needed(void *opaque) { FDCtrl *fdctrl = opaque; +/* not needed on load */ +if (fdctrl->phase == FD_PHASE_RECONSTRUCT) { +return false; +} + return reconstruct_phase(fdctrl) != fdctrl->phase; } -- 2.41.0
[PULL 57/60] MAINTAINERS: Add include/hw/timer/tmu012.h to the SH4 R2D section
From: Thomas Huth tmu012.h is the header that belongs to hw/timer/sh_timer.c, so we should list it in the same section as sh_timer.c. Signed-off-by: Thomas Huth Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Yoshinori Sato Message-ID: <20231026080011.156325-1-th...@redhat.com> Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index c01c2e6ec0..3014e768f7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1617,6 +1617,7 @@ F: hw/intc/sh_intc.c F: hw/pci-host/sh_pci.c F: hw/timer/sh_timer.c F: include/hw/sh4/sh_intc.h +F: include/hw/timer/tmu012.h Shix R: Yoshinori Sato -- 2.41.0
[PATCH v3 4/6] migration: check required entries are loaded, once
From: Marc-André Lureau Signed-off-by: Marc-André Lureau Reviewed-by: Juan Quintela --- migration/savevm.c | 43 +++ 1 file changed, 43 insertions(+) diff --git a/migration/savevm.c b/migration/savevm.c index bc98c2ea6f..2ae65b8088 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -217,6 +217,7 @@ typedef struct SaveStateEntry { void *opaque; CompatEntry *compat; int is_ram; +bool visited; } SaveStateEntry; typedef struct SaveState { @@ -1787,6 +1788,36 @@ int qemu_save_device_state(QEMUFile *f) return qemu_file_get_error(f); } +static void savevm_reset_visited(void) +{ +SaveStateEntry *se; + +QTAILQ_FOREACH(se, _state.handlers, entry) { +se->visited = false; +} +} + +static bool loadvm_check_visited(Error **errp) +{ +SaveStateEntry *se; + +QTAILQ_FOREACH(se, _state.handlers, entry) { +if (se->ops && se->ops->is_active && !se->ops->is_active(se->opaque)) { +continue; +} +if (se->vmsd && !vmstate_section_needed(se->vmsd, se->opaque)) { +continue; +} +if (!se->visited) { +error_setg(errp, "Missing entry '%s' while loading VM", se->idstr); +return false; +} +} + +return true; +} + + static SaveStateEntry *find_se(const char *idstr, uint32_t instance_id) { SaveStateEntry *se; @@ -2592,6 +2623,11 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis, idstr, instance_id); return -EINVAL; } +if (se->visited) { +error_report("error while loading state for instance 0x%"PRIx32" of" + " device '%s'", instance_id, idstr); +return -EINVAL; +} /* Validate version */ if (version_id > se->version_id) { @@ -2629,6 +2665,8 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis, return -EINVAL; } +se->visited = true; + return 0; } @@ -2950,7 +2988,12 @@ int qemu_loadvm_state(QEMUFile *f) cpu_synchronize_all_pre_loadvm(); +savevm_reset_visited(); ret = qemu_loadvm_state_main(f, mis); +if (!loadvm_check_visited(_err)) { +error_report_err(local_err); +return -EINVAL; +} qemu_event_set(>main_thread_load_event); trace_qemu_loadvm_state_post_main(ret); -- 2.41.0
[PULL 60/60] ui/sdl2: use correct key names in win title on mac
From: Adrian Wowk Previously, when using the SDL2 UI on MacOS, the title bar uses incorrect key names (such as Ctrl and Alt instead of the standard MacOS key symbols like ⌃ and ⌥). This commit changes sdl_update_caption in ui/sdl2.c to use the correct symbols when compiling for MacOS (CONFIG_DARWIN is defined). Unfortunately, standard Mac keyboards do not include a "Right-Ctrl" key, so in the case that the SDL grab mode is set to HOT_KEY_MOD_RCTRL, the default text is still used. Signed-off-by: Adrian Wowk Acked-by: Marc-André Lureau Tested-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20231030024119.28342-1-...@adrianwowk.com> Signed-off-by: Philippe Mathieu-Daudé --- ui/sdl2.c | 8 1 file changed, 8 insertions(+) diff --git a/ui/sdl2.c b/ui/sdl2.c index fbfdb64e90..4971963f00 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -172,11 +172,19 @@ static void sdl_update_caption(struct sdl2_console *scon) status = " [Stopped]"; } else if (gui_grab) { if (alt_grab) { +#ifdef CONFIG_DARWIN +status = " - Press ⌃⌥⇧G to exit grab"; +#else status = " - Press Ctrl-Alt-Shift-G to exit grab"; +#endif } else if (ctrl_grab) { status = " - Press Right-Ctrl-G to exit grab"; } else { +#ifdef CONFIG_DARWIN +status = " - Press ⌃⌥G to exit grab"; +#else status = " - Press Ctrl-Alt-G to exit grab"; +#endif } } -- 2.41.0
[PULL 49/60] hw/i2c: pmbus add support for block receive
From: Titus Rwantare PMBus devices can send and receive variable length data using the block read and write format, with the first byte in the payload denoting the length. This is mostly used for strings and on-device logs. Devices can respond to a block read with an empty string. Reviewed-by: Hao Wu Acked-by: Corey Minyard Signed-off-by: Titus Rwantare Message-ID: <20231023-staging-pmbus-v3-v4-1-07a8cb7cd...@google.com> Signed-off-by: Philippe Mathieu-Daudé --- include/hw/i2c/pmbus_device.h | 7 +++ hw/i2c/pmbus_device.c | 30 +- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h index 93f5d57c9d..7dc00cc4d9 100644 --- a/include/hw/i2c/pmbus_device.h +++ b/include/hw/i2c/pmbus_device.h @@ -501,6 +501,13 @@ void pmbus_send64(PMBusDevice *state, uint64_t data); */ void pmbus_send_string(PMBusDevice *state, const char *data); +/** + * @brief Receive data sent with Block Write. + * @param dest - memory with enough capacity to receive the write + * @param len - the capacity of dest + */ +uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len); + /** * @brief Receive data over PMBus * These methods help track how much data is being received over PMBus diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c index cef51663d0..ea15490720 100644 --- a/hw/i2c/pmbus_device.c +++ b/hw/i2c/pmbus_device.c @@ -102,7 +102,6 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data) } size_t len = strlen(data); -g_assert(len > 0); g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN); pmdev->out_buf[len + pmdev->out_buf_len] = len; @@ -112,6 +111,35 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data) pmdev->out_buf_len += len + 1; } +uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len) +{ +/* dest may contain data from previous writes */ +memset(dest, 0, len); + +/* Exclude command code from return value */ +pmdev->in_buf++; +pmdev->in_buf_len--; + +/* The byte after the command code denotes the length */ +uint8_t sent_len = pmdev->in_buf[0]; + +if (sent_len != pmdev->in_buf_len - 1) { +qemu_log_mask(LOG_GUEST_ERROR, + "%s: length mismatch. Expected %d bytes, got %d bytes\n", + __func__, sent_len, pmdev->in_buf_len - 1); +} + +/* exclude length byte */ +pmdev->in_buf++; +pmdev->in_buf_len--; + +if (pmdev->in_buf_len < len) { +len = pmdev->in_buf_len; +} +memcpy(dest, pmdev->in_buf, len); +return len; +} + static uint64_t pmbus_receive_uint(PMBusDevice *pmdev) { -- 2.41.0
[PULL 40/60] hw/cpu: Clean up global variable shadowing
Fix: hw/core/machine.c:1302:22: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] const CPUArchId *cpus = possible_cpus->cpus; ^ hw/core/numa.c:69:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] uint16List *cpus = NULL; ^ hw/acpi/aml-build.c:2005:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] CPUArchIdList *cpus = ms->possible_cpus; ^ hw/core/machine-smp.c:77:14: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] unsigned cpus= config->has_cpus ? config->cpus : 0; ^ include/hw/core/cpu.h:589:17: note: previous declaration is here extern CPUTailQ cpus; ^ Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Ani Sinha Message-Id: <20231010115048.11856-2-phi...@linaro.org> --- include/hw/core/cpu.h | 8 cpu-common.c | 6 +++--- linux-user/main.c | 2 +- target/s390x/cpu_models.c | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index eb943efb8f..77893d7b81 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -586,13 +586,13 @@ static inline CPUArchState *cpu_env(CPUState *cpu) } typedef QTAILQ_HEAD(CPUTailQ, CPUState) CPUTailQ; -extern CPUTailQ cpus; +extern CPUTailQ cpus_queue; -#define first_cpuQTAILQ_FIRST_RCU() +#define first_cpuQTAILQ_FIRST_RCU(_queue) #define CPU_NEXT(cpu)QTAILQ_NEXT_RCU(cpu, node) -#define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, , node) +#define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, _queue, node) #define CPU_FOREACH_SAFE(cpu, next_cpu) \ -QTAILQ_FOREACH_SAFE_RCU(cpu, , node, next_cpu) +QTAILQ_FOREACH_SAFE_RCU(cpu, _queue, node, next_cpu) extern __thread CPUState *current_cpu; diff --git a/cpu-common.c b/cpu-common.c index 45c745ecf6..c81fd72d16 100644 --- a/cpu-common.c +++ b/cpu-common.c @@ -73,7 +73,7 @@ static int cpu_get_free_index(void) return max_cpu_index; } -CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus); +CPUTailQ cpus_queue = QTAILQ_HEAD_INITIALIZER(cpus_queue); static unsigned int cpu_list_generation_id; unsigned int cpu_list_generation_id_get(void) @@ -90,7 +90,7 @@ void cpu_list_add(CPUState *cpu) } else { assert(!cpu_index_auto_assigned); } -QTAILQ_INSERT_TAIL_RCU(, cpu, node); +QTAILQ_INSERT_TAIL_RCU(_queue, cpu, node); cpu_list_generation_id++; } @@ -102,7 +102,7 @@ void cpu_list_remove(CPUState *cpu) return; } -QTAILQ_REMOVE_RCU(, cpu, node); +QTAILQ_REMOVE_RCU(_queue, cpu, node); cpu->cpu_index = UNASSIGNED_CPU_INDEX; cpu_list_generation_id++; } diff --git a/linux-user/main.c b/linux-user/main.c index 0c23584a96..0cdaf30d34 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -156,7 +156,7 @@ void fork_end(int child) Discard information about the parent threads. */ CPU_FOREACH_SAFE(cpu, next_cpu) { if (cpu != thread_cpu) { -QTAILQ_REMOVE_RCU(, cpu, node); +QTAILQ_REMOVE_RCU(_queue, cpu, node); } } qemu_init_cpu_list(); diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 4dead48650..5c455d00c0 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -757,7 +757,7 @@ void s390_set_qemu_cpu_model(uint16_t type, uint8_t gen, uint8_t ec_ga, const S390CPUDef *def = s390_find_cpu_def(type, gen, ec_ga, NULL); g_assert(def); -g_assert(QTAILQ_EMPTY_RCU()); +g_assert(QTAILQ_EMPTY_RCU(_queue)); /* build the CPU model */ s390_qemu_cpu_model.def = def; -- 2.41.0
[PULL 12/60] target/riscv: Remove CPU_RESOLVING_TYPE from 'cpu-qom.h'
CPU_RESOLVING_TYPE is a per-target definition, and is irrelevant for other targets. Move it to "cpu.h". Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: LIU Zhiwei Reviewed-by: Richard Henderson Message-Id: <20231013140116.255-6-phi...@linaro.org> --- target/riscv/cpu-qom.h | 1 - target/riscv/cpu.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h index b9164a8e5b..b78169093f 100644 --- a/target/riscv/cpu-qom.h +++ b/target/riscv/cpu-qom.h @@ -27,7 +27,6 @@ #define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU #define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX) -#define CPU_RESOLVING_TYPE TYPE_RISCV_CPU #define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any") #define TYPE_RISCV_CPU_MAX RISCV_CPU_TYPE_NAME("max") diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index f0dc257a75..144cc94cce 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -32,6 +32,8 @@ #include "qapi/qapi-types-common.h" #include "cpu-qom.h" +#define CPU_RESOLVING_TYPE TYPE_RISCV_CPU + #define TCG_GUEST_DEFAULT_MO 0 /* -- 2.41.0
[PULL 08/60] target: Unify QOM style
Enforce the style described by commit 067109a11c ("docs/devel: mention the spacing requirement for QOM"): The first declaration of a storage or class structure should always be the parent and leave a visual space between that declaration and the new code. It is also useful to separate backing for properties (options driven by the user) and internal state to make navigation easier. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: Zhao Liu Message-Id: <20231013140116.255-2-phi...@linaro.org> --- target/alpha/cpu-qom.h | 2 -- target/alpha/cpu.h | 2 -- target/arm/cpu-qom.h| 4 target/arm/cpu.h| 2 -- target/avr/cpu-qom.h| 3 +-- target/avr/cpu.h| 2 -- target/cris/cpu-qom.h | 2 -- target/cris/cpu.h | 2 -- target/hexagon/cpu.h| 5 + target/hppa/cpu-qom.h | 2 -- target/hppa/cpu.h | 2 -- target/i386/cpu-qom.h | 2 -- target/i386/cpu.h | 2 -- target/loongarch/cpu.h | 4 target/m68k/cpu-qom.h | 2 -- target/m68k/cpu.h | 2 -- target/microblaze/cpu-qom.h | 2 -- target/microblaze/cpu.h | 2 -- target/mips/cpu-qom.h | 2 -- target/mips/cpu.h | 2 -- target/nios2/cpu.h | 4 target/openrisc/cpu.h | 4 target/ppc/cpu.h| 2 -- target/riscv/cpu-qom.h | 3 +-- target/riscv/cpu.h | 2 -- target/rx/cpu-qom.h | 2 -- target/rx/cpu.h | 2 -- target/s390x/cpu-qom.h | 3 +-- target/s390x/cpu.h | 2 -- target/sh4/cpu-qom.h| 2 -- target/sh4/cpu.h| 2 -- target/sparc/cpu-qom.h | 2 -- target/sparc/cpu.h | 2 -- target/tricore/cpu-qom.h| 2 -- target/tricore/cpu.h| 2 -- target/xtensa/cpu-qom.h | 2 -- target/xtensa/cpu.h | 2 -- 37 files changed, 4 insertions(+), 84 deletions(-) diff --git a/target/alpha/cpu-qom.h b/target/alpha/cpu-qom.h index 1f200724b6..c5fbd8f11a 100644 --- a/target/alpha/cpu-qom.h +++ b/target/alpha/cpu-qom.h @@ -35,9 +35,7 @@ OBJECT_DECLARE_CPU_TYPE(AlphaCPU, AlphaCPUClass, ALPHA_CPU) * An Alpha CPU model. */ struct AlphaCPUClass { -/*< private >*/ CPUClass parent_class; -/*< public >*/ DeviceRealize parent_realize; DeviceReset parent_reset; diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h index e2a467ec17..c8d97ac27a 100644 --- a/target/alpha/cpu.h +++ b/target/alpha/cpu.h @@ -259,9 +259,7 @@ typedef struct CPUArchState { * An Alpha CPU. */ struct ArchCPU { -/*< private >*/ CPUState parent_obj; -/*< public >*/ CPUAlphaState env; diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h index d06c08a734..153865d1bb 100644 --- a/target/arm/cpu-qom.h +++ b/target/arm/cpu-qom.h @@ -46,9 +46,7 @@ void aarch64_cpu_register(const ARMCPUInfo *info); * An ARM CPU model. */ struct ARMCPUClass { -/*< private >*/ CPUClass parent_class; -/*< public >*/ const ARMCPUInfo *info; DeviceRealize parent_realize; @@ -62,9 +60,7 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, TYPE_AARCH64_CPU) struct AArch64CPUClass { -/*< private >*/ ARMCPUClass parent_class; -/*< public >*/ }; void register_cp_regs_for_features(ARMCPU *cpu); diff --git a/target/arm/cpu.h b/target/arm/cpu.h index d51dfe48db..2f7ab22169 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -852,9 +852,7 @@ typedef struct { * An ARM CPU core. */ struct ArchCPU { -/*< private >*/ CPUState parent_obj; -/*< public >*/ CPUARMState env; diff --git a/target/avr/cpu-qom.h b/target/avr/cpu-qom.h index 01ea5f160b..d89be01e0f 100644 --- a/target/avr/cpu-qom.h +++ b/target/avr/cpu-qom.h @@ -36,9 +36,8 @@ OBJECT_DECLARE_CPU_TYPE(AVRCPU, AVRCPUClass, AVR_CPU) * A AVR CPU model. */ struct AVRCPUClass { -/*< private >*/ CPUClass parent_class; -/*< public >*/ + DeviceRealize parent_realize; ResettablePhases parent_phases; }; diff --git a/target/avr/cpu.h b/target/avr/cpu.h index 4ce22d8e4f..f8b065ed79 100644 --- a/target/avr/cpu.h +++ b/target/avr/cpu.h @@ -144,9 +144,7 @@ typedef struct CPUArchState { * A AVR CPU. */ struct ArchCPU { -/*< private >*/ CPUState parent_obj; -/*< public >*/ CPUAVRState env; }; diff --git a/target/cris/cpu-qom.h b/target/cris/cpu-qom.h index 431a1d536a..c2fee242f4 100644 --- a/target/cris/cpu-qom.h +++ b/target/cris/cpu-qom.h @@ -36,9 +36,7 @@ OBJECT_DECLARE_CPU_TYPE(CRISCPU, CRISCPUClass, CRIS_CPU) * A CRIS CPU model. */ struct CRISCPUClass { -/*< private >*/ CPUClass parent_class; -/*< public >*/ DeviceRealize parent_realize; ResettablePhases parent_phases; diff --git a/target/cris/cpu.h b/target/cris/cpu.h index 676b8e93ca..6aa445348f 100644 --- a/target/cris/cpu.h +++ b/target/cris/cpu.h @@ -174,9 +174,7 @@ typedef struct
[PULL 09/60] target: Mention 'cpu-qom.h' is target agnostic
"target/foo/cpu-qom.h" is supposed to be target agnostic (include-able by any target). Add such mention in the header. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20231013140116.255-3-phi...@linaro.org> --- target/arm/cpu-qom.h| 2 +- target/hppa/cpu-qom.h | 2 +- target/microblaze/cpu-qom.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h index 153865d1bb..dfb9d5b827 100644 --- a/target/arm/cpu-qom.h +++ b/target/arm/cpu-qom.h @@ -1,5 +1,5 @@ /* - * QEMU ARM CPU + * QEMU ARM CPU QOM header (target agnostic) * * Copyright (c) 2012 SUSE LINUX Products GmbH * diff --git a/target/hppa/cpu-qom.h b/target/hppa/cpu-qom.h index 67f12422c4..4b1d48f7ca 100644 --- a/target/hppa/cpu-qom.h +++ b/target/hppa/cpu-qom.h @@ -1,5 +1,5 @@ /* - * QEMU HPPA CPU + * QEMU HPPA CPU QOM header (target agnostic) * * Copyright (c) 2016 Richard Henderson * diff --git a/target/microblaze/cpu-qom.h b/target/microblaze/cpu-qom.h index 2a734e644d..78f549b57d 100644 --- a/target/microblaze/cpu-qom.h +++ b/target/microblaze/cpu-qom.h @@ -1,5 +1,5 @@ /* - * QEMU MicroBlaze CPU + * QEMU MicroBlaze CPU QOM header (target agnostic) * * Copyright (c) 2012 SUSE LINUX Products GmbH * -- 2.41.0
[PULL 31/60] target/mips: Fix TX79 LQ/SQ opcodes
The base register address offset is *signed*. Cc: qemu-sta...@nongnu.org Fixes: 82a9f9 ("target/mips/tx79: Introduce LQ opcode (Load Quadword)") Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20230914090447.12557-1-phi...@linaro.org> --- target/mips/tcg/tx79.decode | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/mips/tcg/tx79.decode b/target/mips/tcg/tx79.decode index 57d87a2076..578b8c54c0 100644 --- a/target/mips/tcg/tx79.decode +++ b/target/mips/tcg/tx79.decode @@ -24,7 +24,7 @@ @rs .. rs:5 . .. ..sa=0 rt=0 rd=0 @rd .. .. rd:5 . ..sa=0 rs=0 rt=0 -@ldst.. base:5 rt:5 offset:16 +@ldst.. base:5 rt:5 offset:s16 ### -- 2.41.0
[PULL 51/60] hw/i2c: pmbus: add fan support
From: Titus Rwantare PMBus devices may integrate fans whose operation is configurable over PMBus. This commit allows the driver to read and write the fan control registers but does not model the operation of fans. Reviewed-by: Stephen Longfield Acked-by: Corey Minyard Signed-off-by: Titus Rwantare Message-ID: <20231023-staging-pmbus-v3-v4-3-07a8cb7cd...@google.com> Signed-off-by: Philippe Mathieu-Daudé --- include/hw/i2c/pmbus_device.h | 1 + hw/i2c/pmbus_device.c | 176 ++ 2 files changed, 177 insertions(+) diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h index 2e95164aa1..ad431bdc7c 100644 --- a/include/hw/i2c/pmbus_device.h +++ b/include/hw/i2c/pmbus_device.h @@ -258,6 +258,7 @@ OBJECT_DECLARE_TYPE(PMBusDevice, PMBusDeviceClass, #define PB_HAS_TEMP2 BIT_ULL(41) #define PB_HAS_TEMP3 BIT_ULL(42) #define PB_HAS_TEMP_RATING BIT_ULL(43) +#define PB_HAS_FAN BIT_ULL(44) #define PB_HAS_MFR_INFOBIT_ULL(50) #define PB_HAS_STATUS_MFR_SPECIFIC BIT_ULL(51) diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c index ea15490720..c1d8c93056 100644 --- a/hw/i2c/pmbus_device.c +++ b/hw/i2c/pmbus_device.c @@ -500,6 +500,54 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd) } break; +case PMBUS_FAN_CONFIG_1_2:/* R/W byte */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send8(pmdev, pmdev->pages[index].fan_config_1_2); +} else { +goto passthough; +} +break; + +case PMBUS_FAN_COMMAND_1: /* R/W word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].fan_command_1); +} else { +goto passthough; +} +break; + +case PMBUS_FAN_COMMAND_2: /* R/W word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].fan_command_2); +} else { +goto passthough; +} +break; + +case PMBUS_FAN_CONFIG_3_4:/* R/W byte */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send8(pmdev, pmdev->pages[index].fan_config_3_4); +} else { +goto passthough; +} +break; + +case PMBUS_FAN_COMMAND_3: /* R/W word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].fan_command_3); +} else { +goto passthough; +} +break; + +case PMBUS_FAN_COMMAND_4: /* R/W word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].fan_command_4); +} else { +goto passthough; +} +break; + case PMBUS_VOUT_OV_FAULT_LIMIT: /* R/W word */ if (pmdev->pages[index].page_flags & PB_HAS_VOUT) { pmbus_send16(pmdev, pmdev->pages[index].vout_ov_fault_limit); @@ -810,6 +858,22 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd) pmbus_send8(pmdev, pmdev->pages[index].status_mfr_specific); break; +case PMBUS_STATUS_FANS_1_2: /* R/W byte */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send8(pmdev, pmdev->pages[index].status_fans_1_2); +} else { +goto passthough; +} +break; + +case PMBUS_STATUS_FANS_3_4: /* R/W byte */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send8(pmdev, pmdev->pages[index].status_fans_3_4); +} else { +goto passthough; +} +break; + case PMBUS_READ_EIN: /* Read-Only block 5 bytes */ if (pmdev->pages[index].page_flags & PB_HAS_EIN) { pmbus_send(pmdev, pmdev->pages[index].read_ein, 5); @@ -882,6 +946,54 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd) } break; +case PMBUS_READ_FAN_SPEED_1: /* Read-Only word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_1); +} else { +goto passthough; +} +break; + +case PMBUS_READ_FAN_SPEED_2: /* Read-Only word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_2); +} else { +goto passthough; +} +break; + +case PMBUS_READ_FAN_SPEED_3: /* Read-Only word */ +if (pmdev->pages[index].page_flags & PB_HAS_FAN) { +pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_3); +} else { +goto passthough; +} +break; + +case PMBUS_READ_FAN_SPEED_4: /*
[PULL 58/60] MAINTAINERS: Add the CAN documentation file to the CAN section
From: Thomas Huth Add can.rst to the corresponding section in MAINTAINERS, so that the maintainers get CC:-ed on corresponding patches. Signed-off-by: Thomas Huth Reviewed-by: Vikram Garhwal Reviewed-by: Philippe Mathieu-Daudé Acked-by: Pavel Pisa Message-ID: <20231027060931.242491-1-th...@redhat.com> [PMD: Fixed typo in subject] Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3014e768f7..c57868c94c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2588,6 +2588,7 @@ W: https://canbus.pages.fel.cvut.cz/ F: net/can/* F: hw/net/can/* F: include/net/can_*.h +F: docs/system/devices/can.rst OpenPIC interrupt controller M: Mark Cave-Ayland -- 2.41.0
[PULL 55/60] hw/i2c: pmbus: immediately clear faults on request
From: Titus Rwantare The probing process of the generic pmbus driver generates faults to determine if functions are available. These faults were not always cleared resulting in probe failures. Reviewed-by: Patrick Venture Signed-off-by: Titus Rwantare Message-ID: <20231023-staging-pmbus-v3-v4-7-07a8cb7cd...@google.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/i2c/pmbus_device.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c index 3bce39e84e..481e158380 100644 --- a/hw/i2c/pmbus_device.c +++ b/hw/i2c/pmbus_device.c @@ -1244,6 +1244,11 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len) pmdev->in_buf = buf; pmdev->code = buf[0]; /* PMBus command code */ + +if (pmdev->code == PMBUS_CLEAR_FAULTS) { +pmbus_clear_faults(pmdev); +} + if (len == 1) { /* Single length writes are command codes only */ return 0; } -- 2.41.0
[PULL 35/60] target/ppc: Prohibit target specific KVM prototypes on user emulation
None of these target-specific prototypes should be used by user emulation. Remove their declaration there, so we get a compile failure if ever used (instead of having to deal with linker and its possible optimizations, such dead code removal). Suggested-by: Kevin Wolf Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Daniel Henrique Barboza Message-Id: <20231003070427.69621-5-phi...@linaro.org> --- target/ppc/kvm_ppc.h | 4 1 file changed, 4 insertions(+) diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h index 6a4dd9c560..1975fb5ee6 100644 --- a/target/ppc/kvm_ppc.h +++ b/target/ppc/kvm_ppc.h @@ -13,6 +13,10 @@ #include "exec/hwaddr.h" #include "cpu.h" +#ifdef CONFIG_USER_ONLY +#error Cannot include kvm_ppc.h from user emulation +#endif + #ifdef CONFIG_KVM uint32_t kvmppc_get_tbfreq(void); -- 2.41.0
[PULL 36/60] target/nios2: Create IRQs *after* accelerator vCPU is realized
Architecture specific hardware doesn't have a particular dependency on the accelerator vCPU (created with cpu_exec_realizefn), and can be initialized *after* the vCPU is realized. Doing so allows further generic API simplification (in few commits). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20230918160257.30127-12-phi...@linaro.org> --- target/nios2/cpu.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c index 15e499f828..a27732bf2b 100644 --- a/target/nios2/cpu.c +++ b/target/nios2/cpu.c @@ -199,14 +199,6 @@ static void nios2_cpu_realizefn(DeviceState *dev, Error **errp) Nios2CPUClass *ncc = NIOS2_CPU_GET_CLASS(dev); Error *local_err = NULL; -#ifndef CONFIG_USER_ONLY -if (cpu->eic_present) { -qdev_init_gpio_in_named(DEVICE(cpu), eic_set_irq, "EIC", 1); -} else { -qdev_init_gpio_in_named(DEVICE(cpu), iic_set_irq, "IRQ", 32); -} -#endif - cpu_exec_realizefn(cs, _err); if (local_err != NULL) { error_propagate(errp, local_err); @@ -220,6 +212,14 @@ static void nios2_cpu_realizefn(DeviceState *dev, Error **errp) /* We have reserved storage for cpuid; might as well use it. */ cpu->env.ctrl[CR_CPUID] = cs->cpu_index; +#ifndef CONFIG_USER_ONLY +if (cpu->eic_present) { +qdev_init_gpio_in_named(DEVICE(cpu), eic_set_irq, "EIC", 1); +} else { +qdev_init_gpio_in_named(DEVICE(cpu), iic_set_irq, "IRQ", 32); +} +#endif + ncc->parent_realize(dev, errp); } -- 2.41.0
[PULL 33/60] hw/ppc/e500: Restrict ppce500_init_mpic_kvm() to KVM
Inline and guard the single call to kvm_openpic_connect_vcpu() allows to remove kvm-stub.c. Reviewed-by: Michael Tokarev Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20231003070427.69621-3-phi...@linaro.org> --- hw/ppc/e500.c | 4 target/ppc/kvm-stub.c | 19 --- target/ppc/meson.build | 2 +- 3 files changed, 5 insertions(+), 20 deletions(-) delete mode 100644 target/ppc/kvm-stub.c diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index e04114fb3c..384226296b 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -834,6 +834,7 @@ static DeviceState *ppce500_init_mpic_qemu(PPCE500MachineState *pms, static DeviceState *ppce500_init_mpic_kvm(const PPCE500MachineClass *pmc, IrqLines *irqs, Error **errp) { +#ifdef CONFIG_KVM DeviceState *dev; CPUState *cs; @@ -854,6 +855,9 @@ static DeviceState *ppce500_init_mpic_kvm(const PPCE500MachineClass *pmc, } return dev; +#else +g_assert_not_reached(); +#endif } static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms, diff --git a/target/ppc/kvm-stub.c b/target/ppc/kvm-stub.c deleted file mode 100644 index b98e1d404f..00 --- a/target/ppc/kvm-stub.c +++ /dev/null @@ -1,19 +0,0 @@ -/* - * QEMU KVM PPC specific function stubs - * - * Copyright Freescale Inc. 2013 - * - * Author: Alexander Graf - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - * - */ -#include "qemu/osdep.h" -#include "cpu.h" -#include "hw/ppc/openpic_kvm.h" - -int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs) -{ -return -EINVAL; -} diff --git a/target/ppc/meson.build b/target/ppc/meson.build index 97ceb6e7c0..eab4e3e1b3 100644 --- a/target/ppc/meson.build +++ b/target/ppc/meson.build @@ -30,7 +30,7 @@ gen = [ ] ppc_ss.add(when: 'CONFIG_TCG', if_true: gen) -ppc_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c')) +ppc_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c')) ppc_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user_only_helper.c')) ppc_system_ss = ss.source_set() -- 2.41.0
[PULL 42/60] hw/isa/i82378: Propagate error if PC_SPEAKER device creation failed
In commit 40f8214fcd ("hw/audio/pcspk: Inline pcspk_init()") we neglected to give a change to the caller to handle failed device creation cleanly. Respect the caller API contract and propagate the error if creating the PC_SPEAKER device ever failed. This avoid yet another bad API use to be taken as example and copy / pasted all over the code base. Reported-by: Bernhard Beschow Suggested-by: Markus Armbruster Reviewed-by: Richard Henderson Reviewed-by: Bernhard Beschow Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20231020171509.87839-5-phi...@linaro.org> --- hw/isa/i82378.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c index 79ffbb52a0..203b92c264 100644 --- a/hw/isa/i82378.c +++ b/hw/isa/i82378.c @@ -105,7 +105,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp) /* speaker */ pcspk = isa_new(TYPE_PC_SPEAKER); object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), _fatal); -isa_realize_and_unref(pcspk, isabus, _fatal); +if (!isa_realize_and_unref(pcspk, isabus, errp)) { +return; +} /* 2 82C37 (dma) */ isa_create_simple(isabus, "i82374"); -- 2.41.0
[PULL 37/60] target/alpha: Tidy up alpha_cpu_class_by_name()
Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: Gavin Shan Message-Id: <20230908112235.75914-2-phi...@linaro.org> --- target/alpha/cpu.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c index 51b7d8d1bf..fae2cb6ec7 100644 --- a/target/alpha/cpu.c +++ b/target/alpha/cpu.c @@ -142,13 +142,10 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model) typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model); oc = object_class_by_name(typename); g_free(typename); -if (oc != NULL && object_class_is_abstract(oc)) { -oc = NULL; -} /* TODO: remove match everything nonsense */ -/* Default to ev67; no reason not to emulate insns by default. */ -if (!oc) { +if (!oc || object_class_is_abstract(oc)) { +/* Default to ev67; no reason not to emulate insns by default. */ oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67")); } -- 2.41.0
[PULL 18/60] target/riscv: Move TYPE_RISCV_CPU_BASE definition to 'cpu.h'
TYPE_RISCV_CPU_BASE depends on the TARGET_RISCV32/TARGET_RISCV64 definitions which are target specific. Such target specific definition taints "cpu-qom.h". Since "cpu-qom.h" must be target agnostic, remove its target specific definition uses by moving TYPE_RISCV_CPU_BASE to "target/riscv/cpu.h". "target/riscv/cpu-qom.h" is now fully target agnostic. Add a comment clarifying that in the header. Reviewed-by: LIU Zhiwei Reviewed-by: Alistair Francis Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20231013140116.255-12-phi...@linaro.org> --- target/riscv/cpu-qom.h | 8 +--- target/riscv/cpu.h | 6 ++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h index b78169093f..76efb614a6 100644 --- a/target/riscv/cpu-qom.h +++ b/target/riscv/cpu-qom.h @@ -1,5 +1,5 @@ /* - * QEMU RISC-V CPU QOM header + * QEMU RISC-V CPU QOM header (target agnostic) * * Copyright (c) 2023 Ventana Micro Systems Inc. * @@ -44,12 +44,6 @@ #define TYPE_RISCV_CPU_VEYRON_V1RISCV_CPU_TYPE_NAME("veyron-v1") #define TYPE_RISCV_CPU_HOST RISCV_CPU_TYPE_NAME("host") -#if defined(TARGET_RISCV32) -# define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE32 -#elif defined(TARGET_RISCV64) -# define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE64 -#endif - typedef struct CPUArchState CPURISCVState; OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass, RISCV_CPU) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 144cc94cce..d832696418 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -34,6 +34,12 @@ #define CPU_RESOLVING_TYPE TYPE_RISCV_CPU +#if defined(TARGET_RISCV32) +# define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE32 +#elif defined(TARGET_RISCV64) +# define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE64 +#endif + #define TCG_GUEST_DEFAULT_MO 0 /* -- 2.41.0
[PULL 48/60] tests/qtest: ahci-test: add test exposing reset issue with pending callback
From: Fiona Ebner Before commit "hw/ide: reset: cancel async DMA operation before resetting state", this test would fail, because a reset with a pending write operation would lead to an unsolicited write to the first sector of the disk. The test writes a pattern to the beginning of the disk and verifies that it is still intact after a reset with a pending operation. It also checks that the pending operation actually completes correctly. Signed-off-by: Fiona Ebner Message-ID: <20230906130922.142845-2-f.eb...@proxmox.com> Signed-off-by: Philippe Mathieu-Daudé --- tests/qtest/ahci-test.c | 86 - 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c index eea8b5f77b..5a1923f721 100644 --- a/tests/qtest/ahci-test.c +++ b/tests/qtest/ahci-test.c @@ -1424,6 +1424,89 @@ static void test_reset(void) ahci_shutdown(ahci); } +static void test_reset_pending_callback(void) +{ +AHCIQState *ahci; +AHCICommand *cmd; +uint8_t port; +uint64_t ptr1; +uint64_t ptr2; + +int bufsize = 4 * 1024; +int speed = bufsize + (bufsize / 2); +int offset1 = 0; +int offset2 = bufsize / AHCI_SECTOR_SIZE; + +g_autofree unsigned char *tx1 = g_malloc(bufsize); +g_autofree unsigned char *tx2 = g_malloc(bufsize); +g_autofree unsigned char *rx1 = g_malloc0(bufsize); +g_autofree unsigned char *rx2 = g_malloc0(bufsize); + +/* Uses throttling to make test independent of specific environment. */ +ahci = ahci_boot_and_enable("-drive if=none,id=drive0,file=%s," +"cache=writeback,format=%s," +"throttling.bps-write=%d " +"-M q35 " +"-device ide-hd,drive=drive0 ", +tmp_path, imgfmt, speed); + +port = ahci_port_select(ahci); +ahci_port_clear(ahci, port); + +ptr1 = ahci_alloc(ahci, bufsize); +ptr2 = ahci_alloc(ahci, bufsize); + +g_assert(ptr1 && ptr2); + +/* Need two different patterns. */ +do { +generate_pattern(tx1, bufsize, AHCI_SECTOR_SIZE); +generate_pattern(tx2, bufsize, AHCI_SECTOR_SIZE); +} while (memcmp(tx1, tx2, bufsize) == 0); + +qtest_bufwrite(ahci->parent->qts, ptr1, tx1, bufsize); +qtest_bufwrite(ahci->parent->qts, ptr2, tx2, bufsize); + +/* Write to beginning of disk to check it wasn't overwritten later. */ +ahci_guest_io(ahci, port, CMD_WRITE_DMA_EXT, ptr1, bufsize, offset1); + +/* Issue asynchronously to get a pending callback during reset. */ +cmd = ahci_command_create(CMD_WRITE_DMA_EXT); +ahci_command_adjust(cmd, offset2, ptr2, bufsize, 0); +ahci_command_commit(ahci, cmd, port); +ahci_command_issue_async(ahci, cmd); + +ahci_set(ahci, AHCI_GHC, AHCI_GHC_HR); + +ahci_command_free(cmd); + +/* Wait for throttled write to finish. */ +sleep(1); + +/* Start again. */ +ahci_clean_mem(ahci); +ahci_pci_enable(ahci); +ahci_hba_enable(ahci); +port = ahci_port_select(ahci); +ahci_port_clear(ahci, port); + +/* Read and verify. */ +ahci_guest_io(ahci, port, CMD_READ_DMA_EXT, ptr1, bufsize, offset1); +qtest_bufread(ahci->parent->qts, ptr1, rx1, bufsize); +g_assert_cmphex(memcmp(tx1, rx1, bufsize), ==, 0); + +ahci_guest_io(ahci, port, CMD_READ_DMA_EXT, ptr2, bufsize, offset2); +qtest_bufread(ahci->parent->qts, ptr2, rx2, bufsize); +g_assert_cmphex(memcmp(tx2, rx2, bufsize), ==, 0); + +ahci_free(ahci, ptr1); +ahci_free(ahci, ptr2); + +ahci_clean_mem(ahci); + +ahci_shutdown(ahci); +} + static void test_ncq_simple(void) { AHCIQState *ahci; @@ -1945,7 +2028,8 @@ int main(int argc, char **argv) qtest_add_func("/ahci/migrate/dma/halted", test_migrate_halted_dma); qtest_add_func("/ahci/max", test_max); -qtest_add_func("/ahci/reset", test_reset); +qtest_add_func("/ahci/reset/simple", test_reset); +qtest_add_func("/ahci/reset/pending_callback", test_reset_pending_callback); qtest_add_func("/ahci/io/ncq/simple", test_ncq_simple); qtest_add_func("/ahci/migrate/ncq/simple", test_migrate_ncq); -- 2.41.0
[PULL 39/60] exec/cpu: Have cpu_exec_realize() return a boolean
Following the example documented since commit e3fe3988d7 ("error: Document Error API usage rules"), have cpu_exec_realizefn() return a boolean indicating whether an error is set or not. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20230918160257.30127-22-phi...@linaro.org> --- include/hw/core/cpu.h | 2 +- cpu-target.c | 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 5d6f8dca43..eb943efb8f 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1152,7 +1152,7 @@ G_NORETURN void cpu_abort(CPUState *cpu, const char *fmt, ...) /* $(top_srcdir)/cpu.c */ void cpu_class_init_props(DeviceClass *dc); void cpu_exec_initfn(CPUState *cpu); -void cpu_exec_realizefn(CPUState *cpu, Error **errp); +bool cpu_exec_realizefn(CPUState *cpu, Error **errp); void cpu_exec_unrealizefn(CPUState *cpu); void cpu_exec_reset_hold(CPUState *cpu); diff --git a/cpu-target.c b/cpu-target.c index 79363ae370..f3e1ad8bcd 100644 --- a/cpu-target.c +++ b/cpu-target.c @@ -131,13 +131,13 @@ const VMStateDescription vmstate_cpu_common = { }; #endif -void cpu_exec_realizefn(CPUState *cpu, Error **errp) +bool cpu_exec_realizefn(CPUState *cpu, Error **errp) { /* cache the cpu class for the hotpath */ cpu->cc = CPU_GET_CLASS(cpu); if (!accel_cpu_common_realize(cpu, errp)) { -return; +return false; } /* Wait until cpu initialization complete before exposing cpu. */ @@ -159,6 +159,8 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp) vmstate_register(NULL, cpu->cpu_index, cpu->cc->sysemu_ops->legacy_vmsd, cpu); } #endif /* CONFIG_USER_ONLY */ + +return true; } void cpu_exec_unrealizefn(CPUState *cpu) -- 2.41.0
[PULL 19/60] target/ppc: Use env_archcpu() in helper_book3s_msgsndp()
When CPUArchState* is available (here CPUPPCState*), we can use the fast env_archcpu() macro to get ArchCPU* (here PowerPCCPU*). The QOM cast POWERPC_CPU() macro will be slower when building with --enable-qom-cast-debug. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Daniel Henrique Barboza Reviewed-by: Richard Henderson Acked-by: Alistair Francis Message-Id: <20231009110239.66778-2-phi...@linaro.org> --- target/ppc/excp_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 7926114d5c..a42743a3e0 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -3136,7 +3136,7 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) { CPUState *cs = env_cpu(env); -PowerPCCPU *cpu = POWERPC_CPU(cs); +PowerPCCPU *cpu = env_archcpu(env); CPUState *ccs; uint32_t nr_threads = cs->nr_threads; int ttir = rb & PPC_BITMASK(57, 63); -- 2.41.0
[PULL 53/60] hw/sensor: add ADM1266 device model
From: Titus Rwantare The ADM1266 is a cascadable super sequencer with margin control and fault recording. This commit adds basic support for its PMBus commands and models the identification registers that can be modified in a firmware update. Reviewed-by: Hao Wu Acked-by: Corey Minyard Signed-off-by: Titus Rwantare [PMD: Cover file in MAINTAINERS] Message-ID: <20231023-staging-pmbus-v3-v4-5-07a8cb7cd...@google.com> Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + hw/sensor/adm1266.c | 254 ++ hw/arm/Kconfig| 1 + hw/sensor/Kconfig | 5 + hw/sensor/meson.build | 1 + 5 files changed, 262 insertions(+) create mode 100644 hw/sensor/adm1266.c diff --git a/MAINTAINERS b/MAINTAINERS index 126cddd285..e6a2f57442 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -859,6 +859,7 @@ M: Hao Wu L: qemu-...@nongnu.org S: Supported F: hw/*/npcm* +F: hw/sensor/adm1266.c F: include/hw/*/npcm* F: tests/qtest/npcm* F: pc-bios/npcm7xx_bootrom.bin diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c new file mode 100644 index 00..5ae4f82ba1 --- /dev/null +++ b/hw/sensor/adm1266.c @@ -0,0 +1,254 @@ +/* + * Analog Devices ADM1266 Cascadable Super Sequencer with Margin Control and + * Fault Recording with PMBus + * + * https://www.analog.com/media/en/technical-documentation/data-sheets/adm1266.pdf + * + * Copyright 2023 Google LLC + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "hw/i2c/pmbus_device.h" +#include "hw/irq.h" +#include "migration/vmstate.h" +#include "qapi/error.h" +#include "qapi/visitor.h" +#include "qemu/log.h" +#include "qemu/module.h" + +#define TYPE_ADM1266 "adm1266" +OBJECT_DECLARE_SIMPLE_TYPE(ADM1266State, ADM1266) + +#define ADM1266_BLACKBOX_CONFIG 0xD3 +#define ADM1266_PDIO_CONFIG 0xD4 +#define ADM1266_READ_STATE 0xD9 +#define ADM1266_READ_BLACKBOX 0xDE +#define ADM1266_SET_RTC 0xDF +#define ADM1266_GPIO_SYNC_CONFIGURATION 0xE1 +#define ADM1266_BLACKBOX_INFORMATION0xE6 +#define ADM1266_PDIO_STATUS 0xE9 +#define ADM1266_GPIO_STATUS 0xEA + +/* Defaults */ +#define ADM1266_OPERATION_DEFAULT 0x80 +#define ADM1266_CAPABILITY_DEFAULT 0xA0 +#define ADM1266_CAPABILITY_NO_PEC 0x20 +#define ADM1266_PMBUS_REVISION_DEFAULT 0x22 +#define ADM1266_MFR_ID_DEFAULT "ADI" +#define ADM1266_MFR_ID_DEFAULT_LEN 32 +#define ADM1266_MFR_MODEL_DEFAULT "ADM1266-A1" +#define ADM1266_MFR_MODEL_DEFAULT_LEN 32 +#define ADM1266_MFR_REVISION_DEFAULT"25" +#define ADM1266_MFR_REVISION_DEFAULT_LEN8 + +#define ADM1266_NUM_PAGES 17 +/** + * PAGE Index + * Page 0 VH1. + * Page 1 VH2. + * Page 2 VH3. + * Page 3 VH4. + * Page 4 VP1. + * Page 5 VP2. + * Page 6 VP3. + * Page 7 VP4. + * Page 8 VP5. + * Page 9 VP6. + * Page 10 VP7. + * Page 11 VP8. + * Page 12 VP9. + * Page 13 VP10. + * Page 14 VP11. + * Page 15 VP12. + * Page 16 VP13. + */ +typedef struct ADM1266State { +PMBusDevice parent; + +char mfr_id[32]; +char mfr_model[32]; +char mfr_rev[8]; +} ADM1266State; + +static const uint8_t adm1266_ic_device_id[] = {0x03, 0x41, 0x12, 0x66}; +static const uint8_t adm1266_ic_device_rev[] = {0x08, 0x01, 0x08, 0x07, 0x0, +0x0, 0x07, 0x41, 0x30}; + +static void adm1266_exit_reset(Object *obj) +{ +ADM1266State *s = ADM1266(obj); +PMBusDevice *pmdev = PMBUS_DEVICE(obj); + +pmdev->page = 0; +pmdev->capability = ADM1266_CAPABILITY_NO_PEC; + +for (int i = 0; i < ADM1266_NUM_PAGES; i++) { +pmdev->pages[i].operation = ADM1266_OPERATION_DEFAULT; +pmdev->pages[i].revision = ADM1266_PMBUS_REVISION_DEFAULT; +pmdev->pages[i].vout_mode = 0; +pmdev->pages[i].read_vout = pmbus_data2linear_mode(12, 0); +pmdev->pages[i].vout_margin_high = pmbus_data2linear_mode(15, 0); +pmdev->pages[i].vout_margin_low = pmbus_data2linear_mode(3, 0); +pmdev->pages[i].vout_ov_fault_limit = pmbus_data2linear_mode(16, 0); +pmdev->pages[i].revision = ADM1266_PMBUS_REVISION_DEFAULT; +} + +strncpy(s->mfr_id, ADM1266_MFR_ID_DEFAULT, 4); +strncpy(s->mfr_model, ADM1266_MFR_MODEL_DEFAULT, 11); +strncpy(s->mfr_rev, ADM1266_MFR_REVISION_DEFAULT, 3); +} + +static uint8_t adm1266_read_byte(PMBusDevice *pmdev) +{ +ADM1266State *s = ADM1266(pmdev); + +switch (pmdev->code) { +case PMBUS_MFR_ID:/* R/W block */ +pmbus_send_string(pmdev, s->mfr_id); +break; + +case PMBUS_MFR_MODEL: /* R/W block */ +pmbus_send_string(pmdev, s->mfr_model); +break; + +case PMBUS_MFR_REVISION: /* R/W block */ +
[PULL 46/60] hw/cpu: Update the comments of nr_cores and nr_dies
From: Zhao Liu In the nr_threads' comment, specify it represents the number of threads in the "core" to avoid confusion. Also add comment for nr_dies in CPUX86State. Signed-off-by: Zhao Liu Reviewed-by: Philippe Mathieu-Daudé Tested-by: Babu Moger Tested-by: Yongwei Ma Acked-by: Michael S. Tsirkin Message-ID: <20231024090323.1859210-5-zhao1@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- include/hw/core/cpu.h | 2 +- target/i386/cpu.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 77893d7b81..c0c8320413 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -408,7 +408,7 @@ struct qemu_work_item; * See TranslationBlock::TCG CF_CLUSTER_MASK. * @tcg_cflags: Pre-computed cflags for this cpu. * @nr_cores: Number of cores within this CPU package. - * @nr_threads: Number of threads within this CPU. + * @nr_threads: Number of threads within this CPU core. * @running: #true if CPU is currently running (lockless). * @has_waiter: #true if a CPU is currently waiting for the cpu_exec_end; * valid under cpu_list_lock. diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 6c6b066986..b60a417074 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1882,6 +1882,7 @@ typedef struct CPUArchState { TPRAccess tpr_access_type; +/* Number of dies within this CPU package. */ unsigned nr_dies; } CPUX86State; -- 2.41.0
[PULL 54/60] tests/qtest: add tests for ADM1266
From: Titus Rwantare The ADM1266 can have string fields written by the driver, so it's worth specifically testing. Reviewed-by: Hao Wu Acked-by: Corey Minyard Signed-off-by: Titus Rwantare [PMD: Cover file in MAINTAINERS] Message-ID: <20231023-staging-pmbus-v3-v4-6-07a8cb7cd...@google.com> Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS| 1 + tests/qtest/adm1266-test.c | 122 + tests/qtest/meson.build| 1 + 3 files changed, 124 insertions(+) create mode 100644 tests/qtest/adm1266-test.c diff --git a/MAINTAINERS b/MAINTAINERS index e6a2f57442..c01c2e6ec0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -862,6 +862,7 @@ F: hw/*/npcm* F: hw/sensor/adm1266.c F: include/hw/*/npcm* F: tests/qtest/npcm* +F: tests/qtest/adm1266-test.c F: pc-bios/npcm7xx_bootrom.bin F: roms/vbootrom F: docs/system/arm/nuvoton.rst diff --git a/tests/qtest/adm1266-test.c b/tests/qtest/adm1266-test.c new file mode 100644 index 00..6c312c499f --- /dev/null +++ b/tests/qtest/adm1266-test.c @@ -0,0 +1,122 @@ +/* + * Analog Devices ADM1266 Cascadable Super Sequencer with Margin Control and + * Fault Recording with PMBus + * + * Copyright 2022 Google LLC + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include +#include "hw/i2c/pmbus_device.h" +#include "libqtest-single.h" +#include "libqos/qgraph.h" +#include "libqos/i2c.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp/qnum.h" +#include "qemu/bitops.h" + +#define TEST_ID "adm1266-test" +#define TEST_ADDR (0x12) + +#define ADM1266_BLACKBOX_CONFIG 0xD3 +#define ADM1266_PDIO_CONFIG 0xD4 +#define ADM1266_READ_STATE 0xD9 +#define ADM1266_READ_BLACKBOX 0xDE +#define ADM1266_SET_RTC 0xDF +#define ADM1266_GPIO_SYNC_CONFIGURATION 0xE1 +#define ADM1266_BLACKBOX_INFORMATION0xE6 +#define ADM1266_PDIO_STATUS 0xE9 +#define ADM1266_GPIO_STATUS 0xEA + +/* Defaults */ +#define ADM1266_OPERATION_DEFAULT 0x80 +#define ADM1266_CAPABILITY_DEFAULT 0xA0 +#define ADM1266_CAPABILITY_NO_PEC 0x20 +#define ADM1266_PMBUS_REVISION_DEFAULT 0x22 +#define ADM1266_MFR_ID_DEFAULT "ADI" +#define ADM1266_MFR_ID_DEFAULT_LEN 32 +#define ADM1266_MFR_MODEL_DEFAULT "ADM1266-A1" +#define ADM1266_MFR_MODEL_DEFAULT_LEN 32 +#define ADM1266_MFR_REVISION_DEFAULT"25" +#define ADM1266_MFR_REVISION_DEFAULT_LEN8 +#define TEST_STRING_A "a sample" +#define TEST_STRING_B "b sample" +#define TEST_STRING_C "rev c" + +static void compare_string(QI2CDevice *i2cdev, uint8_t reg, + const char *test_str) +{ +uint8_t len = i2c_get8(i2cdev, reg); +char i2c_str[SMBUS_DATA_MAX_LEN] = {0}; + +i2c_read_block(i2cdev, reg, (uint8_t *)i2c_str, len); +g_assert_cmpstr(i2c_str, ==, test_str); +} + +static void write_and_compare_string(QI2CDevice *i2cdev, uint8_t reg, + const char *test_str, uint8_t len) +{ +char buf[SMBUS_DATA_MAX_LEN] = {0}; +buf[0] = len; +strncpy(buf + 1, test_str, len); +i2c_write_block(i2cdev, reg, (uint8_t *)buf, len + 1); +compare_string(i2cdev, reg, test_str); +} + +static void test_defaults(void *obj, void *data, QGuestAllocator *alloc) +{ +uint16_t i2c_value; +QI2CDevice *i2cdev = (QI2CDevice *)obj; + +i2c_value = i2c_get8(i2cdev, PMBUS_OPERATION); +g_assert_cmphex(i2c_value, ==, ADM1266_OPERATION_DEFAULT); + +i2c_value = i2c_get8(i2cdev, PMBUS_REVISION); +g_assert_cmphex(i2c_value, ==, ADM1266_PMBUS_REVISION_DEFAULT); + +compare_string(i2cdev, PMBUS_MFR_ID, ADM1266_MFR_ID_DEFAULT); +compare_string(i2cdev, PMBUS_MFR_MODEL, ADM1266_MFR_MODEL_DEFAULT); +compare_string(i2cdev, PMBUS_MFR_REVISION, ADM1266_MFR_REVISION_DEFAULT); +} + +/* test r/w registers */ +static void test_rw_regs(void *obj, void *data, QGuestAllocator *alloc) +{ +QI2CDevice *i2cdev = (QI2CDevice *)obj; + +/* empty strings */ +i2c_set8(i2cdev, PMBUS_MFR_ID, 0); +compare_string(i2cdev, PMBUS_MFR_ID, ""); + +i2c_set8(i2cdev, PMBUS_MFR_MODEL, 0); +compare_string(i2cdev, PMBUS_MFR_MODEL, ""); + +i2c_set8(i2cdev, PMBUS_MFR_REVISION, 0); +compare_string(i2cdev, PMBUS_MFR_REVISION, ""); + +/* test strings */ +write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_A, + sizeof(TEST_STRING_A)); +write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_B, + sizeof(TEST_STRING_B)); +write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_C, + sizeof(TEST_STRING_C)); +} + +static void adm1266_register_nodes(void) +{ +
[PULL 22/60] target/xtensa: Use env_archcpu() in update_c[compare|count]()
When CPUArchState* is available (here CPUXtensaState*), we can use the fast env_archcpu() macro to get ArchCPU* (here XtensaCPU*). The QOM cast XTENSA_CPU() macro will be slower when building with --enable-qom-cast-debug. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Acked-by: Alistair Francis Message-Id: <20231009110239.66778-5-phi...@linaro.org> --- target/xtensa/op_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c index 7bb8cd6726..496754ba57 100644 --- a/target/xtensa/op_helper.c +++ b/target/xtensa/op_helper.c @@ -37,7 +37,7 @@ void HELPER(update_ccount)(CPUXtensaState *env) { -XtensaCPU *cpu = XTENSA_CPU(env_cpu(env)); +XtensaCPU *cpu = env_archcpu(env); uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); env->ccount_time = now; @@ -58,7 +58,7 @@ void HELPER(wsr_ccount)(CPUXtensaState *env, uint32_t v) void HELPER(update_ccompare)(CPUXtensaState *env, uint32_t i) { -XtensaCPU *cpu = XTENSA_CPU(env_cpu(env)); +XtensaCPU *cpu = env_archcpu(env); uint64_t dcc; qatomic_and(>sregs[INTSET], -- 2.41.0
[PULL 47/60] hw/ide: reset: cancel async DMA operation before resetting state
From: Fiona Ebner If there is a pending DMA operation during ide_bus_reset(), the fact that the IDEState is already reset before the operation is canceled can be problematic. In particular, ide_dma_cb() might be called and then use the reset IDEState which contains the signature after the reset. When used to construct the IO operation this leads to ide_get_sector() returning 0 and nsector being 1. This is particularly bad, because a write command will thus destroy the first sector which often contains a partition table or similar. Traces showing the unsolicited write happening with IDEState 0x5595af6949d0 being used after reset: > ahci_port_write ahci(0x5595af6923f0)[0]: port write [reg:PxSCTL] @ 0x2c: > 0x0300 > ahci_reset_port ahci(0x5595af6923f0)[0]: reset port > ide_reset IDEstate 0x5595af6949d0 > ide_reset IDEstate 0x5595af694da8 > ide_bus_reset_aio aio_cancel > dma_aio_cancel dbs=0x7f64600089a0 > dma_blk_cb dbs=0x7f64600089a0 ret=0 > dma_complete dbs=0x7f64600089a0 ret=0 cb=0x5595acd40b30 > ahci_populate_sglist ahci(0x5595af6923f0)[0] > ahci_dma_prepare_buf ahci(0x5595af6923f0)[0]: prepare buf limit=512 > prepared=512 > ide_dma_cb IDEState 0x5595af6949d0; sector_num=0 n=1 cmd=DMA WRITE > dma_blk_io dbs=0x7f6420802010 bs=0x5595ae2c6c30 offset=0 to_dev=1 > dma_blk_cb dbs=0x7f6420802010 ret=0 > (gdb) p *qiov > $11 = {iov = 0x7f647c76d840, niov = 1, {{nalloc = 1, local_iov = {iov_base = > 0x0, > iov_len = 512}}, {__pad = > "\001\000\000\000\000\000\000\000\000\000\000", > size = 512}}} > (gdb) bt > #0 blk_aio_pwritev (blk=0x5595ae2c6c30, offset=0, qiov=0x7f6420802070, > flags=0, > cb=0x5595ace6f0b0 , opaque=0x7f6420802010) > at ../block/block-backend.c:1682 > #1 0x5595ace6f185 in dma_blk_cb (opaque=0x7f6420802010, ret= out>) > at ../softmmu/dma-helpers.c:179 > #2 0x5595ace6f778 in dma_blk_io (ctx=0x5595ae0609f0, > sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, > io_func=io_func@entry=0x5595ace6ee30 , > io_func_opaque=io_func_opaque@entry=0x5595ae2c6c30, > cb=0x5595acd40b30 , opaque=0x5595af6949d0, > dir=DMA_DIRECTION_TO_DEVICE) at ../softmmu/dma-helpers.c:244 > #3 0x5595ace6f90a in dma_blk_write (blk=0x5595ae2c6c30, > sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, > cb=cb@entry=0x5595acd40b30 , > opaque=opaque@entry=0x5595af6949d0) > at ../softmmu/dma-helpers.c:280 > #4 0x5595acd40e18 in ide_dma_cb (opaque=0x5595af6949d0, ret= out>) > at ../hw/ide/core.c:953 > #5 0x5595ace6f319 in dma_complete (ret=0, dbs=0x7f64600089a0) > at ../softmmu/dma-helpers.c:107 > #6 dma_blk_cb (opaque=0x7f64600089a0, ret=0) at ../softmmu/dma-helpers.c:127 > #7 0x5595ad12227d in blk_aio_complete (acb=0x7f6460005b10) > at ../block/block-backend.c:1527 > #8 blk_aio_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524 > #9 blk_aio_write_entry (opaque=0x7f6460005b10) at > ../block/block-backend.c:1594 > #10 0x5595ad258cfb in coroutine_trampoline (i0=, > i1=) at ../util/coroutine-ucontext.c:177 Signed-off-by: Fiona Ebner Reviewed-by: Philippe Mathieu-Daudé Tested-by: simon.r...@nutanix.com Message-ID: <20230906130922.142845-1-f.eb...@proxmox.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/ide/core.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index b5e0dcd29b..63ba665f3d 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2515,19 +2515,19 @@ static void ide_dummy_transfer_stop(IDEState *s) void ide_bus_reset(IDEBus *bus) { -bus->unit = 0; -bus->cmd = 0; -ide_reset(>ifs[0]); -ide_reset(>ifs[1]); -ide_clear_hob(bus); - -/* pending async DMA */ +/* pending async DMA - needs the IDEState before it is reset */ if (bus->dma->aiocb) { trace_ide_bus_reset_aio(); blk_aio_cancel(bus->dma->aiocb); bus->dma->aiocb = NULL; } +bus->unit = 0; +bus->cmd = 0; +ide_reset(>ifs[0]); +ide_reset(>ifs[1]); +ide_clear_hob(bus); + /* reset dma provider too */ if (bus->dma->ops->reset) { bus->dma->ops->reset(bus->dma); -- 2.41.0
[PULL 10/60] target/arm: Move internal declarations from 'cpu-qom.h' to 'cpu.h'
These definitions and declarations are only used by target/arm/, no need to expose them to generic hw/. Reviewed-by: Richard Henderson Message-Id: <20231013140116.255-4-phi...@linaro.org> Signed-off-by: Philippe Mathieu-Daudé Message-Id: --- target/arm/cpu-qom.h | 28 target/arm/cpu.h | 22 ++ target/arm/internals.h | 6 ++ 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h index dfb9d5b827..35c3b0924e 100644 --- a/target/arm/cpu-qom.h +++ b/target/arm/cpu-qom.h @@ -35,9 +35,6 @@ typedef struct ARMCPUInfo { void (*class_init)(ObjectClass *oc, void *data); } ARMCPUInfo; -void arm_cpu_register(const ARMCPUInfo *info); -void aarch64_cpu_register(const ARMCPUInfo *info); - /** * ARMCPUClass: * @parent_realize: The parent class' realize handler. @@ -63,29 +60,4 @@ struct AArch64CPUClass { ARMCPUClass parent_class; }; -void register_cp_regs_for_features(ARMCPU *cpu); -void init_cpreg_list(ARMCPU *cpu); - -/* Callback functions for the generic timer's timers. */ -void arm_gt_ptimer_cb(void *opaque); -void arm_gt_vtimer_cb(void *opaque); -void arm_gt_htimer_cb(void *opaque); -void arm_gt_stimer_cb(void *opaque); -void arm_gt_hvtimer_cb(void *opaque); - -#define ARM_AFF0_SHIFT 0 -#define ARM_AFF0_MASK (0xFFULL << ARM_AFF0_SHIFT) -#define ARM_AFF1_SHIFT 8 -#define ARM_AFF1_MASK (0xFFULL << ARM_AFF1_SHIFT) -#define ARM_AFF2_SHIFT 16 -#define ARM_AFF2_MASK (0xFFULL << ARM_AFF2_SHIFT) -#define ARM_AFF3_SHIFT 32 -#define ARM_AFF3_MASK (0xFFULL << ARM_AFF3_SHIFT) -#define ARM_DEFAULT_CPUS_PER_CLUSTER 8 - -#define ARM32_AFFINITY_MASK (ARM_AFF0_MASK|ARM_AFF1_MASK|ARM_AFF2_MASK) -#define ARM64_AFFINITY_MASK \ -(ARM_AFF0_MASK|ARM_AFF1_MASK|ARM_AFF2_MASK|ARM_AFF3_MASK) -#define ARM64_AFFINITY_INVALID (~ARM64_AFFINITY_MASK) - #endif diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 2f7ab22169..4a86c8f831 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1116,11 +1116,33 @@ struct ArchCPU { uint64_t gt_cntfrq_hz; }; +/* Callback functions for the generic timer's timers. */ +void arm_gt_ptimer_cb(void *opaque); +void arm_gt_vtimer_cb(void *opaque); +void arm_gt_htimer_cb(void *opaque); +void arm_gt_stimer_cb(void *opaque); +void arm_gt_hvtimer_cb(void *opaque); + unsigned int gt_cntfrq_period_ns(ARMCPU *cpu); void gt_rme_post_el_change(ARMCPU *cpu, void *opaque); void arm_cpu_post_init(Object *obj); +#define ARM_AFF0_SHIFT 0 +#define ARM_AFF0_MASK (0xFFULL << ARM_AFF0_SHIFT) +#define ARM_AFF1_SHIFT 8 +#define ARM_AFF1_MASK (0xFFULL << ARM_AFF1_SHIFT) +#define ARM_AFF2_SHIFT 16 +#define ARM_AFF2_MASK (0xFFULL << ARM_AFF2_SHIFT) +#define ARM_AFF3_SHIFT 32 +#define ARM_AFF3_MASK (0xFFULL << ARM_AFF3_SHIFT) +#define ARM_DEFAULT_CPUS_PER_CLUSTER 8 + +#define ARM32_AFFINITY_MASK (ARM_AFF0_MASK | ARM_AFF1_MASK | ARM_AFF2_MASK) +#define ARM64_AFFINITY_MASK \ +(ARM_AFF0_MASK | ARM_AFF1_MASK | ARM_AFF2_MASK | ARM_AFF3_MASK) +#define ARM64_AFFINITY_INVALID (~ARM64_AFFINITY_MASK) + uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz); #ifndef CONFIG_USER_ONLY diff --git a/target/arm/internals.h b/target/arm/internals.h index c837506e44..143d57c0fe 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -183,6 +183,12 @@ static inline int r14_bank_number(int mode) return (mode == ARM_CPU_MODE_HYP) ? BANK_USRSYS : bank_number(mode); } +void arm_cpu_register(const ARMCPUInfo *info); +void aarch64_cpu_register(const ARMCPUInfo *info); + +void register_cp_regs_for_features(ARMCPU *cpu); +void init_cpreg_list(ARMCPU *cpu); + void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu); void arm_translate_init(void); -- 2.41.0