[RFC 1/2] RDMA/umem: Add support for pinned revocable dmabuf import

[RFC 1/2] RDMA/umem: Add support for pinned revocable dmabuf import

[Jacob Moroni]

In order to eventually import a dmabuf from VFIO, pinned importers
will need to support revocation. This can be achieved by allowing
the drivers to provide a revoke callback when obtaining the umem.

The drivers can use this callback to ensure that the region is
invalidated in a way that guarantees no further HW accesses, but,
in the case of an MR, does not actually release the key for reuse
until the region is fully deregistered (i.e., ibv_dereg_mr).

It should be noted that revocation is asynchronous, so drivers that
wish to switch to this new routine must ensure that their internal
state is protected.

Signed-off-by: Jacob Moroni <jmoroni@google.com>
---
 drivers/infiniband/core/umem_dmabuf.c | 109 +++++++++++++++++++++++---
 drivers/infiniband/hw/mlx5/mr.c       |   2 +-
 include/rdma/ib_umem.h                |  27 ++++++-
 3 files changed, 121 insertions(+), 17 deletions(-)

diff --git a/drivers/infiniband/core/umem_dmabuf.c b/drivers/infiniband/core/umem_dmabuf.c
index f5298c33e581..cf97653df9a9 100644
--- a/drivers/infiniband/core/umem_dmabuf.c
+++ b/drivers/infiniband/core/umem_dmabuf.c
@@ -195,22 +195,64 @@ static struct dma_buf_attach_ops ib_umem_dmabuf_attach_pinned_ops = {
 	.move_notify = ib_umem_dmabuf_unsupported_move_notify,
 };
 
+static void __ib_umem_dmabuf_revoke(struct dma_buf_attachment *attach)
+{
+	struct ib_umem_dmabuf *umem_dmabuf = attach->importer_priv;
+
+	dma_resv_assert_held(umem_dmabuf->attach->dmabuf->resv);
+
+	if (umem_dmabuf->revoked)
+		return;
+
+	/* Will be NULL for drivers that do not request a revocable umem, or
+	 * during the (protected) window between attach and pin+map_pages.
+	 */
+	if (umem_dmabuf->revoke)
+		umem_dmabuf->revoke(umem_dmabuf->revoke_priv);
+
+	/* HW should no longer touch the memory at this point. */
+
+	ib_umem_dmabuf_unmap_pages(umem_dmabuf);
+	if (umem_dmabuf->pinned) {
+		dma_buf_unpin(umem_dmabuf->attach);
+		umem_dmabuf->pinned = 0;
+	}
+	umem_dmabuf->revoked = 1;
+}
+
+static struct dma_buf_attach_ops ib_umem_dmabuf_attach_pinned_revocable_ops = {
+	.allow_peer2peer = true,
+	.move_notify = __ib_umem_dmabuf_revoke,
+};
+
 struct ib_umem_dmabuf *
 ib_umem_dmabuf_get_pinned_with_dma_device(struct ib_device *device,
 					  struct device *dma_device,
 					  unsigned long offset, size_t size,
-					  int fd, int access)
+					  int fd, int access,
+					  void (*revoke)(void *priv),
+					  void *revoke_priv)
 {
 	struct ib_umem_dmabuf *umem_dmabuf;
+	struct dma_buf_attach_ops *ops;
 	int err;
 
+	ops = revoke ?
+		&ib_umem_dmabuf_attach_pinned_revocable_ops :
+		&ib_umem_dmabuf_attach_pinned_ops;
+
 	umem_dmabuf = ib_umem_dmabuf_get_with_dma_device(device, dma_device, offset,
-							 size, fd, access,
-							 &ib_umem_dmabuf_attach_pinned_ops);
+							 size, fd, access, ops);
 	if (IS_ERR(umem_dmabuf))
 		return umem_dmabuf;
 
 	dma_resv_lock(umem_dmabuf->attach->dmabuf->resv, NULL);
+
+	if (umem_dmabuf->revoked) {
+		err = -ENODEV;
+		goto err_release;
+	}
+
 	err = dma_buf_pin(umem_dmabuf->attach);
 	if (err)
 		goto err_release;
@@ -219,12 +261,17 @@ ib_umem_dmabuf_get_pinned_with_dma_device(struct ib_device *device,
 	err = ib_umem_dmabuf_map_pages(umem_dmabuf);
 	if (err)
 		goto err_unpin;
+
+	umem_dmabuf->revoke = revoke;
+	umem_dmabuf->revoke_priv = revoke_priv;
+
 	dma_resv_unlock(umem_dmabuf->attach->dmabuf->resv);
 
 	return umem_dmabuf;
 
 err_unpin:
 	dma_buf_unpin(umem_dmabuf->attach);
+	umem_dmabuf->pinned = 0;
 err_release:
 	dma_resv_unlock(umem_dmabuf->attach->dmabuf->resv);
 	ib_umem_release(&umem_dmabuf->umem);
@@ -238,24 +285,60 @@ struct ib_umem_dmabuf *ib_umem_dmabuf_get_pinned(struct ib_device *device,
 						 int access)
 {
 	return ib_umem_dmabuf_get_pinned_with_dma_device(device, device->dma_device,
-							 offset, size, fd, access);
+							 offset, size, fd, access,
+							 NULL, NULL);
 }
 EXPORT_SYMBOL(ib_umem_dmabuf_get_pinned);
 
+/**
+ * ib_umem_dmabuf_get_pinned_revocable - Get a pinned but revocable umem dmabuf.
+ * @device: IB device.
+ * @offset: Start offset.
+ * @size: Length.
+ * @fd: dmabuf fd.
+ * @access: Access flags.
+ * @revoke: Driver revoke callback.
+ * @revoke_priv: Driver revoke callback private data.
+ *
+ * Obtains a umem from a dmabuf for drivers/devices that can support revocation.
+ *
+ * When a revocation occurs, the revoke callback will be called. The driver must
+ * ensure that the region is no longer accessed when the callback returns. Any
+ * subsequent access attempts should also probably cause an AE.
+ *
+ * If the umem is used for an MR, the driver must ensure that the key remains in
+ * use such that it cannot be obtained by a new region until this region is
+ * fully deregistered (i.e., ibv_dereg_mr).
+ *
+ * If a driver needs to serialize with revoke calls, it can use dma_resv_lock to
+ * avoid needing to embed a lock into every MR.
+ *
+ * If successful, then the revoke callback may be called at any time and will
+ * also be called automatically upon ib_umem_release (serialized). The revoke
+ * callback will be called one time at most.
+ *
+ * If unsuccessful, then the revoke callback will never be called.
+ */
+struct ib_umem_dmabuf *
+ib_umem_dmabuf_get_pinned_revocable(struct ib_device *device,
+				    unsigned long offset,
+				    size_t size, int fd,
+				    int access,
+				    void (*revoke)(void *priv),
+				    void *revoke_priv)
+{
+	return ib_umem_dmabuf_get_pinned_with_dma_device(device, device->dma_device,
+							 offset, size, fd, access,
+							 revoke, revoke_priv);
+}
+EXPORT_SYMBOL(ib_umem_dmabuf_get_pinned_revocable);
+
 void ib_umem_dmabuf_revoke(struct ib_umem_dmabuf *umem_dmabuf)
 {
 	struct dma_buf *dmabuf = umem_dmabuf->attach->dmabuf;
 
 	dma_resv_lock(dmabuf->resv, NULL);
-	if (umem_dmabuf->revoked)
-		goto end;
-	ib_umem_dmabuf_unmap_pages(umem_dmabuf);
-	if (umem_dmabuf->pinned) {
-		dma_buf_unpin(umem_dmabuf->attach);
-		umem_dmabuf->pinned = 0;
-	}
-	umem_dmabuf->revoked = 1;
-end:
+	__ib_umem_dmabuf_revoke(umem_dmabuf->attach);
 	dma_resv_unlock(dmabuf->resv);
 }
 EXPORT_SYMBOL(ib_umem_dmabuf_revoke);
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 665323b90b64..ad8b5bcf1b41 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -1648,7 +1648,7 @@ reg_user_mr_dmabuf(struct ib_pd *pd, struct device *dma_device,
 	else if (dma_device)
 		umem_dmabuf = ib_umem_dmabuf_get_pinned_with_dma_device(&dev->ib_dev,
 				dma_device, offset, length,
-				fd, access_flags);
+				fd, access_flags, NULL, NULL);
 	else
 		umem_dmabuf = ib_umem_dmabuf_get_pinned(
 			&dev->ib_dev, offset, length, fd, access_flags);
