* [PATCH 1/2] RDMA/efa: Check stored completion CTX command ID with received one
2025-12-10 13:06 [PATCH 0/2] RDMA/efa: Admin completion context improvements Yonatan Nachum
@ 2025-12-10 13:06 ` Yonatan Nachum
2025-12-10 13:06 ` [PATCH 2/2] RDMA/efa: Improve admin completion context state machine Yonatan Nachum
2025-12-18 15:12 ` [PATCH 0/2] RDMA/efa: Admin completion context improvements Leon Romanovsky
2 siblings, 0 replies; 4+ messages in thread
From: Yonatan Nachum @ 2025-12-10 13:06 UTC (permalink / raw)
To: jgg, leon, linux-rdma
Cc: mrgolin, sleybo, matua, gal.pressman, Yonatan Nachum,
Daniel Kranzdorf
In admin command completion, we receive a CQE with the command ID which
is constructed from context index and entropy bits from the admin queue
producer counter. To try to detect memory corruptions in the received
CQE, validate the full command ID of the fetched context with the CQE
command ID. If there is a mismatch, complete the CQE with error.
Also use LSBs of the admin queue producer counter to better detect
entropy mismatch between smaller number of commands.
Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com>
Reviewed-by: Michael Margolin <mrgolin@amazon.com>
Signed-off-by: Yonatan Nachum <ynachum@amazon.com>
---
drivers/infiniband/hw/efa/efa_com.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c
index 0e979ca10d24..b31478f3a121 100644
--- a/drivers/infiniband/hw/efa/efa_com.c
+++ b/drivers/infiniband/hw/efa/efa_com.c
@@ -3,6 +3,8 @@
* Copyright 2018-2025 Amazon.com, Inc. or its affiliates. All rights reserved.
*/
+#include <linux/log2.h>
+
#include "efa_com.h"
#include "efa_regs_defs.h"
@@ -317,7 +319,7 @@ static struct efa_comp_ctx *__efa_com_submit_admin_cmd(struct efa_com_admin_queu
/* cmd_id LSBs are the ctx_id and MSBs are entropy bits from pc */
cmd_id = ctx_id & queue_size_mask;
- cmd_id |= aq->sq.pc & ~queue_size_mask;
+ cmd_id |= aq->sq.pc << ilog2(aq->depth);
cmd_id &= EFA_ADMIN_AQ_COMMON_DESC_COMMAND_ID_MASK;
cmd->aq_common_descriptor.command_id = cmd_id;
@@ -418,7 +420,7 @@ static int efa_com_handle_single_admin_completion(struct efa_com_admin_queue *aq
EFA_ADMIN_ACQ_COMMON_DESC_COMMAND_ID);
comp_ctx = efa_com_get_comp_ctx(aq, cmd_id, false);
- if (comp_ctx->status != EFA_CMD_SUBMITTED) {
+ if (comp_ctx->status != EFA_CMD_SUBMITTED || comp_ctx->cmd_id != cmd_id) {
ibdev_err(aq->efa_dev,
"Received completion with unexpected command id[%d], sq producer: %d, sq consumer: %d, cq consumer: %d\n",
cmd_id, aq->sq.pc, aq->sq.cc, aq->cq.cc);
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] RDMA/efa: Improve admin completion context state machine
2025-12-10 13:06 [PATCH 0/2] RDMA/efa: Admin completion context improvements Yonatan Nachum
2025-12-10 13:06 ` [PATCH 1/2] RDMA/efa: Check stored completion CTX command ID with received one Yonatan Nachum
@ 2025-12-10 13:06 ` Yonatan Nachum
2025-12-18 15:12 ` [PATCH 0/2] RDMA/efa: Admin completion context improvements Leon Romanovsky
2 siblings, 0 replies; 4+ messages in thread
From: Yonatan Nachum @ 2025-12-10 13:06 UTC (permalink / raw)
To: jgg, leon, linux-rdma
Cc: mrgolin, sleybo, matua, gal.pressman, Yonatan Nachum,
Daniel Kranzdorf
Add a new unused state to the admin completion contexts state machine
instead of the occupied field. This improves the completion validity
check because it now enforce the context to be in submitted state prior
to completing it. Also add allocated state as a intermediate state
between unused and submitted.
Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com>
Reviewed-by: Michael Margolin <mrgolin@amazon.com>
Signed-off-by: Yonatan Nachum <ynachum@amazon.com>
---
drivers/infiniband/hw/efa/efa_com.c | 91 ++++++++++++++++-------------
1 file changed, 50 insertions(+), 41 deletions(-)
diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c
index b31478f3a121..229b0ad3b0cb 100644
--- a/drivers/infiniband/hw/efa/efa_com.c
+++ b/drivers/infiniband/hw/efa/efa_com.c
@@ -23,6 +23,8 @@
#define EFA_CTRL_SUB_MINOR 1
enum efa_cmd_status {
+ EFA_CMD_UNUSED,
+ EFA_CMD_ALLOCATED,
EFA_CMD_SUBMITTED,
EFA_CMD_COMPLETED,
};
@@ -34,7 +36,6 @@ struct efa_comp_ctx {
enum efa_cmd_status status;
u16 cmd_id;
u8 cmd_opcode;
- u8 occupied;
};
static const char *efa_com_cmd_str(u8 cmd)
@@ -243,7 +244,6 @@ static int efa_com_admin_init_aenq(struct efa_com_dev *edev,
return 0;
}
-/* ID to be used with efa_com_get_comp_ctx */
static u16 efa_com_alloc_ctx_id(struct efa_com_admin_queue *aq)
{
u16 ctx_id;
@@ -265,36 +265,47 @@ static void efa_com_dealloc_ctx_id(struct efa_com_admin_queue *aq,
spin_unlock(&aq->comp_ctx_lock);
}
-static inline void efa_com_put_comp_ctx(struct efa_com_admin_queue *aq,
- struct efa_comp_ctx *comp_ctx)
+static struct efa_comp_ctx *efa_com_alloc_comp_ctx(struct efa_com_admin_queue *aq)
{
- u16 cmd_id = EFA_GET(&comp_ctx->user_cqe->acq_common_descriptor.command,
- EFA_ADMIN_ACQ_COMMON_DESC_COMMAND_ID);
- u16 ctx_id = cmd_id & (aq->depth - 1);
+ struct efa_comp_ctx *comp_ctx;
+ u16 ctx_id;
- ibdev_dbg(aq->efa_dev, "Put completion command_id %#x\n", cmd_id);
- comp_ctx->occupied = 0;
- efa_com_dealloc_ctx_id(aq, ctx_id);
+ ctx_id = efa_com_alloc_ctx_id(aq);
+
+ comp_ctx = &aq->comp_ctx[ctx_id];
+ if (comp_ctx->status != EFA_CMD_UNUSED) {
+ efa_com_dealloc_ctx_id(aq, ctx_id);
+ ibdev_err_ratelimited(aq->efa_dev,
+ "Completion context[%u] is used[%u]\n",
+ ctx_id, comp_ctx->status);
+ return NULL;
+ }
+
+ comp_ctx->status = EFA_CMD_ALLOCATED;
+ ibdev_dbg(aq->efa_dev, "Take completion context[%u]\n", ctx_id);
+ return comp_ctx;
}
-static struct efa_comp_ctx *efa_com_get_comp_ctx(struct efa_com_admin_queue *aq,
- u16 cmd_id, bool capture)
+static inline u16 efa_com_get_comp_ctx_id(struct efa_com_admin_queue *aq,
+ struct efa_comp_ctx *comp_ctx)
{
- u16 ctx_id = cmd_id & (aq->depth - 1);
+ return comp_ctx - aq->comp_ctx;
+}
- if (aq->comp_ctx[ctx_id].occupied && capture) {
- ibdev_err_ratelimited(
- aq->efa_dev,
- "Completion context for command_id %#x is occupied\n",
- cmd_id);
- return NULL;
- }
+static inline void efa_com_dealloc_comp_ctx(struct efa_com_admin_queue *aq,
+ struct efa_comp_ctx *comp_ctx)
+{
+ u16 ctx_id = efa_com_get_comp_ctx_id(aq, comp_ctx);
- if (capture) {
- aq->comp_ctx[ctx_id].occupied = 1;
- ibdev_dbg(aq->efa_dev,
- "Take completion ctxt for command_id %#x\n", cmd_id);
- }
+ ibdev_dbg(aq->efa_dev, "Put completion context[%u]\n", ctx_id);
+ comp_ctx->status = EFA_CMD_UNUSED;
+ efa_com_dealloc_ctx_id(aq, ctx_id);
+}
+
+static inline struct efa_comp_ctx *efa_com_get_comp_ctx_by_cmd_id(struct efa_com_admin_queue *aq,
+ u16 cmd_id)
+{
+ u16 ctx_id = cmd_id & (aq->depth - 1);
return &aq->comp_ctx[ctx_id];
}
@@ -312,10 +323,13 @@ static struct efa_comp_ctx *__efa_com_submit_admin_cmd(struct efa_com_admin_queu
u16 ctx_id;
u16 pi;
+ comp_ctx = efa_com_alloc_comp_ctx(aq);
+ if (!comp_ctx)
+ return ERR_PTR(-EINVAL);
+
queue_size_mask = aq->depth - 1;
pi = aq->sq.pc & queue_size_mask;
-
- ctx_id = efa_com_alloc_ctx_id(aq);
+ ctx_id = efa_com_get_comp_ctx_id(aq, comp_ctx);
/* cmd_id LSBs are the ctx_id and MSBs are entropy bits from pc */
cmd_id = ctx_id & queue_size_mask;
@@ -326,12 +340,6 @@ static struct efa_comp_ctx *__efa_com_submit_admin_cmd(struct efa_com_admin_queu
EFA_SET(&cmd->aq_common_descriptor.flags,
EFA_ADMIN_AQ_COMMON_DESC_PHASE, aq->sq.phase);
- comp_ctx = efa_com_get_comp_ctx(aq, cmd_id, true);
- if (!comp_ctx) {
- efa_com_dealloc_ctx_id(aq, ctx_id);
- return ERR_PTR(-EINVAL);
- }
-
comp_ctx->status = EFA_CMD_SUBMITTED;
comp_ctx->comp_size = comp_size_in_bytes;
comp_ctx->user_cqe = comp;
@@ -372,9 +380,9 @@ static inline int efa_com_init_comp_ctxt(struct efa_com_admin_queue *aq)
}
for (i = 0; i < aq->depth; i++) {
- comp_ctx = efa_com_get_comp_ctx(aq, i, false);
- if (comp_ctx)
- init_completion(&comp_ctx->wait_event);
+ comp_ctx = &aq->comp_ctx[i];
+ comp_ctx->status = EFA_CMD_UNUSED;
+ init_completion(&comp_ctx->wait_event);
aq->comp_ctx_pool[i] = i;
}
@@ -419,11 +427,12 @@ static int efa_com_handle_single_admin_completion(struct efa_com_admin_queue *aq
cmd_id = EFA_GET(&cqe->acq_common_descriptor.command,
EFA_ADMIN_ACQ_COMMON_DESC_COMMAND_ID);
- comp_ctx = efa_com_get_comp_ctx(aq, cmd_id, false);
+ comp_ctx = efa_com_get_comp_ctx_by_cmd_id(aq, cmd_id);
if (comp_ctx->status != EFA_CMD_SUBMITTED || comp_ctx->cmd_id != cmd_id) {
ibdev_err(aq->efa_dev,
- "Received completion with unexpected command id[%d], sq producer: %d, sq consumer: %d, cq consumer: %d\n",
- cmd_id, aq->sq.pc, aq->sq.cc, aq->cq.cc);
+ "Received completion with unexpected command id[%x], status[%d] sq producer[%d], sq consumer[%d], cq consumer[%d]\n",
+ cmd_id, comp_ctx->status, aq->sq.pc, aq->sq.cc,
+ aq->cq.cc);
return -EINVAL;
}
@@ -532,7 +541,7 @@ static int efa_com_wait_and_process_admin_cq_polling(struct efa_comp_ctx *comp_c
err = efa_com_comp_status_to_errno(comp_ctx->user_cqe->acq_common_descriptor.status);
out:
- efa_com_put_comp_ctx(aq, comp_ctx);
+ efa_com_dealloc_comp_ctx(aq, comp_ctx);
return err;
}
@@ -582,7 +591,7 @@ static int efa_com_wait_and_process_admin_cq_interrupts(struct efa_comp_ctx *com
err = efa_com_comp_status_to_errno(comp_ctx->user_cqe->acq_common_descriptor.status);
out:
- efa_com_put_comp_ctx(aq, comp_ctx);
+ efa_com_dealloc_comp_ctx(aq, comp_ctx);
return err;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread