From 2ebc21216f58f6fcbf16f7ec0bebe7f72ab3d8ca Mon Sep 17 00:00:00 2001 From: Hogan Wang Date: Mon, 27 Jul 2020 16:46:20 +0800 Subject: 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 --- include/hw/pci/pci_host.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') 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; -- cgit v1.2.3 From 0c9753ebda274b0e618d7b4032bb2d83d27483ed Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Mon, 27 Jul 2020 17:33:19 +0200 Subject: 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 --- include/hw/virtio/virtio.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') 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); -- cgit v1.2.3