Add support for 0xefa1 device

PATCH Applied Gal Pressman 2 participants 10 messages Jul 22, 2020 – Jul 29, 2020

Subsystems (2)

InfiniBand Amazon RDMA efa driver

lore

Share & tools

lore

Gal Pressman

author

2020-07-22 14:08 UTC #0 lore raw

v4 cover Add support for 0xefa1 device

Hi all,

The following submission adds the needed functionality in order to support 0xefa1 devices, and adds it to the driver pci table.

PR was sent:
https://github.com/linux-rdma/rdma-core/pull/789

Changelog -
v3->v4: https://lore.kernel.org/linux-rdma/[email protected]/
* Remove the user_comp_handshakes array and use the macros explicitly.
* Make efa_user_comp_handshake static.
v2->v3: https://lore.kernel.org/linux-rdma/[email protected]/
* Remove gcc specific stuff to fix clang compilation.
v1->v2: https://lore.kernel.org/linux-rdma/[email protected]/
* Add handshake with userspace provider to verify required features support.

Regards,
Gal

Gal Pressman (4):
RDMA/efa: Expose maximum TX doorbell batch RDMA/efa: Expose minimum SQ size
RDMA/efa: User/kernel compatibility handshake mechanism RDMA/efa: Add EFA 0xefa1 PCI ID

 .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 15 ++++++-
 drivers/infiniband/hw/efa/efa_com_cmd.c       |  2 +
 drivers/infiniband/hw/efa/efa_com_cmd.h       |  2 +
 drivers/infiniband/hw/efa/efa_main.c          |  6 ++-
 drivers/infiniband/hw/efa/efa_verbs.c         | 42 +++++++++++++++++++
 include/uapi/rdma/efa-abi.h                   | 15 ++++++-
 6 files changed, 77 insertions(+), 5 deletions(-)

base-commit: 8e7eafb816ab7e5047b74cb8eb1db2f8f14f7d7a

Signature · 1 line

2.27.0

Gal Pressman

author

2020-07-22 14:09 UTC #1 lore raw

PATCH v4 1/4 RDMA/efa: Expose maximum TX doorbell batch

The device reports the maximum number of bytes to be written before ringing the doorbell (zero means unlimited).

This patch queries the max batch size and reports it back to the userspace library.

Reviewed-by Daniel Kranzdorf <[email protected]>

Reviewed-by Firas JahJah <[email protected]>

Signed-off-by Gal Pressman <[email protected]>

---
 drivers/infiniband/hw/efa/efa_admin_cmds_defs.h | 11 +++++++++++
 drivers/infiniband/hw/efa/efa_com_cmd.c         |  1 +
 drivers/infiniband/hw/efa/efa_com_cmd.h         |  1 +
 drivers/infiniband/hw/efa/efa_verbs.c           |  1 +
 include/uapi/rdma/efa-abi.h                     |  4 +++-
 5 files changed, 17 insertions(+), 1 deletion(-)

drivers/infiniband/hw/efa/efa_admin_cmds_defs.h +11 -0

