Messages in this thread | | | Date | Tue, 28 Jun 2022 19:53:39 +0800 | Subject | Re: [PATCH v9 10/11] iommu: Per-domain I/O page fault handling | From | Baolu Lu <> |
| |
On 2022/6/28 17:10, Ethan Zhao wrote: > Hi, Baolu > > 在 2022/6/28 14:28, Baolu Lu 写道: >> Hi Ethan, >> >> On 2022/6/27 21:03, Ethan Zhao wrote: >>> Hi, >>> >>> 在 2022/6/21 22:43, Lu Baolu 写道: >>>> Tweak the I/O page fault handling framework to route the page faults to >>>> the domain and call the page fault handler retrieved from the domain. >>>> This makes the I/O page fault handling framework possible to serve more >>>> usage scenarios as long as they have an IOMMU domain and install a page >>>> fault handler in it. Some unused functions are also removed to avoid >>>> dead code. >>>> >>>> The iommu_get_domain_for_dev_pasid() which retrieves attached domain >>>> for a {device, PASID} pair is used. It will be used by the page fault >>>> handling framework which knows {device, PASID} reported from the iommu >>>> driver. We have a guarantee that the SVA domain doesn't go away during >>>> IOPF handling, because unbind() waits for pending faults with >>>> iopf_queue_flush_dev() before freeing the domain. Hence, there's no >>>> need >>>> to synchronize life cycle of the iommu domains between the unbind() and >>>> the interrupt threads. >>>> >>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >>>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org> >>>> --- >>>> drivers/iommu/io-pgfault.c | 64 >>>> +++++--------------------------------- >>>> 1 file changed, 7 insertions(+), 57 deletions(-) >>>> >>>> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c >>>> index aee9e033012f..4f24ec703479 100644 >>>> --- a/drivers/iommu/io-pgfault.c >>>> +++ b/drivers/iommu/io-pgfault.c >>>> @@ -69,69 +69,18 @@ static int iopf_complete_group(struct device >>>> *dev, struct iopf_fault *iopf, >>>> return iommu_page_response(dev, &resp); >>>> } >>>> -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; >>>> - >>>> - mmap_read_lock(mm); >>>> - >>>> - 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, NULL); >>>> - status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID : >>>> - IOMMU_PAGE_RESP_SUCCESS; >>>> - >>>> -out_put_mm: >>>> - mmap_read_unlock(mm); >>>> - mmput(mm); >>>> - >>>> - return status; >>>> -} >>>> - >>> >>> Once the iopf_handle_single() is removed, the name of >>> iopf_handle_group() looks a little weired >>> >>> and confused, does this group mean the iommu group (domain) ? while I >>> take some minutes to >> >> No. This is not the iommu group. It's page request group defined by the >> PCI SIG spec. Multiple page requests could be put in a group with a >> same group id. All page requests in a group could be responded to device >> in one shot. > > Thanks your explaination, understand the concept of PCIe PRG. I meant > > do we still have the necessity to mention the "group" here in the name > > iopf_handle_group(), which one is better ? iopf_handle_prg() or > > iopf_handler(), perhaps none of them ? :)
Oh! Sorry for the misunderstanding.
I have no strong feeling to change this naming. :-) All the names express what the helper does. Jean is the author of this framework. If he has the same idea as you, I don't mind renaming it in this patch.
Best regards, baolu
| |