On Fri, May 10, 2024 at 11:14:20AM +0800, Baolu Lu wrote: > On 5/8/24 8:04 AM, Jason Gunthorpe wrote: > > On Tue, Apr 30, 2024 at 10:57:04PM +0800, Lu Baolu wrote: > > > @@ -206,8 +197,11 @@ void iommu_report_device_fault(struct device *dev, struct iopf_fault *evt) > > > if (group == &abort_group) > > > goto err_abort; > > > - group->domain = get_domain_for_iopf(dev, fault); > > > - if (!group->domain) > > > + if (!(fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) || > > > + get_attach_handle_for_iopf(dev, fault->prm.pasid, group)) > > > + get_attach_handle_for_iopf(dev, IOMMU_NO_PASID, group); > > That seems a bit weird looking? > > Agreed. > > > get_attach_handle_for_iopf(dev, > > (fault->prm.flags & > > IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) ? fault->prm.pasid : IOMMU_NO_PASID, > > group); > > The logic here is that it tries the PASID domain and if it doesn't > exist, then tries the RID domain as well. I explained this in the commit > message: > > " > ... if the pasid table of a device is wholly managed by user space, > there is no domain attached to the PASID of the device ... > " Okay, it needs a comment in the code, and the RID fallback should be based aroudn checking for a NESTING domain type which includes the PASID table. (ie ARM and AMD not Intel) We shouldn't just elevate a random PASID to the RID if it isn't approprite.. Jason
