[PATCH for-next v4] RDMA/efa: Add EFA query MR support

[PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Michael Margolin]

Add EFA driver uapi definitions and register a new query MR method that
currently returns the physical interconnects the device is using to
reach the MR. Update admin definitions and efa-abi accordingly.

Reviewed-by: Anas Mousa <anasmous@amazon.com>
Reviewed-by: Firas Jahjah <firasj@amazon.com>
Signed-off-by: Michael Margolin <mrgolin@amazon.com>
---
 drivers/infiniband/hw/efa/efa.h               | 12 +++-
 .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 33 ++++++++-
 drivers/infiniband/hw/efa/efa_com_cmd.c       | 11 ++-
 drivers/infiniband/hw/efa/efa_com_cmd.h       | 12 +++-
 drivers/infiniband/hw/efa/efa_main.c          |  7 +-
 drivers/infiniband/hw/efa/efa_verbs.c         | 71 ++++++++++++++++++-
 include/uapi/rdma/efa-abi.h                   | 21 +++++-
 7 files changed, 160 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h
index 7352a1f5d811..e2bdec32ae80 100644
--- a/drivers/infiniband/hw/efa/efa.h
+++ b/drivers/infiniband/hw/efa/efa.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
 /*
- * Copyright 2018-2021 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #ifndef _EFA_H_
@@ -80,9 +80,19 @@ struct efa_pd {
 	u16 pdn;
 };
 
+struct efa_mr_interconnect_info {
+	u16 recv_ic_id;
+	u16 rdma_read_ic_id;
+	u16 rdma_recv_ic_id;
+	u8 recv_ic_id_valid : 1;
+	u8 rdma_read_ic_id_valid : 1;
+	u8 rdma_recv_ic_id_valid : 1;
+};
+
 struct efa_mr {
 	struct ib_mr ibmr;
 	struct ib_umem *umem;
+	struct efa_mr_interconnect_info ic_info;
 };
 
 struct efa_cq {
diff --git a/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h b/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h
index 9c65bd27bae0..7377c8a9f4d5 100644
--- a/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h
+++ b/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
 /*
- * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #ifndef _EFA_ADMIN_CMDS_H_
@@ -415,6 +415,32 @@ struct efa_admin_reg_mr_resp {
 	 * memory region
 	 */
 	u32 r_key;
