Re: [PATCH for-next] RDMA/vmw_pvrdma: Use resource ids from physical device if available

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/16/19 11:29 AM, Jason Gunthorpe wrote:
> On Thu, May 16, 2019 at 06:24:48PM +0000, Adit Ranadive wrote:
>> From: Bryan Tan <bryantan@xxxxxxxxxx>
>>
>> This change allows the RDMA stack to use physical resource numbers if
>> they are passed up from the device. Doing so allows communication with
>> physical non-ESX endpoints (such as a bare-metal Linux machine or a
>> SR-IOV-enabled VM).
>>
>> 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 QP number assigned to 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.
>>
>> A new version of the create QP response has been added to the device
>> API. The device backend will pass the QP handle up to the driver, if
>> both the device and driver are at the appriopriate version, and the ABI
>> has also been updated to allow the return of the QP handle to the guest
>> library. The PVRDMA version and ABI version have been bumped up because
>> of these non-compatible changes.
>>
>> Reviewed-by: Jorgen Hansen <jhansen@xxxxxxxxxx>
>> Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx>
>> Signed-off-by: Bryan Tan <bryantan@xxxxxxxxxx>
>> ---
>>  drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 15 +++++++++++++-
>>  drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c    |  8 +++++++-
>>  drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c      | 24 +++++++++++++++++++++--
>>  include/uapi/rdma/vmw_pvrdma-abi.h                |  9 ++++++++-
>>  4 files changed, 51 insertions(+), 5 deletions(-)
>> ---
>>
>> The PR for userspace was sent:
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinux-rdma%2Frdma-core%2Fpull%2F531&amp;data=02%7C01%7Caditr%40vmware.com%7Cd4b0e1edecb04a68a4b408d6da2c6295%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636936281516146044&amp;sdata=73ulUeBGYrnZUKnFRXyOAARBPsAyrONYAmLeksaGzDI%3D&amp;reserved=0
>>
>> ---
>> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
>> index 8f9749d54688..86a6c054ea26 100644
>> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
>> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h
>> @@ -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 @@ struct pvrdma_cmd_destroy_bind {
>>  	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;
>> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>> index 40182297f87f..02e337837a2e 100644
>> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
>> @@ -201,7 +201,13 @@ static int pvrdma_register_device(struct pvrdma_dev *dev)
>>  	dev->ib_dev.owner = THIS_MODULE;
>>  	dev->ib_dev.num_comp_vectors = 1;
>>  	dev->ib_dev.dev.parent = &dev->pdev->dev;
>> -	dev->ib_dev.uverbs_abi_ver = PVRDMA_UVERBS_ABI_VERSION;
>> +
>> +	if (dev->dsr_version >= PVRDMA_QPHANDLE_VERSION)
>> +		dev->ib_dev.uverbs_abi_ver = PVRDMA_UVERBS_ABI_VERSION;
>> +	else
>> +		dev->ib_dev.uverbs_abi_ver =
>> +				PVRDMA_UVERBS_NO_QP_HANDLE_ABI_VERSION;
>> +
>>  	dev->ib_dev.uverbs_cmd_mask =
>>  		(1ull << IB_USER_VERBS_CMD_GET_CONTEXT)		|
>>  		(1ull << IB_USER_VERBS_CMD_QUERY_DEVICE)	|
>> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
>> index 0eaaead5baec..8cba7623f379 100644
>> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
>> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_qp.c
>> @@ -195,7 +195,9 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
>>  	union pvrdma_cmd_resp rsp;
>>  	struct pvrdma_cmd_create_qp *cmd = &req.create_qp;
>>  	struct pvrdma_cmd_create_qp_resp *resp = &rsp.create_qp_resp;
>> +	struct pvrdma_cmd_create_qp_resp_v2 *resp_v2 = &rsp.create_qp_resp_v2;
>>  	struct pvrdma_create_qp ucmd;
>> +	struct pvrdma_create_qp_resp qp_resp = {};
>>  	unsigned long flags;
>>  	int ret;
>>  	bool is_srq = !!init_attr->srq;
>> @@ -379,13 +381,31 @@ struct ib_qp *pvrdma_create_qp(struct ib_pd *pd,
>>  	}
>>  
>>  	/* max_send_wr/_recv_wr/_send_sge/_recv_sge/_inline_data */
>> -	qp->qp_handle = resp->qpn;
>>  	qp->port = init_attr->port_num;
>> -	qp->ibqp.qp_num = resp->qpn;
>> +	if (dev->dsr_version >= PVRDMA_QPHANDLE_VERSION) {
>> +		qp->ibqp.qp_num = resp_v2->qpn;
>> +		qp->qp_handle = resp_v2->qp_handle;
>> +	} else {
>> +		qp->ibqp.qp_num = resp->qpn;
>> +		qp->qp_handle = resp->qpn;
>> +	}
>> +
>>  	spin_lock_irqsave(&dev->qp_tbl_lock, flags);
>>  	dev->qp_tbl[qp->qp_handle % dev->dsr->caps.max_qp] = qp;
>>  	spin_unlock_irqrestore(&dev->qp_tbl_lock, flags);
>>  
>> +	if (!qp->is_kernel) {
>> +		/* Copy udata back. */
>> +		qp_resp.qpn = qp->ibqp.qp_num;
>> +		qp_resp.qp_handle = qp->qp_handle;
>> +		if (ib_copy_to_udata(udata, &qp_resp, sizeof(qp_resp))) {
>> +			dev_warn(&dev->pdev->dev,
>> +				 "failed to copy back udata\n");
>> +			pvrdma_destroy_qp(&qp->ibqp, udata);
>> +			return ERR_PTR(-EINVAL);
>> +		}
>> +	}
>> +
>>  	return &qp->ibqp;
>>  
>>  err_pdir:
>> diff --git a/include/uapi/rdma/vmw_pvrdma-abi.h b/include/uapi/rdma/vmw_pvrdma-abi.h
>> index 6e73f0274e41..8ebab11dadcb 100644
>> --- a/include/uapi/rdma/vmw_pvrdma-abi.h
>> +++ b/include/uapi/rdma/vmw_pvrdma-abi.h
>> @@ -49,7 +49,9 @@
>>  
>>  #include <linux/types.h>
>>  
>> -#define PVRDMA_UVERBS_ABI_VERSION	3		/* ABI Version. */
>> +#define PVRDMA_UVERBS_NO_QP_HANDLE_ABI_VERSION	3
>> +#define PVRDMA_UVERBS_ABI_VERSION		4	/* ABI Version. */
> 
> Don't mess with ABI version when all you are doing is making the
> response or request struct longer.

Hmm, I thought we always had to update the ABI in case of such
changes as well? Previously, we weren't copying out qpresp to udata at all
and this adds a new response to user-space.
Also, wouldn't older rdma-core probably break because of this since it 
assumes the qpn reported is the virtual one? I guess that's a bug already
from a backward compat perspective..

> Jason
> 





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux