* [PATCH net v3 1/2] enic: retain admin-channel DMA until queues stop
2026-10-27 15:15 [PATCH net v3 0/2] enic: retain admin DMA resources and identify V2 VFs Satish Kharat
@ 2026-10-27 15:15 ` Satish Kharat
0 siblings, 0 replies; 2+ messages in thread
From: Satish Kharat @ 2026-10-27 15:15 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.
Check that an internal datapath open still has a present device. Recheck
the running state under RTNL in reset workers, and detach under RTNL
during removal before closing the channel to exclude a concurrent new
open.
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 | 86 +++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.c | 1 +
5 files changed, 250 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..5a1621599c5f 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
@@ -2264,6 +2274,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 +2286,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 +2309,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 +2319,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 +2339,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 +2349,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 +2372,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 +2382,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 +3081,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 +3420,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 +3454,8 @@ 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);
/* Close the admin channel and unregister from the PF before
* unregister_netdev() to prevent a late PF notification from
@@ -3413,6 +3472,7 @@ static void enic_remove(struct pci_dev *pdev)
}
enic_admin_channel_close(enic);
}
+ rtnl_unlock();
unregister_netdev(netdev);
/* unregister_netdev() -> enic_stop() stops the notify timer, so
@@ -3429,9 +3489,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] 2+ messages in thread