+
+	/*
+	 * Mask indicating which fields have valid values
+	 * 0 : recv_ic_id
+	 * 1 : rdma_read_ic_id
+	 * 2 : rdma_recv_ic_id
+	 */
+	u8 validity;
+
+	/*
+	 * Physical interconnect used by the device to reach the MR for receive
+	 * operation
+	 */
+	u8 recv_ic_id;
+
+	/*
+	 * Physical interconnect used by the device to reach the MR for RDMA
+	 * read operation
+	 */
+	u8 rdma_read_ic_id;
+
+	/*
+	 * Physical interconnect used by the device to reach the MR for RDMA
+	 * write receive
+	 */
+	u8 rdma_recv_ic_id;
 };
 
 struct efa_admin_dereg_mr_cmd {
@@ -999,6 +1025,11 @@ struct efa_admin_host_info {
 #define EFA_ADMIN_REG_MR_CMD_REMOTE_WRITE_ENABLE_MASK       BIT(1)
 #define EFA_ADMIN_REG_MR_CMD_REMOTE_READ_ENABLE_MASK        BIT(2)
 
+/* reg_mr_resp */
+#define EFA_ADMIN_REG_MR_RESP_RECV_IC_ID_MASK               BIT(0)
+#define EFA_ADMIN_REG_MR_RESP_RDMA_READ_IC_ID_MASK          BIT(1)
+#define EFA_ADMIN_REG_MR_RESP_RDMA_RECV_IC_ID_MASK          BIT(2)
+
 /* create_cq_cmd */
 #define EFA_ADMIN_CREATE_CQ_CMD_INTERRUPT_MODE_ENABLED_MASK BIT(5)
 #define EFA_ADMIN_CREATE_CQ_CMD_VIRT_MASK                   BIT(6)
diff --git a/drivers/infiniband/hw/efa/efa_com_cmd.c b/drivers/infiniband/hw/efa/efa_com_cmd.c
index 576811885d59..d3398c7b0bd0 100644
--- a/drivers/infiniband/hw/efa/efa_com_cmd.c
+++ b/drivers/infiniband/hw/efa/efa_com_cmd.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
 /*
- * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #include "efa_com.h"
@@ -270,6 +270,15 @@ int efa_com_register_mr(struct efa_com_dev *edev,
 
 	result->l_key = cmd_completion.l_key;
 	result->r_key = cmd_completion.r_key;
+	result->ic_info.recv_ic_id = cmd_completion.recv_ic_id;
+	result->ic_info.rdma_read_ic_id = cmd_completion.rdma_read_ic_id;
+	result->ic_info.rdma_recv_ic_id = cmd_completion.rdma_recv_ic_id;
+	result->ic_info.recv_ic_id_valid = EFA_GET(&cmd_completion.validity,
+						   EFA_ADMIN_REG_MR_RESP_RECV_IC_ID);
+	result->ic_info.rdma_read_ic_id_valid = EFA_GET(&cmd_completion.validity,
+							EFA_ADMIN_REG_MR_RESP_RDMA_READ_IC_ID);
+	result->ic_info.rdma_recv_ic_id_valid = EFA_GET(&cmd_completion.validity,
+							EFA_ADMIN_REG_MR_RESP_RDMA_RECV_IC_ID);
 
 	return 0;
 }
diff --git a/drivers/infiniband/hw/efa/efa_com_cmd.h b/drivers/infiniband/hw/efa/efa_com_cmd.h
index fc97f37bb39b..720a99ba0f7d 100644
--- a/drivers/infiniband/hw/efa/efa_com_cmd.h
+++ b/drivers/infiniband/hw/efa/efa_com_cmd.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
 /*
- * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #ifndef _EFA_COM_CMD_H_
@@ -199,6 +199,15 @@ struct efa_com_reg_mr_params {
 	u8 indirect;
 };
 
+struct efa_com_mr_interconnect_info {
+	u16 recv_ic_id;
+	u16 rdma_read_ic_id;
+	u16 rdma_recv_ic_id;
+	u8 recv_ic_id_valid : 1;
+	u8 rdma_read_ic_id_valid : 1;
+	u8 rdma_recv_ic_id_valid : 1;
+};
+
 struct efa_com_reg_mr_result {
 	/*
 	 * To be used in conjunction with local buffers references in SQ and
@@ -210,6 +219,7 @@ struct efa_com_reg_mr_result {
 	 * accessed memory region
 	 */
 	u32 r_key;
+	struct efa_com_mr_interconnect_info ic_info;
 };
 
 struct efa_com_dereg_mr_params {
diff --git a/drivers/infiniband/hw/efa/efa_main.c b/drivers/infiniband/hw/efa/efa_main.c
index 15ee92081118..7b1910a86216 100644
--- a/drivers/infiniband/hw/efa/efa_main.c
+++ b/drivers/infiniband/hw/efa/efa_main.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
 /*
- * Copyright 2018-2022 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #include <linux/module.h>
@@ -9,6 +9,7 @@
 #include <linux/version.h>
 
 #include <rdma/ib_user_verbs.h>
+#include <rdma/uverbs_ioctl.h>
 
 #include "efa.h"
 
@@ -36,6 +37,8 @@ MODULE_DEVICE_TABLE(pci, efa_pci_tbl);
 	(BIT(EFA_ADMIN_FATAL_ERROR) | BIT(EFA_ADMIN_WARNING) | \
 	 BIT(EFA_ADMIN_NOTIFICATION) | BIT(EFA_ADMIN_KEEP_ALIVE))
 
+extern const struct uapi_definition efa_uapi_defs[];
+
 /* This handler will called for unknown event group or unimplemented handlers */
 static void unimplemented_aenq_handler(void *data,
 				       struct efa_admin_aenq_entry *aenq_e)
@@ -432,6 +435,8 @@ static int efa_ib_device_add(struct efa_dev *dev)
 
 	ib_set_device_ops(&dev->ibdev, &efa_dev_ops);
 
+	dev->ibdev.driver_def = efa_uapi_defs;
+
 	err = ib_register_device(&dev->ibdev, "efa_%d", &pdev->dev);
 	if (err)
 		goto err_destroy_eqs;
diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
index 0f8ca99d0827..8f4435706e4d 100644
--- a/drivers/infiniband/hw/efa/efa_verbs.c
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
 /*
- * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #include <linux/dma-buf.h>
@@ -13,6 +13,9 @@
 #include <rdma/ib_user_verbs.h>
 #include <rdma/ib_verbs.h>
 #include <rdma/uverbs_ioctl.h>
+#define UVERBS_MODULE_NAME efa_ib
+#include <rdma/uverbs_named_ioctl.h>
+#include <rdma/ib_user_ioctl_cmds.h>
 
 #include "efa.h"
 #include "efa_io_defs.h"
@@ -1653,6 +1656,12 @@ static int efa_register_mr(struct ib_pd *ibpd, struct efa_mr *mr, u64 start,
 	mr->ibmr.lkey = result.l_key;
 	mr->ibmr.rkey = result.r_key;
 	mr->ibmr.length = length;
+	mr->ic_info.recv_ic_id = result.ic_info.recv_ic_id;
+	mr->ic_info.rdma_read_ic_id = result.ic_info.rdma_read_ic_id;
+	mr->ic_info.rdma_recv_ic_id = result.ic_info.rdma_recv_ic_id;
+	mr->ic_info.recv_ic_id_valid = result.ic_info.recv_ic_id_valid;
+	mr->ic_info.rdma_read_ic_id_valid = result.ic_info.rdma_read_ic_id_valid;
+	mr->ic_info.rdma_recv_ic_id_valid = result.ic_info.rdma_recv_ic_id_valid;
 	ibdev_dbg(&dev->ibdev, "Registered mr[%d]\n", mr->ibmr.lkey);
 
 	return 0;
@@ -1735,6 +1744,39 @@ struct ib_mr *efa_reg_mr(struct ib_pd *ibpd, u64 start, u64 length,
 	return ERR_PTR(err);
 }
 
+static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *attrs)
+{
+	struct ib_mr *ibmr = uverbs_attr_get_obj(attrs, EFA_IB_ATTR_QUERY_MR_HANDLE);
+	struct efa_mr *mr = to_emr(ibmr);
+	u16 ic_id_validity;
+	int ret;
+
+	ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
+			     &mr->ic_info.recv_ic_id, sizeof(mr->ic_info.recv_ic_id));
+	if (ret)
+		return ret;
+
+	ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
+			     &mr->ic_info.rdma_read_ic_id, sizeof(mr->ic_info.rdma_read_ic_id));
+	if (ret)
+		return ret;
+
+	ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,
+			     &mr->ic_info.rdma_recv_ic_id, sizeof(mr->ic_info.rdma_recv_ic_id));
+	if (ret)
+		return ret;
+
+	ic_id_validity = (mr->ic_info.recv_ic_id_valid ?
+			  EFA_QUERY_MR_VALIDITY_RECV_IC_ID : 0) |
+			 (mr->ic_info.rdma_read_ic_id_valid ?
+			  EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID : 0) |
+			 (mr->ic_info.rdma_recv_ic_id_valid ?
+			  EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID : 0);
+
+	return uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
+			      &ic_id_validity, sizeof(ic_id_validity));
+}
+
 int efa_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
 {
 	struct efa_dev *dev = to_edev(ibmr->device);
@@ -2157,3 +2199,30 @@ enum rdma_link_layer efa_port_link_layer(struct ib_device *ibdev,
 	return IB_LINK_LAYER_UNSPECIFIED;
 }
 
+DECLARE_UVERBS_NAMED_METHOD(EFA_IB_METHOD_MR_QUERY,
+			    UVERBS_ATTR_IDR(EFA_IB_ATTR_QUERY_MR_HANDLE,
+					    UVERBS_OBJECT_MR,
+					    UVERBS_ACCESS_READ,
+					    UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
+						UVERBS_ATTR_TYPE(u16),
+						UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
+						UVERBS_ATTR_TYPE(u16),
+						UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
+						UVERBS_ATTR_TYPE(u16),
+						UA_MANDATORY),
+			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,
+						UVERBS_ATTR_TYPE(u16),
+						UA_MANDATORY));
+
+ADD_UVERBS_METHODS(efa_mr,
+		   UVERBS_OBJECT_MR,
+		   &UVERBS_METHOD(EFA_IB_METHOD_MR_QUERY));
+
+const struct uapi_definition efa_uapi_defs[] = {
+	UAPI_DEF_CHAIN_OBJ_TREE(UVERBS_OBJECT_MR,
+				&efa_mr),
+	{},
+};
diff --git a/include/uapi/rdma/efa-abi.h b/include/uapi/rdma/efa-abi.h
index d94c32f28804..701e2d567e41 100644
--- a/include/uapi/rdma/efa-abi.h
+++ b/include/uapi/rdma/efa-abi.h
@@ -1,12 +1,13 @@
 /* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) */
 /*
- * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #ifndef EFA_ABI_USER_H
 #define EFA_ABI_USER_H
 
 #include <linux/types.h>
+#include <rdma/ib_user_ioctl_cmds.h>
 
 /*
  * Increment this value if any changes that break userspace ABI
@@ -134,4 +135,22 @@ struct efa_ibv_ex_query_device_resp {
 	__u32 device_caps;
 };
 
+enum {
+	EFA_QUERY_MR_VALIDITY_RECV_IC_ID = 1 << 0,
+	EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID = 1 << 1,
+	EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID = 1 << 2,
+};
+
+enum efa_query_mr_attrs {
+	EFA_IB_ATTR_QUERY_MR_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
+	EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
+	EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
+	EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
+	EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,
+};
+
+enum efa_mr_methods {
+	EFA_IB_METHOD_MR_QUERY = (1U << UVERBS_ID_NS_SHIFT),
+};
+
 #endif /* EFA_ABI_USER_H */
-- 
2.40.1

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Leon Romanovsky]

On Thu, Jan 04, 2024 at 09:51:55AM +0000, Michael Margolin wrote:
> Add EFA driver uapi definitions and register a new query MR method that
> currently returns the physical interconnects the device is using to
> reach the MR. Update admin definitions and efa-abi accordingly.
> 
> Reviewed-by: Anas Mousa <anasmous@amazon.com>
> Reviewed-by: Firas Jahjah <firasj@amazon.com>
> Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> ---
>  drivers/infiniband/hw/efa/efa.h               | 12 +++-
>  .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 33 ++++++++-
>  drivers/infiniband/hw/efa/efa_com_cmd.c       | 11 ++-
>  drivers/infiniband/hw/efa/efa_com_cmd.h       | 12 +++-
>  drivers/infiniband/hw/efa/efa_main.c          |  7 +-
>  drivers/infiniband/hw/efa/efa_verbs.c         | 71 ++++++++++++++++++-
>  include/uapi/rdma/efa-abi.h                   | 21 +++++-
>  7 files changed, 160 insertions(+), 7 deletions(-)

It is already fourth version of this patch and not a single word about
the changes. Please add changelog in your next submissions.

Applied this patch with the following change.

diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
index 8f4435706e4d..2f412db2edcd 100644
--- a/drivers/infiniband/hw/efa/efa_verbs.c
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1748,7 +1748,7 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
 {
        struct ib_mr *ibmr = uverbs_attr_get_obj(attrs, EFA_IB_ATTR_QUERY_MR_HANDLE);
        struct efa_mr *mr = to_emr(ibmr);
-       u16 ic_id_validity;
+       u16 ic_id_validity = 0;
        int ret;

        ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
@@ -1766,12 +1766,12 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
        if (ret)
                return ret;

-       ic_id_validity = (mr->ic_info.recv_ic_id_valid ?
-                         EFA_QUERY_MR_VALIDITY_RECV_IC_ID : 0) |
-                        (mr->ic_info.rdma_read_ic_id_valid ?
-                         EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID : 0) |
-                        (mr->ic_info.rdma_recv_ic_id_valid ?
-                         EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID : 0);
+       if (mr->ic_info.recv_ic_id_valid)
+               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RECV_IC_ID;
+       if (mr->ic_info.rdma_read_ic_id_valid)
+               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID;
+       if (mr->ic_info.rdma_recv_ic_id_valid)
+               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID;

        return uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
                              &ic_id_validity, sizeof(ic_id_validity));

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Margolin, Michael]

Thanks Leon.

On 1/7/2024 12:02 PM, Leon Romanovsky wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Thu, Jan 04, 2024 at 09:51:55AM +0000, Michael Margolin wrote:
>> Add EFA driver uapi definitions and register a new query MR method that
>> currently returns the physical interconnects the device is using to
>> reach the MR. Update admin definitions and efa-abi accordingly.
>>
>> Reviewed-by: Anas Mousa <anasmous@amazon.com>
>> Reviewed-by: Firas Jahjah <firasj@amazon.com>
>> Signed-off-by: Michael Margolin <mrgolin@amazon.com>
>> ---
>>  drivers/infiniband/hw/efa/efa.h               | 12 +++-
>>  .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 33 ++++++++-
>>  drivers/infiniband/hw/efa/efa_com_cmd.c       | 11 ++-
>>  drivers/infiniband/hw/efa/efa_com_cmd.h       | 12 +++-
>>  drivers/infiniband/hw/efa/efa_main.c          |  7 +-
>>  drivers/infiniband/hw/efa/efa_verbs.c         | 71 ++++++++++++++++++-
>>  include/uapi/rdma/efa-abi.h                   | 21 +++++-
>>  7 files changed, 160 insertions(+), 7 deletions(-)
> It is already fourth version of this patch and not a single word about
> the changes. Please add changelog in your next submissions.
>
> Applied this patch with the following change.
>
> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> index 8f4435706e4d..2f412db2edcd 100644
> --- a/drivers/infiniband/hw/efa/efa_verbs.c
> +++ b/drivers/infiniband/hw/efa/efa_verbs.c
> @@ -1748,7 +1748,7 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
>  {
>         struct ib_mr *ibmr = uverbs_attr_get_obj(attrs, EFA_IB_ATTR_QUERY_MR_HANDLE);
>         struct efa_mr *mr = to_emr(ibmr);
> -       u16 ic_id_validity;
> +       u16 ic_id_validity = 0;
>         int ret;
>
>         ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
> @@ -1766,12 +1766,12 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
>         if (ret)
>                 return ret;
>
> -       ic_id_validity = (mr->ic_info.recv_ic_id_valid ?
> -                         EFA_QUERY_MR_VALIDITY_RECV_IC_ID : 0) |
> -                        (mr->ic_info.rdma_read_ic_id_valid ?
> -                         EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID : 0) |
> -                        (mr->ic_info.rdma_recv_ic_id_valid ?
> -                         EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID : 0);
> +       if (mr->ic_info.recv_ic_id_valid)
> +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RECV_IC_ID;
> +       if (mr->ic_info.rdma_read_ic_id_valid)
> +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID;
> +       if (mr->ic_info.rdma_recv_ic_id_valid)
> +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID;
>
>         return uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
>                               &ic_id_validity, sizeof(ic_id_validity));
>

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Jason Gunthorpe]

