RE: [PATCH v5 06/13] vfio/{iommufd,container}: Remove caps::aw_bits

[Duan, Zhenzhong]

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH v5 06/13] vfio/{iommufd,container}: Remove caps::aw_bits
>
>Remove caps::aw_bits which requires the bcontainer::iova_ranges being
>initialized after device is actually attached. Instead defer that to
>.get_cap() and call vfio_device_get_aw_bits() directly.
>
>This is in preparation for HostIOMMUDevice::realize() being called early
>during attach_device().
>
>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
>---
> include/sysemu/host_iommu_device.h | 3 ---
> backends/iommufd.c                 | 3 ++-
> hw/vfio/container.c                | 5 +----
> hw/vfio/iommufd.c                  | 1 -
> 4 files changed, 3 insertions(+), 9 deletions(-)
>
>diff --git a/include/sysemu/host_iommu_device.h
>b/include/sysemu/host_iommu_device.h
>index ee6c813c8b22..cdeeccec7671 100644
>--- a/include/sysemu/host_iommu_device.h
>+++ b/include/sysemu/host_iommu_device.h
>@@ -19,12 +19,9 @@
>  * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities.
>  *
>  * @type: host platform IOMMU type.
>- *
>- * @aw_bits: host IOMMU address width. 0xff if no limitation.
>  */
> 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 a94d3b90c05c..58032e588f49 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);

I just realized there is an open here. hiod->agent is not necessarily VFIO 
device, can be VDPA device.
May need a bit more work on this.

Thanks
Zhenzhong

>     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 545f4a404125..028533bc39b9 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -724,7 +724,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;
> }
>--
>2.17.2