Skip to contentSkip to conversation

IB/mlx4: Fill in the access_flags if IB_MR_REREG_ACCESS is not specified

PATCH Applied Jason Gunthorpe 1 participant 2 messages Jun 5 – Jun 9

Subsystems (2)
InfiniBand Mellanox MLX4 IB driver
lore
Share & tools
lore

Jason Gunthorpe

author maintainer
#0 lore raw

PATCH 1/1 IB/mlx4: Fill in the access_flags if IB_MR_REREG_ACCESS is not specified

Sashiko noticed mlx4 was using whatever random access flags were provided when IB_MR_REREG_ACCESS is not used. Since IB_MR_REREG_TRANS needs access_flags it used the random ones which means it doesn't work sensibly if userspace provides only IB_MR_REREG_TRANS.

Keep track of the current access_flag of the MR and use it if the user does not specify one.

Also fixup a little confusion around mmr.access, it is the HW access flags so the convert_access() was missing. But nothing reads this by the time rereg_mr can happen.

Fixes 9376932d0c26 ("IB/mlx4_ib: Add support for user MR re-registration")
Signed-off-by Jason Gunthorpe <[email protected]>
---
 drivers/infiniband/hw/mlx4/mlx4_ib.h | 1 +
 drivers/infiniband/hw/mlx4/mr.c      | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Sashiko also noticed the error unwinds are broken too, but properly fixing that that requires switching to use the new mr replacement rereg flow which is not worth doing for this ancient driver.

drivers/infiniband/hw/mlx4/mlx4_ib.h +1 -0
index 5a799d6df93ebc..898c8363422abb 100644
@@ -135,6 +135,7 @@ struct mlx4_ib_mr {
 	dma_addr_t		page_map;
 	u32			npages;
 	u32			max_pages;
+	int			access_flags;
 	struct mlx4_mr		mmr;
 	struct ib_umem	       *umem;
 	size_t			page_map_size;
drivers/infiniband/hw/mlx4/mr.c +7 -2
index 306704f54325cf..de076b176b0ec8 100644
@@ -181,6 +181,7 @@ struct ib_mr *mlx4_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 	if (err)
 		goto err_mr;
 
+	mr->access_flags = access_flags;
 	mr->ibmr.rkey = mr->ibmr.lkey = mr->mmr.key;
 	mr->ibmr.page_size = 1U << shift;
 

@@ -235,6 +236,8 @@ struct ib_mr *mlx4_ib_rereg_user_mr(struct ib_mr *mr, int flags, u64 start,
 
 		if (err)
 			goto release_mpt_entry;
+	} else {
+		mr_access_flags = mmr->access_flags;
 	}
 
 	if (flags & IB_MR_REREG_TRANS) {

@@ -276,8 +279,10 @@ struct ib_mr *mlx4_ib_rereg_user_mr(struct ib_mr *mr, int flags, u64 start,
 	 * return a failure. But dereg_mr will free the resources.
 	 */
 	err = mlx4_mr_hw_write_mpt(dev->dev, &mmr->mmr, pmpt_entry);
-	if (!err && flags & IB_MR_REREG_ACCESS)
-		mmr->mmr.access = mr_access_flags;
+	if (!err && flags & IB_MR_REREG_ACCESS) {
+		mmr->access_flags = mr_access_flags;
+		mmr->mmr.access = convert_access(mr_access_flags);
+	}
 
 release_mpt_entry:
 	mlx4_mr_hw_put_mpt(dev->dev, pmpt_entry);

base-commit: 8f064f2e2bd23f84dbfba26b83e08d5de7311d44

Signature · 1 line
2.43.0
author maintainer Applied
#1 lore raw
Quoted 14 lines · Jason Gunthorpe

Sashiko noticed mlx4 was using whatever random access flags were provided when IB_MR_REREG_ACCESS is not used. Since IB_MR_REREG_TRANS needs access_flags it used the random ones which means it doesn't work sensibly if userspace provides only IB_MR_REREG_TRANS.

Keep track of the current access_flag of the MR and use it if the user does not specify one.

Also fixup a little confusion around mmr.access, it is the HW access flags so the convert_access() was missing. But nothing reads this by the time rereg_mr can happen.

Fixes 9376932d0c26 ("IB/mlx4_ib: Add support for user MR re-registration")
Signed-off-by Jason Gunthorpe <[email protected]>
Quoted 8 lines
---
 drivers/infiniband/hw/mlx4/mlx4_ib.h | 1 +
 drivers/infiniband/hw/mlx4/mr.c      | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Sashiko also noticed the error unwinds are broken too, but properly fixing that that requires switching to use the new mr replacement rereg flow which is not worth doing for this ancient driver.

Applied to for-next

Jason