On Sun, Jan 07, 2024 at 12:02:56PM +0200, Leon Romanovsky wrote:
> On Thu, Jan 04, 2024 at 09:51:55AM +0000, Michael Margolin wrote:
> > Add EFA driver uapi definitions and register a new query MR method that
> > currently returns the physical interconnects the device is using to
> > reach the MR. Update admin definitions and efa-abi accordingly.
> > 
> > Reviewed-by: Anas Mousa <anasmous@amazon.com>
> > Reviewed-by: Firas Jahjah <firasj@amazon.com>
> > Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> > ---
> >  drivers/infiniband/hw/efa/efa.h               | 12 +++-
> >  .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 33 ++++++++-
> >  drivers/infiniband/hw/efa/efa_com_cmd.c       | 11 ++-
> >  drivers/infiniband/hw/efa/efa_com_cmd.h       | 12 +++-
> >  drivers/infiniband/hw/efa/efa_main.c          |  7 +-
> >  drivers/infiniband/hw/efa/efa_verbs.c         | 71 ++++++++++++++++++-
> >  include/uapi/rdma/efa-abi.h                   | 21 +++++-
> >  7 files changed, 160 insertions(+), 7 deletions(-)
> 
> It is already fourth version of this patch and not a single word about
> the changes. Please add changelog in your next submissions.
> 
> Applied this patch with the following change.
> 
> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> index 8f4435706e4d..2f412db2edcd 100644
> --- a/drivers/infiniband/hw/efa/efa_verbs.c
> +++ b/drivers/infiniband/hw/efa/efa_verbs.c
> @@ -1748,7 +1748,7 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
>  {
>         struct ib_mr *ibmr = uverbs_attr_get_obj(attrs, EFA_IB_ATTR_QUERY_MR_HANDLE);
>         struct efa_mr *mr = to_emr(ibmr);
> -       u16 ic_id_validity;
> +       u16 ic_id_validity = 0;
>         int ret;
> 
>         ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
> @@ -1766,12 +1766,12 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
>         if (ret)
>                 return ret;
> 
> -       ic_id_validity = (mr->ic_info.recv_ic_id_valid ?
> -                         EFA_QUERY_MR_VALIDITY_RECV_IC_ID : 0) |
> -                        (mr->ic_info.rdma_read_ic_id_valid ?
> -                         EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID : 0) |
> -                        (mr->ic_info.rdma_recv_ic_id_valid ?
> -                         EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID : 0);
> +       if (mr->ic_info.recv_ic_id_valid)
> +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RECV_IC_ID;
> +       if (mr->ic_info.rdma_read_ic_id_valid)
> +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID;
> +       if (mr->ic_info.rdma_recv_ic_id_valid)
> +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID;

I was saying in the rdma-core PR that this field shouldn't even
exist..

Jason

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Leon Romanovsky]