index bef2bd291054..03e7388af06e 100644
@@ -632,6 +632,17 @@ struct efa_admin_feature_queue_attr_desc {
 
 	/* Maximum number of SGEs for a single RDMA read WQE */
 	u16 max_wr_rdma_sges;
+
+	/*
+	 * Maximum number of bytes that can be written to SQ between two
+	 * consecutive doorbells (in units of 64B). Driver must ensure that only
+	 * complete WQEs are written to queue before issuing a doorbell.
+	 * Examples: max_tx_batch=16 and WQE size = 64B, means up to 16 WQEs can
+	 * be written to SQ between two consecutive doorbells. max_tx_batch=11
+	 * and WQE size = 128B, means up to 5 WQEs can be written to SQ between
+	 * two consecutive doorbells. Zero means unlimited.
+	 */
+	u16 max_tx_batch;
 };
 
 struct efa_admin_feature_aenq_desc {

drivers/infiniband/hw/efa/efa_com_cmd.c +1 -0

index fabd8df2e78f..53cfde5c43d8 100644
@@ -480,6 +480,7 @@ int efa_com_get_device_attr(struct efa_com_dev *edev,
 	result->max_llq_size = resp.u.queue_attr.max_llq_size;
 	result->sub_cqs_per_cq = resp.u.queue_attr.sub_cqs_per_cq;
 	result->max_wr_rdma_sge = resp.u.queue_attr.max_wr_rdma_sges;
+	result->max_tx_batch = resp.u.queue_attr.max_tx_batch;
 
 	err = efa_com_get_feature(edev, &resp, EFA_ADMIN_NETWORK_ATTR);
 	if (err) {

drivers/infiniband/hw/efa/efa_com_cmd.h +1 -0

index 41ce4a476ee6..8df2a26d57d4 100644
@@ -127,6 +127,7 @@ struct efa_com_get_device_attr_result {
 	u16 max_sq_sge;
 	u16 max_rq_sge;
 	u16 max_wr_rdma_sge;
+	u16 max_tx_batch;
 	u8 db_bar;
 };

drivers/infiniband/hw/efa/efa_verbs.c +1 -0

index 08313f7c73bc..f49d14cebe4a 100644
@@ -1525,6 +1525,7 @@ int efa_alloc_ucontext(struct ib_ucontext *ibucontext, struct ib_udata *udata)
 	resp.sub_cqs_per_cq = dev->dev_attr.sub_cqs_per_cq;
 	resp.inline_buf_size = dev->dev_attr.inline_buf_size;
 	resp.max_llq_size = dev->dev_attr.max_llq_size;
+	resp.max_tx_batch = dev->dev_attr.max_tx_batch;
 
 	if (udata && udata->outlen) {
 		err = ib_copy_to_udata(udata, &resp,

include/uapi/rdma/efa-abi.h +3 -1

index 53b6e2036a9b..10781763da37 100644
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) */
 /*
- * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2020 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #ifndef EFA_ABI_USER_H

@@ -31,6 +31,8 @@ struct efa_ibv_alloc_ucontext_resp {
 	__u16 sub_cqs_per_cq;
 	__u16 inline_buf_size;
 	__u32 max_llq_size; /* bytes */
+	__u16 max_tx_batch; /* units of 64 bytes */
+	__u8 reserved_90[6];
 };
 
 struct efa_ibv_alloc_pd_resp {

Signature · 1 line

2.27.0

Gal Pressman

author

2020-07-22 14:09 UTC #2 lore raw

PATCH v4 2/4 RDMA/efa: Expose minimum SQ size

In reply to Gal Pressman

The device reports the minimum SQ size required for creation.

This patch queries the min SQ size and reports it back to the userspace library.

Reviewed-by Firas JahJah <[email protected]>

Reviewed-by Shadi Ammouri <[email protected]>

Signed-off-by Gal Pressman <[email protected]>

---
 drivers/infiniband/hw/efa/efa_admin_cmds_defs.h | 4 ++--
 drivers/infiniband/hw/efa/efa_com_cmd.c         | 1 +
 drivers/infiniband/hw/efa/efa_com_cmd.h         | 1 +
 drivers/infiniband/hw/efa/efa_verbs.c           | 1 +
 include/uapi/rdma/efa-abi.h                     | 3 ++-
 5 files changed, 7 insertions(+), 3 deletions(-)

drivers/infiniband/hw/efa/efa_admin_cmds_defs.h +2 -2

index 03e7388af06e..5484b08bbc5d 100644
@@ -606,8 +606,8 @@ struct efa_admin_feature_queue_attr_desc {
 	/* Number of sub-CQs to be created for each CQ */
 	u16 sub_cqs_per_cq;
 
-	/* MBZ */
-	u16 reserved;
+	/* Minimum number of WQEs per SQ */
+	u16 min_sq_depth;
 
 	/* Maximum number of SGEs (buffers) allowed for a single send WQE */
 	u16 max_wr_send_sges;

drivers/infiniband/hw/efa/efa_com_cmd.c +1 -0

index 53cfde5c43d8..6ac23627f65a 100644
@@ -481,6 +481,7 @@ int efa_com_get_device_attr(struct efa_com_dev *edev,
 	result->sub_cqs_per_cq = resp.u.queue_attr.sub_cqs_per_cq;
 	result->max_wr_rdma_sge = resp.u.queue_attr.max_wr_rdma_sges;
 	result->max_tx_batch = resp.u.queue_attr.max_tx_batch;
+	result->min_sq_depth = resp.u.queue_attr.min_sq_depth;
 
 	err = efa_com_get_feature(edev, &resp, EFA_ADMIN_NETWORK_ATTR);
 	if (err) {

drivers/infiniband/hw/efa/efa_com_cmd.h +1 -0

index 8df2a26d57d4..190bac23f585 100644
@@ -128,6 +128,7 @@ struct efa_com_get_device_attr_result {
 	u16 max_rq_sge;
 	u16 max_wr_rdma_sge;
 	u16 max_tx_batch;
+	u16 min_sq_depth;
 	u8 db_bar;
 };

drivers/infiniband/hw/efa/efa_verbs.c +1 -0

index f49d14cebe4a..26102ab333b2 100644
@@ -1526,6 +1526,7 @@ int efa_alloc_ucontext(struct ib_ucontext *ibucontext, struct ib_udata *udata)
 	resp.inline_buf_size = dev->dev_attr.inline_buf_size;
 	resp.max_llq_size = dev->dev_attr.max_llq_size;
 	resp.max_tx_batch = dev->dev_attr.max_tx_batch;
+	resp.min_sq_wr = dev->dev_attr.min_sq_depth;
 
 	if (udata && udata->outlen) {
 		err = ib_copy_to_udata(udata, &resp,

include/uapi/rdma/efa-abi.h +2 -1

index 10781763da37..7ef2306f8dd4 100644
@@ -32,7 +32,8 @@ struct efa_ibv_alloc_ucontext_resp {
 	__u16 inline_buf_size;
 	__u32 max_llq_size; /* bytes */
 	__u16 max_tx_batch; /* units of 64 bytes */
-	__u8 reserved_90[6];
+	__u16 min_sq_wr;
+	__u8 reserved_a0[4];
 };
 
 struct efa_ibv_alloc_pd_resp {

Signature · 1 line

2.27.0

Gal Pressman

author

2020-07-22 14:09 UTC #3 lore raw

PATCH v4 3/4 RDMA/efa: User/kernel compatibility handshake mechanism

In reply to Gal Pressman

Introduce a mechanism that performs an handshake between the userspace provider and kernel driver which verifies that the user supports all required features in order to operate correctly.

The handshake verifies the needed functionality by comparing the reported device caps and the provider caps. If the device reports a non-zero capability the appropriate comp mask is required from the userspace provider in order to allocate the context.

Reviewed-by Shadi Ammouri <[email protected]>

Reviewed-by Yossi Leybovich <[email protected]>

Signed-off-by Gal Pressman <[email protected]>

---
 drivers/infiniband/hw/efa/efa_verbs.c | 40 +++++++++++++++++++++++++++
 include/uapi/rdma/efa-abi.h           | 10 +++++++
 2 files changed, 50 insertions(+)

drivers/infiniband/hw/efa/efa_verbs.c +40 -0

index 26102ab333b2..fda175836fb6 100644
@@ -1501,11 +1501,39 @@ static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn)
 	return efa_com_dealloc_uar(&dev->edev, &params);
 }
 
+#define EFA_CHECK_USER_COMP(_dev, _comp_mask, _attr, _mask, _attr_str) \
+	(_attr_str = (!(_dev)->dev_attr._attr || ((_comp_mask) & (_mask))) ? \
+		     NULL : #_attr)
+
+static int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
+				   const struct efa_ibv_alloc_ucontext_cmd *cmd)
+{
+	struct efa_dev *dev = to_edev(ibucontext->device);
+	char *attr_str;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, max_tx_batch,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH, attr_str))
+		goto err;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, min_sq_depth,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR,
+				attr_str))
+		goto err;
+
+	return 0;
+
+err:
+	ibdev_dbg(&dev->ibdev, "Userspace handshake failed for %s attribute\n",
+		  attr_str);
+	return -EOPNOTSUPP;
+}
+
 int efa_alloc_ucontext(struct ib_ucontext *ibucontext, struct ib_udata *udata)
 {
 	struct efa_ucontext *ucontext = to_eucontext(ibucontext);
 	struct efa_dev *dev = to_edev(ibucontext->device);
 	struct efa_ibv_alloc_ucontext_resp resp = {};
+	struct efa_ibv_alloc_ucontext_cmd cmd = {};
 	struct efa_com_alloc_uar_result result;
 	int err;

@@ -1514,6 +1542,18 @@ int efa_alloc_ucontext(struct ib_ucontext *ibucontext, struct ib_udata *udata)
 	 * we will ack input fields in our response.
 	 */
 
+	err = ib_copy_from_udata(&cmd, udata,
+				 min(sizeof(cmd), udata->inlen));
+	if (err) {
+		ibdev_dbg(&dev->ibdev,
+			  "Cannot copy udata for alloc_ucontext\n");
+		goto err_out;
+	}
+
+	err = efa_user_comp_handshake(ibucontext, &cmd);
+	if (err)
+		goto err_out;
+
 	err = efa_com_alloc_uar(&dev->edev, &result);
 	if (err)
 		goto err_out;

include/uapi/rdma/efa-abi.h +10 -0

index 7ef2306f8dd4..507a2862bedb 100644
@@ -20,6 +20,16 @@
  * hex bit offset of the field.
  */
 
+enum {
+	EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH  = 1 << 0,
+	EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR = 1 << 1,
+};
+
+struct efa_ibv_alloc_ucontext_cmd {
+	__u32 comp_mask;
+	__u8 reserved_20[4];
+};
+
 enum efa_ibv_user_cmds_supp_udata {
 	EFA_USER_CMDS_SUPP_UDATA_QUERY_DEVICE = 1 << 0,
 	EFA_USER_CMDS_SUPP_UDATA_CREATE_AH    = 1 << 1,

Signature · 1 line

2.27.0

Jason Gunthorpe

maintainer

2020-07-27 18:56 UTC #4 lore raw

Re: [PATCH for-next v4 3/4] RDMA/efa: User/kernel compatibility handshake mechanism

Quoted 31 lines · Gal Pressman

Introduce a mechanism that performs an handshake between the userspace provider and kernel driver which verifies that the user supports all required features in order to operate correctly.
The handshake verifies the needed functionality by comparing the reported device caps and the provider caps. If the device reports a non-zero capability the appropriate comp mask is required from the userspace provider in order to allocate the context.
Reviewed-by Shadi Ammouri <[email protected]>
Reviewed-by Yossi Leybovich <[email protected]>
Signed-off-by Gal Pressman <[email protected]>
 drivers/infiniband/hw/efa/efa_verbs.c | 40 +++++++++++++++++++++++++++
 include/uapi/rdma/efa-abi.h           | 10 +++++++
 2 files changed, 50 insertions(+)
drivers/infiniband/hw/efa/efa_verbs.c +9 -0
index 26102ab333b2..fda175836fb6 100644
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1501,11 +1501,39 @@ static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn)
 	return efa_com_dealloc_uar(&dev->edev, &params);
 }
 
+#define EFA_CHECK_USER_COMP(_dev, _comp_mask, _attr, _mask, _attr_str) \
+	(_attr_str = (!(_dev)->dev_attr._attr || ((_comp_mask) & (_mask))) ? \
+		     NULL : #_attr)
+
+static int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
+				   const struct efa_ibv_alloc_ucontext_cmd *cmd)
+{
+	struct efa_dev *dev = to_edev(ibucontext->device);

drivers/infiniband/hw/efa/efa_verbs.c static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn) · line 1521

Quoted 10 lines

drivers/infiniband/hw/efa/efa_verbs.c +10 -0

+	char *attr_str;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, max_tx_batch,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH, attr_str))
+		goto err;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, min_sq_depth,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR,
+				attr_str))
+		goto err;

But this patch should be first, the kernel should never return a non-zero value unless these input bits are set

Jason

Gal Pressman

author

2020-07-28 11:50 UTC #5 lore raw

Re: [PATCH for-next v4 3/4] RDMA/efa: User/kernel compatibility handshake mechanism

Quoted 37 lines · Jason Gunthorpe

Quoted 36 lines · Gal Pressman
Introduce a mechanism that performs an handshake between the userspace provider and kernel driver which verifies that the user supports all required features in order to operate correctly.
The handshake verifies the needed functionality by comparing the reported device caps and the provider caps. If the device reports a non-zero capability the appropriate comp mask is required from the userspace provider in order to allocate the context.
Reviewed-by Shadi Ammouri <[email protected]>
Reviewed-by Yossi Leybovich <[email protected]>
Signed-off-by Gal Pressman <[email protected]>
 drivers/infiniband/hw/efa/efa_verbs.c | 40 +++++++++++++++++++++++++++
 include/uapi/rdma/efa-abi.h           | 10 +++++++
 2 files changed, 50 insertions(+)
drivers/infiniband/hw/efa/efa_verbs.c +14 -0
index 26102ab333b2..fda175836fb6 100644
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1501,11 +1501,39 @@ static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn)
 	return efa_com_dealloc_uar(&dev->edev, &params);
 }
 
