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.