[v1] vfio: Introduce a VFIOIOMMUClass

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

Posted by Cédric Le Goater 11 months, 3 weeks ago

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

Posted by Duan, Zhenzhong 11 months, 3 weeks ago


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

Posted by Cédric Le Goater 11 months, 3 weeks ago

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
>