mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] RDMA/irdma: always route IB_WR_REG_MR through the allocated PBL
@ 2026-10-07  9:53 Stefan Metzmacher
  2026-10-07 10:26 ` Stefan Metzmacher
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Metzmacher @ 2026-10-07  9:53 UTC (permalink / raw)
  To: linux-rdma
  Cc: metze, Tatyana Nikolova, Jason Gunthorpe, Leon Romanovsky,
	Mustafa Ismail, Shiraz Saleem, linux-kernel

irdma_alloc_mr() always allocates a PBL via irdma_get_pble(), but
irdma_post_send()'s IB_WR_REG_MR handling only pointed the FAST_REGISTER
WQE at that PBL (chunk_size != 0, first_pm_pbl_index) when
iwmr->npages exceeded IRDMA_MIN_PAGES_PER_FMR (1) - leaving chunk_size
at 0 for a single-page (npages == 1) registration, telling hardware to
use direct/PBL-less addressing for a registration that was never
allocated that way.

This reproduces as data corruption / an IOMMU IO_PAGE_FAULT when a
peer later performs an RDMA WRITE/READ against such a single-page
FRWR-registered MR, even though ib_map_mr_sg() itself reports success.
100% reproducible on iWARP, intermittent on RoCEv2, with fs/smb/smbdirect's
own single-page memory registrations on an Intel E810-CQDA2 (irdma).

Intel's current out-of-tree irdma driver (2.6.96) never gates this on
npages - it always sets chunk_size/first_pm_pbl_index based on the
actual PBL level (PBLE_LEVEL_1 vs PBLE_LEVEL_2), matching what
irdma_alloc_mr() actually allocated. Mirror that here instead of the
npages-based heuristic.

Also drop the reg_addr_pa assignment. irdma_sc_mr_fast_register()
packs reg_addr_pa into the FAST_REGISTER WQE's PBLADDR field, which
hardware only consults when chunk_size == 0 (direct/PBL-less
addressing); with chunk_size now always nonzero for this opcode, that
field is never read for IB_WR_REG_MR and the assignment is dead code -
Intel's driver never sets it for this path either (only the separate,
non-FRWR irdma_sc_mr_reg_non_shared() path actually branches on
chunk_size between reg_addr_pa and first_pm_pbl_index). Removing it is
not required for correctness: *palloc->level1.addr is always a
page-aligned address (low 12 bits zero), so it could never have
collided with the first_pm_pbl_index bits sharing those same bit
positions in the WQE word even if some path still read it.

This leaves one deliberate difference from irdma-2.6.96: that driver also
ties local_fence to the WR's IB_SEND_FENCE flag
(stag_info.local_fence = ib_wr->send_flags & IB_SEND_FENCE) rather than
hardcoding it true for IB_WR_REG_MR. That is a separate, non-corruption
behavioural change, so local_fence is left at true here.

I've tested the change with an Intel-E810-CQDA2 card, lspci show is as:
Intel Corporation Ethernet Controller E810-C for QSFP (rev 02)

Fixes: b48c24c2d710 ("RDMA/irdma: Implement device supported verb APIs")
Cc: Tatyana Nikolova <tatyana.e.nikolova@intel.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Mustafa Ismail <mustafa.ismail@intel.com>
Cc: Shiraz Saleem <shiraz.saleem@intel.com>
Cc: linux-rdma@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Assisted-by: Claude:claude-sonnet-5-5
Tested-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 drivers/infiniband/hw/irdma/verbs.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 5d71300da0ff..a524e8fc7738 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -4279,11 +4279,14 @@ static int irdma_post_send(struct ib_qp *ibqp,
 			stag_info.addr_type = IRDMA_ADDR_TYPE_VA_BASED;
 			stag_info.va = (void *)(uintptr_t)iwmr->ibmr.iova;
 			stag_info.total_len = iwmr->ibmr.length;
-			stag_info.reg_addr_pa = *palloc->level1.addr;
-			stag_info.first_pm_pbl_index = palloc->level1.idx;
-			stag_info.local_fence = true;
-			if (iwmr->npages > IRDMA_MIN_PAGES_PER_FMR)
+			if (palloc->level == PBLE_LEVEL_2) {
+				stag_info.chunk_size = 3;
+				stag_info.first_pm_pbl_index = palloc->level2.root.idx;
+			} else {
 				stag_info.chunk_size = 1;
+				stag_info.first_pm_pbl_index = palloc->level1.idx;
+			}
+			stag_info.local_fence = true;
 			err = irdma_sc_mr_fast_register(&iwqp->sc_qp, &stag_info,
 							true);
 			break;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] RDMA/irdma: always route IB_WR_REG_MR through the allocated PBL
  2026-10-07  9:53 [PATCH] RDMA/irdma: always route IB_WR_REG_MR through the allocated PBL Stefan Metzmacher
@ 2026-10-07 10:26 ` Stefan Metzmacher
  2026-10-12  6:33   ` Leon Romanovsky
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Metzmacher @ 2026-10-07 10:26 UTC (permalink / raw)
  To: linux-rdma
  Cc: Tatyana Nikolova, Jason Gunthorpe, Leon Romanovsky,
	Mustafa Ismail, Shiraz Saleem, linux-kernel

Am 07.10.26 um 11:53 schrieb Stefan Metzmacher:
> irdma_alloc_mr() always allocates a PBL via irdma_get_pble(), but
> irdma_post_send()'s IB_WR_REG_MR handling only pointed the FAST_REGISTER
> WQE at that PBL (chunk_size != 0, first_pm_pbl_index) when
> iwmr->npages exceeded IRDMA_MIN_PAGES_PER_FMR (1) - leaving chunk_size
> at 0 for a single-page (npages == 1) registration, telling hardware to
> use direct/PBL-less addressing for a registration that was never
> allocated that way.
> 
> This reproduces as data corruption / an IOMMU IO_PAGE_FAULT when a
> peer later performs an RDMA WRITE/READ against such a single-page
> FRWR-registered MR, even though ib_map_mr_sg() itself reports success.
> 100% reproducible on iWARP, intermittent on RoCEv2, with fs/smb/smbdirect's
> own single-page memory registrations on an Intel E810-CQDA2 (irdma).
> 
> Intel's current out-of-tree irdma driver (2.6.96) never gates this on
> npages - it always sets chunk_size/first_pm_pbl_index based on the
> actual PBL level (PBLE_LEVEL_1 vs PBLE_LEVEL_2), matching what
> irdma_alloc_mr() actually allocated. Mirror that here instead of the
> npages-based heuristic.
> 
> Also drop the reg_addr_pa assignment. irdma_sc_mr_fast_register()
> packs reg_addr_pa into the FAST_REGISTER WQE's PBLADDR field, which
> hardware only consults when chunk_size == 0 (direct/PBL-less
> addressing); with chunk_size now always nonzero for this opcode, that
> field is never read for IB_WR_REG_MR and the assignment is dead code -
> Intel's driver never sets it for this path either (only the separate,
> non-FRWR irdma_sc_mr_reg_non_shared() path actually branches on
> chunk_size between reg_addr_pa and first_pm_pbl_index). Removing it is
> not required for correctness: *palloc->level1.addr is always a
> page-aligned address (low 12 bits zero), so it could never have
> collided with the first_pm_pbl_index bits sharing those same bit
> positions in the WQE word even if some path still read it.
> 
> This leaves one deliberate difference from irdma-2.6.96: that driver also
> ties local_fence to the WR's IB_SEND_FENCE flag
> (stag_info.local_fence = ib_wr->send_flags & IB_SEND_FENCE) rather than
> hardcoding it true for IB_WR_REG_MR. That is a separate, non-corruption
> behavioural change, so local_fence is left at true here.

I realized to late this was changed just recently in:

commit 3fb905f07ea45b31c8f67ba6e4668de46f527e65
Author:     Jacob Moroni <jmoroni@google.com>
AuthorDate: Tue Sep 1 16:00:14 2026 +0000
Commit:     Leon Romanovsky <leon@kernel.org>
CommitDate: Wed Sep 2 12:27:03 2026 -0400

     RDMA/irdma: Enforce local fence for IB_WR_REG_MR

I also noticed that these two from linux-rdma/for-next
are in conflict:

4a93466061d38f159b857aad63a9cc3cd3affadb RDMA/irdma: Remove unused post_sq arguments
c470050f2a7a7748ce35367e828885a8e658d712 RDMA/irdma: Preserve fast-registration IOVA on 32-bit

What's best way forward here, is this something that
would go via linux-rdma/for-rc or via linux-rdma/for-next?

as we're at rc6 already waiting ~3 weeks before it hits upstream
is fine, I have it in my local test kernels anyway...

Should I resubmit a v2 with the commit message clarified
regarding "RDMA/irdma: Enforce local fence for IB_WR_REG_MR"
and should I base this on top of linux-rdma/for-rc or linux-rdma/for-next?

Thanks!
metze


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] RDMA/irdma: always route IB_WR_REG_MR through the allocated PBL
  2026-10-07 10:26 ` Stefan Metzmacher
