[PATCH for-9.0 00/10] vfio: Introduce a VFIOIOMMUClass

[PATCH for-9.0 00/10] vfio: Introduce a VFIOIOMMUClass

[Cédric Le Goater]

Hello,

The VFIO object hierarchy has some constraints because each VFIO type
has a dual nature: a VFIO nature for passthrough support and a bus
nature (PCI, AP, CCW, Platform) for its initial presentation. It
seemed the best approach made because multi-inheritance is not
feasible with QOM and both aspect of the VFIO object, passthrough and
bus, require state. A QOM interface in that case is not sufficient.

One aspect of passthrough is interaction with the IOMMU. IOMMUFD
support was recently added and for this purpose, we introduced an
IOMMU backend framework simply based on a VFIOIOMMUOps struct. We
didn't want to use QOM again because it would have exposed the various
lowlevel backend objects to the QEMU machine and human interface which
felt unnecessary at the time.

The changes of this series introduce a VFIO_IOMMU QOM interface and
its VFIOIOMMUClass to replace the current VFIOIOMMUOps. This provides
better code abstraction for the type1 and sPAPR IOMMU backends and
allows us to improve the vfio_connect_container() implementation.
Also, QOM interfaces are not exposed at the QEMU interface level. Most
important, we can now avoid compiling the sPAPR IOMMU support on
targets not needing it. This saves some text in QEMU.

Applies on vfio-next.

Thanks,

C.

Cédric Le Goater (10):
  vfio/spapr: Extend VFIOIOMMUOps with a release handler
  vfio/container: Introduce vfio_legacy_setup() for further cleanups
  vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()
  vfio/container: Introduce a VFIOIOMMU QOM interface
  vfio/container: Introduce a VFIOIOMMU legacy QOM interface
  vfio/container: Intoduce a new VFIOIOMMUClass::setup handler
  vfio/spapr: Introduce a sPAPR VFIOIOMMU QOM interface
  vfio/iommufd: Introduce a VFIOIOMMU iommufd QOM interface
  vfio/spapr: Only compile sPAPR IOMMU support when needed
  vfio/iommufd: Remove CONFIG_IOMMUFD usage

 include/hw/vfio/vfio-common.h         |   2 -
 include/hw/vfio/vfio-container-base.h |  22 +++-
 hw/vfio/common.c                      |  11 +-
 hw/vfio/container-base.c              |  12 ++-
 hw/vfio/container.c                   | 147 ++++++++++++++++----------
 hw/vfio/iommufd.c                     |  36 +++++--
 hw/vfio/pci.c                         |   2 +-
 hw/vfio/spapr.c                       |  61 ++++++-----
 hw/vfio/meson.build                   |   2 +-
 9 files changed, 195 insertions(+), 100 deletions(-)

-- 
2.43.0

[PATCH for-9.0 01/10] vfio/spapr: Extend VFIOIOMMUOps with a release handler

[Cédric Le Goater]

This allows to abstract a bit more the sPAPR IOMMU support in the
legacy IOMMU backend.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/container.c                   | 10 +++-----
 hw/vfio/spapr.c                       | 35 +++++++++++++++------------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 2ae297ccda93fd97986c852a8329b390fa1ab91f..5c9594b6c77681e5593236e711e7e391e5f2bdff 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -117,5 +117,6 @@ struct VFIOIOMMUOps {
                       Error **errp);
     void (*del_window)(VFIOContainerBase *bcontainer,
                        MemoryRegionSection *section);
+    void (*release)(VFIOContainerBase *bcontainer);
 };
 #endif /* HW_VFIO_VFIO_CONTAINER_BASE_H */
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index b22feb8ded0a0d9ed98d6e206b78c0c6e2554d5c..1e77a2929e90ed1d2ee84062549c477ae651c5a8 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -632,9 +632,8 @@ listener_release_exit:
     QLIST_REMOVE(bcontainer, next);
     vfio_kvm_device_del_group(group);
     memory_listener_unregister(&bcontainer->listener);
-    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
-        container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
-        vfio_spapr_container_deinit(container);
+    if (bcontainer->ops->release) {
+        bcontainer->ops->release(bcontainer);
     }
 
 enable_discards_exit:
@@ -667,9 +666,8 @@ static void vfio_disconnect_container(VFIOGroup *group)
      */
     if (QLIST_EMPTY(&container->group_list)) {
         memory_listener_unregister(&bcontainer->listener);
-        if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
-            container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
-            vfio_spapr_container_deinit(container);
+        if (bcontainer->ops->release) {
+            bcontainer->ops->release(bcontainer);
         }
     }
 
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 5c6426e6973bec606667ebcaca5b0585b184a214..44617dfc6b5f1a2a3a1c37436b76042aebda8b63 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -440,6 +440,24 @@ vfio_spapr_container_del_section_window(VFIOContainerBase *bcontainer,
     }
 }
 
+static void vfio_spapr_container_release(VFIOContainerBase *bcontainer)
+{
+    VFIOContainer *container = container_of(bcontainer, VFIOContainer,
+                                            bcontainer);
+    VFIOSpaprContainer *scontainer = container_of(container, VFIOSpaprContainer,
+                                                  container);
+    VFIOHostDMAWindow *hostwin, *next;
+
+    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+        memory_listener_unregister(&scontainer->prereg_listener);
+    }
+    QLIST_FOREACH_SAFE(hostwin, &scontainer->hostwin_list, hostwin_next,
+                       next) {
+        QLIST_REMOVE(hostwin, hostwin_next);
+        g_free(hostwin);
+    }
+}
+
 static VFIOIOMMUOps vfio_iommu_spapr_ops;
 
 static void setup_spapr_ops(VFIOContainerBase *bcontainer)
@@ -447,6 +465,7 @@ 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;
 }
 
@@ -527,19 +546,3 @@ listener_unregister_exit:
     }
     return ret;
 }
-
-void vfio_spapr_container_deinit(VFIOContainer *container)
-{
-    VFIOSpaprContainer *scontainer = container_of(container, VFIOSpaprContainer,
-                                                  container);
-    VFIOHostDMAWindow *hostwin, *next;
-
-    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
-        memory_listener_unregister(&scontainer->prereg_listener);
-    }
-    QLIST_FOREACH_SAFE(hostwin, &scontainer->hostwin_list, hostwin_next,
-                       next) {
-        QLIST_REMOVE(hostwin, hostwin_next);
-        g_free(hostwin);
-    }
-}
-- 
2.43.0

RE: [PATCH for-9.0 01/10] vfio/spapr: Extend VFIOIOMMUOps with a release handler

[Duan, Zhenzhong]

Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 01/10] vfio/spapr: Extend VFIOIOMMUOps with a
>release handler
>
>This allows to abstract a bit more the sPAPR IOMMU support in the
>legacy IOMMU backend.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

[PATCH for-9.0 02/10] vfio/container: Introduce vfio_legacy_setup() for further cleanups

[Cédric Le Goater]

This will help subsequent patches to unify the initialization of type1
and sPAPR IOMMU backends.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/container.c | 63 +++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 1e77a2929e90ed1d2ee84062549c477ae651c5a8..afcfe8048805c58291d1104ff0ef20bdc457f99c 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -474,6 +474,35 @@ static void vfio_get_iommu_info_migration(VFIOContainer *container,
     }
 }
 
+static int vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
+{
+    VFIOContainer *container = container_of(bcontainer, VFIOContainer,
+                                            bcontainer);
+    g_autofree struct vfio_iommu_type1_info *info = NULL;
+    int ret;
+
+    ret = vfio_get_iommu_info(container, &info);
+    if (ret) {
+        error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
+        return ret;
+    }
+
+    if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
+        bcontainer->pgsizes = info->iova_pgsizes;
+    } else {
+        bcontainer->pgsizes = qemu_real_host_page_size();
+    }
+
+    if (!vfio_get_info_dma_avail(info, &bcontainer->dma_max_mappings)) {
+        bcontainer->dma_max_mappings = 65535;
+    }
+
+    vfio_get_info_iova_range(info, bcontainer);
+
+    vfio_get_iommu_info_migration(container, info);
+    return 0;
+}
+
 static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
                                   Error **errp)
 {
@@ -570,40 +599,18 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     switch (container->iommu_type) {
     case VFIO_TYPE1v2_IOMMU:
     case VFIO_TYPE1_IOMMU:
-    {
-        struct vfio_iommu_type1_info *info;
-
-        ret = vfio_get_iommu_info(container, &info);
-        if (ret) {
-            error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
-            goto enable_discards_exit;
-        }
-
-        if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
-            bcontainer->pgsizes = info->iova_pgsizes;
-        } else {
-            bcontainer->pgsizes = qemu_real_host_page_size();
-        }
-
-        if (!vfio_get_info_dma_avail(info, &bcontainer->dma_max_mappings)) {
-            bcontainer->dma_max_mappings = 65535;
-        }
-
-        vfio_get_info_iova_range(info, bcontainer);
-
-        vfio_get_iommu_info_migration(container, info);
-        g_free(info);
+        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);
-        if (ret) {
-            goto enable_discards_exit;
-        }
         break;
+    default:
+        g_assert_not_reached();
     }
+
+    if (ret) {
+        goto enable_discards_exit;
     }
 
     vfio_kvm_device_add_group(group);
-- 
2.43.0

RE: [PATCH for-9.0 02/10] vfio/container: Introduce vfio_legacy_setup() for further cleanups

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 02/10] vfio/container: Introduce vfio_legacy_setup()
>for further cleanups
>
>This will help subsequent patches to unify the initialization of type1
>and sPAPR IOMMU backends.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

[PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

[Cédric Le Goater]

vfio_init_container() already defines the IOMMU type of the container.
Do the same for the VFIOIOMMUOps struct. This prepares ground for the
following patches that will deduce the associated VFIOIOMMUOps struct
from the IOMMU type.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/container.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8879cb98e686afccc 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer *container,
 }
 
 static int vfio_init_container(VFIOContainer *container, int group_fd,
-                               Error **errp)
+                               VFIOAddressSpace *space, Error **errp)
 {
     int iommu_type, ret;
 
@@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer *container, int group_fd,
     }
 
     container->iommu_type = iommu_type;
+    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
     return 0;
 }
 
@@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     container = g_malloc0(sizeof(*container));
     container->fd = fd;
     bcontainer = &container->bcontainer;
-    vfio_container_init(bcontainer, space, &vfio_legacy_ops);
 
-    ret = vfio_init_container(container, group->fd, errp);
+    ret = vfio_init_container(container, group->fd, space, errp);
     if (ret) {
         goto free_container_exit;
     }
-- 
2.43.0

RE: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>under vfio_init_container()
>
>vfio_init_container() already defines the IOMMU type of the container.
>Do the same for the VFIOIOMMUOps struct. This prepares ground for the
>following patches that will deduce the associated VFIOIOMMUOps struct
>from the IOMMU type.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>
>---
> hw/vfio/container.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8
>879cb98e686afccc 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer
>*container,
> }
>
> static int vfio_init_container(VFIOContainer *container, int group_fd,
>-                               Error **errp)
>+                               VFIOAddressSpace *space, Error **errp)
> {
>     int iommu_type, ret;
>
>@@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer
>*container, int group_fd,
>     }
>
>     container->iommu_type = iommu_type;
>+    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Not related to this patch, not clear if it's deserved to rename
vfio_container_init as vfio_bcontainer_init to distinguish
with vfio_init_container.

Thanks
Zhenzhong

>     return 0;
> }
>
>@@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup *group,
>AddressSpace *as,
>     container = g_malloc0(sizeof(*container));
>     container->fd = fd;
>     bcontainer = &container->bcontainer;
>-    vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>
>-    ret = vfio_init_container(container, group->fd, errp);
>+    ret = vfio_init_container(container, group->fd, space, errp);
>     if (ret) {
>         goto free_container_exit;
>     }
>--
>2.43.0

Re: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

[Cédric Le Goater]

On 12/11/23 06:59, 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 03/10] vfio/container: Initialize VFIOIOMMUOps
>> under vfio_init_container()
>>
>> vfio_init_container() already defines the IOMMU type of the container.
>> Do the same for the VFIOIOMMUOps struct. This prepares ground for the
>> following patches that will deduce the associated VFIOIOMMUOps struct
>>from the IOMMU type.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> hw/vfio/container.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index
>> afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8
>> 879cb98e686afccc 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer
>> *container,
>> }
>>
>> static int vfio_init_container(VFIOContainer *container, int group_fd,
>> -                               Error **errp)
>> +                               VFIOAddressSpace *space, Error **errp)
>> {
>>      int iommu_type, ret;
>>
>> @@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer
>> *container, int group_fd,
>>      }
>>
>>      container->iommu_type = iommu_type;
>> +    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
> 
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> Not related to this patch, not clear if it's deserved to rename
> vfio_container_init as vfio_bcontainer_init to distinguish
> with vfio_init_container.

I agree, the vfio_container_init() and vfio_init_container() names
are confusing. I would keep vfio_container_init() because it is
consistent with all routines handling 'VFIOContainerBase *' ops.

I would be tempted to rename vfio_init_container() to vfio_set_iommu() ?

Also, I will introduce a vfio_connect_setup() helper in v2 doing the
assert as the other routines.

Thanks,

C.





> 
> Thanks
> Zhenzhong
> 
>>      return 0;
>> }
>>
>> @@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup *group,
>> AddressSpace *as,
>>      container = g_malloc0(sizeof(*container));
>>      container->fd = fd;
>>      bcontainer = &container->bcontainer;
>> -    vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>>
>> -    ret = vfio_init_container(container, group->fd, errp);
>> +    ret = vfio_init_container(container, group->fd, space, errp);
>>      if (ret) {
>>          goto free_container_exit;
>>      }
>> --
>> 2.43.0
> 

RE: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>under vfio_init_container()
>
>On 12/11/23 06:59, 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 03/10] vfio/container: Initialize VFIOIOMMUOps
>>> under vfio_init_container()
>>>
>>> vfio_init_container() already defines the IOMMU type of the container.
>>> Do the same for the VFIOIOMMUOps struct. This prepares ground for the
>>> following patches that will deduce the associated VFIOIOMMUOps struct
>>>from the IOMMU type.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>> ---
>>> hw/vfio/container.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>> index
>>>
>afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8
>>> 879cb98e686afccc 100644
>>> --- a/hw/vfio/container.c
>>> +++ b/hw/vfio/container.c
>>> @@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer
>>> *container,
>>> }
>>>
>>> static int vfio_init_container(VFIOContainer *container, int group_fd,
>>> -                               Error **errp)
>>> +                               VFIOAddressSpace *space, Error **errp)
>>> {
>>>      int iommu_type, ret;
>>>
>>> @@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer
>>> *container, int group_fd,
>>>      }
>>>
>>>      container->iommu_type = iommu_type;
>>> +    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
>>
>> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>
>> Not related to this patch, not clear if it's deserved to rename
>> vfio_container_init as vfio_bcontainer_init to distinguish
>> with vfio_init_container.
>
>I agree, the vfio_container_init() and vfio_init_container() names
>are confusing. I would keep vfio_container_init() because it is
>consistent with all routines handling 'VFIOContainerBase *' ops.

Oh, yes, that's better.

>
>I would be tempted to rename vfio_init_container() to vfio_set_iommu() ?

Sounds good.

Thanks
Zhenzhong

>
>Also, I will introduce a vfio_connect_setup() helper in v2 doing the
>assert as the other routines.
>
>Thanks,
>
>C.
>
>
>
>
>
>>
>> Thanks
>> Zhenzhong
>>
>>>      return 0;
>>> }
>>>
>>> @@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup
>*group,
>>> AddressSpace *as,
>>>      container = g_malloc0(sizeof(*container));
>>>      container->fd = fd;
>>>      bcontainer = &container->bcontainer;
>>> -    vfio_container_init(bcontainer, space, &vfio_legacy_ops);
>>>
>>> -    ret = vfio_init_container(container, group->fd, errp);
>>> +    ret = vfio_init_container(container, group->fd, space, errp);
>>>      if (ret) {
>>>          goto free_container_exit;
>>>      }
>>> --
>>> 2.43.0
>>

[PATCH for-9.0 04/10] vfio/container: Introduce a VFIOIOMMU QOM interface

[Cédric Le Goater]

Simply transform the VFIOIOMMUOps struct in an InterfaceClass and do
some initial name replacements. Next changes will start converting
VFIOIOMMUOps.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/vfio/vfio-container-base.h | 18 ++++++++++++++----
 hw/vfio/common.c                      |  2 +-
 hw/vfio/container-base.c              | 12 +++++++++++-
 hw/vfio/pci.c                         |  2 +-
 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 5c9594b6c77681e5593236e711e7e391e5f2bdff..81d49fe562d3840859096dd8a62ac38d62314939 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -16,7 +16,8 @@
 #include "exec/memory.h"
 
 typedef struct VFIODevice VFIODevice;
-typedef struct VFIOIOMMUOps VFIOIOMMUOps;
+typedef struct VFIOIOMMUClass VFIOIOMMUClass;
+#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
 
 typedef struct {
     unsigned long *bitmap;
@@ -34,7 +35,7 @@ typedef struct VFIOAddressSpace {
  * This is the base object for vfio container backends
  */
 typedef struct VFIOContainerBase {
-    const VFIOIOMMUOps *ops;
+    const VFIOIOMMUClass *ops;
     VFIOAddressSpace *space;
     MemoryListener listener;
     Error *error;
@@ -88,10 +89,19 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
 
 void vfio_container_init(VFIOContainerBase *bcontainer,
                          VFIOAddressSpace *space,
-                         const VFIOIOMMUOps *ops);
+                         const VFIOIOMMUClass *ops);
 void vfio_container_destroy(VFIOContainerBase *bcontainer);
 
-struct VFIOIOMMUOps {
+typedef struct VFIOIOMMU VFIOIOMMU;
+
+#define TYPE_VFIO_IOMMU "vfio-iommu"
+
+#define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj), TYPE_VFIO_IOMMU)
+DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU, TYPE_VFIO_IOMMU)
+
+struct VFIOIOMMUClass {
+    InterfaceClass parent_class;
+
     /* basic feature */
     int (*dma_map)(const VFIOContainerBase *bcontainer,
                    hwaddr iova, ram_addr_t size,
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 08a3e576725b1fc9f2f7e425375df3b827c4fe56..49dab41566f07ba7be1100fed1973e028d34467c 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1503,7 +1503,7 @@ retry:
 int vfio_attach_device(char *name, VFIODevice *vbasedev,
                        AddressSpace *as, Error **errp)
 {
-    const VFIOIOMMUOps *ops = &vfio_legacy_ops;
+    const VFIOIOMMUClass *ops = &vfio_legacy_ops;
 
 #ifdef CONFIG_IOMMUFD
     if (vbasedev->iommufd) {
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index 1ffd25bbfa8bd3d404e43b96357273b95f5a0031..913ae49077c4f09b7b27517c1231cfbe4befb7fb 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -72,7 +72,7 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
 }
 
 void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace *space,
-                         const VFIOIOMMUOps *ops)
+                         const VFIOIOMMUClass *ops)
 {
     bcontainer->ops = ops;
     bcontainer->space = space;
@@ -99,3 +99,13 @@ void vfio_container_destroy(VFIOContainerBase *bcontainer)
 
     g_list_free_full(bcontainer->iova_ranges, g_free);
 }
+
+static const TypeInfo types[] = {
+    {
+        .name = TYPE_VFIO_IOMMU,
+        .parent = TYPE_INTERFACE,
+        .class_size = sizeof(VFIOIOMMUClass),
+    },
+};
+
+DEFINE_TYPES(types)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1874ec1aba987cac6cb83f86650e7a5e1968c327..d84a9e73a65de4e4c1cdaf65619a700bd8d6b802 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2488,7 +2488,7 @@ int vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
     VFIODevice *vbasedev = &vdev->vbasedev;
-    const VFIOIOMMUOps *ops = vbasedev->bcontainer->ops;
+    const VFIOIOMMUClass *ops = vbasedev->bcontainer->ops;
 
     return ops->pci_hot_reset(vbasedev, single);
 }
-- 
2.43.0

RE: [PATCH for-9.0 04/10] vfio/container: Introduce a VFIOIOMMU QOM interface

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 04/10] vfio/container: Introduce a VFIOIOMMU
>QOM interface
>
>Simply transform the VFIOIOMMUOps struct in an InterfaceClass and do
>some initial name replacements. Next changes will start converting
>VFIOIOMMUOps.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>
>---
> include/hw/vfio/vfio-container-base.h | 18 ++++++++++++++----
> hw/vfio/common.c                      |  2 +-
> hw/vfio/container-base.c              | 12 +++++++++++-
> hw/vfio/pci.c                         |  2 +-
> 4 files changed, 27 insertions(+), 7 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>5c9594b6c77681e5593236e711e7e391e5f2bdff..81d49fe562d3840859096
>dd8a62ac38d62314939 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -16,7 +16,8 @@
> #include "exec/memory.h"
>
> typedef struct VFIODevice VFIODevice;
>-typedef struct VFIOIOMMUOps VFIOIOMMUOps;
>+typedef struct VFIOIOMMUClass VFIOIOMMUClass;
>+#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
>
> typedef struct {
>     unsigned long *bitmap;
>@@ -34,7 +35,7 @@ typedef struct VFIOAddressSpace {
>  * This is the base object for vfio container backends
>  */
> typedef struct VFIOContainerBase {
>-    const VFIOIOMMUOps *ops;
>+    const VFIOIOMMUClass *ops;
>     VFIOAddressSpace *space;
>     MemoryListener listener;
>     Error *error;
>@@ -88,10 +89,19 @@ int vfio_container_query_dirty_bitmap(const
>VFIOContainerBase *bcontainer,
>
> void vfio_container_init(VFIOContainerBase *bcontainer,
>                          VFIOAddressSpace *space,
>-                         const VFIOIOMMUOps *ops);
>+                         const VFIOIOMMUClass *ops);
> void vfio_container_destroy(VFIOContainerBase *bcontainer);
>
>-struct VFIOIOMMUOps {
>+typedef struct VFIOIOMMU VFIOIOMMU;
>+
>+#define TYPE_VFIO_IOMMU "vfio-iommu"
>+
>+#define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>TYPE_VFIO_IOMMU)

Maybe this #define can be removed or you have other plans?
Otherwise, Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>+DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>TYPE_VFIO_IOMMU)
>+
>+struct VFIOIOMMUClass {
>+    InterfaceClass parent_class;
>+
>     /* basic feature */
>     int (*dma_map)(const VFIOContainerBase *bcontainer,
>                    hwaddr iova, ram_addr_t size,
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index
>08a3e576725b1fc9f2f7e425375df3b827c4fe56..49dab41566f07ba7be1100f
>ed1973e028d34467c 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1503,7 +1503,7 @@ retry:
> int vfio_attach_device(char *name, VFIODevice *vbasedev,
>                        AddressSpace *as, Error **errp)
> {
>-    const VFIOIOMMUOps *ops = &vfio_legacy_ops;
>+    const VFIOIOMMUClass *ops = &vfio_legacy_ops;
>
> #ifdef CONFIG_IOMMUFD
>     if (vbasedev->iommufd) {
>diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
>index
>1ffd25bbfa8bd3d404e43b96357273b95f5a0031..913ae49077c4f09b7b275
>17c1231cfbe4befb7fb 100644
>--- a/hw/vfio/container-base.c
>+++ b/hw/vfio/container-base.c
>@@ -72,7 +72,7 @@ int vfio_container_query_dirty_bitmap(const
>VFIOContainerBase *bcontainer,
> }
>
> void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace
>*space,
>-                         const VFIOIOMMUOps *ops)
>+                         const VFIOIOMMUClass *ops)
> {
>     bcontainer->ops = ops;
>     bcontainer->space = space;
>@@ -99,3 +99,13 @@ void vfio_container_destroy(VFIOContainerBase
>*bcontainer)
>
>     g_list_free_full(bcontainer->iova_ranges, g_free);
> }
>+
>+static const TypeInfo types[] = {
>+    {
>+        .name = TYPE_VFIO_IOMMU,
>+        .parent = TYPE_INTERFACE,
>+        .class_size = sizeof(VFIOIOMMUClass),
>+    },
>+};
>+
>+DEFINE_TYPES(types)
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>index
>1874ec1aba987cac6cb83f86650e7a5e1968c327..d84a9e73a65de4e4c1cdaf
>65619a700bd8d6b802 100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -2488,7 +2488,7 @@ int
>vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
> static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
> {
>     VFIODevice *vbasedev = &vdev->vbasedev;
>-    const VFIOIOMMUOps *ops = vbasedev->bcontainer->ops;
>+    const VFIOIOMMUClass *ops = vbasedev->bcontainer->ops;
>
>     return ops->pci_hot_reset(vbasedev, single);
> }
>--
>2.43.0

Re: [PATCH for-9.0 04/10] vfio/container: Introduce a VFIOIOMMU QOM interface

[Cédric Le Goater]

On 12/11/23 07:08, 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 04/10] vfio/container: Introduce a VFIOIOMMU
>> QOM interface
>>
>> Simply transform the VFIOIOMMUOps struct in an InterfaceClass and do
>> some initial name replacements. Next changes will start converting
>> VFIOIOMMUOps.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> include/hw/vfio/vfio-container-base.h | 18 ++++++++++++++----
>> hw/vfio/common.c                      |  2 +-
>> hw/vfio/container-base.c              | 12 +++++++++++-
>> hw/vfio/pci.c                         |  2 +-
>> 4 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>> container-base.h
>> index
>> 5c9594b6c77681e5593236e711e7e391e5f2bdff..81d49fe562d3840859096
>> dd8a62ac38d62314939 100644
>> --- a/include/hw/vfio/vfio-container-base.h
>> +++ b/include/hw/vfio/vfio-container-base.h
>> @@ -16,7 +16,8 @@
>> #include "exec/memory.h"
>>
>> typedef struct VFIODevice VFIODevice;
>> -typedef struct VFIOIOMMUOps VFIOIOMMUOps;
>> +typedef struct VFIOIOMMUClass VFIOIOMMUClass;
>> +#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
>>
>> typedef struct {
>>      unsigned long *bitmap;
>> @@ -34,7 +35,7 @@ typedef struct VFIOAddressSpace {
>>   * This is the base object for vfio container backends
>>   */
>> typedef struct VFIOContainerBase {
>> -    const VFIOIOMMUOps *ops;
>> +    const VFIOIOMMUClass *ops;
>>      VFIOAddressSpace *space;
>>      MemoryListener listener;
>>      Error *error;
>> @@ -88,10 +89,19 @@ int vfio_container_query_dirty_bitmap(const
>> VFIOContainerBase *bcontainer,
>>
>> void vfio_container_init(VFIOContainerBase *bcontainer,
>>                           VFIOAddressSpace *space,
>> -                         const VFIOIOMMUOps *ops);
>> +                         const VFIOIOMMUClass *ops);
>> void vfio_container_destroy(VFIOContainerBase *bcontainer);
>>
>> -struct VFIOIOMMUOps {
>> +typedef struct VFIOIOMMU VFIOIOMMU;
>> +
>> +#define TYPE_VFIO_IOMMU "vfio-iommu"
>> +
>> +#define VFIO_IOMMU(obj) INTERFACE_CHECK(VFIOIOMMU, (obj),
>> TYPE_VFIO_IOMMU)
> 
> Maybe this #define can be removed or you have other plans?

yes, and we can remove 'struct VFIOIOMMU' also.


Thanks,

C.




> Otherwise, Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> Thanks
> Zhenzhong
> 
>> +DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU,
>> TYPE_VFIO_IOMMU)
>> +
>> +struct VFIOIOMMUClass {
>> +    InterfaceClass parent_class;
>> +
>>      /* basic feature */
>>      int (*dma_map)(const VFIOContainerBase *bcontainer,
>>                     hwaddr iova, ram_addr_t size,
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index
>> 08a3e576725b1fc9f2f7e425375df3b827c4fe56..49dab41566f07ba7be1100f
>> ed1973e028d34467c 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -1503,7 +1503,7 @@ retry:
>> int vfio_attach_device(char *name, VFIODevice *vbasedev,
>>                         AddressSpace *as, Error **errp)
>> {
>> -    const VFIOIOMMUOps *ops = &vfio_legacy_ops;
>> +    const VFIOIOMMUClass *ops = &vfio_legacy_ops;
>>
>> #ifdef CONFIG_IOMMUFD
>>      if (vbasedev->iommufd) {
>> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
>> index
>> 1ffd25bbfa8bd3d404e43b96357273b95f5a0031..913ae49077c4f09b7b275
>> 17c1231cfbe4befb7fb 100644
>> --- a/hw/vfio/container-base.c
>> +++ b/hw/vfio/container-base.c
>> @@ -72,7 +72,7 @@ int vfio_container_query_dirty_bitmap(const
>> VFIOContainerBase *bcontainer,
>> }
>>
>> void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace
>> *space,
>> -                         const VFIOIOMMUOps *ops)
>> +                         const VFIOIOMMUClass *ops)
>> {
>>      bcontainer->ops = ops;
>>      bcontainer->space = space;
>> @@ -99,3 +99,13 @@ void vfio_container_destroy(VFIOContainerBase
>> *bcontainer)
>>
>>      g_list_free_full(bcontainer->iova_ranges, g_free);
>> }
>> +
>> +static const TypeInfo types[] = {
>> +    {
>> +        .name = TYPE_VFIO_IOMMU,
>> +        .parent = TYPE_INTERFACE,
>> +        .class_size = sizeof(VFIOIOMMUClass),
>> +    },
>> +};
>> +
>> +DEFINE_TYPES(types)
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index
>> 1874ec1aba987cac6cb83f86650e7a5e1968c327..d84a9e73a65de4e4c1cdaf
>> 65619a700bd8d6b802 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2488,7 +2488,7 @@ int
>> vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
>> static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
>> {
>>      VFIODevice *vbasedev = &vdev->vbasedev;
>> -    const VFIOIOMMUOps *ops = vbasedev->bcontainer->ops;
>> +    const VFIOIOMMUClass *ops = vbasedev->bcontainer->ops;
>>
>>      return ops->pci_hot_reset(vbasedev, single);
>> }
>> --
>> 2.43.0
> 

[PATCH for-9.0 05/10] vfio/container: Introduce a VFIOIOMMU legacy QOM interface

[Cédric Le Goater]

Convert the legacy VFIOIOMMUOps struct to the new VFIOIOMMU QOM
interface. The set of of operations for this backend can be referenced
with a literal typename instead of a C struct. This will simplify
support of multiple backends.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/vfio/vfio-common.h         |  1 -
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/common.c                      |  6 ++-
 hw/vfio/container.c                   | 59 +++++++++++++++++++++++----
 4 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b8aa8a549532442a31c8e85ce385c992d84f6bd5..14c497b6b0a79466e8f567aceed384ec2c75ea90 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
 typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
 extern VFIOGroupList vfio_group_list;
 extern VFIODeviceList vfio_device_list;
-extern const VFIOIOMMUOps vfio_legacy_ops;
 extern const VFIOIOMMUOps vfio_iommufd_ops;
 extern const MemoryListener vfio_memory_listener;
 extern int vfio_kvm_device_fd;
diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 81d49fe562d3840859096dd8a62ac38d62314939..a31fd9c2e3b9a571083ea8987ac27e91b332c170 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -95,6 +95,7 @@ void vfio_container_destroy(VFIOContainerBase *bcontainer);
 typedef struct VFIOIOMMU VFIOIOMMU;
 
 #define TYPE_VFIO_IOMMU "vfio-iommu"
+#define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
 
 #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/common.c b/hw/vfio/common.c
index 49dab41566f07ba7be1100fed1973e028d34467c..2329d0efc8c1d617f0bfee5283e82b295d2d477d 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1503,13 +1503,17 @@ retry:
 int vfio_attach_device(char *name, VFIODevice *vbasedev,
                        AddressSpace *as, Error **errp)
 {
-    const VFIOIOMMUClass *ops = &vfio_legacy_ops;
+    const VFIOIOMMUClass *ops =
+        VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
 
 #ifdef CONFIG_IOMMUFD
     if (vbasedev->iommufd) {
         ops = &vfio_iommufd_ops;
     }
 #endif
+
+    assert(ops);
+
     return ops->attach_device(name, vbasedev, as, errp);
 }
 
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index f4a0434a5239bfb6a17b91c8879cb98e686afccc..fdf4e116570013732d48071a5122d25b02da715c 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -369,10 +369,30 @@ static int vfio_get_iommu_type(VFIOContainer *container,
     return -EINVAL;
 }
 
+/*
+ * vfio_get_iommu_ops - get a VFIOIOMMUClass associated with a type
+ */
+static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type, Error **errp)
+{
+    ObjectClass *klass = NULL;
+
+    switch (iommu_type) {
+    case VFIO_TYPE1v2_IOMMU:
+    case VFIO_TYPE1_IOMMU:
+        klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
+        break;
+    default:
+        g_assert_not_reached();
+    };
+
+    return VFIO_IOMMU_CLASS(klass);
+}
+
 static int vfio_init_container(VFIOContainer *container, int group_fd,
                                VFIOAddressSpace *space, Error **errp)
 {
     int iommu_type, ret;
+    const VFIOIOMMUClass *vioc = NULL;
 
     iommu_type = vfio_get_iommu_type(container, errp);
     if (iommu_type < 0) {
@@ -401,7 +421,14 @@ static int vfio_init_container(VFIOContainer *container, int group_fd,
     }
 
     container->iommu_type = iommu_type;
-    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
+
+    vioc = vfio_get_iommu_class(iommu_type, errp);
+    if (!vioc) {
+        error_setg(errp, "No available IOMMU models");
+        return -EINVAL;
+    }
+
+    vfio_container_init(&container->bcontainer, space, vioc);
     return 0;
 }
 
@@ -1098,12 +1125,26 @@ out_single:
     return ret;
 }
 
-const VFIOIOMMUOps vfio_legacy_ops = {
-    .dma_map = vfio_legacy_dma_map,
-    .dma_unmap = vfio_legacy_dma_unmap,
-    .attach_device = vfio_legacy_attach_device,
-    .detach_device = vfio_legacy_detach_device,
-    .set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
-    .query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
-    .pci_hot_reset = vfio_legacy_pci_hot_reset,
+static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
+{
+    VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
+
+    vioc->dma_map = vfio_legacy_dma_map;
+    vioc->dma_unmap = vfio_legacy_dma_unmap;
+    vioc->attach_device = vfio_legacy_attach_device;
+    vioc->detach_device = vfio_legacy_detach_device;
+    vioc->set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking;
+    vioc->query_dirty_bitmap = vfio_legacy_query_dirty_bitmap;
+    vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
 };
+
+static const TypeInfo types[] = {
+    {
+        .name = TYPE_VFIO_IOMMU_LEGACY,
+        .parent = TYPE_VFIO_IOMMU,
+        .class_init = vfio_iommu_legacy_class_init,
+        .class_size = sizeof(VFIOIOMMUClass),
+    },
+};
+
+DEFINE_TYPES(types)
-- 
2.43.0

RE: [PATCH for-9.0 05/10] vfio/container: Introduce a VFIOIOMMU legacy QOM interface

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 05/10] vfio/container: Introduce a VFIOIOMMU
>legacy QOM interface
>
>Convert the legacy VFIOIOMMUOps struct to the new VFIOIOMMU QOM
>interface. The set of of operations for this backend can be referenced
>with a literal typename instead of a C struct. This will simplify
>support of multiple backends.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>
>---
> include/hw/vfio/vfio-common.h         |  1 -
> include/hw/vfio/vfio-container-base.h |  1 +
> hw/vfio/common.c                      |  6 ++-
> hw/vfio/container.c                   | 59 +++++++++++++++++++++++----
> 4 files changed, 56 insertions(+), 11 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index
>b8aa8a549532442a31c8e85ce385c992d84f6bd5..14c497b6b0a79466e8f56
>7aceed384ec2c75ea90 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup)
>VFIOGroupList;
> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
> extern VFIOGroupList vfio_group_list;
> extern VFIODeviceList vfio_device_list;
>-extern const VFIOIOMMUOps vfio_legacy_ops;
> extern const VFIOIOMMUOps vfio_iommufd_ops;
> extern const MemoryListener vfio_memory_listener;
> extern int vfio_kvm_device_fd;
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>81d49fe562d3840859096dd8a62ac38d62314939..a31fd9c2e3b9a571083e
>a8987ac27e91b332c170 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -95,6 +95,7 @@ void vfio_container_destroy(VFIOContainerBase
>*bcontainer);
> typedef struct VFIOIOMMU VFIOIOMMU;
>
> #define TYPE_VFIO_IOMMU "vfio-iommu"
>+#define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>
> #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/common.c b/hw/vfio/common.c
>index
>49dab41566f07ba7be1100fed1973e028d34467c..2329d0efc8c1d617f0bfee
>5283e82b295d2d477d 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1503,13 +1503,17 @@ retry:
> int vfio_attach_device(char *name, VFIODevice *vbasedev,
>                        AddressSpace *as, Error **errp)
> {
>-    const VFIOIOMMUClass *ops = &vfio_legacy_ops;
>+    const VFIOIOMMUClass *ops =
>+
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
>
> #ifdef CONFIG_IOMMUFD
>     if (vbasedev->iommufd) {
>         ops = &vfio_iommufd_ops;
>     }
> #endif
>+
>+    assert(ops);
>+
>     return ops->attach_device(name, vbasedev, as, errp);
> }
>
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>f4a0434a5239bfb6a17b91c8879cb98e686afccc..fdf4e116570013732d4807
>1a5122d25b02da715c 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -369,10 +369,30 @@ static int vfio_get_iommu_type(VFIOContainer
>*container,
>     return -EINVAL;
> }
>
>+/*
>+ * vfio_get_iommu_ops - get a VFIOIOMMUClass associated with a type
>+ */
>+static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type,
>Error **errp)
>+{
>+    ObjectClass *klass = NULL;

No need to nullify?
>+
>+    switch (iommu_type) {
>+    case VFIO_TYPE1v2_IOMMU:
>+    case VFIO_TYPE1_IOMMU:
>+        klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
>+        break;
>+    default:
>+        g_assert_not_reached();
>+    };
>+
>+    return VFIO_IOMMU_CLASS(klass);
>+}
>+
> static int vfio_init_container(VFIOContainer *container, int group_fd,
>                                VFIOAddressSpace *space, Error **errp)
> {
>     int iommu_type, ret;
>+    const VFIOIOMMUClass *vioc = NULL;

No need to nullify?

>
>     iommu_type = vfio_get_iommu_type(container, errp);
>     if (iommu_type < 0) {
>@@ -401,7 +421,14 @@ static int vfio_init_container(VFIOContainer
>*container, int group_fd,
>     }
>
>     container->iommu_type = iommu_type;
>-    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
>+
>+    vioc = vfio_get_iommu_class(iommu_type, errp);
>+    if (!vioc) {
>+        error_setg(errp, "No available IOMMU models");
>+        return -EINVAL;
>+    }
>+
>+    vfio_container_init(&container->bcontainer, space, vioc);
>     return 0;
> }
>
>@@ -1098,12 +1125,26 @@ out_single:
>     return ret;
> }
>
>-const VFIOIOMMUOps vfio_legacy_ops = {
>-    .dma_map = vfio_legacy_dma_map,
>-    .dma_unmap = vfio_legacy_dma_unmap,
>-    .attach_device = vfio_legacy_attach_device,
>-    .detach_device = vfio_legacy_detach_device,
>-    .set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
>-    .query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
>-    .pci_hot_reset = vfio_legacy_pci_hot_reset,
>+static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
>+{
>+    VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>+
>+    vioc->dma_map = vfio_legacy_dma_map;
>+    vioc->dma_unmap = vfio_legacy_dma_unmap;
>+    vioc->attach_device = vfio_legacy_attach_device;
>+    vioc->detach_device = vfio_legacy_detach_device;
>+    vioc->set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking;
>+    vioc->query_dirty_bitmap = vfio_legacy_query_dirty_bitmap;
>+    vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
> };
>+
>+static const TypeInfo types[] = {
>+    {
>+        .name = TYPE_VFIO_IOMMU_LEGACY,
>+        .parent = TYPE_VFIO_IOMMU,
>+        .class_init = vfio_iommu_legacy_class_init,
>+        .class_size = sizeof(VFIOIOMMUClass),

Inherit parent class_size is enough? Otherwise,

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>+    },
>+};
>+
>+DEFINE_TYPES(types)
>--
>2.43.0

Re: [PATCH for-9.0 05/10] vfio/container: Introduce a VFIOIOMMU legacy QOM interface

[Cédric Le Goater]

On 12/11/23 07:14, 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 05/10] vfio/container: Introduce a VFIOIOMMU
>> legacy QOM interface
>>
>> Convert the legacy VFIOIOMMUOps struct to the new VFIOIOMMU QOM
>> interface. The set of of operations for this backend can be referenced
>> with a literal typename instead of a C struct. This will simplify
>> support of multiple backends.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>> include/hw/vfio/vfio-common.h         |  1 -
>> include/hw/vfio/vfio-container-base.h |  1 +
>> hw/vfio/common.c                      |  6 ++-
>> hw/vfio/container.c                   | 59 +++++++++++++++++++++++----
>> 4 files changed, 56 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>> common.h
>> index
>> b8aa8a549532442a31c8e85ce385c992d84f6bd5..14c497b6b0a79466e8f56
>> 7aceed384ec2c75ea90 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup)
>> VFIOGroupList;
>> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
>> extern VFIOGroupList vfio_group_list;
>> extern VFIODeviceList vfio_device_list;
>> -extern const VFIOIOMMUOps vfio_legacy_ops;
>> extern const VFIOIOMMUOps vfio_iommufd_ops;
>> extern const MemoryListener vfio_memory_listener;
>> extern int vfio_kvm_device_fd;
>> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>> container-base.h
>> index
>> 81d49fe562d3840859096dd8a62ac38d62314939..a31fd9c2e3b9a571083e
>> a8987ac27e91b332c170 100644
>> --- a/include/hw/vfio/vfio-container-base.h
>> +++ b/include/hw/vfio/vfio-container-base.h
>> @@ -95,6 +95,7 @@ void vfio_container_destroy(VFIOContainerBase
>> *bcontainer);
>> typedef struct VFIOIOMMU VFIOIOMMU;
>>
>> #define TYPE_VFIO_IOMMU "vfio-iommu"
>> +#define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
>>
>> #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/common.c b/hw/vfio/common.c
>> index
>> 49dab41566f07ba7be1100fed1973e028d34467c..2329d0efc8c1d617f0bfee
>> 5283e82b295d2d477d 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -1503,13 +1503,17 @@ retry:
>> int vfio_attach_device(char *name, VFIODevice *vbasedev,
>>                         AddressSpace *as, Error **errp)
>> {
>> -    const VFIOIOMMUClass *ops = &vfio_legacy_ops;
>> +    const VFIOIOMMUClass *ops =
>> +
>> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
>>
>> #ifdef CONFIG_IOMMUFD
>>      if (vbasedev->iommufd) {
>>          ops = &vfio_iommufd_ops;
>>      }
>> #endif
>> +
>> +    assert(ops);
>> +
>>      return ops->attach_device(name, vbasedev, as, errp);
>> }
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index
>> f4a0434a5239bfb6a17b91c8879cb98e686afccc..fdf4e116570013732d4807
>> 1a5122d25b02da715c 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -369,10 +369,30 @@ static int vfio_get_iommu_type(VFIOContainer
>> *container,
>>      return -EINVAL;
>> }
>>
>> +/*
>> + * vfio_get_iommu_ops - get a VFIOIOMMUClass associated with a type
>> + */
>> +static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type,
>> Error **errp)
>> +{
>> +    ObjectClass *klass = NULL;
> 
> No need to nullify?

well, I am not sure. Some compilers might complain. I will check.


>> +
>> +    switch (iommu_type) {
>> +    case VFIO_TYPE1v2_IOMMU:
>> +    case VFIO_TYPE1_IOMMU:
>> +        klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +    };
>> +
>> +    return VFIO_IOMMU_CLASS(klass);
>> +}
>> +
>> static int vfio_init_container(VFIOContainer *container, int group_fd,
>>                                 VFIOAddressSpace *space, Error **errp)
>> {
>>      int iommu_type, ret;
>> +    const VFIOIOMMUClass *vioc = NULL;
> 
> No need to nullify?

No need indeed.
  
>>
>>      iommu_type = vfio_get_iommu_type(container, errp);
>>      if (iommu_type < 0) {
>> @@ -401,7 +421,14 @@ static int vfio_init_container(VFIOContainer
>> *container, int group_fd,
>>      }
>>
>>      container->iommu_type = iommu_type;
>> -    vfio_container_init(&container->bcontainer, space, &vfio_legacy_ops);
>> +
>> +    vioc = vfio_get_iommu_class(iommu_type, errp);
>> +    if (!vioc) {
>> +        error_setg(errp, "No available IOMMU models");
>> +        return -EINVAL;
>> +    }
>> +
>> +    vfio_container_init(&container->bcontainer, space, vioc);
>>      return 0;
>> }
>>
>> @@ -1098,12 +1125,26 @@ out_single:
>>      return ret;
>> }
>>
>> -const VFIOIOMMUOps vfio_legacy_ops = {
>> -    .dma_map = vfio_legacy_dma_map,
>> -    .dma_unmap = vfio_legacy_dma_unmap,
>> -    .attach_device = vfio_legacy_attach_device,
>> -    .detach_device = vfio_legacy_detach_device,
>> -    .set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
>> -    .query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
>> -    .pci_hot_reset = vfio_legacy_pci_hot_reset,
>> +static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
>> +{
>> +    VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>> +
>> +    vioc->dma_map = vfio_legacy_dma_map;
>> +    vioc->dma_unmap = vfio_legacy_dma_unmap;
>> +    vioc->attach_device = vfio_legacy_attach_device;
>> +    vioc->detach_device = vfio_legacy_detach_device;
>> +    vioc->set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking;
>> +    vioc->query_dirty_bitmap = vfio_legacy_query_dirty_bitmap;
>> +    vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
>> };
>> +
>> +static const TypeInfo types[] = {
>> +    {
>> +        .name = TYPE_VFIO_IOMMU_LEGACY,
>> +        .parent = TYPE_VFIO_IOMMU,
>> +        .class_init = vfio_iommu_legacy_class_init,
>> +        .class_size = sizeof(VFIOIOMMUClass),
> 
> Inherit parent class_size is enough? Otherwise,

No need to define class_size again.


Thanks,

C.



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

[PATCH for-9.0 06/10] vfio/container: Intoduce a new VFIOIOMMUClass::setup handler

[Cédric Le Goater]

This will help in converting the sPAPR IOMMU backend to a QOM interface.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/vfio/vfio-container-base.h | 1 +
 hw/vfio/container.c                   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index a31fd9c2e3b9a571083ea8987ac27e91b332c170..870e7dc48e542ddbfc52e12b0ab5fab4771a1ebd 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -104,6 +104,7 @@ struct VFIOIOMMUClass {
     InterfaceClass parent_class;
 
     /* basic feature */
+    int (*setup)(VFIOContainerBase *bcontainer, Error **errp);
     int (*dma_map)(const VFIOContainerBase *bcontainer,
                    hwaddr iova, ram_addr_t size,
                    void *vaddr, bool readonly);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index fdf4e116570013732d48071a5122d25b02da715c..5f5ad8479f083db0be5207f179f3056ae67c49c3 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -1129,6 +1129,7 @@ static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
 {
     VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
 
+    vioc->setup = vfio_legacy_setup;
     vioc->dma_map = vfio_legacy_dma_map;
     vioc->dma_unmap = vfio_legacy_dma_unmap;
     vioc->attach_device = vfio_legacy_attach_device;
-- 
2.43.0

RE: [PATCH for-9.0 06/10] vfio/container: Intoduce a new VFIOIOMMUClass::setup handler

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 06/10] vfio/container: Intoduce a new
>VFIOIOMMUClass::setup handler
>
>This will help in converting the sPAPR IOMMU backend to a QOM interface.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>---
> include/hw/vfio/vfio-container-base.h | 1 +
> hw/vfio/container.c                   | 1 +
> 2 files changed, 2 insertions(+)
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>a31fd9c2e3b9a571083ea8987ac27e91b332c170..870e7dc48e542ddbfc52e
>12b0ab5fab4771a1ebd 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -104,6 +104,7 @@ struct VFIOIOMMUClass {
>     InterfaceClass parent_class;
>
>     /* basic feature */
>+    int (*setup)(VFIOContainerBase *bcontainer, Error **errp);
>     int (*dma_map)(const VFIOContainerBase *bcontainer,
>                    hwaddr iova, ram_addr_t size,
>                    void *vaddr, bool readonly);
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>fdf4e116570013732d48071a5122d25b02da715c..5f5ad8479f083db0be520
>7f179f3056ae67c49c3 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -1129,6 +1129,7 @@ static void
>vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
> {
>     VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>
>+    vioc->setup = vfio_legacy_setup;
>     vioc->dma_map = vfio_legacy_dma_map;
>     vioc->dma_unmap = vfio_legacy_dma_unmap;
>     vioc->attach_device = vfio_legacy_attach_device;
>--
>2.43.0

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

[Cédric Le Goater]

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..4012360c07b7c0a23f170f94a19455c79d3504b1 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..ce5a731ba74600fbb331a80f5148a88e2e43b068 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..46aa14bd2ae6d580c16bba75838cb6aca7d4047f 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),
+    },
+};
+
+DEFINE_TYPES(types)
-- 
2.43.0

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

[Duan, Zhenzhong]

>-----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,

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>+    },
>+};
>+
>+DEFINE_TYPES(types)
>--
>2.43.0

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

[Cédric Le Goater]

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
> 

[PATCH for-9.0 08/10] vfio/iommufd: Introduce a VFIOIOMMU iommufd QOM interface

[Cédric Le Goater]

As previously done for the sPAPR and legacy IOMMU backends, convert
the VFIOIOMMUOps struct to a QOM interface. The set of of operations
for this backend can be referenced with a literal typename instead of
a C struct.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 include/hw/vfio/vfio-common.h         |  1 -
 include/hw/vfio/vfio-container-base.h |  2 +-
 hw/vfio/common.c                      |  2 +-
 hw/vfio/iommufd.c                     | 36 ++++++++++++++++++++-------
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 14c497b6b0a79466e8f567aceed384ec2c75ea90..9b7ef7d02b5a0ad5266bcc4d06cd6874178978e4 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
 typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
 extern VFIOGroupList vfio_group_list;
 extern VFIODeviceList vfio_device_list;
-extern const VFIOIOMMUOps vfio_iommufd_ops;
 extern const MemoryListener vfio_memory_listener;
 extern int vfio_kvm_device_fd;
 
diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 4012360c07b7c0a23f170f94a19455c79d3504b1..5fd02fab5fd627dc6010685e9d956ba20ee329fd 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -17,7 +17,6 @@
 
 typedef struct VFIODevice VFIODevice;
 typedef struct VFIOIOMMUClass VFIOIOMMUClass;
-#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
 
 typedef struct {
     unsigned long *bitmap;
@@ -97,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 TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
 
 #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/common.c b/hw/vfio/common.c
index 2329d0efc8c1d617f0bfee5283e82b295d2d477d..89ff1c7aeda14d20b2e24f8bc251db0a71d4527c 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1508,7 +1508,7 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
 
 #ifdef CONFIG_IOMMUFD
     if (vbasedev->iommufd) {
-        ops = &vfio_iommufd_ops;
+        ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
     }
 #endif
 
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 87a561c54580adc6d7b2711331a00940ff13bd43..807be2545eb147a6fac973752cf37eeede1a4ff6 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -319,6 +319,8 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
     int ret, devfd;
     uint32_t ioas_id;
     Error *err = NULL;
+    const VFIOIOMMUClass *iommufd_vioc =
+        VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
 
     if (vbasedev->fd < 0) {
         devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
@@ -340,7 +342,7 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
     /* try to attach to an existing container in this space */
     QLIST_FOREACH(bcontainer, &space->containers, next) {
         container = container_of(bcontainer, VFIOIOMMUFDContainer, bcontainer);
-        if (bcontainer->ops != &vfio_iommufd_ops ||
+        if (bcontainer->ops != iommufd_vioc ||
             vbasedev->iommufd != container->be) {
             continue;
         }
@@ -374,7 +376,7 @@ static int iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
     container->ioas_id = ioas_id;
 
     bcontainer = &container->bcontainer;
-    vfio_container_init(bcontainer, space, &vfio_iommufd_ops);
+    vfio_container_init(bcontainer, space, iommufd_vioc);
     QLIST_INSERT_HEAD(&space->containers, bcontainer, next);
 
     ret = iommufd_cdev_attach_container(vbasedev, container, errp);
@@ -476,9 +478,11 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
 static VFIODevice *iommufd_cdev_pci_find_by_devid(__u32 devid)
 {
     VFIODevice *vbasedev_iter;
+    const VFIOIOMMUClass *iommufd_vioc =
+        VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
 
     QLIST_FOREACH(vbasedev_iter, &vfio_device_list, global_next) {
-        if (vbasedev_iter->bcontainer->ops != &vfio_iommufd_ops) {
+        if (vbasedev_iter->bcontainer->ops != iommufd_vioc) {
             continue;
         }
         if (devid == vbasedev_iter->devid) {
@@ -621,10 +625,24 @@ out_single:
     return ret;
 }
 
-const VFIOIOMMUOps vfio_iommufd_ops = {
-    .dma_map = iommufd_cdev_map,
-    .dma_unmap = iommufd_cdev_unmap,
-    .attach_device = iommufd_cdev_attach,
-    .detach_device = iommufd_cdev_detach,
-    .pci_hot_reset = iommufd_cdev_pci_hot_reset,
+static void vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
+{
+    VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
+
+    vioc->dma_map = iommufd_cdev_map;
+    vioc->dma_unmap = iommufd_cdev_unmap;
+    vioc->attach_device = iommufd_cdev_attach;
+    vioc->detach_device = iommufd_cdev_detach;
+    vioc->pci_hot_reset = iommufd_cdev_pci_hot_reset;
 };
+
+static const TypeInfo types[] = {
+    {
+        .name = TYPE_VFIO_IOMMU_IOMMUFD,
+        .parent = TYPE_VFIO_IOMMU,
+        .class_init = vfio_iommu_iommufd_class_init,
+        .class_size = sizeof(VFIOIOMMUClass),
+    },
+};
+
+DEFINE_TYPES(types)
-- 
2.43.0

RE: [PATCH for-9.0 08/10] vfio/iommufd: Introduce a VFIOIOMMU iommufd QOM interface

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 08/10] vfio/iommufd: Introduce a VFIOIOMMU
>iommufd QOM interface
>
>As previously done for the sPAPR and legacy IOMMU backends, convert
>the VFIOIOMMUOps struct to a QOM interface. The set of of operations
>for this backend can be referenced with a literal typename instead of
>a C struct.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>---
> include/hw/vfio/vfio-common.h         |  1 -
> include/hw/vfio/vfio-container-base.h |  2 +-
> hw/vfio/common.c                      |  2 +-
> hw/vfio/iommufd.c                     | 36 ++++++++++++++++++++-------
> 4 files changed, 29 insertions(+), 12 deletions(-)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index
>14c497b6b0a79466e8f567aceed384ec2c75ea90..9b7ef7d02b5a0ad5266bcc
>4d06cd6874178978e4 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup)
>VFIOGroupList;
> typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
> extern VFIOGroupList vfio_group_list;
> extern VFIODeviceList vfio_device_list;
>-extern const VFIOIOMMUOps vfio_iommufd_ops;
> extern const MemoryListener vfio_memory_listener;
> extern int vfio_kvm_device_fd;
>
>diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-
>container-base.h
>index
>4012360c07b7c0a23f170f94a19455c79d3504b1..5fd02fab5fd627dc601068
>5e9d956ba20ee329fd 100644
>--- a/include/hw/vfio/vfio-container-base.h
>+++ b/include/hw/vfio/vfio-container-base.h
>@@ -17,7 +17,6 @@
>
> typedef struct VFIODevice VFIODevice;
> typedef struct VFIOIOMMUClass VFIOIOMMUClass;
>-#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
>
> typedef struct {
>     unsigned long *bitmap;
>@@ -97,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 TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
>
> #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/common.c b/hw/vfio/common.c
>index
>2329d0efc8c1d617f0bfee5283e82b295d2d477d..89ff1c7aeda14d20b2e24f
>8bc251db0a71d4527c 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1508,7 +1508,7 @@ int vfio_attach_device(char *name, VFIODevice
>*vbasedev,
>
> #ifdef CONFIG_IOMMUFD
>     if (vbasedev->iommufd) {
>-        ops = &vfio_iommufd_ops;
>+        ops =
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
>     }
> #endif
>
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index
>87a561c54580adc6d7b2711331a00940ff13bd43..807be2545eb147a6fac97
>3752cf37eeede1a4ff6 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -319,6 +319,8 @@ static int iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
>     int ret, devfd;
>     uint32_t ioas_id;
>     Error *err = NULL;
>+    const VFIOIOMMUClass *iommufd_vioc =
>+
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
>
>     if (vbasedev->fd < 0) {
>         devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
>@@ -340,7 +342,7 @@ static int iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
>     /* try to attach to an existing container in this space */
>     QLIST_FOREACH(bcontainer, &space->containers, next) {
>         container = container_of(bcontainer, VFIOIOMMUFDContainer,
>bcontainer);
>-        if (bcontainer->ops != &vfio_iommufd_ops ||
>+        if (bcontainer->ops != iommufd_vioc ||
>             vbasedev->iommufd != container->be) {
>             continue;
>         }
>@@ -374,7 +376,7 @@ static int iommufd_cdev_attach(const char *name,
>VFIODevice *vbasedev,
>     container->ioas_id = ioas_id;
>
>     bcontainer = &container->bcontainer;
>-    vfio_container_init(bcontainer, space, &vfio_iommufd_ops);
>+    vfio_container_init(bcontainer, space, iommufd_vioc);
>     QLIST_INSERT_HEAD(&space->containers, bcontainer, next);
>
>     ret = iommufd_cdev_attach_container(vbasedev, container, errp);
>@@ -476,9 +478,11 @@ static void iommufd_cdev_detach(VFIODevice
>*vbasedev)
> static VFIODevice *iommufd_cdev_pci_find_by_devid(__u32 devid)
> {
>     VFIODevice *vbasedev_iter;
>+    const VFIOIOMMUClass *iommufd_vioc =
>+
>VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
>D));
>
>     QLIST_FOREACH(vbasedev_iter, &vfio_device_list, global_next) {
>-        if (vbasedev_iter->bcontainer->ops != &vfio_iommufd_ops) {
>+        if (vbasedev_iter->bcontainer->ops != iommufd_vioc) {
>             continue;
>         }
>         if (devid == vbasedev_iter->devid) {
>@@ -621,10 +625,24 @@ out_single:
>     return ret;
> }
>
>-const VFIOIOMMUOps vfio_iommufd_ops = {
>-    .dma_map = iommufd_cdev_map,
>-    .dma_unmap = iommufd_cdev_unmap,
>-    .attach_device = iommufd_cdev_attach,
>-    .detach_device = iommufd_cdev_detach,
>-    .pci_hot_reset = iommufd_cdev_pci_hot_reset,
>+static void vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
>+{
>+    VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
>+
>+    vioc->dma_map = iommufd_cdev_map;
>+    vioc->dma_unmap = iommufd_cdev_unmap;
>+    vioc->attach_device = iommufd_cdev_attach;
>+    vioc->detach_device = iommufd_cdev_detach;
>+    vioc->pci_hot_reset = iommufd_cdev_pci_hot_reset;
> };
>+
>+static const TypeInfo types[] = {
>+    {
>+        .name = TYPE_VFIO_IOMMU_IOMMUFD,
>+        .parent = TYPE_VFIO_IOMMU,
>+        .class_init = vfio_iommu_iommufd_class_init,
>+        .class_size = sizeof(VFIOIOMMUClass),
>+    },
>+};
>+
>+DEFINE_TYPES(types)
>--
>2.43.0

[PATCH for-9.0 09/10] vfio/spapr: Only compile sPAPR IOMMU support when needed

[Cédric Le Goater]

sPAPR IOMMU support is only needed for pseries machines. Compile out
support when CONFIG_PSERIES is not set. This saves ~7K of text.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index e5d98b6adc223061f6b0c3e1a7db3ba93d4eef16..bb98493b53e858c53181e224f9cb46892838a8be 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -4,9 +4,9 @@ vfio_ss.add(files(
   'common.c',
   'container-base.c',
   'container.c',
-  'spapr.c',
   'migration.c',
 ))
+vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
 vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
   'iommufd.c',
 ))
-- 
2.43.0

RE: [PATCH for-9.0 09/10] vfio/spapr: Only compile sPAPR IOMMU support when needed

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, December 8, 2023 4:46 PM
>Subject: [PATCH for-9.0 09/10] vfio/spapr: Only compile sPAPR IOMMU
>support when needed
>
>sPAPR IOMMU support is only needed for pseries machines. Compile out
>support when CONFIG_PSERIES is not set. This saves ~7K of text.
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>---
> hw/vfio/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
>index
>e5d98b6adc223061f6b0c3e1a7db3ba93d4eef16..bb98493b53e858c53181e
>224f9cb46892838a8be 100644
>--- a/hw/vfio/meson.build
>+++ b/hw/vfio/meson.build
>@@ -4,9 +4,9 @@ vfio_ss.add(files(
>   'common.c',
>   'container-base.c',
>   'container.c',
>-  'spapr.c',
>   'migration.c',
> ))
>+vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
> vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
>   'iommufd.c',
> ))
>--
>2.43.0

[PATCH for-9.0 10/10] vfio/iommufd: Remove CONFIG_IOMMUFD usage

[Cédric Le Goater]

Availability of the IOMMUFD backend can now be fully determined at
runtime and the ifdef check was a build time protection (for PPC not
supporting it mostly).

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/common.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 89ff1c7aeda14d20b2e24f8bc251db0a71d4527c..0d4d8b8416c6a4770677e1ebe5e1fc7dbaaef004 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -19,7 +19,6 @@
  */
 
 #include "qemu/osdep.h"
-#include CONFIG_DEVICES /* CONFIG_IOMMUFD */
 #include <sys/ioctl.h>
 #ifdef CONFIG_KVM
 #include <linux/kvm.h>
@@ -1506,11 +1505,9 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
     const VFIOIOMMUClass *ops =
         VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
 
-#ifdef CONFIG_IOMMUFD
     if (vbasedev->iommufd) {
         ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
     }
-#endif
 
     assert(ops);
 
-- 
2.43.0

RE: [PATCH for-9.0 10/10] vfio/iommufd: Remove CONFIG_IOMMUFD usage

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: [PATCH for-9.0 10/10] vfio/iommufd: Remove CONFIG_IOMMUFD
>usage
>
>Availability of the IOMMUFD backend can now be fully determined at
>runtime and the ifdef check was a build time protection (for PPC not
>supporting it mostly).
>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong