* [PATCH net-next v10 0/4] net: mana: concurrent HWC requests and dynamic queue depth
@ 2026-11-09 15:44 Wei Hu
2026-11-09 15:45 ` [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization Wei Hu
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Wei Hu @ 2026-11-09 15:44 UTC (permalink / raw)
To: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel, gargaditya,
kees+treewide, kees, mawasthi, weh, edumazet
This is v10 of Long Li's four-patch HWC series, rebased for the reopened
net-next tree. I am carrying the series forward with review fixes and
integration work. Original authorship and sign-offs are retained.
The series supports concurrent requests over the MANA hardware channel and
rebuilds its queues using the queue depth and message sizes required by
the PF. It prepares ownership and teardown, gives each message its own
completion, adds concurrent request tracking, and enables dynamic sizing.
v9 was deferred while net-next was closed. This revision resolves its
conflicts with the upstream throttled-EQE support; it is not an unchanged
RESEND. No new actionable v9 review finding was located during the
reposting check.
Changes since v9:
- Rebase onto current net-next, including the throttled-EQE support from
684603842b78 ("net: mana: Add support to process throttled EQEs").
- In patch 1, use one acquire-loaded CQ-table snapshot in the new throttle
reader, matching the existing normal-EQE reader and HWC table-withdrawal
lifetime contract.
- In patch 1, fence the HWC EQ before destroying its CQ, but keep the EQ
allocation alive until the CQ has detached its upstream child entry.
Preserve the upstream generic CQ detach/RCU implementation and allowance
for empty HWC CQ polls.
- Refresh patch 4's context while preserving the upstream doorbell
client_offset argument and address adjustment.
- Patches 2 and 3 retain their v9 changes. PF-directed queue geometry,
healthy-admission waiting and timeout/cancellation policy are unchanged.
Validation on this exact revision:
- GCC and Clang W=1 MANA Ethernet/RDMA target builds for the unmodified
base and each of the four intermediate commits.
- Strict netdev checkpatch with an 80-column limit and whitespace checks.
- Bounded actual-source userspace regression tests for request ownership,
admission, retention, teardown, initialization, rebuild and TX budgets.
- New GCC/Clang ASan/UBSan tests of the throttle reader and CQ/EQ cleanup
on patches 1 and 4, including partial setup, prior fencing, table
withdrawal and negative controls for the integration changes.
- Source review of ownership, IRQ/RCU readers and failure paths.
These are static/offline checks. No new full-kernel, hardware, runtime
or performance qualification is claimed for this rebase.
Previous revision:
https://lore.kernel.org/all/cover.1792679388.git.weh@linux.microsoft.com/
Deferral while net-next was closed:
https://lore.kernel.org/all/20261022082333.5a259426@kernel.org/
Base: fa5196392691051963726787944db872420dfa75
Long Li (4):
net: mana: prepare HWC ownership for safe reinitialization
net: mana: give each HWC message slot its own completion state
net: mana: support concurrent HWC requests
net: mana: add dynamic HWC queue depth with reinit path
drivers/infiniband/hw/mana/main.c | 19 +-
.../net/ethernet/microsoft/mana/gdma_cdx.c | 5 +-
.../net/ethernet/microsoft/mana/gdma_main.c | 162 ++-
.../net/ethernet/microsoft/mana/gdma_pci.c | 15 +-
.../net/ethernet/microsoft/mana/hw_channel.c | 1022 ++++++++++++++---
.../net/ethernet/microsoft/mana/shm_channel.c | 58 +-
include/net/mana/gdma.h | 17 +
include/net/mana/hw_channel.h | 70 +-
include/net/mana/shm_channel.h | 10 +-
9 files changed, 1157 insertions(+), 221 deletions(-)
base-commit: fa5196392691051963726787944db872420dfa75
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization
2026-11-09 15:44 [PATCH net-next v10 0/4] net: mana: concurrent HWC requests and dynamic queue depth Wei Hu
@ 2026-11-09 15:45 ` Wei Hu
2026-11-13 6:46 ` netdev-bot+sashiko
2026-11-09 15:45 ` [PATCH net-next v10 2/4] net: mana: give each HWC message slot its own completion state Wei Hu
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Wei Hu @ 2026-11-09 15:45 UTC (permalink / raw)
To: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel, gargaditya,
kees+treewide, kees, mawasthi, weh, edumazet
From: Long Li <longli@microsoft.com>
Dynamic HWC queue sizing tears down the bootstrap queues and establishes a
second channel before publishing it. Prepare the existing HWC ownership and
teardown paths so that sequence cannot free or reuse state still reachable
by either the PF or an EQ handler.
The shared-memory aperture carries one request at a time. Serialize the
complete ESTABLISH_HWC and DESTROY_HWC transactions, including possession
polling and response validation. Record setup_active immediately before the
MMIO submission and clear it only after DESTROY_HWC is acknowledged.
Treat an unacknowledged destroy as retained PF ownership. Fence the HWC EQ
and withdraw its CQ from dispatch, but keep all queue memory and wrappers
for a later teardown retry. A new channel creation retries that retained
context before assigning fresh queues to the PF.
When a PCI or CDX transport permanently discards its gdma_context, no such
retry is possible. After IRQ and service-work teardown, release the
software-only HWC state through a common final-disposal helper. Leave the
coherent DMA allocations unreclaimed rather than exposing freed memory to
a PF whose teardown was not acknowledged.
Snapshot the bootstrap CQ count and CQ ID after INIT_DONE. Publish the
fixed bound with the allocated table so later init events cannot grow the
bound past the allocation. Use a single acquire-loaded table snapshot in
both normal and throttled EQE dispatch, with annotated CQ-slot reads.
On acknowledged teardown, withdraw the CQ table and wait for all existing
IRQ-side RCU readers before freeing the HWC queue bundle. This also covers
a completion dispatched through a different EQ, without a per-event
reference. Fence the dedicated EQ's remaining callbacks before releasing
the CQ, completion buffer and work queues. Keep the parent EQ allocated
until CQ destruction has detached its throttled-EQE child entry. Preserve
the existing generic CQ detach and RCU handling.
Link: https://lore.kernel.org/r/20260908035201.402424-2-longli@microsoft.com
Link: https://lore.kernel.org/r/20260908035201.402424-5-longli@microsoft.com
Link: https://lore.kernel.org/r/178910960115.219967.13830871915506436112@kernel.org
Link: https://lore.kernel.org/r/179196575238.434549.10775387794024020898@kernel.org
Link: https://lore.kernel.org/r/sashiko-outbox-170992@kernel.org
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Wei Hu <weh@microsoft.com>
Copilot-Session: 8c2361e6-c0f4-4b0c-bf01-b5e4cdacec50
---
.../net/ethernet/microsoft/mana/gdma_cdx.c | 4 +-
.../net/ethernet/microsoft/mana/gdma_main.c | 41 +++-
.../net/ethernet/microsoft/mana/gdma_pci.c | 4 +-
.../net/ethernet/microsoft/mana/hw_channel.c | 187 +++++++++++++++---
.../net/ethernet/microsoft/mana/shm_channel.c | 58 +++++-
include/net/mana/gdma.h | 4 +
include/net/mana/hw_channel.h | 5 +
include/net/mana/shm_channel.h | 10 +-
8 files changed, 275 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_cdx.c b/drivers/net/ethernet/microsoft/mana/gdma_cdx.c
index 0d01e3d2f0d9..2e0dcb902d7e 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_cdx.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_cdx.c
@@ -279,7 +279,7 @@ static int mana_cdx_gd_probe(struct cdx_device *cdx_dev)
iounmap(bar0_va);
free_gc:
dev_set_drvdata(&cdx_dev->dev, NULL);
- vfree(gc);
+ mana_gd_free_context(gc);
clear_master:
cdx_clear_master(cdx_dev);
return err;
@@ -298,7 +298,7 @@ static int mana_cdx_gd_remove(struct cdx_device *cdx_dev)
iounmap(gc->bar0_va);
dev_set_drvdata(&cdx_dev->dev, NULL);
- vfree(gc);
+ mana_gd_free_context(gc);
return 0;
}
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index ab942d5bfe05..8df130386da3 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -11,6 +11,7 @@
#include <linux/irqdomain.h>
#include <linux/export.h>
#include <linux/uaccess.h>
+#include <linux/vmalloc.h>
#include <net/mana/mana.h>
#include <net/mana/hw_channel.h>
@@ -760,9 +761,15 @@ void mana_gd_free_service_wq(struct gdma_context *gc)
static void mana_gd_schedule_eq_cqs(struct gdma_queue *eq)
{
struct gdma_context *gc = eq->gdma_dev->gdma_context;
+ struct gdma_queue **cq_table;
struct gdma_queue *cq;
u8 i;
+ /* The IRQ's RCU read-side section protects this table snapshot. */
+ cq_table = smp_load_acquire(&gc->cq_table);
+ if (!cq_table)
+ return;
+
for (i = 0; i < GDMA_EQ_MAX_CHILD_CQ; i++) {
cq = rcu_dereference(eq->eq.child_cq[i]);
@@ -770,7 +777,7 @@ static void mana_gd_schedule_eq_cqs(struct gdma_queue *eq)
continue;
if (cq->id >= READ_ONCE(gc->max_num_cqs) ||
- !gc->cq_table || gc->cq_table[cq->id] != cq)
+ READ_ONCE(cq_table[cq->id]) != cq)
continue;
if (cq->cq.callback)
@@ -785,6 +792,7 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
union gdma_eqe_info eqe_info;
enum gdma_eqe_type type;
struct gdma_event event;
+ struct gdma_queue **cq_table;
struct gdma_queue *cq;
struct gdma_eqe *eqe;
u32 cq_id;
@@ -796,11 +804,16 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
switch (type) {
case GDMA_EQE_COMPLETION:
cq_id = eqe->details[0] & 0xFFFFFF;
- if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs))
+ /* The IRQ handler's RCU read-side section protects the table
+ * until HWC teardown has fenced its EQ and waited for readers.
+ */
+ cq_table = smp_load_acquire(&gc->cq_table);
+ if (!cq_table || cq_id >= READ_ONCE(gc->max_num_cqs))
break;
- cq = gc->cq_table[cq_id];
- if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
+ cq = READ_ONCE(cq_table[cq_id]);
+ if (!cq || WARN_ON_ONCE(cq->type != GDMA_CQ ||
+ cq->id != cq_id))
break;
if (cq->cq.callback)
@@ -1032,23 +1045,32 @@ int mana_gd_test_eq(struct gdma_context *gc, struct gdma_queue *eq)
return err;
}
-static void mana_gd_destroy_eq(struct gdma_context *gc, bool flush_evenets,
+static void mana_gd_destroy_eq(struct gdma_context *gc, bool flush_events,
struct gdma_queue *queue)
{
int err;
- if (flush_evenets) {
+ if (queue->eq.msix_index == INVALID_PCI_MSIX_INDEX)
+ return;
+
+ if (flush_events) {
err = mana_gd_test_eq(gc, queue);
if (err && mana_need_log(gc, err))
dev_warn(gc->dev, "Failed to flush EQ: %d\n", err);
}
mana_gd_deregister_irq(queue);
+ queue->eq.msix_index = INVALID_PCI_MSIX_INDEX;
if (queue->eq.disable_needed)
mana_gd_disable_queue(queue);
}
+void mana_gd_fence_eq(struct gdma_context *gc, struct gdma_queue *queue)
+{
+ mana_gd_destroy_eq(gc, false, queue);
+}
+
static int mana_gd_create_eq(struct gdma_dev *gd,
const struct gdma_queue_spec *spec,
bool create_hwq, struct gdma_queue *queue)
@@ -2113,6 +2135,13 @@ void mana_gd_cleanup(struct gdma_context *gc)
}
EXPORT_SYMBOL_NS(mana_gd_cleanup, "NET_MANA");
+void mana_gd_free_context(struct gdma_context *gc)
+{
+ mana_hwc_free_retained_channel(gc);
+ vfree(gc);
+}
+EXPORT_SYMBOL_NS(mana_gd_free_context, "NET_MANA");
+
static int __init gdma_core_init(void)
{
mana_debugfs_root = debugfs_create_dir("mana", NULL);
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_pci.c b/drivers/net/ethernet/microsoft/mana/gdma_pci.c
index ed8925296c8a..7935cb274e36 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_pci.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_pci.c
@@ -725,7 +725,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_iounmap(pdev, bar0_va);
free_gc:
pci_set_drvdata(pdev, NULL);
- vfree(gc);
+ mana_gd_free_context(gc);
release_region:
pci_release_regions(pdev);
disable_dev:
@@ -778,7 +778,7 @@ static void mana_gd_remove(struct pci_dev *pdev)
pci_iounmap(pdev, gc->bar0_va);
- vfree(gc);
+ mana_gd_free_context(gc);
pci_release_regions(pdev);
pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index fb9f38f65911..9e0964072af9 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -134,7 +134,7 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
switch (type) {
case HWC_INIT_DATA_CQID:
- hwc->cq->gdma_cq->id = val;
+ WRITE_ONCE(hwc->hwc_init_cq_id, val);
break;
case HWC_INIT_DATA_RQID:
@@ -158,7 +158,11 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
break;
case HWC_INIT_DATA_MAX_NUM_CQS:
- gd->gdma_context->max_num_cqs = val;
+ /* Store only; establish_channel() commits it to
+ * max_num_cqs once, so a later event cannot grow the
+ * bound past the allocation; pairs with READ_ONCE().
+ */
+ WRITE_ONCE(hwc->hwc_init_max_num_cqs, val);
break;
case HWC_INIT_DATA_PDID:
@@ -382,9 +386,43 @@ static void mana_hwc_comp_event(void *ctx, struct gdma_queue *q_self)
mana_gd_ring_cq(q_self, SET_ARM_BIT);
}
+static int mana_hwc_publish_cq(struct gdma_context *gc,
+ struct gdma_queue *cq)
+{
+ struct gdma_queue **cq_table = READ_ONCE(gc->cq_table);
+ u32 id = READ_ONCE(cq->id);
+
+ if (!cq_table || id >= READ_ONCE(gc->max_num_cqs) ||
+ READ_ONCE(cq_table[id]))
+ return -EINVAL;
+
+ WRITE_ONCE(cq_table[id], cq);
+
+ return 0;
+}
+
+static void mana_hwc_unpublish_cq(struct gdma_context *gc,
+ struct gdma_queue *cq)
+{
+ struct gdma_queue **cq_table = READ_ONCE(gc->cq_table);
+ u32 id;
+
+ if (!cq_table || !cq)
+ return;
+
+ id = READ_ONCE(cq->id);
+ if (id < READ_ONCE(gc->max_num_cqs) &&
+ READ_ONCE(cq_table[id]) == cq)
+ WRITE_ONCE(cq_table[id], NULL);
+}
+
static void mana_hwc_destroy_cq(struct gdma_context *gc, struct hwc_cq *hwc_cq)
{
- kfree(hwc_cq->comp_buf);
+ /* Fence callbacks first, but keep the parent EQ alive until CQ
+ * destruction has detached its child entry.
+ */
+ if (hwc_cq->gdma_eq)
+ mana_gd_fence_eq(gc, hwc_cq->gdma_eq);
if (hwc_cq->gdma_cq)
mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
@@ -392,6 +430,7 @@ static void mana_hwc_destroy_cq(struct gdma_context *gc, struct hwc_cq *hwc_cq)
if (hwc_cq->gdma_eq)
mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
+ kfree(hwc_cq->comp_buf);
kfree(hwc_cq);
}
@@ -674,6 +713,9 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
struct gdma_queue *sq = hwc->txq->gdma_wq;
struct gdma_queue *eq = hwc->cq->gdma_eq;
struct gdma_queue *cq = hwc->cq->gdma_cq;
+ struct gdma_queue **cq_table;
+ u32 num_cqs;
+ u32 cq_id;
int err;
init_completion(&hwc->hwc_init_eqe_comp);
@@ -683,7 +725,7 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
cq->mem_info.dma_handle,
rq->mem_info.dma_handle,
sq->mem_info.dma_handle,
- eq->eq.msix_index);
+ eq->eq.msix_index, &hwc->setup_active);
if (err)
return err;
@@ -694,15 +736,45 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
*max_req_msg_size = hwc->hwc_init_max_req_msg_size;
*max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
- /* Both were set in mana_hwc_init_event_handler(). */
- if (WARN_ON(cq->id >= gc->max_num_cqs))
+ /* Snapshot the device-reported count and id once, so the same value
+ * sizes, bounds and indexes cq_table even across the sleeping
+ * vcalloc() and a concurrent init event.
+ */
+ num_cqs = READ_ONCE(hwc->hwc_init_max_num_cqs);
+ cq_id = READ_ONCE(hwc->hwc_init_cq_id);
+
+ /* Both operands come from untrusted HWC bootstrap events; a missing
+ * MAX_NUM_CQS leaves num_cqs at 0. Reject rather than WARN_ON() so a
+ * malformed device response cannot panic a panic_on_warn guest.
+ */
+ if (cq_id >= num_cqs) {
+ dev_err_ratelimited(hwc->dev,
+ "HWC: bad CQ id %u >= max %u\n",
+ cq_id, num_cqs);
return -EPROTO;
+ }
+
+ /* Init events remain enabled, so commit the validated CQ ID once. */
+ WRITE_ONCE(cq->id, cq_id);
- gc->cq_table = vcalloc(gc->max_num_cqs, sizeof(struct gdma_queue *));
- if (!gc->cq_table)
+ cq_table = vcalloc(num_cqs, sizeof(*cq_table));
+ if (!cq_table)
return -ENOMEM;
- gc->cq_table[cq->id] = cq;
+ /* Publish the bound and the initialised table together; the release
+ * pairs with smp_load_acquire() in mana_gd_process_eqe().
+ */
+ WRITE_ONCE(gc->max_num_cqs, num_cqs);
+ /* Pairs with smp_load_acquire() in mana_gd_process_eqe(). */
+ smp_store_release(&gc->cq_table, cq_table);
+
+ err = mana_hwc_publish_cq(gc, cq);
+ if (err) {
+ dev_err_ratelimited(hwc->dev,
+ "HWC: failed to publish CQ %u: %d\n",
+ cq_id, err);
+ return err;
+ }
return 0;
}
@@ -751,6 +823,18 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
return err;
}
+static void mana_hwc_clear_cq_table(struct gdma_context *gc)
+{
+ struct gdma_queue **cq_table;
+
+ cq_table = READ_ONCE(gc->cq_table);
+ WRITE_ONCE(gc->max_num_cqs, 0);
+ /* Stop new table readers before waiting for prior RCU readers. */
+ smp_store_release(&gc->cq_table, NULL);
+ synchronize_rcu();
+ vfree(cq_table);
+}
+
int mana_hwc_create_channel(struct gdma_context *gc)
{
u32 max_req_msg_size, max_resp_msg_size;
@@ -759,6 +843,13 @@ int mana_hwc_create_channel(struct gdma_context *gc)
u16 q_depth_max;
int err;
+ /* Retry a retained context before assigning queues to the PF again. */
+ if (gd->driver_data) {
+ mana_hwc_destroy_channel(gc);
+ if (gd->driver_data)
+ return -ETIMEDOUT;
+ }
+
hwc = kzalloc_obj(*hwc);
if (!hwc)
return -ENOMEM;
@@ -808,25 +899,45 @@ int mana_hwc_create_channel(struct gdma_context *gc)
return err;
}
+static void mana_hwc_fence_channel(struct gdma_context *gc,
+ struct hw_channel_context *hwc)
+{
+ if (!hwc->cq)
+ return;
+
+ if (hwc->cq->gdma_eq)
+ mana_gd_fence_eq(gc, hwc->cq->gdma_eq);
+
+ if (hwc->cq->gdma_cq)
+ mana_hwc_unpublish_cq(gc, hwc->cq->gdma_cq);
+}
+
void mana_hwc_destroy_channel(struct gdma_context *gc)
{
struct hw_channel_context *hwc = gc->hwc.driver_data;
+ int err;
if (!hwc)
return;
- /* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's
- * non-zero, the HWC worked and we should tear down the HWC here.
+ /* An unacknowledged destroy leaves the PF's mappings live. Fence
+ * software dispatch, but retain every PF-visible allocation.
*/
- if (gc->max_num_cqs > 0) {
- mana_smc_teardown_hwc(&gc->shm_channel, false);
- WRITE_ONCE(gc->max_num_cqs, 0);
+ err = mana_smc_teardown_hwc(&gc->shm_channel, false,
+ &hwc->setup_active);
+ if (err) {
+ dev_err(hwc->dev,
+ "HWC teardown failed: %d, retaining PF-visible resources\n",
+ err);
+ mana_hwc_fence_channel(gc, hwc);
+ return;
}
- if (hwc->cq)
- mana_gd_detach_cq(gc, hwc->cq->gdma_cq);
+ mana_hwc_clear_cq_table(gc);
- synchronize_rcu();
+ /* Fence the EQ before releasing the remaining HWC state. */
+ if (hwc->cq)
+ mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
if (hwc->txq)
mana_hwc_destroy_wq(hwc, hwc->txq);
@@ -834,9 +945,6 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
if (hwc->rxq)
mana_hwc_destroy_wq(hwc, hwc->rxq);
- if (hwc->cq)
- mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
-
kfree(hwc->caller_ctx);
hwc->caller_ctx = NULL;
@@ -852,9 +960,44 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
kfree(hwc);
gc->hwc.driver_data = NULL;
gc->hwc.gdma_context = NULL;
+}
- vfree(gc->cq_table);
- gc->cq_table = NULL;
+static void mana_hwc_free_retained_wq(struct hwc_wq *wq)
+{
+ if (!wq)
+ return;
+
+ kfree(wq->msg_buf);
+ kfree(wq->gdma_wq);
+ kfree(wq);
+}
+
+/* Final disposal after IRQ and service-work teardown, not a retry path.
+ * Free only software state: unacknowledged HWC DMA allocations stay live.
+ */
+void mana_hwc_free_retained_channel(struct gdma_context *gc)
+{
+ struct hw_channel_context *hwc = gc->hwc.driver_data;
+
+ if (!hwc)
+ return;
+
+ gc->hwc.driver_data = NULL;
+ mana_hwc_clear_cq_table(gc);
+
+ if (hwc->cq) {
+ kfree(hwc->cq->gdma_eq);
+ kfree(hwc->cq->gdma_cq);
+ kfree(hwc->cq->comp_buf);
+ kfree(hwc->cq);
+ }
+
+ mana_hwc_free_retained_wq(hwc->txq);
+ mana_hwc_free_retained_wq(hwc->rxq);
+ kfree(hwc->caller_ctx);
+ mana_gd_free_res_map(&hwc->inflight_msg_res);
+ kfree(hwc);
+ gc->hwc.gdma_context = NULL;
}
int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
diff --git a/drivers/net/ethernet/microsoft/mana/shm_channel.c b/drivers/net/ethernet/microsoft/mana/shm_channel.c
index d21b5db06e50..df9d05062dd5 100644
--- a/drivers/net/ethernet/microsoft/mana/shm_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/shm_channel.c
@@ -125,13 +125,24 @@ static int mana_smc_read_response(struct shm_channel *sc, u32 msg_type,
void mana_smc_init(struct shm_channel *sc, struct device *dev,
void __iomem *base)
{
+ /* The first call precedes channel publication. Later calls refresh the
+ * BAR mapping after reset or resume without reinitializing a live lock.
+ */
+ if (!sc->transaction_lock_initialized) {
+ mutex_init(&sc->transaction_lock);
+ sc->transaction_lock_initialized = true;
+ }
+
+ mutex_lock(&sc->transaction_lock);
sc->dev = dev;
sc->base = base;
+ mutex_unlock(&sc->transaction_lock);
}
-int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
- u64 cq_addr, u64 rq_addr, u64 sq_addr,
- u32 eq_msix_index)
+static int mana_smc_setup_hwc_locked(struct shm_channel *sc, bool reset_vf,
+ u64 eq_addr, u64 cq_addr, u64 rq_addr,
+ u64 sq_addr, u32 eq_msix_index,
+ bool *submitted)
{
union smc_proto_hdr *hdr;
u16 all_addr_h4bits = 0;
@@ -144,6 +155,9 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
int err;
int i;
+ lockdep_assert_held(&sc->transaction_lock);
+ *submitted = false;
+
/* Ensure VF already has possession of shared memory */
err = mana_smc_poll_register(sc->base, false);
if (err) {
@@ -229,6 +243,7 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
/* Write 256-message buffer to shared memory (final 32-bit write
* triggers HW to set possession bit to PF).
*/
+ *submitted = true;
dword = (u32 *)shm_buf;
for (i = 0; i < SMC_APERTURE_DWORDS; i++)
writel(*dword++, sc->base + i * SMC_BASIC_UNIT);
@@ -248,11 +263,32 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
return 0;
}
-int mana_smc_teardown_hwc(struct shm_channel *sc, bool reset_vf)
+int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
+ u64 cq_addr, u64 rq_addr, u64 sq_addr,
+ u32 eq_msix_index, bool *submitted)
+{
+ int err;
+
+ mutex_lock(&sc->transaction_lock);
+ err = mana_smc_setup_hwc_locked(sc, reset_vf, eq_addr, cq_addr,
+ rq_addr, sq_addr, eq_msix_index,
+ submitted);
+ mutex_unlock(&sc->transaction_lock);
+
+ return err;
+}
+
+static int mana_smc_teardown_hwc_locked(struct shm_channel *sc, bool reset_vf,
+ bool *setup_active)
{
union smc_proto_hdr hdr = {};
int err;
+ lockdep_assert_held(&sc->transaction_lock);
+
+ if (!*setup_active)
+ return 0;
+
/* Ensure already has possession of shared memory */
err = mana_smc_poll_register(sc->base, false);
if (err) {
@@ -283,5 +319,19 @@ int mana_smc_teardown_hwc(struct shm_channel *sc, bool reset_vf)
return err;
}
+ *setup_active = false;
+
return 0;
}
+
+int mana_smc_teardown_hwc(struct shm_channel *sc, bool reset_vf,
+ bool *setup_active)
+{
+ int err;
+
+ mutex_lock(&sc->transaction_lock);
+ err = mana_smc_teardown_hwc_locked(sc, reset_vf, setup_active);
+ mutex_unlock(&sc->transaction_lock);
+
+ return err;
+}
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 29d152e18596..af62cd6d771e 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -596,6 +596,9 @@ int mana_gd_create_mana_wq_cq(struct gdma_dev *gd,
void mana_gd_destroy_queue(struct gdma_context *gc, struct gdma_queue *queue);
+/* Remove an EQ from interrupt dispatch without freeing its queue memory. */
+void mana_gd_fence_eq(struct gdma_context *gc, struct gdma_queue *queue);
+
int mana_gd_poll_cq(struct gdma_queue *cq, struct gdma_comp *comp, int num_cqe);
void mana_gd_ring_cq(struct gdma_queue *cq, u8 arm_bit);
@@ -1146,6 +1149,7 @@ int mana_gd_verify_vf_version(struct gdma_context *gc);
int mana_gd_query_max_resources(struct gdma_context *gc);
int mana_gd_setup(struct gdma_context *gc);
void mana_gd_cleanup(struct gdma_context *gc);
+void mana_gd_free_context(struct gdma_context *gc);
int mana_gd_detect_devices(struct gdma_context *gc);
int mana_gd_init_registers(struct gdma_context *gc);
irqreturn_t mana_gd_intr(int irq, void *arg);
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 1ab2d66c891e..1320bdfa9d2a 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -185,6 +185,8 @@ struct hw_channel_context {
u16 hwc_init_q_depth_max;
u32 hwc_init_max_req_msg_size;
u32 hwc_init_max_resp_msg_size;
+ u32 hwc_init_max_num_cqs;
+ u32 hwc_init_cq_id;
struct completion hwc_init_eqe_comp;
@@ -199,11 +201,14 @@ struct hw_channel_context {
u32 dest_vrcq_id;
u32 hwc_timeout;
+ /* The PF may own the HWC queues while this is true. */
+ bool setup_active;
struct hwc_caller_ctx *caller_ctx;
};
int mana_hwc_create_channel(struct gdma_context *gc);
void mana_hwc_destroy_channel(struct gdma_context *gc);
+void mana_hwc_free_retained_channel(struct gdma_context *gc);
int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
const void *req, u32 resp_len, void *resp);
diff --git a/include/net/mana/shm_channel.h b/include/net/mana/shm_channel.h
index dbabcfb95daf..c8daa61fa759 100644
--- a/include/net/mana/shm_channel.h
+++ b/include/net/mana/shm_channel.h
@@ -4,6 +4,8 @@
#ifndef _SHM_CHANNEL_H
#define _SHM_CHANNEL_H
+#include <linux/mutex.h>
+
#define SMC_APERTURE_BITS 256
#define SMC_BASIC_UNIT (sizeof(u32))
#define SMC_APERTURE_DWORDS (SMC_APERTURE_BITS / (SMC_BASIC_UNIT * 8))
@@ -13,6 +15,9 @@
struct shm_channel {
struct device *dev;
void __iomem *base;
+ /* Protects base and complete aperture request/response transactions. */
+ struct mutex transaction_lock;
+ bool transaction_lock_initialized;
};
void mana_smc_init(struct shm_channel *sc, struct device *dev,
@@ -20,8 +25,9 @@ void mana_smc_init(struct shm_channel *sc, struct device *dev,
int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
u64 cq_addr, u64 rq_addr, u64 sq_addr,
- u32 eq_msix_index);
+ u32 eq_msix_index, bool *submitted);
-int mana_smc_teardown_hwc(struct shm_channel *sc, bool reset_vf);
+int mana_smc_teardown_hwc(struct shm_channel *sc, bool reset_vf,
+ bool *setup_active);
#endif /* _SHM_CHANNEL_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next v10 2/4] net: mana: give each HWC message slot its own completion state
2026-11-09 15:44 [PATCH net-next v10 0/4] net: mana: concurrent HWC requests and dynamic queue depth Wei Hu
2026-11-09 15:45 ` [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization Wei Hu
@ 2026-11-09 15:45 ` Wei Hu
2026-11-13 6:46 ` netdev-bot+sashiko
2026-11-09 15:45 ` [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests Wei Hu
2026-11-09 15:45 ` [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path Wei Hu
3 siblings, 1 reply; 10+ messages in thread
From: Wei Hu @ 2026-11-09 15:45 UTC (permalink / raw)
To: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel, gargaditya,
kees+treewide, kees, mawasthi, weh, edumazet
From: Long Li <longli@microsoft.com>
Concurrent requests require the response handler and the waiting sender to
agree on the lifetime and result state of each message slot.
Protect the caller's output buffer, Linux error and device status with a
per-slot lock. Publish the output buffer only while the sender is waiting
and withdraw it before every return. A late response cannot copy through a
pointer to caller storage whose lifetime has ended.
Reinitialize completion and result state under both the allocation and
per-slot locks when acquiring the slot. A response handler can still hold
the per-slot lock after the previous sender has returned its admission
permit, so an unpublished bitmap bit alone is not sufficient protection.
If the response records its result while the timed wait expires, return
that result instead of reporting a false timeout.
Keep the baseline's post-timeout admission behavior at this step rather
than introducing a temporary channel-wide latch that suppresses subsequent
teardown commands. This does not solve late-response correlation after ID
reuse. The next patch adds response ownership, per-slot quarantine and the
associated admission and teardown rules together.
Split response-status handling and request cleanup into helpers so the send
path has structured returns rather than phase-jumping gotos.
Link: https://lore.kernel.org/r/20260908035201.402424-3-longli@microsoft.com
Link: https://lore.kernel.org/r/20260908035201.402424-4-longli@microsoft.com
Link: https://lore.kernel.org/r/179196575434.434549.4428132041097324407@kernel.org
Link: https://lore.kernel.org/r/179196575640.434549.9102950927041409151@kernel.org
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Wei Hu <weh@microsoft.com>
Copilot-Session: 8c2361e6-c0f4-4b0c-bf01-b5e4cdacec50
---
.../net/ethernet/microsoft/mana/hw_channel.c | 125 +++++++++++++-----
include/net/mana/hw_channel.h | 4 +-
2 files changed, 95 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 9e0964072af9..6f0ccdeffc4f 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -9,6 +9,7 @@
static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
{
struct gdma_resource *r = &hwc->inflight_msg_res;
+ struct hwc_caller_ctx *ctx;
unsigned long flags;
u32 index;
@@ -19,8 +20,17 @@ static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
index = find_first_zero_bit(hwc->inflight_msg_res.map,
hwc->inflight_msg_res.size);
+ ctx = &hwc->caller_ctx[index];
+ spin_lock(&ctx->lock);
+ reinit_completion(&ctx->comp_event);
+ ctx->output_buf = NULL;
+ ctx->output_buflen = 0;
+ ctx->error = -EINPROGRESS;
+ ctx->status_code = 0;
+
bitmap_set(hwc->inflight_msg_res.map, index, 1);
+ spin_unlock(&ctx->lock);
spin_unlock_irqrestore(&r->lock, flags);
*msg_id = index;
@@ -90,14 +100,21 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
}
ctx = hwc->caller_ctx + msg_id;
- err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
- if (err)
- goto out;
- ctx->status_code = resp_msg->status;
+ spin_lock(&ctx->lock);
+ if (!ctx->output_buf) {
+ spin_unlock(&ctx->lock);
+ mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
+ return;
+ }
- memcpy(ctx->output_buf, resp_msg, resp_len);
-out:
+ err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
+ if (!err) {
+ ctx->status_code = resp_msg->status;
+ memcpy(ctx->output_buf, resp_msg, resp_len);
+ }
+
+ ctx->output_buf = NULL;
ctx->error = err;
/* Must post rx wqe before complete(), otherwise the next rx may
@@ -106,6 +123,7 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
complete(&ctx->comp_event);
+ spin_unlock(&ctx->lock);
}
static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
@@ -696,8 +714,10 @@ static int mana_hwc_test_channel(struct hw_channel_context *hwc, u16 q_depth,
if (!ctx)
return -ENOMEM;
- for (i = 0; i < q_depth; ++i)
+ for (i = 0; i < q_depth; ++i) {
init_completion(&ctx[i].comp_event);
+ spin_lock_init(&ctx[i].lock);
+ }
hwc->caller_ctx = ctx;
@@ -1000,6 +1020,37 @@ void mana_hwc_free_retained_channel(struct gdma_context *gc)
gc->hwc.gdma_context = NULL;
}
+static int mana_hwc_response_status(struct hw_channel_context *hwc,
+ u32 command, int error, u32 status)
+{
+ if (error)
+ return error;
+
+ if (!status || status == GDMA_STATUS_MORE_ENTRIES)
+ return 0;
+
+ if (status == GDMA_STATUS_CMD_UNSUPPORTED)
+ return -EOPNOTSUPP;
+
+ if (command != MANA_QUERY_PHY_STAT)
+ dev_err(hwc->dev, "Command 0x%x failed with status: 0x%x\n",
+ command, status);
+
+ return -EPROTO;
+}
+
+static void mana_hwc_finish_request(struct hw_channel_context *hwc,
+ struct hwc_caller_ctx *ctx, u16 msg_id)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->output_buf = NULL;
+ spin_unlock_irqrestore(&ctx->lock, flags);
+
+ mana_hwc_put_msg_index(hwc, msg_id);
+}
+
int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
const void *req, u32 resp_len, void *resp)
{
@@ -1007,26 +1058,32 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
struct hwc_wq *txq = hwc->txq;
struct gdma_req_hdr *req_msg;
struct hwc_caller_ctx *ctx;
+ unsigned long flags;
u32 dest_vrcq;
u32 dest_vrq;
u32 command;
+ u32 status;
u16 msg_id;
int err;
- mana_hwc_get_msg_index(hwc, &msg_id);
+ err = mana_hwc_get_msg_index(hwc, &msg_id);
+ if (err)
+ return err;
tx_wr = &txq->msg_buf->reqs[msg_id];
+ ctx = hwc->caller_ctx + msg_id;
if (req_len > tx_wr->buf_len) {
dev_err(hwc->dev, "HWC: req msg size: %d > %d\n", req_len,
tx_wr->buf_len);
- err = -EINVAL;
- goto out;
+ mana_hwc_finish_request(hwc, ctx, msg_id);
+ return -EINVAL;
}
- ctx = hwc->caller_ctx + msg_id;
+ spin_lock_irqsave(&ctx->lock, flags);
ctx->output_buf = resp;
ctx->output_buflen = resp_len;
+ spin_unlock_irqrestore(&ctx->lock, flags);
req_msg = (struct gdma_req_hdr *)tx_wr->buf_va;
if (req)
@@ -1048,11 +1105,24 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);
if (err) {
dev_err(hwc->dev, "HWC: Failed to post send WQE: %d\n", err);
- goto out;
+ mana_hwc_finish_request(hwc, ctx, msg_id);
+ return err;
}
if (!wait_for_completion_timeout(&ctx->comp_event,
- (msecs_to_jiffies(hwc->hwc_timeout)))) {
+ msecs_to_jiffies(hwc->hwc_timeout))) {
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->output_buf = NULL;
+ err = ctx->error;
+ status = ctx->status_code;
+ spin_unlock_irqrestore(&ctx->lock, flags);
+
+ if (err != -EINPROGRESS) {
+ mana_hwc_finish_request(hwc, ctx, msg_id);
+ return mana_hwc_response_status(hwc, command, err,
+ status);
+ }
+
if (hwc->hwc_timeout != 0)
dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
command, hwc->hwc_timeout);
@@ -1061,27 +1131,16 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
if (hwc->hwc_timeout > 1)
hwc->hwc_timeout = 1;
- err = -ETIMEDOUT;
- goto out;
+ mana_hwc_finish_request(hwc, ctx, msg_id);
+ return -ETIMEDOUT;
}
- if (ctx->error) {
- err = ctx->error;
- goto out;
- }
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->output_buf = NULL;
+ err = ctx->error;
+ status = ctx->status_code;
+ spin_unlock_irqrestore(&ctx->lock, flags);
- if (ctx->status_code && ctx->status_code != GDMA_STATUS_MORE_ENTRIES) {
- if (ctx->status_code == GDMA_STATUS_CMD_UNSUPPORTED) {
- err = -EOPNOTSUPP;
- goto out;
- }
- if (command != MANA_QUERY_PHY_STAT)
- dev_err(hwc->dev, "Command 0x%x failed with status: 0x%x\n",
- command, ctx->status_code);
- err = -EPROTO;
- goto out;
- }
-out:
- mana_hwc_put_msg_index(hwc, msg_id);
- return err;
+ mana_hwc_finish_request(hwc, ctx, msg_id);
+ return mana_hwc_response_status(hwc, command, err, status);
}
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 1320bdfa9d2a..c7570a5f7376 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -168,10 +168,12 @@ struct hwc_wq {
struct hwc_caller_ctx {
struct completion comp_event;
+ /* Protects initialization, output storage and response state. */
+ spinlock_t lock;
void *output_buf;
u32 output_buflen;
- u32 error; /* Linux error code */
+ int error; /* Linux error code */
u32 status_code;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests
2026-11-09 15:44 [PATCH net-next v10 0/4] net: mana: concurrent HWC requests and dynamic queue depth Wei Hu
2026-11-09 15:45 ` [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization Wei Hu
2026-11-09 15:45 ` [PATCH net-next v10 2/4] net: mana: give each HWC message slot its own completion state Wei Hu
@ 2026-11-09 15:45 ` Wei Hu
2026-11-13 6:46 ` netdev-bot+sashiko
2026-11-09 15:45 ` [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path Wei Hu
3 siblings, 1 reply; 10+ messages in thread
From: Wei Hu @ 2026-11-09 15:45 UTC (permalink / raw)
To: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel, gargaditya,
kees+treewide, kees, mawasthi, weh, edumazet
From: Long Li <longli@microsoft.com>
Give HWC message slots separate sender and response ownership. Initialize
each slot under the inflight resource and per-slot locks, publish its
bitmap bit last, and release the slot only after both references are
released. The per-slot lock also excludes an older duplicate callback from
resetting or consuming the next caller's state.
Serialize SQ posting so concurrent senders cannot corrupt the producer
state. A posted request which times out keeps its response reference and
slot until its late response arrives or teardown cancels it. The bootstrap
channel remains depth one here; the next patch selects and allocates
larger queues.
Wait for a free slot while the channel is healthy instead of imposing a
separate admission deadline. A congestion-only -EBUSY can make destructive
callers skip a teardown command and still free its DMA memory. Keeping
healthy callers queued avoids introducing that failure into the existing
cleanup paths. The response wait of an admitted command remains bounded.
Wake admission waiters when a slot is released, a response is abandoned,
waits are cancelled or the channel stops. If no slot is available, return
-ETIMEDOUT for outstanding abandoned responses or cancelled waits, and
-ENODEV for a stopped channel. This also handles waiters which arrived
before abandonment was recorded, without mistaking healthy contention
for a failed command. Retire the abandoned-response count with its
response reference exactly once.
Keep available slots usable for best-effort commands when waits have been
cancelled, preserving the existing cleanup policy. Suppress expected
timeout/device-gone logging after cancellation or unpublication, but keep
logging live-channel failures and other error classes. This does not
repair the wider data-queue DMA policy after a real control-command
failure or establish device quiescence.
Synchronize hwc_timeout updates with atomic access helpers. Cancellation
is terminal for a channel instance; firmware and query updates may replace
only a live timeout, and fail-fast may only lower a live value to one
millisecond.
Keep setup private until queues, admission state, caller contexts and
the direct EQ test are ready. Runtime teardown unpublishes the channel,
rejects new admissions, wakes waiters and drains active senders before
releasing or retaining PF-owned mappings. Apply the same unpublication
and sender-drain rules to final disposal of retained software state.
Split admission, submission, waiting, response status and unwind into
helpers so mana_hwc_send_request() uses structured returns.
Link: https://lore.kernel.org/r/20260908035201.402424-4-longli@microsoft.com
Link: https://lore.kernel.org/r/20260914175044.2a26bb46@kernel.org
Link: https://lore.kernel.org/r/179196575640.434549.9102950927041409151@kernel.org
Link: https://lore.kernel.org/r/179250339266.3135.15035365058593192217@kernel.org
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Wei Hu <weh@microsoft.com>
Copilot-Session: 8c2361e6-c0f4-4b0c-bf01-b5e4cdacec50
---
.../net/ethernet/microsoft/mana/gdma_cdx.c | 1 +
.../net/ethernet/microsoft/mana/gdma_main.c | 95 ++-
.../net/ethernet/microsoft/mana/gdma_pci.c | 11 +-
.../net/ethernet/microsoft/mana/hw_channel.c | 598 +++++++++++++-----
include/net/mana/gdma.h | 11 +
include/net/mana/hw_channel.h | 46 +-
6 files changed, 574 insertions(+), 188 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_cdx.c b/drivers/net/ethernet/microsoft/mana/gdma_cdx.c
index 2e0dcb902d7e..fa10681fd793 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_cdx.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_cdx.c
@@ -221,6 +221,7 @@ static int mana_cdx_gd_probe(struct cdx_device *cdx_dev)
mutex_init(&gc->eq_test_event_mutex);
mutex_init(&gc->gic_mutex);
+ spin_lock_init(&gc->hwc_lock);
dev_set_drvdata(&cdx_dev->dev, gc);
gc->bar0_pa = cdx_resource_start(cdx_dev, 0);
gc->bar0_size = cdx_resource_len(cdx_dev, 0);
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 8df130386da3..bb4a963c2cf5 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -142,22 +142,26 @@ int mana_gd_init_registers(struct gdma_context *gc)
return mana_gd_init_vf_regs(gc);
}
-/* Suppress logging when we set timeout to zero */
+/* Cancellation and unpublication make teardown request failures expected. */
bool mana_need_log(struct gdma_context *gc, int err)
{
struct hw_channel_context *hwc;
+ bool need_log = true;
+ unsigned long flags;
- if (err != -ETIMEDOUT)
+ if (err != -ETIMEDOUT && err != -ENODEV)
return true;
if (!gc)
return true;
+ spin_lock_irqsave(&gc->hwc_lock, flags);
hwc = gc->hwc.driver_data;
- if (hwc && hwc->hwc_timeout == 0)
- return false;
+ if (!hwc || !mana_hwc_timeout_read(hwc))
+ need_log = false;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
- return true;
+ return need_log;
}
int mana_gd_query_max_resources(struct gdma_context *gc)
@@ -310,7 +314,8 @@ int mana_gd_query_max_resources(struct gdma_context *gc)
return 0;
}
-static int mana_gd_query_hwc_timeout(struct gdma_context *gc, u32 *timeout_val)
+static int mana_gd_query_hwc_timeout(struct gdma_context *gc, u32 timeout_ms,
+ u32 *new_timeout_ms)
{
struct gdma_query_hwc_timeout_resp resp = {};
struct gdma_query_hwc_timeout_req req = {};
@@ -318,12 +323,12 @@ static int mana_gd_query_hwc_timeout(struct gdma_context *gc, u32 *timeout_val)
mana_gd_init_req_hdr(&req.hdr, GDMA_QUERY_HWC_TIMEOUT,
sizeof(req), sizeof(resp));
- req.timeout_ms = *timeout_val;
+ req.timeout_ms = timeout_ms;
err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
if (err || resp.hdr.status)
return err ? err : -EPROTO;
- *timeout_val = resp.timeout_ms;
+ *new_timeout_ms = resp.timeout_ms;
return 0;
}
@@ -378,12 +383,48 @@ int mana_gd_detect_devices(struct gdma_context *gc)
int mana_gd_send_request(struct gdma_context *gc, u32 req_len, const void *req,
u32 resp_len, void *resp)
{
- struct hw_channel_context *hwc = gc->hwc.driver_data;
+ struct hw_channel_context *hwc;
+ unsigned long flags;
+ int err;
+
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ hwc = gc->hwc.driver_data;
+ if (!hwc) {
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+ return -ENODEV;
+ }
+ hwc->active_senders++;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ err = mana_hwc_send_request(hwc, req_len, req, resp_len, resp);
- return mana_hwc_send_request(hwc, req_len, req, resp_len, resp);
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ if (--hwc->active_senders == 0)
+ wake_up(&gc->hwc_drain_waitq);
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ return err;
}
EXPORT_SYMBOL_NS(mana_gd_send_request, "NET_MANA");
+int mana_gd_cancel_hwc_timeout(struct gdma_context *gc)
+{
+ struct hw_channel_context *hwc;
+ unsigned long flags;
+ int err = 0;
+
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ hwc = gc->hwc.driver_data;
+ if (hwc)
+ mana_hwc_timeout_cancel(hwc);
+ else
+ err = -ENODEV;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ return err;
+}
+EXPORT_SYMBOL_NS(mana_gd_cancel_hwc_timeout, "NET_MANA");
+
int mana_gd_alloc_memory(struct gdma_context *gc, unsigned int length,
struct gdma_mem_info *gmi, bool allow_scatter)
{
@@ -996,7 +1037,9 @@ static void mana_gd_deregister_irq(struct gdma_queue *queue)
synchronize_rcu();
}
-int mana_gd_test_eq(struct gdma_context *gc, struct gdma_queue *eq)
+static int mana_gd_test_eq_request(struct gdma_context *gc,
+ struct hw_channel_context *hwc,
+ struct gdma_queue *eq)
{
struct gdma_generate_test_event_req req = {};
struct gdma_general_resp resp = {};
@@ -1014,7 +1057,12 @@ int mana_gd_test_eq(struct gdma_context *gc, struct gdma_queue *eq)
req.hdr.dev_id = eq->gdma_dev->dev_id;
req.queue_index = eq->id;
- err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+ if (hwc)
+ err = mana_hwc_send_request(hwc, sizeof(req), &req,
+ sizeof(resp), &resp);
+ else
+ err = mana_gd_send_request(gc, sizeof(req), &req,
+ sizeof(resp), &resp);
if (err) {
if (mana_need_log(gc, err))
dev_err(dev, "test_eq failed: %d\n", err);
@@ -1045,6 +1093,17 @@ int mana_gd_test_eq(struct gdma_context *gc, struct gdma_queue *eq)
return err;
}
+int mana_gd_test_eq(struct gdma_context *gc, struct gdma_queue *eq)
+{
+ return mana_gd_test_eq_request(gc, NULL, eq);
+}
+
+int mana_gd_test_hwc_eq(struct hw_channel_context *hwc,
+ struct gdma_queue *eq)
+{
+ return mana_gd_test_eq_request(hwc->gdma_dev->gdma_context, hwc, eq);
+}
+
static void mana_gd_destroy_eq(struct gdma_context *gc, bool flush_events,
struct gdma_queue *queue)
{
@@ -1280,6 +1339,7 @@ static int mana_gd_create_dma_region(struct gdma_dev *gd,
if (gmi->nr_pages == 0 && !MANA_PAGE_ALIGNED(gmi->virt_addr))
return -EINVAL;
+ /* The caller must keep the HWC alive throughout queue creation. */
hwc = gc->hwc.driver_data;
req_msg_size = struct_size(req, page_addr_list, num_page);
if (req_msg_size > hwc->max_req_msg_size)
@@ -1482,9 +1542,12 @@ int mana_gd_verify_vf_version(struct gdma_context *gc)
struct gdma_verify_ver_resp resp = {};
struct gdma_verify_ver_req req = {};
struct hw_channel_context *hwc;
+ u32 timeout_ms;
int err;
+ /* The setup caller must exclude concurrent HWC teardown. */
hwc = gc->hwc.driver_data;
+
mana_gd_init_req_hdr(&req.hdr, GDMA_VERIFY_VF_DRIVER_VERSION,
sizeof(req), sizeof(resp));
@@ -1522,12 +1585,16 @@ int mana_gd_verify_vf_version(struct gdma_context *gc)
&gc->pf_cap_flags1);
if (resp.pf_cap_flags1 & GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG) {
- err = mana_gd_query_hwc_timeout(gc, &hwc->hwc_timeout);
+ err = mana_gd_query_hwc_timeout(gc,
+ mana_hwc_timeout_read(hwc),
+ &timeout_ms);
if (err) {
dev_err(gc->dev, "Failed to set the hwc timeout %d\n", err);
return err;
}
- dev_dbg(gc->dev, "set the hwc timeout to %u\n", hwc->hwc_timeout);
+ mana_hwc_timeout_update(hwc, timeout_ms);
+ dev_dbg(gc->dev, "set the hwc timeout to %u\n",
+ mana_hwc_timeout_read(hwc));
}
return 0;
}
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_pci.c b/drivers/net/ethernet/microsoft/mana/gdma_pci.c
index 7935cb274e36..636d9b0dd581 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_pci.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_pci.c
@@ -14,7 +14,6 @@
#include <linux/xarray.h>
#include <net/mana/mana.h>
-#include <net/mana/hw_channel.h>
struct mana_dev_recovery {
struct list_head list;
@@ -85,7 +84,6 @@ static void mana_serv_fpga(struct pci_dev *pdev)
static void mana_serv_reset(struct pci_dev *pdev)
{
struct gdma_context *gc = pci_get_drvdata(pdev);
- struct hw_channel_context *hwc;
int ret;
if (!gc) {
@@ -95,15 +93,13 @@ static void mana_serv_reset(struct pci_dev *pdev)
return;
}
- hwc = gc->hwc.driver_data;
- if (!hwc) {
+ /* HWC is not responding in this case, so don't wait */
+ ret = mana_gd_cancel_hwc_timeout(gc);
+ if (ret) {
dev_err(&pdev->dev, "MANA service: no HWC\n");
goto out;
}
- /* HWC is not responding in this case, so don't wait */
- hwc->hwc_timeout = 0;
-
dev_info(&pdev->dev, "MANA reset cycle start\n");
mana_gd_suspend(pdev, PMSG_SUSPEND);
@@ -674,6 +670,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
mutex_init(&gc->eq_test_event_mutex);
mutex_init(&gc->gic_mutex);
+ spin_lock_init(&gc->hwc_lock);
pci_set_drvdata(pdev, gc);
gc->bar0_pa = pci_resource_start(pdev, 0);
gc->bar0_size = pci_resource_len(pdev, 0);
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 6f0ccdeffc4f..8bc00001d20e 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -6,48 +6,140 @@
#include <net/mana/hw_channel.h>
#include <linux/vmalloc.h>
-static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
+u32 mana_hwc_timeout_read(const struct hw_channel_context *hwc)
+{
+ return READ_ONCE(hwc->hwc_timeout);
+}
+
+void mana_hwc_timeout_update(struct hw_channel_context *hwc, u32 timeout_ms)
+{
+ u32 old_timeout;
+
+ if (!timeout_ms)
+ return;
+
+ old_timeout = mana_hwc_timeout_read(hwc);
+ while (old_timeout &&
+ cmpxchg(&hwc->hwc_timeout, old_timeout,
+ timeout_ms) != old_timeout)
+ old_timeout = mana_hwc_timeout_read(hwc);
+}
+
+void mana_hwc_timeout_cancel(struct hw_channel_context *hwc)
+{
+ xchg(&hwc->hwc_timeout, 0);
+ wake_up_all(&hwc->admission_waitq);
+}
+
+static void mana_hwc_timeout_reduce(struct hw_channel_context *hwc)
+{
+ u32 old_timeout = mana_hwc_timeout_read(hwc);
+
+ while (old_timeout > 1 &&
+ cmpxchg(&hwc->hwc_timeout, old_timeout, 1) != old_timeout)
+ old_timeout = mana_hwc_timeout_read(hwc);
+}
+
+static int mana_hwc_try_get_msg_index(struct hw_channel_context *hwc,
+ void *resp, u32 resp_len,
+ struct hwc_caller_ctx **caller_ctx)
{
struct gdma_resource *r = &hwc->inflight_msg_res;
struct hwc_caller_ctx *ctx;
unsigned long flags;
u32 index;
-
- down(&hwc->sema);
+ int err;
spin_lock_irqsave(&r->lock, flags);
+ if (!hwc->channel_up) {
+ spin_unlock_irqrestore(&r->lock, flags);
+ return -ENODEV;
+ }
- index = find_first_zero_bit(hwc->inflight_msg_res.map,
- hwc->inflight_msg_res.size);
+ index = find_first_zero_bit(r->map, r->size);
+ if (index >= r->size) {
+ err = -EBUSY;
+ if (!mana_hwc_timeout_read(hwc) ||
+ atomic_read(&hwc->abandoned_msgs))
+ err = -ETIMEDOUT;
+ spin_unlock_irqrestore(&r->lock, flags);
+ return err;
+ }
ctx = &hwc->caller_ctx[index];
spin_lock(&ctx->lock);
reinit_completion(&ctx->comp_event);
- ctx->output_buf = NULL;
- ctx->output_buflen = 0;
+ /* Take both references (sender + response handler) before publishing
+ * the slot, so an early response cannot free it under the sender.
+ */
+ refcount_set(&ctx->refcnt, 2);
+ ctx->output_buf = resp;
+ ctx->output_buflen = resp_len;
ctx->error = -EINPROGRESS;
ctx->status_code = 0;
+ ctx->responded = false;
+ ctx->resp_pending = true;
+ ctx->msg_id = index;
- bitmap_set(hwc->inflight_msg_res.map, index, 1);
+ /* The response path takes r->lock before ctx->lock, so publishing the
+ * bitmap last makes every field above visible before it can consume
+ * the response-side reference.
+ */
+ bitmap_set(r->map, index, 1);
spin_unlock(&ctx->lock);
spin_unlock_irqrestore(&r->lock, flags);
- *msg_id = index;
+ *caller_ctx = ctx;
return 0;
}
+static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, void *resp,
+ u32 resp_len,
+ struct hwc_caller_ctx **caller_ctx)
+{
+ int err;
+
+ /* Healthy contention must not skip a destructive control request. */
+ wait_event(hwc->admission_waitq,
+ (err = mana_hwc_try_get_msg_index(hwc, resp, resp_len,
+ caller_ctx)) != -EBUSY);
+ return err;
+}
+
static void mana_hwc_put_msg_index(struct hw_channel_context *hwc, u16 msg_id)
{
struct gdma_resource *r = &hwc->inflight_msg_res;
unsigned long flags;
spin_lock_irqsave(&r->lock, flags);
- bitmap_clear(hwc->inflight_msg_res.map, msg_id, 1);
+ bitmap_clear(r->map, msg_id, 1);
spin_unlock_irqrestore(&r->lock, flags);
- up(&hwc->sema);
+ wake_up(&hwc->admission_waitq);
+}
+
+static void hwc_ctx_put(struct hw_channel_context *hwc,
+ struct hwc_caller_ctx *ctx)
+{
+ if (refcount_dec_and_test(&ctx->refcnt))
+ mana_hwc_put_msg_index(hwc, ctx->msg_id);
+}
+
+static bool mana_hwc_clear_resp_pending(struct hw_channel_context *hwc,
+ struct hwc_caller_ctx *ctx)
+{
+ lockdep_assert_held(&ctx->lock);
+
+ if (!ctx->resp_pending)
+ return false;
+
+ ctx->resp_pending = false;
+ if (ctx->error == -ETIMEDOUT)
+ atomic_dec(&hwc->abandoned_msgs);
+
+ return true;
}
static int mana_hwc_verify_resp_msg(const struct hwc_caller_ctx *caller_ctx,
@@ -90,40 +182,53 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
struct hwc_work_request *rx_req, u16 msg_id)
{
const struct gdma_resp_hdr *resp_msg = rx_req->buf_va;
+ struct gdma_resource *r = &hwc->inflight_msg_res;
struct hwc_caller_ctx *ctx;
+ bool release;
int err;
- if (!test_bit(msg_id, hwc->inflight_msg_res.map)) {
+ spin_lock(&r->lock);
+ if (!test_bit(msg_id, r->map)) {
+ spin_unlock(&r->lock);
dev_err(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id);
mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
return;
}
ctx = hwc->caller_ctx + msg_id;
-
spin_lock(&ctx->lock);
- if (!ctx->output_buf) {
+ spin_unlock(&r->lock);
+
+ /* Consume the response-side reference exactly once. This releases a
+ * quarantined slot after its late response arrives.
+ */
+ release = mana_hwc_clear_resp_pending(hwc, ctx);
+
+ if (ctx->responded) {
spin_unlock(&ctx->lock);
mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
+ if (release)
+ hwc_ctx_put(hwc, ctx);
return;
}
+ ctx->responded = true;
err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
if (!err) {
ctx->status_code = resp_msg->status;
memcpy(ctx->output_buf, resp_msg, resp_len);
}
-
- ctx->output_buf = NULL;
ctx->error = err;
- /* Must post rx wqe before complete(), otherwise the next rx may
- * hit no_wqe error.
+ /* Post RX WQE before completing; the next response may arrive
+ * immediately and needs a posted buffer.
*/
mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
-
complete(&ctx->comp_event);
spin_unlock(&ctx->lock);
+
+ if (release)
+ hwc_ctx_put(hwc, ctx);
}
static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
@@ -214,7 +319,7 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
switch (type) {
case HWC_DATA_CFG_HWC_TIMEOUT:
- hwc->hwc_timeout = val;
+ mana_hwc_timeout_update(hwc, val);
break;
case HWC_DATA_HW_LINK_CONNECT:
@@ -617,6 +722,7 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
hwc_wq->gdma_wq = queue;
hwc_wq->queue_depth = q_depth;
hwc_wq->hwc_cq = hwc_cq;
+ spin_lock_init(&hwc_wq->lock);
err = mana_hwc_alloc_dma_buf(hwc, q_depth, max_msg_size,
&hwc_wq->msg_buf);
@@ -634,7 +740,7 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
return err;
}
-static int mana_hwc_post_tx_wqe(const struct hwc_wq *hwc_txq,
+static int mana_hwc_post_tx_wqe(struct hwc_wq *hwc_txq,
struct hwc_work_request *req,
u32 dest_virt_rq_id, u32 dest_virt_rcq_id,
bool dest_pf)
@@ -673,7 +779,10 @@ static int mana_hwc_post_tx_wqe(const struct hwc_wq *hwc_txq,
req->wqe_req.inline_oob_data = tx_oob;
req->wqe_req.client_data_unit = 0;
+ spin_lock(&hwc_txq->lock);
err = mana_gd_post_and_ring(hwc_txq->gdma_wq, &req->wqe_req, NULL);
+ spin_unlock(&hwc_txq->lock);
+
if (err)
dev_err(dev, "Failed to post WQE on HWC SQ: %d\n", err);
return err;
@@ -684,7 +793,7 @@ static int mana_hwc_init_inflight_msg(struct hw_channel_context *hwc,
{
int err;
- sema_init(&hwc->sema, num_msg);
+ atomic_set(&hwc->abandoned_msgs, 0);
err = mana_gd_alloc_res_map(num_msg, &hwc->inflight_msg_res);
if (err)
@@ -692,43 +801,55 @@ static int mana_hwc_init_inflight_msg(struct hw_channel_context *hwc,
return err;
}
-static int mana_hwc_test_channel(struct hw_channel_context *hwc, u16 q_depth,
- u32 max_req_msg_size, u32 max_resp_msg_size)
+static int mana_hwc_test_channel(struct hw_channel_context *hwc)
{
- struct gdma_context *gc = hwc->gdma_dev->gdma_context;
struct hwc_wq *hwc_rxq = hwc->rxq;
struct hwc_work_request *req;
struct hwc_caller_ctx *ctx;
+ unsigned long flags;
int err;
int i;
/* Post all WQEs on the RQ */
- for (i = 0; i < q_depth; i++) {
+ for (i = 0; i < hwc->num_inflight_msg; i++) {
req = &hwc_rxq->msg_buf->reqs[i];
err = mana_hwc_post_rx_wqe(hwc_rxq, req);
if (err)
return err;
}
- ctx = kzalloc_objs(*ctx, q_depth);
+ ctx = kzalloc_objs(*ctx, hwc->num_inflight_msg);
if (!ctx)
return -ENOMEM;
- for (i = 0; i < q_depth; ++i) {
+ for (i = 0; i < hwc->num_inflight_msg; ++i) {
init_completion(&ctx[i].comp_event);
spin_lock_init(&ctx[i].lock);
}
hwc->caller_ctx = ctx;
- return mana_gd_test_eq(gc, hwc->cq->gdma_eq);
+ /* Setup owns hwc directly; runtime publication follows this test. */
+ spin_lock_irqsave(&hwc->inflight_msg_res.lock, flags);
+ hwc->channel_up = true;
+ spin_unlock_irqrestore(&hwc->inflight_msg_res.lock, flags);
+
+ err = mana_gd_test_hwc_eq(hwc, hwc->cq->gdma_eq);
+ if (err) {
+ spin_lock_irqsave(&hwc->inflight_msg_res.lock, flags);
+ hwc->channel_up = false;
+ spin_unlock_irqrestore(&hwc->inflight_msg_res.lock, flags);
+ }
+
+ return err;
}
-static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
+static int mana_hwc_establish_channel(struct hw_channel_context *hwc,
+ u16 *q_depth,
u32 *max_req_msg_size,
u32 *max_resp_msg_size)
{
- struct hw_channel_context *hwc = gc->hwc.driver_data;
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
struct gdma_queue *rq = hwc->rxq->gdma_wq;
struct gdma_queue *sq = hwc->txq->gdma_wq;
struct gdma_queue *eq = hwc->cq->gdma_eq;
@@ -855,68 +976,103 @@ static void mana_hwc_clear_cq_table(struct gdma_context *gc)
vfree(cq_table);
}
-int mana_hwc_create_channel(struct gdma_context *gc)
+static int mana_hwc_publish_channel(struct hw_channel_context *hwc)
+{
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+ unsigned long flags;
+ int err = 0;
+
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ if (WARN_ON_ONCE(gc->hwc.driver_data))
+ err = -EBUSY;
+ else
+ gc->hwc.driver_data = hwc;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+ return err;
+}
+
+static struct hw_channel_context *
+mana_hwc_unpublish_channel(struct gdma_context *gc)
{
- u32 max_req_msg_size, max_resp_msg_size;
- struct gdma_dev *gd = &gc->hwc;
struct hw_channel_context *hwc;
- u16 q_depth_max;
- int err;
+ unsigned long flags;
- /* Retry a retained context before assigning queues to the PF again. */
- if (gd->driver_data) {
- mana_hwc_destroy_channel(gc);
- if (gd->driver_data)
- return -ETIMEDOUT;
- }
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ hwc = gc->hwc.driver_data;
+ gc->hwc.driver_data = NULL;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
- hwc = kzalloc_obj(*hwc);
- if (!hwc)
- return -ENOMEM;
+ return hwc;
+}
- gd->gdma_context = gc;
- gd->driver_data = hwc;
- hwc->gdma_dev = gd;
- hwc->dev = gc->dev;
- hwc->hwc_timeout = HW_CHANNEL_WAIT_RESOURCE_TIMEOUT_MS;
+static void mana_hwc_retain_channel(struct hw_channel_context *hwc)
+{
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+ unsigned long flags;
- /* HWC's instance number is always 0. */
- gd->dev_id.as_uint32 = 0;
- gd->dev_id.type = GDMA_DEVICE_HWC;
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ if (WARN_ON_ONCE(gc->hwc.driver_data))
+ dev_err(hwc->dev, "HWC retention slot is already occupied\n");
+ else
+ gc->hwc.driver_data = hwc;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
+}
- gd->pdid = INVALID_PDID;
- gd->doorbell = INVALID_DOORBELL;
+static bool mana_hwc_senders_drained(struct gdma_context *gc,
+ struct hw_channel_context *hwc)
+{
+ unsigned long flags;
+ bool drained;
- /* mana_hwc_init_queues() only creates the required data structures,
- * and doesn't touch the HWC device.
- */
- err = mana_hwc_init_queues(hwc, HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
- HW_CHANNEL_MAX_REQUEST_SIZE,
- HW_CHANNEL_MAX_RESPONSE_SIZE);
- if (err) {
- dev_err(hwc->dev, "Failed to initialize HWC: %d\n", err);
- goto out;
- }
+ spin_lock_irqsave(&gc->hwc_lock, flags);
+ drained = hwc->active_senders == 0;
+ spin_unlock_irqrestore(&gc->hwc_lock, flags);
- err = mana_hwc_establish_channel(gc, &q_depth_max, &max_req_msg_size,
- &max_resp_msg_size);
- if (err) {
- dev_err(hwc->dev, "Failed to establish HWC: %d\n", err);
- goto out;
+ return drained;
+}
+
+static void mana_hwc_stop_channel(struct hw_channel_context *hwc)
+{
+ struct gdma_resource *r = &hwc->inflight_msg_res;
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+ unsigned long flags;
+ int i;
+
+ if (hwc->num_inflight_msg) {
+ spin_lock_irqsave(&r->lock, flags);
+ hwc->channel_up = false;
+ spin_unlock_irqrestore(&r->lock, flags);
}
+ mana_hwc_timeout_cancel(hwc);
- err = mana_hwc_test_channel(gc->hwc.driver_data,
- HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
- max_req_msg_size, max_resp_msg_size);
- if (err) {
- dev_err(hwc->dev, "Failed to test HWC: %d\n", err);
- goto out;
+ for (i = 0; hwc->caller_ctx && i < hwc->num_inflight_msg; i++) {
+ struct hwc_caller_ctx *ctx;
+ bool drop_resp_ref;
+
+ spin_lock_irqsave(&r->lock, flags);
+ if (!test_bit(i, r->map)) {
+ spin_unlock_irqrestore(&r->lock, flags);
+ continue;
+ }
+
+ ctx = &hwc->caller_ctx[i];
+ spin_lock(&ctx->lock);
+ spin_unlock(&r->lock);
+
+ if (!ctx->responded)
+ ctx->error = -ENODEV;
+ ctx->output_buf = NULL;
+ drop_resp_ref = mana_hwc_clear_resp_pending(hwc, ctx);
+ ctx->responded = true;
+ complete(&ctx->comp_event);
+ spin_unlock_irqrestore(&ctx->lock, flags);
+
+ if (drop_resp_ref)
+ hwc_ctx_put(hwc, ctx);
}
- return 0;
-out:
- mana_hwc_destroy_channel(gc);
- return err;
+ wait_event(gc->hwc_drain_waitq, mana_hwc_senders_drained(gc, hwc));
}
static void mana_hwc_fence_channel(struct gdma_context *gc,
@@ -932,13 +1088,12 @@ static void mana_hwc_fence_channel(struct gdma_context *gc,
mana_hwc_unpublish_cq(gc, hwc->cq->gdma_cq);
}
-void mana_hwc_destroy_channel(struct gdma_context *gc)
+static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
{
- struct hw_channel_context *hwc = gc->hwc.driver_data;
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
int err;
- if (!hwc)
- return;
+ mana_hwc_stop_channel(hwc);
/* An unacknowledged destroy leaves the PF's mappings live. Fence
* software dispatch, but retain every PF-visible allocation.
@@ -950,7 +1105,8 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
"HWC teardown failed: %d, retaining PF-visible resources\n",
err);
mana_hwc_fence_channel(gc, hwc);
- return;
+ mana_hwc_retain_channel(hwc);
+ return false;
}
mana_hwc_clear_cq_table(gc);
@@ -975,11 +1131,10 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
hwc->gdma_dev->doorbell = INVALID_DOORBELL;
hwc->gdma_dev->pdid = INVALID_PDID;
- hwc->hwc_timeout = 0;
-
kfree(hwc);
- gc->hwc.driver_data = NULL;
gc->hwc.gdma_context = NULL;
+
+ return true;
}
static void mana_hwc_free_retained_wq(struct hwc_wq *wq)
@@ -997,12 +1152,12 @@ static void mana_hwc_free_retained_wq(struct hwc_wq *wq)
*/
void mana_hwc_free_retained_channel(struct gdma_context *gc)
{
- struct hw_channel_context *hwc = gc->hwc.driver_data;
+ struct hw_channel_context *hwc = mana_hwc_unpublish_channel(gc);
if (!hwc)
return;
- gc->hwc.driver_data = NULL;
+ mana_hwc_stop_channel(hwc);
mana_hwc_clear_cq_table(gc);
if (hwc->cq) {
@@ -1020,11 +1175,146 @@ void mana_hwc_free_retained_channel(struct gdma_context *gc)
gc->hwc.gdma_context = NULL;
}
-static int mana_hwc_response_status(struct hw_channel_context *hwc,
- u32 command, int error, u32 status)
+static int mana_hwc_create_bootstrap_channel(struct hw_channel_context *hwc)
{
- if (error)
- return error;
+ u32 max_req_msg_size, max_resp_msg_size;
+ u16 q_depth_max;
+ int err;
+
+ err = mana_hwc_init_queues(hwc, HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+ HW_CHANNEL_MAX_REQUEST_SIZE,
+ HW_CHANNEL_MAX_RESPONSE_SIZE);
+ if (err) {
+ dev_err(hwc->dev, "Failed to initialize HWC: %d\n", err);
+ return err;
+ }
+
+ err = mana_hwc_establish_channel(hwc, &q_depth_max, &max_req_msg_size,
+ &max_resp_msg_size);
+ if (err) {
+ dev_err(hwc->dev, "Failed to establish HWC: %d\n", err);
+ return err;
+ }
+
+ err = mana_hwc_test_channel(hwc);
+ if (err)
+ dev_err(hwc->dev, "Failed to test HWC: %d\n", err);
+
+ return err;
+}
+
+int mana_hwc_create_channel(struct gdma_context *gc)
+{
+ struct gdma_dev *gd = &gc->hwc;
+ struct hw_channel_context *hwc;
+ int err;
+
+ /* Retry a retained context before assigning queues to the PF again. */
+ if (gd->driver_data) {
+ mana_hwc_destroy_channel(gc);
+ if (gd->driver_data)
+ return -ETIMEDOUT;
+ }
+
+ hwc = kzalloc_obj(*hwc);
+ if (!hwc)
+ return -ENOMEM;
+
+ gd->gdma_context = gc;
+ hwc->gdma_dev = gd;
+ hwc->dev = gc->dev;
+ WRITE_ONCE(hwc->hwc_timeout, HW_CHANNEL_WAIT_RESOURCE_TIMEOUT_MS);
+ init_waitqueue_head(&hwc->admission_waitq);
+ init_waitqueue_head(&gc->hwc_drain_waitq);
+
+ /* HWC's instance number is always 0. */
+ gd->dev_id.as_uint32 = 0;
+ gd->dev_id.type = GDMA_DEVICE_HWC;
+
+ gd->pdid = INVALID_PDID;
+ gd->doorbell = INVALID_DOORBELL;
+
+ err = mana_hwc_create_bootstrap_channel(hwc);
+ if (err) {
+ mana_hwc_release_channel(hwc);
+ return err;
+ }
+
+ err = mana_hwc_publish_channel(hwc);
+ if (err) {
+ mana_hwc_release_channel(hwc);
+ return err;
+ }
+
+ return 0;
+}
+
+void mana_hwc_destroy_channel(struct gdma_context *gc)
+{
+ struct hw_channel_context *hwc;
+
+ hwc = mana_hwc_unpublish_channel(gc);
+ if (!hwc)
+ return;
+
+ mana_hwc_release_channel(hwc);
+}
+
+static void mana_hwc_abort_request(struct hw_channel_context *hwc,
+ struct hwc_caller_ctx *ctx)
+{
+ unsigned long flags;
+ bool drop_resp_ref;
+
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->output_buf = NULL;
+ drop_resp_ref = mana_hwc_clear_resp_pending(hwc, ctx);
+ ctx->responded = true;
+ spin_unlock_irqrestore(&ctx->lock, flags);
+
+ if (drop_resp_ref)
+ hwc_ctx_put(hwc, ctx);
+ hwc_ctx_put(hwc, ctx);
+}
+
+static int mana_hwc_submit_request(struct hw_channel_context *hwc,
+ struct hwc_caller_ctx *ctx,
+ struct hwc_work_request *tx_wr)
+{
+ unsigned long flags;
+ bool drop_resp_ref = false;
+ int err;
+
+ spin_lock_irqsave(&ctx->lock, flags);
+ if (ctx->responded) {
+ err = ctx->error ?: -ENODEV;
+ } else {
+ err = mana_hwc_post_tx_wqe(hwc->txq, tx_wr,
+ hwc->dest_vrq_id,
+ hwc->dest_vrcq_id, false);
+ if (err) {
+ ctx->output_buf = NULL;
+ drop_resp_ref = mana_hwc_clear_resp_pending(hwc, ctx);
+ ctx->responded = true;
+ }
+ }
+ spin_unlock_irqrestore(&ctx->lock, flags);
+
+ if (!err)
+ return 0;
+
+ if (drop_resp_ref)
+ hwc_ctx_put(hwc, ctx);
+ hwc_ctx_put(hwc, ctx);
+
+ return err;
+}
+
+static int mana_hwc_response_result(struct hw_channel_context *hwc,
+ u32 command, int err, u32 status)
+{
+ if (err)
+ return err;
if (!status || status == GDMA_STATUS_MORE_ENTRIES)
return 0;
@@ -1039,108 +1329,90 @@ static int mana_hwc_response_status(struct hw_channel_context *hwc,
return -EPROTO;
}
-static void mana_hwc_finish_request(struct hw_channel_context *hwc,
- struct hwc_caller_ctx *ctx, u16 msg_id)
+static int mana_hwc_wait_for_response(struct hw_channel_context *hwc,
+ struct hwc_caller_ctx *ctx,
+ u32 command)
{
unsigned long flags;
+ bool abandoned = false;
+ u32 wait_ms;
+ u32 status;
+ int err;
+
+ wait_ms = mana_hwc_timeout_read(hwc);
+ if (wait_for_completion_timeout(&ctx->comp_event,
+ msecs_to_jiffies(wait_ms))) {
+ spin_lock_irqsave(&ctx->lock, flags);
+ ctx->output_buf = NULL;
+ err = ctx->error;
+ status = ctx->status_code;
+ spin_unlock_irqrestore(&ctx->lock, flags);
+ hwc_ctx_put(hwc, ctx);
+
+ return mana_hwc_response_result(hwc, command, err, status);
+ }
spin_lock_irqsave(&ctx->lock, flags);
ctx->output_buf = NULL;
+ err = ctx->error;
+ status = ctx->status_code;
+ if (err == -EINPROGRESS) {
+ ctx->error = -ETIMEDOUT;
+ ctx->responded = true;
+ atomic_inc(&hwc->abandoned_msgs);
+ abandoned = true;
+ }
spin_unlock_irqrestore(&ctx->lock, flags);
- mana_hwc_put_msg_index(hwc, msg_id);
+ if (!abandoned) {
+ hwc_ctx_put(hwc, ctx);
+ return mana_hwc_response_result(hwc, command, err, status);
+ }
+
+ wake_up_all(&hwc->admission_waitq);
+
+ if (wait_ms)
+ dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
+ command, wait_ms);
+
+ mana_hwc_timeout_reduce(hwc);
+ hwc_ctx_put(hwc, ctx);
+
+ return -ETIMEDOUT;
}
int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
const void *req, u32 resp_len, void *resp)
{
struct hwc_work_request *tx_wr;
- struct hwc_wq *txq = hwc->txq;
struct gdma_req_hdr *req_msg;
struct hwc_caller_ctx *ctx;
- unsigned long flags;
- u32 dest_vrcq;
- u32 dest_vrq;
u32 command;
- u32 status;
- u16 msg_id;
int err;
- err = mana_hwc_get_msg_index(hwc, &msg_id);
+ err = mana_hwc_get_msg_index(hwc, resp, resp_len, &ctx);
if (err)
return err;
- tx_wr = &txq->msg_buf->reqs[msg_id];
- ctx = hwc->caller_ctx + msg_id;
-
+ tx_wr = &hwc->txq->msg_buf->reqs[ctx->msg_id];
if (req_len > tx_wr->buf_len) {
dev_err(hwc->dev, "HWC: req msg size: %d > %d\n", req_len,
tx_wr->buf_len);
- mana_hwc_finish_request(hwc, ctx, msg_id);
+ mana_hwc_abort_request(hwc, ctx);
return -EINVAL;
}
- spin_lock_irqsave(&ctx->lock, flags);
- ctx->output_buf = resp;
- ctx->output_buflen = resp_len;
- spin_unlock_irqrestore(&ctx->lock, flags);
-
- req_msg = (struct gdma_req_hdr *)tx_wr->buf_va;
+ req_msg = tx_wr->buf_va;
if (req)
memcpy(req_msg, req, req_len);
-
- req_msg->req.hwc_msg_id = msg_id;
+ req_msg->req.hwc_msg_id = ctx->msg_id;
tx_wr->msg_size = req_len;
command = req_msg->req.msg_type;
- /* The hardware reports the HWC destination queues through
- * HWC_INIT_DATA_DEST_RQ_ID and HWC_INIT_DATA_DEST_CQ_ID, and
- * always supplies values that are valid for this function, so no
- * PF-specific handling is needed here.
- */
- dest_vrq = hwc->dest_vrq_id;
- dest_vrcq = hwc->dest_vrcq_id;
-
- err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);
- if (err) {
- dev_err(hwc->dev, "HWC: Failed to post send WQE: %d\n", err);
- mana_hwc_finish_request(hwc, ctx, msg_id);
+ err = mana_hwc_submit_request(hwc, ctx, tx_wr);
+ if (err)
return err;
- }
-
- if (!wait_for_completion_timeout(&ctx->comp_event,
- msecs_to_jiffies(hwc->hwc_timeout))) {
- spin_lock_irqsave(&ctx->lock, flags);
- ctx->output_buf = NULL;
- err = ctx->error;
- status = ctx->status_code;
- spin_unlock_irqrestore(&ctx->lock, flags);
-
- if (err != -EINPROGRESS) {
- mana_hwc_finish_request(hwc, ctx, msg_id);
- return mana_hwc_response_status(hwc, command, err,
- status);
- }
-
- if (hwc->hwc_timeout != 0)
- dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
- command, hwc->hwc_timeout);
-
- /* Reduce further waiting if HWC no response */
- if (hwc->hwc_timeout > 1)
- hwc->hwc_timeout = 1;
-
- mana_hwc_finish_request(hwc, ctx, msg_id);
- return -ETIMEDOUT;
- }
-
- spin_lock_irqsave(&ctx->lock, flags);
- ctx->output_buf = NULL;
- err = ctx->error;
- status = ctx->status_code;
- spin_unlock_irqrestore(&ctx->lock, flags);
- mana_hwc_finish_request(hwc, ctx, msg_id);
- return mana_hwc_response_status(hwc, command, err, status);
+ return mana_hwc_wait_for_response(hwc, ctx, command);
}
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index af62cd6d771e..47255e662fc3 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -544,6 +544,15 @@ struct gdma_context {
/* Hardware communication channel (HWC) */
struct gdma_dev hwc;
+ /* Sender drain; the final wakeup runs under hwc_lock. */
+ wait_queue_head_t hwc_drain_waitq;
+
+ /* Protects runtime HWC publication and active sender references.
+ * Setup owns an unpublished HWC directly; timeout updates use atomic
+ * access helpers and do not require this lock.
+ */
+ spinlock_t hwc_lock;
+
/* Azure network adapter */
struct gdma_dev mana;
@@ -1192,6 +1201,8 @@ void mana_gd_free_memory(struct gdma_mem_info *gmi);
int mana_gd_send_request(struct gdma_context *gc, u32 req_len, const void *req,
u32 resp_len, void *resp);
+int mana_gd_cancel_hwc_timeout(struct gdma_context *gc);
+
int mana_gd_destroy_dma_region(struct gdma_context *gc, u64 dma_region_handle);
void mana_register_debugfs(void);
void mana_unregister_debugfs(void);
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index c7570a5f7376..8f2ac3d37770 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -4,6 +4,8 @@
#ifndef _HW_CHANNEL_H
#define _HW_CHANNEL_H
+#include <linux/atomic.h>
+
#define DEFAULT_LOG2_THROTTLING_FOR_ERROR_EQ 4
#define HW_CHANNEL_MAX_REQUEST_SIZE 0x1000
@@ -164,17 +166,32 @@ struct hwc_wq {
u16 queue_depth;
struct hwc_cq *hwc_cq;
+
+ /* Serializes SQ posting; unused for the RQ. */
+ spinlock_t lock;
};
struct hwc_caller_ctx {
struct completion comp_event;
- /* Protects initialization, output storage and response state. */
+
+ /* Initialize under inflight_msg_res.lock and lock, before publication.
+ * Once its bitmap bit is set, lock protects every field below except
+ * msg_id and refcnt. The sender owns output_buf; the response handler
+ * may write it only while holding lock.
+ */
spinlock_t lock;
void *output_buf;
u32 output_buflen;
-
- int error; /* Linux error code */
+ int error;
u32 status_code;
+ bool responded;
+ bool resp_pending;
+
+ /* Tracks sender + response-handler ownership. The last put releases
+ * the bitmap slot under inflight_msg_res.lock.
+ */
+ refcount_t refcnt;
+ u16 msg_id;
};
struct hw_channel_context {
@@ -196,15 +213,30 @@ struct hw_channel_context {
struct hwc_wq *txq;
struct hwc_cq *cq;
- struct semaphore sema;
+ /* Wait for a slot, a response timeout, cancellation or channel stop. */
+ wait_queue_head_t admission_waitq;
struct gdma_resource inflight_msg_res;
+ /* Response references still outstanding after a sender timed out. */
+ atomic_t abandoned_msgs;
u32 dest_vrq_id;
u32 dest_vrcq_id;
+
+ /* Zero permanently cancels waits for this channel instance. Firmware
+ * and query updates may replace a live nonzero value; fail-fast may
+ * only reduce a live value to one millisecond.
+ */
u32 hwc_timeout;
+ /* Checked during admission; cleared on teardown to reject sends. */
+ bool channel_up;
+
/* The PF may own the HWC queues while this is true. */
bool setup_active;
+
+ /* mana_gd_send_request() callers, including waiters; under hwc_lock. */
+ unsigned int active_senders;
+
struct hwc_caller_ctx *caller_ctx;
};
@@ -214,5 +246,11 @@ void mana_hwc_free_retained_channel(struct gdma_context *gc);
int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
const void *req, u32 resp_len, void *resp);
+int mana_gd_test_hwc_eq(struct hw_channel_context *hwc,
+ struct gdma_queue *eq);
+
+u32 mana_hwc_timeout_read(const struct hw_channel_context *hwc);
+void mana_hwc_timeout_update(struct hw_channel_context *hwc, u32 timeout_ms);
+void mana_hwc_timeout_cancel(struct hw_channel_context *hwc);
#endif /* _HW_CHANNEL_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path
2026-11-09 15:44 [PATCH net-next v10 0/4] net: mana: concurrent HWC requests and dynamic queue depth Wei Hu
` (2 preceding siblings ...)
2026-11-09 15:45 ` [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests Wei Hu
@ 2026-11-09 15:45 ` Wei Hu
2026-11-10 15:45 ` sashiko-bot
2026-11-13 6:46 ` netdev-bot+sashiko
3 siblings, 2 replies; 10+ messages in thread
From: Wei Hu @ 2026-11-09 15:45 UTC (permalink / raw)
To: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel, gargaditya,
kees+treewide, kees, mawasthi, weh, edumazet
From: Long Li <longli@microsoft.com>
The HWC starts with depth-one bootstrap queues. Treat the PF-reported depth
as a maximum and rebuild the channel exactly for reports from 2 through
128. Keep the untouched bootstrap queues for depth one and reports above
128, and require a rebuilt channel to repeat the selected depth.
Treat the reported request and response sizes as directional maxima.
Rebuild only when both reports equal the 4096-byte bootstrap allocation.
Keep allocation strides separate from these limits, and share the effective
host-to-PF request budget with the GDMA and RDMA request builders. Bound
RDMA CREATE and ADD_PAGES batches by that budget, checking that the headers
and at least one page address fit before subtracting their sizes.
Reject oversized messages before acquiring a slot or publishing caller
storage. A retained, stopped channel still returns -ENODEV rather than
reporting a size error against uninitialized negotiated limits.
Keep each candidate private through queue creation, establishment and the
direct EQ test. Publish only the final tested channel. If a rebuild fails,
including its functional test, tear it down and restore fresh, tested
bootstrap queues. Stop if teardown is not acknowledged; never reuse
PF-owned mappings. Restore the previous live timeout budget before the
fallback so a failed test's fail-fast reduction does not poison the retry,
without reviving terminal cancellation.
Invalidate the previous HWC doorbell before every ESTABLISH_HWC. Validate
the BAR0 page geometry and reported index before accepting a new doorbell,
and suppress MMIO while it is invalid. This also hardens initial setup,
which previously accepted an unchecked device-supplied index. A malformed
doorbell completes initialization with -EPROTO without relying on later
INIT_DONE events that might require re-arming through that doorbell.
Use the initial PF report to select the queue geometry. Driver-version
advertisement occurs after HWC establishment and cannot negotiate behavior
needed while the channel is being created.
Link: https://lore.kernel.org/r/20260908035201.402424-5-longli@microsoft.com
Link: https://lore.kernel.org/r/179196575846.434549.7933818740976196422@kernel.org
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Wei Hu <weh@microsoft.com>
Copilot-Session: 8c2361e6-c0f4-4b0c-bf01-b5e4cdacec50
---
drivers/infiniband/hw/mana/main.c | 19 +-
.../net/ethernet/microsoft/mana/gdma_main.c | 26 +-
.../net/ethernet/microsoft/mana/hw_channel.c | 316 +++++++++++++++---
include/net/mana/gdma.h | 2 +
include/net/mana/hw_channel.h | 19 +-
5 files changed, 317 insertions(+), 65 deletions(-)
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index 858bbc325eb2..e2ddbcb1f646 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -414,28 +414,38 @@ static int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem
unsigned int create_req_msg_size;
struct hw_channel_context *hwc;
struct ib_block_iter biter;
- size_t max_pgs_add_cmd = 0;
+ size_t max_pgs_add_cmd;
size_t max_pgs_create_cmd;
struct gdma_context *gc;
size_t num_pages_total;
unsigned int tail = 0;
+ u32 req_limit;
u64 *page_addr_list;
void *request_buf;
int err = 0;
gc = mdev_to_gc(dev);
hwc = gc->hwc.driver_data;
+ req_limit = mana_hwc_get_max_req_size(hwc);
+ if (req_limit < struct_size(create_req, page_addr_list, 1) ||
+ req_limit < struct_size(add_req, page_addr_list, 1)) {
+ ibdev_dbg(&dev->ib_dev,
+ "HWC request budget %u is too small for DMA region commands\n",
+ req_limit);
+ return -EMSGSIZE;
+ }
num_pages_total = ib_umem_num_dma_blocks(umem, page_sz);
max_pgs_create_cmd =
- (hwc->max_req_msg_size - sizeof(*create_req)) / sizeof(u64);
+ (req_limit - sizeof(*create_req)) / sizeof(u64);
+ max_pgs_add_cmd = (req_limit - sizeof(*add_req)) / sizeof(u64);
num_pages_to_handle =
min_t(size_t, num_pages_total, max_pgs_create_cmd);
create_req_msg_size =
struct_size(create_req, page_addr_list, num_pages_to_handle);
- request_buf = kzalloc(hwc->max_req_msg_size, GFP_KERNEL);
+ request_buf = kzalloc(req_limit, GFP_KERNEL);
if (!request_buf)
return -ENOMEM;
@@ -478,9 +488,6 @@ static int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem
if (err)
goto out;
- max_pgs_add_cmd = (hwc->max_req_msg_size -
- sizeof(*add_req)) / sizeof(u64);
-
add_req = request_buf;
add_req->dma_region_handle = *gdma_region;
add_req->reserved3 = 0;
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index bb4a963c2cf5..dace16eaeaa5 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -589,12 +589,26 @@ static int mana_gd_disable_queue(struct gdma_queue *queue)
#define DOORBELL_OFFSET_EQ 0xFF8
#define DOORBELL_OFFSET_DIM 0x820
+bool mana_gd_is_valid_doorbell(struct gdma_context *gc, u32 db_id)
+{
+ if (!gc->db_page_size || gc->db_page_off >= gc->bar0_size)
+ return false;
+
+ return gc->db_page_off +
+ gc->db_page_size * ((u64)db_id + 1) <= gc->bar0_size;
+}
+
static void mana_gd_ring_doorbell(struct gdma_context *gc, u32 db_index,
enum gdma_queue_type q_type, u32 qid,
u32 tail_ptr, u8 num_req, u8 client_offset)
{
- void __iomem *addr = gc->db_page_base + gc->db_page_size * db_index;
union gdma_doorbell_entry e = {};
+ void __iomem *addr;
+
+ if (unlikely(db_index == INVALID_DOORBELL))
+ return;
+
+ addr = gc->db_page_base + gc->db_page_size * db_index;
addr += client_offset;
@@ -1342,7 +1356,7 @@ static int mana_gd_create_dma_region(struct gdma_dev *gd,
/* The caller must keep the HWC alive throughout queue creation. */
hwc = gc->hwc.driver_data;
req_msg_size = struct_size(req, page_addr_list, num_page);
- if (req_msg_size > hwc->max_req_msg_size)
+ if (req_msg_size > mana_hwc_get_max_req_size(hwc))
return -EINVAL;
req = kzalloc(req_msg_size, GFP_KERNEL);
@@ -1622,13 +1636,7 @@ int mana_gd_register_device(struct gdma_dev *gd)
return err ? err : -EPROTO;
}
- /* Validate that doorbell page for db_id is within the BAR0 region.
- * In mana_gd_ring_doorbell(), the address is calculated as:
- * addr = db_page_base + db_page_size * db_id
- * = (bar0_va + db_page_off) + (db_page_size * db_id)
- * So we need: db_page_off + db_page_size * (db_id + 1) <= bar0_size
- */
- if (gc->db_page_off + gc->db_page_size * ((u64)resp.db_id + 1) > gc->bar0_size) {
+ if (!mana_gd_is_valid_doorbell(gc, resp.db_id)) {
dev_err(gc->dev, "Doorbell ID %u out of range\n", resp.db_id);
return -EPROTO;
}
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 8bc00001d20e..867f8dd3ea54 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -95,12 +95,30 @@ static int mana_hwc_try_get_msg_index(struct hw_channel_context *hwc,
return 0;
}
-static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, void *resp,
- u32 resp_len,
+static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u32 req_len,
+ void *resp, u32 resp_len,
struct hwc_caller_ctx **caller_ctx)
{
+ struct gdma_resource *r = &hwc->inflight_msg_res;
+ unsigned long flags;
+ u32 req_limit;
+ u32 resp_limit;
int err;
+ req_limit = mana_hwc_get_max_req_size(hwc);
+ resp_limit = min(hwc->rx_msg_size_limit, hwc->max_req_msg_size);
+ if (req_len > req_limit || resp_len > resp_limit) {
+ spin_lock_irqsave(&r->lock, flags);
+ err = hwc->channel_up ? -EMSGSIZE : -ENODEV;
+ spin_unlock_irqrestore(&r->lock, flags);
+
+ if (err == -EMSGSIZE)
+ dev_err(hwc->dev,
+ "HWC: message sizes req=%u/%u resp=%u/%u exceed channel maxima\n",
+ req_len, req_limit, resp_len, resp_limit);
+ return err;
+ }
+
/* Healthy contention must not skip a destructive control request. */
wait_event(hwc->admission_waitq,
(err = mana_hwc_try_get_msg_index(hwc, resp, resp_len,
@@ -246,8 +264,18 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
switch (event->type) {
case GDMA_EQE_HWC_INIT_EQ_ID_DB:
eq_db.as_uint32 = event->details[0];
+ if (!mana_gd_is_valid_doorbell(gd->gdma_context,
+ eq_db.doorbell)) {
+ dev_err(hwc->dev, "HWC: invalid doorbell %u\n",
+ eq_db.doorbell);
+ WRITE_ONCE(hwc->hwc_init_error, -EPROTO);
+ complete(&hwc->hwc_init_eqe_comp);
+ break;
+ }
+
hwc->cq->gdma_eq->id = eq_db.eq_id;
gd->doorbell = eq_db.doorbell;
+ hwc->hwc_init_doorbell = true;
break;
case GDMA_EQE_HWC_INIT_DATA:
@@ -269,7 +297,8 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
break;
case HWC_INIT_DATA_QUEUE_DEPTH:
- hwc->hwc_init_q_depth_max = (u16)val;
+ /* Preserve the full 24-bit report for validation. */
+ hwc->hwc_init_q_depth_max = val;
break;
case HWC_INIT_DATA_MAX_REQUEST:
@@ -638,7 +667,8 @@ static int mana_hwc_alloc_dma_buf(struct hw_channel_context *hwc, u16 q_depth,
dma_buf->num_reqs = q_depth;
- buf_size = MANA_PAGE_ALIGN(q_depth * max_msg_size);
+ /* mana_gd_alloc_memory() requires a power-of-two length. */
+ buf_size = roundup_pow_of_two(MANA_PAGE_ALIGN(q_depth * max_msg_size));
gmi = &dma_buf->mem_info;
err = mana_gd_alloc_memory(gc, buf_size, gmi, false);
@@ -844,10 +874,15 @@ static int mana_hwc_test_channel(struct hw_channel_context *hwc)
return err;
}
-static int mana_hwc_establish_channel(struct hw_channel_context *hwc,
- u16 *q_depth,
- u32 *max_req_msg_size,
- u32 *max_resp_msg_size)
+struct mana_hwc_init_report {
+ u32 queue_depth;
+ u32 max_req_msg_size;
+ u32 max_resp_msg_size;
+};
+
+static int
+mana_hwc_establish_channel(struct hw_channel_context *hwc,
+ struct mana_hwc_init_report *report)
{
struct gdma_context *gc = hwc->gdma_dev->gdma_context;
struct gdma_queue *rq = hwc->rxq->gdma_wq;
@@ -859,6 +894,19 @@ static int mana_hwc_establish_channel(struct hw_channel_context *hwc,
u32 cq_id;
int err;
+ hwc->hwc_init_q_depth_max = 0;
+ hwc->hwc_init_max_req_msg_size = 0;
+ hwc->hwc_init_max_resp_msg_size = 0;
+ hwc->hwc_init_max_num_cqs = 0;
+ hwc->hwc_init_cq_id = 0;
+ hwc->hwc_init_doorbell = false;
+ WRITE_ONCE(hwc->hwc_init_error, 0);
+ gc->hwc.pdid = INVALID_PDID;
+ /* Do not rearm through the previous channel's doorbell. */
+ gc->hwc.doorbell = INVALID_DOORBELL;
+ hwc->dest_vrq_id = 0;
+ hwc->dest_vrcq_id = 0;
+
init_completion(&hwc->hwc_init_eqe_comp);
err = mana_smc_setup_hwc(&gc->shm_channel, false,
@@ -873,9 +921,18 @@ static int mana_hwc_establish_channel(struct hw_channel_context *hwc,
if (!wait_for_completion_timeout(&hwc->hwc_init_eqe_comp, 60 * HZ))
return -ETIMEDOUT;
- *q_depth = hwc->hwc_init_q_depth_max;
- *max_req_msg_size = hwc->hwc_init_max_req_msg_size;
- *max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
+ err = READ_ONCE(hwc->hwc_init_error);
+ if (err)
+ return err;
+
+ if (!hwc->hwc_init_doorbell) {
+ dev_err(hwc->dev, "HWC: missing valid doorbell in init data\n");
+ return -EPROTO;
+ }
+
+ report->queue_depth = hwc->hwc_init_q_depth_max;
+ report->max_req_msg_size = hwc->hwc_init_max_req_msg_size;
+ report->max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
/* Snapshot the device-reported count and id once, so the same value
* sizes, bounds and indexes cq_table even across the sleeping
@@ -925,6 +982,9 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
{
int err;
+ if (q_depth > U16_MAX / 2)
+ return -EINVAL;
+
err = mana_hwc_init_inflight_msg(hwc, q_depth);
if (err)
return err;
@@ -957,10 +1017,11 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
hwc->num_inflight_msg = q_depth;
hwc->max_req_msg_size = max_req_msg_size;
+ hwc->max_resp_msg_size = max_resp_msg_size;
return 0;
out:
- /* mana_hwc_create_channel() will do the cleanup.*/
+ /* The setup path tears down any partially initialized queues. */
return err;
}
@@ -976,6 +1037,33 @@ static void mana_hwc_clear_cq_table(struct gdma_context *gc)
vfree(cq_table);
}
+/* Setup is private; runtime teardown has unpublished and drained senders. */
+static void mana_hwc_destroy_queues(struct hw_channel_context *hwc)
+{
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+
+ mana_hwc_clear_cq_table(gc);
+
+ if (hwc->cq) {
+ mana_hwc_destroy_cq(gc, hwc->cq);
+ hwc->cq = NULL;
+ }
+
+ if (hwc->txq) {
+ mana_hwc_destroy_wq(hwc, hwc->txq);
+ hwc->txq = NULL;
+ }
+ if (hwc->rxq) {
+ mana_hwc_destroy_wq(hwc, hwc->rxq);
+ hwc->rxq = NULL;
+ }
+
+ kfree(hwc->caller_ctx);
+ hwc->caller_ctx = NULL;
+ mana_gd_free_res_map(&hwc->inflight_msg_res);
+ hwc->num_inflight_msg = 0;
+}
+
static int mana_hwc_publish_channel(struct hw_channel_context *hwc)
{
struct gdma_context *gc = hwc->gdma_dev->gdma_context;
@@ -1088,16 +1176,11 @@ static void mana_hwc_fence_channel(struct gdma_context *gc,
mana_hwc_unpublish_cq(gc, hwc->cq->gdma_cq);
}
-static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
+static int mana_hwc_teardown_queues(struct hw_channel_context *hwc)
{
struct gdma_context *gc = hwc->gdma_dev->gdma_context;
int err;
- mana_hwc_stop_channel(hwc);
-
- /* An unacknowledged destroy leaves the PF's mappings live. Fence
- * software dispatch, but retain every PF-visible allocation.
- */
err = mana_smc_teardown_hwc(&gc->shm_channel, false,
&hwc->setup_active);
if (err) {
@@ -1105,28 +1188,26 @@ static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
"HWC teardown failed: %d, retaining PF-visible resources\n",
err);
mana_hwc_fence_channel(gc, hwc);
- mana_hwc_retain_channel(hwc);
- return false;
+ return err;
}
- mana_hwc_clear_cq_table(gc);
-
- /* Fence the EQ before releasing the remaining HWC state. */
- if (hwc->cq)
- mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
-
- if (hwc->txq)
- mana_hwc_destroy_wq(hwc, hwc->txq);
+ mana_hwc_destroy_queues(hwc);
- if (hwc->rxq)
- mana_hwc_destroy_wq(hwc, hwc->rxq);
+ return 0;
+}
- kfree(hwc->caller_ctx);
- hwc->caller_ctx = NULL;
+static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
+{
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+ int err;
- mana_gd_free_res_map(&hwc->inflight_msg_res);
+ mana_hwc_stop_channel(hwc);
- hwc->num_inflight_msg = 0;
+ err = mana_hwc_teardown_queues(hwc);
+ if (err) {
+ mana_hwc_retain_channel(hwc);
+ return false;
+ }
hwc->gdma_dev->doorbell = INVALID_DOORBELL;
hwc->gdma_dev->pdid = INVALID_PDID;
@@ -1175,32 +1256,169 @@ void mana_hwc_free_retained_channel(struct gdma_context *gc)
gc->hwc.gdma_context = NULL;
}
-static int mana_hwc_create_bootstrap_channel(struct hw_channel_context *hwc)
+static int
+mana_hwc_validate_report(struct hw_channel_context *hwc,
+ const struct mana_hwc_init_report *report,
+ u32 expected_depth, bool check_depth,
+ bool require_exact_msg_sizes)
+{
+ if (!report->queue_depth || !report->max_req_msg_size ||
+ !report->max_resp_msg_size) {
+ dev_err(hwc->dev,
+ "HWC: invalid maxima depth=%u req=%u resp=%u\n",
+ report->queue_depth, report->max_req_msg_size,
+ report->max_resp_msg_size);
+ return -EPROTO;
+ }
+
+ if (require_exact_msg_sizes &&
+ (report->max_req_msg_size != HW_CHANNEL_MAX_REQUEST_SIZE ||
+ report->max_resp_msg_size != HW_CHANNEL_MAX_RESPONSE_SIZE)) {
+ dev_err(hwc->dev,
+ "HWC: rebuilt message maxima req=%u resp=%u, expected %u/%u\n",
+ report->max_req_msg_size, report->max_resp_msg_size,
+ HW_CHANNEL_MAX_REQUEST_SIZE,
+ HW_CHANNEL_MAX_RESPONSE_SIZE);
+ return -EPROTO;
+ }
+
+ if (check_depth && report->queue_depth != expected_depth) {
+ dev_err(hwc->dev, "HWC: rebuilt depth %u, expected %u\n",
+ report->queue_depth, expected_depth);
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
+static int mana_hwc_finish_channel(struct hw_channel_context *hwc,
+ const struct mana_hwc_init_report *report)
+{
+ int err;
+
+ /* MAX_REQUEST sizes the RQ; MAX_RESPONSE sizes the SQ. */
+ hwc->rx_msg_size_limit = report->max_req_msg_size;
+ hwc->tx_msg_size_limit = report->max_resp_msg_size;
+
+ err = mana_hwc_test_channel(hwc);
+ if (err)
+ dev_err(hwc->dev, "Failed to test HWC: %d\n", err);
+
+ return err;
+}
+
+static int mana_hwc_build_channel(struct hw_channel_context *hwc, u16 q_depth,
+ struct mana_hwc_init_report *report)
{
- u32 max_req_msg_size, max_resp_msg_size;
- u16 q_depth_max;
int err;
- err = mana_hwc_init_queues(hwc, HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+ err = mana_hwc_init_queues(hwc, q_depth,
HW_CHANNEL_MAX_REQUEST_SIZE,
HW_CHANNEL_MAX_RESPONSE_SIZE);
if (err) {
- dev_err(hwc->dev, "Failed to initialize HWC: %d\n", err);
+ dev_err(hwc->dev, "Failed to initialize HWC depth %u: %d\n",
+ q_depth, err);
return err;
}
- err = mana_hwc_establish_channel(hwc, &q_depth_max, &max_req_msg_size,
- &max_resp_msg_size);
- if (err) {
- dev_err(hwc->dev, "Failed to establish HWC: %d\n", err);
+ err = mana_hwc_establish_channel(hwc, report);
+ if (err)
+ dev_err(hwc->dev, "Failed to establish HWC depth %u: %d\n",
+ q_depth, err);
+
+ return err;
+}
+
+static int
+mana_hwc_restore_bootstrap(struct hw_channel_context *hwc,
+ struct mana_hwc_init_report *report)
+{
+ int err;
+
+ err = mana_hwc_build_channel(hwc,
+ HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+ report);
+ if (err)
return err;
- }
- err = mana_hwc_test_channel(hwc);
+ err = mana_hwc_validate_report(hwc, report, 0, false, false);
if (err)
- dev_err(hwc->dev, "Failed to test HWC: %d\n", err);
+ return err;
- return err;
+ return mana_hwc_finish_channel(hwc, report);
+}
+
+static int mana_hwc_rebuild_channel(struct hw_channel_context *hwc,
+ u16 q_depth,
+ struct mana_hwc_init_report *report)
+{
+ u32 timeout_ms = mana_hwc_timeout_read(hwc);
+ int cleanup_err;
+ int err;
+
+ err = mana_hwc_teardown_queues(hwc);
+ if (err)
+ return err;
+
+ err = mana_hwc_build_channel(hwc, q_depth, report);
+ if (!err)
+ err = mana_hwc_validate_report(hwc, report, q_depth,
+ true, true);
+ if (!err)
+ err = mana_hwc_finish_channel(hwc, report);
+ if (!err)
+ return 0;
+
+ dev_warn(hwc->dev,
+ "HWC depth %u rebuild failed, restoring bootstrap: %d\n",
+ q_depth, err);
+
+ cleanup_err = mana_hwc_teardown_queues(hwc);
+ if (cleanup_err)
+ return cleanup_err;
+
+ /* Do not carry the failed test's reduced wait into the retry. */
+ mana_hwc_timeout_update(hwc, timeout_ms);
+
+ return mana_hwc_restore_bootstrap(hwc, report);
+}
+
+static int mana_hwc_prepare_channel(struct hw_channel_context *hwc)
+{
+ struct mana_hwc_init_report report;
+ u16 rebuild_depth;
+ int err;
+
+ err = mana_hwc_build_channel(hwc,
+ HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+ &report);
+ if (err)
+ return err;
+
+ err = mana_hwc_validate_report(hwc, &report, 0, false, false);
+ if (err)
+ return err;
+
+ if (report.queue_depth > HW_CHANNEL_MAX_QUEUE_DEPTH) {
+ dev_warn(hwc->dev,
+ "HWC depth %u exceeds limit %u, keeping bootstrap\n",
+ report.queue_depth, HW_CHANNEL_MAX_QUEUE_DEPTH);
+ } else if (report.queue_depth >
+ HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH &&
+ (report.max_req_msg_size != HW_CHANNEL_MAX_REQUEST_SIZE ||
+ report.max_resp_msg_size != HW_CHANNEL_MAX_RESPONSE_SIZE)) {
+ dev_warn(hwc->dev,
+ "HWC maxima req=%u resp=%u differ from rebuild policy %u/%u, keeping bootstrap\n",
+ report.max_req_msg_size, report.max_resp_msg_size,
+ HW_CHANNEL_MAX_REQUEST_SIZE,
+ HW_CHANNEL_MAX_RESPONSE_SIZE);
+ } else if (report.queue_depth >
+ HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH) {
+ rebuild_depth = report.queue_depth;
+ return mana_hwc_rebuild_channel(hwc, rebuild_depth, &report);
+ }
+
+ return mana_hwc_finish_channel(hwc, &report);
}
int mana_hwc_create_channel(struct gdma_context *gc)
@@ -1234,7 +1452,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
gd->pdid = INVALID_PDID;
gd->doorbell = INVALID_DOORBELL;
- err = mana_hwc_create_bootstrap_channel(hwc);
+ err = mana_hwc_prepare_channel(hwc);
if (err) {
mana_hwc_release_channel(hwc);
return err;
@@ -1390,7 +1608,7 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
u32 command;
int err;
- err = mana_hwc_get_msg_index(hwc, resp, resp_len, &ctx);
+ err = mana_hwc_get_msg_index(hwc, req_len, resp, resp_len, &ctx);
if (err)
return err;
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 47255e662fc3..21fdb0cbcb5a 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -610,6 +610,8 @@ void mana_gd_fence_eq(struct gdma_context *gc, struct gdma_queue *queue);
int mana_gd_poll_cq(struct gdma_queue *cq, struct gdma_comp *comp, int num_cqe);
+bool mana_gd_is_valid_doorbell(struct gdma_context *gc, u32 db_id);
+
void mana_gd_ring_cq(struct gdma_queue *cq, u8 arm_bit);
ssize_t mana_gd_read_ring(struct gdma_queue *q, char __user *buf,
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 8f2ac3d37770..fd6d04e1e5cb 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -5,6 +5,7 @@
#define _HW_CHANNEL_H
#include <linux/atomic.h>
+#include <linux/minmax.h>
#define DEFAULT_LOG2_THROTTLING_FOR_ERROR_EQ 4
@@ -13,6 +14,9 @@
#define HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH 1
+/* Largest supported rebuild depth; larger reports retain bootstrap queues. */
+#define HW_CHANNEL_MAX_QUEUE_DEPTH 128
+
#define HWC_INIT_DATA_CQID 1
#define HWC_INIT_DATA_RQID 2
#define HWC_INIT_DATA_SQID 3
@@ -200,12 +204,18 @@ struct hw_channel_context {
u16 num_inflight_msg;
u32 max_req_msg_size;
+ u32 max_resp_msg_size;
+ /* Immutable reported maxima for the final RQ and SQ directions. */
+ u32 rx_msg_size_limit;
+ u32 tx_msg_size_limit;
- u16 hwc_init_q_depth_max;
+ u32 hwc_init_q_depth_max;
u32 hwc_init_max_req_msg_size;
u32 hwc_init_max_resp_msg_size;
u32 hwc_init_max_num_cqs;
u32 hwc_init_cq_id;
+ bool hwc_init_doorbell;
+ int hwc_init_error;
struct completion hwc_init_eqe_comp;
@@ -240,6 +250,13 @@ struct hw_channel_context {
struct hwc_caller_ctx *caller_ctx;
};
+/* Host-to-PF requests are bounded by the negotiated TX limit and SQ stride. */
+static inline u32
+mana_hwc_get_max_req_size(const struct hw_channel_context *hwc)
+{
+ return min(hwc->tx_msg_size_limit, hwc->max_resp_msg_size);
+}
+
int mana_hwc_create_channel(struct gdma_context *gc);
void mana_hwc_destroy_channel(struct gdma_context *gc);
void mana_hwc_free_retained_channel(struct gdma_context *gc);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path
2026-11-09 15:45 ` [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path Wei Hu
@ 2026-11-10 15:45 ` sashiko-bot
2026-11-13 6:46 ` netdev-bot+sashiko
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-10 15:45 UTC (permalink / raw)
To: Wei Hu; +Cc: linux-rdma, linux-hyperv
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: mana: duplicate mana_hwc_teardown_queues() on HWC
rebuild failure
--
commit 63009cdb17b7f248623a5c752d26416fb4226096
Author: Long Li <longli@microsoft.com>
net: mana: add dynamic HWC queue depth with reinit path
This commit adds dynamic HWC queue depth negotiation by rebuilding the
bootstrap hardware channel when the PF reports a supported queue depth
between 2 and 128, with fallback to fresh bootstrap queues if the rebuild
fails.
Link: https://lore.kernel.org/r/20260908035201.402424-5-longli@microsoft.com
Link: https://lore.kernel.org/r/179196575846.434549.7933818740976196422@kernel.org
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 8bc00001d20e5..867f8dd3ea547 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
> @@ -1088,16 +1176,11 @@ static void mana_hwc_fence_channel(struct gdma_context *gc,
> mana_hwc_unpublish_cq(gc, hwc->cq->gdma_cq);
> }
>
> -static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
> +static int mana_hwc_teardown_queues(struct hw_channel_context *hwc)
> {
> struct gdma_context *gc = hwc->gdma_dev->gdma_context;
> int err;
>
> - mana_hwc_stop_channel(hwc);
> -
> - /* An unacknowledged destroy leaves the PF's mappings live. Fence
> - * software dispatch, but retain every PF-visible allocation.
> - */
> err = mana_smc_teardown_hwc(&gc->shm_channel, false,
> &hwc->setup_active);
> if (err) {
> @@ -1105,28 +1188,26 @@ static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
> "HWC teardown failed: %d, retaining PF-visible resources\n",
> err);
> mana_hwc_fence_channel(gc, hwc);
> - mana_hwc_retain_channel(hwc);
> - return false;
> + return err;
> }
>
> - mana_hwc_clear_cq_table(gc);
> -
> - /* Fence the EQ before releasing the remaining HWC state. */
> - if (hwc->cq)
> - mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
> -
> - if (hwc->txq)
> - mana_hwc_destroy_wq(hwc, hwc->txq);
> + mana_hwc_destroy_queues(hwc);
>
> - if (hwc->rxq)
> - mana_hwc_destroy_wq(hwc, hwc->rxq);
> + return 0;
> +}
>
> - kfree(hwc->caller_ctx);
> - hwc->caller_ctx = NULL;
> +static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
> +{
> + struct gdma_context *gc = hwc->gdma_dev->gdma_context;
> + int err;
>
> - mana_gd_free_res_map(&hwc->inflight_msg_res);
> + mana_hwc_stop_channel(hwc);
>
> - hwc->num_inflight_msg = 0;
> + err = mana_hwc_teardown_queues(hwc);
> + if (err) {
> + mana_hwc_retain_channel(hwc);
> + return false;
> + }
[ ... ]
> +static int mana_hwc_rebuild_channel(struct hw_channel_context *hwc,
> + u16 q_depth,
> + struct mana_hwc_init_report *report)
> +{
> + u32 timeout_ms = mana_hwc_timeout_read(hwc);
> + int cleanup_err;
> + int err;
> +
> + err = mana_hwc_teardown_queues(hwc);
> + if (err)
> + return err;
> +
> + err = mana_hwc_build_channel(hwc, q_depth, report);
> + if (!err)
> + err = mana_hwc_validate_report(hwc, report, q_depth,
> + true, true);
> + if (!err)
> + err = mana_hwc_finish_channel(hwc, report);
> + if (!err)
> + return 0;
> +
> + dev_warn(hwc->dev,
> + "HWC depth %u rebuild failed, restoring bootstrap: %d\n",
> + q_depth, err);
> +
> + cleanup_err = mana_hwc_teardown_queues(hwc);
> + if (cleanup_err)
> + return cleanup_err;
[ ... ]
> @@ -1234,7 +1452,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
> gd->pdid = INVALID_PDID;
> gd->doorbell = INVALID_DOORBELL;
>
> - err = mana_hwc_create_bootstrap_channel(hwc);
> + err = mana_hwc_prepare_channel(hwc);
> if (err) {
> mana_hwc_release_channel(hwc);
> return err;
[Severity: Medium]
Does mana_hwc_release_channel() issue a duplicate mana_hwc_teardown_queues()
when mana_hwc_rebuild_channel() fails inside mana_hwc_teardown_queues()?
mana_smc_teardown_hwc_locked() only clears hwc->setup_active when the SMC
teardown transaction succeeds. If mana_hwc_teardown_queues() fails in
mana_hwc_rebuild_channel(), it fences the channel and returns the error with
hwc->setup_active still true, and the error path in mana_hwc_create_channel()
then calls mana_hwc_release_channel() -> mana_hwc_teardown_queues() a second
time on the same channel. On an unresponsive shared-memory channel this
stalls for a second 20-40s poll timeout; if the PF completes the first
SMC_MSG_TYPE_DESTROY_HWC right after the first read times out and rejects the
second destroy with a status error, mana_hwc_release_channel() retains the
already-destroyed channel.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794208055.git.weh@linux.microsoft.com?part=4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization
2026-11-09 15:45 ` [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization Wei Hu
@ 2026-11-13 6:46 ` netdev-bot+sashiko
0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-11-13 6:46 UTC (permalink / raw)
To: weh
Cc: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen, netdev, linux-rdma, linux-hyperv, linux-kernel,
gargaditya, kees+treewide, kees, mawasthi, weh, edumazet
Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.
Critical: 0 · High: 0 (1) · Medium: 3 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] An ESTABLISH_HWC that the PF explicitly rejects is still
recorded as live PF ownership.
- [Medium] When DESTROY_HWC is not acknowledged, the retained HWC is
eventually discarded through mana_gd_free_context() ->…
- [Medium] setup_active and the retained HWC mean 'the PF may still own
the HWC queues', but a device reset never clears them, even though a…
- [Low] The commit is presented as preparation for dynamic HWC sizing,
but it also fixes bugs that are reachable in the baseline driver.
- [Low] mana_gd_process_eqe() now drops two cases silently that used to
WARN: a completion EQE whose CQ id is >= max_num_cqs…
- [Low] The new comment in mana_gd_process_eqe() says the IRQ RCU read-
side section 'protects the table until HWC teardown has fenced its EQ…
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch now
documents RCU protection for cq_table in mana_gd_process_eqe(), and…
--
net: mana: prepare HWC ownership for safe reinitialization
This commit serializes ESTABLISH_HWC and DESTROY_HWC on the shared memory
channel and tracks PF ownership of the HWC queues with setup_active. When
DESTROY_HWC is not acknowledged, the HWC context is retained. It also
snapshots the bootstrap CQ count and id, publishes cq_table with
acquire/release ordering, and adds a final-disposal helper used when the
gdma_context is freed.
Link: https://lore.kernel.org/r/20260908035201.402424-2-longli@microsoft.com
Link: https://lore.kernel.org/r/20260908035201.402424-5-longli@microsoft.com
Link: https://lore.kernel.org/r/178910960115.219967.13830871915506436112@kernel.org
Link: https://lore.kernel.org/r/179196575238.434549.10775387794024020898@kernel.org
Link: https://lore.kernel.org/r/sashiko-outbox-170992@kernel.org
> Dynamic HWC queue sizing tears down the bootstrap queues and establishes a
> second channel before publishing it. Prepare the existing HWC ownership and
> teardown paths so that sequence cannot free or reuse state still reachable
> by either the PF or an EQ handler.
[Severity: Low]
The commit is described as preparation for dynamic HWC sizing. Several of
the changes, though, look like fixes for bugs that can be hit in the current
driver:
- mana_hwc_destroy_channel() ignored the return value of
mana_smc_teardown_hwc(). It freed the HWC EQ/CQ/RQ/SQ DMA memory even
when the PF never acknowledged DESTROY_HWC.
- mana_hwc_init_event_handler() wrote gd->gdma_context->max_num_cqs = val
on every HWC_INIT_DATA_MAX_NUM_CQS event. A late event could raise the
bound past the cq_table allocation and allow an out-of-bounds cq_table[]
read in mana_gd_process_eqe().
- mana_hwc_establish_channel() used WARN_ON() on a CQ id and count supplied
by the device.
- DESTROY_HWC was skipped whenever gc->max_num_cqs was 0, even after
ESTABLISH_HWC had been submitted.
Could these be split out as separate fixes with Fixes: tags, or at least
described in the commit message, so they can be picked up for net and
stable?
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index ab942d5bfe058..8df130386da34 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
[ ... ]
> @@ -796,11 +804,16 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
> switch (type) {
> case GDMA_EQE_COMPLETION:
> cq_id = eqe->details[0] & 0xFFFFFF;
> - if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs))
> + /* The IRQ handler's RCU read-side section protects the table
> + * until HWC teardown has fenced its EQ and waited for readers.
> + */
[Severity: Low]
Does this comment match the teardown order? In mana_hwc_destroy_channel()
the table is withdrawn and freed before the HWC EQ is fenced:
mana_hwc_clear_cq_table(gc);
/* Fence the EQ before releasing the remaining HWC state. */
if (hwc->cq)
mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
mana_gd_fence_eq() is only called from inside mana_hwc_destroy_cq().
Readers are actually kept safe by the NULL release-store and the
synchronize_rcu() in mana_hwc_clear_cq_table(), not by an EQ fence.
Could the comment describe that instead? Otherwise a later change might
depend on an EQ fence that has not happened yet at this point.
> + cq_table = smp_load_acquire(&gc->cq_table);
> + if (!cq_table || cq_id >= READ_ONCE(gc->max_num_cqs))
> break;
>
> - cq = gc->cq_table[cq_id];
> - if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
> + cq = READ_ONCE(cq_table[cq_id]);
> + if (!cq || WARN_ON_ONCE(cq->type != GDMA_CQ ||
> + cq->id != cq_id))
> break;
[Severity: Low]
Two cases that used to WARN are now dropped silently: a completion for a CQ
id at or above max_num_cqs, and an empty cq_table slot. The commit message
does not mention this.
The comment in mana_destroy_txq() in mana_en.c still depends on the removed
WARN:
/* Detach the CQ only after the WQ object is destroyed,
* so a completion EQE that arrives before then still
* finds the CQ in cq_table[] rather than hitting the
* WARN in mana_gd_process_eqe(). One grace period then
* covers both mana_gd_intr() and NAPI users.
*/
Should that comment be updated, and should the commit message mention this
behavior change?
>
> if (cq->cq.callback)
[Severity: High]
This is a pre-existing issue and was not introduced by this patch. Now that
the RCU protection of cq_table is documented here, though, mana_ib does not
seem to follow it. mana_ib_remove_cq_cb() in drivers/infiniband/hw/mana/cq.c
does:
kfree(gc->cq_table[cq->queue.id]);
gc->cq_table[cq->queue.id] = NULL;
The gdma_queue is freed before the slot is cleared, and nothing waits for a
grace period. mana_ib_destroy_cq() calls mana_ib_remove_cq_cb() before
mana_ib_gd_destroy_cq(), so the hardware CQ can still raise EQEs at that
point.
Can mana_gd_process_eqe() on another CPU load cq from cq_table[cq_id] and
then read cq->type, cq->id and cq->cq.callback from freed memory? That would
include calling through the freed callback pointer. In a CVM, the host could
also inject such EQEs.
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index fb9f38f65911c..9e0964072af9a 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
> @@ -759,6 +843,13 @@ int mana_hwc_create_channel(struct gdma_context *gc)
> u16 q_depth_max;
> int err;
>
> + /* Retry a retained context before assigning queues to the PF again. */
> + if (gd->driver_data) {
> + mana_hwc_destroy_channel(gc);
> + if (gd->driver_data)
> + return -ETIMEDOUT;
> + }
> +
[Severity: Medium]
What happens to a retained context across a device reset? Only an
acknowledged DESTROY_HWC in mana_smc_teardown_hwc_locked() clears
setup_active, but a reset discards the PF-side HWC state anyway.
mana_serv_reset() does:
mana_gd_suspend(pdev, PMSG_SUSPEND);
msleep(MANA_SERVICE_PERIOD * 1000);
ret = mana_gd_resume(pdev);
if (ret == -ETIMEDOUT || ret == -EPROTO) {
...
mana_serv_rescan(pdev);
Suppose the SMC aperture reads as all-ones, or does not respond, while the
device is being serviced. mana_smc_poll_register() with reset=false treats
bit 31 as PF ownership and times out. mana_hwc_destroy_channel() then
retains the HWC instead of freeing it as it did before this patch.
On resume, mana_gd_resume()->mana_gd_setup()->mana_hwc_create_channel()
retries DESTROY_HWC for an HWC the reset already wiped, instead of creating
a new one.
If that retry fails, create returns -ETIMEDOUT and mana_serv_reset() falls
back to mana_serv_rescan(). That path also leaks the HWC DMA through
mana_gd_remove()->mana_gd_free_context().
Should a reset clear setup_active, or should this path use the reset-aware
SMC mode (reset_vf / SHMEM_VF_RESET_STATE)? Every caller currently passes
reset_vf=false.
[ ... ]
> @@ -852,9 +960,44 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
> kfree(hwc);
> gc->hwc.driver_data = NULL;
> gc->hwc.gdma_context = NULL;
> +}
>
> - vfree(gc->cq_table);
> - gc->cq_table = NULL;
> +static void mana_hwc_free_retained_wq(struct hwc_wq *wq)
> +{
> + if (!wq)
> + return;
> +
> + kfree(wq->msg_buf);
> + kfree(wq->gdma_wq);
> + kfree(wq);
> +}
> +
> +/* Final disposal after IRQ and service-work teardown, not a retry path.
> + * Free only software state: unacknowledged HWC DMA allocations stay live.
> + */
> +void mana_hwc_free_retained_channel(struct gdma_context *gc)
> +{
> + struct hw_channel_context *hwc = gc->hwc.driver_data;
> +
> + if (!hwc)
> + return;
> +
> + gc->hwc.driver_data = NULL;
> + mana_hwc_clear_cq_table(gc);
> +
> + if (hwc->cq) {
> + kfree(hwc->cq->gdma_eq);
> + kfree(hwc->cq->gdma_cq);
> + kfree(hwc->cq->comp_buf);
> + kfree(hwc->cq);
> + }
[Severity: Medium]
These wrappers, together with wq->gdma_wq and wq->msg_buf in
mana_hwc_free_retained_wq(), hold the only mem_info.virt_addr and
dma_handle references to the coherent DMA of the HWC EQ, CQ, RQ and SQ
rings and both message buffers. After they are kfree()d,
mana_gd_free_memory() can never be called on those buffers.
Is there anything that bounds this leak? On the PCI probe path, a failure
with -ETIMEDOUT or -EPROTO frees gc via mana_gd_free_context() at free_gc.
It then schedules mana_dev_recovery_work, which probes again with a fresh
gc:
mana_gd_probe()
mana_gd_setup()
mana_hwc_create_channel() /* ESTABLISH submitted, then fails */
mana_hwc_destroy_channel() /* DESTROY times out, retained */
mana_gd_free_context() /* wrappers freed, DMA leaked */
schedule_delayed_work(&mana_dev_recovery_work.work, ...)
If a PF keeps hanging mid-transaction, does every recovery cycle leak
another set of HWC queues?
In mana_cdx_gd_remove(), cdx_clear_master() runs before
mana_gd_free_context(). On PCI remove, pci_disable_device() follows shortly
after it. Once bus mastering is off, could the retained DMA be freed
instead of leaked?
[ ... ]
> diff --git a/drivers/net/ethernet/microsoft/mana/shm_channel.c b/drivers/net/ethernet/microsoft/mana/shm_channel.c
> index d21b5db06e509..df9d05062dd57 100644
> --- a/drivers/net/ethernet/microsoft/mana/shm_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/shm_channel.c
[ ... ]
> @@ -229,6 +243,7 @@ int mana_smc_setup_hwc(struct shm_channel *sc, bool reset_vf, u64 eq_addr,
> /* Write 256-message buffer to shared memory (final 32-bit write
> * triggers HW to set possession bit to PF).
> */
> + *submitted = true;
> dword = (u32 *)shm_buf;
> for (i = 0; i < SMC_APERTURE_DWORDS; i++)
> writel(*dword++, sc->base + i * SMC_BASIC_UNIT);
[Severity: Medium]
mana_hwc_establish_channel() passes &hwc->setup_active as submitted. What
happens when the PF explicitly rejects ESTABLISH_HWC?
For a well-formed response with a non-zero status, mana_smc_read_response()
returns -EPROTO:
if (hdr.status != 0) {
dev_err(sc->dev, "SMC operation failed: 0x%x\n", hdr.status);
return -EPROTO;
}
setup_active stays true in that case. mana_hwc_destroy_channel() then sends
DESTROY_HWC for a channel the PF said it never created.
If that DESTROY is also rejected or times out, the context stays in
gc->hwc.driver_data. Every later mana_hwc_create_channel() on this
gdma_context then only retries the teardown:
if (gd->driver_data) {
mana_hwc_destroy_channel(gc);
if (gd->driver_data)
return -ETIMEDOUT;
}
Can this keep the HWC from coming back up on PM resume, or after
mana_serv_reset(), until the device is unbound? Should the flag be cleared
when the PF returns an explicit failure status, as opposed to an ambiguous
timeout?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794208055.git.weh%40linux.microsoft.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v10 2/4] net: mana: give each HWC message slot its own completion state
2026-11-09 15:45 ` [PATCH net-next v10 2/4] net: mana: give each HWC message slot its own completion state Wei Hu
@ 2026-11-13 6:46 ` netdev-bot+sashiko
0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-11-13 6:46 UTC (permalink / raw)
To: weh
Cc: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen, netdev, linux-rdma, linux-hyperv, linux-kernel,
gargaditya, kees+treewide, kees, mawasthi, weh, edumazet
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The patch closes memory-safety and wrong-result bugs that are
reachable in the baseline, but it carries no Fixes: tag and no stable…
Pre-existing issues:
- [Medium] At this commit, a late response for a request that already
timed out can be delivered to the next request that reuses the same…
- [Medium] At this commit, mana_hwc_handle_resp() copies resp_len bytes,
a value the device supplies via rx_oob->tx_oob_data_size, from the RX…
- [Medium] At this commit, the timeout path in mana_hwc_send_request()
frees the msg_id slot through mana_hwc_finish_request() while the SQ…
- [Medium] mana_hwc_rx_event_handler() returns without calling
mana_hwc_post_rx_wqe(hwc->rxq, rx_req) when the device-supplied…
--
net: mana: give each HWC message slot its own completion state
This adds a per-slot spinlock to struct hwc_caller_ctx. The lock protects
the caller's output buffer, the Linux error and the device status.
The output buffer is published only while the sender waits and is
withdrawn before every return. Completion and result state are
reinitialized when a slot is acquired, and status handling and cleanup
move into helpers.
Link: https://lore.kernel.org/r/20260908035201.402424-3-longli@microsoft.com
Link: https://lore.kernel.org/r/20260908035201.402424-4-longli@microsoft.com
Link: https://lore.kernel.org/r/179196575434.434549.4428132041097324407@kernel.org
Link: https://lore.kernel.org/r/179196575640.434549.9102950927041409151@kernel.org
> Protect the caller's output buffer, Linux error and device status with a
> per-slot lock. Publish the output buffer only while the sender is waiting
> and withdraw it before every return. A late response cannot copy through a
> pointer to caller storage whose lifetime has ended.
>
> Reinitialize completion and result state under both the allocation and
> per-slot locks when acquiring the slot.
[Severity: Low]
These changes fix bugs that can already be hit in the baseline, but the
commit has no Fixes: tag and no stable note. Should it have them?
Before this patch, mana_hwc_send_request() set ctx->output_buf = resp and
never cleared it. On timeout it called mana_hwc_put_msg_index() and
returned. A late response in mana_hwc_handle_resp() could then memcpy()
through ctx->output_buf into the stack frame of a caller that had already
returned.
The baseline also never reinitialized comp_event after the
init_completion() in mana_hwc_test_channel(). A complete() that landed
after a timeout carried over to the next user of the slot. That user then
returned immediately with stale error and status and an unfilled response
buffer.
Both become easier to hit once hwc_timeout drops to 1 ms after the first
timeout.
Would something like this be appropriate?
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
This commit only partly closes the late-response problem. Could the
message also tell backporters that the follow-up "net: mana: support
concurrent HWC requests" is needed too?
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 9e0964072af9a..6f0ccdeffc4f9 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
> @@ -90,14 +100,21 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
> }
>
> ctx = hwc->caller_ctx + msg_id;
> - err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
> - if (err)
> - goto out;
>
> - ctx->status_code = resp_msg->status;
> + spin_lock(&ctx->lock);
> + if (!ctx->output_buf) {
> + spin_unlock(&ctx->lock);
> + mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
> + return;
> + }
>
> - memcpy(ctx->output_buf, resp_msg, resp_len);
> -out:
> + err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
> + if (!err) {
> + ctx->status_code = resp_msg->status;
> + memcpy(ctx->output_buf, resp_msg, resp_len);
[Severity: Medium]
This isn't a bug introduced by this patch, but can this memcpy() read past
the end of the RX slot?
Here resp_len comes from the device through rx_oob->tx_oob_data_size.
mana_hwc_verify_resp_msg() only checks it against the caller's buffer:
if (resp_len > caller_ctx->output_buflen)
return -EPROTO;
Nothing in mana_hwc_send_request() keeps the caller's resp_len (which
becomes output_buflen) at or below rx_req->buf_len, which is
max_req_msg_size. If a caller passes a larger resp_len, a buggy or
malicious device could make this copy pull adjacent RX DMA slots into the
caller's buffer.
The later patch "net: mana: add dynamic HWC queue depth with reinit path"
fixes this. There, mana_hwc_get_msg_index() rejects any resp_len above
min(hwc->rx_msg_size_limit, hwc->max_req_msg_size).
> + }
> +
> + ctx->output_buf = NULL;
> ctx->error = err;
>
> /* Must post rx wqe before complete(), otherwise the next rx may
> @@ -106,6 +123,7 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
> mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
>
> complete(&ctx->comp_event);
> + spin_unlock(&ctx->lock);
> }
[Severity: Medium]
This isn't a bug introduced by this patch, but the caller of this
function, mana_hwc_rx_event_handler(), drops the RX buffer when the device
sends an out-of-range msg_id:
msg_id = READ_ONCE(resp->response.hwc_msg_id);
if (msg_id >= hwc->num_inflight_msg) {
dev_err(hwc->dev, "HWC RX: wrong msg_id=%u\n", msg_id);
return;
}
rx_req is already known at this point. Should this path call
mana_hwc_post_rx_wqe(hwc->rxq, rx_req) before it returns?
As written, each such completion permanently removes one buffer from the
HWC RQ. After q_depth of them the RQ is empty, and later responses have no
buffer to land in. This still looks present at the end of the series.
[ ... ]
> @@ -1048,11 +1105,24 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
[ ... ]
> if (!wait_for_completion_timeout(&ctx->comp_event,
> - (msecs_to_jiffies(hwc->hwc_timeout)))) {
> + msecs_to_jiffies(hwc->hwc_timeout))) {
[ ... ]
> if (hwc->hwc_timeout != 0)
> dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
> command, hwc->hwc_timeout);
> @@ -1061,27 +1131,16 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
> if (hwc->hwc_timeout > 1)
> hwc->hwc_timeout = 1;
>
> - err = -ETIMEDOUT;
> - goto out;
> + mana_hwc_finish_request(hwc, ctx, msg_id);
> + return -ETIMEDOUT;
[Severity: Medium]
This isn't a bug introduced by this patch, but can a late response for a
request that timed out still reach the next request that reuses the same
msg_id?
Sender A times out and calls
mana_hwc_finish_request()->mana_hwc_put_msg_index(), which clears the
bitmap bit right away. Sender B then gets the same index from
find_first_zero_bit(), publishes ctx->output_buf = resp_B and posts its
request.
When A's late response arrives, mana_hwc_handle_resp() checks only the
msg_id and whether ctx->output_buf is non-NULL:
spin_lock(&ctx->lock);
if (!ctx->output_buf) {
Both checks pass for B's slot. A's response would then be copied into B's
buffer and complete B, and B would return A's data and status.
The commit message says this is not solved here. The later patch "net:
mana: support concurrent HWC requests" handles it by keeping a timed-out
slot quarantined. The slot stays reserved until the late response arrives
or mana_hwc_stop_channel() cancels it.
[Severity: Medium]
This is a pre-existing issue, but is the TX DMA buffer for this msg_id safe
to reuse at this point?
The SQ WQE posted by mana_hwc_post_tx_wqe() points at
txq->msg_buf->reqs[msg_id].buf_va, and the device may not have consumed it
yet. mana_hwc_get_msg_index() uses find_first_zero_bit(), so the next
sender usually gets the same index and overwrites that buffer:
req_msg = (struct gdma_req_hdr *)tx_wr->buf_va;
if (req)
memcpy(req_msg, req, req_len);
Could the device then read a mix of the old and new request contents?
The later patch "net: mana: support concurrent HWC requests" keeps the
slot's bitmap bit, and with it the TX buffer, until the late response
arrives or mana_hwc_stop_channel() cancels it.
> }
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794208055.git.weh%40linux.microsoft.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests
2026-11-09 15:45 ` [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests Wei Hu
@ 2026-11-13 6:46 ` netdev-bot+sashiko
0 siblings, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-11-13 6:46 UTC (permalink / raw)
To: weh
Cc: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen, netdev, linux-rdma, linux-hyperv, linux-kernel,
gargaditya, kees+treewide, kees, mawasthi, weh, edumazet
Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.
Critical: 0 · High: 1 (1) · Medium: 1 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] After one HWC command times out, later commands are rejected
without being posted, so teardown commands never reach the device.
- [Medium] mana_hwc_try_get_msg_index() turns a full bitmap into
-ETIMEDOUT whenever atomic_read(&hwc->abandoned_msgs) is non-zero,…
- [Low] The patch fixes a real bug without naming it: a late HWC response
could be delivered to the wrong command.
- [Low] mana_need_log() now suppresses logging for -ETIMEDOUT/-ENODEV
whenever gc->hwc.driver_data is NULL.
- [Low] gc->hwc_drain_waitq belongs to gdma_context (it sits next to
gc->hwc_lock, which the patch correctly initializes in…
Pre-existing issues:
- [High] Some code reads gc->hwc.driver_data with no hwc_lock and no
active_senders reference, then dereferences it:…
--
net: mana: support concurrent HWC requests
This patch gives each HWC message slot separate sender and response
references. It serializes SQ posting and replaces the semaphore with a
wait queue for admission. It also adds a publish/unpublish and
sender-drain protocol around gc->hwc.driver_data for runtime teardown.
Link: https://lore.kernel.org/r/20260908035201.402424-4-longli@microsoft.com
Link: https://lore.kernel.org/r/20260914175044.2a26bb46@kernel.org
Link: https://lore.kernel.org/r/179196575640.434549.9102950927041409151@kernel.org
Link: https://lore.kernel.org/r/179250339266.3135.15035365058593192217@kernel.org
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 8df130386da34..bb4a963c2cf5d 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -142,22 +142,26 @@ int mana_gd_init_registers(struct gdma_context *gc)
> return mana_gd_init_vf_regs(gc);
> }
>
> -/* Suppress logging when we set timeout to zero */
> +/* Cancellation and unpublication make teardown request failures expected. */
> bool mana_need_log(struct gdma_context *gc, int err)
> {
> struct hw_channel_context *hwc;
> + bool need_log = true;
> + unsigned long flags;
>
> - if (err != -ETIMEDOUT)
> + if (err != -ETIMEDOUT && err != -ENODEV)
> return true;
>
> if (!gc)
> return true;
>
> + spin_lock_irqsave(&gc->hwc_lock, flags);
> hwc = gc->hwc.driver_data;
> - if (hwc && hwc->hwc_timeout == 0)
> - return false;
> + if (!hwc || !mana_hwc_timeout_read(hwc))
> + need_log = false;
> + spin_unlock_irqrestore(&gc->hwc_lock, flags);
[Severity: Low]
Will this also hide live-channel failures during setup?
mana_hwc_test_channel() sets channel_up = true and calls
mana_gd_test_hwc_eq() before mana_hwc_publish_channel(). So
gc->hwc.driver_data is still NULL when mana_gd_test_eq_request() checks
the error:
mana_hwc_create_channel()
mana_hwc_create_bootstrap_channel()
mana_hwc_test_channel()
mana_gd_test_hwc_eq()
mana_gd_test_eq_request()
mana_need_log(gc, err) /* driver_data == NULL -> false */
If the bootstrap EQ test fails with -ETIMEDOUT or -ENODEV, the
"test_eq failed: %d" message is now dropped. The commit message says the
patch will "keep logging live-channel failures", which doesn't match.
Before this patch, driver_data was set at the start of
mana_hwc_create_channel() and the timeout was nonzero, so this was logged.
>
> - return true;
> + return need_log;
> }
[ ... ]
> @@ -1482,9 +1542,12 @@ int mana_gd_verify_vf_version(struct gdma_context *gc)
> struct gdma_verify_ver_resp resp = {};
> struct gdma_verify_ver_req req = {};
> struct hw_channel_context *hwc;
> + u32 timeout_ms;
> int err;
>
> + /* The setup caller must exclude concurrent HWC teardown. */
> hwc = gc->hwc.driver_data;
> +
[Severity: High]
This is a pre-existing issue, but the new comment claims an exclusion
that nothing seems to enforce. Is anything keeping hwc alive between
this read and the later mana_hwc_timeout_read() and
mana_hwc_timeout_update() calls?
GC_PROBE_SUCCEEDED is not cleared on suspend. If a
GDMA_EQE_HWC_RESET_REQUEST arrives during PM resume,
mana_gd_process_eqe() schedules the service work, which runs without
device_lock:
mana_serv_func()
mana_serv_reset()
mana_gd_suspend()
mana_gd_cleanup()
mana_hwc_destroy_channel()
kfree(hwc)
Meanwhile the resume thread may be in
mana_gd_resume()->mana_gd_setup()->mana_gd_verify_vf_version(), still
holding the raw pointer. The new active_senders drain only covers
callers inside mana_gd_send_request().
mana_gd_create_dma_region() (with the similar "The caller must keep the
HWC alive" comment) and mana_ib_gd_create_dma_region() in
drivers/infiniband/hw/mana/main.c have the same problem: both
dereference hwc->max_req_msg_size without hwc_lock.
Teardown now clears driver_data at the very start. Could these readers
also see NULL mid-teardown and dereference it?
> mana_gd_init_req_hdr(&req.hdr, GDMA_VERIFY_VF_DRIVER_VERSION,
> sizeof(req), sizeof(resp));
[ ... ]
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 6f0ccdeffc4f9..8bc00001d20e5 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -6,48 +6,140 @@
[ ... ]
> +static int mana_hwc_try_get_msg_index(struct hw_channel_context *hwc,
> + void *resp, u32 resp_len,
> + struct hwc_caller_ctx **caller_ctx)
> {
> struct gdma_resource *r = &hwc->inflight_msg_res;
> struct hwc_caller_ctx *ctx;
> unsigned long flags;
> u32 index;
> -
> - down(&hwc->sema);
> + int err;
>
> spin_lock_irqsave(&r->lock, flags);
> + if (!hwc->channel_up) {
> + spin_unlock_irqrestore(&r->lock, flags);
> + return -ENODEV;
> + }
>
> - index = find_first_zero_bit(hwc->inflight_msg_res.map,
> - hwc->inflight_msg_res.size);
> + index = find_first_zero_bit(r->map, r->size);
> + if (index >= r->size) {
> + err = -EBUSY;
> + if (!mana_hwc_timeout_read(hwc) ||
> + atomic_read(&hwc->abandoned_msgs))
> + err = -ETIMEDOUT;
> + spin_unlock_irqrestore(&r->lock, flags);
> + return err;
> + }
[Severity: High]
Can this reject teardown commands without ever posting them to the
device?
After a command times out, mana_hwc_wait_for_response() keeps the
response reference. The bitmap bit then stays set until a late response
arrives or mana_hwc_stop_channel() runs:
if (err == -EINPROGRESS) {
ctx->error = -ETIMEDOUT;
ctx->responded = true;
atomic_inc(&hwc->abandoned_msgs);
abandoned = true;
}
On the depth-one bootstrap channel, every later caller finds
index >= r->size with abandoned_msgs != 0. It returns -ETIMEDOUT here,
before anything is written to the SQ or the doorbell is rung.
The same happens when hwc_timeout is 0 after mana_serv_reset() calls
mana_gd_cancel_hwc_timeout(). In the mana_gd_suspend() that follows,
only the first command is posted.
Before this patch, the slot was released on timeout, so the next command
was still posted with a 1 ms wait.
The destructive callers free the memory whatever the error.
mana_destroy_wq_obj() returns void, and mana_destroy_rxq() continues to
page_pool_destroy() and mana_gd_destroy_queue(). mana_gd_destroy_queue()
then calls mana_gd_free_memory() whatever mana_gd_destroy_dma_region()
returns.
If the device is slow but still alive, could it keep WQ objects and DMA
regions that point at pages already returned to the allocator?
mana_hwc_timeout_reduce() drops the timeout to 1 ms after the first
abandonment. Against a device slower than that, roughly every other
later command could be dropped this way.
The commit message says:
A congestion-only -EBUSY can make destructive callers skip a
teardown command and still free its DMA memory.
Doesn't returning -ETIMEDOUT at admission have the same effect on those
callers?
This also seems to remain at the end of the series. "net: mana: add
dynamic HWC queue depth with reinit path" keeps the depth-one channel
when the PF reports depth 1, a depth above 128, or non-default maxima.
[ ... ]
> +static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, void *resp,
> + u32 resp_len,
> + struct hwc_caller_ctx **caller_ctx)
> +{
> + int err;
> +
> + /* Healthy contention must not skip a destructive control request. */
> + wait_event(hwc->admission_waitq,
> + (err = mana_hwc_try_get_msg_index(hwc, resp, resp_len,
> + caller_ctx)) != -EBUSY);
> + return err;
> +}
[Severity: Medium]
Only -EBUSY keeps a caller waiting here. mana_hwc_try_get_msg_index()
turns a full bitmap into -ETIMEDOUT whenever abandoned_msgs is non-zero,
however many of the occupied slots are actually abandoned.
Take a queue of depth N >= 2 with one abandoned slot and N-1 normal
in-flight commands. The next caller would fail at once, with nothing
posted. Waiting would be bounded: each of the other slots is released or
abandoned within its timeout, and abandonment calls wake_up_all() on
admission_waitq.
Would a check such as abandoned_msgs >= r->size match the commit
message's "without mistaking healthy contention for a failed command"?
At depth one the two checks are equivalent. However, "net: mana: add
dynamic HWC queue depth with reinit path" rebuilds the channel with
depths 2..128 and leaves this logic unchanged.
[ ... ]
> @@ -90,40 +182,53 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
> struct hwc_work_request *rx_req, u16 msg_id)
> {
> const struct gdma_resp_hdr *resp_msg = rx_req->buf_va;
> + struct gdma_resource *r = &hwc->inflight_msg_res;
> struct hwc_caller_ctx *ctx;
> + bool release;
> int err;
>
> - if (!test_bit(msg_id, hwc->inflight_msg_res.map)) {
> + spin_lock(&r->lock);
> + if (!test_bit(msg_id, r->map)) {
> + spin_unlock(&r->lock);
> dev_err(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id);
> mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
> return;
> }
>
> ctx = hwc->caller_ctx + msg_id;
> -
> spin_lock(&ctx->lock);
> - if (!ctx->output_buf) {
> + spin_unlock(&r->lock);
> +
> + /* Consume the response-side reference exactly once. This releases a
> + * quarantined slot after its late response arrives.
> + */
> + release = mana_hwc_clear_resp_pending(hwc, ctx);
> +
> + if (ctx->responded) {
[Severity: Low]
This isn't a bug, but the quarantine here also fixes a real misdelivery.
Before the series, a timed-out slot was released immediately, and at
depth one the next request reused msg_id 0. A late response for the old
command then passed test_bit() and was copied into the next caller's
output_buf.
The previous patch in the series says late-response correlation is
handled here. This commit message describes the quarantine but does not
name the misdelivery, and there is no Fixes: tag.
Could a Fixes: tag be added, for stable backports?
[ ... ]
> @@ -1020,11 +1175,146 @@ void mana_hwc_free_retained_channel(struct gdma_context *gc)
[ ... ]
> + gd->gdma_context = gc;
> + hwc->gdma_dev = gd;
> + hwc->dev = gc->dev;
> + WRITE_ONCE(hwc->hwc_timeout, HW_CHANNEL_WAIT_RESOURCE_TIMEOUT_MS);
> + init_waitqueue_head(&hwc->admission_waitq);
> + init_waitqueue_head(&gc->hwc_drain_waitq);
[Severity: Low]
Should gc->hwc_drain_waitq be initialized once in mana_gd_probe() and
mana_cdx_gd_probe(), next to spin_lock_init(&gc->hwc_lock)? It belongs to
gdma_context, but here it is re-initialized on every channel creation,
including each mana_gd_resume()->mana_gd_setup().
mana_hwc_stop_channel() sleeps on this queue after driver_data has been
cleared. A concurrent creator, for example the service reset work racing
PM suspend/resume, would see driver_data == NULL. It would then call
init_waitqueue_head() on a queue that still has a waiter.
Could that orphan the waiter in mana_hwc_stop_channel() and lose its
wakeup?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794208055.git.weh%40linux.microsoft.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path
2026-11-09 15:45 ` [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path Wei Hu
2026-11-10 15:45 ` sashiko-bot
@ 2026-11-13 6:46 ` netdev-bot+sashiko
1 sibling, 0 replies; 10+ messages in thread
From: netdev-bot+sashiko @ 2026-11-13 6:46 UTC (permalink / raw)
To: weh
Cc: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen, netdev, linux-rdma, linux-hyperv, linux-kernel,
gargaditya, kees+treewide, kees, mawasthi, weh, edumazet
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 2
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The patch turns the PF-reported HWC_INIT_DATA_MAX_REQUEST /
HWC_INIT_DATA_MAX_RESPONSE values into enforced per-direction limits.
- [Medium] The rebuild makes the HWC SQ, RQ and CQ rings larger, but
nothing tells the PF the new ring sizes, and nothing checks that it…
- [Low] Before this patch the PF's QUEUE_DEPTH, MAX_REQUEST and
MAX_RESPONSE init reports were read and ignored, so the channel always…
- [Low] The patch fixes an existing out-of-bounds MMIO bug inside a large
feature patch, with no Fixes: tag.
--
net: mana: add dynamic HWC queue depth with reinit path
The HWC is rebuilt at the PF-reported queue depth when that depth is
between 2 and 128 and both reported message sizes equal the 4096-byte
bootstrap allocation. If the rebuild fails, the driver falls back to
fresh bootstrap queues. The reported request and response sizes become
per-direction limits shared with the GDMA and RDMA request builders, and
the device-supplied HWC doorbell index is now checked against BAR0.
Link: https://lore.kernel.org/r/20260908035201.402424-5-longli@microsoft.com
Link: https://lore.kernel.org/r/179196575846.434549.7933818740976196422@kernel.org
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 8bc00001d20e5..867f8dd3ea547 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
> @@ -246,8 +264,18 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
> switch (event->type) {
> case GDMA_EQE_HWC_INIT_EQ_ID_DB:
> eq_db.as_uint32 = event->details[0];
> + if (!mana_gd_is_valid_doorbell(gd->gdma_context,
> + eq_db.doorbell)) {
> + dev_err(hwc->dev, "HWC: invalid doorbell %u\n",
> + eq_db.doorbell);
> + WRITE_ONCE(hwc->hwc_init_error, -EPROTO);
> + complete(&hwc->hwc_init_eqe_comp);
> + break;
> + }
> +
> hwc->cq->gdma_eq->id = eq_db.eq_id;
> gd->doorbell = eq_db.doorbell;
> + hwc->hwc_init_doorbell = true;
> break;
[Severity: Low]
This isn't a bug in the new code, but this check fixes an existing
out-of-bounds MMIO write, together with the INVALID_DOORBELL early return
added to mana_gd_ring_doorbell().
Before this patch, eq_db.doorbell came from the device and was stored
without any check. mana_gd_ring_doorbell() then wrote to:
addr = gc->db_page_base + gc->db_page_size * db_index;
for any index the PF reported. If the INIT_EQ_ID_DB event never arrived,
db_index was INVALID_DOORBELL (UINT_MAX).
The commit message says this "hardens initial setup, which previously
accepted an unchecked device-supplied index". This fix does not depend on
the dynamic depth rebuild. Could it go in a separate patch with a Fixes:
tag, so older kernels can get the check? The doorbell bounds check in
mana_gd_register_device() covers the same kind of bug.
[ ... ]
> @@ -1175,32 +1256,169 @@ void mana_hwc_free_retained_channel(struct gdma_context *gc)
> gc->hwc.gdma_context = NULL;
> }
>
> -static int mana_hwc_create_bootstrap_channel(struct hw_channel_context *hwc)
> +static int
> +mana_hwc_validate_report(struct hw_channel_context *hwc,
> + const struct mana_hwc_init_report *report,
> + u32 expected_depth, bool check_depth,
> + bool require_exact_msg_sizes)
> +{
> + if (!report->queue_depth || !report->max_req_msg_size ||
> + !report->max_resp_msg_size) {
> + dev_err(hwc->dev,
> + "HWC: invalid maxima depth=%u req=%u resp=%u\n",
> + report->queue_depth, report->max_req_msg_size,
> + report->max_resp_msg_size);
> + return -EPROTO;
> + }
[Severity: Low]
Before this patch, the QUEUE_DEPTH, MAX_REQUEST and MAX_RESPONSE init
reports were read and then ignored. The HWC always came up at depth 1
with 4096-byte limits.
mana_hwc_prepare_channel() now runs this check on the bootstrap report
before it decides whether to keep the bootstrap queues.
mana_hwc_restore_bootstrap() runs the same check on the fallback path.
Can a PF that leaves out any of these HWC_INIT_DATA types, or reports 0,
still bring up the HWC? It looks like HWC creation now fails with -EPROTO
even in the depth-1 setup that used to work, with no fallback.
The commit message does not mention this change. If it is intended, could
the commit message say so? Otherwise, should a missing report fall back
to the bootstrap values?
> +
> + if (require_exact_msg_sizes &&
[ ... ]
> +static int mana_hwc_finish_channel(struct hw_channel_context *hwc,
> + const struct mana_hwc_init_report *report)
> +{
> + int err;
> +
> + /* MAX_REQUEST sizes the RQ; MAX_RESPONSE sizes the SQ. */
> + hwc->rx_msg_size_limit = report->max_req_msg_size;
> + hwc->tx_msg_size_limit = report->max_resp_msg_size;
[Severity: Medium]
Which direction does the PF mean by HWC_INIT_DATA_MAX_REQUEST?
Here MAX_REQUEST becomes the RX (response) limit and MAX_RESPONSE becomes
the TX (request) limit. The request budget in hw_channel.h then uses only
fields that come from the response side:
/* Host-to-PF requests are bounded by the negotiated TX limit and SQ stride. */
static inline u32
mana_hwc_get_max_req_size(const struct hw_channel_context *hwc)
{
return min(hwc->tx_msg_size_limit, hwc->max_resp_msg_size);
}
So the PF's MAX_REQUEST report never limits host-to-PF requests.
This matches the existing queue sizing in mana_hwc_init_queues(), where
the RQ stride comes from max_req_msg_size and the SQ stride from
max_resp_msg_size. Before this patch, though, mana_gd_create_dma_region()
and mana_ib_gd_create_dma_region() used hwc->max_req_msg_size as the
request budget. Both constants are 4096 and the PF reports were ignored,
so the two conventions never disagreed in practice.
That changes now. mana_hwc_prepare_channel() keeps the bootstrap queues
for depth 1, or for depths above 128, even when MAX_REQUEST !=
MAX_RESPONSE. This function then installs those reported values as live
limits.
What if MAX_REQUEST means the largest request the PF accepts?
mana_hwc_get_msg_index() would then reject valid requests larger than
MAX_RESPONSE with -EMSGSIZE. Requests larger than the PF's real request
limit would still be sent. The RDMA CREATE and ADD_PAGES batch sizes
(max_pgs_create_cmd and max_pgs_add_cmd) would also come from the wrong
report.
The commit message talks about "directional maxima" and "the effective
host-to-PF request budget". It does not say that the request budget now
comes from MAX_RESPONSE. Could the direction be documented with a
reference to the device specification? Alternatively, could both
directions be limited by min(MAX_REQUEST, MAX_RESPONSE), with the fields
renamed to match?
[ ... ]
> +static int mana_hwc_rebuild_channel(struct hw_channel_context *hwc,
> + u16 q_depth,
> + struct mana_hwc_init_report *report)
> +{
> + u32 timeout_ms = mana_hwc_timeout_read(hwc);
> + int cleanup_err;
> + int err;
> +
> + err = mana_hwc_teardown_queues(hwc);
> + if (err)
> + return err;
> +
> + err = mana_hwc_build_channel(hwc, q_depth, report);
> + if (!err)
> + err = mana_hwc_validate_report(hwc, report, q_depth,
> + true, true);
> + if (!err)
> + err = mana_hwc_finish_channel(hwc, report);
> + if (!err)
> + return 0;
[Severity: Medium]
How does the PF learn the ring sizes of the rebuilt channel?
At depth 1, mana_hwc_create_wq() and mana_hwc_create_cq() create 4 KiB
(MANA_MIN_QSIZE) rings. Above depth 8 the rings get larger:
mana_hwc_create_wq() {
...
if (q_type == GDMA_RQ)
queue_size = roundup_pow_of_two(GDMA_MAX_RQE_SIZE * q_depth);
else
queue_size = roundup_pow_of_two(GDMA_MAX_SQE_SIZE * q_depth);
...
}
At depth 128 the SQ is 64 KiB, the RQ 32 KiB and the CQ 16 KiB.
The HWC queues are only allocated locally; no GDMA_CREATE_QUEUE is sent.
The only message to the PF is ESTABLISH_HWC from
mana_smc_setup_hwc_locked(). It carries the four queue PFNs and the EQ
MSI-X vector, but no sizes.
Doorbells carry an offset that keeps increasing (queue->head *
GDMA_WQE_BU_SIZE in mana_gd_wq_ring_doorbell()). The device therefore
needs the same ring size to wrap at the same point as the driver.
The bootstrap channel already runs with 4 KiB rings while the PF reports a
larger depth. So the PF does not seem to size the rings from the depth it
reports.
The checks here also don't seem able to catch a mismatch. This
mana_hwc_validate_report() call only checks that the PF repeats the same
depth. mana_hwc_test_channel() sends one GENERATE_TEST_EQE request, which
never wraps a ring.
What happens if the PF keeps assuming 4 KiB rings? The SQ would first wrap
after about 8 WQEs, once the channel is already published. Could the driver
and device then disagree on ring positions, so that stale WQEs are run
again, CQEs are lost, or the device DMAs at the wrong ring offsets?
The fallback through mana_hwc_restore_bootstrap() sends a third
ESTABLISH_HWC with 4 KiB rings after one with larger rings. That depends
on the same undocumented rule.
Could the commit message or a comment describe how the PF learns the ring
geometry?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794208055.git.weh%40linux.microsoft.com
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-13 6:46 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 15:44 [PATCH net-next v10 0/4] net: mana: concurrent HWC requests and dynamic queue depth Wei Hu
2026-11-09 15:45 ` [PATCH net-next v10 1/4] net: mana: prepare HWC ownership for safe reinitialization Wei Hu
2026-11-13 6:46 ` netdev-bot+sashiko
2026-11-09 15:45 ` [PATCH net-next v10 2/4] net: mana: give each HWC message slot its own completion state Wei Hu
2026-11-13 6:46 ` netdev-bot+sashiko
2026-11-09 15:45 ` [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests Wei Hu
2026-11-13 6:46 ` netdev-bot+sashiko
2026-11-09 15:45 ` [PATCH net-next v10 4/4] net: mana: add dynamic HWC queue depth with reinit path Wei Hu
2026-11-10 15:45 ` sashiko-bot
2026-11-13 6:46 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox