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

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Dec 10, 2024 at 01:52:28PM +0800, Yi Liu wrote:
> On 2024/12/9 23:05, Jason Gunthorpe wrote:
> > On Mon, Dec 09, 2024 at 10:51:51AM +0800, Yi Liu wrote:
> > > On 2024/12/9 09:08, Baolu Lu wrote:
> > > > On 12/7/24 20:01, 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.
> > > > 
> > > > Is it a valid use case for an iommu driver to intercept the
> > > > IOMMU_HWPT_FAULT_ID_VALID flag as an indication that user space requests
> > > > an iopf-capable domain?
> > 
> > For now we have no use case, so we can drop it from drivers, I think.
> > 
> > PRI support or not is decided when the domain is attached based on the
> > fault_handler in the struct iommu_domain.
> 
> hmmm. It sounds reasonable. But I failed to see such checks in iommu
> drivers. 

It would not surprise me, but also we are deliberately being very lax
on fault support to allow devices with internal fault queues.

> So attaching a non-fault capable domain to a device that has
> enabled PRI can succeed. 

Yes

> But the PRQs from this device will be responded as bad PRQ.

The core code should be doing this in the fault handler when it
fails to find an appropriate domain to deliver the fault to.

Inside iommu_report_device_fault() find_fault_handler() will fail:

	if (!attach_handle->domain->iopf_handler)
		return NULL;

Which should permanently abort the PRI to the device.

> yes, iommu driver should be able to fail such attempts in attach phase.
> So I may keep this patch as it is since IOMMU_HWPT_FAULT_ID_VALID is not
> supposed to be used by iommu drivers.

It makes sense

IMHO we don't have a clear set of rules when the iommu driver should
permit a fault capable domain to be attached. We don't have any API to
tell if the device will handle faults internally or if the device
requires PRI.

A device that can do internal faults only requires ATS support from
the iommu. Otherwise you need PRI enabled at the device and PRI
support in the iommu HW.

Then there is the special ARM stall mode too..

For now I would leave this as is.

Jason