Skip to contentSkip to conversation

RDMA/vmw_pvrdma: Use resource ids from physical device if available

PATCH Applied Adit Ranadive 2 participants 3 messages Oct 28, 2019

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 RDMA/vmw_pvrdma: Use resource ids from physical device if available

From: Bryan Tan <[email protected]>

This change allows the RDMA stack to use physical resource numbers if they are passed up from the device. This is accomplished by separating the concept of the QP number from the QP handle. Previously, the two were the same, as the QP number was exposed to the guest and also used to reference a virtual QP in the device backend.

With physical resource numbers exposed, the QP number given to the guest is the number assigned from the physical HCA's QP, while the QP handle is still the internal handle used to reference a virtual QP. Regardless of whether the device is exposing physical ids, the driver will still try to pick up the QP handle from the backend if possible. The MR keys exposed to the guest will also be the MR keys created by the physical HCA, instead of virtual MR keys. The distinction between handle and keys is already present for MRs so there is no need to do anything special here.

A new version of the create QP response has been added to the device API to pass up the QP number and handle. The driver will also report these to userspace in the udata response if userspace supports it or not create the queuepair if not. I also had to do a refactor of the destroy qp code to reuse it if we fail to copy to userspace.

---

Github PR for rdma-core:
- https://github.com/linux-rdma/rdma-core/pull/581

Changes:
v2 -> v3:
- Use < for size check for outlen and minimum for copying to udata
v1 -> v2:
- Dropped the create qp flag
v0 -> v1:
- Dropped the ABI version bump
- Added a new create qp flag to indicate userspace support
- Refactor of destroy/free qp
- Updated commit description

 .../infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h |  15 ++-
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c  | 118 +++++++++++++-----
 include/uapi/rdma/vmw_pvrdma-abi.h            |   5 +
 3 files changed, 109 insertions(+), 29 deletions(-)
drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +14 -1
index 8f9749d54688..86a6c054ea26 100644
@@ -58,7 +58,8 @@
 #define PVRDMA_ROCEV1_VERSION		17
 #define PVRDMA_ROCEV2_VERSION		18
 #define PVRDMA_PPN64_VERSION		19
-#define PVRDMA_VERSION			PVRDMA_PPN64_VERSION
+#define PVRDMA_QPHANDLE_VERSION		20
+#define PVRDMA_VERSION			PVRDMA_QPHANDLE_VERSION
 
 #define PVRDMA_BOARD_ID			1
 #define PVRDMA_REV_ID			1
@@ -581,6 +582,17 @@ struct pvrdma_cmd_create_qp_resp {
 	u32 max_inline_data;
 };
 
+struct pvrdma_cmd_create_qp_resp_v2 {
+	struct pvrdma_cmd_resp_hdr hdr;
+	u32 qpn;
+	u32 qp_handle;
+	u32 max_send_wr;
+	u32 max_recv_wr;
+	u32 max_send_sge;
+	u32 max_recv_sge;
+	u32 max_inline_data;
+};
+
 struct pvrdma_cmd_modify_qp {
 	struct pvrdma_cmd_hdr hdr;
 	u32 qp_handle;
@@ -663,6 +675,7 @@ union pvrdma_cmd_resp {
 	struct pvrdma_cmd_create_cq_resp create_cq_resp;
 	struct pvrdma_cmd_resize_cq_resp resize_cq_resp;
 	struct pvrdma_cmd_create_qp_resp create_qp_resp;
+	struct pvrdma_cmd_create_qp_resp_v2 create_qp_resp_v2;
 	struct pvrdma_cmd_query_qp_resp query_qp_resp;
 	struct pvrdma_cmd_destroy_qp_resp destroy_qp_resp;
 	struct pvrdma_cmd_create_srq_resp create_srq_resp;
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c +90 -28

183 lines not on this page · raw message

include/uapi/rdma/vmw_pvrdma-abi.h +5 -0
index 6e73f0274e41..f8b638c73371 100644
@@ -179,6 +179,11 @@ struct pvrdma_create_qp {
 	__aligned_u64 qp_addr;
 };
 
+struct pvrdma_create_qp_resp {
+	__u32 qpn;
+	__u32 qp_handle;
+};
+
 /* PVRDMA masked atomic compare and swap */
 struct pvrdma_ex_cmp_swap {
 	__aligned_u64 swap_val;
Signature · 1 line
2.18.1
#1 lore raw
Quoted 6 lines · Adit Ranadive
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c +5 -0
+	if (!qp->is_kernel) {
+		if (udata->outlen >= sizeof(qp_resp)) {
+			qp_resp.qpn = qp->ibqp.qp_num;
+			qp_resp.qp_handle = qp->qp_handle;
+

drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c struct ib_qp *pvrdma_create_qp(struct ib_pd *pd, · line 424

Quoted 10 lines
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c +10 -0
+			if (ib_copy_to_udata(udata, &qp_resp,
+					     min(udata->outlen,
+						 sizeof(qp_resp)))) {
+				dev_warn(&dev->pdev->dev,
+					 "failed to copy back udata\n");
+				__pvrdma_destroy_qp(dev, qp);
+				return ERR_PTR(-EINVAL);
+			}
+		}
+	}

This is just supposed to be like this:

+ if (udata) {
+ qp_resp.qpn = qp->ibqp.qp_num;
+ qp_resp.qp_handle = qp->qp_handle;
+
+ if (ib_copy_to_udata(udata, &qp_resp,
+ min(udata->outlen, sizeof(qp_resp)))) {
+ dev_warn(&dev->pdev->dev,
+ "failed to copy back udata\n");
+ __pvrdma_destroy_qp(dev, qp);
+ return ERR_PTR(-EINVAL);

I fixed it

Applied to for-next

Thanks,
Jason

#2 lore raw
Quoted 32 lines · Jason Gunthorpe
Quoted 6 lines · Adit Ranadive
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c +5 -0
+	if (!qp->is_kernel) {
+		if (udata->outlen >= sizeof(qp_resp)) {
+			qp_resp.qpn = qp->ibqp.qp_num;
+			qp_resp.qp_handle = qp->qp_handle;
+

drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c struct ib_qp *pvrdma_create_qp(struct ib_pd *pd, · line 424

Quoted 10 lines
drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c +10 -0
+			if (ib_copy_to_udata(udata, &qp_resp,
+					     min(udata->outlen,
+						 sizeof(qp_resp)))) {
+				dev_warn(&dev->pdev->dev,
+					 "failed to copy back udata\n");
+				__pvrdma_destroy_qp(dev, qp);
+				return ERR_PTR(-EINVAL);
+			}
+		}
+	}

This is just supposed to be like this:

drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c +10 -0
+       if (udata) {
+               qp_resp.qpn = qp->ibqp.qp_num;
+               qp_resp.qp_handle = qp->qp_handle;
+
+               if (ib_copy_to_udata(udata, &qp_resp,
+                                    min(udata->outlen, sizeof(qp_resp)))) {
+                       dev_warn(&dev->pdev->dev,
+                                "failed to copy back udata\n");
+                       __pvrdma_destroy_qp(dev, qp);
+                       return ERR_PTR(-EINVAL);
Quoted 4 lines

I fixed it

Applied to for-next

Makes sense. Thanks!

Quoted 3 lines

Thanks,
Jason