1. Patchew
  2. QEMU
  3. [PATCH v2 00/17] vfio: QOMify VFIOContainer
  4. View patch

[PATCH v2 06/17] vfio/container: Simplify vfio_container_init()

Cédric Le Goater posted 17 patches 10 months, 4 weeks ago
[PATCH v2 06/17] vfio/container: Simplify vfio_container_init()
Posted by Cédric Le Goater 10 months, 4 weeks ago 
Assign the base container VFIOAddressSpace 'space' pointer in
vfio_address_space_insert().

To be noted that vfio_connect_container() will assign the 'space'
pointer later in the execution flow. This should not have any
consequence.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-container-base.h | 1 -
 hw/vfio/common.c                      | 1 +
 hw/vfio/container-base.c              | 3 +--
 hw/vfio/container.c                   | 6 +++---
 hw/vfio/iommufd.c                     | 2 +-
 5 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 442c0dfc4c1774753c239c2c8360dcd1540d44fa..d505f63607ec40e6aa44aeb3e20848ac780562a1 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -87,7 +87,6 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
                    VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
 
 void vfio_container_init(VFIOContainerBase *bcontainer,
-                         VFIOAddressSpace *space,
                          const VFIOIOMMUClass *ops);
 void vfio_container_destroy(VFIOContainerBase *bcontainer);
 
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 8cdf26c6f5a490cfa02bdf1087a91948709aaa33..1686a0bed23bd95467bfb00a0c39a4d966e49cae 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1512,6 +1512,7 @@ void vfio_address_space_insert(VFIOAddressSpace *space,
                                VFIOContainerBase *bcontainer)
 {
     QLIST_INSERT_HEAD(&space->containers, bcontainer, next);
+    bcontainer->space = space;
 }
 
 struct vfio_device_info *vfio_get_device_info(int fd)
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index 760d9d0622b2e847ecb3368c88df772efb06043f..280f0dd2db1fc3939fe9925ce00a2c50d0e14196 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -71,11 +71,10 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
                                                errp);
 }
 
-void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace *space,
+void vfio_container_init(VFIOContainerBase *bcontainer,
                          const VFIOIOMMUClass *ops)
 {
     bcontainer->ops = ops;
-    bcontainer->space = space;
     bcontainer->error = NULL;
     bcontainer->dirty_pages_supported = false;
     bcontainer->dma_max_mappings = 0;
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 0237c216987ff64a6d11bef8688bb000d93a7f09..dc85a79cb9e62b72312f79da994c53608b6cef48 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -394,7 +394,7 @@ static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type, Error **errp)
 }
 
 static bool vfio_set_iommu(VFIOContainer *container, int group_fd,
-                           VFIOAddressSpace *space, Error **errp)
+                           Error **errp)
 {
     int iommu_type;
     const VFIOIOMMUClass *vioc;
@@ -432,7 +432,7 @@ static bool vfio_set_iommu(VFIOContainer *container, int group_fd,
         return false;
     }
 
-    vfio_container_init(&container->bcontainer, space, vioc);
+    vfio_container_init(&container->bcontainer, vioc);
     return true;
 }
 
@@ -614,7 +614,7 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     container->fd = fd;
     bcontainer = &container->bcontainer;
 
