Re: [PATCH v2 11/17] vfio/container: Change VFIOContainerBase to use QOM

[Eric Auger]

On 6/17/24 08:34, Cédric Le Goater wrote:
> VFIOContainerBase was made a QOM interface because we believed that a
> QOM object would expose all the IOMMU backends to the QEMU machine and
> human interface. This only applies to user creatable devices or objects.
>
> Change the VFIOContainerBase nature from interface to object and make
> the necessary adjustments in the VFIO_IOMMU hierarchy.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  include/hw/vfio/vfio-common.h         |  4 ++++
>  include/hw/vfio/vfio-container-base.h | 12 +++---------
>  hw/vfio/container-base.c              |  4 +++-
>  hw/vfio/container.c                   |  1 +
>  hw/vfio/iommufd.c                     |  1 +
>  hw/vfio/spapr.c                       |  3 +++
>  6 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 
> 825d80130bd435fe50830c8ae5b7905d18104dd6..e8ddf92bb18547f0d3b811b3d757cbae7fec8b8d
>  100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -84,6 +84,8 @@ typedef struct VFIOContainer {
>      QLIST_HEAD(, VFIOGroup) group_list;
>  } VFIOContainer;
>  
> +OBJECT_DECLARE_SIMPLE_TYPE(VFIOContainer, VFIO_IOMMU_LEGACY);
> +
>  typedef struct VFIOHostDMAWindow {
>      hwaddr min_iova;
>      hwaddr max_iova;
> @@ -99,6 +101,8 @@ typedef struct VFIOIOMMUFDContainer {
>      uint32_t ioas_id;
>  } VFIOIOMMUFDContainer;
>  
> +OBJECT_DECLARE_SIMPLE_TYPE(VFIOIOMMUFDContainer, VFIO_IOMMU_IOMMUFD);
> +
>  typedef struct VFIODeviceOps VFIODeviceOps;
>  
>  typedef struct VFIODevice {
> diff --git a/include/hw/vfio/vfio-container-base.h 
> b/include/hw/vfio/vfio-container-base.h
> index 
> d505f63607ec40e6aa44aeb3e20848ac780562a1..b079b76f68975c5701a289ce9012e912a8e44fc6
>  100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -34,6 +34,7 @@ typedef struct VFIOAddressSpace {
>   * This is the base object for vfio container backends
>   */
>  typedef struct VFIOContainerBase {
> +    Object parent;
>      const VFIOIOMMUClass *ops;
>      VFIOAddressSpace *space;
>      MemoryListener listener;
> @@ -96,17 +97,10 @@ void vfio_container_destroy(VFIOContainerBase 
> *bcontainer);
>  #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
>  #define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
>  
> -/*
> - * VFIOContainerBase is not an abstract QOM object because it felt
> - * unnecessary to expose all the IOMMU backends to the QEMU machine
> - * and human interface. However, we can still abstract the IOMMU
> - * backend handlers using a QOM interface class. This provides more
> - * flexibility when referencing the various implementations.
> - */
> -DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU, TYPE_VFIO_IOMMU)
> +OBJECT_DECLARE_TYPE(VFIOContainerBase, VFIOIOMMUClass, VFIO_IOMMU)
>  
>  struct VFIOIOMMUClass {
> -    InterfaceClass parent_class;
> +    ObjectClass parent_class;
>  
>      /* Properties */
>      const char *hiod_typename;
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index 
> 280f0dd2db1fc3939fe9925ce00a2c50d0e14196..98c15e174dd78df5146ee83c05c98f3ea9c1e52c
>  100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -102,8 +102,10 @@ void vfio_container_destroy(VFIOContainerBase 
> *bcontainer)
>  static const TypeInfo types[] = {
>      {
>          .name = TYPE_VFIO_IOMMU,
> -        .parent = TYPE_INTERFACE,
> +        .parent = TYPE_OBJECT,
> +        .instance_size = sizeof(VFIOContainerBase),
>          .class_size = sizeof(VFIOIOMMUClass),
> +        .abstract = true,
>      },
>  };
>  
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 
> 31bdc46a96d1626b237227a25007957e1d472757..3ae52530a9b500bd53ec9f9e66c73253d97c9aba
>  100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -1196,6 +1196,7 @@ static const TypeInfo types[] = {
>      {
>          .name = TYPE_VFIO_IOMMU_LEGACY,
>          .parent = TYPE_VFIO_IOMMU,
> +        .instance_size = sizeof(VFIOContainer),
>          .class_init = vfio_iommu_legacy_class_init,
>      }, {
>          .name = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO,
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 
> e5d9334142418514215528b9523f12c031792c7f..3e9d642034c2d2234ea701952c94a78ab32e9147
>  100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -672,6 +672,7 @@ static const TypeInfo types[] = {
>      {
>          .name = TYPE_VFIO_IOMMU_IOMMUFD,
>          .parent = TYPE_VFIO_IOMMU,
> +        .instance_size = sizeof(VFIOIOMMUFDContainer),
>          .class_init = vfio_iommu_iommufd_class_init,
>      }, {
>          .name = TYPE_HOST_IOMMU_DEVICE_IOMMUFD_VFIO,
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 
> 47b040f1bcca7dd0b5cf052d941b43541e98a3c5..018bd2048194a6a2db83ed740025a7060181698f
>  100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -30,6 +30,8 @@ typedef struct VFIOSpaprContainer {
>      QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
>  } VFIOSpaprContainer;
>  
> +OBJECT_DECLARE_SIMPLE_TYPE(VFIOSpaprContainer, VFIO_IOMMU_SPAPR);
> +
>  static bool vfio_prereg_listener_skipped_section(MemoryRegionSection 
> *section)
>  {
>      if (memory_region_is_iommu(section->mr)) {
> @@ -548,6 +550,7 @@ static const TypeInfo types[] = {
>      {
>          .name = TYPE_VFIO_IOMMU_SPAPR,
>          .parent = TYPE_VFIO_IOMMU_LEGACY,
> +        .instance_size = sizeof(VFIOSpaprContainer),
>          .class_init = vfio_iommu_spapr_class_init,
>      },
>  };