diff --git a/include/rdma/ib_umem.h b/include/rdma/ib_umem.h
index 0a8e092c0ea8..3d37d5b79dd4 100644
--- a/include/rdma/ib_umem.h
+++ b/include/rdma/ib_umem.h
@@ -36,6 +36,8 @@ struct ib_umem_dmabuf {
 	struct scatterlist *last_sg;
 	unsigned long first_sg_offset;
 	unsigned long last_sg_trim;
+	void (*revoke)(void *priv);
+	void *revoke_priv;
 	void *private;
 	u8 pinned : 1;
 	u8 revoked : 1;
@@ -169,10 +171,19 @@ struct ib_umem_dmabuf *ib_umem_dmabuf_get_pinned(struct ib_device *device,
 						 size_t size, int fd,
 						 int access);
 struct ib_umem_dmabuf *
+ib_umem_dmabuf_get_pinned_revocable(struct ib_device *device,
+				    unsigned long offset,
+				    size_t size, int fd,
+				    int access,
+				    void (*revoke)(void *priv),
+				    void *revoke_priv);
+struct ib_umem_dmabuf *
 ib_umem_dmabuf_get_pinned_with_dma_device(struct ib_device *device,
 					  struct device *dma_device,
 					  unsigned long offset, size_t size,
-					  int fd, int access);
+					  int fd, int access,
+					  void (*revoke)(void *priv),
+					  void *revoke_priv);
 int ib_umem_dmabuf_map_pages(struct ib_umem_dmabuf *umem_dmabuf);
 void ib_umem_dmabuf_unmap_pages(struct ib_umem_dmabuf *umem_dmabuf);
 void ib_umem_dmabuf_release(struct ib_umem_dmabuf *umem_dmabuf);
@@ -220,12 +231,22 @@ ib_umem_dmabuf_get_pinned(struct ib_device *device, unsigned long offset,
 {
 	return ERR_PTR(-EOPNOTSUPP);
 }
-
+static inline struct ib_umem_dmabuf *
+ib_umem_dmabuf_get_pinned_revocable(struct ib_device *device,
+				    unsigned long offset,
+				    size_t size, int fd, int access,
+				    void (*revoke)(void *priv),
+				    void *revoke_priv)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
 static inline struct ib_umem_dmabuf *
 ib_umem_dmabuf_get_pinned_with_dma_device(struct ib_device *device,
 					  struct device *dma_device,
 					  unsigned long offset, size_t size,
-					  int fd, int access)
+					  int fd, int access,
+					  void (*revoke)(void *priv),
+					  void *revoke_priv)
 {
 	return ERR_PTR(-EOPNOTSUPP);
 }
-- 
2.53.0.371.g1d285c8824-goog

[RFC 2/2] RDMA/irdma: Add pinned revocable dmabuf support

[Jacob Moroni]

Some dmabuf exporters (like VFIO) will require that pinned
importers support revocation. In order to support this, the new
ib_umem_dmabuf_get_pinned_revocable method can be used, which
allows the driver to provide a revoke callback for the umem.

Upon revocation, the driver will invalidate the region in HW
so that it is no longer accessed.

It is worth noting that the irdma driver handles MR key allocation
in software; the command submitted to hardware during the revoke
invalidates the key, but the key is not available for reuse until
the region is fully deregistered (i.e., ibv_dereg_mr).

Tested with lockdep+kasan and a modified VFIO that allows pinned
importers by triggering a VFIO_DEVICE_RESET while the region is
registered to ensure that the callback is executed properly.

Signed-off-by: Jacob Moroni <jmoroni@google.com>
---
 drivers/infiniband/hw/irdma/main.h  |   1 +
 drivers/infiniband/hw/irdma/verbs.c | 125 ++++++++++++++++++++--------
 drivers/infiniband/hw/irdma/verbs.h |   1 +
 3 files changed, 94 insertions(+), 33 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/main.h b/drivers/infiniband/hw/irdma/main.h
index d320d1a228b3..240c7977903d 100644
--- a/drivers/infiniband/hw/irdma/main.h
+++ b/drivers/infiniband/hw/irdma/main.h
@@ -20,6 +20,7 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-resv.h>
 #include <linux/workqueue.h>
 #include <linux/slab.h>
 #include <linux/io.h>
diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 15af53237217..c269f0954f82 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -3359,19 +3359,14 @@ static int irdma_reg_user_mr_type_mem(struct irdma_mr *iwmr, int access,
 	return err;
 }
 
