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 <c...@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 <c...@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.d...@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