On Mon, Jan 08, 2024 at 09:05:54AM -0400, Jason Gunthorpe wrote:
> On Sun, Jan 07, 2024 at 12:02:56PM +0200, Leon Romanovsky wrote:
> > On Thu, Jan 04, 2024 at 09:51:55AM +0000, Michael Margolin wrote:
> > > Add EFA driver uapi definitions and register a new query MR method that
> > > currently returns the physical interconnects the device is using to
> > > reach the MR. Update admin definitions and efa-abi accordingly.
> > > 
> > > Reviewed-by: Anas Mousa <anasmous@amazon.com>
> > > Reviewed-by: Firas Jahjah <firasj@amazon.com>
> > > Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> > > ---
> > >  drivers/infiniband/hw/efa/efa.h               | 12 +++-
> > >  .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 33 ++++++++-
> > >  drivers/infiniband/hw/efa/efa_com_cmd.c       | 11 ++-
> > >  drivers/infiniband/hw/efa/efa_com_cmd.h       | 12 +++-
> > >  drivers/infiniband/hw/efa/efa_main.c          |  7 +-
> > >  drivers/infiniband/hw/efa/efa_verbs.c         | 71 ++++++++++++++++++-
> > >  include/uapi/rdma/efa-abi.h                   | 21 +++++-
> > >  7 files changed, 160 insertions(+), 7 deletions(-)
> > 
> > It is already fourth version of this patch and not a single word about
> > the changes. Please add changelog in your next submissions.
> > 
> > Applied this patch with the following change.
> > 
> > diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> > index 8f4435706e4d..2f412db2edcd 100644
> > --- a/drivers/infiniband/hw/efa/efa_verbs.c
> > +++ b/drivers/infiniband/hw/efa/efa_verbs.c
> > @@ -1748,7 +1748,7 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
> >  {
> >         struct ib_mr *ibmr = uverbs_attr_get_obj(attrs, EFA_IB_ATTR_QUERY_MR_HANDLE);
> >         struct efa_mr *mr = to_emr(ibmr);
> > -       u16 ic_id_validity;
> > +       u16 ic_id_validity = 0;
> >         int ret;
> > 
> >         ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
> > @@ -1766,12 +1766,12 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
> >         if (ret)
> >                 return ret;
> > 
> > -       ic_id_validity = (mr->ic_info.recv_ic_id_valid ?
> > -                         EFA_QUERY_MR_VALIDITY_RECV_IC_ID : 0) |
> > -                        (mr->ic_info.rdma_read_ic_id_valid ?
> > -                         EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID : 0) |
> > -                        (mr->ic_info.rdma_recv_ic_id_valid ?
> > -                         EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID : 0);
> > +       if (mr->ic_info.recv_ic_id_valid)
> > +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RECV_IC_ID;
> > +       if (mr->ic_info.rdma_read_ic_id_valid)
> > +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID;
> > +       if (mr->ic_info.rdma_recv_ic_id_valid)
> > +               ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID;
> 
> I was saying in the rdma-core PR that this field shouldn't even
> exist..

Something like that?

diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
index 2f412db2edcd..97619b29afe0 100644
--- a/drivers/infiniband/hw/efa/efa_verbs.c
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1748,33 +1748,29 @@ static int UVERBS_HANDLER(EFA_IB_METHOD_MR_QUERY)(struct uverbs_attr_bundle *att
 {
 	struct ib_mr *ibmr = uverbs_attr_get_obj(attrs, EFA_IB_ATTR_QUERY_MR_HANDLE);
 	struct efa_mr *mr = to_emr(ibmr);
-	u16 ic_id_validity = 0;
 	int ret;
 
-	ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
-			     &mr->ic_info.recv_ic_id, sizeof(mr->ic_info.recv_ic_id));
-	if (ret)
-		return ret;
-
-	ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
-			     &mr->ic_info.rdma_read_ic_id, sizeof(mr->ic_info.rdma_read_ic_id));
-	if (ret)
-		return ret;
-
-	ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,
-			     &mr->ic_info.rdma_recv_ic_id, sizeof(mr->ic_info.rdma_recv_ic_id));
-	if (ret)
-		return ret;
+	if (mr->ic_info.recv_ic_id_valid) {
+		ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
+				     &mr->ic_info.recv_ic_id, sizeof(mr->ic_info.recv_ic_id));
+		if (ret)
+			return ret;
+	}
 
-	if (mr->ic_info.recv_ic_id_valid)
-		ic_id_validity |= EFA_QUERY_MR_VALIDITY_RECV_IC_ID;
-	if (mr->ic_info.rdma_read_ic_id_valid)
-		ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_READ_IC_ID;
-	if (mr->ic_info.rdma_recv_ic_id_valid)
-		ic_id_validity |= EFA_QUERY_MR_VALIDITY_RDMA_RECV_IC_ID;
+	if (mr->ic_info.rdma_read_ic_id_valid) {
+		ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
+				     &mr->ic_info.rdma_read_ic_id, sizeof(mr->ic_info.rdma_read_ic_id));
+		if (ret)
+			return ret;
+	}
 
-	return uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
-			      &ic_id_validity, sizeof(ic_id_validity));
+	if (mr->ic_info.rdma_recv_ic_id_valid) {
+		ret = uverbs_copy_to(attrs, EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,
+				     &mr->ic_info.rdma_recv_ic_id, sizeof(mr->ic_info.rdma_recv_ic_id));
+		if (ret)
+			return ret;
+	}
+	return 0;
 }
 
 int efa_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)
@@ -2204,18 +2200,15 @@ DECLARE_UVERBS_NAMED_METHOD(EFA_IB_METHOD_MR_QUERY,
 					    UVERBS_OBJECT_MR,
 					    UVERBS_ACCESS_READ,
 					    UA_MANDATORY),
