Re: [PATCH] iommufd: Deal with IOMMU_HWPT_FAULT_ID_VALID in iommufd core

[Yi Liu <yi.l.liu@intel.com>]

On 2024/12/12 04:02, Jason Gunthorpe wrote:
> On Sat, Dec 07, 2024 at 04:01:08AM -0800, Yi Liu wrote:
>> IOMMU_HWPT_FAULT_ID_VALID is used to mark if the fault_id field of
>> iommu_hwp_alloc is valid or not. As the fault_id field is handled in
>> the iommufd core, so it makes sense to sanitize the
>> IOMMU_HWPT_FAULT_ID_VALID flag in the iommufd core, and mask it out
>> before passing the user flags to the iommu drivers.
>>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c |  8 +-------
>>   drivers/iommu/intel/iommu.c                         |  3 +--
>>   drivers/iommu/iommufd/hw_pagetable.c                | 10 +++++++---
>>   drivers/iommu/iommufd/selftest.c                    |  2 +-
>>   4 files changed, 10 insertions(+), 13 deletions(-)
> 
> I'm going to take this, but there is still something missing - drivers
> do not check domain->iopf_handler during attach and will happily
> install these domains without any HW support for fault reporting.

Apologize about it, per Kevin's remark[1] attach fault-capable domain to
non-PRI capable device shall fail when trying to enable PRI on the
devices that do not support it. So the current code does not have the
gap I mentioned in prior email [2].

[1] 
https://lore.kernel.org/linux-iommu/BN9PR11MB5276FE72F52332E1BD40AE288C3E2@BN9PR11MB5276.namprd11.prod.outlook.com/
[2] 
https://lore.kernel.org/linux-iommu/95a85b9b-a17b-4df5-a1ac-7a0880950960@intel.com/

> Not sure this is so great.. I suggest adding a flag to the iommu_ops
> 'supports_fault_reporting', as just a global flag if the driver could
> *possibly* support this. Core code will fail attach (and maybe alloc?)
> if it is not set. If it is set driver must check domain->iopf_handler
> during attach against HW capability.
> 

This flag should be per device I guess. But as above remark, it's not
urgent to add this for now. :)

-- 
Regards,
Yi Liu