All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] hw/pci-host: save/restore pci host config register
@ 2020-07-27  8:46 Hogan Wang
  2020-07-27  8:46 ` [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones Hogan Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Hogan Wang @ 2020-07-27  8:46 UTC (permalink / raw)
  To: marcel.apfelbaum, dgilbert, jusual, mst, qemu-devel
  Cc: wangxinxin.wang, weidong.huang, hogan.wang, Hogan Wang

From: Hogan Wang <king.wang@huawei.com>

The pci host config register is used to save PCI address for
read/write config data. If guest write a value to config register,
and then pause the vcpu to migrate, After the migration, the guest
continue to write pci config data, and the write data will be ignored
because of new qemu process lost the config register state.

Reproduction steps are:
1. guest booting in seabios.
2. guest enable the SMRAM in seabios:piix4_apmc_smm_setup, and then
   expect to disable the SMRAM by pci_config_writeb.
3. after guest write the pci host config register, and then pasued vcpu
   to finish migration.
4. guest write config data(0x0A) fail to disable the SMRAM becasue of
   config register state lost.
5. guest continue to boot and crash in ipxe option ROM due to SMRAM in
   enabled state.

Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
---
 hw/i386/pc.c              |  4 +++-
 hw/pci/pci_host.c         | 33 +++++++++++++++++++++++++++++++++
 include/hw/pci/pci_host.h |  1 +
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3d419d5991..f6ff0c5514 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -97,7 +97,9 @@
 #include "fw_cfg.h"
 #include "trace.h"
 
-GlobalProperty pc_compat_5_0[] = {};
+GlobalProperty pc_compat_5_0[] = {
+    { "pci-host-bridge", "x-config-reg-migration-enabled", "off" },
+};
 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;
-- 
2.27.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27  8:46 [PATCH v4 1/2] hw/pci-host: save/restore pci host config register Hogan Wang
@ 2020-07-27  8:46 ` Hogan Wang
  2020-07-27 13:08   ` Michael S. Tsirkin
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Hogan Wang @ 2020-07-27  8:46 UTC (permalink / raw)
  To: marcel.apfelbaum, dgilbert, jusual, mst, qemu-devel
  Cc: wangxinxin.wang, weidong.huang, hogan.wang

The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
Refer to i440FX and ICH9-LPC spcifications, there are some reserved
configuration registers can used to save/restore PCIHostState.config_reg.
It's nasty but friendly to old ones.

Reproducer steps:
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 can get KVM "emulation failure" error on vm2.
This patch can fix it.

Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
---
 hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
 hw/pci-host/q35.c    | 30 ++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 8ed2417f0c..b78c8bc5f9 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -64,6 +64,14 @@ typedef struct I440FXState {
  */
 #define I440FX_COREBOOT_RAM_SIZE 0x57
 
+/* Older I440FX machines (5.0 and older) not support i440FX-pcihost state
+ * migration, use some reserved INTEL 82441 configuration registers to
+ * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
+ * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
+ * (DBX) Table 1. PMC Configuration Space]
+ */
+#define I440FX_PCI_HOST_CONFIG_REG 0x94
+
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
@@ -98,8 +106,30 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_post_load(void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;
+    PCIDevice *dev;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
 
     i440fx_update_memory_mappings(d);
+
+    if (!s->mig_enabled) {
+        dev = PCI_DEVICE(d);
+        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
+    }
+    return 0;
+}
+
+static int i440fx_pre_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->mig_enabled) {
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
+                     s->config_reg);
+    }
     return 0;
 }
 
@@ -107,6 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
     .name = "I440FX",
     .version_id = 3,
     .minimum_version_id = 3,
+    .pre_save = i440fx_pre_save,
     .post_load = i440fx_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b67cb9c29f..bed66be181 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -43,6 +43,15 @@
 
 #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
 
+/* Older Q35 machines (5.0 and older) not support q35-pcihost state
+ * migration, use some reserved INTEL MCH configuration registers to
+ * save/restore q35-pcihost config register. Refer to [Intel 3 Series
+ * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
+ * Map (D0:F0)]
+ */
+#define Q35_PCI_HOST_CONFIG_REG 0x70
+
+
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
@@ -513,7 +522,27 @@ static void mch_update(MCHPCIState *mch)
 static int mch_post_load(void *opaque, int version_id)
 {
     MCHPCIState *mch = opaque;
+    PCIDevice *dev;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
     mch_update(mch);
+    if (!s->mig_enabled) {
+        dev = PCI_DEVICE(mch);
+        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
+    }
+    return 0;
+}
+
+static int mch_pre_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->mig_enabled) {
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
+    }
     return 0;
 }
 
@@ -521,6 +550,7 @@ static const VMStateDescription vmstate_mch = {
     .name = "mch",
     .version_id = 1,
     .minimum_version_id = 1,
+    .pre_save = mch_pre_save,
     .post_load = mch_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
-- 
2.27.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27  8:46 ` [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones Hogan Wang
@ 2020-07-27 13:08   ` Michael S. Tsirkin
  2020-07-27 13:53     ` Dr. David Alan Gilbert
  2020-07-27 14:30   ` Michael S. Tsirkin
  2020-07-27 15:47   ` Michael S. Tsirkin
  2 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2020-07-27 13:08 UTC (permalink / raw)
  To: Hogan Wang; +Cc: weidong.huang, wangxinxin.wang, jusual, dgilbert, qemu-devel

On Mon, Jul 27, 2020 at 04:46:21PM +0800, Hogan Wang wrote:
> The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> configuration registers can used to save/restore PCIHostState.config_reg.
> It's nasty but friendly to old ones.
> 
> Reproducer steps:
> 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 can get KVM "emulation failure" error on vm2.
> This patch can fix it.
> 
> Signed-off-by: Hogan Wang <hogan.wang@huawei.com>

Question - did you test that this migrates successfully to
old QEMU too? Thanks!
And I guess this needs a CC stable?

> ---
>  hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
>  hw/pci-host/q35.c    | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 8ed2417f0c..b78c8bc5f9 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -64,6 +64,14 @@ typedef struct I440FXState {
>   */
>  #define I440FX_COREBOOT_RAM_SIZE 0x57
>  
> +/* Older I440FX machines (5.0 and older) not support i440FX-pcihost state
> + * migration, use some reserved INTEL 82441 configuration registers to
> + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> + * (DBX) Table 1. PMC Configuration Space]
> + */
> +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> +
>  static void i440fx_update_memory_mappings(PCII440FXState *d)
>  {
>      int i;
> @@ -98,8 +106,30 @@ static void i440fx_write_config(PCIDevice *dev,
>  static int i440fx_post_load(void *opaque, int version_id)
>  {
>      PCII440FXState *d = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>  
>      i440fx_update_memory_mappings(d);
> +
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(d);
> +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> +    }
> +    return 0;
> +}
> +
> +static int i440fx_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> +                     s->config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -107,6 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
>      .name = "I440FX",
>      .version_id = 3,
>      .minimum_version_id = 3,
> +    .pre_save = i440fx_pre_save,
>      .post_load = i440fx_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b67cb9c29f..bed66be181 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -43,6 +43,15 @@
>  
>  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
>  
> +/* Older Q35 machines (5.0 and older) not support q35-pcihost state
> + * migration, use some reserved INTEL MCH configuration registers to
> + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> + * Map (D0:F0)]
> + */
> +#define Q35_PCI_HOST_CONFIG_REG 0x70
> +
> +
>  static void q35_host_realize(DeviceState *dev, Error **errp)
>  {
>      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> @@ -513,7 +522,27 @@ static void mch_update(MCHPCIState *mch)
>  static int mch_post_load(void *opaque, int version_id)
>  {
>      MCHPCIState *mch = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>      mch_update(mch);
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(mch);
> +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> +    }
> +    return 0;
> +}
> +
> +static int mch_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -521,6 +550,7 @@ static const VMStateDescription vmstate_mch = {
>      .name = "mch",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .pre_save = mch_pre_save,
>      .post_load = mch_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> -- 
> 2.27.0
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27 13:08   ` Michael S. Tsirkin
@ 2020-07-27 13:53     ` Dr. David Alan Gilbert
  2020-07-27 14:08       ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2020-07-27 13:53 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: weidong.huang, wangxinxin.wang, jusual, qemu-devel, Hogan Wang

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Mon, Jul 27, 2020 at 04:46:21PM +0800, Hogan Wang wrote:
> > The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> > Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> > configuration registers can used to save/restore PCIHostState.config_reg.
> > It's nasty but friendly to old ones.
> > 
> > Reproducer steps:
> > 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 can get KVM "emulation failure" error on vm2.
> > This patch can fix it.
> > 
> > Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> 
> Question - did you test that this migrates successfully to
> old QEMU too? Thanks!

I'm not seeing how it will; and isn't pci_host used in lots of
architectures?

Dave

> And I guess this needs a CC stable?
> 
> > ---
> >  hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
> >  hw/pci-host/q35.c    | 30 ++++++++++++++++++++++++++++++
> >  2 files changed, 61 insertions(+)
> > 
> > diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> > index 8ed2417f0c..b78c8bc5f9 100644
> > --- a/hw/pci-host/i440fx.c
> > +++ b/hw/pci-host/i440fx.c
> > @@ -64,6 +64,14 @@ typedef struct I440FXState {
> >   */
> >  #define I440FX_COREBOOT_RAM_SIZE 0x57
> >  
> > +/* Older I440FX machines (5.0 and older) not support i440FX-pcihost state
> > + * migration, use some reserved INTEL 82441 configuration registers to
> > + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> > + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> > + * (DBX) Table 1. PMC Configuration Space]
> > + */
> > +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> > +
> >  static void i440fx_update_memory_mappings(PCII440FXState *d)
> >  {
> >      int i;
> > @@ -98,8 +106,30 @@ static void i440fx_write_config(PCIDevice *dev,
> >  static int i440fx_post_load(void *opaque, int version_id)
> >  {
> >      PCII440FXState *d = opaque;
> > +    PCIDevice *dev;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> >  
> >      i440fx_update_memory_mappings(d);
> > +
> > +    if (!s->mig_enabled) {
> > +        dev = PCI_DEVICE(d);
> > +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int i440fx_pre_save(void *opaque)
> > +{
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> > +                     s->config_reg);
> > +    }
> >      return 0;
> >  }
> >  
> > @@ -107,6 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
> >      .name = "I440FX",
> >      .version_id = 3,
> >      .minimum_version_id = 3,
> > +    .pre_save = i440fx_pre_save,
> >      .post_load = i440fx_post_load,
> >      .fields = (VMStateField[]) {
> >          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index b67cb9c29f..bed66be181 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -43,6 +43,15 @@
> >  
> >  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
> >  
> > +/* Older Q35 machines (5.0 and older) not support q35-pcihost state
> > + * migration, use some reserved INTEL MCH configuration registers to
> > + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> > + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> > + * Map (D0:F0)]
> > + */
> > +#define Q35_PCI_HOST_CONFIG_REG 0x70
> > +
> > +
> >  static void q35_host_realize(DeviceState *dev, Error **errp)
> >  {
> >      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> > @@ -513,7 +522,27 @@ static void mch_update(MCHPCIState *mch)
> >  static int mch_post_load(void *opaque, int version_id)
> >  {
> >      MCHPCIState *mch = opaque;
> > +    PCIDevice *dev;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> >      mch_update(mch);
> > +    if (!s->mig_enabled) {
> > +        dev = PCI_DEVICE(mch);
> > +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int mch_pre_save(void *opaque)
> > +{
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> > +    }
> >      return 0;
> >  }
> >  
> > @@ -521,6 +550,7 @@ static const VMStateDescription vmstate_mch = {
> >      .name = "mch",
> >      .version_id = 1,
> >      .minimum_version_id = 1,
> > +    .pre_save = mch_pre_save,
> >      .post_load = mch_post_load,
> >      .fields = (VMStateField[]) {
> >          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> > -- 
> > 2.27.0
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27 13:53     ` Dr. David Alan Gilbert
@ 2020-07-27 14:08       ` Michael S. Tsirkin
  0 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2020-07-27 14:08 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: weidong.huang, wangxinxin.wang, jusual, qemu-devel, Hogan Wang

On Mon, Jul 27, 2020 at 02:53:29PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Mon, Jul 27, 2020 at 04:46:21PM +0800, Hogan Wang wrote:
> > > The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> > > Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> > > configuration registers can used to save/restore PCIHostState.config_reg.
> > > It's nasty but friendly to old ones.
> > > 
> > > Reproducer steps:
> > > 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 can get KVM "emulation failure" error on vm2.
> > > This patch can fix it.
> > > 
> > > Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> > 
> > Question - did you test that this migrates successfully to
> > old QEMU too? Thanks!
> 
> I'm not seeing how it will;

Well the way this is supposed to work is, it's a reserved register.
So if you migrate to an old qemu it will ignore it, the bug is
not fixed but if you got lucky and did not run into the bug,
the guest will keep going.

Migrating back has a small chance to mess up the guest in a way that is
different from how it was messed up previously, because old qemu will
send a stale config value if you run into the race.
The scariest thing  is it can convert a buggy write into register 0
into a write into another register - potentially more dangerous.
OTOH not writing at all which is what write to register 0 does
it also unlikely to work as expected ...

I really think we need a way to add migrated data without breaking
compat with old machine types, and have old types just ignore it. If we
had such a way we could now use it and at least we would not be making
the problem bigger for anyway.


> and isn't pci_host used in lots of
> architectures?

It doesn't affect these others. They can add a work around
too if there's a migrated register.

> 
> Dave
> 
> > And I guess this needs a CC stable?
> > 
> > > ---
> > >  hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
> > >  hw/pci-host/q35.c    | 30 ++++++++++++++++++++++++++++++
> > >  2 files changed, 61 insertions(+)
> > > 
> > > diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> > > index 8ed2417f0c..b78c8bc5f9 100644
> > > --- a/hw/pci-host/i440fx.c
> > > +++ b/hw/pci-host/i440fx.c
> > > @@ -64,6 +64,14 @@ typedef struct I440FXState {
> > >   */
> > >  #define I440FX_COREBOOT_RAM_SIZE 0x57
> > >  
> > > +/* Older I440FX machines (5.0 and older) not support i440FX-pcihost state
> > > + * migration, use some reserved INTEL 82441 configuration registers to
> > > + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> > > + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> > > + * (DBX) Table 1. PMC Configuration Space]
> > > + */
> > > +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> > > +
> > >  static void i440fx_update_memory_mappings(PCII440FXState *d)
> > >  {
> > >      int i;
> > > @@ -98,8 +106,30 @@ static void i440fx_write_config(PCIDevice *dev,
> > >  static int i440fx_post_load(void *opaque, int version_id)
> > >  {
> > >      PCII440FXState *d = opaque;
> > > +    PCIDevice *dev;
> > > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > > +                                   object_resolve_path("/machine/i440fx", NULL),
> > > +                                   TYPE_PCI_HOST_BRIDGE);
> > >  
> > >      i440fx_update_memory_mappings(d);
> > > +
> > > +    if (!s->mig_enabled) {
> > > +        dev = PCI_DEVICE(d);
> > > +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> > > +    }
> > > +    return 0;
> > > +}
> > > +
> > > +static int i440fx_pre_save(void *opaque)
> > > +{
> > > +    PCIDevice *dev = opaque;
> > > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > > +                                   object_resolve_path("/machine/i440fx", NULL),
> > > +                                   TYPE_PCI_HOST_BRIDGE);
> > > +    if (!s->mig_enabled) {
> > > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> > > +                     s->config_reg);
> > > +    }
> > >      return 0;
> > >  }
> > >  
> > > @@ -107,6 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
> > >      .name = "I440FX",
> > >      .version_id = 3,
> > >      .minimum_version_id = 3,
> > > +    .pre_save = i440fx_pre_save,
> > >      .post_load = i440fx_post_load,
> > >      .fields = (VMStateField[]) {
> > >          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> > > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > > index b67cb9c29f..bed66be181 100644
> > > --- a/hw/pci-host/q35.c
> > > +++ b/hw/pci-host/q35.c
> > > @@ -43,6 +43,15 @@
> > >  
> > >  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
> > >  
> > > +/* Older Q35 machines (5.0 and older) not support q35-pcihost state
> > > + * migration, use some reserved INTEL MCH configuration registers to
> > > + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> > > + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> > > + * Map (D0:F0)]
> > > + */
> > > +#define Q35_PCI_HOST_CONFIG_REG 0x70
> > > +
> > > +
> > >  static void q35_host_realize(DeviceState *dev, Error **errp)
> > >  {
> > >      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> > > @@ -513,7 +522,27 @@ static void mch_update(MCHPCIState *mch)
> > >  static int mch_post_load(void *opaque, int version_id)
> > >  {
> > >      MCHPCIState *mch = opaque;
> > > +    PCIDevice *dev;
> > > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > > +                                   object_resolve_path("/machine/q35", NULL),
> > > +                                   TYPE_PCI_HOST_BRIDGE);
> > >      mch_update(mch);
> > > +    if (!s->mig_enabled) {
> > > +        dev = PCI_DEVICE(mch);
> > > +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> > > +    }
> > > +    return 0;
> > > +}
> > > +
> > > +static int mch_pre_save(void *opaque)
> > > +{
> > > +    PCIDevice *dev = opaque;
> > > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > > +                                   object_resolve_path("/machine/q35", NULL),
> > > +                                   TYPE_PCI_HOST_BRIDGE);
> > > +    if (!s->mig_enabled) {
> > > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> > > +    }
> > >      return 0;
> > >  }
> > >  
> > > @@ -521,6 +550,7 @@ static const VMStateDescription vmstate_mch = {
> > >      .name = "mch",
> > >      .version_id = 1,
> > >      .minimum_version_id = 1,
> > > +    .pre_save = mch_pre_save,
> > >      .post_load = mch_post_load,
> > >      .fields = (VMStateField[]) {
> > >          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> > > -- 
> > > 2.27.0
> > > 
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27  8:46 ` [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones Hogan Wang
  2020-07-27 13:08   ` Michael S. Tsirkin
@ 2020-07-27 14:30   ` Michael S. Tsirkin
  2020-07-27 15:47   ` Michael S. Tsirkin
  2 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2020-07-27 14:30 UTC (permalink / raw)
  To: Hogan Wang; +Cc: weidong.huang, wangxinxin.wang, jusual, dgilbert, qemu-devel

On Mon, Jul 27, 2020 at 04:46:21PM +0800, Hogan Wang wrote:
> The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> configuration registers can used to save/restore PCIHostState.config_reg.
> It's nasty but friendly to old ones.
> 
> Reproducer steps:
> 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 can get KVM "emulation failure" error on vm2.
> This patch can fix it.
> 
> Signed-off-by: Hogan Wang <hogan.wang@huawei.com>

OK I dropped this one for now, pls address comments I posted
if at all possible.
Patch 1 is in my tree though, I fixed up some minor issues and
tweaked the commit log.

Thanks a lot and great job noticing and fixing this!


> ---
>  hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
>  hw/pci-host/q35.c    | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 8ed2417f0c..b78c8bc5f9 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -64,6 +64,14 @@ typedef struct I440FXState {
>   */
>  #define I440FX_COREBOOT_RAM_SIZE 0x57
>  
> +/* Older I440FX machines (5.0 and older) not support i440FX-pcihost state
> + * migration, use some reserved INTEL 82441 configuration registers to
> + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> + * (DBX) Table 1. PMC Configuration Space]
> + */
> +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> +
>  static void i440fx_update_memory_mappings(PCII440FXState *d)
>  {
>      int i;
> @@ -98,8 +106,30 @@ static void i440fx_write_config(PCIDevice *dev,
>  static int i440fx_post_load(void *opaque, int version_id)
>  {
>      PCII440FXState *d = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>  
>      i440fx_update_memory_mappings(d);
> +
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(d);
> +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> +    }
> +    return 0;
> +}
> +
> +static int i440fx_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> +                     s->config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -107,6 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
>      .name = "I440FX",
>      .version_id = 3,
>      .minimum_version_id = 3,
> +    .pre_save = i440fx_pre_save,
>      .post_load = i440fx_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b67cb9c29f..bed66be181 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -43,6 +43,15 @@
>  
>  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
>  
> +/* Older Q35 machines (5.0 and older) not support q35-pcihost state
> + * migration, use some reserved INTEL MCH configuration registers to
> + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> + * Map (D0:F0)]
> + */
> +#define Q35_PCI_HOST_CONFIG_REG 0x70
> +
> +
>  static void q35_host_realize(DeviceState *dev, Error **errp)
>  {
>      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> @@ -513,7 +522,27 @@ static void mch_update(MCHPCIState *mch)
>  static int mch_post_load(void *opaque, int version_id)
>  {
>      MCHPCIState *mch = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>      mch_update(mch);
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(mch);
> +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> +    }
> +    return 0;
> +}
> +
> +static int mch_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -521,6 +550,7 @@ static const VMStateDescription vmstate_mch = {
>      .name = "mch",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .pre_save = mch_pre_save,
>      .post_load = mch_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> -- 
> 2.27.0
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27  8:46 ` [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones Hogan Wang
  2020-07-27 13:08   ` Michael S. Tsirkin
  2020-07-27 14:30   ` Michael S. Tsirkin
@ 2020-07-27 15:47   ` Michael S. Tsirkin
  2020-07-28  3:27     ` [PATCH v5] " Hogan Wang
  2 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2020-07-27 15:47 UTC (permalink / raw)
  To: Hogan Wang; +Cc: weidong.huang, wangxinxin.wang, jusual, dgilbert, qemu-devel

On Mon, Jul 27, 2020 at 04:46:21PM +0800, Hogan Wang wrote:
> The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> configuration registers can used to save/restore PCIHostState.config_reg.
> It's nasty but friendly to old ones.
> 
> Reproducer steps:
> 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

Great report, I copied it to the previous patch too.

> Before this patch, we can get KVM "emulation failure" error on vm2.

we can get -> we get

> This patch can fix it.

-> this patch fixes it

> 
> Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> ---
>  hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
>  hw/pci-host/q35.c    | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 8ed2417f0c..b78c8bc5f9 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -64,6 +64,14 @@ typedef struct I440FXState {
>   */
>  #define I440FX_COREBOOT_RAM_SIZE 0x57
>  
> +/* Older I440FX machines (5.0 and older) not support i440FX-pcihost state

not support -> do not support

> + * migration, use some reserved INTEL 82441 configuration registers to
> + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> + * (DBX) Table 1. PMC Configuration Space]
> + */
> +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> +
>  static void i440fx_update_memory_mappings(PCII440FXState *d)
>  {
>      int i;
> @@ -98,8 +106,30 @@ static void i440fx_write_config(PCIDevice *dev,
>  static int i440fx_post_load(void *opaque, int version_id)
>  {
>      PCII440FXState *d = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>  
>      i440fx_update_memory_mappings(d);
> +
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(d);
> +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);

So I guess at this point we want to reset the register to 0,
so it's not guest visible.


> +    }
> +    return 0;
> +}
> +
> +static int i440fx_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> +                     s->config_reg);
> +    }
>      return 0;
>  }


And I guess we want a post_save hook to clear this?

> @@ -107,6 +137,7 @@ static const VMStateDescription vmstate_i440fx = {
>      .name = "I440FX",
>      .version_id = 3,
>      .minimum_version_id = 3,
> +    .pre_save = i440fx_pre_save,
>      .post_load = i440fx_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b67cb9c29f..bed66be181 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -43,6 +43,15 @@
>  
>  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
>  
> +/* Older Q35 machines (5.0 and older) not support q35-pcihost state

not support -> do not support.

> + * migration, use some reserved INTEL MCH configuration registers to
> + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> + * Map (D0:F0)]
> + */
> +#define Q35_PCI_HOST_CONFIG_REG 0x70
> +
> +

extra empty line

>  static void q35_host_realize(DeviceState *dev, Error **errp)
>  {
>      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> @@ -513,7 +522,27 @@ static void mch_update(MCHPCIState *mch)
>  static int mch_post_load(void *opaque, int version_id)
>  {
>      MCHPCIState *mch = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>      mch_update(mch);
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(mch);
> +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> +    }
> +    return 0;
> +}
> +
> +static int mch_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -521,6 +550,7 @@ static const VMStateDescription vmstate_mch = {
>      .name = "mch",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .pre_save = mch_pre_save,
>      .post_load = mch_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),


Same comments.

> -- 
> 2.27.0
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5] hw/pci-host: save/restore pci host config register for old ones
  2020-07-27 15:47   ` Michael S. Tsirkin
@ 2020-07-28  3:27     ` Hogan Wang
  2020-07-29 14:04       ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Hogan Wang @ 2020-07-28  3:27 UTC (permalink / raw)
  To: marcel.apfelbaum, dgilbert, jusual, mst, qemu-devel
  Cc: wangxinxin.wang, weidong.huang, hogan.wang

The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
Refer to i440FX and ICH9-LPC spcifications, there are some reserved
configuration registers can used to save/restore PCIHostState.config_reg.
It's nasty but friendly to old ones.

Reproducer steps:
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.

Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
---
 hw/pci-host/i440fx.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
 hw/pci-host/q35.c    | 44 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 8ed2417f0c..419e27c21a 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -64,6 +64,14 @@ typedef struct I440FXState {
  */
 #define I440FX_COREBOOT_RAM_SIZE 0x57
 
+/* Older I440FX machines (5.0 and older) do not support i440FX-pcihost state
+ * migration, use some reserved INTEL 82441 configuration registers to
+ * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
+ * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
+ * (DBX) Table 1. PMC Configuration Space]
+ */
+#define I440FX_PCI_HOST_CONFIG_REG 0x94
+
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
@@ -98,15 +106,53 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_post_load(void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;
+    PCIDevice *dev;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
 
     i440fx_update_memory_mappings(d);
+
+    if (!s->mig_enabled) {
+        dev = PCI_DEVICE(d);
+        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
+    }
+    return 0;
+}
+
+static int i440fx_pre_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->mig_enabled) {
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
+                     s->config_reg);
+    }
+    return 0;
+}
+
+static int i440fx_post_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->mig_enabled) {
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
+    }
     return 0;
 }
 
+
 static const VMStateDescription vmstate_i440fx = {
     .name = "I440FX",
     .version_id = 3,
     .minimum_version_id = 3,
+    .pre_save = i440fx_pre_save,
+    .post_save = i440fx_post_save,
     .post_load = i440fx_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b67cb9c29f..d87f892945 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -43,6 +43,14 @@
 
 #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
 
+/* Older Q35 machines (5.0 and older) do not support q35-pcihost state
+ * migration, use some reserved INTEL MCH configuration registers to
+ * save/restore q35-pcihost config register. Refer to [Intel 3 Series
+ * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
+ * Map (D0:F0)]
+ */
+#define Q35_PCI_HOST_CONFIG_REG 0x70
+
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
@@ -513,14 +521,50 @@ static void mch_update(MCHPCIState *mch)
 static int mch_post_load(void *opaque, int version_id)
 {
     MCHPCIState *mch = opaque;
+    PCIDevice *dev;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
     mch_update(mch);
+    if (!s->mig_enabled) {
+        dev = PCI_DEVICE(mch);
+        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
+    }
+    return 0;
+}
+
+static int mch_pre_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->mig_enabled) {
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
+    }
     return 0;
 }
 
+static int mch_post_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->mig_enabled) {
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
+    }
+    return 0;
+}
+
+
 static const VMStateDescription vmstate_mch = {
     .name = "mch",
     .version_id = 1,
     .minimum_version_id = 1,
+    .pre_save = mch_pre_save,
+    .post_save = mch_post_save,
     .post_load = mch_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
-- 
2.27.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5] hw/pci-host: save/restore pci host config register for old ones
  2020-07-28  3:27     ` [PATCH v5] " Hogan Wang
@ 2020-07-29 14:04       ` Michael S. Tsirkin
  2020-07-29 18:42         ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2020-07-29 14:04 UTC (permalink / raw)
  To: Hogan Wang; +Cc: weidong.huang, wangxinxin.wang, jusual, dgilbert, qemu-devel

On Tue, Jul 28, 2020 at 11:27:09AM +0800, Hogan Wang wrote:
> The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> configuration registers can used to save/restore PCIHostState.config_reg.
> It's nasty but friendly to old ones.
> 
> Reproducer steps:
> 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.
> 
> Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> ---
>  hw/pci-host/i440fx.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/pci-host/q35.c    | 44 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 8ed2417f0c..419e27c21a 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -64,6 +64,14 @@ typedef struct I440FXState {
>   */
>  #define I440FX_COREBOOT_RAM_SIZE 0x57
>  
> +/* Older I440FX machines (5.0 and older) do not support i440FX-pcihost state
> + * migration, use some reserved INTEL 82441 configuration registers to
> + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> + * (DBX) Table 1. PMC Configuration Space]
> + */
> +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> +
>  static void i440fx_update_memory_mappings(PCII440FXState *d)
>  {
>      int i;
> @@ -98,15 +106,53 @@ static void i440fx_write_config(PCIDevice *dev,
>  static int i440fx_post_load(void *opaque, int version_id)
>  {
>      PCII440FXState *d = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>  
>      i440fx_update_memory_mappings(d);
> +
> +    if (!s->mig_enabled) {

Thinking more about it, I think we should rename mig_enabled to
config_reg_mig_enabled or something like this.


> +        dev = PCI_DEVICE(d);
> +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
> +    }
> +    return 0;
> +}
> +
> +static int i440fx_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> +                     s->config_reg);
> +    }
> +    return 0;
> +}
> +
> +static int i440fx_post_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/i440fx", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
> +    }
>      return 0;
>  }
>  
> +

Extra empty line.


>  static const VMStateDescription vmstate_i440fx = {
>      .name = "I440FX",
>      .version_id = 3,
>      .minimum_version_id = 3,
> +    .pre_save = i440fx_pre_save,
> +    .post_save = i440fx_post_save,
>      .post_load = i440fx_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b67cb9c29f..d87f892945 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -43,6 +43,14 @@
>  
>  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
>  
> +/* Older Q35 machines (5.0 and older) do not support q35-pcihost state
> + * migration, use some reserved INTEL MCH configuration registers to
> + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> + * Map (D0:F0)]
> + */
> +#define Q35_PCI_HOST_CONFIG_REG 0x70
> +
>  static void q35_host_realize(DeviceState *dev, Error **errp)
>  {
>      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> @@ -513,14 +521,50 @@ static void mch_update(MCHPCIState *mch)
>  static int mch_post_load(void *opaque, int version_id)
>  {
>      MCHPCIState *mch = opaque;
> +    PCIDevice *dev;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
>      mch_update(mch);
> +    if (!s->mig_enabled) {
> +        dev = PCI_DEVICE(mch);
> +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
> +    }
> +    return 0;
> +}
> +
> +static int mch_pre_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> +    }
>      return 0;
>  }
>  
> +static int mch_post_save(void *opaque)
> +{
> +    PCIDevice *dev = opaque;
> +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> +                                   object_resolve_path("/machine/q35", NULL),
> +                                   TYPE_PCI_HOST_BRIDGE);
> +    if (!s->mig_enabled) {
> +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
> +    }
> +    return 0;
> +}
> +
> +
>  static const VMStateDescription vmstate_mch = {
>      .name = "mch",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .pre_save = mch_pre_save,
> +    .post_save = mch_post_save,
>      .post_load = mch_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> -- 
> 2.27.0
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5] hw/pci-host: save/restore pci host config register for old ones
  2020-07-29 14:04       ` Michael S. Tsirkin
@ 2020-07-29 18:42         ` Dr. David Alan Gilbert
  2020-07-30  1:27           ` [PATCH 2/6] " Hogan Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2020-07-29 18:42 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: weidong.huang, wangxinxin.wang, jusual, qemu-devel, Hogan Wang

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Tue, Jul 28, 2020 at 11:27:09AM +0800, Hogan Wang wrote:
> > The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> > Refer to i440FX and ICH9-LPC spcifications, there are some reserved
> > configuration registers can used to save/restore PCIHostState.config_reg.
> > It's nasty but friendly to old ones.
> > 
> > Reproducer steps:
> > 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.
> > 
> > Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> > ---
> >  hw/pci-host/i440fx.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >  hw/pci-host/q35.c    | 44 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 90 insertions(+)
> > 
> > diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> > index 8ed2417f0c..419e27c21a 100644
> > --- a/hw/pci-host/i440fx.c
> > +++ b/hw/pci-host/i440fx.c
> > @@ -64,6 +64,14 @@ typedef struct I440FXState {
> >   */
> >  #define I440FX_COREBOOT_RAM_SIZE 0x57
> >  
> > +/* Older I440FX machines (5.0 and older) do not support i440FX-pcihost state
> > + * migration, use some reserved INTEL 82441 configuration registers to
> > + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
> > + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
> > + * (DBX) Table 1. PMC Configuration Space]
> > + */
> > +#define I440FX_PCI_HOST_CONFIG_REG 0x94
> > +
> >  static void i440fx_update_memory_mappings(PCII440FXState *d)
> >  {
> >      int i;
> > @@ -98,15 +106,53 @@ static void i440fx_write_config(PCIDevice *dev,
> >  static int i440fx_post_load(void *opaque, int version_id)
> >  {
> >      PCII440FXState *d = opaque;
> > +    PCIDevice *dev;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> >  
> >      i440fx_update_memory_mappings(d);
> > +
> > +    if (!s->mig_enabled) {
> 
> Thinking more about it, I think we should rename mig_enabled to
> config_reg_mig_enabled or something like this.

Agreed.

Dave

> 
> > +        dev = PCI_DEVICE(d);
> > +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int i440fx_pre_save(void *opaque)
> > +{
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> > +                     s->config_reg);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int i440fx_post_save(void *opaque)
> > +{
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
> > +    }
> >      return 0;
> >  }
> >  
> > +
> 
> Extra empty line.
> 
> 
> >  static const VMStateDescription vmstate_i440fx = {
> >      .name = "I440FX",
> >      .version_id = 3,
> >      .minimum_version_id = 3,
> > +    .pre_save = i440fx_pre_save,
> > +    .post_save = i440fx_post_save,
> >      .post_load = i440fx_post_load,
> >      .fields = (VMStateField[]) {
> >          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index b67cb9c29f..d87f892945 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -43,6 +43,14 @@
> >  
> >  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
> >  
> > +/* Older Q35 machines (5.0 and older) do not support q35-pcihost state
> > + * migration, use some reserved INTEL MCH configuration registers to
> > + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> > + * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
> > + * Map (D0:F0)]
> > + */
> > +#define Q35_PCI_HOST_CONFIG_REG 0x70
> > +
> >  static void q35_host_realize(DeviceState *dev, Error **errp)
> >  {
> >      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> > @@ -513,14 +521,50 @@ static void mch_update(MCHPCIState *mch)
> >  static int mch_post_load(void *opaque, int version_id)
> >  {
> >      MCHPCIState *mch = opaque;
> > +    PCIDevice *dev;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> >      mch_update(mch);
> > +    if (!s->mig_enabled) {
> > +        dev = PCI_DEVICE(mch);
> > +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int mch_pre_save(void *opaque)
> > +{
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> > +    }
> >      return 0;
> >  }
> >  
> > +static int mch_post_save(void *opaque)
> > +{
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +
> >  static const VMStateDescription vmstate_mch = {
> >      .name = "mch",
> >      .version_id = 1,
> >      .minimum_version_id = 1,
> > +    .pre_save = mch_pre_save,
> > +    .post_save = mch_post_save,
> >      .post_load = mch_post_load,
> >      .fields = (VMStateField[]) {
> >          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> > -- 
> > 2.27.0
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/6] hw/pci-host: save/restore pci host config register for old ones
  2020-07-29 18:42         ` Dr. David Alan Gilbert
@ 2020-07-30  1:27           ` Hogan Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Hogan Wang @ 2020-07-30  1:27 UTC (permalink / raw)
  To: dgilbert, qemu-devel, mst, marcel.apfelbaum, jusual
  Cc: wangxinxin.wang, weidong.huang, hogan.wang

The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
Refer to i440FX and ICH9-LPC spcifications, there are some reserved
configuration registers can used to save/restore PCIHostState.config_reg.
It's nasty but friendly to old ones.

Reproducer steps:
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.

Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
---
 hw/pci-host/i440fx.c      | 46 +++++++++++++++++++++++++++++++++++++++
 hw/pci-host/q35.c         | 44 +++++++++++++++++++++++++++++++++++++
 hw/pci/pci_host.c         |  4 ++--
 include/hw/pci/pci_host.h |  2 +-
 4 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 8ed2417f0c..707e7e9dfb 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -64,6 +64,14 @@ typedef struct I440FXState {
  */
 #define I440FX_COREBOOT_RAM_SIZE 0x57
 
+/* Older I440FX machines (5.0 and older) do not support i440FX-pcihost state
+ * migration, use some reserved INTEL 82441 configuration registers to
+ * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX PCISET
+ * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS ACCELERATOR
+ * (DBX) Table 1. PMC Configuration Space]
+ */
+#define I440FX_PCI_HOST_CONFIG_REG 0x94
+
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
@@ -98,15 +106,53 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_post_load(void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;
+    PCIDevice *dev;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
 
     i440fx_update_memory_mappings(d);
+
+    if (!s->config_reg_mig_enabled) {
+        dev = PCI_DEVICE(d);
+        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
+    }
+    return 0;
+}
+
+static int i440fx_pre_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->config_reg_mig_enabled) {
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
+                     s->config_reg);
+    }
+    return 0;
+}
+
+static int i440fx_post_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/i440fx", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->config_reg_mig_enabled) {
+        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
+    }
     return 0;
 }
 
+
 static const VMStateDescription vmstate_i440fx = {
     .name = "I440FX",
     .version_id = 3,
     .minimum_version_id = 3,
+    .pre_save = i440fx_pre_save,
+    .post_save = i440fx_post_save,
     .post_load = i440fx_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b67cb9c29f..40c975948c 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -43,6 +43,14 @@
 
 #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
 
+/* Older Q35 machines (5.0 and older) do not support q35-pcihost state
+ * migration, use some reserved INTEL MCH configuration registers to
+ * save/restore q35-pcihost config register. Refer to [Intel 3 Series
+ * Chipset Family Datasheet Table 5-1. DRAM Controller Register Address
+ * Map (D0:F0)]
+ */
+#define Q35_PCI_HOST_CONFIG_REG 0x70
+
 static void q35_host_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
@@ -513,14 +521,50 @@ static void mch_update(MCHPCIState *mch)
 static int mch_post_load(void *opaque, int version_id)
 {
     MCHPCIState *mch = opaque;
+    PCIDevice *dev;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
     mch_update(mch);
+    if (!s->config_reg_mig_enabled) {
+        dev = PCI_DEVICE(mch);
+        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
+    }
+    return 0;
+}
+
+static int mch_pre_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->config_reg_mig_enabled) {
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
+    }
     return 0;
 }
 
+static int mch_post_save(void *opaque)
+{
+    PCIDevice *dev = opaque;
+    PCIHostState *s = OBJECT_CHECK(PCIHostState,
+                                   object_resolve_path("/machine/q35", NULL),
+                                   TYPE_PCI_HOST_BRIDGE);
+    if (!s->config_reg_mig_enabled) {
+        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
+    }
+    return 0;
+}
+
+
 static const VMStateDescription vmstate_mch = {
     .name = "mch",
     .version_id = 1,
     .minimum_version_id = 1,
+    .pre_save = mch_pre_save,
+    .post_save = mch_post_save,
     .post_load = mch_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 8ca5fadcbd..c24daea84e 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -205,7 +205,7 @@ const MemoryRegionOps pci_host_data_be_ops = {
 static bool pci_host_needed(void *opaque)
 {
     PCIHostState *s = opaque;
-    return s->mig_enabled;
+    return s->config_reg_mig_enabled;
 }
 
 const VMStateDescription vmstate_pcihost = {
@@ -221,7 +221,7 @@ const VMStateDescription vmstate_pcihost = {
 
 static Property pci_host_properties_common[] = {
     DEFINE_PROP_BOOL("x-config-reg-migration-enabled", PCIHostState,
-                     mig_enabled, true),
+                     config_reg_mig_enabled, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index 6210a7e14d..23d6249979 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -45,7 +45,7 @@ struct PCIHostState {
     MemoryRegion data_mem;
     MemoryRegion mmcfg;
     uint32_t config_reg;
-    bool mig_enabled;
+    bool config_reg_mig_enabled;
     PCIBus *bus;
 
     QLIST_ENTRY(PCIHostState) next;
-- 
2.27.0




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5] hw/pci-host: save/restore pci host config register for old ones
@ 2020-07-30  1:33 Wangjing (Hogan, Cloud Infrastructure Service Product Dept.)
  0 siblings, 0 replies; 12+ messages in thread
From: Wangjing (Hogan, Cloud Infrastructure Service Product Dept.) @ 2020-07-30  1:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Huangweidong (C), Wangxin (Alexander), jusual, dgilbert, qemu-devel

> On Tue, Jul 28, 2020 at 11:27:09AM +0800, Hogan Wang wrote:
> > The i440fx and q35 machines integrate i440FX or MCH PCI device by default.
> > Refer to i440FX and ICH9-LPC spcifications, there are some reserved 
> > configuration registers can used to save/restore PCIHostState.config_reg.
> > It's nasty but friendly to old ones.
> > 
> > Reproducer steps:
> > 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.
> > 
> > Signed-off-by: Hogan Wang <hogan.wang@huawei.com>
> > ---
> >  hw/pci-host/i440fx.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >  hw/pci-host/q35.c    | 44 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 90 insertions(+)
> > 
> > diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c index 
> > 8ed2417f0c..419e27c21a 100644
> > --- a/hw/pci-host/i440fx.c
> > +++ b/hw/pci-host/i440fx.c
> > @@ -64,6 +64,14 @@ typedef struct I440FXState {
> >   */
> >  #define I440FX_COREBOOT_RAM_SIZE 0x57
> >  
> > +/* Older I440FX machines (5.0 and older) do not support 
> > +i440FX-pcihost state
> > + * migration, use some reserved INTEL 82441 configuration registers 
> > +to
> > + * save/restore i440FX-pcihost config register. Refer to [INTEL 440FX 
> > +PCISET
> > + * 82441FX PCI AND MEMORY CONTROLLER (PMC) AND 82442FX DATA BUS 
> > +ACCELERATOR
> > + * (DBX) Table 1. PMC Configuration Space]  */ #define 
> > +I440FX_PCI_HOST_CONFIG_REG 0x94
> > +
> >  static void i440fx_update_memory_mappings(PCII440FXState *d)  {
> >      int i;
> > @@ -98,15 +106,53 @@ static void i440fx_write_config(PCIDevice *dev,  
> > static int i440fx_post_load(void *opaque, int version_id)  {
> >      PCII440FXState *d = opaque;
> > +    PCIDevice *dev;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> >  
> >      i440fx_update_memory_mappings(d);
> > +
> > +    if (!s->mig_enabled) {
> 
> Thinking more about it, I think we should rename mig_enabled to config_reg_mig_enabled or something like this.
> 
Thanks for your pertinent suggestions, I will resend a new patch to fix it.
>
> > +        dev = PCI_DEVICE(d);
> > +        s->config_reg = pci_get_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG]);
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int i440fx_pre_save(void *opaque) {
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG],
> > +                     s->config_reg);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int i440fx_post_save(void *opaque) {
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/i440fx", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[I440FX_PCI_HOST_CONFIG_REG], 0);
> > +    }
> >      return 0;
> >  }
> >  
> > +
> 
> Extra empty line.
> 
> 
> >  static const VMStateDescription vmstate_i440fx = {
> >      .name = "I440FX",
> >      .version_id = 3,
> >      .minimum_version_id = 3,
> > +    .pre_save = i440fx_pre_save,
> > +    .post_save = i440fx_post_save,
> >      .post_load = i440fx_post_load,
> >      .fields = (VMStateField[]) {
> >          VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState), diff --git 
> > a/hw/pci-host/q35.c b/hw/pci-host/q35.c index b67cb9c29f..d87f892945 
> > 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -43,6 +43,14 @@
> >  
> >  #define Q35_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 35)
> >  
> > +/* Older Q35 machines (5.0 and older) do not support q35-pcihost 
> > +state
> > + * migration, use some reserved INTEL MCH configuration registers to
> > + * save/restore q35-pcihost config register. Refer to [Intel 3 Series
> > + * Chipset Family Datasheet Table 5-1. DRAM Controller Register 
> > +Address
> > + * Map (D0:F0)]
> > + */
> > +#define Q35_PCI_HOST_CONFIG_REG 0x70
> > +
> >  static void q35_host_realize(DeviceState *dev, Error **errp)  {
> >      PCIHostState *pci = PCI_HOST_BRIDGE(dev); @@ -513,14 +521,50 @@ 
> > static void mch_update(MCHPCIState *mch)  static int 
> > mch_post_load(void *opaque, int version_id)  {
> >      MCHPCIState *mch = opaque;
> > +    PCIDevice *dev;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> >      mch_update(mch);
> > +    if (!s->mig_enabled) {
> > +        dev = PCI_DEVICE(mch);
> > +        s->config_reg = pci_get_long(&dev->config[Q35_PCI_HOST_CONFIG_REG]);
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int mch_pre_save(void *opaque) {
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], s->config_reg);
> > +    }
> >      return 0;
> >  }
> >  
> > +static int mch_post_save(void *opaque) {
> > +    PCIDevice *dev = opaque;
> > +    PCIHostState *s = OBJECT_CHECK(PCIHostState,
> > +                                   object_resolve_path("/machine/q35", NULL),
> > +                                   TYPE_PCI_HOST_BRIDGE);
> > +    if (!s->mig_enabled) {
> > +        pci_set_long(&dev->config[Q35_PCI_HOST_CONFIG_REG], 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +
> >  static const VMStateDescription vmstate_mch = {
> >      .name = "mch",
> >      .version_id = 1,
> >      .minimum_version_id = 1,
> > +    .pre_save = mch_pre_save,
> > +    .post_save = mch_post_save,
> >      .post_load = mch_post_load,
> >      .fields = (VMStateField[]) {
> >          VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
> > --
> > 2.27.0
> > 



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-07-30  1:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27  8:46 [PATCH v4 1/2] hw/pci-host: save/restore pci host config register Hogan Wang
2020-07-27  8:46 ` [PATCH v4 2/2] hw/pci-host: save/restore pci host config register for old ones Hogan Wang
2020-07-27 13:08   ` Michael S. Tsirkin
2020-07-27 13:53     ` Dr. David Alan Gilbert
2020-07-27 14:08       ` Michael S. Tsirkin
2020-07-27 14:30   ` Michael S. Tsirkin
2020-07-27 15:47   ` Michael S. Tsirkin
2020-07-28  3:27     ` [PATCH v5] " Hogan Wang
2020-07-29 14:04       ` Michael S. Tsirkin
2020-07-29 18:42         ` Dr. David Alan Gilbert
2020-07-30  1:27           ` [PATCH 2/6] " Hogan Wang
2020-07-30  1:33 [PATCH v5] " Wangjing (Hogan, Cloud Infrastructure Service Product Dept.)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.