-			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
-						UVERBS_ATTR_TYPE(u16),
-						UA_MANDATORY),
 			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
 						UVERBS_ATTR_TYPE(u16),
-						UA_MANDATORY),
+						UA_OPTIONAL),
 			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
 						UVERBS_ATTR_TYPE(u16),
-						UA_MANDATORY),
+						UA_OPTIONAL),
 			    UVERBS_ATTR_PTR_OUT(EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,
 						UVERBS_ATTR_TYPE(u16),
-						UA_MANDATORY));
+						UA_OPTIONAL));
 
 ADD_UVERBS_METHODS(efa_mr,
 		   UVERBS_OBJECT_MR,
diff --git a/include/uapi/rdma/efa-abi.h b/include/uapi/rdma/efa-abi.h
index 701e2d567e41..fbe5936e22cd 100644
--- a/include/uapi/rdma/efa-abi.h
+++ b/include/uapi/rdma/efa-abi.h
@@ -143,7 +143,6 @@ enum {
 
 enum efa_query_mr_attrs {
 	EFA_IB_ATTR_QUERY_MR_HANDLE = (1U << UVERBS_ID_NS_SHIFT),
-	EFA_IB_ATTR_QUERY_MR_RESP_IC_ID_VALIDITY,
 	EFA_IB_ATTR_QUERY_MR_RESP_RECV_IC_ID,
 	EFA_IB_ATTR_QUERY_MR_RESP_RDMA_READ_IC_ID,
 	EFA_IB_ATTR_QUERY_MR_RESP_RDMA_RECV_IC_ID,

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Jason Gunthorpe]

On Mon, Jan 08, 2024 at 08:01:40PM +0200, Leon Romanovsky wrote:
> > I was saying in the rdma-core PR that this field shouldn't even
> > exist..
> 
> Something like that?

Yeah, like that. However it is difficult to get the out valid uattr
back in the rdma-core side.

This is best if the ID's can have well defined not-valid values such
as 0 or -1.

Jason

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Leon Romanovsky]

On Mon, Jan 8, 2024, at 20:06, Jason Gunthorpe wrote:
> On Mon, Jan 08, 2024 at 08:01:40PM +0200, Leon Romanovsky wrote:
>> > I was saying in the rdma-core PR that this field shouldn't even
>> > exist..
>> 
>> Something like that?
>
> Yeah, like that. However it is difficult to get the out valid uattr
> back in the rdma-core side.
>
> This is best if the ID's can have well defined not-valid values such
> as 0 or -1.

Michael tried something like that in previous versions by defining 0xffff as not valid.

I didn't like it because there's no promise from PCI core that it is invalid value.



>
> Jason

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Leon Romanovsky]