@ 2026-10-12  6:33   ` Leon Romanovsky
  0 siblings, 0 replies; 3+ messages in thread
From: Leon Romanovsky @ 2026-10-12  6:33 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: linux-rdma, Tatyana Nikolova, Jason Gunthorpe, Mustafa Ismail,
	Shiraz Saleem, linux-kernel

On Wed, Oct 07, 2026 at 12:26:25PM +0200, Stefan Metzmacher wrote:
> Am 07.10.26 um 11:53 schrieb Stefan Metzmacher:
> > irdma_alloc_mr() always allocates a PBL via irdma_get_pble(), but
> > irdma_post_send()'s IB_WR_REG_MR handling only pointed the FAST_REGISTER
> > WQE at that PBL (chunk_size != 0, first_pm_pbl_index) when
> > iwmr->npages exceeded IRDMA_MIN_PAGES_PER_FMR (1) - leaving chunk_size
> > at 0 for a single-page (npages == 1) registration, telling hardware to
> > use direct/PBL-less addressing for a registration that was never
> > allocated that way.
> > 
> > This reproduces as data corruption / an IOMMU IO_PAGE_FAULT when a
> > peer later performs an RDMA WRITE/READ against such a single-page
> > FRWR-registered MR, even though ib_map_mr_sg() itself reports success.
> > 100% reproducible on iWARP, intermittent on RoCEv2, with fs/smb/smbdirect's
> > own single-page memory registrations on an Intel E810-CQDA2 (irdma).
> > 
> > Intel's current out-of-tree irdma driver (2.6.96) never gates this on
> > npages - it always sets chunk_size/first_pm_pbl_index based on the
> > actual PBL level (PBLE_LEVEL_1 vs PBLE_LEVEL_2), matching what
> > irdma_alloc_mr() actually allocated. Mirror that here instead of the
> > npages-based heuristic.
> > 
> > Also drop the reg_addr_pa assignment. irdma_sc_mr_fast_register()
> > packs reg_addr_pa into the FAST_REGISTER WQE's PBLADDR field, which
> > hardware only consults when chunk_size == 0 (direct/PBL-less
> > addressing); with chunk_size now always nonzero for this opcode, that
> > field is never read for IB_WR_REG_MR and the assignment is dead code -
> > Intel's driver never sets it for this path either (only the separate,
> > non-FRWR irdma_sc_mr_reg_non_shared() path actually branches on
> > chunk_size between reg_addr_pa and first_pm_pbl_index). Removing it is
> > not required for correctness: *palloc->level1.addr is always a
> > page-aligned address (low 12 bits zero), so it could never have
> > collided with the first_pm_pbl_index bits sharing those same bit
> > positions in the WQE word even if some path still read it.
> > 
> > This leaves one deliberate difference from irdma-2.6.96: that driver also
> > ties local_fence to the WR's IB_SEND_FENCE flag
> > (stag_info.local_fence = ib_wr->send_flags & IB_SEND_FENCE) rather than
> > hardcoding it true for IB_WR_REG_MR. That is a separate, non-corruption
> > behavioural change, so local_fence is left at true here.
> 
> I realized to late this was changed just recently in:
> 
> commit 3fb905f07ea45b31c8f67ba6e4668de46f527e65
> Author:     Jacob Moroni <jmoroni@google.com>
> AuthorDate: Tue Sep 1 16:00:14 2026 +0000
> Commit:     Leon Romanovsky <leon@kernel.org>
> CommitDate: Wed Sep 2 12:27:03 2026 -0400
> 
>     RDMA/irdma: Enforce local fence for IB_WR_REG_MR
> 
> I also noticed that these two from linux-rdma/for-next
> are in conflict:
> 
> 4a93466061d38f159b857aad63a9cc3cd3affadb RDMA/irdma: Remove unused post_sq arguments
> c470050f2a7a7748ce35367e828885a8e658d712 RDMA/irdma: Preserve fast-registration IOVA on 32-bit
> 
> What's best way forward here, is this something that
> would go via linux-rdma/for-rc or via linux-rdma/for-next?
> 
> as we're at rc6 already waiting ~3 weeks before it hits upstream
> is fine, I have it in my local test kernels anyway...
> 
> Should I resubmit a v2 with the commit message clarified
> regarding "RDMA/irdma: Enforce local fence for IB_WR_REG_MR"
> and should I base this on top of linux-rdma/for-rc or linux-rdma/for-next?

After -rc4, almost everything goes into the -next branch. The only patches
we still take into -rc are those that fix UAPI breakage, regressions
(introduced in the current cycle, at least), kernel panics in non-debug
builds, builds breakage, and other small issues that make you say, “Oh dear...” :).

Thanks

> 
> Thanks!
> metze
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-12  6:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07  9:53 [PATCH] RDMA/irdma: always route IB_WR_REG_MR through the allocated PBL Stefan Metzmacher
2026-10-07 10:26 ` Stefan Metzmacher
2026-10-12  6:33   ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®