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
