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 <c...@redhat.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.d...@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
