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

Cédric Le Goater posted 10 patches 11 months, 3 weeks ago
There is a newer version of this series
[PATCH for-9.0 07/10] vfio/spapr: Introduce a sPAPR VFIOIOMMU QOM interface
Posted by Cédric Le Goater 11 months, 3 weeks ago
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
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 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
Posted by Cédric Le Goater 11 months, 3 weeks ago
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
>