From c22aba196341995f2e45c37eed36cc7be09e4bc3 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 27 Jul 2020 13:59:05 +0200 Subject: [PATCH 1/7] virtio-mem-pci: force virtio version 1 Trying to run simple virtio-mem-pci examples currently fails with qemu-system-x86_64: -device virtio-mem-pci,id=vm0,memdev=mem0,node=0, requested-size=300M: device is modern-only, use disable-legacy=on due to the added safety checks in 9b3a35ec8236 ("virtio: verify that legacy support is not accidentally on"). As noted by Conny, we have to force virtio version 1. While at it, use qdev_realize() to set the parent bus and realize - like most other virtio-*-pci implementations. Fixes: 0b9a2443a48b ("virtio-pci: Proxy for virtio-mem") Reviewed-by: Cornelia Huck Cc: Cornelia Huck Cc: "Michael S. Tsirkin" Signed-off-by: David Hildenbrand Message-Id: <20200727115905.129397-1-david@redhat.com> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-mem-pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c index d375280ee1..590cec041b 100644 --- a/hw/virtio/virtio-mem-pci.c +++ b/hw/virtio/virtio-mem-pci.c @@ -21,8 +21,8 @@ static void virtio_mem_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) VirtIOMEMPCI *mem_pci = VIRTIO_MEM_PCI(vpci_dev); DeviceState *vdev = DEVICE(&mem_pci->vdev); - qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); - object_property_set_bool(OBJECT(vdev), "realized", true, errp); + virtio_pci_force_virtio_1(vpci_dev); + qdev_realize(vdev, BUS(&vpci_dev->bus), errp); } static void virtio_mem_pci_set_addr(MemoryDeviceState *md, uint64_t addr, From 2ebc21216f58f6fcbf16f7ec0bebe7f72ab3d8ca Mon Sep 17 00:00:00 2001 From: Hogan Wang Date: Mon, 27 Jul 2020 16:46:20 +0800 Subject: [PATCH 2/7] hw/pci-host: save/restore pci host config register The pci host config register is used to save PCI address for read/write config data. If guest writes a value to config register, and then QEMU pauses the vcpu to migrate, after the migration, the guest will continue to write pci config data, and the write data will be ignored because of new qemu process losing the config register state. To trigger the bug: 1. guest is booting in seabios. 2. guest enables the SMRAM in seabios:piix4_apmc_smm_setup, and then expects to disable the SMRAM by pci_config_writeb. 3. after guest writes the pci host config register, QEMU pauses vcpu to finish migration. 4. guest write of config data(0x0A) fails to disable the SMRAM because the config register state is lost. 5. guest continues to boot and crashes in ipxe option ROM due to SMRAM in enabled state. Example Reproducer: step 1. Make modifications to seabios and qemu for increase reproduction efficiency, write 0xf0 to 0x402 port notify qemu to stop vcpu after 0x0cf8 port wrote i440 configure register. qemu stop vcpu when catch 0x402 port wrote 0xf0. seabios:/src/hw/pci.c @@ -52,6 +52,11 @@ void pci_config_writeb(u16 bdf, u32 addr, u8 val) writeb(mmconfig_addr(bdf, addr), val); } else { outl(ioconfig_cmd(bdf, addr), PORT_PCI_CMD); + if (bdf == 0 && addr == 0x72 && val == 0xa) { + dprintf(1, "stop vcpu\n"); + outb(0xf0, 0x402); // notify qemu to stop vcpu + dprintf(1, "resume vcpu\n"); + } outb(val, PORT_PCI_DATA + (addr & 3)); } } qemu:hw/char/debugcon.c @@ -60,6 +61,9 @@ static void debugcon_ioport_write(void *opaque, hwaddr addr, uint64_t val, printf(" [debugcon: write addr=0x%04" HWADDR_PRIx " val=0x%02" PRIx64 "]\n", addr, val); #endif + if (ch == 0xf0) { + vm_stop(RUN_STATE_PAUSED); + } /* XXX this blocks entire thread. Rewrite to use * qemu_chr_fe_write and background I/O callbacks */ qemu_chr_fe_write_all(&s->chr, &ch, 1); step 2. start vm1 by the following command line, and then vm stopped. $ qemu-system-x86_64 -machine pc-i440fx-5.0,accel=kvm\ -netdev tap,ifname=tap-test,id=hostnet0,vhost=on,downscript=no,script=no\ -device virtio-net-pci,netdev=hostnet0,id=net0,bus=pci.0,addr=0x13,bootindex=3\ -device cirrus-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2\ -chardev file,id=seabios,path=/var/log/test.seabios,append=on\ -device isa-debugcon,iobase=0x402,chardev=seabios\ -monitor stdio step 3. start vm2 to accept vm1 state. $ qemu-system-x86_64 -machine pc-i440fx-5.0,accel=kvm\ -netdev tap,ifname=tap-test1,id=hostnet0,vhost=on,downscript=no,script=no\ -device virtio-net-pci,netdev=hostnet0,id=net0,bus=pci.0,addr=0x13,bootindex=3\ -device cirrus-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2\ -chardev file,id=seabios,path=/var/log/test.seabios,append=on\ -device isa-debugcon,iobase=0x402,chardev=seabios\ -monitor stdio \ -incoming tcp:127.0.0.1:8000 step 4. execute the following qmp command in vm1 to migrate. (qemu) migrate tcp:127.0.0.1:8000 step 5. execute the following qmp command in vm2 to resume vcpu. (qemu) cont Before this patch, we get KVM "emulation failure" error on vm2. This patch fixes it. Cc: qemu-stable@nongnu.org Signed-off-by: Hogan Wang Message-Id: <20200727084621.3279-1-hogan.wang@huawei.com> Reported-by: "Dr. David Alan Gilbert" Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/core/machine.c | 1 + hw/i386/pc.c | 3 ++- hw/pci/pci_host.c | 33 +++++++++++++++++++++++++++++++++ include/hw/pci/pci_host.h | 1 + 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 2f881d6d75..8d1a90c6cf 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -29,6 +29,7 @@ #include "migration/vmstate.h" GlobalProperty hw_compat_5_0[] = { + { "pci-host-bridge", "x-config-reg-migration-enabled", "off" }, { "virtio-balloon-device", "page-poison", "false" }, { "vmport", "x-read-set-eax", "off" }, { "vmport", "x-signal-unsupported-cmd", "off" }, diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 3d419d5991..47c5ca3e34 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -97,7 +97,8 @@ #include "fw_cfg.h" #include "trace.h" -GlobalProperty pc_compat_5_0[] = {}; +GlobalProperty pc_compat_5_0[] = { +}; const size_t pc_compat_5_0_len = G_N_ELEMENTS(pc_compat_5_0); GlobalProperty pc_compat_4_2[] = { diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index ce7bcdb1d5..8ca5fadcbd 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -22,8 +22,10 @@ #include "hw/pci/pci.h" #include "hw/pci/pci_bridge.h" #include "hw/pci/pci_host.h" +#include "hw/qdev-properties.h" #include "qemu/module.h" #include "hw/pci/pci_bus.h" +#include "migration/vmstate.h" #include "trace.h" /* debug PCI */ @@ -200,12 +202,43 @@ const MemoryRegionOps pci_host_data_be_ops = { .endianness = DEVICE_BIG_ENDIAN, }; +static bool pci_host_needed(void *opaque) +{ + PCIHostState *s = opaque; + return s->mig_enabled; +} + +const VMStateDescription vmstate_pcihost = { + .name = "PCIHost", + .needed = pci_host_needed, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(config_reg, PCIHostState), + VMSTATE_END_OF_LIST() + } +}; + +static Property pci_host_properties_common[] = { + DEFINE_PROP_BOOL("x-config-reg-migration-enabled", PCIHostState, + mig_enabled, true), + DEFINE_PROP_END_OF_LIST(), +}; + +static void pci_host_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + device_class_set_props(dc, pci_host_properties_common); + dc->vmsd = &vmstate_pcihost; +} + static const TypeInfo pci_host_type_info = { .name = TYPE_PCI_HOST_BRIDGE, .parent = TYPE_SYS_BUS_DEVICE, .abstract = true, .class_size = sizeof(PCIHostBridgeClass), .instance_size = sizeof(PCIHostState), + .class_init = pci_host_class_init, }; static void pci_host_register_types(void) diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h index 9ce088bd13..6210a7e14d 100644 --- a/include/hw/pci/pci_host.h +++ b/include/hw/pci/pci_host.h @@ -45,6 +45,7 @@ struct PCIHostState { MemoryRegion data_mem; MemoryRegion mmcfg; uint32_t config_reg; + bool mig_enabled; PCIBus *bus; QLIST_ENTRY(PCIHostState) next; From 4fdecf0543b49b8e171510104f3117538b9d1fe9 Mon Sep 17 00:00:00 2001 From: Raphael Norwitz Date: Fri, 17 Jul 2020 04:21:30 +0000 Subject: [PATCH 3/7] Fix vhost-user buffer over-read on ram hot-unplug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS vhost-user protocol feature introduced a shadow-table, used by the backend to dynamically determine how a vdev's memory regions have changed since the last vhost_user_set_mem_table() call. On hot-remove, a memmove() operation is used to overwrite the removed shadow region descriptor(s). The size parameter of this memmove was off by 1 such that if a VM with a backend supporting the VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS filled it's shadow-table (by performing the maximum number of supported hot-add operatons) and attempted to remove the last region, Qemu would read an out of bounds value and potentially crash. This change fixes the memmove() bounds such that this erroneous read can never happen. Signed-off-by: Peter Turschmid Signed-off-by: Raphael Norwitz Message-Id: <1594799958-31356-1-git-send-email-raphael.norwitz@nutanix.com> Fixes: f1aeb14b0809 ("Transmit vhost-user memory regions individually") Reviewed-by: Marc-André Lureau Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 31231218dc..d7e2423762 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -672,7 +672,7 @@ static int send_remove_regions(struct vhost_dev *dev, memmove(&u->shadow_regions[shadow_reg_idx], &u->shadow_regions[shadow_reg_idx + 1], sizeof(struct vhost_memory_region) * - (u->num_shadow_regions - shadow_reg_idx)); + (u->num_shadow_regions - shadow_reg_idx - 1)); u->num_shadow_regions--; } From 8fe9805c73c277dc2feeaa83de73d6a58bf23f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 23 Jul 2020 19:19:35 +0200 Subject: [PATCH 4/7] libvhost-user: Report descriptor index on panic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to report the index of the descriptor, not its pointer. Fixes: 7b2e5c65f4 ("contrib: add libvhost-user") Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200723171935.18535-1-philmd@redhat.com> Reviewed-by: Marc-André Lureau Reviewed-by: Raphael Norwitz Reviewed-by: Stefan Hajnoczi Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- contrib/libvhost-user/libvhost-user.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index d315db1396..53f16bdf08 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -2074,7 +2074,7 @@ virtqueue_get_head(VuDev *dev, VuVirtq *vq, /* If their number is silly, that's a fatal mistake. */ if (*head >= vq->vring.num) { - vu_panic(dev, "Guest says index %u is available", head); + vu_panic(dev, "Guest says index %u is available", *head); return false; } @@ -2133,7 +2133,7 @@ virtqueue_read_next_desc(VuDev *dev, struct vring_desc *desc, smp_wmb(); if (*next >= max) { - vu_panic(dev, "Desc next is %u", next); + vu_panic(dev, "Desc next is %u", *next); return VIRTQUEUE_READ_DESC_ERROR; } From 386494f29e2b902bc501176573bb4c69eab666c4 Mon Sep 17 00:00:00 2001 From: Cindy Lu Date: Fri, 10 Jul 2020 14:46:42 +0800 Subject: [PATCH 5/7] vhost-vdpa :Fix Coverity CID 1430270 / CID 1420267 In the function vhost_vdpa_dma_map/unmap, The struct msg was not initialized all its fields. Signed-off-by: Cindy Lu Message-Id: <20200710064642.24505-1-lulu@redhat.com> Reviewed-by: Peter Maydell Acked-by: Jason Wang Reviewed-by: Li Qiang Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-vdpa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 65d5aaf08a..4580f3efd8 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -37,7 +37,7 @@ static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section) static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, void *vaddr, bool readonly) { - struct vhost_msg_v2 msg; + struct vhost_msg_v2 msg = {}; int fd = v->device_fd; int ret = 0; @@ -60,7 +60,7 @@ static int vhost_vdpa_dma_map(struct vhost_vdpa *v, hwaddr iova, hwaddr size, static int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, hwaddr iova, hwaddr size) { - struct vhost_msg_v2 msg; + struct vhost_msg_v2 msg = {}; int fd = v->device_fd; int ret = 0; From d0d89526f70ded5ac41a4c6bb071c0d919b772db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 3 Jul 2020 20:34:50 +0200 Subject: [PATCH 6/7] MAINTAINERS: Cover the firmware JSON schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an entry to cover firmware.json (see commit 3a0adfc9bf: schema that describes the different uses and properties of virtual machine firmware). Cc: Laszlo Ersek Cc: Gerd Hoffmann Cc: Michael S. Tsirkin Cc: Kashyap Chamarthy Cc: Daniel P. Berrange Suggested-by: Laszlo Ersek Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200703183450.32398-1-philmd@redhat.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Laszlo Ersek Reviewed-by: Kashyap Chamarthy Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- MAINTAINERS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3395abd4e1..0886eb3d2b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2667,6 +2667,14 @@ F: include/hw/i2c/smbus_master.h F: include/hw/i2c/smbus_slave.h F: include/hw/i2c/smbus_eeprom.h +Firmware schema specifications +M: Laszlo Ersek +M: Philippe Mathieu-Daudé +R: Daniel P. Berrange +R: Kashyap Chamarthy +S: Maintained +F: docs/interop/firmware.json + EDK2 Firmware M: Laszlo Ersek M: Philippe Mathieu-Daudé From 0c9753ebda274b0e618d7b4032bb2d83d27483ed Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Mon, 27 Jul 2020 17:33:19 +0200 Subject: [PATCH 7/7] virtio-pci: fix virtio_pci_queue_enabled() In legacy mode, virtio_pci_queue_enabled() falls back to virtio_queue_enabled() to know if the queue is enabled. But virtio_queue_enabled() calls again virtio_pci_queue_enabled() if k->queue_enabled is set. This ends in a crash after a stack overflow. The problem can be reproduced with "-device virtio-net-pci,disable-legacy=off,disable-modern=true -net tap,vhost=on" And a look to the backtrace is very explicit: ... #4 0x000000010029a438 in virtio_queue_enabled () #5 0x0000000100497a9c in virtio_pci_queue_enabled () ... #130902 0x000000010029a460 in virtio_queue_enabled () #130903 0x0000000100497a9c in virtio_pci_queue_enabled () #130904 0x000000010029a460 in virtio_queue_enabled () #130905 0x0000000100454a20 in vhost_net_start () ... This patch fixes the problem by introducing a new function for the legacy case and calls it from virtio_pci_queue_enabled(). It also calls it from virtio_queue_enabled() to avoid code duplication. Fixes: f19bcdfedd53 ("virtio-pci: implement queue_enabled method") Cc: Jason Wang Cc: Cindy Lu CC: Michael S. Tsirkin Signed-off-by: Laurent Vivier Message-Id: <20200727153319.43716-1-lvivier@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.c | 2 +- hw/virtio/virtio.c | 7 ++++++- include/hw/virtio/virtio.h | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ada1101d07..4ad3ad81a2 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1116,7 +1116,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int n) return proxy->vqs[vdev->queue_sel].enabled; } - return virtio_queue_enabled(vdev, n); + return virtio_queue_enabled_legacy(vdev, n); } static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy, diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 546a198e79..e983025217 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -3309,6 +3309,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n) return vdev->vq[n].vring.desc; } +bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n) +{ + return virtio_queue_get_desc_addr(vdev, n) != 0; +} + bool virtio_queue_enabled(VirtIODevice *vdev, int n) { BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); @@ -3317,7 +3322,7 @@ bool virtio_queue_enabled(VirtIODevice *vdev, int n) if (k->queue_enabled) { return k->queue_enabled(qbus->parent, n); } - return virtio_queue_get_desc_addr(vdev, n) != 0; + return virtio_queue_enabled_legacy(vdev, n); } hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 198ffc7626..e424df12cf 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -295,6 +295,7 @@ typedef struct VirtIORNGConf VirtIORNGConf; VIRTIO_F_RING_PACKED, false) hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); +bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n); bool virtio_queue_enabled(VirtIODevice *vdev, int n); hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n); hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);