>-----Original Message-----
>From: Eric Auger <eric.au...@redhat.com>
>Subject: Re: [PATCH v6 07/19] vfio/container: Implement
>HostIOMMUDeviceClass::realize() handler
>
>Hi Zhenzhong,
>
>On 6/4/24 04:45, Duan, Zhenzhong wrote:
>> Hi Eric,
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.au...@redhat.com>
>>> Subject: Re: [PATCH v6 07/19] vfio/container: Implement
>>> HostIOMMUDeviceClass::realize() handler
>>>
>>> Hi Zhenzhong,
>>>
>>> On 6/3/24 08:10, Zhenzhong Duan wrote:
>>>> Utilize range_get_last_bit() to get host IOMMU address width and
>>>> package it in HostIOMMUDeviceCaps for query with .get_cap().
>>>>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>>> ---
>>>> hw/vfio/container.c | 26 ++++++++++++++++++++++++++
>>>> 1 file changed, 26 insertions(+)
>>>>
>>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>>> index c4fca2dfca..48800fe92f 100644
>>>> --- a/hw/vfio/container.c
>>>> +++ b/hw/vfio/container.c
>>>> @@ -1136,6 +1136,31 @@ static void
>>> vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
>>>> vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
>>>> };
>>>>
>>>> +static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void
>>> *opaque,
>>>> + Error **errp)
>>>> +{
>>>> + VFIODevice *vdev = opaque;
>>>> + /* iova_ranges is a sorted list */
>>>> + GList *l = g_list_last(vdev->bcontainer->iova_ranges);
>>>> +
>>>> + /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS with
>>> legacy backend */
>>> I don't get the comment as HOST_IOMMU_DEVICE_CAP_AW_BITS
>support
>>> seems
>>> to be introduced in [PATCH v6 11/19] backends/iommufd: Implement
>>> HostIOMMUDeviceClass::get_cap() handler
>> Sorry about my poor English, I mean legacy backend only support
>> HOST_IOMMU_DEVICE_CAP_AW_BITS, no other caps.
>> May be only:
>>
>> /* Only support query HOST_IOMMU_DEVICE_CAP_AW_BITS */
>no problem. Then I would put this comment in the commit msg instead.
>Something like "the realize function populates the capabilities. For now
>only the aw_bits caps is computed".
Sure.
>
>
>>
>>>> + if (l) {
>>>> + Range *range = l->data;
>>>> + hiod->caps.aw_bits = range_get_last_bit(range) + 1;
>>>> + } else {
>>>> + hiod->caps.aw_bits = 0xff;
>>> why this value?
>> 0xff means no limitation on aw_bits from host side. Aw_bits check
>> should always pass. This could be a case that an old kernel doesn't
>> support query iova ranges.
>>
>> Will add a define like:
>>
>> #define HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX 0xff
>Wouldn't 64 bits be a better choice?
Yes, 64 bits is large enough, will it.
> Also maybe add a comment explaining
>that iova_ranges may be void for old kernels that do not support the query?
Sure.
Thanks
Zhenzhong