+#define EFA_CHECK_USER_COMP(_dev, _comp_mask, _attr, _mask, _attr_str) \
+	(_attr_str = (!(_dev)->dev_attr._attr || ((_comp_mask) & (_mask))) ? \
+		     NULL : #_attr)
+
+static int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
+				   const struct efa_ibv_alloc_ucontext_cmd *cmd)
+{
+	struct efa_dev *dev = to_edev(ibucontext->device);
+	char *attr_str;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, max_tx_batch,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH, attr_str))
+		goto err;

Quoted 8 lines

drivers/infiniband/hw/efa/efa_verbs.c static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn) · line 1521
Quoted 5 lines
drivers/infiniband/hw/efa/efa_verbs.c +5 -0
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, min_sq_depth,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR,
+				attr_str))
+		goto err;
But this patch should be first, the kernel should never return a non-zero value unless these input bits are set

But that's exactly what this patch does, it can only fail in case max_tx_batch/min_sq_depth is turned on by the device.

Anyway, the order doesn't matter as long as the pciid patch is last.

Jason Gunthorpe

maintainer

2020-07-28 12:28 UTC #6 lore raw

Re: [PATCH for-next v4 3/4] RDMA/efa: User/kernel compatibility handshake mechanism

Quoted 47 lines · Gal Pressman

