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".
>
>>> + 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? Also maybe add a comment explaining
that iova_ranges may be void for old kernels that do not support the query?
Eric
>
> Thanks
> Zhenzhong
>
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data)
>>> +{
>>> + HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc);
>>> +
>>> + hioc->realize = hiod_legacy_vfio_realize;
>>> +};
>>> +
>>> static const TypeInfo types[] = {
>>> {
>>> .name = TYPE_VFIO_IOMMU_LEGACY,
>>> @@ -1144,6 +1169,7 @@ static const TypeInfo types[] = {
>>> }, {
>>> .name = TYPE_HOST_IOMMU_DEVICE_LEGACY_VFIO,
>>> .parent = TYPE_HOST_IOMMU_DEVICE,
>>> + .class_init = hiod_legacy_vfio_class_init,
>>> }
>>> };
>>>
>> Thanks
>>
>> Eric
