Re: [PATCH v6 11/19] backends/iommufd: Implement HostIOMMUDeviceClass::get_cap() handler

[Eric Auger <eric.auger@redhat.com>]

On 6/4/24 05:23, Duan, Zhenzhong wrote:
> Hi Cédric, Eric,
>
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Subject: Re: [PATCH v6 11/19] backends/iommufd: Implement
>> HostIOMMUDeviceClass::get_cap() handler
>>
>> On 6/3/24 13:32, Eric Auger wrote:
>>>
>>> On 6/3/24 08:10, Zhenzhong Duan wrote:
>>>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>> ---
>>>>   backends/iommufd.c | 23 +++++++++++++++++++++++
>>>>   1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>>>> index c7e969d6f7..f2f7a762a0 100644
>>>> --- a/backends/iommufd.c
>>>> +++ b/backends/iommufd.c
>>>> @@ -230,6 +230,28 @@ bool
>> iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
>>>>       return true;
>>>>   }
>>>>
>>>> +static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap,
>> Error **errp)
>>>> +{
>>>> +    HostIOMMUDeviceCaps *caps = &hiod->caps;
>>>> +
>>>> +    switch (cap) {
>>>> +    case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
>>>> +        return caps->type;
>>>> +    case HOST_IOMMU_DEVICE_CAP_AW_BITS:
>>>> +        return caps->aw_bits;
>>>> +    default:
>>>> +        error_setg(errp, "Not support get cap %x", cap);
>>> can't you add details about the faulting HostIOMMUDevice by tracing the
>>> devid for instance?
>> yes.
> devid isn't added to make this series simpler.
> It's added in nesting series, https://github.com/yiliu1765/qemu/commit/5333b1a0ae03b3c5119b46a1af786d199f103889
>
> Do you want to add devid in this series for tracing purpose or adding trace in nesting series is fine for you?

what would be nice is to get a common way to identify a HostIOMMUDevice,
can't we use the name of the VFIO/VDPA device? devid does not exist on
legacy container. At least a kind of wrapper may be relevant to extract
the name.

Eric
>
>>> I would rephrase the error message into No support for capability 0x%x
>> I was going to propose "Unsupported capability ..."
> Sounds better, will do.
>
> Thanks
> Zhenzhong
>