Hi Joao, On 7/16/24 20:22, Joao Martins wrote: > On 16/07/2024 18:40, Eric Auger wrote: >> Hi Joao, >> >> 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. >> It is difficult to parse the above sentence. Would deserve some rephrasing. >> >> Also properties have a different meaning in qemu. > I think I will remove the above paragraph and instead adopt below with some > rephrasing: > > Remove caps::aw_bits which requires the > bcontainer::iova_ranges to be inititalized after device is actually > initialized attached. Instead defer that to .get_cap() and call s/initialized//g > vfio_device_get_aw_bits() directly. > > This is in preparation for HostIOMMUDevice::realize() being called early > during > attach_device(). > > Better? Yes sounds better
Eric > >>> This means removing caps::aw_bits which requires the >>> bcontainer::iova_ranges to be inititalized after device is actually >> initialized > Yes > >>> attached. Instead defer that to .get_cap() and call >>> vfio_device_get_aw_bits() directly. >>> >>> Suggested-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >>> Signed-off-by: Joao Martins <joao.m.mart...@oracle.com> >>> --- >>> 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; >> the doc comment needs to be updated accordingly. >>> } 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; >>> }