Quoted 37 lines · Jason Gunthorpe
Quoted 36 lines · Gal Pressman
Introduce a mechanism that performs an handshake between the userspace provider and kernel driver which verifies that the user supports all required features in order to operate correctly.
The handshake verifies the needed functionality by comparing the reported device caps and the provider caps. If the device reports a non-zero capability the appropriate comp mask is required from the userspace provider in order to allocate the context.
Reviewed-by Shadi Ammouri <[email protected]>
Reviewed-by Yossi Leybovich <[email protected]>
Signed-off-by Gal Pressman <[email protected]>
 drivers/infiniband/hw/efa/efa_verbs.c | 40 +++++++++++++++++++++++++++
 include/uapi/rdma/efa-abi.h           | 10 +++++++
 2 files changed, 50 insertions(+)
drivers/infiniband/hw/efa/efa_verbs.c +14 -0
index 26102ab333b2..fda175836fb6 100644
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1501,11 +1501,39 @@ static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn)
 	return efa_com_dealloc_uar(&dev->edev, &params);
 }
 
+#define EFA_CHECK_USER_COMP(_dev, _comp_mask, _attr, _mask, _attr_str) \
+	(_attr_str = (!(_dev)->dev_attr._attr || ((_comp_mask) & (_mask))) ? \
+		     NULL : #_attr)
+
+static int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
+				   const struct efa_ibv_alloc_ucontext_cmd *cmd)
+{
+	struct efa_dev *dev = to_edev(ibucontext->device);
+	char *attr_str;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, max_tx_batch,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH, attr_str))
+		goto err;
Quoted 8 lines
drivers/infiniband/hw/efa/efa_verbs.c static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn) · line 1521
Quoted 5 lines
drivers/infiniband/hw/efa/efa_verbs.c +5 -0
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, min_sq_depth,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR,
+				attr_str))
+		goto err;
But this patch should be first, the kernel should never return a non-zero value unless these input bits are set

Quoted 2 lines

But that's exactly what this patch does, it can only fail in case max_tx_batch/min_sq_depth is turned on by the device.

My point is the series is out of order, the introduction of the two uapi parts should be in the same patch

Quoted 1 line

Anyway, the order doesn't matter as long as the pciid patch is last.

Oh?

Jason

Gal Pressman

author

2020-07-28 13:07 UTC #7 lore raw

Re: [PATCH for-next v4 3/4] RDMA/efa: User/kernel compatibility handshake mechanism

Quoted 51 lines · Jason Gunthorpe

