Re: [PATCH for-9.0 07/10] vfio/spapr: Introduce a sPAPR VFIOIOMMU QOM interface

["Cédric Le Goater" <clg@redhat.com>]

On 12/11/23 07:25, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Sent: Friday, December 8, 2023 4:46 PM
>> Subject: [PATCH for-9.0 07/10] vfio/spapr: Introduce a sPAPR VFIOIOMMU
>> QOM interface
>>
>> Move vfio_spapr_container_setup() to a VFIOIOMMUClass::setup handler
>> and convert the sPAPR VFIOIOMMUOps struct to a QOM interface. The
>> sPAPR QOM interface inherits from the legacy QOM interface because
>> because both have the same basic needs. The sPAPR interface is then
>> extended with the handlers specific to the sPAPR IOMMU.
>>
>> This allows reuse and provides better abstraction of the backends. It
>> will be useful to avoid compiling the sPAPR IOMMU backend on targets
>> not supporting it.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> include/hw/vfio/vfio-container-base.h |  1 +
>> hw/vfio/container.c                   | 18 ++++--------
>> hw/vfio/spapr.c                       | 40 +++++++++++++++++----------
>> 3 files changed, 32 insertions(+), 27 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>> container-base.h
>> index
>> 870e7dc48e542ddbfc52e12b0ab5fab4771a1ebd..4012360c07b7c0a23f170f
>> 94a19455c79d3504b1 100644
>> --- a/include/hw/vfio/vfio-container-base.h
>> +++ b/include/hw/vfio/vfio-container-base.h
>> @@ -96,6 +96,7 @@ typedef struct VFIOIOMMU VFIOIOMMU;
>>
>> #define TYPE_VFIO_IOMMU "vfio-iommu"
>> #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>> +#define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>>
>> #define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>> TYPE_VFIO_IOMMU)
>> DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>> TYPE_VFIO_IOMMU)
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index
>> 5f5ad8479f083db0be5207f179f3056ae67c49c3..ce5a731ba74600fbb331a8
>> 0f5148a88e2e43b068 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -381,6 +381,10 @@ static const VFIOIOMMUClass
>> *vfio_get_iommu_class(int iommu_type, Error **errp)
>>      case VFIO_TYPE1_IOMMU:
>>          klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
>>          break;
>> +    case VFIO_SPAPR_TCE_v2_IOMMU:
>> +    case VFIO_SPAPR_TCE_IOMMU:
>> +        klass = object_class_by_name(TYPE_VFIO_IOMMU_SPAPR);
>> +        break;
>>      default:
>>          g_assert_not_reached();
>>      };
>> @@ -623,19 +627,9 @@ static int vfio_connect_container(VFIOGroup
>> *group, AddressSpace *as,
>>          goto free_container_exit;
>>      }
>>
>> -    switch (container->iommu_type) {
>> -    case VFIO_TYPE1v2_IOMMU:
>> -    case VFIO_TYPE1_IOMMU:
>> -        ret = vfio_legacy_setup(bcontainer, errp);
>> -        break;
>> -    case VFIO_SPAPR_TCE_v2_IOMMU:
>> -    case VFIO_SPAPR_TCE_IOMMU:
>> -        ret = vfio_spapr_container_init(container, errp);
>> -        break;
>> -    default:
>> -        g_assert_not_reached();
>> -    }
>> +    assert(bcontainer->ops->setup);
>>
>> +    ret = bcontainer->ops->setup(bcontainer, errp);
>>      if (ret) {
>>          goto enable_discards_exit;
>>      }
>> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
>> index
>> 44617dfc6b5f1a2a3a1c37436b76042aebda8b63..46aa14bd2ae6d580c16bb
>> a75838cb6aca7d4047f 100644
>> --- a/hw/vfio/spapr.c
>> +++ b/hw/vfio/spapr.c
>> @@ -458,20 +458,11 @@ static void
>> vfio_spapr_container_release(VFIOContainerBase *bcontainer)
>>      }
>> }
>>
>> -static VFIOIOMMUOps vfio_iommu_spapr_ops;
>> -
>> -static void setup_spapr_ops(VFIOContainerBase *bcontainer)
>> -{
>> -    vfio_iommu_spapr_ops = *bcontainer->ops;
>> -    vfio_iommu_spapr_ops.add_window =
>> vfio_spapr_container_add_section_window;
>> -    vfio_iommu_spapr_ops.del_window =
>> vfio_spapr_container_del_section_window;
>> -    vfio_iommu_spapr_ops.release = vfio_spapr_container_release;
>> -    bcontainer->ops = &vfio_iommu_spapr_ops;
>> -}
>> -
>> -int vfio_spapr_container_init(VFIOContainer *container, Error **errp)
>> +static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>> +                                      Error **errp)
>> {
>> -    VFIOContainerBase *bcontainer = &container->bcontainer;
>> +    VFIOContainer *container = container_of(bcontainer, VFIOContainer,
>> +                                            bcontainer);
>>      VFIOSpaprContainer *scontainer = container_of(container,
>> VFIOSpaprContainer,
>>                                                    container);
>>      struct vfio_iommu_spapr_tce_info info;
>> @@ -536,8 +527,6 @@ int vfio_spapr_container_init(VFIOContainer
>> *container, Error **errp)
>>                            0x1000);
>>      }
>>
>> -    setup_spapr_ops(bcontainer);
>> -
>>      return 0;
>>
>> listener_unregister_exit:
>> @@ -546,3 +535,24 @@ listener_unregister_exit:
>>      }
>>      return ret;
>> }
>> +
>> +static void vfio_iommu_spapr_class_init(ObjectClass *klass, void *data)
>> +{
>> +    VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>> +
>> +    vioc->add_window = vfio_spapr_container_add_section_window;
>> +    vioc->del_window = vfio_spapr_container_del_section_window;
>> +    vioc->release = vfio_spapr_container_release;
>> +    vioc->setup = vfio_spapr_container_setup;
>> +};
>> +
>> +static const TypeInfo types[] = {
>> +    {
>> +        .name = TYPE_VFIO_IOMMU_SPAPR,
>> +        .parent = TYPE_VFIO_IOMMU_LEGACY,
>> +        .class_init = vfio_iommu_spapr_class_init,
>> +        .class_size = sizeof(VFIOIOMMUClass),
> 
> Inherit parent class_size is enough? Otherwise,

yes.


Thanks,

C.


> 
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> Thanks
> Zhenzhong
> 
>> +    },
>> +};
>> +
>> +DEFINE_TYPES(types)
>> --
>> 2.43.0
>