* [PATCH net v4 0/2] enic: retain admin DMA resources and identify V2 VFs
@ 2026-11-01 19:07 Satish Kharat
2026-11-01 19:07 ` [PATCH net v4 1/2] enic: retain admin-channel DMA until queues stop Satish Kharat
2026-11-01 19:07 ` [PATCH net v4 2/2] enic: require DEVCMD2 and set the V2 VF mailbox source Satish Kharat
0 siblings, 2 replies; 5+ messages in thread
From: Satish Kharat @ 2026-11-01 19:07 UTC (permalink / raw)
To: netdev
Cc: Sesidhar Baddela, Aravind Dhanasekaran, Reese Faucette,
Satish Kharat, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Fix premature freeing of admin-channel buffers and set the correct V2 VF
source ID in mailbox messages. This is v4 of the two-patch admin-only
series. The generic datapath quarantine proposal withdrawn in v2 remains
out of scope, including its page-pool/device lifetime problem.
The first patch releases the admin WQ/RQ/CQ allocation only after both
complete queue-disable helpers succeed. Give both queues a first attempt,
then retry once only a helper which timed out. The 500 ms polling bound
is local to admin shutdown, and both RQ disable/check passes are preserved.
Ordinary datapath and DEVCMD2 queue timeouts are unchanged. Admin receive
buffers are driver-owned mappings, not page-pool pages returned later
from the networking stack.
Keep PCI bus mastering enabled during runtime admin quarantine so command
transport and ordinary teardown can continue. An affected V2 VF is closed
and cannot reopen while its control channel is quarantined. Detach the
netdev under RTNL during removal, then perform the VF unregister handshake
and admin close after releasing RTNL. Skip admin reopen after a failed
datapath open. Clear local VF registration after channel replies have been
drained, including after a failed re-registration closes the channel.
Removal makes a final direct admin queue check. If it still fails, finish
normal generic teardown and attempt CMD_CLOSE before final containment. Retain
the inert owner and unresolved DMA targets rather than free memory the
device might still reference. This intentionally leaves allocations and
a BAR reservation behind. It blocks ordinary reprobe while the PCI device
remains enumerated; PCI remove/rescan is not a supported recovery method.
A PCI-device reference keeps the reservation name valid, but does not
preserve IOMMU attachment or establish that DMA stopped. No IRQ, work or
other CPU callback may enter retained objects after module unload. DMA
debugging can report pending allocations. Reboot is the supported recovery.
The second patch requires DEVCMD2 for SR-IOV V2 and obtains the VF source
index from CMD_SRIOV_STATS_GET. A deployed async PF compares that source
against the CQ-derived sender. After QP enable, obtain and validate the ID
while receive interrupts remain masked. Publish send readiness before
unmasking, so queued notifications can be processed and acknowledged with
a valid source. Sends remain disabled until the ID is valid, using the
existing send gate. Notification handling does not rely on PF retransmission.
Keep the statistics target persistent and observe command completion
separately from firmware status. A later blocking QP or close result
fences earlier statistics DMA, including an error result. A pre-post
failure, posted timeout or NOWAIT posting does not establish completion.
This command ordering does not replace admin queue-stop proof.
Sashiko's feedback prompted this audit. The pre-existing failed-reset
reopen lifecycle and generic queue-disable error paths remain separate
follow-up work; this revision does not claim to fix them. Source review
and testing were assisted by LLM tools. The author is responsible for
the submitted changes.
Validation:
- V4 passed build and static checks at both patch boundaries. Sparse
reported the same existing diagnostics as the public base.
- Focused V4 tests verified that a late registration reply is cleared after
failed re-registration closes the channel. During an injected five-second
VF-unregister delay after detach, an unrelated RTNL operation completed.
- Full functional and interoperability tests passed with Cisco VIC PFs
and V2 VFs, including a nonzero VF and all four combinations of the
development ENIC and deployed async PF/VF drivers. Traffic covered
bidirectional ICMP, TCP, UDP and jumbo frames.
- Tests covered station/address/filter and administrative-MAC policy
changes, concurrent address updates and reopen, and multiple module
reload, ring resize, interface reopen and VF destroy/recreate cycles.
- A test-only mailbox-send failure exercised reset recovery. The real
send completed before the failure was injected; recovery restored
traffic, and the uninstrumented candidate then reloaded successfully.
- Focused V3 tests verified queued PF notification delivery after VF-ID
discovery, discovery-failure cleanup and statistics-buffer cleanup
after a completed close. Fault injection let real DMA complete.
- Earlier fault-injection tests covered admin queue retry, quarantine,
final removal cleanup and terminal retention. They let the real queues
stop; they did not reproduce a physically stuck queue.
The functional and interoperability tests included later VF/PF feature
patches. These two fixes alone do not enable in-tree PF SR-IOV support.
Physical device removal was not tested; a physical DMA-engine failure
was not reproduced.
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
Changes in v4:
- Clear vf_registered after closing the channel on both failed VF
re-registration paths; a late reply may have set it before the send
reported a timeout.
- Hold RTNL only while detaching the netdev during removal. Perform the VF
unregister handshake and admin close after releasing RTNL.
- Keep the valid pre-existing generic datapath repeated-stop problem as
separate follow-up work rather than adding a partial fix to this series.
- Link to v3: https://patch.msgid.link/20261027-b4-enic-dma-vf-index-net-generic-first-v1-20261010-v3-0-ba237ed71241@cisco.com
Changes in v3:
- Use observed blocking CMD_CLOSE completion, including an error result,
to fence earlier statistics DMA in probe unwind and removal.
- Obtain the VF source ID after QP enable but before receive interrupts
are unmasked, and permit sends before delivering queued notifications.
Use the existing send gate instead of a redundant invalid-ID check.
- Validate delayed notifications, discovery failures and completed-close
cleanup on a VF.
- Rerun the full functional suite and all four PF/VF driver pairings
with the v3 fixes in the later VF/PF feature compositions.
- Clear local VF registration after channel close in both reset workers.
- Correct comments and explain quarantine, recovery and retention limits.
- Link to v2: https://patch.msgid.link/20261022-b4-enic-dma-vf-index-net-generic-first-v1-20261010-v2-0-8db19b4a714e@cisco.com
Changes in v2:
- Withdraw v1 patch 1's generic datapath quarantine proposal. Rewrite v1
patch 2 as the first patch here, followed by the VF mailbox-source fix.
- Keep the admin buffers' lifetime separate from generic page pools and
notification teardown. Keep bus mastering enabled at runtime.
- Bound longer polling to the two admin queues, preserve both RQ passes,
and retry only a complete disable helper returning -ETIMEDOUT.
- Serialize remove/open and skip admin reopen after datapath-open failure.
- Observe DEVCMD2 transport completion independently of command success,
including a completed QP error, without treating pre-post errors or
NOWAIT posting as a fence.
- Clarify final remove recheck, terminal retention and reprobe limits.
- Link to v1: https://patch.msgid.link/20261016-b4-enic-dma-vf-index-net-generic-first-v1-20261010-v1-0-d27b55f69883@cisco.com
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@kernel.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
---
Satish Kharat (2):
enic: retain admin-channel DMA until queues stop
enic: require DEVCMD2 and set the V2 VF mailbox source
drivers/net/ethernet/cisco/enic/enic.h | 22 ++-
drivers/net/ethernet/cisco/enic/enic_admin.c | 247 +++++++++++++++++++++-----
drivers/net/ethernet/cisco/enic/enic_admin.h | 4 +-
drivers/net/ethernet/cisco/enic/enic_dev.c | 44 +++++
drivers/net/ethernet/cisco/enic/enic_dev.h | 3 +
drivers/net/ethernet/cisco/enic/enic_main.c | 113 +++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.c | 13 +-
drivers/net/ethernet/cisco/enic/vnic_dev.c | 85 ++++++++-
drivers/net/ethernet/cisco/enic/vnic_dev.h | 15 ++
drivers/net/ethernet/cisco/enic/vnic_devcmd.h | 34 ++++
10 files changed, 513 insertions(+), 67 deletions(-)
---
base-commit: 0df412012a26389582b120844d098e1e961f10a6
change-id: 20261010-b4-enic-dma-vf-index-net-generic-first-v1-20261010-d9abf4510335
Best regards,
--
Satish Kharat <satishkh@cisco.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net v4 1/2] enic: retain admin-channel DMA until queues stop
2026-11-01 19:07 [PATCH net v4 0/2] enic: retain admin DMA resources and identify V2 VFs Satish Kharat
@ 2026-11-01 19:07 ` Satish Kharat
2026-11-02 19:08 ` netdev-bot+sashiko
2026-11-01 19:07 ` [PATCH net v4 2/2] enic: require DEVCMD2 and set the V2 VF mailbox source Satish Kharat
1 sibling, 1 reply; 5+ messages in thread
From: Satish Kharat @ 2026-11-01 19:07 UTC (permalink / raw)
To: netdev
Cc: Sesidhar Baddela, Aravind Dhanasekaran, Reese Faucette,
Satish Kharat, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Admin close and the partial-open unwind currently free the admin
WQ/RQ/CQ allocation even when disabling one of the queues times out. A
posted WQ message or an RQ buffer can then remain reachable by the
device after its mapping and allocation have been released.
Track the channel as DOWN, LIVE or QUARANTINED. Stop its IRQ and work,
block new sends and join an in-flight sender before queue shutdown. Give
both queues a first disable attempt, then retry only a complete helper
that timed out. Poll for up to 500 ms per pass locally to the admin
path, preserving the RQ's two disable/check passes. Ordinary datapath
and DEVCMD2 queue timeouts are unchanged.
RUNNING == 0 proves that queue DMA has stopped. A nonzero value leaves
ownership unresolved; it does not establish that DMA is active. Release
the admin graph only when both complete disable helpers succeed. Retain
the graph otherwise and refuse to reopen it. Keep bus mastering enabled
at runtime so command transport and ordinary teardown can continue. A V2
VF with a quarantined control channel is closed and cannot reopen. Reset
callers must not reset or reopen a retained admin graph, or reopen admin
queues after a failed datapath open. Clear local VF registration after
draining channel replies, including when recovery exits early. Clear it
again after a failed re-registration closes the channel because a late
REGISTER_REPLY may have set it before the WQ completion timed out.
Check that an internal datapath open still has a present device. Recheck
the running state under RTNL in reset workers. During removal, detach the
netdev under RTNL and then release RTNL before the VF unregister handshake
and admin close. The detach and present-device checks exclude a concurrent
new open without holding the global lock across protocol and queue waits.
Removal makes a final direct admin queue check. If that still fails,
finish ordinary generic teardown and attempt CMD_CLOSE before clearing
bus mastering and disabling the PCI function. Keep the inert netdev,
vnic_dev, admin allocations and control targets rather than freeing
device-owned memory. Admin buffers use driver-owned mappings, never page
pools or pages returned later from sockets. This patch does not repair
the separate generic datapath queue-disable error path.
Keep the BAR reservation to block an ordinary rebind while the PCI
device remains enumerated. Use the PCI device name for the reservation
and hold a PCI-device reference so the owner-name pointer remains valid
after module unload. That reference does not preserve the IOMMU
attachment or prove DMA stopped. No IRQ, work or other CPU callback may
use the retained objects. DMA debugging can report the deliberately
pending allocations; reboot is the supported terminal recovery.
Fixes: 3258931d4052 ("enic: add admin channel open and close for SR-IOV")
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 17 ++-
drivers/net/ethernet/cisco/enic/enic_admin.c | 196 +++++++++++++++++++++------
drivers/net/ethernet/cisco/enic/enic_admin.h | 4 +-
drivers/net/ethernet/cisco/enic/enic_main.c | 88 +++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.c | 1 +
5 files changed, 252 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 7a509a056990..35127c549e64 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -238,6 +238,12 @@ enum enic_vf_type {
ENIC_VF_TYPE_V2,
};
+enum enic_admin_chan_state {
+ ENIC_ADMIN_CHAN_DOWN,
+ ENIC_ADMIN_CHAN_LIVE,
+ ENIC_ADMIN_CHAN_QUARANTINED,
+};
+
/* Per-instance private data structure */
struct enic {
struct net_device *netdev;
@@ -298,13 +304,12 @@ struct enic {
/* Admin channel resources for SR-IOV MBOX */
bool has_admin_channel;
- /* true only while the admin WQ/RQ/CQ are allocated and enabled; gates
- * enic_admin_channel_close() so it is a no-op after a failed (re)open
- * left the resources freed.
- */
- bool admin_chan_up;
- /* set on send timeout; cleared on channel re-open */
+ /* QUARANTINED retains admin DMA until the final remove-time recheck. */
+ enum enic_admin_chan_state admin_chan_state;
+ /* Blocks sends throughout close, partial open and timeout recovery. */
bool mbox_send_disabled;
+ /* A send timed out or channel shutdown left queue ownership unresolved. */
+ bool mbox_tx_poisoned;
struct vnic_wq admin_wq;
struct vnic_rq admin_rq;
struct vnic_cq admin_cq[2];
diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c
index 61c82b48044d..4325c6d9c30e 100644
--- a/drivers/net/ethernet/cisco/enic/enic_admin.c
+++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
@@ -5,6 +5,7 @@
#include <linux/netdevice.h>
#include <linux/dma-mapping.h>
#include <linux/interrupt.h>
+#include <linux/iopoll.h>
#include "vnic_dev.h"
#include "vnic_wq.h"
@@ -530,11 +531,22 @@ static void enic_admin_msg_drain(struct enic *enic)
int enic_admin_channel_open(struct enic *enic)
{
+ int close_err;
int err;
if (!enic->has_admin_channel)
return -ENODEV;
-
+ if (READ_ONCE(enic->admin_chan_state) != ENIC_ADMIN_CHAN_DOWN) {
+ netdev_err(enic->netdev,
+ "Refusing to open non-down admin channel (%u)\n",
+ READ_ONCE(enic->admin_chan_state));
+ return -EBUSY;
+ }
+ if (READ_ONCE(enic->mbox_tx_poisoned)) {
+ netdev_err(enic->netdev,
+ "Refusing to reopen poisoned admin channel\n");
+ return -EIO;
+ }
/* Keep MBOX sends disabled for the entire open sequence. It is
* cleared only after every resource is allocated and enabled below,
* so any early error return here leaves sends disabled and a
@@ -567,6 +579,7 @@ int enic_admin_channel_open(struct enic *enic)
vnic_wq_enable(&enic->admin_wq);
vnic_rq_enable(&enic->admin_rq);
+ WRITE_ONCE(enic->admin_chan_state, ENIC_ADMIN_CHAN_LIVE);
err = enic_admin_rq_fill(enic, GFP_KERNEL);
if (err) {
@@ -600,60 +613,51 @@ int enic_admin_channel_open(struct enic *enic)
enic->admin_cq[0].last_color,
enic->admin_cq[1].last_color);
- enic->admin_chan_up = true;
-
return 0;
disable_queues:
- enic_admin_teardown_intr(enic);
- enic_admin_qp_type_set(enic, QP_DISABLE);
- if (vnic_wq_disable(&enic->admin_wq))
- netdev_warn(enic->netdev, "Failed to disable admin WQ\n");
- if (vnic_rq_disable(&enic->admin_rq))
- netdev_warn(enic->netdev, "Failed to disable admin RQ\n");
- cancel_work_sync(&enic->admin_msg_work);
- enic_admin_msg_drain(enic);
- enic_admin_rq_drain(enic);
+ close_err = enic_admin_channel_close(enic);
+ if (close_err)
+ netdev_err(enic->netdev,
+ "Failed to close partial admin channel: %d\n",
+ close_err);
+ return err;
free_resources:
enic_admin_free_resources(enic);
return err;
}
-void enic_admin_channel_close(struct enic *enic)
+static int enic_admin_wq_disable(struct enic *enic)
{
- int err;
-
- if (!enic->has_admin_channel)
- return;
-
- /* Nothing to tear down if the channel was never (re)opened, e.g. a
- * failed enic_admin_channel_open() in probe or in the reset path;
- * otherwise the disable/clean calls below dereference freed resources.
- */
- if (!enic->admin_chan_up)
- return;
-
- WRITE_ONCE(enic->mbox_send_disabled, true);
+ struct vnic_wq *wq = &enic->admin_wq;
+ u32 running;
- netdev_dbg(enic->netdev, "admin channel close\n");
+ iowrite32(0, &wq->ctrl->enable);
+ return read_poll_timeout(ioread32, running, !running, 10, 500000,
+ false, &wq->ctrl->running);
+}
- vnic_intr_mask(&enic->admin_intr);
- enic_admin_teardown_intr(enic);
- cancel_work_sync(&enic->link_notify_work);
- cancel_work_sync(&enic->admin_msg_work);
- enic_admin_msg_drain(enic);
+static int enic_admin_rq_disable(struct enic *enic)
+{
+ struct vnic_rq *rq = &enic->admin_rq;
+ u32 running;
+ int err;
+ int i;
- enic_admin_qp_type_set(enic, QP_DISABLE);
+ /* Preserve the two disable/check passes used for the RQ mini-cache. */
+ for (i = 0; i < 2; i++) {
+ iowrite32(0, &rq->ctrl->enable);
+ err = read_poll_timeout(ioread32, running, !running, 10, 500000,
+ false, &rq->ctrl->running);
+ if (err)
+ return err;
+ }
- err = vnic_wq_disable(&enic->admin_wq);
- if (err)
- netdev_warn(enic->netdev,
- "Failed to disable admin WQ: %d\n", err);
- err = vnic_rq_disable(&enic->admin_rq);
- if (err)
- netdev_warn(enic->netdev,
- "Failed to disable admin RQ: %d\n", err);
+ return 0;
+}
+static void enic_admin_channel_release(struct enic *enic)
+{
vnic_wq_clean(&enic->admin_wq, enic_admin_wq_buf_clean);
enic_admin_rq_drain(enic);
vnic_cq_clean(&enic->admin_cq[0]);
@@ -662,6 +666,114 @@ void enic_admin_channel_close(struct enic *enic)
enic->admin_rq_handler = NULL;
enic_admin_free_resources(enic);
+ WRITE_ONCE(enic->admin_chan_state, ENIC_ADMIN_CHAN_DOWN);
+}
+
+int enic_admin_channel_close(struct enic *enic)
+{
+ int qp_err;
+ int rq_err;
+ int wq_err;
+
+ if (!enic->has_admin_channel)
+ return 0;
+ if (READ_ONCE(enic->admin_chan_state) == ENIC_ADMIN_CHAN_DOWN)
+ return 0;
+ if (READ_ONCE(enic->admin_chan_state) == ENIC_ADMIN_CHAN_QUARANTINED)
+ return -EBUSY;
+
+ WRITE_ONCE(enic->mbox_send_disabled, true);
+
+ if (READ_ONCE(enic->admin_chan_state) == ENIC_ADMIN_CHAN_LIVE) {
+ netdev_dbg(enic->netdev, "admin channel quiesce\n");
+
+ vnic_intr_mask(&enic->admin_intr);
+ enic_admin_teardown_intr(enic);
+ cancel_work_sync(&enic->link_notify_work);
+ cancel_work_sync(&enic->admin_msg_work);
+ enic_admin_msg_drain(enic);
+ }
+
+ /* A sender may have passed the gate before it was raised. Wait only
+ * after cancelling workers which can themselves wait for mbox_lock.
+ */
+ mutex_lock(&enic->mbox_lock);
+ qp_err = enic_admin_qp_type_set(enic, QP_DISABLE);
+ if (qp_err)
+ netdev_warn(enic->netdev,
+ "Failed to disable admin QP type: %d\n", qp_err);
+
+ wq_err = enic_admin_wq_disable(enic);
+ rq_err = enic_admin_rq_disable(enic);
+
+ /* Run both complete disable helpers once before retrying only a helper
+ * which timed out.
+ */
+ if (wq_err == -ETIMEDOUT)
+ wq_err = enic_admin_wq_disable(enic);
+ if (rq_err == -ETIMEDOUT)
+ rq_err = enic_admin_rq_disable(enic);
- enic->admin_chan_up = false;
+ if (wq_err)
+ netdev_warn(enic->netdev,
+ "Failed to disable admin WQ after retry: %d\n",
+ wq_err);
+ if (rq_err)
+ netdev_warn(enic->netdev,
+ "Failed to disable admin RQ after retry: %d\n",
+ rq_err);
+
+ /* QP_DISABLE does not report every low-level stop failure. The direct
+ * WQ/RQ RUNNING observations own the admin allocation graph.
+ */
+ if (wq_err || rq_err) {
+ WRITE_ONCE(enic->admin_chan_state, ENIC_ADMIN_CHAN_QUARANTINED);
+ WRITE_ONCE(enic->mbox_tx_poisoned, true);
+ mutex_unlock(&enic->mbox_lock);
+ /* Retained targets remain valid for late DMA. Keep bus mastering
+ * enabled for the independent datapath and command transport.
+ */
+ return wq_err ?: rq_err;
+ }
+
+ enic_admin_channel_release(enic);
+ WRITE_ONCE(enic->mbox_tx_poisoned, false);
+ mutex_unlock(&enic->mbox_lock);
+
+ /* Firmware rejection does not undermine the WQ/RQ proof. */
+ return qp_err;
+}
+
+int enic_admin_channel_recheck_on_remove(struct enic *enic)
+{
+ int rq_err;
+ int wq_err;
+
+ if (READ_ONCE(enic->admin_chan_state) != ENIC_ADMIN_CHAN_QUARANTINED)
+ return 0;
+
+ /* Channel close already detached every CPU entry point and attempted
+ * QP_DISABLE. Removal is the last direct check of queue ownership.
+ */
+ mutex_lock(&enic->mbox_lock);
+ wq_err = enic_admin_wq_disable(enic);
+ rq_err = enic_admin_rq_disable(enic);
+ if (wq_err || rq_err) {
+ mutex_unlock(&enic->mbox_lock);
+ return wq_err ?: rq_err;
+ }
+
+ enic_admin_channel_release(enic);
+ WRITE_ONCE(enic->mbox_tx_poisoned, false);
+ mutex_unlock(&enic->mbox_lock);
+ netdev_info(enic->netdev,
+ "Admin queues stopped at remove; releasing DMA graph\n");
+
+ return 0;
+}
+
+bool enic_admin_channel_is_quarantined(const struct enic *enic)
+{
+ return READ_ONCE(enic->admin_chan_state) ==
+ ENIC_ADMIN_CHAN_QUARANTINED;
}
diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.h b/drivers/net/ethernet/cisco/enic/enic_admin.h
index 62c80220b0ca..5945bbb2e6d4 100644
--- a/drivers/net/ethernet/cisco/enic/enic_admin.h
+++ b/drivers/net/ethernet/cisco/enic/enic_admin.h
@@ -20,7 +20,9 @@ struct enic_admin_msg {
};
int enic_admin_channel_open(struct enic *enic);
-void enic_admin_channel_close(struct enic *enic);
+int enic_admin_channel_close(struct enic *enic);
+int enic_admin_channel_recheck_on_remove(struct enic *enic);
+bool enic_admin_channel_is_quarantined(const struct enic *enic);
unsigned int enic_admin_wq_cq_service(struct enic *enic);
unsigned int enic_admin_rq_cq_service(struct enic *enic);
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 6b5824f45a14..04e4248d4872 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1736,6 +1736,13 @@ static int enic_open(struct net_device *netdev)
.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
};
+ /* Internal reopen paths also bypass the networking core's presence check. */
+ if (!netif_device_present(netdev))
+ return -ENODEV;
+ if (enic_is_sriov_vf_v2(enic) &&
+ enic_admin_channel_is_quarantined(enic))
+ return -EBUSY;
+
err = enic_request_intr(enic);
if (err) {
netdev_err(netdev, "Unable to request irq.\n");
@@ -2206,6 +2213,9 @@ static void enic_admin_chan_reopen(struct enic *enic)
{
int err;
+ if (enic_admin_channel_is_quarantined(enic))
+ return;
+
/* Install the MBOX receive handler and clear pending reply state before
* opening the channel, so the handler is in place before the admin
* interrupt is unmasked and no early completion is dropped. Keep the
@@ -2237,6 +2247,7 @@ static void enic_admin_chan_reopen(struct enic *enic)
"MBOX capability check after reset failed: %d\n",
err);
enic_admin_channel_close(enic);
+ WRITE_ONCE(enic->vf_registered, false);
return;
}
err = enic_mbox_vf_register(enic);
@@ -2245,6 +2256,7 @@ static void enic_admin_chan_reopen(struct enic *enic)
"MBOX VF re-registration after reset failed: %d\n",
err);
enic_admin_channel_close(enic);
+ WRITE_ONCE(enic->vf_registered, false);
}
} else {
/* The link came back up during enic_open() above while MBOX
@@ -2264,6 +2276,9 @@ static void enic_reset(struct work_struct *work)
rtnl_lock();
+ if (!netif_running(enic->netdev))
+ goto unlock;
+
/* Stop any activity from infiniband */
enic_set_api_busy(enic, true);
@@ -2273,8 +2288,17 @@ static void enic_reset(struct work_struct *work)
* DMAs from the about-to-be-reset rings) and frees the admin resources
* so they are cleanly re-allocated afterwards.
*/
- if (enic_has_admin_chan(enic))
+ if (enic_has_admin_chan(enic)) {
enic_admin_channel_close(enic);
+ /* Channel close has drained registration replies. */
+ if (enic_is_sriov_vf_v2(enic))
+ WRITE_ONCE(enic->vf_registered, false);
+ }
+ if (enic_admin_channel_is_quarantined(enic)) {
+ /* Core close excludes netpoll and clears __LINK_STATE_START first. */
+ dev_close(enic->netdev);
+ goto reset_out;
+ }
enic_stop(enic->netdev);
if (enic_is_sriov_vf_v2(enic))
@@ -2287,7 +2311,8 @@ static void enic_reset(struct work_struct *work)
enic_dev_set_ig_vlan_rewrite_mode(enic);
enic_ext_cq(enic);
- enic_open(enic->netdev);
+ if (enic_open(enic->netdev))
+ goto reset_out;
/* Re-establish the admin/MBOX channel after the data path is back up.
* It was fully torn down by enic_admin_channel_close() above;
@@ -2296,12 +2321,17 @@ static void enic_reset(struct work_struct *work)
*/
if (enic_has_admin_chan(enic))
enic_admin_chan_reopen(enic);
+ if (enic_is_sriov_vf_v2(enic) &&
+ enic_admin_channel_is_quarantined(enic))
+ dev_close(enic->netdev);
+reset_out:
/* Allow infiniband to fiddle with the device again */
enic_set_api_busy(enic, false);
call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev);
+unlock:
rtnl_unlock();
}
@@ -2311,6 +2341,9 @@ static void enic_tx_hang_reset(struct work_struct *work)
rtnl_lock();
+ if (!netif_running(enic->netdev))
+ goto unlock;
+
/* Stop any activity from infiniband */
enic_set_api_busy(enic, true);
@@ -2318,8 +2351,16 @@ static void enic_tx_hang_reset(struct work_struct *work)
* the same reason as the soft reset path: stop the admin QP and free
* the admin resources before the hardware queues are wiped.
*/
- if (enic_has_admin_chan(enic))
+ if (enic_has_admin_chan(enic)) {
enic_admin_channel_close(enic);
+ /* Channel close has drained registration replies. */
+ if (enic_is_sriov_vf_v2(enic))
+ WRITE_ONCE(enic->vf_registered, false);
+ }
+ if (enic_admin_channel_is_quarantined(enic)) {
+ dev_close(enic->netdev);
+ goto reset_out;
+ }
enic_dev_hang_notify(enic);
enic_stop(enic->netdev);
@@ -2333,7 +2374,8 @@ static void enic_tx_hang_reset(struct work_struct *work)
enic_dev_set_ig_vlan_rewrite_mode(enic);
enic_ext_cq(enic);
- enic_open(enic->netdev);
+ if (enic_open(enic->netdev))
+ goto reset_out;
/* Re-establish the admin/MBOX channel after the data path is back up.
* It was fully torn down by enic_admin_channel_close() above;
@@ -2342,12 +2384,17 @@ static void enic_tx_hang_reset(struct work_struct *work)
*/
if (enic_has_admin_chan(enic))
enic_admin_chan_reopen(enic);
+ if (enic_is_sriov_vf_v2(enic) &&
+ enic_admin_channel_is_quarantined(enic))
+ dev_close(enic->netdev);
+reset_out:
/* Allow infiniband to fiddle with the device again */
enic_set_api_busy(enic, false);
call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev);
+unlock:
rtnl_unlock();
}
@@ -3036,7 +3083,8 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_out_free_netdev;
}
- err = pci_request_regions(pdev, DRV_NAME);
+ /* A terminal admin allocation keeps the reservation after module unload. */
+ err = pci_request_regions(pdev, dev_name(dev));
if (err) {
dev_err(dev, "Cannot request PCI regions, aborting\n");
goto err_out_disable_device;
@@ -3374,6 +3422,17 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
#endif
err_out_vnic_unregister:
+ if (enic_admin_channel_is_quarantined(enic)) {
+ /* Normal generic cleanup and CMD_CLOSE have already run. Retain
+ * only the inert admin/control owners and their PCI reservation.
+ */
+ pci_dev_get(pdev);
+ pci_clear_master(pdev);
+ pci_disable_device(pdev);
+ pci_set_drvdata(pdev, NULL);
+ dev_err(dev, "Control DMA unresolved; retaining resources until reboot\n");
+ return err;
+ }
vnic_dev_unregister(enic->vdev);
err_out_iounmap:
enic_iounmap(enic);
@@ -3397,6 +3456,9 @@ static void enic_remove(struct pci_dev *pdev)
disable_work_sync(&enic->reset);
disable_work_sync(&enic->tx_hang_reset);
disable_work_sync(&enic->change_mtu_work);
+ rtnl_lock();
+ netif_device_detach(netdev);
+ rtnl_unlock();
/* Close the admin channel and unregister from the PF before
* unregister_netdev() to prevent a late PF notification from
@@ -3429,9 +3491,25 @@ static void enic_remove(struct pci_dev *pdev)
pci_disable_sriov(pdev);
}
#endif
+ if (enic_admin_channel_is_quarantined(enic))
+ enic_admin_channel_recheck_on_remove(enic);
enic_dev_deinit(enic);
vnic_dev_close(enic->vdev);
kfree(enic->pp);
+ if (enic_admin_channel_is_quarantined(enic)) {
+ /* All generic queues and CPU entry points followed ordinary
+ * teardown. No admin buffer can return from the network stack.
+ * Pin the PCI object for the reservation's owner-name pointer;
+ * this reference does not fence DMA or preserve IOMMU state.
+ */
+ pci_dev_get(pdev);
+ pci_clear_master(pdev);
+ pci_disable_device(pdev);
+ pci_set_drvdata(pdev, NULL);
+ dev_err(&pdev->dev,
+ "Control DMA unresolved; retaining resources until reboot\n");
+ return;
+ }
vnic_dev_unregister(enic->vdev);
enic_iounmap(enic);
pci_release_regions(pdev);
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index 5c93ca49552a..e106ee7d411d 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -150,6 +150,7 @@ static int enic_mbox_send_msg_id(struct enic *enic, u8 msg_type,
* Mark the channel unusable so no further sends are attempted.
*/
WRITE_ONCE(enic->mbox_send_disabled, true);
+ WRITE_ONCE(enic->mbox_tx_poisoned, true);
}
netdev_dbg(enic->netdev,
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net v4 2/2] enic: require DEVCMD2 and set the V2 VF mailbox source
2026-11-01 19:07 [PATCH net v4 0/2] enic: retain admin DMA resources and identify V2 VFs Satish Kharat
2026-11-01 19:07 ` [PATCH net v4 1/2] enic: retain admin-channel DMA until queues stop Satish Kharat
@ 2026-11-01 19:07 ` Satish Kharat
1 sibling, 0 replies; 5+ messages in thread
From: Satish Kharat @ 2026-11-01 19:07 UTC (permalink / raw)
To: netdev
Cc: Sesidhar Baddela, Aravind Dhanasekaran, Reese Faucette,
Satish Kharat, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
V2 VFs currently transmit mailbox source zero. A deployed async PF
checks that field against the sender identity obtained from CQ metadata,
so VFs other than VF zero cannot register with that PF.
Require DEVCMD2 for V2 PF support and V2 VF probe, preserving the
DEVCMD1 fallback for ordinary PF Ethernet, V1 and usNIC. Enable the
admin QP, then query CMD_SRIOV_STATS_GET while its receive interrupt is
still masked. Validate the returned version and zero-based VF index, and
publish that index as the mailbox source. Permit sends before unmasking
the interrupt so queued notifications can be processed and acknowledged
with a valid source. Sends remain disabled until the ID is valid,
without a separate unknown-ID send check or reliance on PF
retransmission. Query failures use the partial-open close path and its
existing ownership checks.
Keep the coherent statistics target for the vNIC lifetime. Extend the
DEVCMD2 transport with an optional per-call observation of NOT_POSTED,
POSTED or COMPLETED. A posted statistics command without a result leaves
its target unresolved. A later blocking QP or close command result
fences that target through command ordering, including a firmware error
result. Observe close completion in both probe unwind and removal so a
completed close does not leave the statistics target unnecessarily
retained. Pre-submission errors, posted timeouts and NOWAIT posting do
not establish completion. This keeps transport completion separate from
command success and admin WQ/RQ queue-stop proof.
On an unresolved probe failure retain the inert control owner and target
through final containment instead of freeing memory which the command
may still write. Refuse reuse of an unresolved statistics target.
Fixes: 1f0c856b5963 ("enic: add MBOX core send and receive for admin channel")
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 5 ++
drivers/net/ethernet/cisco/enic/enic_admin.c | 51 ++++++++++++++--
drivers/net/ethernet/cisco/enic/enic_dev.c | 44 ++++++++++++++
drivers/net/ethernet/cisco/enic/enic_dev.h | 3 +
drivers/net/ethernet/cisco/enic/enic_main.c | 29 +++++++--
drivers/net/ethernet/cisco/enic/enic_mbox.c | 12 +++-
drivers/net/ethernet/cisco/enic/vnic_dev.c | 85 ++++++++++++++++++++++++++-
drivers/net/ethernet/cisco/enic/vnic_dev.h | 15 +++++
drivers/net/ethernet/cisco/enic/vnic_devcmd.h | 34 +++++++++++
9 files changed, 263 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 35127c549e64..9dfaddbcecf3 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -271,6 +271,11 @@ struct enic {
u16 num_vfs;
enum enic_vf_type vf_type;
bool vf_registered;
+ u16 vf_id;
+ /* A CMD_SRIOV_STATS_GET timeout leaves its persistent coherent target
+ * device-owned until a later serialized command completes.
+ */
+ bool sriov_stats_dma_unresolved;
u32 pf_cap_version;
unsigned int enable_count;
spinlock_t enic_api_lock;
diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c
index 4325c6d9c30e..acd3bf29fdcc 100644
--- a/drivers/net/ethernet/cisco/enic/enic_admin.c
+++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
@@ -16,6 +16,7 @@
#include "vnic_devcmd.h"
#include "enic.h"
#include "enic_admin.h"
+#include "enic_dev.h"
#include "cq_desc.h"
#include "cq_enet_desc.h"
#include "wq_enet_desc.h"
@@ -401,12 +402,23 @@ static void enic_admin_teardown_intr(struct enic *enic)
static int enic_admin_qp_type_set(struct enic *enic, u32 enable)
{
+ enum vnic_devcmd2_status status;
u64 a0 = QP_TYPE_ADMIN, a1 = enable;
int wait = 1000;
int err;
spin_lock_bh(&enic->devcmd_lock);
- err = vnic_dev_cmd(enic->vdev, CMD_QP_TYPE_SET, &a0, &a1, wait);
+ if (vnic_dev_uses_devcmd2(enic->vdev)) {
+ err = vnic_dev_cmd2_status(enic->vdev, CMD_QP_TYPE_SET,
+ &a0, &a1, wait, &status);
+ /* Ordered completion fences the earlier statistics DMA target,
+ * including an error result. Admin queues still need RUNNING == 0.
+ */
+ if (status == VNIC_DEVCMD2_COMPLETED)
+ WRITE_ONCE(enic->sriov_stats_dma_unresolved, false);
+ } else {
+ err = vnic_dev_cmd(enic->vdev, CMD_QP_TYPE_SET, &a0, &a1, wait);
+ }
spin_unlock_bh(&enic->devcmd_lock);
return err;
@@ -547,6 +559,8 @@ int enic_admin_channel_open(struct enic *enic)
"Refusing to reopen poisoned admin channel\n");
return -EIO;
}
+ if (READ_ONCE(enic->sriov_stats_dma_unresolved))
+ return -EBUSY;
/* Keep MBOX sends disabled for the entire open sequence. It is
* cleared only after every resource is allocated and enabled below,
* so any early error return here leaves sends disabled and a
@@ -595,15 +609,40 @@ int enic_admin_channel_open(struct enic *enic)
goto disable_queues;
}
- vnic_intr_unmask(&enic->admin_intr);
+ if (enic_is_sriov_vf_v2(enic) &&
+ READ_ONCE(enic->vf_id) == ENIC_MBOX_DST_PF) {
+ struct vnic_sriov_stats *stats;
+ u32 version;
+ u16 vf_id;
+
+ /* Keep received notifications queued until their ACK can use
+ * the VF source ID. The statistics query needs QP_ENABLE first.
+ */
+ err = enic_dev_sriov_stats(enic, &stats);
+ if (err) {
+ netdev_err(enic->netdev,
+ "Failed to get SR-IOV statistics: %d\n", err);
+ goto disable_queues;
+ }
+ version = le32_to_cpu(stats->ver);
+ vf_id = le16_to_cpu(stats->vf_index);
+ if (version != VNIC_SRIOV_STATS_VERSION ||
+ vf_id == ENIC_MBOX_DST_PF) {
+ netdev_err(enic->netdev,
+ "Invalid SR-IOV statistics version %u VF index %#x\n",
+ version, vf_id);
+ err = -EINVAL;
+ goto disable_queues;
+ }
+ WRITE_ONCE(enic->vf_id, vf_id);
+ }
/* Only now that the admin WQ/RQ/CQ and interrupt are fully allocated,
- * programmed and enabled is it safe to allow MBOX sends. Clearing this
- * earlier opened a window where a concurrent sender (e.g. link-notify
- * work scheduled by a post-reset link-up) could call enic_mbox_send_msg()
- * against a not-yet-allocated admin_wq and crash.
+ * programmed and enabled is it safe to allow MBOX sends. Publish send
+ * readiness before unmasking so queued notifications can be acknowledged.
*/
WRITE_ONCE(enic->mbox_send_disabled, false);
+ vnic_intr_unmask(&enic->admin_intr);
netdev_dbg(enic->netdev,
"admin channel open: intr=%u wq_avail=%u rq_avail=%u cq0_color=%u cq1_color=%u\n",
diff --git a/drivers/net/ethernet/cisco/enic/enic_dev.c b/drivers/net/ethernet/cisco/enic/enic_dev.c
index 659787f73cf1..7d5298e6df02 100644
--- a/drivers/net/ethernet/cisco/enic/enic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/enic_dev.c
@@ -171,6 +171,50 @@ int enic_dev_intr_coal_timer_info(struct enic *enic)
return err;
}
+int enic_dev_close(struct enic *enic)
+{
+ enum vnic_devcmd2_status status;
+ u64 a0 = 0, a1 = 0;
+ int err;
+
+ spin_lock_bh(&enic->devcmd_lock);
+ if (vnic_dev_uses_devcmd2(enic->vdev)) {
+ err = vnic_dev_cmd2_status(enic->vdev, CMD_CLOSE,
+ &a0, &a1, 1000, &status);
+ /* A close result also fences earlier statistics DMA, including
+ * an error result. Admin queues still need RUNNING == 0.
+ */
+ if (status == VNIC_DEVCMD2_COMPLETED)
+ WRITE_ONCE(enic->sriov_stats_dma_unresolved, false);
+ } else {
+ err = vnic_dev_close(enic->vdev);
+ }
+ spin_unlock_bh(&enic->devcmd_lock);
+
+ return err;
+}
+
+int enic_dev_sriov_stats(struct enic *enic,
+ struct vnic_sriov_stats **stats)
+{
+ enum vnic_devcmd2_status status;
+ int err;
+
+ *stats = NULL;
+ spin_lock_bh(&enic->devcmd_lock);
+ if (READ_ONCE(enic->sriov_stats_dma_unresolved)) {
+ err = -EBUSY;
+ goto unlock;
+ }
+ err = vnic_dev_sriov_stats(enic->vdev, stats, &status);
+ if (status == VNIC_DEVCMD2_POSTED)
+ WRITE_ONCE(enic->sriov_stats_dma_unresolved, true);
+unlock:
+ spin_unlock_bh(&enic->devcmd_lock);
+
+ return err;
+}
+
/* rtnl lock is held */
int enic_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
{
diff --git a/drivers/net/ethernet/cisco/enic/enic_dev.h b/drivers/net/ethernet/cisco/enic/enic_dev.h
index 698d0cb02064..eed1674ff30f 100644
--- a/drivers/net/ethernet/cisco/enic/enic_dev.h
+++ b/drivers/net/ethernet/cisco/enic/enic_dev.h
@@ -40,6 +40,9 @@ int enic_dev_set_ig_vlan_rewrite_mode(struct enic *enic);
int enic_dev_enable(struct enic *enic);
int enic_dev_disable(struct enic *enic);
int enic_dev_intr_coal_timer_info(struct enic *enic);
+int enic_dev_close(struct enic *enic);
+int enic_dev_sriov_stats(struct enic *enic,
+ struct vnic_sriov_stats **stats);
int enic_dev_status_to_errno(int devcmd_status);
#endif /* _ENIC_DEV_H_ */
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 04e4248d4872..d08822559152 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -2203,6 +2203,12 @@ static bool enic_has_admin_chan(struct enic *enic)
(enic_sriov_enabled(enic) && enic->vf_type == ENIC_VF_TYPE_V2);
}
+static bool enic_control_dma_unresolved(struct enic *enic)
+{
+ return enic_admin_channel_is_quarantined(enic) ||
+ READ_ONCE(enic->sriov_stats_dma_unresolved);
+}
+
/* Re-establish the admin/MBOX channel after a reset has re-created the data
* path. Mirrors the relevant part of the probe / SR-IOV-enable sequence:
* reinitialise MBOX and reopen the channel, then for a VF re-run the PF
@@ -2888,6 +2894,12 @@ static void enic_sriov_detect_vf_type(struct enic *enic)
if (enic->vf_type != ENIC_VF_TYPE_V2)
return;
+ if (!vnic_dev_uses_devcmd2(enic->vdev)) {
+ dev_warn(&pdev->dev,
+ "SR-IOV V2 requires DEVCMD2; disabling V2 support\n");
+ enic->vf_type = ENIC_VF_TYPE_NONE;
+ return;
+ }
/* A successful command means firmware recognizes
* VIC_FEATURE_SRIOV; supported_versions is available
@@ -3072,6 +3084,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
enic = netdev_priv(netdev);
enic->netdev = netdev;
enic->pdev = pdev;
+ WRITE_ONCE(enic->vf_id, ENIC_MBOX_DST_PF);
spin_lock_init(&enic->vf_link_state_lock);
/* Setup PCI resources
@@ -3139,6 +3152,12 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto err_out_vnic_unregister;
+ if (enic_is_sriov_vf_v2(enic) &&
+ !vnic_dev_uses_devcmd2(enic->vdev)) {
+ dev_err(dev, "SR-IOV V2 VF requires DEVCMD2\n");
+ err = -EOPNOTSUPP;
+ goto err_out_vnic_unregister;
+ }
#ifdef CONFIG_PCI_IOV
enic_sriov_detect_vf_type(enic);
@@ -3267,6 +3286,8 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
"MBOX VF registration failed: %d\n", err);
goto err_out_admin_close;
}
+ dev_info(dev, "V2 VF registered, VF index %u\n",
+ READ_ONCE(enic->vf_id));
}
netif_set_real_num_tx_queues(netdev, enic->wq_count);
@@ -3411,7 +3432,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err_out_dev_deinit:
enic_dev_deinit(enic);
err_out_dev_close:
- vnic_dev_close(enic->vdev);
+ enic_dev_close(enic);
err_out_disable_sriov:
kfree(enic->pp);
err_out_disable_sriov_pp:
@@ -3422,7 +3443,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
#endif
err_out_vnic_unregister:
- if (enic_admin_channel_is_quarantined(enic)) {
+ if (enic_control_dma_unresolved(enic)) {
/* Normal generic cleanup and CMD_CLOSE have already run. Retain
* only the inert admin/control owners and their PCI reservation.
*/
@@ -3494,9 +3515,9 @@ static void enic_remove(struct pci_dev *pdev)
if (enic_admin_channel_is_quarantined(enic))
enic_admin_channel_recheck_on_remove(enic);
enic_dev_deinit(enic);
- vnic_dev_close(enic->vdev);
+ enic_dev_close(enic);
kfree(enic->pp);
- if (enic_admin_channel_is_quarantined(enic)) {
+ if (enic_control_dma_unresolved(enic)) {
/* All generic queues and CPU entry points followed ordinary
* teardown. No admin buffer can return from the network stack.
* Pin the PCI object for the reservation's owner-name pointer;
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index e106ee7d411d..1716da29eee7 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -18,10 +18,17 @@
#define ENIC_MBOX_POLL_TIMEOUT_US 5000000
#define ENIC_MBOX_POLL_INTERVAL_US 100
-static void enic_mbox_fill_hdr(struct enic_mbox_hdr *hdr, u8 msg_type,
+static void enic_mbox_fill_hdr(struct enic *enic, struct enic_mbox_hdr *hdr,
+ u8 msg_type,
u16 dst_vnic_id, u16 msg_len, u64 msg_num)
{
+ u16 src_vnic_id = ENIC_MBOX_DST_PF;
+
+ if (enic_is_sriov_vf_v2(enic))
+ src_vnic_id = READ_ONCE(enic->vf_id);
+
memset(hdr, 0, sizeof(*hdr));
+ hdr->src_vnic_id = cpu_to_le16(src_vnic_id);
hdr->dst_vnic_id = cpu_to_le16(dst_vnic_id);
hdr->msg_type = msg_type;
hdr->msg_len = cpu_to_le16(msg_len);
@@ -85,7 +92,8 @@ static int enic_mbox_send_msg_id(struct enic *enic, u8 msg_type,
reply_expected = true;
}
- enic_mbox_fill_hdr(buf, msg_type, dst_vnic_id, total_len, msg_num);
+ enic_mbox_fill_hdr(enic, buf, msg_type, dst_vnic_id, total_len,
+ msg_num);
if (payload_len) {
void *dst = buf + sizeof(struct enic_mbox_hdr);
diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c b/drivers/net/ethernet/cisco/enic/vnic_dev.c
index c8d657e97094..92eec1455ceb 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
@@ -287,8 +287,9 @@ static int _vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
return -ETIMEDOUT;
}
-static int _vnic_dev_cmd2(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
- int wait)
+static int _vnic_dev_cmd2_status(struct vnic_dev *vdev,
+ enum vnic_devcmd_cmd cmd, int wait,
+ enum vnic_devcmd2_status *status)
{
struct devcmd2_controller *dc2c = vdev->devcmd2;
struct devcmd2_result *result;
@@ -298,6 +299,9 @@ static int _vnic_dev_cmd2(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
u32 fetch_index, new_posted;
u32 posted = dc2c->posted;
+ if (status)
+ *status = VNIC_DEVCMD2_NOT_POSTED;
+
fetch_index = ioread32(&dc2c->wq_ctrl->fetch_index);
if (fetch_index == 0xFFFFFFFF)
@@ -326,6 +330,8 @@ static int _vnic_dev_cmd2(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
wmb();
iowrite32(new_posted, &dc2c->wq_ctrl->posted_index);
dc2c->posted = new_posted;
+ if (status)
+ *status = VNIC_DEVCMD2_POSTED;
if (dc2c->cmd_ring[posted].flags & DEVCMD2_FNORESULT)
return 0;
@@ -340,7 +346,10 @@ static int _vnic_dev_cmd2(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
}
for (delay = 0; delay < wait; delay++) {
- if (result->color == color) {
+ if (READ_ONCE(result->color) == color) {
+ dma_rmb();
+ if (status)
+ *status = VNIC_DEVCMD2_COMPLETED;
if (result->error) {
err = result->error;
if (err != ERR_ECMDUNKNOWN ||
@@ -363,6 +372,31 @@ static int _vnic_dev_cmd2(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
return -ETIMEDOUT;
}
+static int _vnic_dev_cmd2(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
+ int wait)
+{
+ return _vnic_dev_cmd2_status(vdev, cmd, wait, NULL);
+}
+
+int vnic_dev_cmd2_status(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
+ u64 *a0, u64 *a1, int wait,
+ enum vnic_devcmd2_status *status)
+{
+ int err;
+
+ *status = VNIC_DEVCMD2_NOT_POSTED;
+ if (!vdev->devcmd2 || vdev->proxy != PROXY_NONE)
+ return -EOPNOTSUPP;
+
+ memset(vdev->args, 0, sizeof(vdev->args));
+ vdev->args[0] = *a0;
+ vdev->args[1] = *a1;
+ err = _vnic_dev_cmd2_status(vdev, cmd, wait, status);
+ *a0 = vdev->args[0];
+ *a1 = vdev->args[1];
+ return err;
+}
+
static int vnic_dev_init_devcmd1(struct vnic_dev *vdev)
{
vdev->devcmd = vnic_dev_get_res(vdev, RES_TYPE_DEVCMD, 0);
@@ -1046,6 +1080,11 @@ void vnic_dev_unregister(struct vnic_dev *vdev)
dma_free_coherent(&vdev->pdev->dev,
sizeof(struct vnic_stats),
vdev->stats, vdev->stats_pa);
+ if (vdev->sriov_stats)
+ dma_free_coherent(&vdev->pdev->dev,
+ sizeof(struct vnic_sriov_stats),
+ vdev->sriov_stats,
+ vdev->sriov_stats_pa);
if (vdev->fw_info)
dma_free_coherent(&vdev->pdev->dev,
sizeof(struct vnic_devcmd_fw_info),
@@ -1088,6 +1127,11 @@ struct pci_dev *vnic_dev_get_pdev(struct vnic_dev *vdev)
}
EXPORT_SYMBOL(vnic_dev_get_pdev);
+bool vnic_dev_uses_devcmd2(struct vnic_dev *vdev)
+{
+ return vdev->devcmd2;
+}
+
int vnic_devcmd_init(struct vnic_dev *vdev)
{
void __iomem *res;
@@ -1295,3 +1339,38 @@ int vnic_dev_capable_rss_hash_type(struct vnic_dev *vdev, u8 *rss_hash_type)
return 0;
}
+
+int vnic_dev_sriov_stats(struct vnic_dev *vdev,
+ struct vnic_sriov_stats **stats,
+ enum vnic_devcmd2_status *status)
+{
+ u64 a0, a1;
+ int err;
+
+ *stats = NULL;
+ *status = VNIC_DEVCMD2_NOT_POSTED;
+ if (!vdev->sriov_stats) {
+ vdev->sriov_stats =
+ dma_alloc_coherent(&vdev->pdev->dev,
+ sizeof(*vdev->sriov_stats),
+ &vdev->sriov_stats_pa,
+ GFP_ATOMIC);
+ if (!vdev->sriov_stats)
+ return -ENOMEM;
+ }
+
+ memset(vdev->sriov_stats, 0, sizeof(*vdev->sriov_stats));
+ a0 = vdev->sriov_stats_pa;
+ a1 = sizeof(*vdev->sriov_stats);
+ err = vnic_dev_cmd2_status(vdev, CMD_SRIOV_STATS_GET, &a0, &a1,
+ 1000, status);
+ if (!err) {
+ /* Order CPU reads from the coherent target after command
+ * completion.
+ */
+ dma_rmb();
+ *stats = vdev->sriov_stats;
+ }
+
+ return err;
+}
diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.h b/drivers/net/ethernet/cisco/enic/vnic_dev.h
index 7fdd8c661c99..6a4d7ce3b636 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_dev.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_dev.h
@@ -93,6 +93,8 @@ struct vnic_dev {
dma_addr_t linkstatus_pa;
struct vnic_stats *stats;
dma_addr_t stats_pa;
+ struct vnic_sriov_stats *sriov_stats;
+ dma_addr_t sriov_stats_pa;
struct vnic_devcmd_fw_info *fw_info;
dma_addr_t fw_info_pa;
enum vnic_proxy_type proxy;
@@ -106,6 +108,12 @@ struct vnic_dev {
struct vnic_stats;
+enum vnic_devcmd2_status {
+ VNIC_DEVCMD2_NOT_POSTED,
+ VNIC_DEVCMD2_POSTED,
+ VNIC_DEVCMD2_COMPLETED,
+};
+
void *vnic_dev_priv(struct vnic_dev *vdev);
unsigned int vnic_dev_get_res_count(struct vnic_dev *vdev,
enum vnic_res_type type);
@@ -118,6 +126,9 @@ void vnic_dev_free_desc_ring(struct vnic_dev *vdev,
struct vnic_dev_ring *ring);
int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
u64 *a0, u64 *a1, int wait);
+int vnic_dev_cmd2_status(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
+ u64 *a0, u64 *a1, int wait,
+ enum vnic_devcmd2_status *status);
void vnic_dev_cmd_proxy_by_index_start(struct vnic_dev *vdev, u16 index);
void vnic_dev_cmd_proxy_end(struct vnic_dev *vdev);
int vnic_dev_fw_info(struct vnic_dev *vdev,
@@ -163,6 +174,7 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
void *priv, struct pci_dev *pdev, struct vnic_dev_bar *bar,
unsigned int num_bars);
struct pci_dev *vnic_dev_get_pdev(struct vnic_dev *vdev);
+bool vnic_dev_uses_devcmd2(struct vnic_dev *vdev);
int vnic_dev_init_prov2(struct vnic_dev *vdev, u8 *buf, u32 len);
int vnic_dev_enable2(struct vnic_dev *vdev, int active);
int vnic_dev_enable2_done(struct vnic_dev *vdev, int *status);
@@ -177,5 +189,8 @@ int vnic_dev_overlay_offload_cfg(struct vnic_dev *vdev, u8 overlay,
int vnic_dev_get_supported_feature_ver(struct vnic_dev *vdev, u8 feature,
u64 *supported_versions, u64 *a1);
int vnic_dev_capable_rss_hash_type(struct vnic_dev *vdev, u8 *rss_hash_type);
+int vnic_dev_sriov_stats(struct vnic_dev *vdev,
+ struct vnic_sriov_stats **stats,
+ enum vnic_devcmd2_status *status);
#endif /* _VNIC_DEV_H_ */
diff --git a/drivers/net/ethernet/cisco/enic/vnic_devcmd.h b/drivers/net/ethernet/cisco/enic/vnic_devcmd.h
index 90ca06691ebd..9f397df88c50 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_devcmd.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_devcmd.h
@@ -461,6 +461,14 @@ enum vnic_devcmd_cmd {
* in: (u32) a1 = enable (1) / disable (0)
*/
CMD_QP_TYPE_SET = _CMDC(_CMD_DIR_WRITE, _CMD_VTYPE_ENET, 97),
+
+ /*
+ * Get SR-IOV capabilities, resource usage, and this function's logical
+ * VF index.
+ * in: (u64) a0 = DMA address of struct vnic_sriov_stats
+ * in: (u32) a1 = length of the buffer
+ */
+ CMD_SRIOV_STATS_GET = _CMDC(_CMD_DIR_WRITE, _CMD_VTYPE_ENET, 98),
};
#define QP_TYPE_ADMIN 0
@@ -468,6 +476,32 @@ enum vnic_devcmd_cmd {
#define QP_ENABLE 1
#define QP_DISABLE 0
+#define VNIC_SRIOV_STATS_VERSION 1
+
+struct vnic_sriov_stats {
+ __le32 ver;
+ u8 sriov_vlan_membership_cap;
+ u8 sriov_vlan_membership_enabled;
+ u8 sriov_rss_vf_full_cap;
+ u8 sriov_host_rx_stats;
+ __le32 ig_classifier0_tcam_cfg;
+ __le32 ig_classifier0_tcam_free;
+ __le32 eg_classifier0_tcam_cfg;
+ __le32 eg_classifier0_tcam_free;
+ __le32 ig_classifier1_tcam_cfg;
+ __le32 ig_classifier1_tcam_free;
+ __le32 eg_classifier1_tcam_cfg;
+ __le32 eg_classifier1_tcam_free;
+ __le32 sriov_ig_flow_table_cfg;
+ __le32 sriov_ig_flow_table_free;
+ __le32 sriov_eg_flow_table_cfg;
+ __le32 sriov_eg_flow_table_free;
+ u8 admin_qp_ready[32];
+ __le16 vf_index;
+ __le16 reserved1;
+ __le32 reserved2[256 - 23];
+};
+
/* CMD_ENABLE2 flags */
#define CMD_ENABLE2_STANDBY 0x0
#define CMD_ENABLE2_ACTIVE 0x1
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v4 1/2] enic: retain admin-channel DMA until queues stop
@ 2026-11-02 19:08 ` netdev-bot+sashiko
2026-11-03 12:27 ` Satish Kharat
0 siblings, 1 reply; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-11-02 19:08 UTC (permalink / raw)
To: satishkh
Cc: netdev, sebaddel, aradhana, rfaucett, andrew+netdev, davem,
edumazet, kuba, pabeni
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 0 (1) · Medium: 2 · Low: 1
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] When probe fails, enic_probe() keeps a quarantined admin graph
permanently without ever re-checking the admin queues.
- [Medium] The new admin disable helpers accept only RUNNING == 0 as
proof that the queue stopped: `read_poll_timeout(ioread32, running,…
- [Low] The 'Refusing to reopen poisoned admin channel' (-EIO) branch in
enic_admin_channel_open() can never run.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is severe
and sits on a path the patch edits.
--
enic: retain admin-channel DMA until queues stop
This commit tracks the admin channel as DOWN, LIVE or QUARANTINED. It
keeps the admin WQ/RQ/CQ allocations when the admin queues cannot be shown
to have stopped (RUNNING == 0) after disable. Reset workers close the
netdev instead of reopening a quarantined channel. Probe failure and remove
keep the netdev, vnic_dev, admin DMA and BAR reservation until reboot when
the queues still cannot be shown to have stopped.
> diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c
> index 61c82b48044d0..4325c6d9c30e3 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_admin.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
[ ... ]
> @@ -530,11 +531,22 @@ static void enic_admin_msg_drain(struct enic *enic)
>
> int enic_admin_channel_open(struct enic *enic)
> {
> + int close_err;
> int err;
>
> if (!enic->has_admin_channel)
> return -ENODEV;
> -
> + if (READ_ONCE(enic->admin_chan_state) != ENIC_ADMIN_CHAN_DOWN) {
> + netdev_err(enic->netdev,
> + "Refusing to open non-down admin channel (%u)\n",
> + READ_ONCE(enic->admin_chan_state));
> + return -EBUSY;
> + }
> + if (READ_ONCE(enic->mbox_tx_poisoned)) {
> + netdev_err(enic->netdev,
> + "Refusing to reopen poisoned admin channel\n");
> + return -EIO;
> + }
[Severity: Low]
Can this mbox_tx_poisoned check ever fire?
mbox_tx_poisoned appears to be set in only two places. One is
enic_mbox_send_msg_id() on a send timeout, which needs a LIVE channel. The
other is enic_admin_channel_close() when it moves to QUARANTINED.
Both transitions back to DOWN clear the flag under mbox_lock. These are the
tail of enic_admin_channel_close() and
enic_admin_channel_recheck_on_remove():
enic_admin_channel_release(enic);
WRITE_ONCE(enic->mbox_tx_poisoned, false);
mutex_unlock(&enic->mbox_lock);
Close takes mbox_lock before it decides, so an in-flight sender's timeout
is always resolved first. That means mbox_tx_poisoned should already be
false whenever the admin_chan_state check just above passes.
The field comment in enic.h ("A send timed out or channel shutdown left
queue ownership unresolved.") and the commit message present this flag as
a separate guard. Is there a path that leaves the state DOWN with the flag
still set? If not, can the flag and this -EIO branch be dropped?
[ ... ]
> -void enic_admin_channel_close(struct enic *enic)
> +static int enic_admin_wq_disable(struct enic *enic)
> {
[ ... ]
> + struct vnic_wq *wq = &enic->admin_wq;
> + u32 running;
>
> - netdev_dbg(enic->netdev, "admin channel close\n");
> + iowrite32(0, &wq->ctrl->enable);
> + return read_poll_timeout(ioread32, running, !running, 10, 500000,
> + false, &wq->ctrl->running);
> +}
[Severity: Medium]
What happens here if the function has been surprise removed, has lost its
link, or is DPC-contained? In those cases ioread32() of wq->ctrl->running
returns 0xFFFFFFFF, and this poll treats that as still running. The poll in
enic_admin_rq_disable() behaves the same way.
On remove of such a V2 VF the sequence looks like this:
enic_remove()
enic_mbox_vf_unregister() /* times out */
enic_admin_channel_close()
enic_admin_wq_disable() /* 2 x 500 ms, -ETIMEDOUT */
enic_admin_rq_disable() /* 2 x 500 ms, -ETIMEDOUT */
admin_chan_state = QUARANTINED
enic_admin_channel_recheck_on_remove() /* fails again */
"Control DMA unresolved; retaining resources until reboot"
This leaks the unregistered netdev, the vnic_dev, the BAR mappings, the
admin DMA rings and buffers, the region reservation and a pci_dev
reference. Before this patch the timeout only produced a warning and the
graph was freed.
A function that is gone cannot master the bus. The driver already treats
all-ones as "device gone" elsewhere, for example in _vnic_dev_cmd2():
if (fetch_index == 0xFFFFFFFF)
return -ENODEV;
Should these helpers check for an all-ones read, or consult
pci_channel_offline() or pci_dev_is_disconnected(), before deciding that
queue ownership is unresolved?
[ ... ]
> diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
> index 6b5824f45a14c..04e4248d48725 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
[ ... ]
> @@ -2287,7 +2311,8 @@ static void enic_reset(struct work_struct *work)
> enic_dev_set_ig_vlan_rewrite_mode(enic);
> enic_ext_cq(enic);
>
> - enic_open(enic->netdev);
> + if (enic_open(enic->netdev))
> + goto reset_out;
[Severity: High]
This isn't a bug introduced by this patch, but this path is being changed
here. What state is the netdev left in when enic_open() fails?
enic_stop() has already disabled NAPI, freed the IRQs, destroyed the page
pools and unset the notify buffer. enic_open() can fail in
enic_request_intr(), enic_dev_notify_set(), page_pool_create(), the RQ
fill or enic_dev_enable(). Its error unwind leaves NAPI disabled and those
resources freed.
The goto reset_out only skips the admin reopen. __LINK_STATE_START stays
set, so the device still looks running.
The next ndo_stop then runs enic_stop() a second time. That can come from
a user ifdown, dev_close(), or unregister_netdev() in enic_remove():
enic_stop()
for (i = 0; i < enic->rq_count; i++)
napi_disable(&enic->napi[i]);
napi_disable_locked() then waits on a context that is already disabled:
while (val & (NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC)) {
usleep_range(20, 200);
val = READ_ONCE(n->state);
}
The first disable left SCHED/NPSVC set. Can this loop forever while
holding RTNL and the netdev instance lock?
enic_tx_hang_reset() has the same exit after its enic_open() call. The
baseline ignored enic_open()'s return value and had the same double stop.
This patch handles the admin side of a failed open but leaves the datapath
state unchanged.
Should this failure path record that the datapath is down, so that a later
enic_stop() does not run against it again?
[ ... ]
> @@ -3374,6 +3422,17 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> }
> #endif
> err_out_vnic_unregister:
> + if (enic_admin_channel_is_quarantined(enic)) {
> + /* Normal generic cleanup and CMD_CLOSE have already run. Retain
> + * only the inert admin/control owners and their PCI reservation.
> + */
> + pci_dev_get(pdev);
> + pci_clear_master(pdev);
> + pci_disable_device(pdev);
> + pci_set_drvdata(pdev, NULL);
> + dev_err(dev, "Control DMA unresolved; retaining resources until reboot\n");
> + return err;
> + }
> vnic_dev_unregister(enic->vdev);
[Severity: Medium]
Should this path recheck the admin queues before keeping everything until
reboot, the way enic_remove() does?
enic_remove() runs the recheck after SR-IOV teardown and keeps resources
only if the RUNNING check still fails:
if (enic_admin_channel_is_quarantined(enic))
enic_admin_channel_recheck_on_remove(enic);
Here the quarantined case returns immediately. That skips
vnic_dev_unregister(), enic_iounmap(), pci_release_regions() and
free_netdev(). Since drvdata is NULL and the driver never binds,
enic_remove() never runs for this instance. The "final remove-time
recheck" promised by the enic.h comment on admin_chan_state therefore
never happens:
/* QUARANTINED retains admin DMA until the final remove-time recheck. */
Two V2 VF probe failures reach this point. One is enic_admin_channel_open()
failing after LIVE, with the close in disable_queues timing out. The other
is err_out_admin_close after a failed capability check, registration,
set_mac_addr or register_netdev(), again with the close timing out.
The admin queues may stop later, for example during enic_dev_deinit() or
CMD_CLOSE in the unwind. This path would still leak the netdev, vnic_dev,
BAR mappings, admin DMA rings and buffers and a pci_dev reference. It also
keeps the BAR reservation, so a rebind fails with -EBUSY until reboot. The
remove path would have released the same state.
The commit message only describes the remove-time recheck. The next patch
in the series, "enic: require DEVCMD2 and set the V2 VF mailbox source",
widens this condition to enic_control_dma_unresolved() but still adds no
recheck.
Is the weaker policy at probe time intended?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261101-b4-enic-dma-vf-index-net-generic-first-v1-20261010-v4-0-a7e209210453%40cisco.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v4 1/2] enic: retain admin-channel DMA until queues stop
2026-11-02 19:08 ` netdev-bot+sashiko
@ 2026-11-03 12:27 ` Satish Kharat
0 siblings, 0 replies; 5+ messages in thread
From: Satish Kharat @ 2026-11-03 12:27 UTC (permalink / raw)
To: netdev-bot+sashiko
Cc: netdev, sebaddel, aradhana, rfaucett, andrew+netdev, davem,
edumazet, kuba, pabeni
Thanks for the review.
I agree that mbox_tx_poisoned does not represent an independent state in
this series. The field came from the follow-on SR-IOV mailbox-recovery
work, where a posted-send timeout must be distinguished while the admin
channel is still LIVE. In this two-patch fixes series, unresolved queue
ownership is already represented by admin_chan_state being QUARANTINED,
while mbox_send_disabled blocks new sends. V5 therefore removes
mbox_tx_poisoned. I will introduce that state with the follow-on recovery
patch that actually needs and consumes it.
I also agree that probe failure should make a final queue check after
ordinary teardown has had an opportunity to stop the queues. V5 runs
generic teardown and CMD_CLOSE before the final direct admin WQ/RQ
check in both probe unwind and removal. It releases the admin graph if
both queues have stopped and otherwise retains it.
I do not plan to treat an all-ones MMIO read or a recoverable PCI
channel-offline state as proof that DMA has stopped. Those observations
show that the queue registers are inaccessible, but do not establish
completion of previously issued DMA. V5 keeps the conservative
retention policy and documents it explicitly.
The failed-reset/reopen followed by a repeated datapath stop is a valid
pre-existing lifecycle issue. It is separate from admin DMA ownership,
so I will address it independently with fault injection at the
enic_open() failure points followed by administrative close and device
removal.
I have made these changes in V5 and will post the revision shortly.
pw-bot: cr
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-03 12:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-01 19:07 [PATCH net v4 0/2] enic: retain admin DMA resources and identify V2 VFs Satish Kharat
2026-11-01 19:07 ` [PATCH net v4 1/2] enic: retain admin-channel DMA until queues stop Satish Kharat
2026-11-02 19:08 ` netdev-bot+sashiko
2026-11-03 12:27 ` Satish Kharat
2026-11-01 19:07 ` [PATCH net v4 2/2] enic: require DEVCMD2 and set the V2 VF mailbox source Satish Kharat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox