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

[Eric Auger]

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.duan@intel.com>
>>> Signed-off-by: Joao Martins <joao.m.martins@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;
>>>  }