Re: [PATCH v3] failover: fix unplug pending detection
On Tue, 5 Oct 2021, Laurent Vivier wrote: > On 05/10/2021 17:14, Michael S. Tsirkin wrote: > > On Fri, Oct 01, 2021 at 10:25:02AM +0200, Laurent Vivier wrote: > > > Failover needs to detect the end of the PCI unplug to start migration > > > after the VFIO card has been unplugged. > > > > > > To do that, a flag is set in pcie_cap_slot_unplug_request_cb() and reset > > > in > > > pcie_unplug_device(). > > > > > > But since > > > 17858a169508 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on > > > Q35") > > > we have switched to ACPI unplug and these functions are not called anymore > > > and the flag not set. So failover migration is not able to detect if card > > > is really unplugged and acts as it's done as soon as it's started. So it > > > doesn't wait the end of the unplug to start the migration. We don't see > > > any > > > problem when we test that because ACPI unplug is faster than PCIe native > > > hotplug and when the migration really starts the unplug operation is > > > already done. > > > > > > See c000a9bd06ea ("pci: mark device having guest unplug request pending") > > > a99c4da9fc2a ("pci: mark devices partially unplugged") > > > > > > Signed-off-by: Laurent Vivier > > > Reviewed-by: Ani Sinha > > > > Laurent, are you thinking of addressing Gerd's comment? > > No, because as said by Ani, it's not the scope of this patch. The patch only > aligns ACPI to PCIe Native to be able to manage failover. > > The problem reported by Gerd and Daniel has been introduced by another patch, > globally. > but I thought Julia's fix commit cce8944cc9efab47d4bf29cfffb3470371c3541b addressed this for native pcie and since it is at a high enough level, it should catch the acpi hotplug path equally as well.
Re: [PATCH] tests/docker/dockerfiles: Bump fedora-i386-cross to fedora 34
On 10/5/21 3:16 PM, Paolo Bonzini wrote: On 05/10/21 22:58, Richard Henderson wrote: For unknown and unrepeatable reasons, the cross-i386-tci test has started failing. "Fix" this by updating the container to use fedora 34. Add sysprof-capture-devel as a new dependency of glib2-devel that was not correctly spelled out in the rpm rules. Use dnf update Just In Case -- there are presently out-of-date packages in the upstream docker registry. Signed-off-by: Richard Henderson --- tests/docker/dockerfiles/fedora-i386-cross.docker | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker index 820740d5be..f62a71ce22 100644 --- a/tests/docker/dockerfiles/fedora-i386-cross.docker +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker @@ -1,4 +1,5 @@ -FROM registry.fedoraproject.org/fedora:33 +FROM registry.fedoraproject.org/fedora:34 + ENV PACKAGES \ bzip2 \ ccache \ @@ -20,10 +21,11 @@ ENV PACKAGES \ pcre-devel.i686 \ perl-Test-Harness \ pixman-devel.i686 \ + sysprof-capture-devel.i686 \ zlib-devel.i686 ENV QEMU_CONFIGURE_OPTS --cpu=i386 --disable-vhost-user ENV PKG_CONFIG_LIBDIR /usr/lib/pkgconfig -RUN dnf install -y $PACKAGES +RUN dnf update -y && dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt Reviewed-by: Paolo Bonzini I would say, go ahead and apply it to the tree directly to unbreak CI. Done. CI is now green again. r~
Re: [PATCH v2] target/ppc: Fix the test raising the decrementer exception
On Tue, Oct 05, 2021 at 07:33:24AM +0200, Cédric Le Goater wrote: > Commit 4d9b8ef9b5ab ("target/ppc: Fix 64-bit decrementer") introduced > new int64t variables and broke the test triggering the decrementer > exception. Revert partially the change to evaluate both clause of the > if statement. > > Reported-by: Coverity CID 1464061 > Fixes: 4d9b8ef9b5ab ("target/ppc: Fix 64-bit decrementer") > Suggested-by: Peter Maydell > Signed-off-by: Cédric Le Goater Applied to ppc-for-6.2 thanks. > --- > hw/ppc/ppc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index f5d012f860af..a724b0bb5ecb 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -848,7 +848,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, > uint64_t *nextp, > * On MSB edge based DEC implementations the MSB going from 0 -> 1 > triggers > * an edge interrupt, so raise it here too. > */ > -if ((signed_value < 3) || > +if ((value < 3) || > ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || > ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0 >&& signed_decr >= 0)) { -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH] ide: Cap LBA28 capacity announcement to 2^28-1
Ping? Samuel Thibault, le mar. 24 août 2021 12:43:44 +0200, a ecrit: > The LBA28 capacity (at offsets 60/61 of identification) is supposed to > express the maximum size supported by LBA28 commands. If the device is > larger than this, we have to cap it to 2^28-1. > > At least NetBSD happens to be using this value to determine whether to use > LBA28 or LBA48 for its commands, using LBA28 for sectors that don't need > LBA48. This commit thus fixes NetBSD access to disks larger than 128GiB. > > Signed-off-by: Samuel Thibault > --- > hw/ide/core.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index fd69ca3167..e28f8aad61 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -98,8 +98,12 @@ static void put_le16(uint16_t *p, unsigned int v) > static void ide_identify_size(IDEState *s) > { > uint16_t *p = (uint16_t *)s->identify_data; > -put_le16(p + 60, s->nb_sectors); > -put_le16(p + 61, s->nb_sectors >> 16); > +int64_t nb_sectors_lba28 = s->nb_sectors; > +if (nb_sectors_lba28 >= 1 << 28) { > +nb_sectors_lba28 = (1 << 28) - 1; > +} > +put_le16(p + 60, nb_sectors_lba28); > +put_le16(p + 61, nb_sectors_lba28 >> 16); > put_le16(p + 100, s->nb_sectors); > put_le16(p + 101, s->nb_sectors >> 16); > put_le16(p + 102, s->nb_sectors >> 32); > -- > 2.32.0 >
Re: [PATCH] net/slirp: Use newer slirp_*_hostxfwd API
Nicholas Ngai, le sam. 25 sept. 2021 16:22:02 -0700, a ecrit: > Sorry for the duplicate email. The cc’s for the maintainers on the email > didn’t go through the first time. > > Nicholas Ngai > > On 9/25/21 2:48 PM, Nicholas Ngai wrote: > > libslirp provides a newer slirp_*_hostxfwd API meant for > > address-agnostic forwarding instead of the is_udp parameter which is > > limited to just TCP/UDP. > > > > Signed-off-by: Nicholas Ngai Reviewed-by: Samuel Thibault > > --- > > net/slirp.c | 64 +++-- > > 1 file changed, 42 insertions(+), 22 deletions(-) > > > > diff --git a/net/slirp.c b/net/slirp.c > > index ad3a838e0b..49ae01a2f0 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -643,12 +643,17 @@ static SlirpState *slirp_lookup(Monitor *mon, const > > char *id) > > void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict) > > { > > -struct in_addr host_addr = { .s_addr = INADDR_ANY }; > > -int host_port; > > +struct sockaddr_in host_addr = { > > +.sin_family = AF_INET, > > +.sin_addr = { > > +.s_addr = INADDR_ANY, > > +}, > > +}; > > +int port; > > +int flags = 0; > > char buf[256]; > > const char *src_str, *p; > > SlirpState *s; > > -int is_udp = 0; > > int err; > > const char *arg1 = qdict_get_str(qdict, "arg1"); > > const char *arg2 = qdict_get_try_str(qdict, "arg2"); > > @@ -670,9 +675,9 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict > > *qdict) > > } > > if (!strcmp(buf, "tcp") || buf[0] == '\0') { > > -is_udp = 0; > > +/* Do nothing; already TCP. */ > > } else if (!strcmp(buf, "udp")) { > > -is_udp = 1; > > +flags |= SLIRP_HOSTFWD_UDP; > > } else { > > goto fail_syntax; > > } > > @@ -680,15 +685,17 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict > > *qdict) > > if (get_str_sep(buf, sizeof(buf), , ':') < 0) { > > goto fail_syntax; > > } > > -if (buf[0] != '\0' && !inet_aton(buf, _addr)) { > > +if (buf[0] != '\0' && !inet_aton(buf, _addr.sin_addr)) { > > goto fail_syntax; > > } > > -if (qemu_strtoi(p, NULL, 10, _port)) { > > +if (qemu_strtoi(p, NULL, 10, )) { > > goto fail_syntax; > > } > > +host_addr.sin_port = htons(port); > > -err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port); > > +err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) _addr, > > +sizeof(host_addr), flags); > > monitor_printf(mon, "host forwarding rule for %s %s\n", src_str, > > err ? "not found" : "removed"); > > @@ -700,12 +707,22 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict > > *qdict) > > static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error > > **errp) > > { > > -struct in_addr host_addr = { .s_addr = INADDR_ANY }; > > -struct in_addr guest_addr = { .s_addr = 0 }; > > -int host_port, guest_port; > > +struct sockaddr_in host_addr = { > > +.sin_family = AF_INET, > > +.sin_addr = { > > +.s_addr = INADDR_ANY, > > +}, > > +}; > > +struct sockaddr_in guest_addr = { > > +.sin_family = AF_INET, > > +.sin_addr = { > > +.s_addr = 0, > > +}, > > +}; > > +int flags = 0; > > +int port; > > const char *p; > > char buf[256]; > > -int is_udp; > > char *end; > > const char *fail_reason = "Unknown reason"; > > @@ -715,9 +732,9 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, Error **errp) > > goto fail_syntax; > > } > > if (!strcmp(buf, "tcp") || buf[0] == '\0') { > > -is_udp = 0; > > +/* Do nothing; already TCP. */ > > } else if (!strcmp(buf, "udp")) { > > -is_udp = 1; > > +flags |= SLIRP_HOSTFWD_UDP; > > } else { > > fail_reason = "Bad protocol name"; > > goto fail_syntax; > > @@ -727,7 +744,7 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, Error **errp) > > fail_reason = "Missing : separator"; > > goto fail_syntax; > > } > > -if (buf[0] != '\0' && !inet_aton(buf, _addr)) { > > +if (buf[0] != '\0' && !inet_aton(buf, _addr.sin_addr)) { > > fail_reason = "Bad host address"; > > goto fail_syntax; > > } > > @@ -736,29 +753,32 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, Error **errp) > > fail_reason = "Bad host port separator"; > > goto fail_syntax; > > } > > -host_port = strtol(buf, , 0); > > -if (*end != '\0' || host_port < 0 || host_port > 65535) { > > +port = strtol(buf, , 0); > > +if (*end != '\0' || port < 0 || port > 65535) { > > fail_reason = "Bad host port"; > > goto fail_syntax; > > } > > +
Re: [PULL 00/57] pc,pci,virtio: features, fixes
On 10/5/21 2:32 PM, Michael S. Tsirkin wrote: On Tue, Oct 05, 2021 at 10:21:43AM -0700, Richard Henderson wrote: On 10/5/21 9:00 AM, Michael S. Tsirkin wrote: The following changes since commit 9618c5badaa8eed25259cf095ff880efb939fbe7: Merge remote-tracking branch 'remotes/vivier/tags/trivial-branch-for-6.2-pull-request' into staging (2021-10-04 16:27:35 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to c7d2f59cf940b8c8c52c29d5fa25613fe662f7b6: hw/i386/amd_iommu: Add description/category to TYPE_AMD_IOMMU_PCI (2021-10-05 11:46:45 -0400) ... You missed updating the stub version of these functions: ../src/hw/net/vhost_net-stub.c:34:5: error: conflicting types for ‘vhost_net_start’ 34 | int vhost_net_start(VirtIODevice *dev, | ^~~ In file included from ../src/hw/net/vhost_net-stub.c:19: /home/rth/qemu-publish/src/include/net/vhost_net.h:24:5: note: previous declaration of ‘vhost_net_start’ was here 24 | int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, | ^~~ ../src/hw/net/vhost_net-stub.c:40:6: error: conflicting types for ‘vhost_net_stop’ 40 | void vhost_net_stop(VirtIODevice *dev, | ^~ In file included from ../src/hw/net/vhost_net-stub.c:19: /home/rth/qemu-publish/src/include/net/vhost_net.h:26:6: note: previous declaration of ‘vhost_net_stop’ was here 26 | void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, | ^~ Indeed. I dropped these patches for now. Could you refetch from same tag and confirm it's ok? Yes, that works. Applied, thanks. r~
Re: [RFC PATCH] meson.build: don't include libbpf in the common source set
On 05/10/21 22:25, Richard Henderson wrote: On 10/5/21 12:27 PM, Paolo Bonzini wrote: On 05/10/21 20:24, Alex Bennée wrote: This library is only needed for the softmmu targets and as such break static *-user builds where libbpf is detected and it tries to link it into the user binaries. Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") Signed-off-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 60f4f45165..d8bcf13b21 100644 --- a/meson.build +++ b/meson.build @@ -2307,7 +2307,7 @@ subdir('bsd-user') subdir('linux-user') subdir('ebpf') -common_ss.add(libbpf) +softmmu_ss.add(libbpf) It should not be needed at all, since ebpf/meson.build has softmmu_ss.add(when: libbpf, if_true: files('ebpf_rss.c'), if_false: files('ebpf_rss-stub.c')) (which already adds libbpf if needed). Ooo, magic side effects. I'll note that the manual doesn't say that it adds and dependencies from varnames_and_deps, only that it checks them. Good point, it's in an example above: # Include zlib.c if the zlib dependency was found, and link zlib # in the executable ss.add(when: zlib, if_true: files('zlib.c')) Paolo
Re: [PATCH] tests/docker/dockerfiles: Bump fedora-i386-cross to fedora 34
On 05/10/21 22:58, Richard Henderson wrote: For unknown and unrepeatable reasons, the cross-i386-tci test has started failing. "Fix" this by updating the container to use fedora 34. Add sysprof-capture-devel as a new dependency of glib2-devel that was not correctly spelled out in the rpm rules. Use dnf update Just In Case -- there are presently out-of-date packages in the upstream docker registry. Signed-off-by: Richard Henderson --- tests/docker/dockerfiles/fedora-i386-cross.docker | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker index 820740d5be..f62a71ce22 100644 --- a/tests/docker/dockerfiles/fedora-i386-cross.docker +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker @@ -1,4 +1,5 @@ -FROM registry.fedoraproject.org/fedora:33 +FROM registry.fedoraproject.org/fedora:34 + ENV PACKAGES \ bzip2 \ ccache \ @@ -20,10 +21,11 @@ ENV PACKAGES \ pcre-devel.i686 \ perl-Test-Harness \ pixman-devel.i686 \ +sysprof-capture-devel.i686 \ zlib-devel.i686 ENV QEMU_CONFIGURE_OPTS --cpu=i386 --disable-vhost-user ENV PKG_CONFIG_LIBDIR /usr/lib/pkgconfig -RUN dnf install -y $PACKAGES +RUN dnf update -y && dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt Reviewed-by: Paolo Bonzini I would say, go ahead and apply it to the tree directly to unbreak CI. Paolo
Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote: > On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote: > > * Michael S. Tsirkin (m...@redhat.com) wrote: > > > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote: > > > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote: > > > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote: > > > > > > It might be useful for the cases when a slow block layer should be > > > > > > replaced > > > > > > with a more performant one on running VM without stopping, i.e. > > > > > > with very low > > > > > > downtime comparable with the one on migration. > > > > > > > > > > > > It's possible to achive that for two reasons: > > > > > > > > > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the > > > > > > same. > > > > > > They consist of the identical VMSTATE_VIRTIO_DEVICE and differs > > > > > > from > > > > > > each other in the values of migration service fields only. > > > > > > 2.The device driver used in the guest is the same: virtio-blk > > > > > > > > > > > > In the series cross-migration is achieved by adding a new type. > > > > > > The new type uses virtio-blk VMState instead of vhost-user-blk > > > > > > specific > > > > > > VMstate, also it implements migration save/load callbacks to be > > > > > > compatible > > > > > > with migration stream produced by "virtio-blk" device. > > > > > > > > > > > > Adding the new type instead of modifying the existing one is > > > > > > convenent. > > > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk > > > > > > device from the existing non-compatible one using qemu machinery > > > > > > without any > > > > > > other modifiactions. That gives all the variety of qemu device > > > > > > related > > > > > > constraints out of box. > > > > > > > > > > Hmm I'm not sure I understand. What is the advantage for the user? > > > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk? > > > > > We could add some hacks to make it compatible for old machine types. > > > > > > > > The point is that virtio-blk and vhost-user-blk are not > > > > migration-compatible ATM. OTOH they are the same device from the guest > > > > POV so there's nothing fundamentally preventing the migration between > > > > the two. In particular, we see it as a means to switch between the > > > > storage backend transports via live migration without disrupting the > > > > guest. > > > > > > > > Migration-wise virtio-blk and vhost-user-blk have in common > > > > > > > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE > > > > > > > > The two differ in > > > > > > > > - the name and the version of the VMStateDescription > > > > > > > > - virtio-blk has an extra migration section (via .save/.load callbacks > > > > on VirtioDeviceClass) containing requests in flight > > > > > > > > It looks like to become migration-compatible with virtio-blk, > > > > vhost-user-blk has to start using VMStateDescription of virtio-blk and > > > > provide compatible .save/.load callbacks. It isn't entirely obvious how > > > > to make this machine-type-dependent, so we came up with a simpler idea > > > > of defining a new device that shares most of the implementation with the > > > > original vhost-user-blk except for the migration stuff. We're certainly > > > > open to suggestions on how to reconcile this under a single > > > > vhost-user-blk device, as this would be more user-friendly indeed. > > > > > > > > We considered using a class property for this and defining the > > > > respective compat clause, but IIUC the class constructors (where .vmsd > > > > and .save/.load are defined) are not supposed to depend on class > > > > properties. > > > > > > > > Thanks, > > > > Roman. > > > > > > So the question is how to make vmsd depend on machine type. > > > CC Eduardo who poked at this kind of compat stuff recently, > > > paolo who looked at qom things most recently and dgilbert > > > for advice on migration. > > > > I don't think I've seen anyone change vmsd name dependent on machine > > type; making fields appear/disappear is easy - that just ends up as a > > property on the device that's checked; I guess if that property is > > global (rather than per instance) then you can check it in > > vhost_user_blk_class_init and swing the dc->vmsd pointer? > > class_init can be called very early during QEMU initialization, > so it's too early to make decisions based on machine type. > > Making a specific vmsd appear/disappear based on machine > configuration or state is "easy", by implementing > VMStateDescription.needed. But this would require registering > both vmsds (one of them would need to be registered manually > instead of using DeviceClass.vmsd). > > I don't remember what are the consequences of not using > DeviceClass.vmsd to register a vmsd, I only remember it was > subtle. See commit b170fce3dd06 ("cpu: Register >
Re: Deprecate the ppc405 boards in QEMU?
On Tue, 5 Oct 2021, Thomas Huth wrote: On 05/10/2021 14.17, BALATON Zoltan wrote: On Tue, 5 Oct 2021, Thomas Huth wrote: On 05/10/2021 10.07, Thomas Huth wrote: On 05/10/2021 10.05, Alexey Kardashevskiy wrote: [...] What is so special about taihu? taihu is the other 405 board defined in hw/ppc/ppc405_boards.c (which I suggested to deprecate now) I've now also played with the u-boot sources a little bit, and with some bit of tweaking, it's indeed possible to compile the old taihu board there. However, it does not really work with QEMU anymore, it immediately triggers an assert(): $ qemu-system-ppc -M taihu -bios u-boot.bin -serial null -serial mon:stdio ** ERROR:accel/tcg/tcg-accel-ops.c:79:tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()) Aborted (core dumped) Maybe it's similar to this: 2025fc6766ab25501e0041c564c44bb0f7389774 The helper_load_dcr() and helper_store_dcr() in target/ppc/timebase_helper.c seem to lock/unlock the iothread but I'm not sure if that's necessary. Also not sure why this does not happen with 460ex but that maybe uses different code. It's rather the other way round, the locking is missing here instead. I can get the serial output with the current QEMU when I add the following patch (not sure whether that's the right spot, though): diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index f5d012f860..bb57f1c9ed 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -336,6 +336,8 @@ void store_40x_dbcr0(CPUPPCState *env, uint32_t val) { PowerPCCPU *cpu = env_archcpu(env); +qemu_mutex_lock_iothread(); + switch ((val >> 28) & 0x3) { case 0x0: /* No action */ @@ -353,6 +355,8 @@ void store_40x_dbcr0(CPUPPCState *env, uint32_t val) ppc40x_system_reset(cpu); break; } + +qemu_mutex_unlock_iothread(); } /* PowerPC 40x internal IRQ controller */ Going back to QEMU v2.3.0, I can see at least a little bit of output, but it then also triggers an assert() during DRAM initialization: $ qemu-system-ppc -M taihu -bios u-boot.bin -serial null -serial mon:stdio Reset PowerPC core U-Boot 2014.10-rc2-00123-g461be2f96e-dirty (Oct 05 2021 - 10:02:56) CPU: AMCC PowerPC 405EP Rev. B at 770 MHz (PLB=256 OPB=128 EBC=128) I2C boot EEPROM disabled Internal PCI arbiter enabled 16 KiB I-Cache 16 KiB D-Cache Board: Taihu - AMCC PPC405EP Evaluation Board I2C: ready DRAM: qemu-system-ppc: memory.c:1693: memory_region_del_subregion: Assertion `subregion->container == mr' failed. Aborted (core dumped) The assert can be avoided with this patch: diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c index 980c48944f..3a4a094772 100644 --- a/hw/ppc/ppc4xx_devs.c +++ b/hw/ppc/ppc4xx_devs.c @@ -169,7 +170,8 @@ static target_ulong sdram_size (uint32_t bcr) static void sdram_set_bcr(ppc4xx_sdram_t *sdram, int i, uint32_t bcr, int enabled) { -if (sdram->bcr[i] & 0x0001) { +if (sdram->bcr[i] & 0x0001 && +memory_region_is_mapped(>containers[i])) { /* Unmap RAM */ #ifdef DEBUG_SDRAM printf("%s: unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n", @@ -220,8 +222,7 @@ static void sdram_unmap_bcr (ppc4xx_sdram_t *sdram) printf("%s: Unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n", __func__, sdram_base(sdram->bcr[i]), sdram_size(sdram->bcr[i])); #endif -memory_region_del_subregion(get_system_memory(), ->ram_memories[i]); +sdram_set_bcr(sdram, i, 0x, 0); } } which then detects 128MiB RAM but leaves the controller disabled and thus RAM unmapped then it does not continue (not sure if because of disabled SDRAM controller or some other reason). I get this with #define DEBUG_SDRAM: Board: Taihu - AMCC PPC405EP Evaluation Board I2C: ready DRAM: dcr_write_sdram: enable SDRAM controller sdram_set_bcr: Map RAM area 0400 sdram_set_bcr: Map RAM area 0400 0400 dcr_write_sdram: disable SDRAM controller sdram_unmap_bcr: Unmap RAM area 0400 sdram_set_bcr: unmap RAM area 0400 sdram_unmap_bcr: Unmap RAM area 0400 0400 sdram_set_bcr: unmap RAM area 0400 0400 dcr_write_sdram: enable SDRAM controller sdram_set_bcr: Map RAM area 0400 sdram_set_bcr: Map RAM area 0400 0400 sdram_set_bcr: unmap RAM area 0400 0400 dcr_write_sdram: disable SDRAM controller sdram_unmap_bcr: Unmap RAM area 0400 sdram_set_bcr: unmap RAM area 0400 sdram_unmap_bcr: Unmap RAM area 0040 128 MiB If this is simliar to the sam460ex u-boot then AFAIR that looks for SPD data from memory modules and sets up RAM according to those at this point (probably the same here as there's an i2c init before DRAM) then also runs some speed calibration routine that may need
Re: [PULL 00/12] Misc changes for 2021-10-05
On 10/5/21 9:43 AM, Paolo Bonzini wrote: The following changes since commit 9618c5badaa8eed25259cf095ff880efb939fbe7: Merge remote-tracking branch 'remotes/vivier/tags/trivial-branch-for-6.2-pull-request' into staging (2021-10-04 16:27:35 -0700) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to bb647c49b8f1f986d8171dd61db65e8a8d255be0: meson: show library versions in the summary (2021-10-05 13:10:29 +0200) * Meson version update * fix search path when configuring with --cpu * support for measured SEV boot with -kernel (Dov) * fix missing BQL locks (Emanuele) * retrieve applesmc key from the host (Pedro) * KVM PV feature documentation (Vitaly) Applied, thanks. r~
Re: [PULL 00/57] pc,pci,virtio: features, fixes
On Tue, Oct 05, 2021 at 10:21:43AM -0700, Richard Henderson wrote: > On 10/5/21 9:00 AM, Michael S. Tsirkin wrote: > > The following changes since commit 9618c5badaa8eed25259cf095ff880efb939fbe7: > > > >Merge remote-tracking branch > > 'remotes/vivier/tags/trivial-branch-for-6.2-pull-request' into staging > > (2021-10-04 16:27:35 -0700) > > > > are available in the Git repository at: > > > >git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > > > for you to fetch changes up to c7d2f59cf940b8c8c52c29d5fa25613fe662f7b6: > > > >hw/i386/amd_iommu: Add description/category to TYPE_AMD_IOMMU_PCI > > (2021-10-05 11:46:45 -0400) > > > > > > pc,pci,virtio: features, fixes > > > > VDPA multiqueue support. > > A huge acpi refactoring. > > Fixes, cleanups all over the place. > > > > Signed-off-by: Michael S. Tsirkin > > > > > > Ani Sinha (3): > >bios-tables-test: allow changes in DSDT ACPI tables for q35 > >hw/i386/acpi: fix conflicting IO address range for acpi pci hotplug > > in q35 > >bios-tables-test: Update ACPI DSDT table golden blobs for q35 > > > > Dr. David Alan Gilbert (1): > >virtio-balloon: Fix page-poison subsection name > > > > Igor Mammedov (35): > >acpi: add helper routines to initialize ACPI tables > >acpi: build_rsdt: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_xsdt: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_slit: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_fadt: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_tpm2: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: acpi_build_hest: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: build_mcfg: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_hmat: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: nvdimm_build_nfit: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: nvdimm_build_ssdt: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: vmgenid_build_acpi: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: x86: build_dsdt: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: build_hpet: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_tpm_tcpa: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: arm/x86: build_srat: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: use build_append_int_noprefix() API to compose SRAT table > >acpi: build_dmar_q35: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: build_waet: use acpi_table_begin()/acpi_table_end() instead of > > build_header() > >acpi: build_amd_iommu: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: madt: arm/x86: use acpi_table_begin()/acpi_table_end() instead > > of build_header() > >acpi: x86: remove dead code > >acpi: x86: set enabled when composing _MAT entries > >acpi: x86: madt: use build_append_int_noprefix() API to compose MADT > > table > >acpi: arm/virt: madt: use build_append_int_noprefix() API to compose > > MADT table > >acpi: build_dsdt_microvm: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: arm: virt: build_dsdt: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: arm: virt: build_iort: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: arm/virt: convert build_iort() to endian agnostic > > build_append_FOO() API > >acpi: arm/virt: build_spcr: fix invalid cast > >acpi: arm/virt: build_spcr: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: arm/virt: build_gtdt: use acpi_table_begin()/acpi_table_end() > > instead of build_header() > >acpi: build_facs: use build_append_int_noprefix() API to compose > > table > >acpi: remove no longer used build_header() > >acpi: AcpiGenericAddress no longer used to map/access fields of > > MMIO, drop packed attribute > > > > Jason Wang (10): > >vhost-vdpa: open device fd in net_init_vhost_vdpa() > >vhost-vdpa: classify one time request > >vhost-vdpa: prepare for the multiqueue support > >vhost-vdpa: let net_vhost_vdpa_init() returns NetClientState * > >net:
Re: [PATCH v7 3/8] qmp: add QMP command x-debug-query-virtio
On Tue, Oct 05, 2021 at 12:45:48PM -0400, Jonah Palmer wrote: > From: Laurent Vivier > > This new command lists all the instances of VirtIODevice with > their QOM paths and virtio type/name. > > Signed-off-by: Jonah Palmer > --- > hw/virtio/meson.build | 2 ++ > hw/virtio/virtio-stub.c| 14 ++ > hw/virtio/virtio.c | 27 +++ > include/hw/virtio/virtio.h | 1 + > qapi/meson.build | 1 + > qapi/qapi-schema.json | 1 + > qapi/virtio.json | 66 > ++ > tests/qtest/qmp-cmd-test.c | 1 + > 8 files changed, 113 insertions(+) > create mode 100644 hw/virtio/virtio-stub.c > create mode 100644 qapi/virtio.json > > [Jonah: VirtioInfo member 'type' is now of type string and no longer > relies on defining a QAPI list of virtio device type enumerations > to match the VirtIODevice name with qapi_enum_parse().] Hmm; depending on how much information you want to cram in strings, we may want to rebase this series on top of Dan's work to add the HumanReadableText QAPI type: https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg07717.html > +++ b/qapi/virtio.json > @@ -0,0 +1,66 @@ > +# -*- Mode: Python -*- > +# vim: filetype=python > +# > + > +## > +# = Virtio devices > +## > + > +## > +# @VirtioInfo: > +# > +# Information about a given VirtIODevice > +# > +# @path: VirtIO device canonical QOM path. > +# > +# @type: VirtIO device name. > +# > +# Since: 6.2 > +# > +## > +{ 'struct': 'VirtioInfo', > +'data': { > +'path': 'str', > +'type': 'str' > +} > +} > + > +## > +# @x-debug-query-virtio: > +# > +# Return a list of all initalized VirtIO devices > +# > +# Returns: list of gathered @VirtioInfo devices > +# > +# Since: 6.2 > +# > +# Example: > +# > +# -> { "execute": "x-debug-query-virtio" } > +# <- { "return": [ > +#{ > +#"path": "/machine/peripheral-anon/device[4]/virtio-backend", > +#"type": "virtio-input" > +#}, > +#{ > +#"path": "/machine/peripheral/crypto0/virtio-backend", > +#"type": "virtio-crypto" > +#}, > +#{ > +#"path": "/machine/peripheral-anon/device[2]/virtio-backend", > +#"type": "virtio-scsi" > +#}, > +#{ > +#"path": "/machine/peripheral-anon/device[1]/virtio-backend", > +#"type": "virtio-net" > +#}, > +#{ > +#"path": "/machine/peripheral-anon/device[0]/virtio-backend", > +#"type": "virtio-serial" > +#} > +# ] > +#} > +# > +## > + > +{ 'command': 'x-debug-query-virtio', 'returns': ['VirtioInfo'] } But for now, it looks like 'str' is the correct type. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 1/8] virtio: drop name parameter for virtio_init()
On Tue, Oct 05, 2021 at 12:45:46PM -0400, Jonah Palmer wrote: > This patch drops the name parameter for the virtio_init function. > > The pair between the numeric device ID and the string device ID > (name) of a virtio device already exists, but not in a way that > let's us map between them. s/let's/lets/ > > This patch will let us do this and removes the need for the name > parameter in virtio_init(). > > Signed-off-by: Jonah Palmer > --- > +++ b/hw/virtio/virtio.c > @@ -133,6 +133,43 @@ struct VirtQueue > QLIST_ENTRY(VirtQueue) node; > }; > > +const char *virtio_device_names[] = { > +[VIRTIO_ID_NET] = "virtio-net", > +[VIRTIO_ID_BLOCK] = "virtio-blk", > +[VIRTIO_ID_CONSOLE] = "virtio-serial", > +[VIRTIO_ID_RNG] = "virtio-rng", > +[VIRTIO_ID_BALLOON] = "virtio-balloon", > +[VIRTIO_ID_IOMEM] = "virtio-iomem", > +[VIRTIO_ID_RPMSG] = "virtio-rpmsg", > +[VIRTIO_ID_SCSI] = "virtio-scsi", > +[VIRTIO_ID_9P] = "virtio-9p", > +[VIRTIO_ID_MAC80211_WLAN] = "virtio-mac-wlan", > +[VIRTIO_ID_RPROC_SERIAL] = "virtio-rproc-serial", > +[VIRTIO_ID_CAIF] = "virtio-caif", > +[VIRTIO_ID_MEMORY_BALLOON] = "virtio-mem-balloon", > +[VIRTIO_ID_GPU] = "virtio-gpu", > +[VIRTIO_ID_CLOCK] = "virtio-clk", > +[VIRTIO_ID_INPUT] = "virtio-input", > +[VIRTIO_ID_VSOCK] = "vhost-vsock", > +[VIRTIO_ID_CRYPTO] = "virtio-crypto", > +[VIRTIO_ID_SIGNAL_DIST] = "virtio-signal", > +[VIRTIO_ID_PSTORE] = "virtio-pstore", > +[VIRTIO_ID_IOMMU] = "virtio-iommu", > +[VIRTIO_ID_MEM] = "virtio-mem", > +[VIRTIO_ID_SOUND] = "virtio-sound", > +[VIRTIO_ID_FS] = "vhost-user-fs", > +[VIRTIO_ID_PMEM] = "virtio-pmem", > +[VIRTIO_ID_MAC80211_HWSIM] = "virtio-mac-hwsim", > +[VIRTIO_ID_I2C_ADAPTER] = "vhost-user-i2c", > +[VIRTIO_ID_BT] = "virtio-bluetooth" > +}; Are these IDs consecutive, or can the array have gaps? > + > +static const char *virtio_id_to_name(uint16_t device_id) > +{ > +assert(device_id < G_N_ELEMENTS(virtio_device_names)); > +return virtio_device_names[device_id]; If the latter, you may also want to assert that you aren't returning NULL. > +++ b/include/standard-headers/linux/virtio_ids.h > @@ -55,6 +55,7 @@ > #define VIRTIO_ID_FS 26 /* virtio filesystem */ > #define VIRTIO_ID_PMEM 27 /* virtio pmem */ > #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */ > +#define VIRTIO_ID_I2C_ADAPTER 34 /* virtio I2C adapater */ > #define VIRTIO_ID_BT 40 /* virtio bluetooth */ And it looks like the array has gaps. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[PATCH] tests/docker/dockerfiles: Bump fedora-i386-cross to fedora 34
For unknown and unrepeatable reasons, the cross-i386-tci test has started failing. "Fix" this by updating the container to use fedora 34. Add sysprof-capture-devel as a new dependency of glib2-devel that was not correctly spelled out in the rpm rules. Use dnf update Just In Case -- there are presently out-of-date packages in the upstream docker registry. Signed-off-by: Richard Henderson --- tests/docker/dockerfiles/fedora-i386-cross.docker | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker index 820740d5be..f62a71ce22 100644 --- a/tests/docker/dockerfiles/fedora-i386-cross.docker +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker @@ -1,4 +1,5 @@ -FROM registry.fedoraproject.org/fedora:33 +FROM registry.fedoraproject.org/fedora:34 + ENV PACKAGES \ bzip2 \ ccache \ @@ -20,10 +21,11 @@ ENV PACKAGES \ pcre-devel.i686 \ perl-Test-Harness \ pixman-devel.i686 \ +sysprof-capture-devel.i686 \ zlib-devel.i686 ENV QEMU_CONFIGURE_OPTS --cpu=i386 --disable-vhost-user ENV PKG_CONFIG_LIBDIR /usr/lib/pkgconfig -RUN dnf install -y $PACKAGES +RUN dnf update -y && dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt -- 2.25.1
[RFC PATCH] meson.build: don't include libbpf in the common source set
This library is only needed for the softmmu targets and as such break static *-user builds where libbpf is detected and it tries to link it into the user binaries. Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") Signed-off-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 60f4f45165..d8bcf13b21 100644 --- a/meson.build +++ b/meson.build @@ -2307,7 +2307,7 @@ subdir('bsd-user') subdir('linux-user') subdir('ebpf') -common_ss.add(libbpf) +softmmu_ss.add(libbpf) bsd_user_ss.add(files('gdbstub.c')) specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss) -- 2.30.2
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On Tue, Oct 5, 2021 at 2:41 PM Laurent Vivier wrote: > Le 05/10/2021 à 21:26, Paolo Bonzini a écrit : > > On 27/09/21 11:52, Daniel P. Berrangé wrote: > >>bsd_user_ss.add(files('gdbstub.c')) > >>specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss) > >> > >> > >> So without this change, we're already correctly dropping bsd_user_ss > >> in its entirity, when not on BSD. > >> > >> With this change, we're dropping some, but not all, of bsd_user_ss > >> files - gdbstub.c remains. > >> > >> So this change on its own doesn't make a whole lot of sense. > > > > Agreed; that said, the gdbstub.c files added by > > > > bsd_user_ss.add(files('gdbstub.c')) > > linux_user_ss.add(files('gdbstub.c', 'thunk.c')) > > > > are unnecessary because there is already > > > > specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone) > > > > So, with those two instances of gdbstub.c removed, both patches are > > > > Acked-by: Paolo Bonzini > > > > I can take them if a v2 is sent updated accordingly... > I'd planned on rolling them into the bsd-user patch set that I'd been working on that is almost ready to go in (this is I think the only remaining issue with the patch train, though I need to double check threads). I'd planned on doing that tomorrow, but would be happy to coordinate with you since one of them does touch linux-user. Warner
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
Le 05/10/2021 à 21:26, Paolo Bonzini a écrit : > On 27/09/21 11:52, Daniel P. Berrangé wrote: >> bsd_user_ss.add(files('gdbstub.c')) >> specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss) >> >> >> So without this change, we're already correctly dropping bsd_user_ss >> in its entirity, when not on BSD. >> >> With this change, we're dropping some, but not all, of bsd_user_ss >> files - gdbstub.c remains. >> >> So this change on its own doesn't make a whole lot of sense. > > Agreed; that said, the gdbstub.c files added by > > bsd_user_ss.add(files('gdbstub.c')) > linux_user_ss.add(files('gdbstub.c', 'thunk.c')) > > are unnecessary because there is already > > specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone) > > So, with those two instances of gdbstub.c removed, both patches are > > Acked-by: Paolo Bonzini > I can take them if a v2 is sent updated accordingly... Thanks, Laurent
Re: [RFC PATCH] meson.build: don't include libbpf in the common source set
On 10/5/21 12:27 PM, Paolo Bonzini wrote: On 05/10/21 20:24, Alex Bennée wrote: This library is only needed for the softmmu targets and as such break static *-user builds where libbpf is detected and it tries to link it into the user binaries. Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") Signed-off-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 60f4f45165..d8bcf13b21 100644 --- a/meson.build +++ b/meson.build @@ -2307,7 +2307,7 @@ subdir('bsd-user') subdir('linux-user') subdir('ebpf') -common_ss.add(libbpf) +softmmu_ss.add(libbpf) It should not be needed at all, since ebpf/meson.build has softmmu_ss.add(when: libbpf, if_true: files('ebpf_rss.c'), if_false: files('ebpf_rss-stub.c')) (which already adds libbpf if needed). Ooo, magic side effects. I'll note that the manual doesn't say that it adds and dependencies from varnames_and_deps, only that it checks them. r~
Re: [PATCH 10/13] virtiofsd: Custom threadpool for remote blocking posix locks requests
On Mon, Oct 04, 2021 at 03:54:31PM +0100, Stefan Hajnoczi wrote: > On Thu, Sep 30, 2021 at 11:30:34AM -0400, Vivek Goyal wrote: > > Add a new custom threadpool using posix threads that specifically > > service locking requests. > > > > In the case of a fcntl(SETLKW) request, if the guest is waiting > > for a lock or locks and issues a hard-reboot through SYSRQ then virtiofsd > > unblocks the blocked threads by sending a signal to them and waking > > them up. > > > > The current threadpool (GThreadPool) is not adequate to service the > > locking requests that result in a thread blocking. That is because > > GLib does not provide an API to cancel the request while it is > > serviced by a thread. In addition, a user might be running virtiofsd > > without a threadpool (--thread-pool-size=0), thus a locking request > > that blocks, will block the main virtqueue thread that services requests > > from servicing any other requests. > > > > The only exception occurs when the lock is of type F_UNLCK. In this case > > the request is serviced by the main virtqueue thread or a GThreadPool > > thread to avoid a deadlock, when all the threads in the custom threadpool > > are blocked. > > > > Then virtiofsd proceeds to cleanup the state of the threads, release > > them back to the system and re-initialize. > > Is there another way to cancel SETLKW without resorting to a new thread > pool? Since this only matters when shutting down or restarting, can we > close all plock->fd file descriptors to kick the GThreadPool workers out > of fnctl()? Ok, I tested this. If a thread is blocked on OFD lock and another thread closes associated "fd", it does not unblock the thread which is blocked on lock. So closing OFD can't be used for unblocking a thread. Even if it could be, it can't be a replacement for a thread pool in general as we can't block main thread otherwise it can deadlock. But we could have used another glib thread pool (instead of a custom thread pool which can handle signals to unblock threads). If you are curious, here is my test program. https://github.com/rhvgoyal/misc/blob/master/virtiofs-tests/ofd-lock.c Comments in there explain how to use it. It can block on an OFD lock and one can send SIGUSR1 which will close fd. Thanks Vivek
Re: [PATCH v4 11/11] tests/acpi: add expected blobs for VIOT test on q35 machine
On 10/1/21 7:33 PM, Jean-Philippe Brucker wrote: > Add expected blobs of the VIOT and DSDT table for the VIOT test on the > q35 machine. > > Since the test instantiates a virtio device and two PCIe expander > bridges, DSDT.viot has more blocks than the base DSDT (long diff not > shown here). The VIOT table generated for the q35 test is: > > [000h 4]Signature : "VIOT"[Virtual I/O > Translation Table] > [004h 0004 4] Table Length : 0070 > [008h 0008 1] Revision : 00 > [009h 0009 1] Checksum : 3D > [00Ah 0010 6] Oem ID : "BOCHS " > [010h 0016 8] Oem Table ID : "BXPC" > [018h 0024 4] Oem Revision : 0001 > [01Ch 0028 4] Asl Compiler ID : "BXPC" > [020h 0032 4]Asl Compiler Revision : 0001 > > [024h 0036 2] Node count : 0003 > [026h 0038 2] Node offset : 0030 > [028h 0040 8] Reserved : > > [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] > [031h 0049 1] Reserved : 00 > [032h 0050 2] Length : 0010 > > [034h 0052 2] PCI Segment : > [036h 0054 2] PCI BDF number : 0010 > [038h 0056 8] Reserved : > > [040h 0064 1] Type : 01 [PCI Range] > [041h 0065 1] Reserved : 00 > [042h 0066 2] Length : 0018 > > [044h 0068 4] Endpoint start : 3000 > [048h 0072 2]PCI Segment start : > [04Ah 0074 2] PCI Segment end : > [04Ch 0076 2]PCI BDF start : 3000 > [04Eh 0078 2] PCI BDF end : 30FF > [050h 0080 2] Output node : 0030 > [052h 0082 6] Reserved : > > [058h 0088 1] Type : 01 [PCI Range] > [059h 0089 1] Reserved : 00 > [05Ah 0090 2] Length : 0018 > > [05Ch 0092 4] Endpoint start : 1000 > [060h 0096 2]PCI Segment start : > [062h 0098 2] PCI Segment end : > [064h 0100 2]PCI BDF start : 1000 > [066h 0102 2] PCI BDF end : 10FF > [068h 0104 2] Output node : 0030 > [06Ah 0106 6] Reserved : > > Signed-off-by: Jean-Philippe Brucker Reviewed-by: Eric Auger Eric > --- > tests/qtest/bios-tables-test-allowed-diff.h | 2 -- > tests/data/acpi/q35/DSDT.viot | Bin 0 -> 9398 bytes > tests/data/acpi/q35/VIOT.viot | Bin 0 -> 112 bytes > 3 files changed, 2 deletions(-) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h > b/tests/qtest/bios-tables-test-allowed-diff.h > index fa213e4738..dfb8523c8b 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1,3 +1 @@ > /* List of comma-separated changed AML files to ignore */ > -"tests/data/acpi/q35/DSDT.viot", > -"tests/data/acpi/q35/VIOT.viot", > diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot > index > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..b41270ff6d63493c2ae379ddd1d3e28f190a6c01 > 100644 > GIT binary patch > literal 9398 > zcmeHNO>7&-8J*>iv|O}G~OOGG$M|57BBoWHhc5OS9yDTx$CQgH$r;8Idr*-4Q_ > z5(9Az1F`}niVsB-#zBvCpa8wKr(7GLxfJNZhXOUwQxCo5S`_gq>icGPq#2R|qEj!C > zfZhFO-E)yj*v)_%j$|bWD4v61&3MJ6@sGF_Mv( > z(Y~GJ$Ji9i%ul_-ddc|xw*RT`zx{!4bOW~WnR9oe8@#vYZ!iK~-v}&=4xHj-r&;K< > zcU`OQR*iT=DGueakdEt~iRCoxImzW@o+PvCPVNXSM0Z?!3la@A7=V7VmARrY)yk > z{pY1`=FY$P>E*ZcU;gqRzq<396$4-adlUOh0d4%7zBT9fosWB0jax+L=jQv zQRdK@z^9UXwkV>i=J#J~?>_G}@-A=VM7>texw(0?%WX7MbJqC}W*M`obLj6+2L}g# > z7KhBa!JMioR2I#0z1Wf}4QL}(?VWPHRb@6~_rFcDSo^j^@$^f@nwPCNyiPXrY^T}E > zvw%wcfQq{B`j+GO?T>ms>-oupgMHSY{HWJupLA{Zum8sP*}gR;+Lp2=-%n6m?tjZ- > zjG;9@c#>K}{oUR@TWRJyyo-^34o#_78fy{Dw`^y5>Zzy%5~{uX^m4%iSX`qhT8~!A > zG^eeZlHoI-8Ai$2Vq4f>h#*^g_hNN*{g5>^t+7liet~+Zy}PhdZ_UfPW8!)n8rHEU > zO2#|UccP|wVTaee;I38=IdP!Tn zmOARAox0m>8Og6~%fzLjz(wD!XR-0J?VV zfm^7pSF`ns_j0yv6jt12mU+DH7MCLJ$0#~D2(}3k+%T>(s-yiwD+AC-UHoLQ!1- > zZTt}HXS}hx*Q`$VSHhuj|GB^ZyZOw!)sJSsuAcdeTMekL*MH;pAM0IX{WHC*Rs
Re: [PATCH v4 09/11] tests/acpi: add test cases for VIOT
On 10/1/21 7:33 PM, Jean-Philippe Brucker wrote: > Add two test cases for VIOT, one on the q35 machine and the other on > virt. To test complex topologies the q35 test has two PCIe buses that > bypass the IOMMU (and are therefore not described by VIOT), and two > buses that are translated by virtio-iommu. > > Signed-off-by: Jean-Philippe Brucker > --- > tests/qtest/bios-tables-test.c | 38 ++ > 1 file changed, 38 insertions(+) > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > index 4f11d03055..b6cb383bd9 100644 > --- a/tests/qtest/bios-tables-test.c > +++ b/tests/qtest/bios-tables-test.c > @@ -1403,6 +1403,42 @@ static void test_acpi_virt_tcg(void) > free_test_data(); > } > > +static void test_acpi_q35_viot(void) > +{ > +test_data data = { > +.machine = MACHINE_Q35, > +.variant = ".viot", > +}; > + > +/* > + * To keep things interesting, two buses bypass the IOMMU. > + * VIOT should only describes the other two buses. > + */ > +test_acpi_one("-machine default_bus_bypass_iommu=on " > + "-device virtio-iommu " may be better to use virtio-iommu-pci? > + "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 " > + "-device > pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on " > + "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0", > + ); > +free_test_data(); > +} > + > +static void test_acpi_virt_viot(void) > +{ > +test_data data = { > +.machine = "virt", > +.uefi_fl1 = "pc-bios/edk2-aarch64-code.fd", > +.uefi_fl2 = "pc-bios/edk2-arm-vars.fd", > +.cd = > "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2", > +.ram_start = 0x4000ULL, > +.scan_len = 128ULL * 1024 * 1024, > +}; > + > +test_acpi_one("-cpu cortex-a57 " > + "-device virtio-iommu", ); > +free_test_data(); > +} > + > static void test_oem_fields(test_data *data) > { > int i; > @@ -1567,12 +1603,14 @@ int main(int argc, char *argv[]) > if (strcmp(arch, "x86_64") == 0) { > qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg); > } > +qtest_add_func("acpi/q35/viot", test_acpi_q35_viot); > } else if (strcmp(arch, "aarch64") == 0) { > qtest_add_func("acpi/virt", test_acpi_virt_tcg); > qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem); > qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp); > qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb); > qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt); > +qtest_add_func("acpi/virt/viot", test_acpi_virt_viot); > } > ret = g_test_run(); > boot_sector_cleanup(disk); Besides Reviewed-by: Eric Auger Eric
Re: [PATCH v4 10/11] tests/acpi: add expected blob for VIOT test on virt machine
Hi Jean, On 10/1/21 7:33 PM, Jean-Philippe Brucker wrote: > The VIOT blob contains the following: > > [000h 4]Signature : "VIOT"[Virtual I/O > Translation Table] > [004h 0004 4] Table Length : 0058 > [008h 0008 1] Revision : 00 > [009h 0009 1] Checksum : 66 > [00Ah 0010 6] Oem ID : "BOCHS " > [010h 0016 8] Oem Table ID : "BXPC" > [018h 0024 4] Oem Revision : 0001 > [01Ch 0028 4] Asl Compiler ID : "BXPC" > [020h 0032 4]Asl Compiler Revision : 0001 > > [024h 0036 2] Node count : 0002 > [026h 0038 2] Node offset : 0030 > [028h 0040 8] Reserved : > > [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU] > [031h 0049 1] Reserved : 00 > [032h 0050 2] Length : 0010 > > [034h 0052 2] PCI Segment : > [036h 0054 2] PCI BDF number : 0008 > [038h 0056 8] Reserved : > > [040h 0064 1] Type : 01 [PCI Range] > [041h 0065 1] Reserved : 00 > [042h 0066 2] Length : 0018 > > [044h 0068 4] Endpoint start : > [048h 0072 2]PCI Segment start : > [04Ah 0074 2] PCI Segment end : > [04Ch 0076 2]PCI BDF start : > [04Eh 0078 2] PCI BDF end : 00FF > [050h 0080 2] Output node : 0030 > [052h 0082 6] Reserved : I noticed the spec does not clearly say the virtio-iommu-pci BDF does not need to be excluded from the PCI range. Shouldn't it be clarified? Besides Reviewed-by: Eric Auger Eric > > Signed-off-by: Jean-Philippe Brucker > --- > tests/qtest/bios-tables-test-allowed-diff.h | 1 - > tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes > 2 files changed, 1 deletion(-) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h > b/tests/qtest/bios-tables-test-allowed-diff.h > index 29b5b1eabc..fa213e4738 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1,4 +1,3 @@ > /* List of comma-separated changed AML files to ignore */ > -"tests/data/acpi/virt/VIOT", > "tests/data/acpi/q35/DSDT.viot", > "tests/data/acpi/q35/VIOT.viot", > diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT > index > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d > 100644 > GIT binary patch > literal 88 > zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN`)2L0cSHX > I{D-Rq0Q5fy0RR91 > > literal 0 > HcmV?d1 >
Re: [RFC PATCH] meson.build: don't include libbpf in the common source set
On 05/10/21 20:24, Alex Bennée wrote: This library is only needed for the softmmu targets and as such break static *-user builds where libbpf is detected and it tries to link it into the user binaries. Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") Signed-off-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 60f4f45165..d8bcf13b21 100644 --- a/meson.build +++ b/meson.build @@ -2307,7 +2307,7 @@ subdir('bsd-user') subdir('linux-user') subdir('ebpf') -common_ss.add(libbpf) +softmmu_ss.add(libbpf) It should not be needed at all, since ebpf/meson.build has softmmu_ss.add(when: libbpf, if_true: files('ebpf_rss.c'), if_false: files('ebpf_rss-stub.c')) (which already adds libbpf if needed). Paolo
Re: [RFC PATCH] meson.build: don't include libbpf in the common source set
On 05/10/21 21:00, Philippe Mathieu-Daudé wrote: On 10/5/21 20:24, Alex Bennée wrote: This library is only needed for the softmmu targets and as such break static *-user builds where libbpf is detected and it tries to link it into the user binaries. Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") Signed-off-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 60f4f45165..d8bcf13b21 100644 --- a/meson.build +++ b/meson.build @@ -2307,7 +2307,7 @@ subdir('bsd-user') subdir('linux-user') subdir('ebpf') -common_ss.add(libbpf) +softmmu_ss.add(libbpf) bsd_user_ss.add(files('gdbstub.c')) specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss) Patch already pending on the list: https://www.mail-archive.com/qemu-devel@nongnu.org/msg834876.html Not the same patch, that one is already in. Paolo
Re: [PATCH v3] target/i386: Include 'hw/i386/apic.h' locally
On 10/5/21 18:45, Laurent Vivier wrote: > Le 05/10/2021 à 16:57, Michael S. Tsirkin a écrit : >> On Wed, Sep 29, 2021 at 06:31:24PM +0200, Philippe Mathieu-Daudé wrote: >>> Instead of including a sysemu-specific header in "cpu.h" >>> (which is shared with user-mode emulations), include it >>> locally when required. >>> >>> Acked-by: Paolo Bonzini >>> Signed-off-by: Philippe Mathieu-Daudé >> >> Acked-by: Michael S. Tsirkin >> >> which tree? trivial I guess? > > Yes, but for me the patch was not correct because there is no need to update > target/i386/cpu-dump.c > But perhaps I misunderstood the answer from Philippe? You understood correctly, this series requires a respin.
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On 27/09/21 11:52, Daniel P. Berrangé wrote: bsd_user_ss.add(files('gdbstub.c')) specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss) So without this change, we're already correctly dropping bsd_user_ss in its entirity, when not on BSD. With this change, we're dropping some, but not all, of bsd_user_ss files - gdbstub.c remains. So this change on its own doesn't make a whole lot of sense. Agreed; that said, the gdbstub.c files added by bsd_user_ss.add(files('gdbstub.c')) linux_user_ss.add(files('gdbstub.c', 'thunk.c')) are unnecessary because there is already specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone) So, with those two instances of gdbstub.c removed, both patches are Acked-by: Paolo Bonzini Thanks, Paolo
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On 27/09/21 11:54, Peter Maydell wrote: True, but "meson.build is evaluated but just does nothing or adds files to a sourceset that isn't used" is pretty common (hw/pci/meson.build is evaluated even if we're not building a system with PCI support, for example). Selection of files from hw/pci/meson.build is based on per-target definitions, so there's no way around when:/if_true:. (Technically, hw/pci/meson.build also as an if_false, so there's *really* no way around parsing it). Instead, when the definition is constant across all targets, it is possible to use either when:/if_true: or an "if" as in if have_user util_ss.add(files('selfmap.c')) endif or the various "if m[0].found()" found in directories that build shared modules. In this case I personally lean more towards the latter, but when:/if_true: is a little more compact so both are acceptable. Paolo
Re: [PATCH v4 07/11] pc: Allow instantiating a virtio-iommu device
Hi Jean, On 10/1/21 7:33 PM, Jean-Philippe Brucker wrote: > Allow instantiating a virtio-iommu device by adding an ACPI Virtual I/O > Translation table (VIOT), which describes the relation between the > virtio-iommu and the endpoints it manages. > > Add a hotplug handler for virtio-iommu on x86 and set the necessary > reserved region property. On x86, the [0xfee0, 0xfeef] DMA > region is reserved for MSIs. DMA transactions to this range either > trigger IRQ remapping in the IOMMU or bypasses IOMMU translation. > > Although virtio-iommu does not support IRQ remapping it must be informed > of the reserved region so that it can forward DMA transactions targeting > this region. > > Signed-off-by: Jean-Philippe Brucker Reviewed-by: Eric Auger tested by a protecting a virtio-net-pci device plugged onto a pxb-pcie and setting default-bus-bypass-iommu=true on pcie.0. As described in the cover letter, without [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG the ahci emits some failure if it is protected by the virtio-iommu: qemu-system-x86_64: virtio_iommu_translate sid=250 is not known!! qemu-system-x86_64: no buffer available in event queue to report event qemu-system-x86_64: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address ../.. Invalid access at addr 0x7FFA6900, size 4, region '(null)', reason: rejected But this is expected. So feel free to add Tested-by: Eric Auger Thanks Eric > --- > include/hw/i386/pc.h | 2 ++ > hw/i386/acpi-build.c | 5 + > hw/i386/pc.c | 24 ++-- > hw/i386/Kconfig | 1 + > 4 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 82cf7b7e30..f3ba1ee4c0 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -45,6 +45,8 @@ typedef struct PCMachineState { > bool pit_enabled; > bool hpet_enabled; > bool default_bus_bypass_iommu; > +bool virtio_iommu; > +uint16_t virtio_iommu_bdf; > uint64_t max_fw_size; > > /* ACPI Memory hotplug IO base address */ > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index d1c28440f4..4e46585709 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -71,6 +71,7 @@ > > #include "hw/acpi/ipmi.h" > #include "hw/acpi/hmat.h" > +#include "hw/acpi/viot.h" > > /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and > * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows > @@ -2593,6 +2594,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState > *machine) > build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id, > x86ms->oem_table_id); > } > +} else if (pcms->virtio_iommu) { > +acpi_add_table(table_offsets, tables_blob); > +build_viot(machine, tables_blob, tables->linker, > pcms->virtio_iommu_bdf, > + x86ms->oem_id, x86ms->oem_table_id); > } > if (machine->nvdimms_state->is_enabled) { > nvdimm_build_acpi(table_offsets, tables_blob, tables->linker, > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 789ccb6ef4..31710bc4fb 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -83,6 +83,7 @@ > #include "hw/i386/intel_iommu.h" > #include "hw/net/ne2000-isa.h" > #include "standard-headers/asm-x86/bootparam.h" > +#include "hw/virtio/virtio-iommu.h" > #include "hw/virtio/virtio-pmem-pci.h" > #include "hw/virtio/virtio-mem-pci.h" > #include "hw/mem/memory-device.h" > @@ -1367,8 +1368,11 @@ static void pc_virtio_md_pci_unplug(HotplugHandler > *hotplug_dev, > static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, >DeviceState *dev, Error **errp) > { > -if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) && > -x86_iommu_get_default()) { > +PCMachineState *pcms = PC_MACHINE(hotplug_dev); > + > +if ((object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) || > + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) && > +(x86_iommu_get_default() || pcms->virtio_iommu)) { > error_setg(errp, "QEMU does not support multiple vIOMMUs " > "for x86 yet."); > return; > @@ -1381,6 +1385,15 @@ static void > pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, > } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) || > object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) { > pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp); > +} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) { > +/* Declare the APIC range as the reserved MSI region */ > +char *resv_prop_str = g_strdup_printf("0xfee0:0xfeef:%d", > + VIRTIO_IOMMU_RESV_MEM_T_MSI); > + > +object_property_set_uint(OBJECT(dev), "len-reserved-regions", 1, > errp); > +
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
On 27/09/21 07:24, Philippe Mathieu-Daudé wrote: Why here and not in the parent meson.build? This is what Paolo recommended me to do last time I added a conditional inclusion. Personally I prefer having it in the call site rather than the callee (no need to read the callee to notice it isn't called). I guess this is for readability, to not clutter meson.build? files more... Yes, pretty much. In this case it's quite obvious that bsd-user is BSD-only, but I prefer it if dir/meson.build has the knowledge of what goes on in dir/. That said, we're not terribly consistent, see have_block and have_tools, so either will be okay. Paolo Paolo, what is your preference?
[RFC PATCH 3/4] hw/scsi/scsi-generic: Use automatic AIO context lock
Use the automatic AIO context acquire/release in scsi_command_complete(). Signed-off-by: Philippe Mathieu-Daudé --- hw/scsi/scsi-generic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 665baf900e4..08ef623c030 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -114,9 +114,9 @@ static void scsi_command_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; -aio_context_acquire(blk_get_aio_context(s->conf.blk)); -scsi_command_complete_noio(r, ret); -aio_context_release(blk_get_aio_context(s->conf.blk)); +WITH_AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(s->conf.blk)) { +scsi_command_complete_noio(r, ret); +} } static int execute_command(BlockBackend *blk, -- 2.31.1
[RFC PATCH 2/4] hw/scsi/scsi-disk: Use automatic AIO context lock
Use the automatic AIO context acquire/release in scsi_block_realize(). Signed-off-by: Philippe Mathieu-Daudé --- hw/scsi/scsi-disk.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index e8a547dbb7d..fa2d8543718 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2605,7 +2605,6 @@ static int get_device_type(SCSIDiskState *s) static void scsi_block_realize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); -AioContext *ctx; int sg_version; int rc; @@ -2620,8 +2619,7 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp) "be removed in a future version"); } -ctx = blk_get_aio_context(s->qdev.conf.blk); -aio_context_acquire(ctx); +AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(dev->conf.blk)); /* check we are using a driver managing SG_IO (version 3 and after) */ rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, _version); @@ -2630,18 +2628,18 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp) if (rc != -EPERM) { error_append_hint(errp, "Is this a SCSI device?\n"); } -goto out; +return; } if (sg_version < 3) { error_setg(errp, "scsi generic interface too old"); -goto out; +return; } /* get device type from INQUIRY data */ rc = get_device_type(s); if (rc < 0) { error_setg(errp, "INQUIRY failed"); -goto out; +return; } /* Make a guess for the block size, we'll fix it when the guest sends. @@ -2661,9 +2659,6 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp) scsi_realize(>qdev, errp); scsi_generic_read_device_inquiry(>qdev); - -out: -aio_context_release(ctx); } typedef struct SCSIBlockReq { -- 2.31.1
[RFC PATCH 1/4] block/aio: Add automatically released aio_context variants
Similarly to commit 5626f8c6d46 ("rcu: Add automatically released rcu_read_lock variants"): AIO_CONTEXT_ACQUIRE_GUARD() acquires the aio context and then uses glib's g_auto infrastructure (and thus whatever the compiler's hooks are) to release it on all exits of the block. WITH_AIO_CONTEXT_ACQUIRE_GUARD() is similar but is used as a wrapper for the lock, i.e.: WITH_AIO_CONTEXT_ACQUIRE_GUARD() { stuff with context acquired } Inspired-by: Dr. David Alan Gilbert Signed-off-by: Philippe Mathieu-Daudé --- include/block/aio.h | 24 1 file changed, 24 insertions(+) diff --git a/include/block/aio.h b/include/block/aio.h index 47fbe9d81f2..4fa5a5c2720 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -294,6 +294,30 @@ void aio_context_acquire(AioContext *ctx); /* Relinquish ownership of the AioContext. */ void aio_context_release(AioContext *ctx); +static inline AioContext *aio_context_auto_acquire(AioContext *ctx) +{ +aio_context_acquire(ctx); +return ctx; +} + +static inline void aio_context_auto_release(AioContext *ctx) +{ +aio_context_release(ctx); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(AioContext, aio_context_auto_release) + +#define WITH_AIO_CONTEXT_ACQUIRE_GUARD(ctx) \ +WITH_AIO_CONTEXT_ACQUIRE_GUARD_(glue(_aio_context_auto, __COUNTER__), ctx) + +#define WITH_AIO_CONTEXT_ACQUIRE_GUARD_(var, ctx) \ +for (g_autoptr(AioContext) var = aio_context_auto_acquire(ctx); \ +(var); aio_context_auto_release(var), (var) = NULL) + +#define AIO_CONTEXT_ACQUIRE_GUARD(ctx) \ +g_autoptr(AioContext) _aio_context_auto __attribute__((unused)) \ += aio_context_auto_acquire(ctx) + /** * aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will * run only once and as soon as possible. -- 2.31.1
Re: [RFC PATCH] meson.build: don't include libbpf in the common source set
On 10/5/21 20:24, Alex Bennée wrote: > This library is only needed for the softmmu targets and as such > break static *-user builds where libbpf is detected and it tries to > link it into the user binaries. > > Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") > Signed-off-by: Alex Bennée > --- > meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index 60f4f45165..d8bcf13b21 100644 > --- a/meson.build > +++ b/meson.build > @@ -2307,7 +2307,7 @@ subdir('bsd-user') > subdir('linux-user') > subdir('ebpf') > > -common_ss.add(libbpf) > +softmmu_ss.add(libbpf) > > bsd_user_ss.add(files('gdbstub.c')) > specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss) > Patch already pending on the list: https://www.mail-archive.com/qemu-devel@nongnu.org/msg834876.html
Re: [RFC PATCH] meson.build: don't include libbpf in the common source set
On 10/5/21 11:24 AM, Alex Bennée wrote: This library is only needed for the softmmu targets and as such break static *-user builds where libbpf is detected and it tries to link it into the user binaries. Fixes: 46627f41b6 ("ebpf: Added eBPF RSS loader.") Signed-off-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 60f4f45165..d8bcf13b21 100644 --- a/meson.build +++ b/meson.build @@ -2307,7 +2307,7 @@ subdir('bsd-user') subdir('linux-user') subdir('ebpf') -common_ss.add(libbpf) +softmmu_ss.add(libbpf) Indeed. I also think it should go into ebpf/meson.build, just to keep everything together. Something like softmmu_ss.add(when: libbpf, if_true: files('ebpf_rss.c', libbpf), if_false: files('ebpf_rss-stub.c')) r~
[RFC PATCH 4/4] hw/block/virtio-blk: Use automatic AIO context lock
Use the automatic AIO context acquire/release in virtio_blk_reset(). Signed-off-by: Philippe Mathieu-Daudé --- hw/block/virtio-blk.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index f139cd7cc9c..2dd6428e7b3 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -896,24 +896,22 @@ static void virtio_blk_dma_restart_cb(void *opaque, bool running, static void virtio_blk_reset(VirtIODevice *vdev) { VirtIOBlock *s = VIRTIO_BLK(vdev); -AioContext *ctx; VirtIOBlockReq *req; -ctx = blk_get_aio_context(s->blk); -aio_context_acquire(ctx); -blk_drain(s->blk); - -/* We drop queued requests after blk_drain() because blk_drain() itself can - * produce them. */ -while (s->rq) { -req = s->rq; -s->rq = req->next; -virtqueue_detach_element(req->vq, >elem, 0); -virtio_blk_free_request(req); +WITH_AIO_CONTEXT_ACQUIRE_GUARD(blk_get_aio_context(s->blk)) { +blk_drain(s->blk); +/* + * We drop queued requests after blk_drain() because + * blk_drain() itself can produce them. + */ +while (s->rq) { +req = s->rq; +s->rq = req->next; +virtqueue_detach_element(req->vq, >elem, 0); +virtio_blk_free_request(req); +} } -aio_context_release(ctx); - assert(!s->dataplane_started); blk_set_enable_write_cache(s->blk, s->original_wce); } -- 2.31.1
[RFC PATCH 0/4] aio: AIO_CONTEXT_ACQUIRE_GUARD() macro experiment
Experiment to use glib g_autoptr/autofree features with AIO context. Since this is a RFC, only few examples are provided. TODO: Document the macros in docs/devel/multiple-iothreads.txt Philippe Mathieu-Daudé (4): block/aio: Add automatically released aio_context variants hw/scsi/scsi-disk: Use automatic AIO context lock hw/scsi/scsi-generic: Use automatic AIO context lock hw/block/virtio-blk: Use automatic AIO context lock include/block/aio.h| 24 hw/block/virtio-blk.c | 26 -- hw/scsi/scsi-disk.c| 13 - hw/scsi/scsi-generic.c | 6 +++--- 4 files changed, 43 insertions(+), 26 deletions(-) -- 2.31.1
Re: [PATCH 1/2] bsd-user: Only process meson rules on BSD host
Philippe Mathieu-Daudé writes: > On Mon, Sep 27, 2021 at 11:15 AM Peter Maydell > wrote: >> On Sun, 26 Sept 2021 at 23:04, Philippe Mathieu-Daudé >> wrote: >> > Reported-by: Warner Losh >> > Signed-off-by: Philippe Mathieu-Daudé >> > --- >> > bsd-user/meson.build | 4 >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/bsd-user/meson.build b/bsd-user/meson.build >> > index 03695493408..a7607e1c884 100644 >> > --- a/bsd-user/meson.build >> > +++ b/bsd-user/meson.build >> > @@ -1,3 +1,7 @@ >> > +if not config_host.has_key('CONFIG_BSD') >> > + subdir_done() >> > +endif >> > + >> > bsd_user_ss.add(files( >> >'bsdload.c', >> >'elfload.c', >> >> >> So, what's the reason for this change? > > https://lore.kernel.org/qemu-devel/canczdfprc16ezjqcwjmyeapx6eym9nxsoqatbagr+czis4r...@mail.gmail.com/ > > linux-user/meson.build is evaluated on bsd, and bsd-user/meson.build on Linux. > >> The commit messages and >> the cover letter don't really explain it. Is this fixing a bug >> (if so what?), a precaution to avoid possible future bugs, >> fixing a performance issue with how long meson takes to run (if >> so, how much effect does this have), or something else? > > I'll wait for feedback from Paolo, then work on the explanation. Ping Paolo? -- Alex Bennée
Re: [PATCH 1/2] hw/arm/virt: Rename default_bus_bypass_iommu
On 8/11/21 10:58 AM, Jean-Philippe Brucker wrote: > Since commit d8fb7d0969d5 ("vl: switch -M parsing to keyval"), machine > parameter definitions cannot use underscores, because keyval_dashify() > transforms them to dashes and the parser doesn't find the parameter. > > This affects option default_bus_bypass_iommu which was introduced in the > same release: > > $ qemu-system-aarch64 -M virt,default_bus_bypass_iommu=on > qemu-system-aarch64: Property 'virt-6.1-machine.default-bus-bypass-iommu' not > found > > Rename the parameter to "default-bus-bypass-iommu". Passing > "default_bus_bypass_iommu" is still valid since the underscore are > transformed automatically. > > Fixes: 6d7a85483a06 ("hw/arm/virt: Add default_bus_bypass_iommu machine > option") > Signed-off-by: Jean-Philippe Brucker Reviewed-by: Eric Auger Tested-by: Eric Auger Thanks Eric > --- > hw/arm/virt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index b4598d3fe6..7075cdc15e 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2671,10 +2671,10 @@ static void virt_machine_class_init(ObjectClass *oc, > void *data) >"Set the IOMMU type. " >"Valid values are none and > smmuv3"); > > -object_class_property_add_bool(oc, "default_bus_bypass_iommu", > +object_class_property_add_bool(oc, "default-bus-bypass-iommu", > virt_get_default_bus_bypass_iommu, > virt_set_default_bus_bypass_iommu); > -object_class_property_set_description(oc, "default_bus_bypass_iommu", > +object_class_property_set_description(oc, "default-bus-bypass-iommu", >"Set on/off to enable/disable " >"bypass_iommu for default root > bus"); > >
Re: [PATCH 1/2] hw/arm/virt: Rename default_bus_bypass_iommu
Hi Paolo, Peter, On 10/2/21 7:30 AM, Markus Armbruster wrote: > Markus Armbruster writes: > >> Markus Armbruster writes: >> >>> Did this series fall through the cracks for 6.1? >> >> Missed 6.1. What now? > > If I understand this correctly, it's a regression in 6.1. Paolo, please > advise on what should be done. Can we get those 2 fixes merged? Maybe we should also add Cc: qemu-sta...@nongnu.org If so is there a chance the fix gets backported to 6.1? Thanks Eric > >>> Jean-Philippe Brucker writes: >>> Since commit d8fb7d0969d5 ("vl: switch -M parsing to keyval"), machine parameter definitions cannot use underscores, because keyval_dashify() transforms them to dashes and the parser doesn't find the parameter. This affects option default_bus_bypass_iommu which was introduced in the same release: $ qemu-system-aarch64 -M virt,default_bus_bypass_iommu=on qemu-system-aarch64: Property 'virt-6.1-machine.default-bus-bypass-iommu' not found Rename the parameter to "default-bus-bypass-iommu". Passing "default_bus_bypass_iommu" is still valid since the underscore are transformed automatically. Fixes: 6d7a85483a06 ("hw/arm/virt: Add default_bus_bypass_iommu machine option") Signed-off-by: Jean-Philippe Brucker --- hw/arm/virt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index b4598d3fe6..7075cdc15e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2671,10 +2671,10 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) "Set the IOMMU type. " "Valid values are none and smmuv3"); -object_class_property_add_bool(oc, "default_bus_bypass_iommu", +object_class_property_add_bool(oc, "default-bus-bypass-iommu", virt_get_default_bus_bypass_iommu, virt_set_default_bus_bypass_iommu); -object_class_property_set_description(oc, "default_bus_bypass_iommu", +object_class_property_set_description(oc, "default-bus-bypass-iommu", "Set on/off to enable/disable " "bypass_iommu for default root bus"); > >
Re: [PULL 00/57] pc,pci,virtio: features, fixes
On 10/5/21 9:00 AM, Michael S. Tsirkin wrote: The following changes since commit 9618c5badaa8eed25259cf095ff880efb939fbe7: Merge remote-tracking branch 'remotes/vivier/tags/trivial-branch-for-6.2-pull-request' into staging (2021-10-04 16:27:35 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to c7d2f59cf940b8c8c52c29d5fa25613fe662f7b6: hw/i386/amd_iommu: Add description/category to TYPE_AMD_IOMMU_PCI (2021-10-05 11:46:45 -0400) pc,pci,virtio: features, fixes VDPA multiqueue support. A huge acpi refactoring. Fixes, cleanups all over the place. Signed-off-by: Michael S. Tsirkin Ani Sinha (3): bios-tables-test: allow changes in DSDT ACPI tables for q35 hw/i386/acpi: fix conflicting IO address range for acpi pci hotplug in q35 bios-tables-test: Update ACPI DSDT table golden blobs for q35 Dr. David Alan Gilbert (1): virtio-balloon: Fix page-poison subsection name Igor Mammedov (35): acpi: add helper routines to initialize ACPI tables acpi: build_rsdt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_xsdt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_slit: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_fadt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_tpm2: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: acpi_build_hest: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_mcfg: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_hmat: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: nvdimm_build_nfit: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: nvdimm_build_ssdt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: vmgenid_build_acpi: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: x86: build_dsdt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_hpet: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_tpm_tcpa: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: arm/x86: build_srat: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: use build_append_int_noprefix() API to compose SRAT table acpi: build_dmar_q35: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_waet: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_amd_iommu: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: madt: arm/x86: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: x86: remove dead code acpi: x86: set enabled when composing _MAT entries acpi: x86: madt: use build_append_int_noprefix() API to compose MADT table acpi: arm/virt: madt: use build_append_int_noprefix() API to compose MADT table acpi: build_dsdt_microvm: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: arm: virt: build_dsdt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: arm: virt: build_iort: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: arm/virt: convert build_iort() to endian agnostic build_append_FOO() API acpi: arm/virt: build_spcr: fix invalid cast acpi: arm/virt: build_spcr: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: arm/virt: build_gtdt: use acpi_table_begin()/acpi_table_end() instead of build_header() acpi: build_facs: use build_append_int_noprefix() API to compose table acpi: remove no longer used build_header() acpi: AcpiGenericAddress no longer used to map/access fields of MMIO, drop packed attribute Jason Wang (10): vhost-vdpa: open device fd in net_init_vhost_vdpa() vhost-vdpa: classify one time request vhost-vdpa: prepare for the multiqueue support vhost-vdpa: let net_vhost_vdpa_init() returns NetClientState * net: introduce control client vhost-net: control virtqueue support virtio-net: use "queue_pairs" instead of "queues" when possible vhost: record the last virtqueue index for the virtio device virtio-net: vhost control virtqueue support vhost-vdpa: multiqueue support Li Zhijian (1): nvdimm: release the correct device list Philippe Mathieu-Daudé (5): hw/virtio: Acquire RCU read lock in virtqueue_packed_drop_all() hw/virtio: Have
[PATCH v7 3/8] qmp: add QMP command x-debug-query-virtio
From: Laurent Vivier This new command lists all the instances of VirtIODevice with their QOM paths and virtio type/name. Signed-off-by: Jonah Palmer --- hw/virtio/meson.build | 2 ++ hw/virtio/virtio-stub.c| 14 ++ hw/virtio/virtio.c | 27 +++ include/hw/virtio/virtio.h | 1 + qapi/meson.build | 1 + qapi/qapi-schema.json | 1 + qapi/virtio.json | 66 ++ tests/qtest/qmp-cmd-test.c | 1 + 8 files changed, 113 insertions(+) create mode 100644 hw/virtio/virtio-stub.c create mode 100644 qapi/virtio.json [Jonah: VirtioInfo member 'type' is now of type string and no longer relies on defining a QAPI list of virtio device type enumerations to match the VirtIODevice name with qapi_enum_parse().] diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build index bc352a6..d409735 100644 --- a/hw/virtio/meson.build +++ b/hw/virtio/meson.build @@ -6,8 +6,10 @@ softmmu_virtio_ss.add(when: 'CONFIG_VHOST', if_false: files('vhost-stub.c')) softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss) softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c')) +softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('virtio-stub.c')) softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c')) +softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('virtio-stub.c')) virtio_ss = ss.source_set() virtio_ss.add(files('virtio.c')) diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c new file mode 100644 index 000..d4a88f5 --- /dev/null +++ b/hw/virtio/virtio-stub.c @@ -0,0 +1,14 @@ +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "qapi/qapi-commands-virtio.h" + +static void *qmp_virtio_unsupported(Error **errp) +{ +error_setg(errp, "Virtio is disabled"); +return NULL; +} + +VirtioInfoList *qmp_x_debug_query_virtio(Error **errp) +{ +return qmp_virtio_unsupported(errp); +} diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 4af20c0..a454e2f 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -13,6 +13,8 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qapi/qapi-commands-virtio.h" +#include "qapi/qapi-visit-virtio.h" #include "cpu.h" #include "trace.h" #include "qemu/error-report.h" @@ -29,6 +31,9 @@ #include "sysemu/runstate.h" #include "standard-headers/linux/virtio_ids.h" +/* QAPI list of VirtIODevices */ +static QTAILQ_HEAD(, VirtIODevice) virtio_list; + /* * The alignment to use between consumer and producer parts of vring. * x86 pagesize again. This is the default, used by transports like PCI @@ -3709,6 +3714,7 @@ static void virtio_device_realize(DeviceState *dev, Error **errp) vdev->listener.commit = virtio_memory_listener_commit; memory_listener_register(>listener, vdev->dma_as); +QTAILQ_INSERT_TAIL(_list, vdev, next); } static void virtio_device_unrealize(DeviceState *dev) @@ -3723,6 +3729,7 @@ static void virtio_device_unrealize(DeviceState *dev) vdc->unrealize(dev); } +QTAILQ_REMOVE(_list, vdev, next); g_free(vdev->bus_name); vdev->bus_name = NULL; } @@ -3896,6 +3903,8 @@ static void virtio_device_class_init(ObjectClass *klass, void *data) vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl; vdc->legacy_features |= VIRTIO_LEGACY_FEATURES; + +QTAILQ_INIT(_list); } bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev) @@ -3906,6 +3915,24 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev) return virtio_bus_ioeventfd_enabled(vbus); } +VirtioInfoList *qmp_x_debug_query_virtio(Error **errp) +{ +VirtioInfoList *list = NULL; +VirtioInfoList *node; +VirtIODevice *vdev; + +QTAILQ_FOREACH(vdev, _list, next) { +DeviceState *dev = DEVICE(vdev); +node = g_new0(VirtioInfoList, 1); +node->value = g_new(VirtioInfo, 1); +node->value->path = g_strdup(dev->canonical_path); +node->value->type = g_strdup(vdev->name); +QAPI_LIST_PREPEND(list, node->value); +} + +return list; +} + static const TypeInfo virtio_device_info = { .name = TYPE_VIRTIO_DEVICE, .parent = TYPE_DEVICE, diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 105b98c..eceaafc 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -110,6 +110,7 @@ struct VirtIODevice bool use_guest_notifier_mask; AddressSpace *dma_as; QLIST_HEAD(, VirtQueue) *vector_queues; +QTAILQ_ENTRY(VirtIODevice) next; }; struct VirtioDeviceClass { diff --git a/qapi/meson.build b/qapi/meson.build index c356a38..df5662e 100644 --- a/qapi/meson.build +++ b/qapi/meson.build @@ -45,6 +45,7 @@ qapi_all_modules = [ 'sockets', 'trace', 'transaction', + 'virtio', 'yank', ] if have_system diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json index 4912b97..1512ada 100644 --- a/qapi/qapi-schema.json +++
[PATCH v7 5/8] qmp: decode feature & status bits in virtio-status
From: Laurent Vivier Display feature names instead of bitmaps for host, guest, and backend for VirtIODevice. Display status names instead of bitmaps for VirtIODevice. Display feature names instead of bitmaps for backend, protocol, acked, and features (hdev->features) for vhost devices. Decode features according to device type. Decode status according to configuration status bitmap (config_status_map). Decode vhost user protocol features according to vhost user protocol bitmap (vhost_user_protocol_map). Transport features are on the first line. Undecoded bits (if any) are stored in a separate field. Vhost device field wont show if there's no vhost active for a given VirtIODevice. Signed-off-by: Jonah Palmer --- hw/block/virtio-blk.c | 28 ++ hw/char/virtio-serial-bus.c| 11 + hw/display/virtio-gpu-base.c | 18 +- hw/input/virtio-input.c| 11 +- hw/net/virtio-net.c| 47 hw/scsi/virtio-scsi.c | 17 ++ hw/virtio/vhost-user-fs.c | 10 + hw/virtio/vhost-vsock-common.c | 10 + hw/virtio/virtio-balloon.c | 14 + hw/virtio/virtio-crypto.c | 10 + hw/virtio/virtio-iommu.c | 14 + hw/virtio/virtio.c | 273 +++- include/hw/virtio/vhost.h | 3 + include/hw/virtio/virtio.h | 17 ++ qapi/virtio.json | 574 ++--- 15 files changed, 1012 insertions(+), 45 deletions(-) [Jonah: Added vhost feature 'LOG_ALL' to all virtio feature maps for virtio devices who can use vhost. This includes virtio/vhost devices that previously did not have a feature map defined, such as virtio-input, vhost-user-fs, vhost-vsock, and virtio-crypto. Defined an enumeration and mapping of vhost user protocol features in virtio.c for decoding vhost user protocol features. Support to decode vhost user protocol features added. Added support to also decode VirtIODevice status bits via. a mapping of virtio config statuses. Needed to define a list of QAPI enumerations of virtio device types here because we can't discriminate the virtio device type in VirtioDeviceFeatures QAPI union based on a 'str' QAPI type.] diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 505e574..c2e901f 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qapi/qapi-visit-virtio.h" #include "qemu/iov.h" #include "qemu/module.h" #include "qemu/error-report.h" @@ -32,6 +33,7 @@ #include "hw/virtio/virtio-bus.h" #include "migration/qemu-file-types.h" #include "hw/virtio/virtio-access.h" +#include "standard-headers/linux/vhost_types.h" /* Config size before the discard support (hide associated config fields) */ #define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \ @@ -48,6 +50,32 @@ static const VirtIOFeature feature_sizes[] = { {} }; +qmp_virtio_feature_map_t blk_map[] = { +#define FEATURE_ENTRY(name) \ +{ VIRTIO_BLK_F_##name, VIRTIO_BLK_FEATURE_##name } +FEATURE_ENTRY(SIZE_MAX), +FEATURE_ENTRY(SEG_MAX), +FEATURE_ENTRY(GEOMETRY), +FEATURE_ENTRY(RO), +FEATURE_ENTRY(BLK_SIZE), +FEATURE_ENTRY(TOPOLOGY), +FEATURE_ENTRY(MQ), +FEATURE_ENTRY(DISCARD), +FEATURE_ENTRY(WRITE_ZEROES), +#ifndef VIRTIO_BLK_NO_LEGACY +FEATURE_ENTRY(BARRIER), +FEATURE_ENTRY(SCSI), +FEATURE_ENTRY(FLUSH), +FEATURE_ENTRY(CONFIG_WCE), +#endif /* !VIRTIO_BLK_NO_LEGACY */ +#undef FEATURE_ENTRY +#define FEATURE_ENTRY(name) \ +{ VHOST_F_##name, VIRTIO_BLK_FEATURE_##name } +FEATURE_ENTRY(LOG_ALL), +#undef FEATURE_ENTRY +{ -1, -1 } +}; + static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features) { s->config_size = MAX(VIRTIO_BLK_CFG_SIZE, diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 746c92b..f91418b 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -20,6 +20,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qapi/qapi-visit-virtio.h" #include "qemu/iov.h" #include "qemu/main-loop.h" #include "qemu/module.h" @@ -32,6 +33,16 @@ #include "hw/virtio/virtio-serial.h" #include "hw/virtio/virtio-access.h" +qmp_virtio_feature_map_t serial_map[] = { +#define FEATURE_ENTRY(name) \ +{ VIRTIO_CONSOLE_F_##name, VIRTIO_SERIAL_FEATURE_##name } +FEATURE_ENTRY(SIZE), +FEATURE_ENTRY(MULTIPORT), +FEATURE_ENTRY(EMERG_WRITE), +#undef FEATURE_ENTRY +{ -1, -1 } +}; + static struct VirtIOSerialDevices { QLIST_HEAD(, VirtIOSerial) devices; } vserdevices; diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index 5411a7b..a322349 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -12,13 +12,29 @@ */ #include "qemu/osdep.h" - +#include "standard-headers/linux/vhost_types.h" #include "hw/virtio/virtio-gpu.h" #include "migration/blocker.h" #include "qapi/error.h"
Re: [PATCH v3 6/6] tests/qapi-schema: Test cases for aliases
Am 05.10.2021 um 15:49 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 02.10.2021 um 15:33 hat Markus Armbruster geschrieben: > >> I apologize for this wall of text. It's a desparate attempt to cut > >> through the complexity and my confusion, and make sense of the actual > >> problems we're trying to solve. > >> > >> So, what problems exactly are we trying to solve? > > > > I'll start with replying to your final question because I think it's > > more helpful to start with the big picture than with details. > > > > So tools like libvirt want to have a single consistent interface to > > configure things on startup and at runtime. We also intend to support > > configuration files that should this time support all of the options and > > not just a few chosen ones. > > Yes. > > > The hypothesis is that QAPIfying the command line is the correct > > solution for both of these problems, i.e. all available command line > > options must be present in the QAPI schema and will be processed by a > > single parser shared with QMP to make sure they are consistent. > > Yes. > > This leads us to JSON option arguments and configuration files. > Well-suited for management applications that already use QMP. > > > Adding QAPIfied versions of individual command line options are steps > > towards this goal. As soon as they exist for every option, the final > > conversion from an open coded getopt() loop (or in fact a hand crafted > > parser in the case of vl.c) to something generated from the QAPI schema > > should be reasonably easy. > > Yes. > > > You're right that adding a second JSON-based command line interface for > > every option can already achieve the goal of providing a unified > > external interface, at the cost of (interface and code) duplication. Is > > this duplication desirable? Certainly no. Is it acceptable? You might > > get different opinions on this one. > > We'd certainly prefer CLI options to match corresponding QMP commands > exactly. > > Unfortunately, existing CLI options deviate from corresponding QMP > commands, and existing CLI options without corresponding QMP commands > may violate QMP design rules. > > Note: these issues pertain to human-friendly option syntax. The > machine-friendly option syntax is still limited to just a few options, > and it does match QMP there. On the other hand, we only have a handful of existing options that are very complex. Most of them aren't even structured and just take a single scalar value. So I'm relatively sure that we know the big ones, and we're working on them. Another point here is that I would argue that there is a difference between existing options and human-friendly options. Not all existing options are human-friendly just because they aren't machine friendly either, and there is probably potential for human-friendly syntax that doesn't exist yet. So I would treat support for existing options (i.e. compatibility) and support for human-friendly options (i.e. usability) as two separate problems. > > In my opinion, we should try to get rid of hand crafted parsers where > > it's reasonably possible, and take advantage of the single unified > > option structure that QAPI provides. -chardev specifically has a hand > > crafted parser that essentially duplicates the automatically generated > > QAPI visitor code, except for the nesting and some differences in option > > names. > > We should definitely parse JSON option arguments with the QAPI > machinery, and nothing more. Yes, no doubt. (And we don't even consistently do that yet, like device-add going through QemuOpts after going through QAPI and throwing away all type information and silently ignoring anything it doesn't know to handle.) > Parsing human-friendly arguments with it is desirable, but the need for > backward compatibility can make it difficult. Even where compatibility > is of no concern, simply swapping concrete JSON syntax for dotted keys > may result in human interfaces that are less than friendly. > > Are we in agreement that this is the problem at hand? As far as I am concerned, compatibility is the problem at hand, usability isn't. This doesn't mean that I'm opposed to having actually human friendly options, quite the opposite. But getting machine friendly options is already a big project. Making human interfaces friendlier would be adding another project of similar size, and I don't feel like tackling a second project at the same time when the first one is already hard. > > Aliases are one tool that can help bridge these differences in a generic > > way with minimal effort in the individual case. They are not _necessary_ > > to solve the problem; we could instead just use manually written code to > > manipulate input QDicts so that QAPI visitors accept them. Even with > > aliases, there are a few things left in the chardev code that are > > converted this way. Aliases just greatly reduce the amount of this code > > and make the conversion
Re: [PATCH 2/2] hw/i386: Rename default_bus_bypass_iommu
On 8/11/21 10:58 AM, Jean-Philippe Brucker wrote: > Since commit d8fb7d0969d5 ("vl: switch -M parsing to keyval"), machine > parameter definitions cannot use underscores, because keyval_dashify() > transforms them to dashes and the parser doesn't find the parameter. > > This affects option default_bus_bypass_iommu which was introduced in the > same release: > > $ qemu-system-x86_64 -M q35,default_bus_bypass_iommu=on > qemu-system-x86_64: Property 'pc-q35-6.1-machine.default-bus-bypass-iommu' > not found > > Rename the parameter to "default-bus-bypass-iommu". Passing > "default_bus_bypass_iommu" is still valid since the underscore are > transformed automatically. > > Fixes: c9e96b04fc19 ("hw/i386: Add a default_bus_bypass_iommu pc machine > option") > Signed-off-by: Jean-Philippe Brucker Reviewed-by: Eric Auger Tested-by: Eric Auger Eric > --- > hw/i386/pc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index fb24f000e7..ce4756ad59 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1779,7 +1779,7 @@ static void pc_machine_class_init(ObjectClass *oc, void > *data) > object_class_property_add_bool(oc, "hpet", > pc_machine_get_hpet, pc_machine_set_hpet); > > -object_class_property_add_bool(oc, "default_bus_bypass_iommu", > +object_class_property_add_bool(oc, "default-bus-bypass-iommu", > pc_machine_get_default_bus_bypass_iommu, > pc_machine_set_default_bus_bypass_iommu); > >
[PATCH v7 1/8] virtio: drop name parameter for virtio_init()
This patch drops the name parameter for the virtio_init function. The pair between the numeric device ID and the string device ID (name) of a virtio device already exists, but not in a way that let's us map between them. This patch will let us do this and removes the need for the name parameter in virtio_init(). Signed-off-by: Jonah Palmer --- hw/9pfs/virtio-9p-device.c | 2 +- hw/block/vhost-user-blk.c | 2 +- hw/block/virtio-blk.c | 2 +- hw/char/virtio-serial-bus.c | 4 +-- hw/display/virtio-gpu-base.c| 2 +- hw/input/virtio-input.c | 3 +- hw/net/virtio-net.c | 2 +- hw/scsi/virtio-scsi.c | 3 +- hw/virtio/vhost-user-fs.c | 3 +- hw/virtio/vhost-user-i2c.c | 6 +--- hw/virtio/vhost-user-vsock.c| 2 +- hw/virtio/vhost-vsock-common.c | 4 +-- hw/virtio/vhost-vsock.c | 2 +- hw/virtio/virtio-balloon.c | 3 +- hw/virtio/virtio-crypto.c | 2 +- hw/virtio/virtio-iommu.c| 3 +- hw/virtio/virtio-mem.c | 3 +- hw/virtio/virtio-pmem.c | 3 +- hw/virtio/virtio-rng.c | 2 +- hw/virtio/virtio.c | 43 +++-- include/hw/virtio/vhost-vsock-common.h | 2 +- include/hw/virtio/virtio-gpu.h | 3 +- include/hw/virtio/virtio.h | 3 +- include/standard-headers/linux/virtio_ids.h | 1 + 24 files changed, 65 insertions(+), 40 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 54ee93b..5f522e6 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -216,7 +216,7 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp) } v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag); -virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size); +virtio_init(vdev, VIRTIO_ID_9P, v->config_size); v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output); } diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index ba13cb8..f61f8c1 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -490,7 +490,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) return; } -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, +virtio_init(vdev, VIRTIO_ID_BLOCK, sizeof(struct virtio_blk_config)); s->virtqs = g_new(VirtQueue *, s->num_queues); diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index f139cd7..505e574 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1213,7 +1213,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) virtio_blk_set_config_size(s, s->host_features); -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, s->config_size); +virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size); s->blk = conf->conf.blk; s->rq = NULL; diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index dd6bc27..746c92b 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -1044,8 +1044,8 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp) VIRTIO_CONSOLE_F_EMERG_WRITE)) { config_size = offsetof(struct virtio_console_config, emerg_wr); } -virtio_init(vdev, "virtio-serial", VIRTIO_ID_CONSOLE, -config_size); + +virtio_init(vdev, VIRTIO_ID_CONSOLE, config_size); /* Spawn a new virtio-serial bus on which the ports will ride as devices */ qbus_create_inplace(>bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS, diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index c8da480..5411a7b 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -170,7 +170,7 @@ virtio_gpu_base_device_realize(DeviceState *qdev, } g->virtio_config.num_scanouts = cpu_to_le32(g->conf.max_outputs); -virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU, +virtio_init(VIRTIO_DEVICE(g), VIRTIO_ID_GPU, sizeof(struct virtio_gpu_config)); if (virtio_gpu_virgl_enabled(g->conf)) { diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c index 54bcb46..5b5398b 100644 --- a/hw/input/virtio-input.c +++ b/hw/input/virtio-input.c @@ -257,8 +257,7 @@ static void virtio_input_device_realize(DeviceState *dev, Error **errp) vinput->cfg_size += 8; assert(vinput->cfg_size <= sizeof(virtio_input_config)); -virtio_init(vdev, "virtio-input", VIRTIO_ID_INPUT, -vinput->cfg_size); +virtio_init(vdev, VIRTIO_ID_INPUT, vinput->cfg_size); vinput->evt = virtio_add_queue(vdev, 64,
[PULL 06/12] migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread
From: Emanuele Giuseppe Esposito init_dirty_bitmap_migration assumes the iothread lock (BQL) to be held, but instead it isn't. Instead of adding the lock to qemu_savevm_state_setup(), follow the same pattern as the other ->save_setup callbacks and lock+unlock inside dirty_bitmap_save_setup(). Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi Message-Id: <20211005080751.3797161-2-eespo...@redhat.com> Signed-off-by: Paolo Bonzini --- migration/block-dirty-bitmap.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 35f5ef688d..9aba7d9c22 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -1215,7 +1215,10 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) { DBMSaveState *s = &((DBMState *)opaque)->save; SaveBitmapState *dbms = NULL; + +qemu_mutex_lock_iothread(); if (init_dirty_bitmap_migration(s) < 0) { +qemu_mutex_unlock_iothread(); return -1; } @@ -1223,7 +1226,7 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) send_bitmap_start(f, s, dbms); } qemu_put_bitmap_flags(f, DIRTY_BITMAP_MIG_FLAG_EOS); - +qemu_mutex_unlock_iothread(); return 0; } -- 2.31.1
[PATCH v7 6/8] qmp: add QMP commands for virtio/vhost queue-status
From: Laurent Vivier These new commands show the internal status of a VirtIODevice's VirtQueue and a vhost device's vhost_virtqueue (if active). Signed-off-by: Jonah Palmer --- hw/virtio/virtio-stub.c | 14 +++ hw/virtio/virtio.c | 103 +++ qapi/virtio.json| 262 3 files changed, 379 insertions(+) [Jonah: Added vhost support for qmp_x_debug_virtio_queue_status such that if a VirtIODevice's vhost device is active, shadow_avail_idx is hidden and last_avail_idx is retrieved from the vhost op vhost_get_vring_base(). Also added a new QMP command qmp_x_debug_virtio_vhost_queue_status that shows the interal status of a VirtIODevice's vhost device if it's active.] diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c index ddb592f..387803d 100644 --- a/hw/virtio/virtio-stub.c +++ b/hw/virtio/virtio-stub.c @@ -17,3 +17,17 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp) { return qmp_virtio_unsupported(errp); } + +VirtVhostQueueStatus *qmp_x_debug_virtio_vhost_queue_status(const char *path, +uint16_t queue, +Error **errp) +{ +return qmp_virtio_unsupported(errp); +} + +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path, + uint16_t queue, + Error **errp) +{ +return qmp_virtio_unsupported(errp); +} diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index f0e2b40..8d74dbf 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -4284,6 +4284,109 @@ VirtioStatus *qmp_x_debug_virtio_status(const char *path, Error **errp) return status; } +VirtVhostQueueStatus *qmp_x_debug_virtio_vhost_queue_status(const char *path, +uint16_t queue, +Error **errp) +{ +VirtIODevice *vdev; +VirtVhostQueueStatus *status; + +vdev = virtio_device_find(path); +if (vdev == NULL) { +error_setg(errp, "Path %s is not a VirtIODevice", path); +return NULL; +} + +if (!vdev->vhost_started) { +error_setg(errp, "Error: vhost device has not started yet"); +return NULL; +} + +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); +struct vhost_dev *hdev = vdc->get_vhost(vdev); + +if (queue < hdev->vq_index || queue >= hdev->vq_index + hdev->nvqs) { +error_setg(errp, "Invalid vhost virtqueue number %d", queue); +return NULL; +} + +status = g_new0(VirtVhostQueueStatus, 1); +status->device_name = g_strdup(vdev->name); +status->kick = hdev->vqs[queue].kick; +status->call = hdev->vqs[queue].call; +status->desc = (uint64_t)(unsigned long)hdev->vqs[queue].desc; +status->avail = (uint64_t)(unsigned long)hdev->vqs[queue].avail; +status->used = (uint64_t)(unsigned long)hdev->vqs[queue].used; +status->num = hdev->vqs[queue].num; +status->desc_phys = hdev->vqs[queue].desc_phys; +status->desc_size = hdev->vqs[queue].desc_size; +status->avail_phys = hdev->vqs[queue].avail_phys; +status->avail_size = hdev->vqs[queue].avail_size; +status->used_phys = hdev->vqs[queue].used_phys; +status->used_size = hdev->vqs[queue].used_size; + +return status; +} + +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path, + uint16_t queue, + Error **errp) +{ +VirtIODevice *vdev; +VirtQueueStatus *status; + +vdev = virtio_device_find(path); +if (vdev == NULL) { +error_setg(errp, "Path %s is not a VirtIODevice", path); +return NULL; +} + +if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) { +error_setg(errp, "Invalid virtqueue number %d", queue); +return NULL; +} + +status = g_new0(VirtQueueStatus, 1); +status->device_name = g_strdup(vdev->name); +status->queue_index = vdev->vq[queue].queue_index; +status->inuse = vdev->vq[queue].inuse; +status->vring_num = vdev->vq[queue].vring.num; +status->vring_num_default = vdev->vq[queue].vring.num_default; +status->vring_align = vdev->vq[queue].vring.align; +status->vring_desc = vdev->vq[queue].vring.desc; +status->vring_avail = vdev->vq[queue].vring.avail; +status->vring_used = vdev->vq[queue].vring.used; +status->used_idx = vdev->vq[queue].used_idx; +status->signalled_used = vdev->vq[queue].signalled_used; +status->signalled_used_valid = vdev->vq[queue].signalled_used_valid; + +if (vdev->vhost_started) { +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); +struct vhost_dev *hdev = vdc->get_vhost(vdev); + +/* check if vq index
Re: [PATCH 1/2] block/backup: avoid integer overflow of `max-workers`
10/5/21 19:11, Stefano Garzarella wrote: QAPI generates `struct BackupPerf` where `max-workers` value is stored in an `int64_t` variable. But block_copy_async(), and the underlying code, uses an `int` parameter. At the end that variable is used to initialize `max_busy_tasks` in block/aio_task.c causing the following assertion failure if a value greater than INT_MAX(2147483647) is used: ../block/aio_task.c:63: aio_task_pool_wait_one: Assertion `pool->busy_tasks > 0' failed. Let's check that `max-workers` doesn't exceed INT_MAX and print an error in that case. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310 I glad to see that someone experiments with my experimental API :) Signed-off-by: Stefano Garzarella Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/backup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/backup.c b/block/backup.c index 687d2882bc..8b072db5d9 100644 --- a/block/backup.c +++ b/block/backup.c @@ -407,8 +407,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, return NULL; } -if (perf->max_workers < 1) { -error_setg(errp, "max-workers must be greater than zero"); +if (perf->max_workers < 1 || perf->max_workers > INT_MAX) { +error_setg(errp, "max-workers must be between 1 and %d", INT_MAX); return NULL; } -- Best regards, Vladimir
[PATCH v7 4/8] qmp: add QMP command x-debug-virtio-status
From: Laurent Vivier This new command shows the status of a VirtIODevice, including its corresponding vhost device status (if active). Next patch will improve output by decoding feature bits, including vhost device's feature bits (backend, protocol, acked, and features). Also will decode status bits of a VirtIODevice. Next patch will also suppress the vhost device field from displaying if no vhost device is active for a given VirtIODevice. Signed-off-by: Jonah Palmer --- hw/virtio/virtio-stub.c | 5 + hw/virtio/virtio.c | 96 +++ qapi/virtio.json| 245 3 files changed, 346 insertions(+) [Jonah: Added more fields of VirtIODevice to display including name, status, isr, queue_sel, vm_running, broken, disabled, used_started, started, start_on_kick, disable_legacy_check, bus_name, and use_guest_notifier_mask. Also added vhost support that displays the status of the VirtIODevice's corresponding vhost device if it's active. Vhost device fields include n_mem_sections, n_tmp_sections, nvqs, vq_index, features, acked_features, backend_features, protocol_features, max_queues, backend_cap, log_enabled and log_size.] diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c index d4a88f5..ddb592f 100644 --- a/hw/virtio/virtio-stub.c +++ b/hw/virtio/virtio-stub.c @@ -12,3 +12,8 @@ VirtioInfoList *qmp_x_debug_query_virtio(Error **errp) { return qmp_virtio_unsupported(errp); } + +VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp) +{ +return qmp_virtio_unsupported(errp); +} diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index a454e2f..04a44e8 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -3933,6 +3933,102 @@ VirtioInfoList *qmp_x_debug_query_virtio(Error **errp) return list; } +static VirtIODevice *virtio_device_find(const char *path) +{ +VirtIODevice *vdev; + +QTAILQ_FOREACH(vdev, _list, next) { +DeviceState *dev = DEVICE(vdev); + +if (strcmp(dev->canonical_path, path) != 0) { +continue; +} +return vdev; +} + +return NULL; +} + +VirtioStatus *qmp_x_debug_virtio_status(const char *path, Error **errp) +{ +VirtIODevice *vdev; +VirtioStatus *status; + +vdev = virtio_device_find(path); +if (vdev == NULL) { +error_setg(errp, "Path %s is not a VirtIO device", path); +return NULL; +} + +status = g_new0(VirtioStatus, 1); +status->vhost_dev = g_new0(VhostStatus, 1); +status->name = g_strdup(vdev->name); +status->device_id = vdev->device_id; +status->vhost_started = vdev->vhost_started; +status->guest_features = vdev->guest_features; +status->host_features = vdev->host_features; +status->backend_features = vdev->backend_features; + +switch (vdev->device_endian) { +case VIRTIO_DEVICE_ENDIAN_LITTLE: +status->device_endian = VIRTIO_STATUS_ENDIANNESS_LITTLE; +break; +case VIRTIO_DEVICE_ENDIAN_BIG: +status->device_endian = VIRTIO_STATUS_ENDIANNESS_BIG; +break; +default: +status->device_endian = VIRTIO_STATUS_ENDIANNESS_UNKNOWN; +break; +} + +status->num_vqs = virtio_get_num_queues(vdev); +status->status = vdev->status; +status->isr = vdev->isr; +status->queue_sel = vdev->queue_sel; +status->vm_running = vdev->vm_running; +status->broken = vdev->broken; +status->disabled = vdev->disabled; +status->use_started = vdev->use_started; +status->started = vdev->started; +status->start_on_kick = vdev->start_on_kick; +status->disable_legacy_check = vdev->disable_legacy_check; +status->bus_name = g_strdup(vdev->bus_name); +status->use_guest_notifier_mask = vdev->use_guest_notifier_mask; + +if (vdev->vhost_started) { +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); +struct vhost_dev *hdev = vdc->get_vhost(vdev); + +status->vhost_dev->n_mem_sections = hdev->n_mem_sections; +status->vhost_dev->n_tmp_sections = hdev->n_tmp_sections; +status->vhost_dev->nvqs = hdev->nvqs; +status->vhost_dev->vq_index = hdev->vq_index; +status->vhost_dev->features = hdev->features; +status->vhost_dev->acked_features = hdev->acked_features; +status->vhost_dev->backend_features = hdev->backend_features; +status->vhost_dev->protocol_features = hdev->protocol_features; +status->vhost_dev->max_queues = hdev->max_queues; +status->vhost_dev->backend_cap = hdev->backend_cap; +status->vhost_dev->log_enabled = hdev->log_enabled; +status->vhost_dev->log_size = hdev->log_size; +} else { +status->vhost_dev->n_mem_sections = 0; +status->vhost_dev->n_tmp_sections = 0; +status->vhost_dev->nvqs = 0; +status->vhost_dev->vq_index = 0; +status->vhost_dev->features = 0; +
[PATCH v7 8/8] hmp: add virtio commands
From: Laurent Vivier This patch implements the HMP versions of the virtio QMP commands. Signed-off-by: Jonah Palmer --- docs/system/monitor.rst | 2 + hmp-commands-virtio.hx | 250 ++ hmp-commands.hx | 10 ++ hw/virtio/virtio.c | 355 include/monitor/hmp.h | 5 + meson.build | 1 + monitor/misc.c | 17 +++ 7 files changed, 640 insertions(+) create mode 100644 hmp-commands-virtio.hx [Jonah: Added new HMP command for vhost queue status (virtio vhost-queue-status) as well as HMP helper functions to dump decoded vhost protocol features and virtio device configuration statuses.] diff --git a/docs/system/monitor.rst b/docs/system/monitor.rst index ff5c434..10418fc 100644 --- a/docs/system/monitor.rst +++ b/docs/system/monitor.rst @@ -21,6 +21,8 @@ The following commands are available: .. hxtool-doc:: hmp-commands.hx +.. hxtool-doc:: hmp-commands-virtio.hx + .. hxtool-doc:: hmp-commands-info.hx Integer expressions diff --git a/hmp-commands-virtio.hx b/hmp-commands-virtio.hx new file mode 100644 index 000..36aab94 --- /dev/null +++ b/hmp-commands-virtio.hx @@ -0,0 +1,250 @@ +HXCOMM Use DEFHEADING() to define headings in both help text and rST. +HXCOMM Text between SRST and ERST is copied to the rST version and +HXCOMM discarded from C version. +HXCOMM +HXCOMM DEF(command, args, callback, arg_string, help) is used to construct +HXCOMM monitor info commands. +HXCOMM +HXCOMM HXCOMM can be used for comments, discarded from both rST and C. +HXCOMM +HXCOMM In this file, generally SRST fragments should have two extra +HXCOMM spaces of indent, so that the documentation list item for "virtio cmd" +HXCOMM appears inside the documentation list item for the top level +HXCOMM "virtio" documentation entry. The exception is the first SRST +HXCOMM fragment that defines that top level entry. + +SRST + ``virtio`` *subcommand* + Show various information about virtio + + Example: + + List all sub-commands:: + + (qemu) virtio + virtio query -- List all available virtio devices + virtio status path -- Display status of a given virtio device + virtio queue-status path queue -- Display status of a given virtio queue + virtio vhost-queue-status path queue -- Display status of a given vhost queue + virtio queue-element path queue [index] -- Display element of a given virtio queue + +ERST + + { +.name = "query", +.args_type = "", +.params = "", +.help = "List all available virtio devices", +.cmd= hmp_virtio_query, +.flags = "p", + }, + +SRST + ``virtio query`` + List all available virtio devices + + Example: + + List all available virtio devices in the machine:: + + (qemu) virtio query + /machine/peripheral/vsock0/virtio-backend [vhost-vsock] + /machine/peripheral/crypto0/virtio-backend [virtio-crypto] + /machine/peripheral-anon/device[2]/virtio-backend [virtio-scsi] + /machine/peripheral-anon/device[1]/virtio-backend [virtio-net] + /machine/peripheral-anon/device[0]/virtio-backend [virtio-serial] + +ERST + + { +.name = "status", +.args_type = "path:s", +.params = "path", +.help = "Display status of a given virtio device", +.cmd= hmp_virtio_status, +.flags = "p", + }, + +SRST + ``virtio status`` *path* + Display status of a given virtio device + + Example: + + Dump the status of virtio-net (vhost on):: + + (qemu) virtio status /machine/peripheral-anon/device[1]/virtio-backend + /machine/peripheral-anon/device[1]/virtio-backend: +device_name: virtio-net (vhost) +device_id: 1 +vhost_started: true +bus_name:(null) +broken: false +disabled:false +disable_legacy_check:false +started: true +use_started: true +start_on_kick: false +use_guest_notifier_mask: true +vm_running: true +num_vqs: 3 +queue_sel: 2 +isr: 1 +endianness: little +status: acknowledge, driver, features-ok, driver-ok +Guest features: event-idx, indirect-desc, version-1 + ctrl-mac-addr, guest-announce, ctrl-vlan, ctrl-rx, ctrl-vq, status, mrg-rxbuf, + host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, guest-tso6, + guest-tso4, mac, ctrl-guest-offloads, guest-csum, csum +Host features:protocol-features, event-idx, indirect-desc, version-1, any-layout, notify-on-empty + gso, ctrl-mac-addr, guest-announce, ctrl-rx-extra, ctrl-vlan, ctrl-rx, ctrl-vq, + status, mrg-rxbuf, host-ufo, host-ecn, host-tso6, host-tso4, guest-ufo, guest-ecn, + guest-tso6, guest-tso4, mac,
Re: [PATCH v3] target/i386: Include 'hw/i386/apic.h' locally
Le 05/10/2021 à 16:57, Michael S. Tsirkin a écrit : > On Wed, Sep 29, 2021 at 06:31:24PM +0200, Philippe Mathieu-Daudé wrote: >> Instead of including a sysemu-specific header in "cpu.h" >> (which is shared with user-mode emulations), include it >> locally when required. >> >> Acked-by: Paolo Bonzini >> Signed-off-by: Philippe Mathieu-Daudé > > Acked-by: Michael S. Tsirkin > > which tree? trivial I guess? Yes, but for me the patch was not correct because there is no need to update target/i386/cpu-dump.c But perhaps I misunderstood the answer from Philippe? Thanks Laurent > >> --- >> target/i386/cpu.h| 4 >> hw/i386/kvmvapic.c | 1 + >> hw/i386/x86.c| 1 + >> target/i386/cpu-dump.c | 1 + >> target/i386/cpu-sysemu.c | 1 + >> target/i386/cpu.c| 1 + >> target/i386/gdbstub.c| 4 >> target/i386/hax/hax-all.c| 1 + >> target/i386/helper.c | 1 + >> target/i386/hvf/hvf.c| 1 + >> target/i386/hvf/x86_emu.c| 1 + >> target/i386/nvmm/nvmm-all.c | 1 + >> target/i386/tcg/sysemu/misc_helper.c | 1 + >> target/i386/tcg/sysemu/seg_helper.c | 1 + >> target/i386/whpx/whpx-all.c | 1 + >> 15 files changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h >> index c2954c71ea0..4411718bb7a 100644 >> --- a/target/i386/cpu.h >> +++ b/target/i386/cpu.h >> @@ -2045,10 +2045,6 @@ typedef X86CPU ArchCPU; >> #include "exec/cpu-all.h" >> #include "svm.h" >> >> -#if !defined(CONFIG_USER_ONLY) >> -#include "hw/i386/apic.h" >> -#endif >> - >> static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc, >> target_ulong *cs_base, uint32_t >> *flags) >> { >> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c >> index 43f8a8f679e..7333818bdd1 100644 >> --- a/hw/i386/kvmvapic.c >> +++ b/hw/i386/kvmvapic.c >> @@ -16,6 +16,7 @@ >> #include "sysemu/hw_accel.h" >> #include "sysemu/kvm.h" >> #include "sysemu/runstate.h" >> +#include "hw/i386/apic.h" >> #include "hw/i386/apic_internal.h" >> #include "hw/sysbus.h" >> #include "hw/boards.h" >> diff --git a/hw/i386/x86.c b/hw/i386/x86.c >> index 00448ed55aa..e0218f8791f 100644 >> --- a/hw/i386/x86.c >> +++ b/hw/i386/x86.c >> @@ -43,6 +43,7 @@ >> #include "target/i386/cpu.h" >> #include "hw/i386/topology.h" >> #include "hw/i386/fw_cfg.h" >> +#include "hw/i386/apic.h" >> #include "hw/intc/i8259.h" >> #include "hw/rtc/mc146818rtc.h" >> >> diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c >> index 02b635a52cf..0158fd2bf28 100644 >> --- a/target/i386/cpu-dump.c >> +++ b/target/i386/cpu-dump.c >> @@ -22,6 +22,7 @@ >> #include "qemu/qemu-print.h" >> #ifndef CONFIG_USER_ONLY >> #include "hw/i386/apic_internal.h" >> +#include "hw/i386/apic.h" >> #endif >> >> /***/ >> diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c >> index 37b7c562f53..4e8a6973d08 100644 >> --- a/target/i386/cpu-sysemu.c >> +++ b/target/i386/cpu-sysemu.c >> @@ -30,6 +30,7 @@ >> #include "hw/qdev-properties.h" >> >> #include "exec/address-spaces.h" >> +#include "hw/i386/apic.h" >> #include "hw/i386/apic_internal.h" >> >> #include "cpu-internal.h" >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 6b029f1bdf1..52422cbf21b 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -33,6 +33,7 @@ >> #include "standard-headers/asm-x86/kvm_para.h" >> #include "hw/qdev-properties.h" >> #include "hw/i386/topology.h" >> +#include "hw/i386/apic.h" >> #ifndef CONFIG_USER_ONLY >> #include "exec/address-spaces.h" >> #include "hw/boards.h" >> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c >> index 098a2ad15a9..5438229c1a9 100644 >> --- a/target/i386/gdbstub.c >> +++ b/target/i386/gdbstub.c >> @@ -21,6 +21,10 @@ >> #include "cpu.h" >> #include "exec/gdbstub.h" >> >> +#ifndef CONFIG_USER_ONLY >> +#include "hw/i386/apic.h" >> +#endif >> + >> #ifdef TARGET_X86_64 >> static const int gpr_map[16] = { >> R_EAX, R_EBX, R_ECX, R_EDX, R_ESI, R_EDI, R_EBP, R_ESP, >> diff --git a/target/i386/hax/hax-all.c b/target/i386/hax/hax-all.c >> index bf65ed6fa92..cd89e3233a9 100644 >> --- a/target/i386/hax/hax-all.c >> +++ b/target/i386/hax/hax-all.c >> @@ -32,6 +32,7 @@ >> #include "sysemu/reset.h" >> #include "sysemu/runstate.h" >> #include "hw/boards.h" >> +#include "hw/i386/apic.h" >> >> #include "hax-accel-ops.h" >> >> diff --git a/target/i386/helper.c b/target/i386/helper.c >> index 533b29cb91b..874beda98ae 100644 >> --- a/target/i386/helper.c >> +++ b/target/i386/helper.c >> @@ -26,6 +26,7 @@ >> #ifndef CONFIG_USER_ONLY >> #include "sysemu/hw_accel.h" >> #include "monitor/monitor.h" >> +#include "hw/i386/apic.h" >> #endif >> >> void cpu_sync_bndcs_hflags(CPUX86State *env)
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
Am 05.10.2021 um 17:52 hat Damien Hedde geschrieben: > > > On 10/5/21 16:37, Kevin Wolf wrote: > > Am 27.09.2021 um 13:39 hat Kevin Wolf geschrieben: > > > Am 27.09.2021 um 13:06 hat Damien Hedde geschrieben: > > > > On 9/24/21 11:04, Kevin Wolf wrote: > > > > > Directly call qdev_device_add_from_qdict() for QMP device_add instead > > > > > of > > > > > first going through QemuOpts and converting back to QDict. > > > > > > > > > > Note that this changes the behaviour of device_add, though in ways > > > > > that > > > > > should be considered bug fixes: > > > > > > > > > > QemuOpts ignores differences between data types, so you could > > > > > successfully pass a string "123" for an integer property, or a string > > > > > "on" for a boolean property (and vice versa). After this change, the > > > > > correct data type for the property must be used in the JSON input. > > > > > > > > > > qemu_opts_from_qdict() also silently ignores any options whose value > > > > > is > > > > > a QDict, QList or QNull. > > > > > > > > > > To illustrate, the following QMP command was accepted before and is > > > > > now > > > > > rejected for both reasons: > > > > > > > > > > { "execute": "device_add", > > > > > "arguments": { "driver": "scsi-cd", > > > > >"drive": { "completely": "invalid" }, > > > > >"physical_block_size": "4096" } } > > > > > > > > > > Signed-off-by: Kevin Wolf > > > > > --- > > > > >softmmu/qdev-monitor.c | 18 +++--- > > > > >1 file changed, 11 insertions(+), 7 deletions(-) > > > > > > > > > > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c > > > > > index c09b7430eb..8622ccade6 100644 > > > > > --- a/softmmu/qdev-monitor.c > > > > > +++ b/softmmu/qdev-monitor.c > > > > > @@ -812,7 +812,8 @@ void hmp_info_qdm(Monitor *mon, const QDict > > > > > *qdict) > > > > >qdev_print_devinfos(true); > > > > >} > > > > > -void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp) > > > > > +static void monitor_device_add(QDict *qdict, QObject **ret_data, > > > > > + bool from_json, Error **errp) > > > > >{ > > > > >QemuOpts *opts; > > > > >DeviceState *dev; > > > > > @@ -825,7 +826,9 @@ void qmp_device_add(QDict *qdict, QObject > > > > > **ret_data, Error **errp) > > > > >qemu_opts_del(opts); > > > > >return; > > > > >} > > > > > -dev = qdev_device_add(opts, errp); > > > > > +qemu_opts_del(opts); > > > > > + > > > > > +dev = qdev_device_add_from_qdict(qdict, from_json, errp); > > > > > > > > Hi Kevin, > > > > > > > > I'm wandering if deleting the opts (which remove it from the "device" > > > > opts > > > > list) is really a no-op ? > > > > > > It's not exactly a no-op. Previously, the QemuOpts would only be freed > > > when the device is destroying, now we delete it immediately after > > > creating the device. This could matter in some cases. > > > > > > The one case I was aware of is that QemuOpts used to be responsible for > > > checking for duplicate IDs. Obviously, it can't do this job any more > > > when we call qemu_opts_del() right after creating the device. This is > > > the reason for patch 6. > > > > > > > The opts list is, eg, traversed in hw/net/virtio-net.c in the function > > > > failover_find_primary_device_id() which may be called during the > > > > virtio_net_set_features() (a TYPE_VIRTIO_NET method). > > > > I do not have the knowledge to tell when this method is called. But If > > > > this > > > > is after we create the devices. Then the list will be empty at this > > > > point > > > > now. > > > > > > > > It seems, there are 2 other calling sites of > > > > "qemu_opts_foreach(qemu_find_opts("device"), [...]" in net/vhost-user.c > > > > and > > > > net/vhost-vdpa.c > > > > > > Yes, you are right. These callers probably need to be changed. Going > > > through the command line options rather than looking at the actual > > > device objects that exist doesn't feel entirely clean anyway. > > > > So I tried to have a look at the virtio-net case, and ended up very > > confused. > > > > Obviously looking at command line options (even of a differrent device) > > from within a device is very unclean. With a non-broken, i.e. type safe, > > device-add (as well as with the JSON CLI option introduced by this > > series), we can't have a QemuOpts any more that is by definition unsafe. > > So this code needs a replacement. > > > > My naive idea was that we just need to look at runtime state instead. > > Don't search the options for a device with a matching 'failover_pair_id' > > (which, by the way, would fail as soon as any other device introduces a > > property with the same name), but search for actual PCIDevices in qdev > > that have pci_dev->failover_pair_id set accordingly. > > > > However, the logic in failover_add_primary() suggests that we can have a > > state where QemuOpts for a device
Re: Deprecate the ppc405 boards in QEMU? (was: [PATCH v3 4/7] MAINTAINERS: Orphan obscure ppc platforms)
On 10/5/21 10:49, Daniel P. Berrangé wrote: > On Tue, Oct 05, 2021 at 06:44:23AM +0200, Christophe Leroy wrote: >> I will look at it, please allow me a few weeks though. > > Once something is deprecated, it remains in QEMU for a minimum of two > release cycles, before being deleted. At any time in that deprecation > period it can be returned to supported status, if someone provides a > good enough justification to keep it. My understanding is once being in deprecated state for 2 releases, it can be removed, but it doesn't have to be removed (assuming it is functional and nobody complains). Am I incorrect? I am raising this because the nanoMIPS support is in deprecated state since more than 2 releases, but it is still in-tree and I try to keep it functional. However, since no toolchain reached mainstream GCC/LLVM it is not easy to maintain. By keeping it in that state we give some time to other communities to have their toolchain upstreamed / merged. > IOW, we can deprecate this now, and you still have plenty of time to > investigate more. Yes, almost 8 months :)
[PATCH v7 7/8] qmp: add QMP command x-debug-virtio-queue-element
From: Laurent Vivier This new command shows the information of a VirtQueue element. Signed-off-by: Jonah Palmer --- hw/virtio/virtio-stub.c | 9 +++ hw/virtio/virtio.c | 154 ++ qapi/virtio.json| 191 3 files changed, 354 insertions(+) [Jonah: Added support to display driver (used vring) and device (avail vring) areas, including a new function vring_used_flags() to retrieve the used vring flags of a given element.] diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c index 387803d..6c282b3 100644 --- a/hw/virtio/virtio-stub.c +++ b/hw/virtio/virtio-stub.c @@ -31,3 +31,12 @@ VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path, { return qmp_virtio_unsupported(errp); } + +VirtioQueueElement *qmp_x_debug_virtio_queue_element(const char *path, + uint16_t queue, + bool has_index, + uint16_t index, + Error **errp) +{ +return qmp_virtio_unsupported(errp); +} diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 8d74dbf..0d67a36 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -478,6 +478,19 @@ static inline void vring_used_write(VirtQueue *vq, VRingUsedElem *uelem, address_space_cache_invalidate(>used, pa, sizeof(VRingUsedElem)); } +/* Called within rcu_read_lock(). */ +static inline uint16_t vring_used_flags(VirtQueue *vq) +{ +VRingMemoryRegionCaches *caches = vring_get_region_caches(vq); +hwaddr pa = offsetof(VRingUsed, flags); + +if (!caches) { +return 0; +} + +return virtio_lduw_phys_cached(vq->vdev, >used, pa); +} + /* Called within rcu_read_lock(). */ static uint16_t vring_used_idx(VirtQueue *vq) { @@ -4387,6 +4400,147 @@ VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path, return status; } +static VirtioRingDescFlagsList *qmp_decode_vring_desc_flags(uint16_t flags) +{ +VirtioRingDescFlagsList *list = NULL; +VirtioRingDescFlagsList *node; +int i; + +struct { +uint16_t flag; +VirtioRingDescFlags value; +} map[] = { +{ VRING_DESC_F_NEXT, VIRTIO_RING_DESC_FLAGS_NEXT }, +{ VRING_DESC_F_WRITE, VIRTIO_RING_DESC_FLAGS_WRITE }, +{ VRING_DESC_F_INDIRECT, VIRTIO_RING_DESC_FLAGS_INDIRECT }, +{ 1 << VRING_PACKED_DESC_F_AVAIL, VIRTIO_RING_DESC_FLAGS_AVAIL }, +{ 1 << VRING_PACKED_DESC_F_USED, VIRTIO_RING_DESC_FLAGS_USED }, +{ 0, -1 } +}; + +for (i = 0; map[i].flag; i++) { +if ((map[i].flag & flags) == 0) { +continue; +} +node = g_malloc0(sizeof(VirtioRingDescFlagsList)); +node->value = map[i].value; +node->next = list; +list = node; +} + +return list; +} + +VirtioQueueElement *qmp_x_debug_virtio_queue_element(const char *path, + uint16_t queue, + bool has_index, + uint16_t index, + Error **errp) +{ +VirtIODevice *vdev; +VirtQueue *vq; +VirtioQueueElement *element = NULL; + +vdev = virtio_device_find(path); +if (vdev == NULL) { +error_setg(errp, "Path %s is not a VirtIO device", path); +return NULL; +} + +if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) { +error_setg(errp, "Invalid virtqueue number %d", queue); +return NULL; +} +vq = >vq[queue]; + +if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) { +error_setg(errp, "Packed ring not supported"); +return NULL; +} else { +unsigned int head, i, max; +VRingMemoryRegionCaches *caches; +MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID; +MemoryRegionCache *desc_cache; +VRingDesc desc; +VirtioRingDescList *list = NULL; +VirtioRingDescList *node; +int rc; + +RCU_READ_LOCK_GUARD(); + +max = vq->vring.num; + +if (!has_index) { +head = vring_avail_ring(vq, vq->last_avail_idx % vq->vring.num); +} else { +head = vring_avail_ring(vq, index % vq->vring.num); +} +i = head; + +caches = vring_get_region_caches(vq); +if (!caches) { +error_setg(errp, "Region caches not initialized"); +return NULL; +} +if (caches->desc.len < max * sizeof(VRingDesc)) { +error_setg(errp, "Cannot map descriptor ring"); +return NULL; +} + +desc_cache = >desc; +vring_split_desc_read(vdev, , desc_cache, i); +if (desc.flags & VRING_DESC_F_INDIRECT)
[PULL 09/12] meson: switch minimum meson version to 0.58.2, minimum recommended to 0.59.2
Meson 0.58.2 does not need b_staticpic=$pie anymore, and has stabilized the keyval module. Remove the workaround and use a few replacements for features deprecated in the 0.57.0 release cycle. One feature that we would like to use is passing dependencies to summary. However, that was broken in 0.59.0 and 0.59.1. Therefore, use the embedded Meson if the host has anything older than 0.59.2, but allow --meson= to use 0.58.2. Reviewed-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- configure | 8 ++ docs/meson.build | 14 - meson.build | 54 --- plugins/meson.build | 4 +-- scripts/mtest2make.py | 7 ++--- tests/qapi-schema/meson.build | 4 +-- tests/qtest/meson.build | 2 +- tests/unit/meson.build| 2 +- trace/meson.build | 4 +-- 9 files changed, 44 insertions(+), 55 deletions(-) diff --git a/configure b/configure index 1d3f099498..877bf3d76a 100755 --- a/configure +++ b/configure @@ -1994,7 +1994,7 @@ python_version=$($python -c 'import sys; print("%d.%d.%d" % (sys.version_info[0] python="$python -B" if test -z "$meson"; then -if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.55.3; then +if test "$explicit_python" = no && has meson && version_ge "$(meson --version)" 0.59.2; then meson=meson elif test $git_submodules_action != 'ignore' ; then meson=git @@ -5163,10 +5163,6 @@ if test "$skip_meson" = no; then mv $cross config-meson.cross rm -rf meson-private meson-info meson-logs - unset staticpic - if ! version_ge "$($meson --version)" 0.56.0; then -staticpic=$(if test "$pie" = yes; then echo true; else echo false; fi) - fi NINJA=$ninja $meson setup \ --prefix "$prefix" \ --libdir "$libdir" \ @@ -5186,7 +5182,6 @@ if test "$skip_meson" = no; then -Dwerror=$(if test "$werror" = yes; then echo true; else echo false; fi) \ -Dstrip=$(if test "$strip_opt" = yes; then echo true; else echo false; fi) \ -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) \ -${staticpic:+-Db_staticpic=$staticpic} \ -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo false; fi) \ -Db_lto=$lto -Dcfi=$cfi -Dcfi_debug=$cfi_debug \ -Dmalloc=$malloc -Dmalloc_trim=$malloc_trim -Dsparse=$sparse \ @@ -5222,6 +5217,7 @@ else perl -i -ne ' s/^gettext = true$/gettext = auto/; s/^gettext = false$/gettext = disabled/; + /^b_staticpic/ && next; print;' meson-private/cmd_line.txt fi fi diff --git a/docs/meson.build b/docs/meson.build index cffe1ecf1d..be4dc30f39 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -37,14 +37,14 @@ endif if build_docs SPHINX_ARGS += ['-Dversion=' + meson.project_version(), '-Drelease=' + config_host['PKGVERSION']] - sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py', - meson.source_root() / 'docs/sphinx/hxtool.py', - meson.source_root() / 'docs/sphinx/kerneldoc.py', - meson.source_root() / 'docs/sphinx/kernellog.py', - meson.source_root() / 'docs/sphinx/qapidoc.py', - meson.source_root() / 'docs/sphinx/qmp_lexer.py', + sphinx_extn_depends = [ meson.current_source_dir() / 'sphinx/depfile.py', + meson.current_source_dir() / 'sphinx/hxtool.py', + meson.current_source_dir() / 'sphinx/kerneldoc.py', + meson.current_source_dir() / 'sphinx/kernellog.py', + meson.current_source_dir() / 'sphinx/qapidoc.py', + meson.current_source_dir() / 'sphinx/qmp_lexer.py', qapi_gen_depends ] - sphinx_template_files = [ meson.source_root() / 'docs/_templates/footer.html' ] + sphinx_template_files = [ meson.project_source_root() / 'docs/_templates/footer.html' ] have_ga = have_tools and config_host.has_key('CONFIG_GUEST_AGENT') diff --git a/meson.build b/meson.build index 60f4f45165..17e77fe4ef 100644 --- a/meson.build +++ b/meson.build @@ -1,14 +1,10 @@ -project('qemu', ['c'], meson_version: '>=0.55.0', -default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 'b_colorout=auto'] + - (meson.version().version_compare('>=0.56.0') ? [ 'b_staticpic=false' ] : []), -version: run_command('head', meson.source_root() / 'VERSION').stdout().strip()) +project('qemu', ['c'], meson_version: '>=0.58.2', +default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 'b_colorout=auto', + 'b_staticpic=false'], +version: files('VERSION')) not_found = dependency('', required: false) -if meson.version().version_compare('>=0.56.0') - keyval =
[PATCH 1/2] block/backup: avoid integer overflow of `max-workers`
QAPI generates `struct BackupPerf` where `max-workers` value is stored in an `int64_t` variable. But block_copy_async(), and the underlying code, uses an `int` parameter. At the end that variable is used to initialize `max_busy_tasks` in block/aio_task.c causing the following assertion failure if a value greater than INT_MAX(2147483647) is used: ../block/aio_task.c:63: aio_task_pool_wait_one: Assertion `pool->busy_tasks > 0' failed. Let's check that `max-workers` doesn't exceed INT_MAX and print an error in that case. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310 Signed-off-by: Stefano Garzarella --- block/backup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/backup.c b/block/backup.c index 687d2882bc..8b072db5d9 100644 --- a/block/backup.c +++ b/block/backup.c @@ -407,8 +407,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, return NULL; } -if (perf->max_workers < 1) { -error_setg(errp, "max-workers must be greater than zero"); +if (perf->max_workers < 1 || perf->max_workers > INT_MAX) { +error_setg(errp, "max-workers must be between 1 and %d", INT_MAX); return NULL; } -- 2.31.1
Re: [Virtio-fs] [PATCH 13/13] virtiofsd, seccomp: Add clock_nanosleep() to allow list
On Tue, Oct 05, 2021 at 04:50:43PM +0100, Stefan Hajnoczi wrote: > On Tue, Oct 05, 2021 at 11:16:18AM -0400, Vivek Goyal wrote: > > On Tue, Oct 05, 2021 at 01:22:58PM +0100, Stefan Hajnoczi wrote: > > > On Thu, Sep 30, 2021 at 11:30:37AM -0400, Vivek Goyal wrote: > > > > g_usleep() calls nanosleep() and that now seems to call > > > > clock_nanosleep() > > > > syscall. Now these patches are making use of g_usleep(). So add > > > > clock_nanosleep() to list of allowed syscalls. > > > > > > > > Signed-off-by: Vivek Goyal > > > > --- > > > > tools/virtiofsd/passthrough_seccomp.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/tools/virtiofsd/passthrough_seccomp.c > > > > b/tools/virtiofsd/passthrough_seccomp.c > > > > index cd24b40b78..03080806c0 100644 > > > > --- a/tools/virtiofsd/passthrough_seccomp.c > > > > +++ b/tools/virtiofsd/passthrough_seccomp.c > > > > @@ -117,6 +117,7 @@ static const int syscall_allowlist[] = { > > > > SCMP_SYS(writev), > > > > SCMP_SYS(umask), > > > > SCMP_SYS(nanosleep), > > > > +SCMP_SYS(clock_nanosleep), > > > > > > This patch can be dropped once sleep has been replaced by a condvar. > > > > There is another sleep in do_pool_destroy() where we are waiting > > for all current threads to exit. > > > > do_pool_destroy() { > > g_usleep(1); > > } > > That won't be necessary if there's a way to avoid the thread pool :). > See my other reply about closing the OFD instead of using signals to > cancel blocking fcntl(2). Hi Stefan, I responded to that email already. man fnctl does not say anything about closing fd will unblock the waiter with -EINTR and I had a quick look at kernel code and did not find anything which suggested closing fd will unblock current waiters. So is this something you know works or you want me to try and see if it works? Thanks Vivek
[PATCH v7 0/8] hmp,qmp: Add commands to introspect virtio devices
This series introduces new QMP/HMP commands to dump the status of a virtio device at different levels. [Jonah: Rebasing previous patchset from July (v6). Original patches are from Laurent Vivier from May 2020. Rebase from v6 to v7 includes adding ability to map between the numeric device ID and the string device ID (virtio device name), a get_vhost() callback function for VirtIODevices, display more fields of a VirtIODevice (including it's corresponding vhost device), support to decode vhost user protocol features, support to decode virtio configuration statuses, vhost support for displaying virtio queue statuses including a new command to introspect a vhost device's queue status, and lastly support to display driver and device areas when introspecting a VirtIODevice's virtqueue element.] 1. Main command HMP Only: virtio [subcommand] Example: List all sub-commands: (qemu) virtio virtio query -- List all available virtio devices virtio status path -- Display status of a given virtio device virtio queue-status path queue -- Display status of a given virtio queue virtio vhost-queue-status path queue -- Display status of a given vhost queue virtio queue-element path queue [index] -- Display element of a given virtio queue 2. List available virtio devices in the machine HMP Form: virtio query Example: (qemu) virtio query /machine/peripheral/vsock0/virtio-backend [vhost-vsock] /machine/peripheral/crypto0/virtio-backend [virtio-crypto] /machine/peripheral-anon/device[2]/virtio-backend [virtio-scsi] /machine/peripheral-anon/device[1]/virtio-backend [virtio-net] /machine/peripheral-anon/device[0]/virtio-backend [virtio-serial] QMP Form: { 'command': 'x-debug-query-virtio', 'returns': ['VirtioInfo'] } Example: -> { "execute": "x-debug-query-virtio" } <- { "return": [ { "path": "/machine/peripheral/vsock0/virtio-backend", "type": "vhost-vsock" }, { "path": "/machine/peripheral/crypto0/virtio-backend", "type": "virtio-crypto" }, { "path": "/machine/peripheral-anon/device[2]/virtio-backend", "type": "virtio-scsi" }, { "path": "/machine/peripheral-anon/device[1]/virtio-backend", "type": "virtio-net" }, { "path": "/machine/peripheral-anon/device[0]/virtio-backend", "type": "virtio-serial" } ] } 3. Display status of a given virtio device HMP Form: virtio status Example: (qemu) virtio status /machine/peripheral/vsock0/virtio-backend /machine/peripheral/vsock0/virtio-backend: device_name: vhost-vsock (vhost) device_id: 19 vhost_started: true bus_name:(null) broken: false disabled:false disable_legacy_check:false started: true use_started: true start_on_kick: false use_guest_notifier_mask: true vm_running: true num_vqs: 3 queue_sel: 2 isr: 0 endianness: little status: acknowledge, driver, features-ok, driver-ok Guest features: event-idx, indirect-desc, version-1 Host features:protocol-features, event-idx, indirect-desc, version-1, any-layout, notify-on-empty Backend features: VHost: nvqs: 2 vq_index: 0 max_queues: 0 n_mem_sections: 4 n_tmp_sections: 4 backend_cap:0 log_enabled:false log_size: 0 Features: event-idx, indirect-desc, version-1, any-layout, notify-on-empty, log-all Acked features:event-idx, indirect-desc, version-1 Backend features: Protocol features: QMP Form: { 'command': 'x-debug-virtio-status', 'data': { 'path': 'str' }, 'returns': 'VirtioStatus' } Example: -> { "execute": "x-debug-virtio-status", "arguments": { "path": "/machine/peripheral/vsock0/virtio-backend" } } <- { "return": { "device-endian": "little", "bus-name": "",
[PULL 02/12] sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
From: Dov Murik Add the sev_add_kernel_loader_hashes function to calculate the hashes of the kernel/initrd/cmdline and fill a designated OVMF encrypted hash table area. For this to work, OVMF must support an encrypted area to place the data which is advertised via a special GUID in the OVMF reset table. The hashes of each of the files is calculated (or the string in the case of the cmdline with trailing '\0' included). Each entry in the hashes table is GUID identified and since they're passed through the sev_encrypt_flash interface, the hashes will be accumulated by the AMD PSP measurement (SEV_LAUNCH_MEASURE). Co-developed-by: James Bottomley Signed-off-by: James Bottomley Signed-off-by: Dov Murik Reviewed-by: Daniel P. Berrangé Message-Id: <20210930054915.13252-2-dovmu...@linux.ibm.com> Signed-off-by: Paolo Bonzini --- target/i386/sev-stub.c | 5 ++ target/i386/sev.c | 137 + target/i386/sev_i386.h | 12 3 files changed, 154 insertions(+) diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c index 0227cb5177..d8e6583171 100644 --- a/target/i386/sev-stub.c +++ b/target/i386/sev-stub.c @@ -81,3 +81,8 @@ sev_get_attestation_report(const char *mnonce, Error **errp) error_setg(errp, "SEV is not available in this QEMU"); return NULL; } + +bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) +{ +g_assert_not_reached(); +} diff --git a/target/i386/sev.c b/target/i386/sev.c index fa7210473a..bcd9260fa4 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -23,6 +23,7 @@ #include "qemu/base64.h" #include "qemu/module.h" #include "qemu/uuid.h" +#include "crypto/hash.h" #include "sysemu/kvm.h" #include "sev_i386.h" #include "sysemu/sysemu.h" @@ -83,6 +84,32 @@ typedef struct __attribute__((__packed__)) SevInfoBlock { uint32_t reset_addr; } SevInfoBlock; +#define SEV_HASH_TABLE_RV_GUID "7255371f-3a3b-4b04-927b-1da6efa8d454" +typedef struct QEMU_PACKED SevHashTableDescriptor { +/* SEV hash table area guest address */ +uint32_t base; +/* SEV hash table area size (in bytes) */ +uint32_t size; +} SevHashTableDescriptor; + +/* hard code sha256 digest size */ +#define HASH_SIZE 32 + +typedef struct QEMU_PACKED SevHashTableEntry { +QemuUUID guid; +uint16_t len; +uint8_t hash[HASH_SIZE]; +} SevHashTableEntry; + +typedef struct QEMU_PACKED SevHashTable { +QemuUUID guid; +uint16_t len; +SevHashTableEntry cmdline; +SevHashTableEntry initrd; +SevHashTableEntry kernel; +uint8_t padding[]; +} SevHashTable; + static SevGuestState *sev_guest; static Error *sev_mig_blocker; @@ -1071,6 +1098,116 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size) return 0; } +static const QemuUUID sev_hash_table_header_guid = { +.data = UUID_LE(0x9438d606, 0x4f22, 0x4cc9, 0xb4, 0x79, 0xa7, 0x93, +0xd4, 0x11, 0xfd, 0x21) +}; + +static const QemuUUID sev_kernel_entry_guid = { +.data = UUID_LE(0x4de79437, 0xabd2, 0x427f, 0xb8, 0x35, 0xd5, 0xb1, +0x72, 0xd2, 0x04, 0x5b) +}; +static const QemuUUID sev_initrd_entry_guid = { +.data = UUID_LE(0x44baf731, 0x3a2f, 0x4bd7, 0x9a, 0xf1, 0x41, 0xe2, +0x91, 0x69, 0x78, 0x1d) +}; +static const QemuUUID sev_cmdline_entry_guid = { +.data = UUID_LE(0x97d02dd8, 0xbd20, 0x4c94, 0xaa, 0x78, 0xe7, 0x71, +0x4d, 0x36, 0xab, 0x2a) +}; + +/* + * Add the hashes of the linux kernel/initrd/cmdline to an encrypted guest page + * which is included in SEV's initial memory measurement. + */ +bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) +{ +uint8_t *data; +SevHashTableDescriptor *area; +SevHashTable *ht; +uint8_t cmdline_hash[HASH_SIZE]; +uint8_t initrd_hash[HASH_SIZE]; +uint8_t kernel_hash[HASH_SIZE]; +uint8_t *hashp; +size_t hash_len = HASH_SIZE; +int aligned_len; + +if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, , NULL)) { +error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid"); +return false; +} +area = (SevHashTableDescriptor *)data; + +/* + * Calculate hash of kernel command-line with the terminating null byte. If + * the user doesn't supply a command-line via -append, the 1-byte "\0" will + * be used. + */ +hashp = cmdline_hash; +if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->cmdline_data, + ctx->cmdline_size, , _len, errp) < 0) { +return false; +} +assert(hash_len == HASH_SIZE); + +/* + * Calculate hash of initrd. If the user doesn't supply an initrd via + * -initrd, an empty buffer will be used (ctx->initrd_size == 0). + */ +hashp = initrd_hash; +if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->initrd_data, + ctx->initrd_size, , _len, errp) < 0) { +return false; +
[PATCH 0/2] block: avoid integer overflow of `max-workers` and assert `max_busy_tasks`
This series contains a patch that avoids an integer overflow of `max-workers` (struct BackupPerf) by adding a check and a patch that asserts this condition where the problem occurs. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310 Signed-off-by: Stefano Garzarella Stefano Garzarella (2): block/backup: avoid integer overflow of `max-workers` block/aio_task: assert `max_busy_tasks` is greater than 0 block/aio_task.c | 2 ++ block/backup.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.31.1
[PULL 01/12] i386: docs: Briefly describe KVM PV features
From: Vitaly Kuznetsov KVM PV features don't seem to be documented anywhere, in particular, the fact that some of the features are enabled by default and some are not can only be figured out from the code. Signed-off-by: Vitaly Kuznetsov Message-Id: <20211004140445.624875-1-vkuzn...@redhat.com> Signed-off-by: Paolo Bonzini --- docs/system/i386/kvm-pv.rst | 100 docs/system/target-i386.rst | 1 + 2 files changed, 101 insertions(+) create mode 100644 docs/system/i386/kvm-pv.rst diff --git a/docs/system/i386/kvm-pv.rst b/docs/system/i386/kvm-pv.rst new file mode 100644 index 00..1e5a9923ef --- /dev/null +++ b/docs/system/i386/kvm-pv.rst @@ -0,0 +1,100 @@ +Paravirtualized KVM features + + +Description +--- + +In some cases when implementing hardware interfaces in software is slow, ``KVM`` +implements its own paravirtualized interfaces. + +Setup +- + +Paravirtualized ``KVM`` features are represented as CPU flags. The following +features are enabled by default for any CPU model when ``KVM`` acceleration is +enabled: + +- ``kvmclock`` +- ``kvm-nopiodelay`` +- ``kvm-asyncpf`` +- ``kvm-steal-time`` +- ``kvm-pv-eoi`` +- ``kvmclock-stable-bit`` + +``kvm-msi-ext-dest-id`` feature is enabled by default in x2apic mode with split +irqchip (e.g. "-machine ...,kernel-irqchip=split -cpu ...,x2apic"). + +Note: when CPU model ``host`` is used, QEMU passes through all supported +paravirtualized ``KVM`` features to the guest. + +Existing features +- + +``kvmclock`` + Expose a ``KVM`` specific paravirtualized clocksource to the guest. Supported + since Linux v2.6.26. + +``kvm-nopiodelay`` + The guest doesn't need to perform delays on PIO operations. Supported since + Linux v2.6.26. + +``kvm-mmu`` + This feature is deprecated. + +``kvm-asyncpf`` + Enable asynchronous page fault mechanism. Supported since Linux v2.6.38. + Note: since Linux v5.10 the feature is deprecated and not enabled by ``KVM``. + Use ``kvm-asyncpf-int`` instead. + +``kvm-steal-time`` + Enable stolen (when guest vCPU is not running) time accounting. Supported + since Linux v3.1. + +``kvm-pv-eoi`` + Enable paravirtualized end-of-interrupt signaling. Supported since Linux + v3.10. + +``kvm-pv-unhalt`` + Enable paravirtualized spinlocks support. Supported since Linux v3.12. + +``kvm-pv-tlb-flush`` + Enable paravirtualized TLB flush mechanism. Supported since Linux v4.16. + +``kvm-pv-ipi`` + Enable paravirtualized IPI mechanism. Supported since Linux v4.19. + +``kvm-poll-control`` + Enable host-side polling on HLT control from the guest. Supported since Linux + v5.10. + +``kvm-pv-sched-yield`` + Enable paravirtualized sched yield feature. Supported since Linux v5.10. + +``kvm-asyncpf-int`` + Enable interrupt based asynchronous page fault mechanism. Supported since Linux + v5.10. + +``kvm-msi-ext-dest-id`` + Support 'Extended Destination ID' for external interrupts. The feature allows + to use up to 32768 CPUs without IRQ remapping (but other limits may apply making + the number of supported vCPUs for a given configuration lower). Supported since + Linux v5.10. + +``kvmclock-stable-bit`` + Tell the guest that guest visible TSC value can be fully trusted for kvmclock + computations and no warps are expected. Supported since Linux v2.6.35. + +Supplementary features +-- + +``kvm-pv-enforce-cpuid`` + Limit the supported paravirtualized feature set to the exposed features only. + Note, by default, ``KVM`` allows the guest to use all currently supported + paravirtualized features even when they were not announced in guest visible + CPUIDs. Supported since Linux v5.10. + + +Useful links + + +Please refer to Documentation/virt/kvm in Linux for additional details. diff --git a/docs/system/target-i386.rst b/docs/system/target-i386.rst index 6a86d63863..4daa53c35d 100644 --- a/docs/system/target-i386.rst +++ b/docs/system/target-i386.rst @@ -26,6 +26,7 @@ Architectural features :maxdepth: 1 i386/cpu + i386/kvm-pv i386/sgx .. _pcsys_005freq: -- 2.31.1
[PATCH v7 2/8] virtio: add vhost support for virtio devices
This patch adds a get_vhost() callback function for VirtIODevices that returns the device's corresponding vhost_dev structure if the vhost device is running. This patch also adds a vhost_started flag for VirtIODevices. Previously, a VirtIODevice wouldn't be able to tell if its corresponding vhost device was active or not. Signed-off-by: Jonah Palmer --- hw/block/vhost-user-blk.c | 7 +++ hw/display/vhost-user-gpu.c| 7 +++ hw/input/vhost-user-input.c| 7 +++ hw/net/virtio-net.c| 9 + hw/scsi/vhost-scsi.c | 8 hw/virtio/vhost-user-fs.c | 7 +++ hw/virtio/vhost-vsock-common.c | 7 +++ hw/virtio/vhost.c | 3 +++ hw/virtio/virtio-crypto.c | 10 ++ hw/virtio/virtio.c | 1 + include/hw/virtio/virtio.h | 3 +++ 11 files changed, 69 insertions(+) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index f61f8c1..b059da1 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -568,6 +568,12 @@ static void vhost_user_blk_instance_init(Object *obj) "/disk@0,0", DEVICE(obj)); } +static struct vhost_dev *vhost_user_blk_get_vhost(VirtIODevice *vdev) +{ +VHostUserBlk *s = VHOST_USER_BLK(vdev); +return >dev; +} + static const VMStateDescription vmstate_vhost_user_blk = { .name = "vhost-user-blk", .minimum_version_id = 1, @@ -602,6 +608,7 @@ static void vhost_user_blk_class_init(ObjectClass *klass, void *data) vdc->get_features = vhost_user_blk_get_features; vdc->set_status = vhost_user_blk_set_status; vdc->reset = vhost_user_blk_reset; +vdc->get_vhost = vhost_user_blk_get_vhost; } static const TypeInfo vhost_user_blk_info = { diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c index 49df56c..6e93b46 100644 --- a/hw/display/vhost-user-gpu.c +++ b/hw/display/vhost-user-gpu.c @@ -565,6 +565,12 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp) g->vhost_gpu_fd = -1; } +static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev) +{ +VhostUserGPU *g = VHOST_USER_GPU(vdev); +return >vhost->dev; +} + static Property vhost_user_gpu_properties[] = { VIRTIO_GPU_BASE_PROPERTIES(VhostUserGPU, parent_obj.conf), DEFINE_PROP_END_OF_LIST(), @@ -586,6 +592,7 @@ vhost_user_gpu_class_init(ObjectClass *klass, void *data) vdc->guest_notifier_pending = vhost_user_gpu_guest_notifier_pending; vdc->get_config = vhost_user_gpu_get_config; vdc->set_config = vhost_user_gpu_set_config; +vdc->get_vhost = vhost_user_gpu_get_vhost; device_class_set_props(dc, vhost_user_gpu_properties); } diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c index 273e96a..43d2ff3 100644 --- a/hw/input/vhost-user-input.c +++ b/hw/input/vhost-user-input.c @@ -79,6 +79,12 @@ static void vhost_input_set_config(VirtIODevice *vdev, virtio_notify_config(vdev); } +static struct vhost_dev *vhost_input_get_vhost(VirtIODevice *vdev) +{ +VHostUserInput *vhi = VHOST_USER_INPUT(vdev); +return >vhost->dev; +} + static const VMStateDescription vmstate_vhost_input = { .name = "vhost-user-input", .unmigratable = 1, @@ -93,6 +99,7 @@ static void vhost_input_class_init(ObjectClass *klass, void *data) dc->vmsd = _vhost_input; vdc->get_config = vhost_input_get_config; vdc->set_config = vhost_input_set_config; +vdc->get_vhost = vhost_input_get_vhost; vic->realize = vhost_input_realize; vic->change_active = vhost_input_change_active; } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index bf59f8b..6e54436 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3549,6 +3549,14 @@ static bool dev_unplug_pending(void *opaque) return vdc->primary_unplug_pending(dev); } +static struct vhost_dev *virtio_net_get_vhost(VirtIODevice *vdev) +{ +VirtIONet *n = VIRTIO_NET(vdev); +NetClientState *nc = qemu_get_queue(n->nic); +struct vhost_net *net = get_vhost_net(nc->peer); +return >dev; +} + static const VMStateDescription vmstate_virtio_net = { .name = "virtio-net", .minimum_version_id = VIRTIO_NET_VM_VERSION, @@ -3651,6 +3659,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data) vdc->post_load = virtio_net_post_load_virtio; vdc->vmsd = _virtio_net_device; vdc->primary_unplug_pending = primary_unplug_pending; +vdc->get_vhost = virtio_net_get_vhost; } static const TypeInfo virtio_net_info = { diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 039caf2..b0a9c45 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -264,6 +264,13 @@ static void vhost_scsi_unrealize(DeviceState *dev) virtio_scsi_common_unrealize(dev); } +static struct vhost_dev *vhost_scsi_get_vhost(VirtIODevice *vdev) +{ +VHostSCSI *s = VHOST_SCSI(vdev); +VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s); +
Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote: > * Michael S. Tsirkin (m...@redhat.com) wrote: > > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote: > > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote: > > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote: > > > > > It might be useful for the cases when a slow block layer should be > > > > > replaced > > > > > with a more performant one on running VM without stopping, i.e. with > > > > > very low > > > > > downtime comparable with the one on migration. > > > > > > > > > > It's possible to achive that for two reasons: > > > > > > > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the > > > > > same. > > > > > They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from > > > > > each other in the values of migration service fields only. > > > > > 2.The device driver used in the guest is the same: virtio-blk > > > > > > > > > > In the series cross-migration is achieved by adding a new type. > > > > > The new type uses virtio-blk VMState instead of vhost-user-blk > > > > > specific > > > > > VMstate, also it implements migration save/load callbacks to be > > > > > compatible > > > > > with migration stream produced by "virtio-blk" device. > > > > > > > > > > Adding the new type instead of modifying the existing one is > > > > > convenent. > > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk > > > > > device from the existing non-compatible one using qemu machinery > > > > > without any > > > > > other modifiactions. That gives all the variety of qemu device related > > > > > constraints out of box. > > > > > > > > Hmm I'm not sure I understand. What is the advantage for the user? > > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk? > > > > We could add some hacks to make it compatible for old machine types. > > > > > > The point is that virtio-blk and vhost-user-blk are not > > > migration-compatible ATM. OTOH they are the same device from the guest > > > POV so there's nothing fundamentally preventing the migration between > > > the two. In particular, we see it as a means to switch between the > > > storage backend transports via live migration without disrupting the > > > guest. > > > > > > Migration-wise virtio-blk and vhost-user-blk have in common > > > > > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE > > > > > > The two differ in > > > > > > - the name and the version of the VMStateDescription > > > > > > - virtio-blk has an extra migration section (via .save/.load callbacks > > > on VirtioDeviceClass) containing requests in flight > > > > > > It looks like to become migration-compatible with virtio-blk, > > > vhost-user-blk has to start using VMStateDescription of virtio-blk and > > > provide compatible .save/.load callbacks. It isn't entirely obvious how > > > to make this machine-type-dependent, so we came up with a simpler idea > > > of defining a new device that shares most of the implementation with the > > > original vhost-user-blk except for the migration stuff. We're certainly > > > open to suggestions on how to reconcile this under a single > > > vhost-user-blk device, as this would be more user-friendly indeed. > > > > > > We considered using a class property for this and defining the > > > respective compat clause, but IIUC the class constructors (where .vmsd > > > and .save/.load are defined) are not supposed to depend on class > > > properties. > > > > > > Thanks, > > > Roman. > > > > So the question is how to make vmsd depend on machine type. > > CC Eduardo who poked at this kind of compat stuff recently, > > paolo who looked at qom things most recently and dgilbert > > for advice on migration. > > I don't think I've seen anyone change vmsd name dependent on machine > type; making fields appear/disappear is easy - that just ends up as a > property on the device that's checked; I guess if that property is > global (rather than per instance) then you can check it in > vhost_user_blk_class_init and swing the dc->vmsd pointer? class_init can be called very early during QEMU initialization, so it's too early to make decisions based on machine type. Making a specific vmsd appear/disappear based on machine configuration or state is "easy", by implementing VMStateDescription.needed. But this would require registering both vmsds (one of them would need to be registered manually instead of using DeviceClass.vmsd). I don't remember what are the consequences of not using DeviceClass.vmsd to register a vmsd, I only remember it was subtle. See commit b170fce3dd06 ("cpu: Register VMStateDescription through CPUState") and related threads. CCing Philippe, who might remember the details here. If that's an important use case, I would suggest allowing devices to implement a DeviceClass.get_vmsd method, which would override DeviceClass.vmsd if necessary. Is the
[PULL 03/12] x86/sev: generate SEV kernel loader hashes in x86_load_linux
From: Dov Murik If SEV is enabled and a kernel is passed via -kernel, pass the hashes of kernel/initrd/cmdline in an encrypted guest page to OVMF for SEV measured boot. Co-developed-by: James Bottomley Signed-off-by: James Bottomley Signed-off-by: Dov Murik Reviewed-by: Daniel P. Berrangé Message-Id: <20210930054915.13252-3-dovmu...@linux.ibm.com> Signed-off-by: Paolo Bonzini --- hw/i386/x86.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 41ef9a84a9..0c7c054e3a 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -47,6 +47,7 @@ #include "hw/i386/fw_cfg.h" #include "hw/intc/i8259.h" #include "hw/rtc/mc146818rtc.h" +#include "target/i386/sev_i386.h" #include "hw/acpi/cpu_hotplug.h" #include "hw/irq.h" @@ -780,6 +781,7 @@ void x86_load_linux(X86MachineState *x86ms, const char *initrd_filename = machine->initrd_filename; const char *dtb_filename = machine->dtb; const char *kernel_cmdline = machine->kernel_cmdline; +SevKernelLoaderContext sev_load_ctx = {}; /* Align to 16 bytes as a paranoia measure */ cmdline_size = (strlen(kernel_cmdline) + 16) & ~15; @@ -926,6 +928,8 @@ void x86_load_linux(X86MachineState *x86ms, fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_ADDR, cmdline_addr); fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline) + 1); fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline); +sev_load_ctx.cmdline_data = (char *)kernel_cmdline; +sev_load_ctx.cmdline_size = strlen(kernel_cmdline) + 1; if (protocol >= 0x202) { stl_p(header + 0x228, cmdline_addr); @@ -1007,6 +1011,8 @@ void x86_load_linux(X86MachineState *x86ms, fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr); fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size); fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size); +sev_load_ctx.initrd_data = initrd_data; +sev_load_ctx.initrd_size = initrd_size; stl_p(header + 0x218, initrd_addr); stl_p(header + 0x21c, initrd_size); @@ -1065,15 +1071,32 @@ void x86_load_linux(X86MachineState *x86ms, load_image_size(dtb_filename, setup_data->data, dtb_size); } -memcpy(setup, header, MIN(sizeof(header), setup_size)); +/* + * If we're starting an encrypted VM, it will be OVMF based, which uses the + * efi stub for booting and doesn't require any values to be placed in the + * kernel header. We therefore don't update the header so the hash of the + * kernel on the other side of the fw_cfg interface matches the hash of the + * file the user passed in. + */ +if (!sev_enabled()) { +memcpy(setup, header, MIN(sizeof(header), setup_size)); +} fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr); fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size); fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size); +sev_load_ctx.kernel_data = (char *)kernel; +sev_load_ctx.kernel_size = kernel_size; fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr); fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size); fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup, setup_size); +sev_load_ctx.setup_data = (char *)setup; +sev_load_ctx.setup_size = setup_size; + +if (sev_enabled()) { +sev_add_kernel_loader_hashes(_load_ctx, _fatal); +} option_rom[nb_option_roms].bootindex = 0; option_rom[nb_option_roms].name = "linuxboot.bin"; -- 2.31.1
[PULL 12/12] meson: show library versions in the summary
Meson 0.57 allows passing external programs and dependency objects to summary(). Use this to show library versions and paths in the summary. Signed-off-by: Paolo Bonzini --- meson.build | 112 +--- 1 file changed, 54 insertions(+), 58 deletions(-) diff --git a/meson.build b/meson.build index 17e77fe4ef..7b596fdcd9 100644 --- a/meson.build +++ b/meson.build @@ -2859,13 +2859,13 @@ summary_info = {} summary_info += {'git': config_host['GIT']} summary_info += {'make': config_host['MAKE']} summary_info += {'python':'@0@ (version: @1@)'.format(python.full_path(), python.language_version())} -summary_info += {'sphinx-build': sphinx_build.found()} +summary_info += {'sphinx-build': sphinx_build} if config_host.has_key('HAVE_GDB_BIN') summary_info += {'gdb': config_host['HAVE_GDB_BIN']} endif summary_info += {'genisoimage': config_host['GENISOIMAGE']} if targetos == 'windows' and config_host.has_key('CONFIG_GUEST_AGENT') - summary_info += {'wixl':wixl.found() ? wixl.full_path() : false} + summary_info += {'wixl':wixl} endif if slirp_opt != 'disabled' and 'CONFIG_SLIRP_SMBD' in config_host summary_info += {'smbd':config_host['CONFIG_SMBD_COMMAND']} @@ -2956,7 +2956,7 @@ if get_option('cfi') summary_info += {'CFI debug support': get_option('cfi_debug')} endif summary_info += {'strip binaries':get_option('strip')} -summary_info += {'sparse':sparse.found() ? sparse.full_path() : false} +summary_info += {'sparse':sparse} summary_info += {'mingw32 support': targetos == 'windows'} # snarf the cross-compilation information for tests @@ -3028,19 +3028,19 @@ if have_block summary_info += {'vvfat support': config_host.has_key('CONFIG_VVFAT')} summary_info += {'qed support': config_host.has_key('CONFIG_QED')} summary_info += {'parallels support': config_host.has_key('CONFIG_PARALLELS')} - summary_info += {'FUSE exports': fuse.found()} + summary_info += {'FUSE exports': fuse} endif summary(summary_info, bool_yn: true, section: 'Block layer support') # Crypto summary_info = {} summary_info += {'TLS priority': config_host['CONFIG_TLS_PRIORITY']} -summary_info += {'GNUTLS support':gnutls.found()} -summary_info += {'GNUTLS crypto': gnutls_crypto.found()} -# TODO: add back version -summary_info += {'libgcrypt': gcrypt.found()} -# TODO: add back version -summary_info += {'nettle':nettle.found()} +summary_info += {'GNUTLS support':gnutls} +if gnutls.found() + summary_info += {' GNUTLS crypto': gnutls_crypto.found()} +endif +summary_info += {'libgcrypt': gcrypt} +summary_info += {'nettle':nettle} if nettle.found() summary_info += {' XTS': xts != 'private'} endif @@ -3052,76 +3052,72 @@ summary(summary_info, bool_yn: true, section: 'Crypto') # Libraries summary_info = {} if targetos == 'darwin' - summary_info += {'Cocoa support': cocoa.found()} + summary_info += {'Cocoa support': cocoa} endif -# TODO: add back version -summary_info += {'SDL support': sdl.found()} -summary_info += {'SDL image support': sdl_image.found()} -# TODO: add back version -summary_info += {'GTK support': gtk.found()} -summary_info += {'pixman':pixman.found()} -# TODO: add back version -summary_info += {'VTE support': vte.found()} -# TODO: add back version -summary_info += {'slirp support': slirp_opt == 'disabled' ? false : slirp_opt} -summary_info += {'libtasn1': tasn1.found()} -summary_info += {'PAM': pam.found()} -summary_info += {'iconv support': iconv.found()} -summary_info += {'curses support':curses.found()} -# TODO: add back version -summary_info += {'virgl support': virgl.found()} -summary_info += {'curl support': curl.found()} -summary_info += {'Multipath support': mpathpersist.found()} -summary_info += {'VNC support': vnc.found()} +summary_info += {'SDL support': sdl} +summary_info += {'SDL image support': sdl_image} +summary_info += {'GTK support': gtk} +summary_info += {'pixman':pixman} +summary_info += {'VTE support': vte} +summary_info += {'slirp support': slirp_opt == 'internal' ? slirp_opt : slirp} +summary_info += {'libtasn1': tasn1} +summary_info += {'PAM': pam} +summary_info += {'iconv support': iconv} +summary_info += {'curses support':curses} +summary_info += {'virgl support': virgl} +summary_info += {'curl support': curl} +summary_info += {'Multipath support': mpathpersist} +summary_info += {'VNC support': vnc} if vnc.found() - summary_info += {'VNC SASL support': sasl.found()} - summary_info += {'VNC JPEG support': jpeg.found()} - summary_info += {'VNC PNG support': png.found()} + summary_info += {'VNC SASL support': sasl} +
[PULL 33/57] acpi: build_waet: use acpi_table_begin()/acpi_table_end() instead of build_header()
From: Igor Mammedov it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-20-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index c65ab1d6a5..e5cc4f7daa 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2223,10 +2223,10 @@ static void build_waet(GArray *table_data, BIOSLinker *linker, const char *oem_id, const char *oem_table_id) { -int waet_start = table_data->len; +AcpiTable table = { .sig = "WAET", .rev = 1, .oem_id = oem_id, +.oem_table_id = oem_table_id }; -/* WAET header */ -acpi_data_push(table_data, sizeof(AcpiTableHeader)); +acpi_table_begin(, table_data); /* * Set "ACPI PM timer good" flag. * @@ -2235,9 +2235,7 @@ build_waet(GArray *table_data, BIOSLinker *linker, const char *oem_id, * Which avoids costly VMExits caused by guest re-reading it unnecessarily. */ build_append_int_noprefix(table_data, 1 << 1 /* ACPI PM timer good */, 4); - -build_header(linker, table_data, (void *)(table_data->data + waet_start), - "WAET", table_data->len - waet_start, 1, oem_id, oem_table_id); +acpi_table_end(linker, ); } /* -- MST
[PULL 10/12] hexagon: use env keyword argument to pass PYTHONPATH
This feature is new in meson 0.57 and allows getting rid of the "env" wrapper. Signed-off-by: Paolo Bonzini --- target/hexagon/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build index 6fd9360b74..c6d858ffb2 100644 --- a/target/hexagon/meson.build +++ b/target/hexagon/meson.build @@ -156,7 +156,8 @@ dectree_generated = custom_target( 'dectree_generated.h.inc', output: 'dectree_generated.h.inc', depends: [iset_py], -command: ['env', 'PYTHONPATH=' + meson.current_build_dir(), files('dectree.py'), '@OUTPUT@'], +env: {'PYTHONPATH': meson.current_build_dir()}, +command: [python, files('dectree.py'), '@OUTPUT@'], ) hexagon_ss.add(dectree_generated) -- 2.31.1
[PULL 00/12] Misc changes for 2021-10-05
The following changes since commit 9618c5badaa8eed25259cf095ff880efb939fbe7: Merge remote-tracking branch 'remotes/vivier/tags/trivial-branch-for-6.2-pull-request' into staging (2021-10-04 16:27:35 -0700) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to bb647c49b8f1f986d8171dd61db65e8a8d255be0: meson: show library versions in the summary (2021-10-05 13:10:29 +0200) * Meson version update * fix search path when configuring with --cpu * support for measured SEV boot with -kernel (Dov) * fix missing BQL locks (Emanuele) * retrieve applesmc key from the host (Pedro) * KVM PV feature documentation (Vitaly) Dov Murik (2): sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot x86/sev: generate SEV kernel loader hashes in x86_load_linux Emanuele Giuseppe Esposito (2): migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread migration: add missing qemu_mutex_lock_iothread in migration_completion Paolo Bonzini (6): configure, meson: move CPU_CFLAGS out of QEMU_CFLAGS meson: bump submodule to 0.59.2 meson: switch minimum meson version to 0.58.2, minimum recommended to 0.59.2 hexagon: use env keyword argument to pass PYTHONPATH target/xtensa: list cores in a text file meson: show library versions in the summary Pedro Tôrres (1): hw/misc: applesmc: use host osk as default on macs Vitaly Kuznetsov (1): i386: docs: Briefly describe KVM PV features configure | 19 ++-- docs/meson.build | 14 +-- docs/system/i386/kvm-pv.rst| 100 + docs/system/target-i386.rst| 1 + hw/i386/x86.c | 25 +- hw/misc/applesmc.c | 192 - meson | 2 +- meson.build| 166 +-- migration/block-dirty-bitmap.c | 5 +- migration/migration.c | 3 + plugins/meson.build| 4 +- scripts/mtest2make.py | 7 +- target/hexagon/meson.build | 3 +- target/i386/sev-stub.c | 5 ++ target/i386/sev.c | 137 + target/i386/sev_i386.h | 12 +++ target/xtensa/cores.list | 9 ++ target/xtensa/import_core.sh | 3 + target/xtensa/meson.build | 4 +- tests/qapi-schema/meson.build | 4 +- tests/qtest/meson.build| 2 +- tests/unit/meson.build | 2 +- trace/meson.build | 4 +- 23 files changed, 597 insertions(+), 126 deletions(-) create mode 100644 docs/system/i386/kvm-pv.rst create mode 100644 target/xtensa/cores.list -- 2.31.1
[PULL 27/57] acpi: x86: build_dsdt: use acpi_table_begin()/acpi_table_end() instead of build_header()
From: Igor Mammedov it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-14-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f4d6ae3d02..e17451bc6d 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1405,12 +1405,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, #endif int i; VMBusBridge *vmbus_bridge = vmbus_bridge_find(); +AcpiTable table = { .sig = "DSDT", .rev = 1, .oem_id = x86ms->oem_id, +.oem_table_id = x86ms->oem_table_id }; +acpi_table_begin(, table_data); dsdt = init_aml_allocator(); -/* Reserve space for header */ -acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader)); - build_dbg_aml(dsdt); if (misc->is_piix4) { sb_scope = aml_scope("_SB"); @@ -1867,9 +1867,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, /* copy AML table into ACPI tables blob and patch header there */ g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len); -build_header(linker, table_data, -(void *)(table_data->data + table_data->len - dsdt->buf->len), - "DSDT", dsdt->buf->len, 1, x86ms->oem_id, x86ms->oem_table_id); +acpi_table_end(linker, ); free_aml_allocator(); } -- MST
[PULL 07/12] migration: add missing qemu_mutex_lock_iothread in migration_completion
From: Emanuele Giuseppe Esposito qemu_savevm_state_complete_postcopy assumes the iothread lock (BQL) to be held, but instead it isn't. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Dr. David Alan Gilbert Message-Id: <20211005080751.3797161-3-eespo...@redhat.com> Signed-off-by: Paolo Bonzini --- migration/migration.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/migration/migration.c b/migration/migration.c index bb909781b7..6ac807ef3d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -3168,7 +3168,10 @@ static void migration_completion(MigrationState *s) } else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) { trace_migration_completion_postcopy_end(); +qemu_mutex_lock_iothread(); qemu_savevm_state_complete_postcopy(s->to_dst_file); +qemu_mutex_unlock_iothread(); + trace_migration_completion_postcopy_end_after_complete(); } else if (s->state == MIGRATION_STATUS_CANCELLING) { goto fail; -- 2.31.1
Re: [PATCH v6 05/10] ACPI ERST: support for ACPI ERST feature
Igor, again thanks for the detailed review. Inline responses below. eric On 10/5/21 6:39 AM, Igor Mammedov wrote: On Mon, 4 Oct 2021 16:13:09 -0500 Eric DeVolder wrote: Igor, thanks for the close examination. Inline responses below. eric On 9/21/21 10:30 AM, Igor Mammedov wrote: On Thu, 5 Aug 2021 18:30:34 -0400 Eric DeVolder wrote: This implements a PCI device for ACPI ERST. This implements the non-NVRAM "mode" of operation for ERST as it is supported by Linux and Windows. Signed-off-by: Eric DeVolder --- hw/acpi/erst.c | 750 +++ hw/acpi/meson.build | 1 + hw/acpi/trace-events | 15 ++ 3 files changed, 766 insertions(+) create mode 100644 hw/acpi/erst.c diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c new file mode 100644 index 000..eb4ab34 --- /dev/null +++ b/hw/acpi/erst.c @@ -0,0 +1,750 @@ +/* + * ACPI Error Record Serialization Table, ERST, Implementation + * + * ACPI ERST introduced in ACPI 4.0, June 16, 2009. + * ACPI Platform Error Interfaces : Error Serialization + * + * Copyright (c) 2021 Oracle and/or its affiliates. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include +#include +#include + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "hw/qdev-core.h" +#include "exec/memory.h" +#include "qom/object.h" +#include "hw/pci/pci.h" +#include "qom/object_interfaces.h" +#include "qemu/error-report.h" +#include "migration/vmstate.h" +#include "hw/qdev-properties.h" +#include "hw/acpi/acpi.h" +#include "hw/acpi/acpi-defs.h" +#include "hw/acpi/aml-build.h" +#include "hw/acpi/bios-linker-loader.h" +#include "exec/address-spaces.h" +#include "sysemu/hostmem.h" +#include "hw/acpi/erst.h" +#include "trace.h" + +/* ACPI 4.0: Table 17-16 Serialization Actions */ +#define ACTION_BEGIN_WRITE_OPERATION 0x0 +#define ACTION_BEGIN_READ_OPERATION 0x1 +#define ACTION_BEGIN_CLEAR_OPERATION 0x2 +#define ACTION_END_OPERATION 0x3 +#define ACTION_SET_RECORD_OFFSET 0x4 +#define ACTION_EXECUTE_OPERATION 0x5 +#define ACTION_CHECK_BUSY_STATUS 0x6 +#define ACTION_GET_COMMAND_STATUS0x7 +#define ACTION_GET_RECORD_IDENTIFIER 0x8 +#define ACTION_SET_RECORD_IDENTIFIER 0x9 +#define ACTION_GET_RECORD_COUNT 0xA +#define ACTION_BEGIN_DUMMY_WRITE_OPERATION 0xB +#define ACTION_RESERVED 0xC +#define ACTION_GET_ERROR_LOG_ADDRESS_RANGE 0xD +#define ACTION_GET_ERROR_LOG_ADDRESS_LENGTH 0xE +#define ACTION_GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES 0xF +#define ACTION_GET_EXECUTE_OPERATION_TIMINGS 0x10 + +/* ACPI 4.0: Table 17-17 Command Status Definitions */ +#define STATUS_SUCCESS0x00 +#define STATUS_NOT_ENOUGH_SPACE 0x01 +#define STATUS_HARDWARE_NOT_AVAILABLE 0x02 +#define STATUS_FAILED 0x03 +#define STATUS_RECORD_STORE_EMPTY 0x04 +#define STATUS_RECORD_NOT_FOUND 0x05 + + +/* UEFI 2.1: Appendix N Common Platform Error Record */ +#define UEFI_CPER_RECORD_MIN_SIZE 128U +#define UEFI_CPER_RECORD_LENGTH_OFFSET 20U +#define UEFI_CPER_RECORD_ID_OFFSET 96U +#define IS_UEFI_CPER_RECORD(ptr) \ +(((ptr)[0] == 'C') && \ + ((ptr)[1] == 'P') && \ + ((ptr)[2] == 'E') && \ + ((ptr)[3] == 'R')) +#define THE_UEFI_CPER_RECORD_ID(ptr) \ +(*(uint64_t *)(&(ptr)[UEFI_CPER_RECORD_ID_OFFSET])) + +/* + * This implementation is an ACTION (cmd) and VALUE (data) + * interface consisting of just two 64-bit registers. + */ +#define ERST_REG_SIZE (16UL) +#define ERST_ACTION_OFFSET (0UL) /* action (cmd) */ +#define ERST_VALUE_OFFSET (8UL) /* argument/value (data) */ + +/* + * ERST_RECORD_SIZE is the buffer size for exchanging ERST + * record contents. Thus, it defines the maximum record size. + * As this is mapped through a PCI BAR, it must be a power of + * two and larger than UEFI_CPER_RECORD_MIN_SIZE. + * The backing storage is divided into fixed size "slots", + * each ERST_RECORD_SIZE in length, and each "slot" + * storing a single record. No attempt at optimizing storage + * through compression, compaction, etc is attempted. + * NOTE that slot 0 is reserved for the backing storage header. + * Depending upon the size of the backing storage, additional + * slots will be part of the slot 0 header in order to account + * for a record_id for each available remaining slot. + */ +/* 8KiB records, not too small, not too big */ +#define ERST_RECORD_SIZE (8192UL) + +#define ACPI_ERST_MEMDEV_PROP "memdev" + +/* + * From the ACPI ERST spec sections: + * A record id of all 0s is used to indicate + * 'unspecified' record id. + * A record id of all 1s is used to indicate + * empty or end. + */ +#define ERST_UNSPECIFIED_RECORD_ID (0UL) +#define ERST_EMPTY_END_RECORD_ID (~0UL) +#define ERST_EXECUTE_OPERATION_MAGIC 0x9CUL +#define ERST_IS_VALID_RECORD_ID(rid) \ +((rid != ERST_UNSPECIFIED_RECORD_ID) && \ + (rid != ERST_EMPTY_END_RECORD_ID)) + +typedef
[PULL 55/57] hw/i386/amd_iommu: Rename amdviPCI TypeInfo
From: Philippe Mathieu-Daudé Per 'QEMU Coding Style': Naming == Variables are lower_case_with_underscores; easy to type and read. Rename amdviPCI variable as amdvi_pci. amdviPCI_register_types() register more than PCI types: TYPE_AMD_IOMMU_DEVICE inherits TYPE_X86_IOMMU_DEVICE which itself inherits TYPE_SYS_BUS_DEVICE. Rename it more generically as amdvi_register_types(). Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210926175648.1649075-2-f4...@amsat.org> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 2801dff97c..0c994facde 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1621,7 +1621,7 @@ static const TypeInfo amdvi = { .class_init = amdvi_class_init }; -static const TypeInfo amdviPCI = { +static const TypeInfo amdvi_pci = { .name = TYPE_AMD_IOMMU_PCI, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(AMDVIPCIState), @@ -1645,11 +1645,11 @@ static const TypeInfo amdvi_iommu_memory_region_info = { .class_init = amdvi_iommu_memory_region_class_init, }; -static void amdviPCI_register_types(void) +static void amdvi_register_types(void) { -type_register_static(); +type_register_static(_pci); type_register_static(); type_register_static(_iommu_memory_region_info); } -type_init(amdviPCI_register_types); +type_init(amdvi_register_types); -- MST
[PULL 08/12] meson: bump submodule to 0.59.2
The update to 0.57 has been delayed due to it causing warnings for some actual issues, but it brings in important bugfixes and new features. 0.58 also brings in a bugfix that is useful for modinfo. Important bugfixes: - 0.57: https://github.com/mesonbuild/meson/pull/7760, build: use PIE objects for non-PIC static libraries if b_pie=true - 0.57: https://github.com/mesonbuild/meson/pull/7900, thus avoiding unnecessary rebuilds after running meson. - 0.58.2: https://github.com/mesonbuild/meson/pull/8900, fixes for passing extract_objects() to custom_target (useful for modinfo) Features: - 0.57: the keyval module has now been stabilized - 0.57: env argument to custom_target (useful for hexagon) - 0.57: Feature parity between "meson test" and QEMU's TAP driver - 0.57: https://github.com/mesonbuild/meson/pull/8231, allows bringing back version numbers in the configuration summary - 0.59: Utility methods for feature objects Signed-off-by: Paolo Bonzini --- meson | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson b/meson index 776acd2a80..b25d94e7c7 16 --- a/meson +++ b/meson @@ -1 +1 @@ -Subproject commit 776acd2a805c9b42b4f0375150977df42130317f +Subproject commit b25d94e7c77fda05a7fdfe8afe562cf9760d69da -- 2.31.1
[PULL 53/57] virtio-balloon: Fix page-poison subsection name
From: "Dr. David Alan Gilbert" The subsection name for page-poison was typo'd as: vitio-balloon-device/page-poison Note the missing 'r' in virtio. When we have a machine type that enables page poison, and the guest enables it (which needs a new kernel), things fail rather unpredictably. The fallout from this is that most of the other subsections fail to load, including things like the feature bits in the device, one possible fallout is that the physical addresses of the queues then get aligned differently and we fail with an error about last_avail_idx being wrong. It's not obvious to me why this doesn't produce a more obvious failure, but virtio's vmstate loading is a bit open-coded. Fixes: 7483cbbaf82 ("virtio-balloon: Implement support for page poison reporting feature") bz: https://bugzilla.redhat.com/show_bug.cgi?id=1984401 Signed-off-by: Dr. David Alan Gilbert Message-Id: <20210914131716.102851-1-dgilb...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: David Hildenbrand --- hw/virtio/virtio-balloon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 5a69dce35d..c6962fcbfe 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -852,7 +852,7 @@ static const VMStateDescription vmstate_virtio_balloon_free_page_hint = { }; static const VMStateDescription vmstate_virtio_balloon_page_poison = { -.name = "vitio-balloon-device/page-poison", +.name = "virtio-balloon-device/page-poison", .version_id = 1, .minimum_version_id = 1, .needed = virtio_balloon_page_poison_support, -- MST
[PULL 05/12] configure, meson: move CPU_CFLAGS out of QEMU_CFLAGS
Flags that choose the target architecture, such as -m32 on x86, affect all invocations of the compiler driver, for example including options such as --print-search-dirs. To ensure that they are treated as such, place them in the cross file in the [binaries] section instead of including them in QEMU_CFLAGS. Signed-off-by: Paolo Bonzini --- configure | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/configure b/configure index b0b1a1cc25..1d3f099498 100755 --- a/configure +++ b/configure @@ -142,11 +142,11 @@ lines: ${BASH_LINENO[*]}" } do_cc() { -do_compiler "$cc" "$@" +do_compiler "$cc" $CPU_CFLAGS "$@" } do_cxx() { -do_compiler "$cxx" "$@" +do_compiler "$cxx" $CPU_CFLAGS "$@" } # Append $2 to the variable named $1, with space separation @@ -1688,7 +1688,6 @@ esac eval "cross_cc_${cpu}=\$cc" cross_cc_vars="$cross_cc_vars cross_cc_${cpu}" -QEMU_CFLAGS="$CPU_CFLAGS $QEMU_CFLAGS" # For user-mode emulation the host arch has to be one we explicitly # support, even if we're using TCI. @@ -5114,9 +5113,9 @@ if test "$skip_meson" = no; then echo "c_link_args = [${LDFLAGS:+$(meson_quote $LDFLAGS)}]" >> $cross echo "cpp_link_args = [${LDFLAGS:+$(meson_quote $LDFLAGS)}]" >> $cross echo "[binaries]" >> $cross - echo "c = [$(meson_quote $cc)]" >> $cross - test -n "$cxx" && echo "cpp = [$(meson_quote $cxx)]" >> $cross - test -n "$objcc" && echo "objc = [$(meson_quote $objcc)]" >> $cross + echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross + test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross + test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> $cross echo "ar = [$(meson_quote $ar)]" >> $cross echo "nm = [$(meson_quote $nm)]" >> $cross echo "pkgconfig = [$(meson_quote $pkg_config_exe)]" >> $cross -- 2.31.1
[PULL 11/12] target/xtensa: list cores in a text file
Avoid that leftover files affect the build; instead, use the same mechanism that was in place before the Meson transition of updating a file from import_core.sh. Starting with Meson 0.57, the file can be easily read from the filesystem module, so do that instead of using run_command. Signed-off-by: Paolo Bonzini --- target/xtensa/cores.list | 9 + target/xtensa/import_core.sh | 3 +++ target/xtensa/meson.build| 4 ++-- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 target/xtensa/cores.list diff --git a/target/xtensa/cores.list b/target/xtensa/cores.list new file mode 100644 index 00..5772a00ab2 --- /dev/null +++ b/target/xtensa/cores.list @@ -0,0 +1,9 @@ +core-dc232b.c +core-dc233c.c +core-de212.c +core-de233_fpu.c +core-dsp3400.c +core-fsf.c +core-sample_controller.c +core-test_kc705_be.c +core-test_mmuhifi_c3.c diff --git a/target/xtensa/import_core.sh b/target/xtensa/import_core.sh index 396b264be9..df66d09393 100755 --- a/target/xtensa/import_core.sh +++ b/target/xtensa/import_core.sh @@ -66,3 +66,6 @@ static XtensaConfig $NAME __attribute__((unused)) = { REGISTER_CORE($NAME) EOF + +grep -qxf core-${NAME}.c "$BASE"/cores.list || \ +echo core-${NAME}.c >> "$BASE"/cores.list diff --git a/target/xtensa/meson.build b/target/xtensa/meson.build index 7c4efa6c62..20bbf9b335 100644 --- a/target/xtensa/meson.build +++ b/target/xtensa/meson.build @@ -1,7 +1,7 @@ xtensa_ss = ss.source_set() -xtensa_cores = run_command('sh', '-c', 'cd $MESON_SOURCE_ROOT/$MESON_SUBDIR ; ls -1 core-*.c') -xtensa_ss.add(files(xtensa_cores.stdout().strip().split('\n'))) +xtensa_cores = fs.read('cores.list') +xtensa_ss.add(files(xtensa_cores.strip().split('\n'))) xtensa_ss.add(files( 'cpu.c', -- 2.31.1
[PULL 49/57] acpi: AcpiGenericAddress no longer used to map/access fields of MMIO, drop packed attribute
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-36-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/acpi-defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index ee733840aa..c97e8633ad 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -55,7 +55,7 @@ struct AcpiGenericAddress { uint8_t access_width;/* ACPI 3.0: Minimum Access size (ACPI 3.0), ACPI 2.0: Reserved, Table 5-1 */ uint64_t address;/* 64-bit address of struct or register */ -} QEMU_PACKED; +}; typedef struct AcpiFadtData { struct AcpiGenericAddress pm1a_cnt; /* PM1a_CNT_BLK */ -- MST
[PULL 04/12] hw/misc: applesmc: use host osk as default on macs
From: Pedro Tôrres When running on a Mac, QEMU is able to get the host OSK and use it as the default value for the AppleSMC device. The OSK query operation doesn't require administrator privileges and can be executed by any user on the system. This patch is based on open-source code from Apple, just like the implementation from VirtualBox. Apple: https://opensource.apple.com/source/IOKitUser/IOKitUser-647.6.13/pwr_mgt.subproj/IOPMLibPrivate.c https://opensource.apple.com/source/PowerManagement/PowerManagement-637.60.1/pmconfigd/PrivateLib.c VirtualBox: https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Devices/EFI/DevSmc.cpp#L516 Signed-off-by: Pedro Tôrres --- hw/misc/applesmc.c | 192 - 1 file changed, 191 insertions(+), 1 deletion(-) diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c index 1b9acaf1d3..cec247b5ee 100644 --- a/hw/misc/applesmc.c +++ b/hw/misc/applesmc.c @@ -38,6 +38,171 @@ #include "qemu/timer.h" #include "qom/object.h" +#if defined(__APPLE__) && defined(__MACH__) +#include + +enum { +kSMCSuccess = 0x00, +kSMCKeyNotFound = 0x84 +}; + +enum { +kSMCUserClientOpen = 0x00, +kSMCUserClientClose = 0x01, +kSMCHandleYPCEvent = 0x02, +kSMCReadKey = 0x05, +kSMCGetKeyInfo = 0x09 +}; + +typedef struct SMCVersion { +uint8_t major; +uint8_t minor; +uint8_t build; +uint8_t reserved; +uint16_t release; +} SMCVersion; + +typedef struct SMCPLimitData { +uint16_t version; +uint16_t length; +uint32_t cpuPLimit; +uint32_t gpuPLimit; +uint32_t memPLimit; +} SMCPLimitData; + +typedef struct SMCKeyInfoData { +IOByteCount dataSize; +uint32_tdataType; +uint8_t dataAttributes; +} SMCKeyInfoData; + +typedef struct { +uint32_t key; +SMCVersion vers; +SMCPLimitData pLimitData; +SMCKeyInfoData keyInfo; +uint8_tresult; +uint8_tstatus; +uint8_tdata8; +uint32_t data32; +uint8_tbytes[32]; +} SMCParamStruct; + +static IOReturn smc_call_struct_method(uint32_t selector, + SMCParamStruct *inputStruct, + SMCParamStruct *outputStruct) +{ +IOReturn ret; + +size_t inputStructCnt = sizeof(SMCParamStruct); +size_t outputStructCnt = sizeof(SMCParamStruct); + +io_service_t smcService = IO_OBJECT_NULL; +io_connect_t smcConnect = IO_OBJECT_NULL; + +smcService = IOServiceGetMatchingService(kIOMasterPortDefault, + IOServiceMatching("AppleSMC")); +if (smcService == IO_OBJECT_NULL) { +ret = kIOReturnNotFound; +goto exit; +} + +ret = IOServiceOpen(smcService, mach_task_self(), 1, ); +if (ret != kIOReturnSuccess) { +smcConnect = IO_OBJECT_NULL; +goto exit; +} +if (smcConnect == IO_OBJECT_NULL) { +ret = kIOReturnError; +goto exit; +} + +ret = IOConnectCallMethod(smcConnect, kSMCUserClientOpen, + NULL, 0, NULL, 0, + NULL, NULL, NULL, NULL); +if (ret != kIOReturnSuccess) { +goto exit; +} + +ret = IOConnectCallStructMethod(smcConnect, selector, +inputStruct, inputStructCnt, +outputStruct, ); + +exit: +if (smcConnect != IO_OBJECT_NULL) { +IOConnectCallMethod(smcConnect, kSMCUserClientClose, +NULL, 0, NULL, 0, NULL, +NULL, NULL, NULL); +IOServiceClose(smcConnect); +} + +return ret; +} + +static IOReturn smc_read_key(uint32_t key, + uint8_t *bytes, + IOByteCount *dataSize) +{ +IOReturn ret; + +SMCParamStruct inputStruct; +SMCParamStruct outputStruct; + +if (key == 0 || bytes == NULL) { +ret = kIOReturnCannotWire; +goto exit; +} + +/* determine key's data size */ +memset(, 0, sizeof(SMCParamStruct)); +inputStruct.data8 = kSMCGetKeyInfo; +inputStruct.key = key; + +memset(, 0, sizeof(SMCParamStruct)); +ret = smc_call_struct_method(kSMCHandleYPCEvent, , ); +if (ret != kIOReturnSuccess) { +goto exit; +} +if (outputStruct.result == kSMCKeyNotFound) { +ret = kIOReturnNotFound; +goto exit; +} +if (outputStruct.result != kSMCSuccess) { +ret = kIOReturnInternalError; +goto exit; +} + +/* get key value */ +memset(, 0, sizeof(SMCParamStruct)); +inputStruct.data8 = kSMCReadKey; +inputStruct.key = key; +inputStruct.keyInfo.dataSize = outputStruct.keyInfo.dataSize; + +memset(, 0, sizeof(SMCParamStruct)); +ret = smc_call_struct_method(kSMCHandleYPCEvent, , ); +if (ret != kIOReturnSuccess) { +goto exit; +} +if (outputStruct.result ==
Re: [RFC PATCH v2 03/25] block/block-backend.c: assertions for block-backend
On Tue, Oct 05, 2021 at 10:31:53AM -0400, Emanuele Giuseppe Esposito wrote: > All the global state (GS) API functions will check that > qemu_in_main_thread() returns true. If not, it means > that the safety of BQL cannot be guaranteed, and > they need to be moved to I/O. > > Signed-off-by: Emanuele Giuseppe Esposito > --- > block/block-backend.c | 89 +- > softmmu/qdev-monitor.c | 2 + > 2 files changed, 90 insertions(+), 1 deletion(-) > > diff --git a/block/block-backend.c b/block/block-backend.c > index d31ae16b99..9cd3b27b53 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -227,6 +227,7 @@ static void blk_root_activate(BdrvChild *child, Error > **errp) > > void blk_set_force_allow_inactivate(BlockBackend *blk) > { > +g_assert(qemu_in_main_thread()); Why g_assert()? > @@ -661,6 +676,7 @@ bool monitor_add_blk(BlockBackend *blk, const char *name, > Error **errp) > { > assert(!blk->name); > assert(name && name[0]); > +g_assert(qemu_in_main_thread()); especially why mixed spellings? Per osdep.h, we don't support builds with NDEBUG or G_DISABLE_ASSERT defined to their non-default values, so behavior isn't really different, but consistency says we use 'assert' more frequently than 'g_assert'. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[PULL 46/57] acpi: arm/virt: build_gtdt: use acpi_table_begin()/acpi_table_end() instead of build_header()
From: Igor Mammedov it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. while at it, replace packed structure with endian agnostic build_append_FOO() API. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-33-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/acpi-defs.h | 25 - hw/arm/virt-acpi-build.c| 75 - 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 012c4ffb3a..0b375e7589 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -131,29 +131,4 @@ struct AcpiFacsDescriptorRev1 { } QEMU_PACKED; typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; -/* - * Generic Timer Description Table (GTDT) - */ -#define ACPI_GTDT_INTERRUPT_MODE_LEVEL(0 << 0) -#define ACPI_GTDT_INTERRUPT_MODE_EDGE (1 << 0) -#define ACPI_GTDT_CAP_ALWAYS_ON (1 << 2) - -struct AcpiGenericTimerTable { -ACPI_TABLE_HEADER_DEF -uint64_t counter_block_addresss; -uint32_t reserved; -uint32_t secure_el1_interrupt; -uint32_t secure_el1_flags; -uint32_t non_secure_el1_interrupt; -uint32_t non_secure_el1_flags; -uint32_t virtual_timer_interrupt; -uint32_t virtual_timer_flags; -uint32_t non_secure_el2_interrupt; -uint32_t non_secure_el2_flags; -uint64_t counter_read_block_address; -uint32_t platform_timer_count; -uint32_t platform_timer_offset; -} QEMU_PACKED; -typedef struct AcpiGenericTimerTable AcpiGenericTimerTable; - #endif diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 7b79fae0ad..6cec97352b 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -559,40 +559,61 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) acpi_table_end(linker, ); } -/* GTDT */ +/* + * ACPI spec, Revision 5.1 + * 5.2.24 Generic Timer Description Table (GTDT) + */ static void build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); -int gtdt_start = table_data->len; -AcpiGenericTimerTable *gtdt; -uint32_t irqflags; +/* + * Table 5-117 Flag Definitions + * set only "Timer interrupt Mode" and assume "Timer Interrupt + * polarity" bit as '0: Interrupt is Active high' + */ +uint32_t irqflags = vmc->claim_edge_triggered_timers ? +1 : /* Interrupt is Edge triggered */ +0; /* Interrupt is Level triggered */ +AcpiTable table = { .sig = "GTDT", .rev = 2, .oem_id = vms->oem_id, +.oem_table_id = vms->oem_table_id }; -if (vmc->claim_edge_triggered_timers) { -irqflags = ACPI_GTDT_INTERRUPT_MODE_EDGE; -} else { -irqflags = ACPI_GTDT_INTERRUPT_MODE_LEVEL; -} +acpi_table_begin(, table_data); -gtdt = acpi_data_push(table_data, sizeof *gtdt); -/* The interrupt values are the same with the device tree when adding 16 */ -gtdt->secure_el1_interrupt = cpu_to_le32(ARCH_TIMER_S_EL1_IRQ + 16); -gtdt->secure_el1_flags = cpu_to_le32(irqflags); +/* CntControlBase Physical Address */ +/* FIXME: invalid value, should be 0x if not impl. ? */ +build_append_int_noprefix(table_data, 0, 8); +build_append_int_noprefix(table_data, 0, 4); /* Reserved */ +/* + * FIXME: clarify comment: + * The interrupt values are the same with the device tree when adding 16 + */ +/* Secure EL1 timer GSIV */ +build_append_int_noprefix(table_data, ARCH_TIMER_S_EL1_IRQ + 16, 4); +/* Secure EL1 timer Flags */ +build_append_int_noprefix(table_data, irqflags, 4); +/* Non-Secure EL1 timer GSIV */ +build_append_int_noprefix(table_data, ARCH_TIMER_NS_EL1_IRQ + 16, 4); +/* Non-Secure EL1 timer Flags */ +build_append_int_noprefix(table_data, irqflags | + 1UL << 2, /* Always-on Capability */ + 4); +/* Virtual timer GSIV */ +build_append_int_noprefix(table_data, ARCH_TIMER_VIRT_IRQ + 16, 4); +/* Virtual Timer Flags */ +build_append_int_noprefix(table_data, irqflags, 4); +/* Non-Secure EL2 timer GSIV */ +build_append_int_noprefix(table_data, ARCH_TIMER_NS_EL2_IRQ + 16, 4); +/* Non-Secure EL2 timer Flags */ +build_append_int_noprefix(table_data, irqflags, 4); +/* CntReadBase Physical address */ +build_append_int_noprefix(table_data, 0, 8); +/* Platform Timer Count */ +build_append_int_noprefix(table_data, 0, 4); +/* Platform Timer Offset */ +build_append_int_noprefix(table_data, 0, 4); -gtdt->non_secure_el1_interrupt = cpu_to_le32(ARCH_TIMER_NS_EL1_IRQ + 16); -gtdt->non_secure_el1_flags = cpu_to_le32(irqflags |
Re: [PATCH 0/2] block: avoid integer overflow of `max-workers` and assert `max_busy_tasks`
10/5/21 19:11, Stefano Garzarella wrote: This series contains a patch that avoids an integer overflow of `max-workers` (struct BackupPerf) by adding a check and a patch that asserts this condition where the problem occurs. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310 Signed-off-by: Stefano Garzarella Stefano Garzarella (2): block/backup: avoid integer overflow of `max-workers` block/aio_task: assert `max_busy_tasks` is greater than 0 block/aio_task.c | 2 ++ block/backup.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) Thanks for fixing, I'm applying it to my jobs branch. -- Best regards, Vladimir
[PULL 06/57] vhost-vdpa: let net_vhost_vdpa_init() returns NetClientState *
From: Jason Wang This patch switches to let net_vhost_vdpa_init() to return NetClientState *. This is used for the callers to allocate multiqueue NetClientState for multiqueue support. Signed-off-by: Jason Wang Message-Id: <20210907090322.1756-5-jasow...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- net/vhost-vdpa.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 73d29a74ef..834dab28dd 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -155,8 +155,10 @@ static NetClientInfo net_vhost_vdpa_info = { .has_ufo = vhost_vdpa_has_ufo, }; -static int net_vhost_vdpa_init(NetClientState *peer, const char *device, - const char *name, int vdpa_device_fd) +static NetClientState *net_vhost_vdpa_init(NetClientState *peer, + const char *device, + const char *name, + int vdpa_device_fd) { NetClientState *nc = NULL; VhostVDPAState *s; @@ -170,8 +172,9 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device, ret = vhost_vdpa_add(nc, (void *)>vhost_vdpa); if (ret) { qemu_del_net_client(nc); +return NULL; } -return ret; +return nc; } static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp) @@ -196,7 +199,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name, NetClientState *peer, Error **errp) { const NetdevVhostVDPAOptions *opts; -int vdpa_device_fd, ret; +int vdpa_device_fd; +NetClientState *nc; assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA); opts = >u.vhost_vdpa; @@ -211,10 +215,11 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name, return -errno; } -ret = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, vdpa_device_fd); -if (ret) { +nc = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name, vdpa_device_fd); +if (!nc) { qemu_close(vdpa_device_fd); +return -1; } -return ret; +return 0; } -- MST
Re: [PATCH 2/2] block/aio_task: assert `max_busy_tasks` is greater than 0
10/5/21 19:11, Stefano Garzarella wrote: All code in block/aio_task.c expects `max_busy_tasks` to always be greater than 0. Assert this condition during the AioTaskPool creation where `max_busy_tasks` is set. Signed-off-by: Stefano Garzarella Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH v2 1/3] virtio: turn VIRTQUEUE_MAX_SIZE into a variable
On Dienstag, 5. Oktober 2021 17:10:40 CEST Stefan Hajnoczi wrote: > On Tue, Oct 05, 2021 at 03:15:26PM +0200, Christian Schoenebeck wrote: > > On Dienstag, 5. Oktober 2021 14:45:56 CEST Stefan Hajnoczi wrote: > > > On Mon, Oct 04, 2021 at 09:38:04PM +0200, Christian Schoenebeck wrote: > > > > Refactor VIRTQUEUE_MAX_SIZE to effectively become a runtime > > > > variable per virtio user. > > > > > > virtio user == virtio device model? > > > > Yes > > > > > > Reasons: > > > > > > > > (1) VIRTQUEUE_MAX_SIZE should reflect the absolute theoretical > > > > > > > > maximum queue size possible. Which is actually the maximum > > > > queue size allowed by the virtio protocol. The appropriate > > > > value for VIRTQUEUE_MAX_SIZE would therefore be 32768: > > > > > > > > https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs > > > > 01.h > > > > tml#x1-240006 > > > > > > > > Apparently VIRTQUEUE_MAX_SIZE was instead defined with a > > > > more or less arbitrary value of 1024 in the past, which > > > > limits the maximum transfer size with virtio to 4M > > > > (more precise: 1024 * PAGE_SIZE, with the latter typically > > > > being 4k). > > > > > > Being equal to IOV_MAX is a likely reason. Buffers with more iovecs than > > > that cannot be passed to host system calls (sendmsg(2), pwritev(2), > > > etc). > > > > Yes, that's use case dependent. Hence the solution to opt-in if it is > > desired and feasible. > > > > > > (2) Additionally the current value of 1024 poses a hidden limit, > > > > > > > > invisible to guest, which causes a system hang with the > > > > following QEMU error if guest tries to exceed it: > > > > > > > > virtio: too many write descriptors in indirect table > > > > > > I don't understand this point. 2.6.5 The Virtqueue Descriptor Table says: > > > The number of descriptors in the table is defined by the queue size > > > for > > > > > > this virtqueue: this is the maximum possible descriptor chain length. > > > > > > and 2.6.5.3.1 Driver Requirements: Indirect Descriptors says: > > > A driver MUST NOT create a descriptor chain longer than the Queue Size > > > of > > > > > > the device. > > > > > > Do you mean a broken/malicious guest driver that is violating the spec? > > > That's not a hidden limit, it's defined by the spec. > > > > https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg00781.html > > https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg00788.html > > > > You can already go beyond that queue size at runtime with the indirection > > table. The only actual limit is the currently hard coded value of 1k > > pages. > > Hence the suggestion to turn that into a variable. > > Exceeding Queue Size is a VIRTIO spec violation. Drivers that operate > outsided the spec do so at their own risk. They may not be compatible > with all device implementations. Yes, I am ware about that. And still, this practice is already done, which apparently is not limited to 9pfs. > The limit is not hidden, it's Queue Size as defined by the spec :). > > If you have a driver that is exceeding the limit, then please fix the > driver. I absolutely understand your position, but I hope you also understand that this violation of the specs is a theoretical issue, it is not a real-life problem right now, and due to lack of man power unfortunately I have to prioritize real-life problems over theoretical ones ATM. Keep in mind that right now I am the only person working on 9pfs actively, I do this voluntarily whenever I find a free time slice, and I am not paid for it either. I don't see any reasonable way with reasonable effort to do what you are asking for here in 9pfs, and Greg may correct me here if I am saying anything wrong. If you are seeing any specific real-life issue here, then please tell me which one, otherwise I have to postpone that "specs violation" issue. There is still a long list of real problems that I need to hunt down in 9pfs, afterwards I can continue with theoretical ones if you want, but right now I simply can't, sorry. > > > > (3) Unfortunately not all virtio users in QEMU would currently > > > > > > > > work correctly with the new value of 32768. > > > > > > > > So let's turn this hard coded global value into a runtime > > > > variable as a first step in this commit, configurable for each > > > > virtio user by passing a corresponding value with virtio_init() > > > > call. > > > > > > virtio_add_queue() already has an int queue_size argument, why isn't > > > that enough to deal with the maximum queue size? There's probably a good > > > reason for it, but please include it in the commit description. > > > > [...] > > > > > Can you make this value per-vq instead of per-vdev since virtqueues can > > > have different queue sizes? > > > > > > The same applies to the rest of this patch. Anything using > > > vdev->queue_max_size should probably use vq->vring.num
[PULL 44/57] acpi: arm/virt: build_spcr: fix invalid cast
From: Igor Mammedov implicit cast to structure uint8_t member didn't raise error when assigning value from incorrect enum, but when using build_append_gas() (next patch) it will error out with (clang): implicit conversion from enumeration type 'AmlRegionSpace' to different enumeration type 'AmlAddressSpace' fix cast error by using correct AML_AS_SYSTEM_MEMORY enum Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-31-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/virt-acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 8c382915a9..7b8706b305 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -465,7 +465,7 @@ build_spcr(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) spcr->interface_type = 0x3;/* ARM PL011 UART */ -spcr->base_address.space_id = AML_SYSTEM_MEMORY; +spcr->base_address.space_id = AML_AS_SYSTEM_MEMORY; spcr->base_address.bit_width = 8; spcr->base_address.bit_offset = 0; spcr->base_address.access_width = 1; -- MST
[PULL 01/57] hw/virtio: Acquire RCU read lock in virtqueue_packed_drop_all()
From: Philippe Mathieu-Daudé vring_get_region_caches() must be called with the RCU read lock acquired. virtqueue_packed_drop_all() does not, and uses the 'caches' pointer. Fix that by using the RCU_READ_LOCK_GUARD() macro. Reported-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210906104318.1569967-3-phi...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefano Garzarella --- hw/virtio/virtio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 240759ff0b..dd0ab433b8 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1703,6 +1703,8 @@ static unsigned int virtqueue_packed_drop_all(VirtQueue *vq) VirtIODevice *vdev = vq->vdev; VRingPackedDesc desc; +RCU_READ_LOCK_GUARD(); + caches = vring_get_region_caches(vq); if (!caches) { return 0; -- MST
Re: In-tree docs vs. Wiki [Was: Re: [PATCH 0/3] rSTify SubmitAPatch, TrivialPatches, and SpellCheck wiki pages]
On 10/5/21 18:03, Kashyap Chamarthy wrote: > On Tue, Oct 05, 2021 at 04:37:50PM +0100, Daniel P. Berrangé wrote: >> On Tue, Oct 05, 2021 at 05:07:06PM +0200, Philippe Mathieu-Daudé wrote: > > [...] > >>> One point Peter raised on IRC is it is easier to update a Wiki page >>> than get a patch merged into the repository. IOW we are making things >>> harder. >> >> There are factors to consider beyond ease of contributions. >> >> Certain information here is documenting a fundamental part of the >> QEMU community operation & processes. That ought to have a high >> trust level and be subject to review of content changes. I'd say >> the SubmitAPatch page falls in this category. >> >> Other information is essentially random adhoc user written content >> that isn't critical to the project. This can live with a low trust >> level and little-to-no review. >> >> IMHO, the wiki should only be considered for the latter type of >> content, with any important project content being subject to >> review. >> >> I also feel like docs in git is more likely to be kept upto date >> by the regular maintainers, than wikis which can become a bit of >> outdated mess. > > I agree. Here's an example that proves your point: had I written this > huge 'live-block-operations.rst'[1] doc as a Wiki, pretty sure it > would've been slowly rotting away. Now I see 5 other contributors > besides me (including Peter, yourself, and Paolo in this thread) that > have patched it ... by virtue of it being in-tree. > > That makes me even more convinced that having development, interface, > and any valuable docs that are in-tree are more well-maintained. This example is very convincing :) > (FWIW, I seem to have more motivation to write docs in rST or similar > formats that can be iterated over, with in-line reviews like regular > patches. I can't claim the same level of motivation to write Wiki pages > somehow.) > >> It is a shame that our normal contribution workflow doesn't make >> it easy for simple docs changes to be accepted and merged :-( > > Yeah; improving that can be nicer. > > [1] https://qemu.readthedocs.io/en/latest/interop/live-block-operations.html >
Re: Deprecate the ppc405 boards in QEMU? (was: [PATCH v3 4/7] MAINTAINERS: Orphan obscure ppc platforms)
On Tue, Oct 05, 2021 at 06:15:35PM +0200, Philippe Mathieu-Daudé wrote: > On 10/5/21 10:49, Daniel P. Berrangé wrote: > > On Tue, Oct 05, 2021 at 06:44:23AM +0200, Christophe Leroy wrote: > > >> I will look at it, please allow me a few weeks though. > > > > Once something is deprecated, it remains in QEMU for a minimum of two > > release cycles, before being deleted. At any time in that deprecation > > period it can be returned to supported status, if someone provides a > > good enough justification to keep it. > > My understanding is once being in deprecated state for 2 releases, it > can be removed, but it doesn't have to be removed (assuming it is > functional and nobody complains). Am I incorrect? It should be removed after 2 releases. Things live longer because people forget or are busy with other things, but ultimately anything in the deprecated list is liable to be deleted at any point after the 2 release minimum is up. If we change our minds about deleting something, then it should be un-deprecated. > I am raising this because the nanoMIPS support is in deprecated state > since more than 2 releases, but it is still in-tree and I try to keep > it functional. However, since no toolchain reached mainstream GCC/LLVM > it is not easy to maintain. By keeping it in that state we give some > time to other communities to have their toolchain upstreamed / merged. If you're trying to keep it functional and aren't going to remove it, then it shouldn't be marked deprecated. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PULL 43/57] acpi: arm/virt: convert build_iort() to endian agnostic build_append_FOO() API
From: Igor Mammedov Drop usage of packed structures and explicit endian conversions when building IORT table use endian agnostic build_append_int_noprefix() API to build it. Signed-off-by: Igor Mammedov Message-Id: <20210924122802.1455362-30-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Eric Auger Tested-by: Eric Auger --- include/hw/acpi/acpi-defs.h | 71 hw/arm/virt-acpi-build.c| 156 2 files changed, 89 insertions(+), 138 deletions(-) diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 195f90caf6..6f2f08a9de 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -188,75 +188,4 @@ struct AcpiGenericTimerTable { } QEMU_PACKED; typedef struct AcpiGenericTimerTable AcpiGenericTimerTable; -/* - * IORT node types - */ - -#define ACPI_IORT_NODE_HEADER_DEF /* Node format common fields */ \ -uint8_t type; \ -uint16_t length;\ -uint8_t revision; \ -uint32_t reserved; \ -uint32_t mapping_count; \ -uint32_t mapping_offset; - -/* Values for node Type above */ -enum { -ACPI_IORT_NODE_ITS_GROUP = 0x00, -ACPI_IORT_NODE_NAMED_COMPONENT = 0x01, -ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02, -ACPI_IORT_NODE_SMMU = 0x03, -ACPI_IORT_NODE_SMMU_V3 = 0x04 -}; - -struct AcpiIortIdMapping { -uint32_t input_base; -uint32_t id_count; -uint32_t output_base; -uint32_t output_reference; -uint32_t flags; -} QEMU_PACKED; -typedef struct AcpiIortIdMapping AcpiIortIdMapping; - -struct AcpiIortMemoryAccess { -uint32_t cache_coherency; -uint8_t hints; -uint16_t reserved; -uint8_t memory_flags; -} QEMU_PACKED; -typedef struct AcpiIortMemoryAccess AcpiIortMemoryAccess; - -struct AcpiIortItsGroup { -ACPI_IORT_NODE_HEADER_DEF -uint32_t its_count; -uint32_t identifiers[]; -} QEMU_PACKED; -typedef struct AcpiIortItsGroup AcpiIortItsGroup; - -#define ACPI_IORT_SMMU_V3_COHACC_OVERRIDE 1 - -struct AcpiIortSmmu3 { -ACPI_IORT_NODE_HEADER_DEF -uint64_t base_address; -uint32_t flags; -uint32_t reserved2; -uint64_t vatos_address; -uint32_t model; -uint32_t event_gsiv; -uint32_t pri_gsiv; -uint32_t gerr_gsiv; -uint32_t sync_gsiv; -AcpiIortIdMapping id_mapping_array[]; -} QEMU_PACKED; -typedef struct AcpiIortSmmu3 AcpiIortSmmu3; - -struct AcpiIortRC { -ACPI_IORT_NODE_HEADER_DEF -AcpiIortMemoryAccess memory_properties; -uint32_t ats_attribute; -uint32_t pci_segment_number; -AcpiIortIdMapping id_mapping_array[]; -} QEMU_PACKED; -typedef struct AcpiIortRC AcpiIortRC; - #endif diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 42ea460313..8c382915a9 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -240,6 +240,28 @@ static void acpi_dsdt_add_tpm(Aml *scope, VirtMachineState *vms) } #endif +#define ID_MAPPING_ENTRY_SIZE 20 +#define SMMU_V3_ENTRY_SIZE 60 +#define ROOT_COMPLEX_ENTRY_SIZE 32 +#define IORT_NODE_OFFSET 48 + +static void build_iort_id_mapping(GArray *table_data, uint32_t input_base, + uint32_t id_count, uint32_t out_ref) +{ +/* Identity RID mapping covering the whole input RID range */ +build_append_int_noprefix(table_data, input_base, 4); /* Input base */ +build_append_int_noprefix(table_data, id_count, 4); /* Number of IDs */ +build_append_int_noprefix(table_data, input_base, 4); /* Output base */ +build_append_int_noprefix(table_data, out_ref, 4); /* Output Reference */ +build_append_int_noprefix(table_data, 0, 4); /* Flags */ +} + +struct AcpiIortIdMapping { +uint32_t input_base; +uint32_t id_count; +}; +typedef struct AcpiIortIdMapping AcpiIortIdMapping; + /* Build the iort ID mapping to SMMUv3 for a given PCI host bridge */ static int iort_host_bridges(Object *obj, void *opaque) @@ -282,17 +304,16 @@ static void build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { int i, nb_nodes, rc_mapping_count; -AcpiIortIdMapping *idmap; -AcpiIortItsGroup *its; -AcpiIortSmmu3 *smmu; -AcpiIortRC *rc; -const uint32_t iort_node_offset = 48; +const uint32_t iort_node_offset = IORT_NODE_OFFSET; size_t node_size, smmu_offset = 0; +AcpiIortIdMapping *idmap; GArray *smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping)); GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping)); AcpiTable table = { .sig = "IORT", .rev = 0, .oem_id = vms->oem_id, .oem_table_id = vms->oem_table_id }; +/* Table 2 The IORT */ +acpi_table_begin(, table_data); if (vms->iommu == VIRT_IOMMU_SMMUV3) { AcpiIortIdMapping next_range = {0}; @@ -330,100 +351,101 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) nb_nodes
[PULL 52/57] bios-tables-test: Update ACPI DSDT table golden blobs for q35
From: Ani Sinha We have modified the IO address range for ACPI pci hotplug in q35. See change: 5adcc9e39e6a5 ("hw/i386/acpi: fix conflicting IO address range for acpi pci hotplug in q35") The ACPI DSDT table golden blobs must be regenrated in order to make the unit tests pass. This change updates the golden ACPI DSDT table blobs. Following is the ASL diff between the blobs: @@ -1,30 +1,30 @@ /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20190509 (64-bit version) * Copyright (c) 2000 - 2019 Intel Corporation * * Disassembling to symbolic ASL+ operators * - * Disassembly of tests/data/acpi/q35/DSDT, Tue Sep 14 09:04:06 2021 + * Disassembly of /tmp/aml-52DP90, Tue Sep 14 09:04:06 2021 * * Original Table Header: * Signature"DSDT" * Length 0x2061 (8289) * Revision 0x01 32-bit table (V1), no 64-bit math support - * Checksum 0xE5 + * Checksum 0xF9 * OEM ID "BOCHS " * OEM Table ID "BXPC" * OEM Revision 0x0001 (1) * Compiler ID "BXPC" * Compiler Version 0x0001 (1) */ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPC", 0x0001) { Scope (\) { OperationRegion (DBG, SystemIO, 0x0402, One) Field (DBG, ByteAcc, NoLock, Preserve) { DBGB, 8 } @@ -226,46 +226,46 @@ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { IO (Decode16, 0x0070, // Range Minimum 0x0070, // Range Maximum 0x01, // Alignment 0x08, // Length ) IRQNoFlags () {8} }) } } Scope (_SB.PCI0) { -OperationRegion (PCST, SystemIO, 0x0CC4, 0x08) +OperationRegion (PCST, SystemIO, 0x0CC0, 0x08) Field (PCST, DWordAcc, NoLock, WriteAsZeros) { PCIU, 32, PCID, 32 } -OperationRegion (SEJ, SystemIO, 0x0CCC, 0x04) +OperationRegion (SEJ, SystemIO, 0x0CC8, 0x04) Field (SEJ, DWordAcc, NoLock, WriteAsZeros) { B0EJ, 32 } -OperationRegion (BNMR, SystemIO, 0x0CD4, 0x08) +OperationRegion (BNMR, SystemIO, 0x0CD0, 0x08) Field (BNMR, DWordAcc, NoLock, WriteAsZeros) { BNUM, 32, PIDX, 32 } Mutex (BLCK, 0x00) Method (PCEJ, 2, NotSerialized) { Acquire (BLCK, 0x) BNUM = Arg0 B0EJ = (One << Arg1) Release (BLCK) Return (Zero) } @@ -3185,34 +3185,34 @@ 0x0620, // Range Minimum 0x0620, // Range Maximum 0x01, // Alignment 0x10, // Length ) }) } Device (PHPR) { Name (_HID, "PNP0A06" /* Generic Container Device */) // _HID: Hardware ID Name (_UID, "PCI Hotplug resources") // _UID: Unique ID Name (_STA, 0x0B) // _STA: Status Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { IO (Decode16, -0x0CC4, // Range Minimum -0x0CC4, // Range Maximum +0x0CC0, // Range Minimum +0x0CC0, // Range Maximum 0x01, // Alignment 0x18, // Length ) }) } } Scope (\) { Name (_S3, Package (0x04) // _S3_: S3 System State { One, One, Zero, Zero }) Signed-off-by: Ani Sinha Acked-by: Igor Mammedov Message-Id: <20210916132838.3469580-4-...@anisinha.ca> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test-allowed-diff.h | 12 tests/data/acpi/q35/DSDT| Bin 8289 -> 8289 bytes tests/data/acpi/q35/DSDT.acpihmat | Bin 9614 -> 9614 bytes tests/data/acpi/q35/DSDT.bridge | Bin 11003 -> 11003 bytes tests/data/acpi/q35/DSDT.cphp | Bin 8753 -> 8753 bytes tests/data/acpi/q35/DSDT.dimmpxm| Bin 9943 -> 9943 bytes tests/data/acpi/q35/DSDT.ipmibt | Bin 8364 -> 8364 bytes tests/data/acpi/q35/DSDT.memhp | Bin 9648 -> 9648 bytes tests/data/acpi/q35/DSDT.mmio64 | Bin 9419 -> 9419 bytes tests/data/acpi/q35/DSDT.nohpet | Bin 8147 -> 8147 bytes tests/data/acpi/q35/DSDT.numamem
[PATCH 2/2] block/aio_task: assert `max_busy_tasks` is greater than 0
All code in block/aio_task.c expects `max_busy_tasks` to always be greater than 0. Assert this condition during the AioTaskPool creation where `max_busy_tasks` is set. Signed-off-by: Stefano Garzarella --- block/aio_task.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/aio_task.c b/block/aio_task.c index 88989fa248..9bd17ea2c1 100644 --- a/block/aio_task.c +++ b/block/aio_task.c @@ -98,6 +98,8 @@ AioTaskPool *coroutine_fn aio_task_pool_new(int max_busy_tasks) { AioTaskPool *pool = g_new0(AioTaskPool, 1); +assert(max_busy_tasks > 0); + pool->main_co = qemu_coroutine_self(); pool->max_busy_tasks = max_busy_tasks; -- 2.31.1
[PULL 54/57] nvdimm: release the correct device list
From: Li Zhijian Signed-off-by: Li Zhijian Message-Id: <20210624110415.187164-1-lizhij...@cn.fujitsu.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- hw/acpi/nvdimm.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 5f9b552d6a..0d43da19ea 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -339,10 +339,10 @@ nvdimm_build_structure_caps(GArray *structures, uint32_t capabilities) static GArray *nvdimm_build_device_structure(NVDIMMState *state) { -GSList *device_list = nvdimm_get_device_list(); +GSList *device_list, *list = nvdimm_get_device_list(); GArray *structures = g_array_new(false, true /* clear */, 1); -for (; device_list; device_list = device_list->next) { +for (device_list = list; device_list; device_list = device_list->next) { DeviceState *dev = device_list->data; /* build System Physical Address Range Structure. */ @@ -357,7 +357,7 @@ static GArray *nvdimm_build_device_structure(NVDIMMState *state) /* build NVDIMM Control Region Structure. */ nvdimm_build_structure_dcr(structures, dev); } -g_slist_free(device_list); +g_slist_free(list); if (state->persistence) { nvdimm_build_structure_caps(structures, state->persistence); @@ -1333,9 +1333,9 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data, void nvdimm_build_srat(GArray *table_data) { -GSList *device_list = nvdimm_get_device_list(); +GSList *device_list, *list = nvdimm_get_device_list(); -for (; device_list; device_list = device_list->next) { +for (device_list = list; device_list; device_list = device_list->next) { DeviceState *dev = device_list->data; Object *obj = OBJECT(dev); uint64_t addr, size; @@ -1348,7 +1348,7 @@ void nvdimm_build_srat(GArray *table_data) build_srat_memory(table_data, addr, size, node, MEM_AFFINITY_ENABLED | MEM_AFFINITY_NON_VOLATILE); } -g_slist_free(device_list); +g_slist_free(list); } void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, -- MST
[PULL 39/57] acpi: arm/virt: madt: use build_append_int_noprefix() API to compose MADT table
From: Igor Mammedov Drop usage of packed structures and explicit endian conversions when building MADT table for arm/x86 and use endian agnostic build_append_int_noprefix() API to build it. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-26-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/acpi-defs.h | 84 hw/arm/virt-acpi-build.c| 148 ++-- 2 files changed, 89 insertions(+), 143 deletions(-) diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 3f174ba208..bcada37601 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -163,90 +163,6 @@ struct AcpiFacsDescriptorRev1 { } QEMU_PACKED; typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; -/* Values for Type in APIC sub-headers */ - -#define ACPI_APIC_GENERIC_CPU_INTERFACE 11 -#define ACPI_APIC_GENERIC_DISTRIBUTOR 12 -#define ACPI_APIC_GENERIC_MSI_FRAME 13 -#define ACPI_APIC_GENERIC_REDISTRIBUTOR 14 -#define ACPI_APIC_GENERIC_TRANSLATOR15 -#define ACPI_APIC_RESERVED 16 /* 16 and greater are reserved */ - -/* - * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE) - */ -#define ACPI_SUB_HEADER_DEF /* Common ACPI sub-structure header */\ -uint8_t type; \ -uint8_t length; - -/* Sub-structures for MADT */ - -struct AcpiMadtGenericCpuInterface { -ACPI_SUB_HEADER_DEF -uint16_t reserved; -uint32_t cpu_interface_number; -uint32_t uid; -uint32_t flags; -uint32_t parking_version; -uint32_t performance_interrupt; -uint64_t parked_address; -uint64_t base_address; -uint64_t gicv_base_address; -uint64_t gich_base_address; -uint32_t vgic_interrupt; -uint64_t gicr_base_address; -uint64_t arm_mpidr; -} QEMU_PACKED; - -typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface; - -/* GICC CPU Interface Flags */ -#define ACPI_MADT_GICC_ENABLED 1 - -struct AcpiMadtGenericDistributor { -ACPI_SUB_HEADER_DEF -uint16_t reserved; -uint32_t gic_id; -uint64_t base_address; -uint32_t global_irq_base; -/* ACPI 5.1 Errata 1228 Present GIC version in MADT table */ -uint8_t version; -uint8_t reserved2[3]; -} QEMU_PACKED; - -typedef struct AcpiMadtGenericDistributor AcpiMadtGenericDistributor; - -struct AcpiMadtGenericMsiFrame { -ACPI_SUB_HEADER_DEF -uint16_t reserved; -uint32_t gic_msi_frame_id; -uint64_t base_address; -uint32_t flags; -uint16_t spi_count; -uint16_t spi_base; -} QEMU_PACKED; - -typedef struct AcpiMadtGenericMsiFrame AcpiMadtGenericMsiFrame; - -struct AcpiMadtGenericRedistributor { -ACPI_SUB_HEADER_DEF -uint16_t reserved; -uint64_t base_address; -uint32_t range_length; -} QEMU_PACKED; - -typedef struct AcpiMadtGenericRedistributor AcpiMadtGenericRedistributor; - -struct AcpiMadtGenericTranslator { -ACPI_SUB_HEADER_DEF -uint16_t reserved; -uint32_t translation_id; -uint64_t base_address; -uint32_t reserved2; -} QEMU_PACKED; - -typedef struct AcpiMadtGenericTranslator AcpiMadtGenericTranslator; - /* * Generic Timer Description Table (GTDT) */ diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index e3bdcd44e8..a9a78d904a 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -568,94 +568,124 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) } /* - * ACPI spec, Revision 5.0 + * ACPI spec, Revision 5.1 Errata A * 5.2.12 Multiple APIC Description Table (MADT) */ +static void build_append_gicr(GArray *table_data, uint64_t base, uint32_t size) +{ +build_append_int_noprefix(table_data, 0xE, 1); /* Type */ +build_append_int_noprefix(table_data, 16, 1); /* Length */ +build_append_int_noprefix(table_data, 0, 2);/* Reserved */ +/* Discovery Range Base Addres */ +build_append_int_noprefix(table_data, base, 8); +build_append_int_noprefix(table_data, size, 4); /* Discovery Range Length */ +} + static void build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { +int i; VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); const MemMapEntry *memmap = vms->memmap; -const int *irqmap = vms->irqmap; -AcpiMadtGenericDistributor *gicd; -AcpiMadtGenericMsiFrame *gic_msi; -int i; AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id, .oem_table_id = vms->oem_table_id }; acpi_table_begin(, table_data); /* Local Interrupt Controller Address */ build_append_int_noprefix(table_data, 0, 4); -build_append_int_noprefix(table_data, 0, 4); /* Flags */ +build_append_int_noprefix(table_data, 0, 4); /* Flags */ -gicd = acpi_data_push(table_data, sizeof *gicd); -gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR; -
[PULL 50/57] bios-tables-test: allow changes in DSDT ACPI tables for q35
From: Ani Sinha We are going to commit a change to fix IO address range allocated for acpi pci hotplug in q35. This affects DSDT tables. This change allows DSDT table modification so that unit tests are not broken. Signed-off-by: Ani Sinha Acked-by: Igor Mammedov Message-Id: <20210916132838.3469580-2-...@anisinha.ca> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/qtest/bios-tables-test-allowed-diff.h | 12 1 file changed, 12 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..c06da38af3 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,13 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/q35/DSDT", +"tests/data/acpi/q35/DSDT.tis.tpm12", +"tests/data/acpi/q35/DSDT.tis.tpm2", +"tests/data/acpi/q35/DSDT.bridge", +"tests/data/acpi/q35/DSDT.mmio64", +"tests/data/acpi/q35/DSDT.ipmibt", +"tests/data/acpi/q35/DSDT.cphp", +"tests/data/acpi/q35/DSDT.memhp", +"tests/data/acpi/q35/DSDT.numamem", +"tests/data/acpi/q35/DSDT.nohpet", +"tests/data/acpi/q35/DSDT.dimmpxm", +"tests/data/acpi/q35/DSDT.acpihmat", -- MST
[PULL 35/57] acpi: madt: arm/x86: use acpi_table_begin()/acpi_table_end() instead of build_header()
From: Igor Mammedov it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-22-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/acpi-defs.h | 9 - hw/arm/virt-acpi-build.c| 19 +++ hw/i386/acpi-common.c | 19 +++ 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index c4f0a202e8..c7fa5caa06 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -176,15 +176,6 @@ typedef struct AcpiFacsDescriptorRev1 AcpiFacsDescriptorRev1; #define ACPI_DUAL_PIC0 #define ACPI_MULTIPLE_APIC 1 -/* Master MADT */ - -struct AcpiMultipleApicTable { -ACPI_TABLE_HEADER_DEF /* ACPI common table header */ -uint32_t local_apic_address; /* Physical address of local APIC */ -uint32_t flags; -} QEMU_PACKED; -typedef struct AcpiMultipleApicTable AcpiMultipleApicTable; - /* Values for Type in APIC sub-headers */ #define ACPI_APIC_PROCESSOR 0 diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 6ba02cf281..e3bdcd44e8 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -567,19 +567,26 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) vms->oem_table_id); } -/* MADT */ +/* + * ACPI spec, Revision 5.0 + * 5.2.12 Multiple APIC Description Table (MADT) + */ static void build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) { VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); -int madt_start = table_data->len; const MemMapEntry *memmap = vms->memmap; const int *irqmap = vms->irqmap; AcpiMadtGenericDistributor *gicd; AcpiMadtGenericMsiFrame *gic_msi; int i; +AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id, +.oem_table_id = vms->oem_table_id }; -acpi_data_push(table_data, sizeof(AcpiMultipleApicTable)); +acpi_table_begin(, table_data); +/* Local Interrupt Controller Address */ +build_append_int_noprefix(table_data, 0, 4); +build_append_int_noprefix(table_data, 0, 4); /* Flags */ gicd = acpi_data_push(table_data, sizeof *gicd); gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR; @@ -650,11 +657,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS); gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE); } - -build_header(linker, table_data, - (void *)(table_data->data + madt_start), "APIC", - table_data->len - madt_start, 3, vms->oem_id, - vms->oem_table_id); +acpi_table_end(linker, ); } /* FADT */ diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c index 1f5947fcf9..a0cde1d874 100644 --- a/hw/i386/acpi-common.c +++ b/hw/i386/acpi-common.c @@ -71,24 +71,29 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, } } +/* + * ACPI spec, Revision 1.0b + * 5.2.8 Multiple APIC Description Table + */ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, X86MachineState *x86ms, AcpiDeviceIf *adev, const char *oem_id, const char *oem_table_id) { MachineClass *mc = MACHINE_GET_CLASS(x86ms); const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms)); -int madt_start = table_data->len; AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev); bool x2apic_mode = false; -AcpiMultipleApicTable *madt; AcpiMadtIoApic *io_apic; AcpiMadtIntsrcovr *intsrcovr; int i; +AcpiTable table = { .sig = "APIC", .rev = 1, .oem_id = oem_id, +.oem_table_id = oem_table_id }; -madt = acpi_data_push(table_data, sizeof *madt); -madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS); -madt->flags = cpu_to_le32(1); +acpi_table_begin(, table_data); +/* Local APIC Address */ +build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4); +build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ for (i = 0; i < apic_ids->len; i++) { adevc->madt_cpu(adev, i, apic_ids, table_data); @@ -156,8 +161,6 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, local_nmi->lint = 1; /* ACPI_LINT1 */ } -build_header(linker, table_data, - (void *)(table_data->data + madt_start), "APIC", - table_data->len - madt_start, 1, oem_id, oem_table_id); +acpi_table_end(linker, ); } -- MST
[PULL 57/57] hw/i386/amd_iommu: Add description/category to TYPE_AMD_IOMMU_PCI
From: Philippe Mathieu-Daudé TYPE_AMD_IOMMU_PCI is user-creatable but not well described. Implement its class_init() handler to add it to the 'Misc devices' category, and add a description. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210926175648.1649075-4-f4...@amsat.org> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/amd_iommu.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 9014690ba3..9242a0d3ed 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1621,10 +1621,19 @@ static const TypeInfo amdvi_sysbus = { .class_init = amdvi_sysbus_class_init }; +static void amdvi_pci_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); + +set_bit(DEVICE_CATEGORY_MISC, dc->categories); +dc->desc = "AMD IOMMU (AMD-Vi) DMA Remapping device"; +} + static const TypeInfo amdvi_pci = { .name = TYPE_AMD_IOMMU_PCI, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(AMDVIPCIState), +.class_init = amdvi_pci_class_init, .interfaces = (InterfaceInfo[]) { { INTERFACE_CONVENTIONAL_PCI_DEVICE }, { }, -- MST
In-tree docs vs. Wiki [Was: Re: [PATCH 0/3] rSTify SubmitAPatch, TrivialPatches, and SpellCheck wiki pages]
On Tue, Oct 05, 2021 at 04:37:50PM +0100, Daniel P. Berrangé wrote: > On Tue, Oct 05, 2021 at 05:07:06PM +0200, Philippe Mathieu-Daudé wrote: [...] > > One point Peter raised on IRC is it is easier to update a Wiki page > > than get a patch merged into the repository. IOW we are making things > > harder. > > There are factors to consider beyond ease of contributions. > > Certain information here is documenting a fundamental part of the > QEMU community operation & processes. That ought to have a high > trust level and be subject to review of content changes. I'd say > the SubmitAPatch page falls in this category. > > Other information is essentially random adhoc user written content > that isn't critical to the project. This can live with a low trust > level and little-to-no review. > > IMHO, the wiki should only be considered for the latter type of > content, with any important project content being subject to > review. > > I also feel like docs in git is more likely to be kept upto date > by the regular maintainers, than wikis which can become a bit of > outdated mess. I agree. Here's an example that proves your point: had I written this huge 'live-block-operations.rst'[1] doc as a Wiki, pretty sure it would've been slowly rotting away. Now I see 5 other contributors besides me (including Peter, yourself, and Paolo in this thread) that have patched it ... by virtue of it being in-tree. That makes me even more convinced that having development, interface, and any valuable docs that are in-tree are more well-maintained. (FWIW, I seem to have more motivation to write docs in rST or similar formats that can be iterated over, with in-line reviews like regular patches. I can't claim the same level of motivation to write Wiki pages somehow.) > It is a shame that our normal contribution workflow doesn't make > it easy for simple docs changes to be accepted and merged :-( Yeah; improving that can be nicer. [1] https://qemu.readthedocs.io/en/latest/interop/live-block-operations.html -- /kashyap
[PULL 22/57] acpi: build_mcfg: use acpi_table_begin()/acpi_table_end() instead of build_header()
From: Igor Mammedov it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. Signed-off-by: Igor Mammedov Reviewed-by: Eric Auger Message-Id: <20210924122802.1455362-9-imamm...@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/pci.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/hw/acpi/pci.c b/hw/acpi/pci.c index 75b1103ec4..20b70dcd81 100644 --- a/hw/acpi/pci.c +++ b/hw/acpi/pci.c @@ -28,19 +28,20 @@ #include "hw/acpi/pci.h" #include "hw/pci/pcie_host.h" +/* + * PCI Firmware Specification, Revision 3.0 + * 4.1.2 MCFG Table Description. + */ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info, const char *oem_id, const char *oem_table_id) { -int mcfg_start = table_data->len; +AcpiTable table = { .sig = "MCFG", .rev = 1, +.oem_id = oem_id, .oem_table_id = oem_table_id }; + +acpi_table_begin(, table_data); -/* - * PCI Firmware Specification, Revision 3.0 - * 4.1.2 MCFG Table Description. - */ -acpi_data_push(table_data, sizeof(AcpiTableHeader)); /* Reserved */ build_append_int_noprefix(table_data, 0, 8); - /* * Memory Mapped Enhanced Configuration Space Base Address Allocation * Structure @@ -56,6 +57,5 @@ void build_mcfg(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info, /* Reserved */ build_append_int_noprefix(table_data, 0, 4); -build_header(linker, table_data, (void *)(table_data->data + mcfg_start), - "MCFG", table_data->len - mcfg_start, 1, oem_id, oem_table_id); +acpi_table_end(linker, ); } -- MST
[PULL 51/57] hw/i386/acpi: fix conflicting IO address range for acpi pci hotplug in q35
From: Ani Sinha Change caf108bc58790 ("hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35") selects an IO address range for acpi based PCI hotplug for q35 arbitrarily. It starts at address 0x0cc4 and ends at 0x0cdb. At the time when the patch was written but the final version of the patch was not yet pushed upstream, this address range was free and did not conflict with any other IO address ranges. However, with the following change, this address range was no longer conflict free as in this change, the IO address range (value of ACPI_PCIHP_SIZE) was incremented by four bytes: b32bd763a1ca92 ("pci: introduce acpi-index property for PCI device") This can be seen from the output of QMP command 'info mtree' : 0600-0603 (prio 0, i/o): acpi-evt 0604-0605 (prio 0, i/o): acpi-cnt 0608-060b (prio 0, i/o): acpi-tmr 0620-062f (prio 0, i/o): acpi-gpe0 0630-0637 (prio 0, i/o): acpi-smi 0cc4-0cdb (prio 0, i/o): acpi-pci-hotplug 0cd8-0ce3 (prio 0, i/o): acpi-cpu-hotplug It shows that there is a region of conflict between IO regions of acpi pci hotplug and acpi cpu hotplug. Unfortunately, the change caf108bc58790 did not update the IO address range appropriately before it was pushed upstream to accommodate the increased length of the IO address space introduced in change b32bd763a1ca92. Due to this bug, windows guests complain 'This device cannot find enough free resources it can use' in the device manager panel for extended IO buses. This issue also breaks the correct functioning of pci hotplug as the following shows that the IO space for pci hotplug has been truncated: (qemu) info mtree -f FlatView #0 AS "I/O", root: io Root memory region: io 0cc4-0cd7 (prio 0, i/o): acpi-pci-hotplug 0cd8-0cf7 (prio 0, i/o): acpi-cpu-hotplug Therefore, in this fix, we adjust the IO address range for the acpi pci hotplug so that it does not conflict with cpu hotplug and there is no truncation of IO spaces. The starting IO address of PCI hotplug region has been decremented by four bytes in order to accommodate four byte increment in the IO address space introduced by change b32bd763a1ca92 ("pci: introduce acpi-index property for PCI device") After fixing, the following are the corrected IO ranges: 0600-0603 (prio 0, i/o): acpi-evt 0604-0605 (prio 0, i/o): acpi-cnt 0608-060b (prio 0, i/o): acpi-tmr 0620-062f (prio 0, i/o): acpi-gpe0 0630-0637 (prio 0, i/o): acpi-smi 0cc0-0cd7 (prio 0, i/o): acpi-pci-hotplug 0cd8-0ce3 (prio 0, i/o): acpi-cpu-hotplug This change has been tested using a Windows Server 2019 guest VM. Windows no longer complains after this change. Fixes: caf108bc58790 ("hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/561 Signed-off-by: Ani Sinha Reviewed-by: Igor Mammedov Reviewed-by: Julia Suvorova Message-Id: <20210916132838.3469580-3-...@anisinha.ca> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/acpi/ich9.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index a329ce43ab..f04f1791bd 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -29,7 +29,7 @@ #include "hw/acpi/acpi_dev_interface.h" #include "hw/acpi/tco.h" -#define ACPI_PCIHP_ADDR_ICH9 0x0cc4 +#define ACPI_PCIHP_ADDR_ICH9 0x0cc0 typedef struct ICH9LPCPMRegs { /* -- MST