On Mon, Jan 08, 2024 at 09:40:32PM +0200, Leon Romanovsky wrote:
> 
> 
> On Mon, Jan 8, 2024, at 20:06, Jason Gunthorpe wrote:
> > On Mon, Jan 08, 2024 at 08:01:40PM +0200, Leon Romanovsky wrote:
> >> > I was saying in the rdma-core PR that this field shouldn't even
> >> > exist..
> >> 
> >> Something like that?
> >
> > Yeah, like that. However it is difficult to get the out valid uattr
> > back in the rdma-core side.
> >
> > This is best if the ID's can have well defined not-valid values such
> > as 0 or -1.
> 
> Michael tried something like that in previous versions by defining 0xffff as not valid.
> 
> I didn't like it because there's no promise from PCI core that it is invalid value.

Michael,
What do you think?

Thanks

> 
> 
> 
> >
> > Jason
> 

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Margolin, Michael]

On 1/9/2024 8:05 PM, Leon Romanovsky wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Mon, Jan 08, 2024 at 09:40:32PM +0200, Leon Romanovsky wrote:
>>
>> On Mon, Jan 8, 2024, at 20:06, Jason Gunthorpe wrote:
>>> On Mon, Jan 08, 2024 at 08:01:40PM +0200, Leon Romanovsky wrote:
>>>>> I was saying in the rdma-core PR that this field shouldn't even
>>>>> exist..
>>>> Something like that?
>>> Yeah, like that. However it is difficult to get the out valid uattr
>>> back in the rdma-core side.
>>>
>>> This is best if the ID's can have well defined not-valid values such
>>> as 0 or -1.
>> Michael tried something like that in previous versions by defining 0xffff as not valid.
>>
>> I didn't like it because there's no promise from PCI core that it is invalid value.
> Michael,
> What do you think?
>
> Thanks

In this case I prefer to keep the explicit validity for consistency with
the device originated values.

In general I think it will be useful to have some convenient way to get
attr validity from the ioctl mechanism in rdma-core.

Michael

Re: [PATCH for-next v4] RDMA/efa: Add EFA query MR support

[Leon Romanovsky]

On Thu, 04 Jan 2024 09:51:55 +0000, Michael Margolin wrote:
> Add EFA driver uapi definitions and register a new query MR method that
> currently returns the physical interconnects the device is using to
> reach the MR. Update admin definitions and efa-abi accordingly.
> 
> 

Applied, thanks!

[1/1] RDMA/efa: Add EFA query MR support
      https://git.kernel.org/rdma/rdma/c/2307157c850960

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>