Re: [PATCH] hw/pci-host: save/restore pci host config register for old ones
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Mon, Aug 10, 2020 at 04:58:06PM +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(>chr, , 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
>
>
> dgilbrt so what is your take on this?
I think from a migration point of view I'm OK with it; so,
Acked-by: Dr. David Alan Gilbert
If you know the bridges well enough to know that it's safe to misuse
that other register, then I think it's OK.
> Also, how about a capability that will make hacks like this
> one redundant in the future?
I have an idea for that; basically you'd have to store a bunch of these
droppable-subsections into a rambuffer, then save them with a specially
marked section; that's a section that would then individually get
treated as a set of subsections, but if the subsections are unknown the
destination would shrug rather than fall over.
Dave
>
> > ---
> > 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)
> > {
> >
Re: [PATCH] hw/pci-host: save/restore pci host config register for old ones
On Mon, Aug 10, 2020 at 04:58:06PM +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(>chr, , 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
dgilbrt so what is your take on this?
Also, how about a capability that will make hacks like this
one redundant in the future?
> ---
> 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(>config[I440FX_PCI_HOST_CONFIG_REG]);
> +pci_set_long(>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),
> +
[PATCH] hw/pci-host: save/restore pci host config register for old ones
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(>chr, , 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
---
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(>config[I440FX_PCI_HOST_CONFIG_REG]);
+pci_set_long(>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(>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),
+
Re: [PATCH] hw/pci-host: save/restore pci host config register
On Thu, Jul 23, 2020 at 01:48:47PM +0200, Laszlo Ersek wrote:
> On 07/23/20 12:49, Wang King wrote:
> > From: Hogan Wang
> >
> > 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.
> >
> > Example:
> > 1. guest booting in seabios.
> > 2. guest enabled the SMM memory window in piix4_apmc_smm_setup, and
> > then try to close the SMM memory window.
> > 3. pasued vcpu to finish migration.
> > 4. guest close the SMM memory window fail becasue of config register
> > state lost.
> > 5. guest continue to boot and crash in ipxe option ROM (SMM memory
> > window is enabled).
> >
> > Due to the complex guest, the negative effect is unpredictable.
Is there a way to build a unit test for this btw?
That would be great ...
> > ---
> > hw/pci-host/i440fx.c | 11 +++
> > hw/pci-host/q35.c | 11 +++
> > hw/pci/pci_host.c | 11 +++
> > hw/pci/pcie_host.c | 11 +++
> > include/hw/pci/pci_host.h | 10 ++
> > include/hw/pci/pcie_host.h | 10 ++
> > 6 files changed, 64 insertions(+)
> >
> > diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> > index 8ed2417f0c..17705bb025 100644
> > --- a/hw/pci-host/i440fx.c
> > +++ b/hw/pci-host/i440fx.c
> > @@ -118,6 +118,16 @@ static const VMStateDescription vmstate_i440fx = {
> > }
> > };
> >
> > +static const VMStateDescription vmstate_i440fx_pcihost = {
> > +.name = "I440FX_PCIHost",
> > +.version_id = 1,
> > +.minimum_version_id = 1,
> > +.fields = (VMStateField[]) {
> > +VMSTATE_PCI_HOST(parent_obj, I440FXState),
> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > static void i440fx_pcihost_get_pci_hole_start(Object *obj, Visitor *v,
> >const char *name, void
> > *opaque,
> >Error **errp)
> > @@ -398,6 +408,7 @@ static void i440fx_pcihost_class_init(ObjectClass
> > *klass, void *data)
> > hc->root_bus_path = i440fx_pcihost_root_bus_path;
> > dc->realize = i440fx_pcihost_realize;
> > dc->fw_name = "pci";
> > +dc->vmsd = _i440fx_pcihost;
> > device_class_set_props(dc, i440fx_props);
> > /* Reason: needs to be wired up by pc_init1 */
> > dc->user_creatable = false;
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index b67cb9c29f..5e323be2e3 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -165,6 +165,16 @@ static void q35_host_get_pci_hole64_end(Object *obj,
> > Visitor *v,
> > visit_type_uint64(v, name, , errp);
> > }
> >
> > +static const VMStateDescription vmstate_q35_pcihost = {
> > +.name = "Q35_PCIHost",
> > +.version_id = 1,
> > +.minimum_version_id = 1,
> > +.fields = (VMStateField[]) {
> > +VMSTATE_PCIE_HOST(parent_obj, Q35PCIHost),
> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > /*
> > * NOTE: setting defaults for the mch.* fields in this table
> > * doesn't work, because mch is a separate QOM object that is
> > @@ -194,6 +204,7 @@ static void q35_host_class_init(ObjectClass *klass,
> > void *data)
> >
> > hc->root_bus_path = q35_host_root_bus_path;
> > dc->realize = q35_host_realize;
> > +dc->vmsd = _q35_pcihost;
> > device_class_set_props(dc, q35_host_props);
> > /* Reason: needs to be wired up by pc_q35_init */
> > dc->user_creatable = false;
> > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> > index ce7bcdb1d5..7cdd5a3ea3 100644
> > --- a/hw/pci/pci_host.c
> > +++ b/hw/pci/pci_host.c
> > @@ -24,6 +24,7 @@
> > #include "hw/pci/pci_host.h"
> > #include "qemu/module.h"
> > #include "hw/pci/pci_bus.h"
> > +#include "migration/vmstate.h"
> > #include "trace.h"
> >
> > /* debug PCI */
> > @@ -200,6 +201,16 @@ const MemoryRegionOps pci_host_data_be_ops = {
> > .endianness = DEVICE_BIG_ENDIAN,
> > };
> >
> > +const VMStateDescription vmstate_pcihost = {
> > +.name = "PCIHost",
> > +.version_id = 1,
> > +.minimum_version_id = 1,
> > +.fields = (VMStateField[]) {
> > +VMSTATE_UINT32(config_reg, PCIHostState),
> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > static const TypeInfo pci_host_type_info = {
> > .name = TYPE_PCI_HOST_BRIDGE,
> > .parent = TYPE_SYS_BUS_DEVICE,
> > diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
> > index 3534006f99..a653c39bb7 100644
> > --- a/hw/pci/pcie_host.c
> > +++ b/hw/pci/pcie_host.c
> > @@ -24,6 +24,7 @@
> > #include "hw/pci/pcie_host.h"
> > #include "qemu/module.h"
> > #include "exec/address-spaces.h"
> > +#include "migration/vmstate.h"
> >
> > /* a helper function to get a PCIDevice for a given
Re: [PATCH] hw/pci-host: save/restore pci host config register
On Thu, Jul 23, 2020 at 06:49:35PM +0800, Wang King wrote:
> From: Hogan Wang
>
> 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.
Wow I can't believe we have such a bug after so many years.
Question is, this will break cross-version migration if we just add it.
Could we use some trick so people can upgrade in the field
without breaking migration?
I regret we still don't have an extensible format where we could
add fields without breaking everything ...
CC Julua, Dgilbert to take a look.
> Example:
> 1. guest booting in seabios.
> 2. guest enabled the SMM memory window in piix4_apmc_smm_setup, and
> then try to close the SMM memory window.
> 3. pasued vcpu to finish migration.
> 4. guest close the SMM memory window fail becasue of config register
> state lost.
> 5. guest continue to boot and crash in ipxe option ROM (SMM memory
> window is enabled).
>
> Due to the complex guest, the negative effect is unpredictable.
Could we get a sign-off please?
The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch. The rules are pretty simple: if you
can certify the below:
Developer's Certificate of Origin 1.1
^
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
then you just add a line saying::
Signed-off-by: Random J Developer
using your real name (sorry, no pseudonyms or anonymous contributions.)
> ---
> hw/pci-host/i440fx.c | 11 +++
> hw/pci-host/q35.c | 11 +++
> hw/pci/pci_host.c | 11 +++
> hw/pci/pcie_host.c | 11 +++
> include/hw/pci/pci_host.h | 10 ++
> include/hw/pci/pcie_host.h | 10 ++
> 6 files changed, 64 insertions(+)
>
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 8ed2417f0c..17705bb025 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -118,6 +118,16 @@ static const VMStateDescription vmstate_i440fx = {
> }
> };
>
> +static const VMStateDescription vmstate_i440fx_pcihost = {
> +.name = "I440FX_PCIHost",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCI_HOST(parent_obj, I440FXState),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> static void i440fx_pcihost_get_pci_hole_start(Object *obj, Visitor *v,
>const char *name, void *opaque,
>Error **errp)
> @@ -398,6 +408,7 @@ static void i440fx_pcihost_class_init(ObjectClass *klass,
> void *data)
> hc->root_bus_path = i440fx_pcihost_root_bus_path;
> dc->realize = i440fx_pcihost_realize;
> dc->fw_name = "pci";
> +dc->vmsd = _i440fx_pcihost;
> device_class_set_props(dc, i440fx_props);
> /* Reason: needs to be wired up by pc_init1 */
> dc->user_creatable = false;
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b67cb9c29f..5e323be2e3 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -165,6 +165,16 @@ static void q35_host_get_pci_hole64_end(Object *obj,
> Visitor *v,
> visit_type_uint64(v, name, , errp);
> }
>
> +static const VMStateDescription vmstate_q35_pcihost = {
> +.name = "Q35_PCIHost",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCIE_HOST(parent_obj, Q35PCIHost),
> +
Re: [PATCH] hw/pci-host: save/restore pci host config register
On 07/23/20 12:49, Wang King wrote:
> From: Hogan Wang
>
> 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.
>
> Example:
> 1. guest booting in seabios.
> 2. guest enabled the SMM memory window in piix4_apmc_smm_setup, and
> then try to close the SMM memory window.
> 3. pasued vcpu to finish migration.
> 4. guest close the SMM memory window fail becasue of config register
> state lost.
> 5. guest continue to boot and crash in ipxe option ROM (SMM memory
> window is enabled).
>
> Due to the complex guest, the negative effect is unpredictable.
> ---
> hw/pci-host/i440fx.c | 11 +++
> hw/pci-host/q35.c | 11 +++
> hw/pci/pci_host.c | 11 +++
> hw/pci/pcie_host.c | 11 +++
> include/hw/pci/pci_host.h | 10 ++
> include/hw/pci/pcie_host.h | 10 ++
> 6 files changed, 64 insertions(+)
>
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index 8ed2417f0c..17705bb025 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -118,6 +118,16 @@ static const VMStateDescription vmstate_i440fx = {
> }
> };
>
> +static const VMStateDescription vmstate_i440fx_pcihost = {
> +.name = "I440FX_PCIHost",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCI_HOST(parent_obj, I440FXState),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> static void i440fx_pcihost_get_pci_hole_start(Object *obj, Visitor *v,
>const char *name, void *opaque,
>Error **errp)
> @@ -398,6 +408,7 @@ static void i440fx_pcihost_class_init(ObjectClass *klass,
> void *data)
> hc->root_bus_path = i440fx_pcihost_root_bus_path;
> dc->realize = i440fx_pcihost_realize;
> dc->fw_name = "pci";
> +dc->vmsd = _i440fx_pcihost;
> device_class_set_props(dc, i440fx_props);
> /* Reason: needs to be wired up by pc_init1 */
> dc->user_creatable = false;
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index b67cb9c29f..5e323be2e3 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -165,6 +165,16 @@ static void q35_host_get_pci_hole64_end(Object *obj,
> Visitor *v,
> visit_type_uint64(v, name, , errp);
> }
>
> +static const VMStateDescription vmstate_q35_pcihost = {
> +.name = "Q35_PCIHost",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCIE_HOST(parent_obj, Q35PCIHost),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> /*
> * NOTE: setting defaults for the mch.* fields in this table
> * doesn't work, because mch is a separate QOM object that is
> @@ -194,6 +204,7 @@ static void q35_host_class_init(ObjectClass *klass, void
> *data)
>
> hc->root_bus_path = q35_host_root_bus_path;
> dc->realize = q35_host_realize;
> +dc->vmsd = _q35_pcihost;
> device_class_set_props(dc, q35_host_props);
> /* Reason: needs to be wired up by pc_q35_init */
> dc->user_creatable = false;
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index ce7bcdb1d5..7cdd5a3ea3 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -24,6 +24,7 @@
> #include "hw/pci/pci_host.h"
> #include "qemu/module.h"
> #include "hw/pci/pci_bus.h"
> +#include "migration/vmstate.h"
> #include "trace.h"
>
> /* debug PCI */
> @@ -200,6 +201,16 @@ const MemoryRegionOps pci_host_data_be_ops = {
> .endianness = DEVICE_BIG_ENDIAN,
> };
>
> +const VMStateDescription vmstate_pcihost = {
> +.name = "PCIHost",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(config_reg, PCIHostState),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> static const TypeInfo pci_host_type_info = {
> .name = TYPE_PCI_HOST_BRIDGE,
> .parent = TYPE_SYS_BUS_DEVICE,
> diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
> index 3534006f99..a653c39bb7 100644
> --- a/hw/pci/pcie_host.c
> +++ b/hw/pci/pcie_host.c
> @@ -24,6 +24,7 @@
> #include "hw/pci/pcie_host.h"
> #include "qemu/module.h"
> #include "exec/address-spaces.h"
> +#include "migration/vmstate.h"
>
> /* a helper function to get a PCIDevice for a given mmconfig address */
> static inline PCIDevice *pcie_dev_find_by_mmcfg_addr(PCIBus *s,
> @@ -121,6 +122,16 @@ void pcie_host_mmcfg_update(PCIExpressHost *e,
> memory_region_transaction_commit();
> }
>
> +const VMStateDescription vmstate_pciehost = {
> +.name = "PCIEHost",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCI_HOST(pci,
[PATCH] hw/pci-host: save/restore pci host config register
From: Hogan Wang
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.
Example:
1. guest booting in seabios.
2. guest enabled the SMM memory window in piix4_apmc_smm_setup, and
then try to close the SMM memory window.
3. pasued vcpu to finish migration.
4. guest close the SMM memory window fail becasue of config register
state lost.
5. guest continue to boot and crash in ipxe option ROM (SMM memory
window is enabled).
Due to the complex guest, the negative effect is unpredictable.
---
hw/pci-host/i440fx.c | 11 +++
hw/pci-host/q35.c | 11 +++
hw/pci/pci_host.c | 11 +++
hw/pci/pcie_host.c | 11 +++
include/hw/pci/pci_host.h | 10 ++
include/hw/pci/pcie_host.h | 10 ++
6 files changed, 64 insertions(+)
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 8ed2417f0c..17705bb025 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -118,6 +118,16 @@ static const VMStateDescription vmstate_i440fx = {
}
};
+static const VMStateDescription vmstate_i440fx_pcihost = {
+.name = "I440FX_PCIHost",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_HOST(parent_obj, I440FXState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static void i440fx_pcihost_get_pci_hole_start(Object *obj, Visitor *v,
const char *name, void *opaque,
Error **errp)
@@ -398,6 +408,7 @@ static void i440fx_pcihost_class_init(ObjectClass *klass,
void *data)
hc->root_bus_path = i440fx_pcihost_root_bus_path;
dc->realize = i440fx_pcihost_realize;
dc->fw_name = "pci";
+dc->vmsd = _i440fx_pcihost;
device_class_set_props(dc, i440fx_props);
/* Reason: needs to be wired up by pc_init1 */
dc->user_creatable = false;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b67cb9c29f..5e323be2e3 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -165,6 +165,16 @@ static void q35_host_get_pci_hole64_end(Object *obj,
Visitor *v,
visit_type_uint64(v, name, , errp);
}
+static const VMStateDescription vmstate_q35_pcihost = {
+.name = "Q35_PCIHost",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_PCIE_HOST(parent_obj, Q35PCIHost),
+VMSTATE_END_OF_LIST()
+}
+};
+
/*
* NOTE: setting defaults for the mch.* fields in this table
* doesn't work, because mch is a separate QOM object that is
@@ -194,6 +204,7 @@ static void q35_host_class_init(ObjectClass *klass, void
*data)
hc->root_bus_path = q35_host_root_bus_path;
dc->realize = q35_host_realize;
+dc->vmsd = _q35_pcihost;
device_class_set_props(dc, q35_host_props);
/* Reason: needs to be wired up by pc_q35_init */
dc->user_creatable = false;
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index ce7bcdb1d5..7cdd5a3ea3 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -24,6 +24,7 @@
#include "hw/pci/pci_host.h"
#include "qemu/module.h"
#include "hw/pci/pci_bus.h"
+#include "migration/vmstate.h"
#include "trace.h"
/* debug PCI */
@@ -200,6 +201,16 @@ const MemoryRegionOps pci_host_data_be_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
+const VMStateDescription vmstate_pcihost = {
+.name = "PCIHost",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(config_reg, PCIHostState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const TypeInfo pci_host_type_info = {
.name = TYPE_PCI_HOST_BRIDGE,
.parent = TYPE_SYS_BUS_DEVICE,
diff --git a/hw/pci/pcie_host.c b/hw/pci/pcie_host.c
index 3534006f99..a653c39bb7 100644
--- a/hw/pci/pcie_host.c
+++ b/hw/pci/pcie_host.c
@@ -24,6 +24,7 @@
#include "hw/pci/pcie_host.h"
#include "qemu/module.h"
#include "exec/address-spaces.h"
+#include "migration/vmstate.h"
/* a helper function to get a PCIDevice for a given mmconfig address */
static inline PCIDevice *pcie_dev_find_by_mmcfg_addr(PCIBus *s,
@@ -121,6 +122,16 @@ void pcie_host_mmcfg_update(PCIExpressHost *e,
memory_region_transaction_commit();
}
+const VMStateDescription vmstate_pciehost = {
+.name = "PCIEHost",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_HOST(pci, PCIExpressHost),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const TypeInfo pcie_host_type_info = {
.name = TYPE_PCIE_HOST_BRIDGE,
.parent = TYPE_PCI_HOST_BRIDGE,
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index 9ce088bd13..fc88305e04 100644
--- a/include/hw/pci/pci_host.h
