Re: [PATCH v7 04/24] iommu: Add a page fault handler

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

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@linaro.org>
[...]
> > +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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu