On 6/4/24 10:46, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.au...@redhat.com>
>> Subject: Re: [PATCH v6 11/19] backends/iommufd: Implement
>> HostIOMMUDeviceClass::get_cap() handler
>>
>>
>>
>> On 6/4/24 05:23, Duan, Zhenzhong wrote:
>>> Hi Cédric, Eric,
>>>
>>>> -----Original Message-----
>>>> From: Cédric Le Goater <c...@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 <c...@redhat.com>
>>>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@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/5333b1a0ae03b3c5119b46a1
>> af786d199f103889
>>> 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.
> Getting name directly is not easy, we can save a copy in .realize(), like
> below:
sounds good + dealloc
Eric
>
> --- a/include/sysemu/host_iommu_device.h
> +++ b/include/sysemu/host_iommu_device.h
> @@ -33,6 +33,7 @@ OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass,
> HOST_IOMMU_DEVICE)
> struct HostIOMMUDevice {
> Object parent_obj;
>
> + char *name;
> HostIOMMUDeviceCaps caps;
> };
>
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index f2f7a762a0..84fefbc9ee 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -240,7 +240,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod,
> int cap, Error **errp)
> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> return caps->aw_bits;
> default:
> - error_setg(errp, "Not support get cap %x", cap);
> + 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 a830426647..e78538efec 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -1152,6 +1152,7 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice
> *hiod, void *opaque,
> } else {
> hiod->caps.aw_bits = 0xff;
> }
> + hiod->name = g_strdup(vdev->name);
>
> return true;
> }
> @@ -1165,7 +1166,7 @@ static int hiod_legacy_vfio_get_cap(HostIOMMUDevice
> *hiod, int cap,
> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> return caps->aw_bits;
> default:
> - error_setg(errp, "Not support get cap %x", cap);
> + 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 8fd8d52bc2..2df3aed47f 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -637,6 +637,7 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice
> *hiod, void *opaque,
> return false;
> }
>
> + hiod->name = g_strdup(vdev->name);
> caps->type = type;