-static struct irdma_mr *irdma_alloc_iwmr(struct ib_umem *region,
-					 struct ib_pd *pd, u64 virt,
-					 enum irdma_memreg_type reg_type)
+static int irdma_init_iwmr(struct irdma_mr *iwmr, struct ib_umem *region,
+			   struct ib_pd *pd, u64 virt,
+			   enum irdma_memreg_type reg_type)
 {
 	struct irdma_device *iwdev = to_iwdev(pd->device);
 	struct irdma_pbl *iwpbl;
-	struct irdma_mr *iwmr;
 	unsigned long pgsz_bitmap;
 
-	iwmr = kzalloc_obj(*iwmr);
-	if (!iwmr)
-		return ERR_PTR(-ENOMEM);
-
 	iwpbl = &iwmr->iwpbl;
 	iwpbl->iwmr = iwmr;
 	iwmr->region = region;
@@ -3384,21 +3379,14 @@ static struct irdma_mr *irdma_alloc_iwmr(struct ib_umem *region,
 		iwdev->rf->sc_dev.hw_attrs.page_size_cap : SZ_4K;
 
 	iwmr->page_size = ib_umem_find_best_pgsz(region, pgsz_bitmap, virt);
-	if (unlikely(!iwmr->page_size)) {
-		kfree(iwmr);
-		return ERR_PTR(-EOPNOTSUPP);
-	}
+	if (unlikely(!iwmr->page_size))
+		return -EOPNOTSUPP;
 
 	iwmr->len = region->length;
 	iwpbl->user_base = virt;
 	iwmr->page_cnt = ib_umem_num_dma_blocks(region, iwmr->page_size);
 
-	return iwmr;
-}
-
-static void irdma_free_iwmr(struct irdma_mr *iwmr)
-{
-	kfree(iwmr);
+	return 0;
 }
 
 static int irdma_reg_user_mr_type_qp(struct irdma_mem_reg_req req,
@@ -3547,12 +3535,16 @@ static struct ib_mr *irdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
 		return ERR_PTR(-EFAULT);
 	}
 
-	iwmr = irdma_alloc_iwmr(region, pd, virt, req.reg_type);
-	if (IS_ERR(iwmr)) {
+	iwmr = kzalloc_obj(*iwmr);
+	if (!iwmr) {
 		ib_umem_release(region);
-		return (struct ib_mr *)iwmr;
+		return ERR_PTR(-ENOMEM);
 	}
 
+	err = irdma_init_iwmr(iwmr, region, pd, virt, req.reg_type);
+	if (err)
+		goto error;
+
 	switch (req.reg_type) {
 	case IRDMA_MEMREG_TYPE_QP:
 		err = irdma_reg_user_mr_type_qp(req, udata, iwmr);
@@ -3585,11 +3577,39 @@ static struct ib_mr *irdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
 	return &iwmr->ibmr;
 error:
 	ib_umem_release(region);
-	irdma_free_iwmr(iwmr);
+	kfree(iwmr);
 
 	return ERR_PTR(err);
 }
 
+static int irdma_hwdereg_mr(struct ib_mr *ib_mr);
+
+static void irdma_umem_dmabuf_revoke(void *priv)
+{
+	struct irdma_mr *iwmr = priv;
+	int err;
+
+	iwmr->revoked = true;
+
+	if (!iwmr->is_hwreg)
+		return;
+
+	/* Invalidate the key in hardware. This does not actually release the
+	 * key for potential reuse - that only occurs when the region is fully
+	 * deregistered.
+	 */
+	err = irdma_hwdereg_mr(&iwmr->ibmr);
+	if (err) {
+		struct irdma_device *iwdev = to_iwdev(iwmr->ibmr.device);
+
+		ibdev_err(&iwdev->ibdev, "dmabuf mr invalidate failed %d", err);
+		if (!iwdev->rf->reset) {
+			iwdev->rf->reset = true;
+			iwdev->rf->gen_ops.request_reset(iwdev->rf);
+		}
+	}
+}
+
 static struct ib_mr *irdma_reg_user_mr_dmabuf(struct ib_pd *pd, u64 start,
 					      u64 len, u64 virt,
 					      int fd, int access,
@@ -3607,31 +3627,45 @@ static struct ib_mr *irdma_reg_user_mr_dmabuf(struct ib_pd *pd, u64 start,
 	if (len > iwdev->rf->sc_dev.hw_attrs.max_mr_size)
 		return ERR_PTR(-EINVAL);
 
-	umem_dmabuf = ib_umem_dmabuf_get_pinned(pd->device, start, len, fd, access);
+	iwmr = kzalloc_obj(*iwmr);
+	if (!iwmr)
+		return ERR_PTR(-ENOMEM);
+
+	umem_dmabuf =
+		ib_umem_dmabuf_get_pinned_revocable(pd->device, start, len, fd,
+						    access,
+						    irdma_umem_dmabuf_revoke,
+						    iwmr);
 	if (IS_ERR(umem_dmabuf)) {
 		ibdev_dbg(&iwdev->ibdev, "Failed to get dmabuf umem[%pe]\n",
 			  umem_dmabuf);
+		kfree(iwmr);
 		return ERR_CAST(umem_dmabuf);
 	}
 
-	iwmr = irdma_alloc_iwmr(&umem_dmabuf->umem, pd, virt, IRDMA_MEMREG_TYPE_MEM);
-	if (IS_ERR(iwmr)) {
-		err = PTR_ERR(iwmr);
+	err = irdma_init_iwmr(iwmr, &umem_dmabuf->umem, pd, virt,
+			      IRDMA_MEMREG_TYPE_MEM);
+	if (err)
 		goto err_release;
-	}
 
-	err = irdma_reg_user_mr_type_mem(iwmr, access, true);
+	dma_resv_lock(umem_dmabuf->attach->dmabuf->resv, NULL);
+	/* Catch revocations that occur before grabbing dma_resv_lock. */
+	err = iwmr->revoked ?
+		-ENODEV : irdma_reg_user_mr_type_mem(iwmr, access, true);
+	dma_resv_unlock(umem_dmabuf->attach->dmabuf->resv);
+
 	if (err)
-		goto err_iwmr;
+		goto err_release;
 
 	return &iwmr->ibmr;
 
-err_iwmr:
-	irdma_free_iwmr(iwmr);
-
 err_release:
+	/* ib_umem_release will result in the irdma_umem_dmabuf_revoke callback
+	 * being called, but it ends up being a no-op if the region has not been
+	 * successfully registered with HW because iwmr->is_hwreg is false.
+	 */
 	ib_umem_release(&umem_dmabuf->umem);
-
+	kfree(iwmr);
 	return ERR_PTR(err);
 }
 
@@ -3899,6 +3933,28 @@ static void irdma_del_memlist(struct irdma_mr *iwmr,
 	}
 }
 
+/**
+ * irdma_dereg_mr_dmabuf - deregister a dmabuf mr
+ * @iwdev: iwarp device
+ * @iwmr: mr
+ */
+static int irdma_dereg_mr_dmabuf(struct irdma_device *iwdev,
+				 struct irdma_mr *iwmr)
+{
+	struct irdma_pbl *iwpbl = &iwmr->iwpbl;
+
+	/* Causes a synchronous revoke which then causes HW invalidation. */
+	ib_umem_release(iwmr->region);
+
+	irdma_free_stag(iwdev, iwmr->stag);
+
+	if (iwpbl->pbl_allocated)
+		irdma_free_pble(iwdev->rf->pble_rsrc, &iwpbl->pble_alloc);
+
+	kfree(iwmr);
+	return 0;
+}
+
 /**
  * irdma_dereg_mr - deregister mr
  * @ib_mr: mr ptr for dereg
@@ -3911,6 +3967,9 @@ static int irdma_dereg_mr(struct ib_mr *ib_mr, struct ib_udata *udata)
 	struct irdma_pbl *iwpbl = &iwmr->iwpbl;
 	int ret;
 
+	if (iwmr->region && iwmr->region->is_dmabuf)
+		return irdma_dereg_mr_dmabuf(iwdev, iwmr);
+
 	if (iwmr->type != IRDMA_MEMREG_TYPE_MEM) {
 		if (iwmr->region) {
 			struct irdma_ucontext *ucontext;
diff --git a/drivers/infiniband/hw/irdma/verbs.h b/drivers/infiniband/hw/irdma/verbs.h
index aabbb3442098..612c66c91db4 100644
--- a/drivers/infiniband/hw/irdma/verbs.h
+++ b/drivers/infiniband/hw/irdma/verbs.h
@@ -113,6 +113,7 @@ struct irdma_mr {
 	int access;
 	bool is_hwreg:1;
 	bool dma_mr:1;
+	bool revoked:1;
 	u16 type;
 	u32 page_cnt;
 	u64 page_size;
-- 
2.53.0.371.g1d285c8824-goog

Re: [RFC 2/2] RDMA/irdma: Add pinned revocable dmabuf support

[Jason Gunthorpe]

On Mon, Feb 23, 2026 at 07:53:33PM +0000, Jacob Moroni wrote:

> +static void irdma_umem_dmabuf_revoke(void *priv)
> +{
> +	struct irdma_mr *iwmr = priv;
> +	int err;
> +
> +	iwmr->revoked = true;
> +
> +	if (!iwmr->is_hwreg)
> +		return;
> +
> +	/* Invalidate the key in hardware. This does not actually release the
> +	 * key for potential reuse - that only occurs when the region is fully
> +	 * deregistered.
> +	 */
> +	err = irdma_hwdereg_mr(&iwmr->ibmr);
> +	if (err) {
> +		struct irdma_device *iwdev = to_iwdev(iwmr->ibmr.device);
> +
> +		ibdev_err(&iwdev->ibdev, "dmabuf mr invalidate failed %d", err);
> +		if (!iwdev->rf->reset) {
> +			iwdev->rf->reset = true;
> +			iwdev->rf->gen_ops.request_reset(iwdev->rf);
> +		}
> +	}
> +}
> +
>  static struct ib_mr *irdma_reg_user_mr_dmabuf(struct ib_pd *pd, u64 start,
>  					      u64 len, u64 virt,
>  					      int fd, int access,
> @@ -3607,31 +3627,45 @@ static struct ib_mr *irdma_reg_user_mr_dmabuf(struct ib_pd *pd, u64 start,
>  	if (len > iwdev->rf->sc_dev.hw_attrs.max_mr_size)
>  		return ERR_PTR(-EINVAL);
>  
> -	umem_dmabuf = ib_umem_dmabuf_get_pinned(pd->device, start, len, fd, access);
> +	iwmr = kzalloc_obj(*iwmr);
> +	if (!iwmr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	umem_dmabuf =
> +		ib_umem_dmabuf_get_pinned_revocable(pd->device, start, len, fd,
> +						    access,
> +						    irdma_umem_dmabuf_revoke,
> +						    iwmr);
>  	if (IS_ERR(umem_dmabuf)) {
>  		ibdev_dbg(&iwdev->ibdev, "Failed to get dmabuf umem[%pe]\n",
>  			  umem_dmabuf);
> +		kfree(iwmr);
>  		return ERR_CAST(umem_dmabuf);
>  	}
>  
> -	iwmr = irdma_alloc_iwmr(&umem_dmabuf->umem, pd, virt, IRDMA_MEMREG_TYPE_MEM);
> -	if (IS_ERR(iwmr)) {
> -		err = PTR_ERR(iwmr);
> +	err = irdma_init_iwmr(iwmr, &umem_dmabuf->umem, pd, virt,
> +			      IRDMA_MEMREG_TYPE_MEM);

Is it OK to call irdma_hwdereg_mr() before this? Seems really sketchy

I think if revoke is being used you have to use a protocol where the
dmabuf reservation lock is left held for the caller to complete setup
so you don't have revoke races.

I guess this is some ib_umem_dmabuf_get_pinned_revocable_and_lock()
pattern

Maybe it can be some two step process:

 ib_umem_dmabuf_get_pinned_locked()

 [..]
 ib_umem_dmabuf_set_pinned_revocable()

 dma_buf__resv_unlock()


?

Then you don't need to restructure all the iwmr allocation (which
should be a seperated patch too)

Jason

Re: [RFC 2/2] RDMA/irdma: Add pinned revocable dmabuf support

[Jacob Moroni]

Thanks for reviewing again.

> Is it OK to call irdma_hwdereg_mr() before this? Seems really sketchy

This should be okay because irdma_hwdereg_mr won't be called unless
is_hwreg is true, and it's initialized to false when iwmr is allocated (prior to
the callback being registered). This will become less sketchy if we use the
approach you described below though.

> I think if revoke is being used you have to use a protocol where the
> dmabuf reservation lock is left held for the caller to complete setup
> so you don't have revoke races.

> Maybe it can be some two step process:

Yeah, that would certainly make things easier on the irdma driver side, both
by avoiding races and also by not forcing the MR to exist at the time of the
call (many drivers do have the MR allocated at this point, but at least bnxt_re
doesn't from what I can tell).

I've created a new revision; I will send it out now as a formal patch series.

Thanks,
Jake