[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[qemu-xen staging] hw/pci-host: save/restore pci host config register



commit 2ebc21216f58f6fcbf16f7ec0bebe7f72ab3d8ca
Author:     Hogan Wang <king.wang@xxxxxxxxxx>
AuthorDate: Mon Jul 27 16:46:20 2020 +0800
Commit:     Michael S. Tsirkin <mst@xxxxxxxxxx>
CommitDate: Mon Jul 27 10:24:39 2020 -0400

    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@xxxxxxxxxx
    Signed-off-by: Hogan Wang <hogan.wang@xxxxxxxxxx>
    Message-Id: <20200727084621.3279-1-hogan.wang@xxxxxxxxxx>
    Reported-by: "Dr. David Alan Gilbert" <dgilbert@xxxxxxxxxx>
    Reviewed-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
    Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
---
 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;
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#staging



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.