IB/hfi1: fix the PIO_CRED credit-return mmap
Subsystems (3)
cover IB/hfi1: fix the PIO_CRED credit-return mmap
The PIO_CRED case of hfi1_file_mmap() hands user space the credit-return page for its send context. On a two-socket host with a translating IOMMU it instead returns a page mapped from a frame above MAXPHYADDR, and the first user read takes "Corrupted page table" / "Oops: Bad pagetable". Correcting only the offset arithmetic replaces the Oops with a silent wrong-page mapping, so every transfer that uses send PIO hangs instead.
The failure is intermittent: it depends on which of the credit-return pages the context's entry lands on, which follows the hardware send context index and so varies from boot to boot.
Patch 1 fixes the node used to resolve the credit-return buffer, which has been wrong since the driver was merged. Patch 2 fixes the byte offset applied to a typed pointer and the use of dma_mmap_coherent(), both introduced when this case was converted to the DMA API. They are separate patches because they have different Fixes: tags, but only the two together make the mapping correct: patch 2 without patch 1 turns the hang into an -ENXIO from iommu_dma_mmap()'s bounds check, because the cross-node offset exceeds the buffer.
Found while debugging psm2_ep_open() Oopsing a Dell T7610 (Xeon E5-2650 v2, Intel IOMMU in DMA-FQ mode) talking to a Threadripper PRO 3995WX, both Omni-Path 100. With the series applied to both hosts, psm2_ep_open() succeeds, send PIO, send DMA and the default mixed mode all work, and MPI over the PSM2 MTL sustains 95 Gb/s. Forcing send PIO only (PSM2_SDMA=0), which hung indefinitely before, now completes.
Based on rdma/for-rc at 31b7c700670830a0e8a4cdcd451c88a13cc5dc48.
Shuhei Takeshita (2):
IB/hfi1: Resolve the credit-return buffer through the send context's
node
IB/hfi1: Fix the PIO_CRED credit-return mmap
drivers/infiniband/hw/hfi1/file_ops.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
Signature · 1 line
2.43.0
PATCH 1/2 IB/hfi1: Resolve the credit-return buffer through the send context's node
hfi1_file_mmap()'s PIO_CRED case derives this context's credit-return page offset, and the DMA handle for it, from dd->cr_base[uctxt->numa_id]. uctxt->numa_id is the node of whichever CPU the process happened to be running on, but the entry itself lives in the credit-return allocation of the send context's own node:
sc->hw_free = &sc->dd->cr_base[sc->node].va[gc].cr[index];
and user send contexts are allocated with sc_alloc(dd, SC_USER, ..., dd->node), the HFI-local node. On a multi-socket host with the process running off that node the two allocations differ, so the subtraction produces an offset into an unrelated buffer and the DMA handle belongs to the wrong allocation.
Use the send context's own node for all three references. The continuation lines are reindented at the same time; they mixed spaces and tabs.
--- drivers/infiniband/hw/hfi1/file_ops.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/infiniband/hw/hfi1/file_ops.c +4 -4
@@ -382,10 +382,10 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma)* of enabled contexts > 64 and 128 respectively). */ cr_page_offset = ((u64 )uctxt->sc->hw_free - - (u64 )dd->cr_base[uctxt->numa_id].va) & -PAGE_MASK ; - memvirt = dd->cr_base[uctxt->numa_id].va + cr_page_offset; - memdma = dd->cr_base[uctxt->numa_id].dma + cr_page_offset; + (u64 )dd->cr_base[uctxt->sc->node].va) & +PAGE_MASK ; + memvirt = dd->cr_base[uctxt->sc->node].va + cr_page_offset; + memdma = dd->cr_base[uctxt->sc->node].dma + cr_page_offset; memlen =PAGE_SIZE ; flags &= ~VM_MAYWRITE ; flags |=VM_DONTCOPY |VM_DONTEXPAND ;
Signature · 1 line
2.43.0
PATCH 2/2 IB/hfi1: Fix the PIO_CRED credit-return mmap
In reply to Shuhei Takeshitahfi1_file_mmap()'s PIO_CRED case must hand user space the single credit-return page that holds this context's entry. That page is the second or third page of the per-node credit-return allocation once the hardware send context index reaches 64 or 128, so the failure below is
intermittent: when the entry lands on the first page the offset is zero and everything works.
Two things are wrong.
First, cr_page_offset is a byte offset but .va is a struct credit_return *, so adding it is pointer arithmetic and scales the offset by sizeof(struct credit_return) == 64. memvirt then lands 256 KiB or 512 KiB past a 10240-byte allocation. With an IOMMU translating, that address is inside the vmalloc range but in no vm_area, so dma_mmap_coherent() -> iommu_dma_mmap() finds no pages, vmalloc_to_pfn() returns page_to_pfn(NULL), and remap_pfn_range() installs a frame above MAXPHYADDR. The first user read then takes:
psm2_ep_open_pr: Corrupted page table at address 7a14d007e000 PGD 800000013886a067 P4D 800000013886a067 PUD 13886b067 PMD 13886c067
PTE 800049168e911235 Oops: Bad pagetable: 000d [#1] SMP PTI
Second, and still wrong once the arithmetic is corrected, dma_mmap_coherent() describes a whole coherent buffer and selects the page within it with vma->vm_pgoff. Offsetting cpu_addr has no effect:
for a vmap'd allocation iommu_dma_mmap() uses cpu_addr only to locate the vm_area and then maps pages[vm_pgoff], which hfi1_file_mmap() has just set to 0. User space therefore always receives the first credit-return page, every credit read is for the wrong context, and send PIO stalls forever.
Use the DMA API as intended: pass the base of the allocation with its full length and select the page with vm_pgoff. A separate length is needed because memlen must keep describing the VMA for the existing size check. The dma-direct path stays correct as well, since dma_direct_mmap() adds the same vm_pgoff to the base pfn.
Tested on a Dell T7610 (Xeon E5-2650 v2, Intel IOMMU in DMA-FQ mode) against a Threadripper PRO 3995WX peer, both Omni-Path 100. Before this change psm2_ep_open() Oopses the kernel; with only the arithmetic corrected psm2_ep_open() succeeds but any transfer that uses send PIO hangs, PSM2_SDMA=2 (send PIO disabled) completing normally while PSM2_SDMA=0 (send PIO only) hangs every time. With this change send PIO, send DMA and the default mixed mode all work.
--- drivers/infiniband/hw/hfi1/file_ops.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
drivers/infiniband/hw/hfi1/file_ops.c +16 -5
@@ -326,6 +326,7 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma)void *memvirt =NULL ;dma_addr_t memdma =0 ;u8 subctxt, mapio =0 , vmf =0 , type; +size_t memdmalen =0 ;ssize_t memlen =0 ;int ret =0 ;u16 ctxt;
@@ -371,7 +372,9 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma) mapio =1 ;break ;case PIO_CRED : { +struct credit_return_base *cr = &dd->cr_base[uctxt->sc->node];u64 cr_page_offset; +if (flags &VM_WRITE ) { ret = -EPERM ; goto done;
@@ -381,11 +384,18 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma)* second or third page allocated for credit returns (if number * of enabled contexts > 64 and 128 respectively). */ - cr_page_offset = ((u64 )uctxt->sc->hw_free - - (u64 )dd->cr_base[uctxt->sc->node].va) & + cr_page_offset = ((u64 )uctxt->sc->hw_free - (u64 )cr->va) &PAGE_MASK ; - memvirt = dd->cr_base[uctxt->sc->node].va + cr_page_offset; - memdma = dd->cr_base[uctxt->sc->node].dma + cr_page_offset; +/* +* dma_mmap_coherent() describes the whole coherent buffer and +* selects the page within it with vma->vm_pgoff, so pass the +* base of the allocation and its length and let vm_pgoff pick +* the page. +*/ + vma->vm_pgoff = cr_page_offset >>PAGE_SHIFT ; + memvirt = cr->va; + memdma = cr->dma; + memdmalen =TXE_NUM_CONTEXTS *sizeof (struct credit_return ); memlen =PAGE_SIZE ; flags &= ~VM_MAYWRITE ; flags |=VM_DONTCOPY |VM_DONTEXPAND ;
@@ -567,7 +577,8 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma) ret =0 ; }else if (memdma) { ret =dma_mmap_coherent (&dd->pcidev->dev, vma, - memvirt, memdma, memlen); + memvirt, memdma, + memdmalen ? memdmalen : memlen); }else if (mapio) { ret = io_remap_pfn_range(vma, vma->vm_start, PFN_DOWN(memaddr),
Signature · 1 line
2.43.0
Leon Romanovsky
AppliedQuoted 8 lines · Shuhei Takeshita
The
PIO_CREDcase ofhfi1_file_mmap()hands user space the credit-return page for its send context. On a two-socket host with a translating IOMMU it instead returns a page mapped from a frame above MAXPHYADDR, and the first user read takes "Corrupted page table" / "Oops: Bad pagetable". Correcting only the offset arithmetic replaces the Oops with a silent wrong-page mapping, so every transfer that uses send PIO hangs instead.[...]
Applied, thanks!
[1/2] IB/hfi1: Resolve the credit-return buffer through the send context's node
https://git.kernel.org/rdma/rdma/c/975396b9e5a402 [2/2] IB/hfi1: Fix the PIO_CRED credit-return mmap https://git.kernel.org/rdma/rdma/c/62f0f34fbd2b2d
Best regards,
Signature · 1 line
Leon Romanovsky <[email protected]>