Re: [Qemu-devel] [PATCH V2] hw/pcie: fix the generic pcie root port to support migration

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Marcel Apfelbaum (marcel@redhat.com) wrote:
> Add msix state to pcie-root-ports's vmstate
> in order to support migration.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> v1 -> v2:
>  - Added x-migrate-msix compat property to avoid
>    breaking migration (Dave)
> 
>  hw/pci-bridge/gen_pcie_root_port.c | 25 +++++++++++++++++++++++++
>  include/hw/compat.h                |  4 ++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index 8ebffa8..cb694d6 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -20,6 +20,14 @@
>  #define GEN_PCIE_ROOT_PORT_AER_OFFSET           0x100
>  #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR       1
>  
> +typedef struct GenPCIERootPort {
> +    /*< private >*/
> +    PCIESlot parent_obj;
> +    /*< public >*/
> +
> +    bool migrate_msix;
> +} GenPCIERootPort;
> +
>  static uint8_t gen_rp_aer_vector(const PCIDevice *d)
>  {
>      return 0;
> @@ -45,6 +53,13 @@ static void gen_rp_interrupts_uninit(PCIDevice *d)
>      msix_uninit_exclusive_bar(d);
>  }
>  
> +static bool gen_rp_test_migrate_msix(void *opaque, int version_id)
> +{
> +    GenPCIERootPort *rp = opaque;
> +
> +    return rp->migrate_msix;
> +}
> +
>  static const VMStateDescription vmstate_rp_dev = {
>      .name = "pcie-root-port",
>      .version_id = 1,
> @@ -54,10 +69,18 @@ static const VMStateDescription vmstate_rp_dev = {
>          VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
>          VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
>                         PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> +        VMSTATE_MSIX_TEST(parent_obj.parent_obj.parent_obj.parent_obj,
> +                          GenPCIERootPort,
> +                          gen_rp_test_migrate_msix),

So that's  GenPCIERootPort,
    parent_obj is PCIESlot
    parent_obj is PCIEPort
    parent_obj is PCIEBridge
    parent_obj is PCIDevice

and msix_save is expecting a PCIDevice, so yep.

>          VMSTATE_END_OF_LIST()
>      }
>  };
>  
> +static Property gen_rp_props[] = {
> +    DEFINE_PROP_BOOL("x-migrate-msix", GenPCIERootPort, migrate_msix, true),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
>  static void gen_rp_dev_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -68,6 +91,7 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data)
>      k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;
>      dc->desc = "PCI Express Root Port";
>      dc->vmsd = &vmstate_rp_dev;
> +    dc->props = gen_rp_props;
>      rpc->aer_vector = gen_rp_aer_vector;
>      rpc->interrupts_init = gen_rp_interrupts_init;
>      rpc->interrupts_uninit = gen_rp_interrupts_uninit;
> @@ -77,6 +101,7 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data)
>  static const TypeInfo gen_rp_dev_info = {
>      .name          = TYPE_GEN_PCIE_ROOT_PORT,
>      .parent        = TYPE_PCIE_ROOT_PORT,
> +    .instance_size = sizeof(GenPCIERootPort),
>      .class_init    = gen_rp_dev_class_init,
>  };
>  
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 55b1765..4fb6a19 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -6,6 +6,10 @@
>          .driver   = "pci-bridge",\
>          .property = "shpc",\
>          .value    = "off",\
> +    },{\
> +        .driver   = "pcie-root-port",\
> +        .property = "x-migrate-msix",\
> +        .value    = "false",\

You could lose the x-  but that's OK.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>      },
>  
>  #define HW_COMPAT_2_8 \
> -- 
> 2.9.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK