Hi Baolu,
Thanks for the review. I'm only now reworking this and realized I've never
sent a reply, sorry about that.
On Wed, May 20, 2020 at 02:42:21PM +0800, Lu Baolu wrote:
> Hi Jean,
>
> On 2020/5/20 1:54, Jean-Philippe Brucker wrote:
> > Some systems allow devices to handle I/O Page Faults in the core mm. For
> > example systems implementing the PCIe PRI extension or Arm SMMU stall
> > model. Infrastructure for reporting these recoverable page faults was
> > added to the IOMMU core by commit 0c830e6b3282 ("iommu: Introduce device
> > fault report API"). Add a page fault handler for host SVA.
> >
> > IOMMU driver can now instantiate several fault workqueues and link them
> > to IOPF-capable devices. Drivers can choose between a single global
> > workqueue, one per IOMMU device, one per low-level fault queue, one per
> > domain, etc.
> >
> > When it receives a fault event, supposedly in an IRQ handler, the IOMMU
> > driver reports the fault using iommu_report_device_fault(), which calls
> > the registered handler. The page fault handler then calls the mm fault
> > handler, and reports either success or failure with iommu_page_response().
> > When the handler succeeded, the IOMMU retries the access.
> >
> > The iopf_param pointer could be embedded into iommu_fault_param. But
> > putting iopf_param into the iommu_param structure allows us not to care
> > about ordering between calls to iopf_queue_add_device() and
> > iommu_register_device_fault_handler().
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx>
[...]
> > +static enum iommu_page_response_code
> > +iopf_handle_single(struct iopf_fault *iopf)
> > +{
> > + vm_fault_t ret;
> > + struct mm_struct *mm;
> > + struct vm_area_struct *vma;
> > + unsigned int access_flags = 0;
> > + unsigned int fault_flags = FAULT_FLAG_REMOTE;
> > + struct iommu_fault_page_request *prm = &iopf->fault.prm;
> > + enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
> > +
> > + if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
> > + return status;
> > +
> > + mm = iommu_sva_find(prm->pasid);
> > + if (IS_ERR_OR_NULL(mm))
> > + return status;
> > +
> > + down_read(&mm->mmap_sem);
> > +
> > + vma = find_extend_vma(mm, prm->addr);
> > + if (!vma)
> > + /* Unmapped area */
> > + goto out_put_mm;
> > +
> > + if (prm->perm & IOMMU_FAULT_PERM_READ)
> > + access_flags |= VM_READ;
> > +
> > + if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
> > + access_flags |= VM_WRITE;
> > + fault_flags |= FAULT_FLAG_WRITE;
> > + }
> > +
> > + if (prm->perm & IOMMU_FAULT_PERM_EXEC) {
> > + access_flags |= VM_EXEC;
> > + fault_flags |= FAULT_FLAG_INSTRUCTION;
> > + }
> > +
> > + if (!(prm->perm & IOMMU_FAULT_PERM_PRIV))
> > + fault_flags |= FAULT_FLAG_USER;
> > +
> > + if (access_flags & ~vma->vm_flags)
> > + /* Access fault */
> > + goto out_put_mm;
> > +
> > + ret = handle_mm_fault(vma, prm->addr, fault_flags);
> > + status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
>
> Do you mind telling why it's IOMMU_PAGE_RESP_INVALID but not
> IOMMU_PAGE_RESP_FAILURE?
PAGE_RESP_FAILURE maps to PRI Response code "Response Failure" which
indicates a catastrophic error and causes the function to disable PRI.
Instead PAGE_RESP_INVALID maps to PRI Response code "Invalid request",
which tells the function that the address is invalid and there is no point
retrying this particular access.
Thanks,
Jean
