Re: [PATCH v3 05/10] vfio/iommufd: Probe and request hwpt dirty tracking capability

[Joao Martins <joao.m.martins@oracle.com>]

On 09/07/2024 07:28, Cédric Le Goater wrote:
> On 7/8/24 4:34 PM, Joao Martins wrote:
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 2ca9a32cc7b6..1b5b46d28ed6 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -212,6 +212,20 @@ static bool iommufd_cdev_detach_ioas_hwpt(VFIODevice
>> *vbasedev, Error **errp)
>>       return true;
>>   }
>>   +static bool iommufd_device_dirty_tracking(IOMMUFDBackend *iommufd,
>> +                                          VFIODevice *vbasedev)
>> +{
>> +    enum iommu_hw_info_type type;
>> +    uint64_t caps;
>> +
>> +    if (!iommufd_backend_get_device_info(iommufd, vbasedev->devid, &type,
>> +                                         NULL, 0, &caps, NULL)) {
> 
> I think we should report the error and not ignore it.
> 
> That said, since we are probing the hw features of the host IOMMU device,
> could we use the data cached in the HostIOMMUDevice struct instead ?
> This means would need to move the ->realize() call doing the probing
> before attaching the device in vfio_attach_device(). That way we would
> catch probing errors in one place. Does this make sense ?

Yeap. It also helps centralizing cap checking in addition.

This stanadlone use of iommufd_backend_get_device_info() was also annoying me a
little, and there doesn't seem to have a reason not to move the initialization
of caps earlier. I'll do that

	Joao