Re: [PATCH v4 06/12] vfio/{iommufd,container}: Remove caps::aw_bits

[Cédric Le Goater]

On 7/12/24 13:46, Joao Martins wrote:
> In preparation to moving HostIOMMUDevice realize() being able to called
> early during attach_device(), remove properties that rely on container
> being initialized.
>
> This means removing caps::aw_bits which requires the
> bcontainer::iova_ranges to be inititalized after device is actually
> attached. Instead defer that to .get_cap() and call
> vfio_device_get_aw_bits() directly.
>
> Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/sysemu/host_iommu_device.h | 1 -
>   backends/iommufd.c                 | 3 ++-
>   hw/vfio/container.c                | 5 +----
>   hw/vfio/iommufd.c                  | 1 -
>   4 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/include/sysemu/host_iommu_device.h 
> b/include/sysemu/host_iommu_device.h
> index ee6c813c8b22..20e77cf54568 100644
> --- a/include/sysemu/host_iommu_device.h
> +++ b/include/sysemu/host_iommu_device.h
> @@ -24,7 +24,6 @@
>    */
>   typedef struct HostIOMMUDeviceCaps {
>       uint32_t type;
> -    uint8_t aw_bits;
>   } HostIOMMUDeviceCaps;
>   
>   #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index 5d3dfa917415..41a9dec3b2c5 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -18,6 +18,7 @@
>   #include "qemu/error-report.h"
>   #include "monitor/monitor.h"
>   #include "trace.h"
> +#include "hw/vfio/vfio-common.h"
>   #include <sys/ioctl.h>
>   #include <linux/iommufd.h>
>   
> @@ -270,7 +271,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>       case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
>           return caps->type;
>       case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> -        return caps->aw_bits;
> +        return vfio_device_get_aw_bits(hiod->agent);
>       default:
>           error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>           return -EINVAL;
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 88ede913d6f7..c27f448ba26e 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -1144,7 +1144,6 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice 
> *hiod, void *opaque,
>       VFIODevice *vdev = opaque;
>   
>       hiod->name = g_strdup(vdev->name);
> -    hiod->caps.aw_bits = vfio_device_get_aw_bits(vdev);
>       hiod->agent = opaque;
>   
>       return true;
> @@ -1153,11 +1152,9 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice 
> *hiod, void *opaque,
>   static int hiod_legacy_vfio_get_cap(HostIOMMUDevice *hiod, int cap,
>                                       Error **errp)
>   {
> -    HostIOMMUDeviceCaps *caps = &hiod->caps;
> -
>       switch (cap) {
>       case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> -        return caps->aw_bits;
> +        return vfio_device_get_aw_bits(hiod->agent);
>       default:
>           error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>           return -EINVAL;
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 325c7598d5a1..873c919e319c 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -722,7 +722,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice 
> *hiod, void *opaque,
>   
>       hiod->name = g_strdup(vdev->name);
>       caps->type = type;
> -    caps->aw_bits = vfio_device_get_aw_bits(vdev);
>   
>       return true;
>   }