Re: [PATCH 3/6] iommufd: Add IOMMU_DEVICE_GET_INFO

[Joao Martins]

On 10/02/2023 07:55, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
>> Sent: Thursday, February 9, 2023 12:17 PM
>> + * zeroed if the user buffer is larger than the data kernel has.
>> + *
>> + * The type specific data would be used to sync capability between the
>> + * vIOMMU and the hardware IOMMU, also for the availabillity checking of
>> + * iommu hardware features like dirty page tracking in I/O page table.
> 
> It's fine to report format information related to stage-1 page table
> which userspace manages.
> 
> but IMHO this should not be an interface to report which capability is
> supported by iommufd. Having hardware supporting dirty bit 
> doesn't mean the underlying iommu driver provides necessary support
> to iommufd dirty tracking.
> 

+1

In fact this shouldn't really be a way to check any capability as we are dumping
straight away the IOMMU hardware registers. By dumping raw IOMMU hardware data,
forces the application to understand IOMMU hardware specific formats.  Maybe
that's OK for hw nesting as there's a lot of info you need to know for the
vIOMMU pgtables, pasid and etc so both are tightly coupled. But it is a bit
disconnected from what really the software (IOMMUFD) and driver can use, without
getting onto assumptions.

[Calling it IOMMU_DEVICE_GET_HW_INFO would be bit more obvious if you're not
asking IOMMUFD support]

For capability checking, I think this really should be returning capabilities
that both IOMMU driver supports ... and that IOMMUFD understands and marshalled
on a format of its own defined by IOMMUFD. Maybe using IOMMU_CAP or some other
thing within the kernel (now that's per-device), or even simpler. That's at
least had written initially for the dirty tracking series.