Quoted 47 lines · Gal Pressman
Quoted 37 lines · Jason Gunthorpe
Quoted 36 lines · Gal Pressman
Introduce a mechanism that performs an handshake between the userspace provider and kernel driver which verifies that the user supports all required features in order to operate correctly.
The handshake verifies the needed functionality by comparing the reported device caps and the provider caps. If the device reports a non-zero capability the appropriate comp mask is required from the userspace provider in order to allocate the context.
Reviewed-by Shadi Ammouri <[email protected]>
Reviewed-by Yossi Leybovich <[email protected]>
Signed-off-by Gal Pressman <[email protected]>
 drivers/infiniband/hw/efa/efa_verbs.c | 40 +++++++++++++++++++++++++++
 include/uapi/rdma/efa-abi.h           | 10 +++++++
 2 files changed, 50 insertions(+)
drivers/infiniband/hw/efa/efa_verbs.c +14 -0
index 26102ab333b2..fda175836fb6 100644
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1501,11 +1501,39 @@ static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn)
 	return efa_com_dealloc_uar(&dev->edev, &params);
 }
 
+#define EFA_CHECK_USER_COMP(_dev, _comp_mask, _attr, _mask, _attr_str) \
+	(_attr_str = (!(_dev)->dev_attr._attr || ((_comp_mask) & (_mask))) ? \
+		     NULL : #_attr)
+
+static int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
+				   const struct efa_ibv_alloc_ucontext_cmd *cmd)
+{
+	struct efa_dev *dev = to_edev(ibucontext->device);
+	char *attr_str;
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, max_tx_batch,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH, attr_str))
+		goto err;
Quoted 8 lines
drivers/infiniband/hw/efa/efa_verbs.c static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn) · line 1521
Quoted 5 lines
drivers/infiniband/hw/efa/efa_verbs.c +5 -0
+
+	if (EFA_CHECK_USER_COMP(dev, cmd->comp_mask, min_sq_depth,
+				EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR,
+				attr_str))
+		goto err;
But this patch should be first, the kernel should never return a non-zero value unless these input bits are set
Quoted 2 lines
But that's exactly what this patch does, it can only fail in case max_tx_batch/min_sq_depth is turned on by the device.

Quoted 6 lines

My point is the series is out of order, the introduction of the two uapi parts should be in the same patch
Quoted 1 line
Anyway, the order doesn't matter as long as the pciid patch is last.
Oh?

0xefa0 devices will not turn on these bits, so as long as the 0xefa1 patch is last this order is fine.

Gal Pressman

author

2020-07-22 14:14 UTC #8 lore raw

PATCH v4 4/4 RDMA/efa: Add EFA 0xefa1 PCI ID

In reply to Gal Pressman

Add support for 0xefa1 devices.

Reviewed-by Shadi Ammouri <[email protected]>

Reviewed-by Yossi Leybovich <[email protected]>

Signed-off-by Gal Pressman <[email protected]>

---
 drivers/infiniband/hw/efa/efa_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/infiniband/hw/efa/efa_main.c +4 -2

index 82145574c928..92d701146320 100644
@@ -12,10 +12,12 @@
 
 #include "efa.h"
 
-#define PCI_DEV_ID_EFA_VF 0xefa0
+#define PCI_DEV_ID_EFA0_VF 0xefa0
+#define PCI_DEV_ID_EFA1_VF 0xefa1
 
 static const struct pci_device_id efa_pci_tbl[] = {
-	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA_VF) },
+	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA0_VF) },
+	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA1_VF) },
 	{ }
 };

Signature · 1 line

2.27.0

Jason Gunthorpe

maintainer Applied

2020-07-29 12:28 UTC #9 lore raw

In reply to Gal Pressman

Quoted 18 lines · Gal Pressman

Hi all,
The following submission adds the needed functionality in order to support 0xefa1 devices, and adds it to the driver pci table.
PR was sent:
https://github.com/linux-rdma/rdma-core/pull/789
Changelog -
v3->v4: https://lore.kernel.org/linux-rdma/[email protected]/
* Remove the user_comp_handshakes array and use the macros explicitly.
* Make efa_user_comp_handshake static.
v2->v3: https://lore.kernel.org/linux-rdma/[email protected]/
* Remove gcc specific stuff to fix clang compilation.
v1->v2: https://lore.kernel.org/linux-rdma/[email protected]/
* Add handshake with userspace provider to verify required features support.

Quoted 8 lines

Regards,
Gal
Gal Pressman (4):
RDMA/efa: Expose maximum TX doorbell batch RDMA/efa: Expose minimum SQ size
RDMA/efa: User/kernel compatibility handshake mechanism RDMA/efa: Add EFA 0xefa1 PCI ID

Applied to for-next

Jason