Re: [PATCH] vfio/iommufd: no hw_info for mediated devices

[Steven Sistare <steven.sistare@oracle.com>]

On 9/10/2024 3:51 PM, Steven Sistare wrote:
> On 9/10/2024 3:29 PM, Joao Martins wrote:
>> On 10/09/2024 18:40, Steve Sistare wrote:
>>> IOMMU_GET_HW_INFO fails for a vfio mediated device (aka mdev), because
>>> the devid is associated with kernel type IOMMUFD_OBJ_ACCESS, not
>>> IOMMUFD_OBJ_DEVICE.  Assume IOMMU_HW_INFO_TYPE_NONE and proceed.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>
>>> ---
>>>   hw/vfio/iommufd.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index cb7257b..d8928d4 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -635,6 +635,7 @@ static void vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
>>>   static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>>                                         Error **errp)
>>>   {
>>> +    Error *err = NULL;
>>>       VFIODevice *vdev = opaque;
>>>       HostIOMMUDeviceCaps *caps = &hiod->caps;
>>>       enum iommu_hw_info_type type;
>>> @@ -645,8 +646,9 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>>       hiod->agent = opaque;
>>>       if (!iommufd_backend_get_device_info(vdev->iommufd, vdev->devid,
>>> -                                         &type, &data, sizeof(data), errp)) {
>>> -        return false;
>>> +                                         &type, &data, sizeof(data), &err)) {
>>> +        warn_report_err(err);
>>> +        type = IOMMU_HW_INFO_TYPE_NONE;
>>>       }
>>>       hiod->name = g_strdup(vdev->name);
>>
>> At least we aren't supposed to call realize() on an mdev.
>>
>> See commit 9f17604195c ("vfio/iommufd: Don't initialize nor set a
>> HOST_IOMMU_DEVICE with mdev"). There's another refactor later on in commit
>> 83a4d596a93 ("vfio/{iommufd, container}: Invoke HostIOMMUDevice::realize()
>> during attach_device()") where we also don't realize() when vbasedev->hiod is NULL.
>>
>> Unless the stty mdevs aren't matching examples, then this shouldn't be possible
>> (I didn't have one with real hw behind to test).
>>
>> So if you are hitting this codepath then vbasedev::mdev is false, which I don't
>> understand how it's possible?
> 
> I exercised this with the mtty sample mdev, and mdev is true:
> 
> [...]
> 
> My qemu tree is a bit old, I will update and look for the issues you described.

After updating all is well without my patch, so I withdraw it, thanks.

- Steve