-    if (!vfio_set_iommu(container, group->fd, space, errp)) {
+    if (!vfio_set_iommu(container, group->fd, errp)) {
         goto free_container_exit;
     }
 
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 9f8f33e383a38827ceca0f73cb77f5ca6b123198..e5d9334142418514215528b9523f12c031792c7f 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -357,7 +357,7 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
     container->ioas_id = ioas_id;
 
     bcontainer = &container->bcontainer;
-    vfio_container_init(bcontainer, space, iommufd_vioc);
+    vfio_container_init(bcontainer, iommufd_vioc);
     vfio_address_space_insert(space, bcontainer);
 
     if (!iommufd_cdev_attach_container(vbasedev, container, errp)) {
-- 
2.45.2
Re: [PATCH v2 06/17] vfio/container: Simplify vfio_container_init()
Posted by Eric Auger 10 months, 4 weeks ago 
Hi Cédric,

On 6/17/24 08:33, Cédric Le Goater wrote:
> Assign the base container VFIOAddressSpace 'space' pointer in
> vfio_address_space_insert().

OK I get it now. Maybe in the previous patch, say that the

vfio_address_space_insert() will be enhanced with additional initializations.

>
> To be noted that vfio_connect_container() will assign the 'space'
> pointer later in the execution flow. This should not have any
> consequence.

You do not explain the motivation of that change.

>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  include/hw/vfio/vfio-container-base.h | 1 -
>  hw/vfio/common.c                      | 1 +
>  hw/vfio/container-base.c              | 3 +--
>  hw/vfio/container.c                   | 6 +++---
>  hw/vfio/iommufd.c                     | 2 +-
>  5 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index 442c0dfc4c1774753c239c2c8360dcd1540d44fa..d505f63607ec40e6aa44aeb3e20848ac780562a1 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -87,7 +87,6 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>                     VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
>  
>  void vfio_container_init(VFIOContainerBase *bcontainer,
> -                         VFIOAddressSpace *space,
>                           const VFIOIOMMUClass *ops);
>  void vfio_container_destroy(VFIOContainerBase *bcontainer);
>  
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 8cdf26c6f5a490cfa02bdf1087a91948709aaa33..1686a0bed23bd95467bfb00a0c39a4d966e49cae 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1512,6 +1512,7 @@ void vfio_address_space_insert(VFIOAddressSpace *space,
>                                 VFIOContainerBase *bcontainer)
>  {
>      QLIST_INSERT_HEAD(&space->containers, bcontainer, next);
> +    bcontainer->space = space;
>  }
>  
>  struct vfio_device_info *vfio_get_device_info(int fd)
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index 760d9d0622b2e847ecb3368c88df772efb06043f..280f0dd2db1fc3939fe9925ce00a2c50d0e14196 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -71,11 +71,10 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>                                                 errp);
>  }
>  
> -void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace *space,
> +void vfio_container_init(VFIOContainerBase *bcontainer,
>                           const VFIOIOMMUClass *ops)
>  {
>      bcontainer->ops = ops;
> -    bcontainer->space = space;
>      bcontainer->error = NULL;
>      bcontainer->dirty_pages_supported = false;
>      bcontainer->dma_max_mappings = 0;
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 0237c216987ff64a6d11bef8688bb000d93a7f09..dc85a79cb9e62b72312f79da994c53608b6cef48 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -394,7 +394,7 @@ static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type, Error **errp)
>  }
>  
>  static bool vfio_set_iommu(VFIOContainer *container, int group_fd,
> -                           VFIOAddressSpace *space, Error **errp)
> +                           Error **errp)
>  {
>      int iommu_type;
>      const VFIOIOMMUClass *vioc;
> @@ -432,7 +432,7 @@ static bool vfio_set_iommu(VFIOContainer *container, int group_fd,
>          return false;
>      }
>  
> -    vfio_container_init(&container->bcontainer, space, vioc);
> +    vfio_container_init(&container->bcontainer, vioc);
>      return true;
>  }
>  
> @@ -614,7 +614,7 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>      container->fd = fd;
>      bcontainer = &container->bcontainer;
>  
> -    if (!vfio_set_iommu(container, group->fd, space, errp)) {
> +    if (!vfio_set_iommu(container, group->fd, errp)) {
>          goto free_container_exit;
>      }
>  
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 9f8f33e383a38827ceca0f73cb77f5ca6b123198..e5d9334142418514215528b9523f12c031792c7f 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -357,7 +357,7 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>      container->ioas_id = ioas_id;
>  
>      bcontainer = &container->bcontainer;
> -    vfio_container_init(bcontainer, space, iommufd_vioc);
> +    vfio_container_init(bcontainer, iommufd_vioc);
>      vfio_address_space_insert(space, bcontainer);
>  
>      if (!iommufd_cdev_attach_container(vbasedev, container, errp)) {
Thanks

Eric
Re: [PATCH v2 06/17] vfio/container: Simplify vfio_container_init()
Posted by Cédric Le Goater 10 months, 3 weeks ago 
On 6/17/24 4:25 PM, Eric Auger wrote:
> Hi Cédric,
> 
> On 6/17/24 08:33, Cédric Le Goater wrote:
>> Assign the base container VFIOAddressSpace 'space' pointer in
>> vfio_address_space_insert().
> 
> OK I get it now. Maybe in the previous patch, say that the
> 
> vfio_address_space_insert() will be enhanced with additional initializations.



> 
>>
>> To be noted that vfio_connect_container() will assign the 'space'
>> pointer later in the execution flow. This should not have any
>> consequence.
> 
> You do not explain the motivation of that change.

Changed to:

   Assign the base container VFIOAddressSpace 'space' pointer in
   vfio_address_space_insert(). The ultimate goal is to remove
   vfio_container_init() and instead rely on an .instance_init() handler
   to perfom the initialization of VFIOContainerBase.

   To be noted that vfio_connect_container() will assign the 'space'
   pointer later in the execution flow. This should not have any
   consequence.

Thanks,

C.



> 
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/hw/vfio/vfio-container-base.h | 1 -
>>   hw/vfio/common.c                      | 1 +
>>   hw/vfio/container-base.c              | 3 +--
>>   hw/vfio/container.c                   | 6 +++---
>>   hw/vfio/iommufd.c                     | 2 +-
>>   5 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
>> index 442c0dfc4c1774753c239c2c8360dcd1540d44fa..d505f63607ec40e6aa44aeb3e20848ac780562a1 100644
>> --- a/include/hw/vfio/vfio-container-base.h
>> +++ b/include/hw/vfio/vfio-container-base.h
>> @@ -87,7 +87,6 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>>                      VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
>>   
>>   void vfio_container_init(VFIOContainerBase *bcontainer,
>> -                         VFIOAddressSpace *space,
>>                            const VFIOIOMMUClass *ops);
>>   void vfio_container_destroy(VFIOContainerBase *bcontainer);
>>   
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 8cdf26c6f5a490cfa02bdf1087a91948709aaa33..1686a0bed23bd95467bfb00a0c39a4d966e49cae 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -1512,6 +1512,7 @@ void vfio_address_space_insert(VFIOAddressSpace *space,
>>                                  VFIOContainerBase *bcontainer)
>>   {
>>       QLIST_INSERT_HEAD(&space->containers, bcontainer, next);
>> +    bcontainer->space = space;
>>   }
>>   
>>   struct vfio_device_info *vfio_get_device_info(int fd)
>> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
>> index 760d9d0622b2e847ecb3368c88df772efb06043f..280f0dd2db1fc3939fe9925ce00a2c50d0e14196 100644
>> --- a/hw/vfio/container-base.c
>> +++ b/hw/vfio/container-base.c
>> @@ -71,11 +71,10 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
>>                                                  errp);
>>   }
>>   
>> -void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace *space,
>> +void vfio_container_init(VFIOContainerBase *bcontainer,
>>                            const VFIOIOMMUClass *ops)
>>   {
>>       bcontainer->ops = ops;
>> -    bcontainer->space = space;
>>       bcontainer->error = NULL;
>>       bcontainer->dirty_pages_supported = false;
>>       bcontainer->dma_max_mappings = 0;
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 0237c216987ff64a6d11bef8688bb000d93a7f09..dc85a79cb9e62b72312f79da994c53608b6cef48 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -394,7 +394,7 @@ static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type, Error **errp)
>>   }
>>   
>>   static bool vfio_set_iommu(VFIOContainer *container, int group_fd,
>> -                           VFIOAddressSpace *space, Error **errp)
>> +                           Error **errp)
>>   {
>>       int iommu_type;
>>       const VFIOIOMMUClass *vioc;
>> @@ -432,7 +432,7 @@ static bool vfio_set_iommu(VFIOContainer *container, int group_fd,
>>           return false;
>>       }
>>   
>> -    vfio_container_init(&container->bcontainer, space, vioc);
>> +    vfio_container_init(&container->bcontainer, vioc);
>>       return true;
>>   }
>>   
>> @@ -614,7 +614,7 @@ static bool vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>>       container->fd = fd;
>>       bcontainer = &container->bcontainer;
>>   
>> -    if (!vfio_set_iommu(container, group->fd, space, errp)) {
>> +    if (!vfio_set_iommu(container, group->fd, errp)) {
>>           goto free_container_exit;
>>       }
>>   
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 9f8f33e383a38827ceca0f73cb77f5ca6b123198..e5d9334142418514215528b9523f12c031792c7f 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -357,7 +357,7 @@ static bool iommufd_cdev_attach(const char *name, VFIODevice *vbasedev,
>>       container->ioas_id = ioas_id;
>>   
>>       bcontainer = &container->bcontainer;
>> -    vfio_container_init(bcontainer, space, iommufd_vioc);
>> +    vfio_container_init(bcontainer, iommufd_vioc);
>>       vfio_address_space_insert(space, bcontainer);
>>   
>>       if (!iommufd_cdev_attach_container(vbasedev, container, errp)) {
> Thanks
> 
> Eric
>

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.