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