Re: [PATCH v3 2/2] hw/pci-host: save/restore pci host config register for old ones

["Michael S. Tsirkin" <mst@redhat.com>]

On Sat, Jul 25, 2020 at 08:54:37PM +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.
> 
> Signed-off-by: Hogan Wang <hogan.wang@huawei.com>

Just to make sure, how was this tested?
Did you test that migration to an old qemu e.g. the last release
with an old machine type works with this patch?


> ---
>  hw/pci-host/i440fx.c | 31 +++++++++++++++++++++++++++++++
>  hw/pci-host/q35.c    | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index c662903dbb..9e2cfdb052 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -65,6 +65,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;
> @@ -99,8 +107,30 @@ static void i440fx_write_config(PCIDevice *dev,
>  static int i440fx_post_load(void *opaque, int version_id)
>  {
>      PCII440FXState *d = opaque;
> +    uint8_t *config;
> +    I440FXState *s = OBJECT_CHECK(I440FXState,
> +                                  object_resolve_path("/machine/i440fx", NULL),
> +                                  TYPE_PCI_HOST_BRIDGE);
>  
>      i440fx_update_memory_mappings(d);
> +
> +    if (!s->migration_enabled) {
> +        config = &d->parent_obj.config[I440FX_PCI_HOST_CONFIG_REG];
> +        s->parent_obj.config_reg = pci_get_long(config);
> +    }
> +    return 0;
> +}
> +
> +static int i440fx_pre_save(void *opaque)
> +{
> +    PCIDevice *d = opaque;
> +    I440FXState *s = OBJECT_CHECK(I440FXState,
> +                                  object_resolve_path("/machine/i440fx", NULL),
> +                                  TYPE_PCI_HOST_BRIDGE);
> +    if (!s->migration_enabled) {
> +        pci_set_long(&d->config[I440FX_PCI_HOST_CONFIG_REG],
> +                     s->parent_obj.config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -108,6 +138,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 bb41665da4..67e08dedc5 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);
> @@ -532,7 +541,34 @@ static void mch_update(MCHPCIState *mch)
>  static int mch_post_load(void *opaque, int version_id)
>  {
>      MCHPCIState *mch = opaque;
> +    uint8_t *config;
> +    Q35PCIHost *s = OBJECT_CHECK(Q35PCIHost,
> +                                 object_resolve_path("/machine/q35", NULL),
> +                                 TYPE_PCI_HOST_BRIDGE);
> +    PCIHostState *pci = PCI_HOST_BRIDGE(s);
> +
>      mch_update(mch);
> +    if (!s->migration_enabled) {
> +        config = &mch->parent_obj.config[Q35_PCI_HOST_CONFIG_REG];
> +        pci->config_reg = pci_get_long(config);
> +    }
> +
> +    return 0;
> +}
> +
> +static int mch_pre_save(void *opaque)
> +{
> +    MCHPCIState *mch = opaque;
> +    uint8_t *config;
> +    Q35PCIHost *s = OBJECT_CHECK(Q35PCIHost,
> +                                 object_resolve_path("/machine/q35", NULL),
> +                                 TYPE_PCI_HOST_BRIDGE);
> +    PCIHostState *pci = PCI_HOST_BRIDGE(s);
> +
> +    if (!s->migration_enabled) {
> +        config = &mch->parent_obj.config[Q35_PCI_HOST_CONFIG_REG];
> +        pci_set_long(config, pci->config_reg);
> +    }
>      return 0;
>  }
>  
> @@ -540,6 +576,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
>