[PATCH net v8] net: libwx: fix VF PM suspend/resume flow

[PATCH net v8] net: libwx: fix VF PM suspend/resume flow

[Mengyuan Lou]

The shared PM callbacks used by txgbevf and ngbevf free queue vectors and
disable PCI without first stopping a running VF or draining service
activity. Resume does not re-enable PCI or restore the queues and IRQs of
a running VF. Service work can therefore access torn-down resources, and a
running interface is not restored correctly after resume.

Stop a running VF under RTNL before freeing its interrupt resources,
preserving its administrative running state. Drain the service timer and
work before disabling PCI.

Re-enable PCI on resume and defer PF readiness checks, VF reset and
interrupt setup to the service task. Retry readiness and reset failures
while keeping the device detached. Reopen the interface only if it is
still administratively running. If interrupt setup or reopening fails,
stop recovery, close the interface if necessary, and leave the device
detached with PCI disabled.

Track freed resources and PCI enable state to avoid duplicate teardown.
Reject ring-parameter changes while queue vectors are unavailable.

When backporting to kernels that do not have commit e73e4d187a1f52 ("net:
wangxun: add pcie error handler"), add WX_STATE_DISABLED and
WX_STATE_RES_FREED to enum wx_state before WX_STATE_NBITS. The PCIe
error-handler implementation from that commit is not needed.

Fixes: 377d180bd71c ("net: wangxun: add txgbevf build")
Cc: stable@vger.kernel.org
Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
---
Changelogs:
v8:
- Rename the goto labels in wxvf_reopen_subtask() to describe the action
  taken on failure, and merge the duplicate open-failure and hard-failure
  labels into a single err_disable_device label.
v7: https://lore.kernel.org/netdev/20261029104220.77344-1-mengyuanlou@net-swift.com/
- Skip the deferred reopen once the netdev is no longer registered, so a
  pending reopen cannot race with unregister_netdev() during removal.
- Correct the v5 changelog: the reopen is driven by the service timer,
  not scheduled immediately from resume.
v6: https://lore.kernel.org/netdev/20261019060216.38458-1-mengyuanlou@net-swift.com/
- Clarify the enum dependency for stable backports to kernels without the
  PCIe error-handler commit.
- Keep API negotiation and MAC refresh after reset errors for existing
  callers, while returning the reset error to the resume retry path.
v5: https://lore.kernel.org/netdev/20261012080913.49444-1-mengyuanlou@net-swift.com/
- Keep the deferred reopen pending while the PF is not ready, so the
  service timer retries it instead of giving up on the first failure.
- Serialize the PF mailbox probe with mbx_lock, send a firmware-version
  request, and propagate VF reset failures.
- Serialize link updates with RTNL and skip them while reopening is pending.
- Track PCI and resource state to avoid duplicate teardown. Use dev_close()
  on reopen failure and reject ring-parameter changes without queue vectors.
- Retarget the patch to net and add Fixes and Cc: stable tags.
v4: https://lore.kernel.org/netdev/20260908095801.61346-1-mengyuanlou@net-swift.com/
- Refactor patch description and subject to focus on PM flow rework for
  net-next instead of bug fixes, removing the Fixes tag.
- Unconditionally restore the interrupt scheme in wxvf_reopen_subtask()
  to fix NULL msix_entry dereference on previously admin-DOWN interfaces.
- Add WX_FLAG_NEED_REOPEN guard in wxvf_link_config_subtask() to block
  mailbox and register operations before reopening completes.
- Validate netif_running() under rtnl_lock() in wxvf_reopen_subtask()
  before invoking wxvf_open() to respect administrative DOWN events
  during deferral.
- Synchronize core netdev flags (__LINK_STATE_START and IFF_UP) and
  trigger DOWN notifications on wxvf_reopen_subtask() failure paths.
- Flush the service timer and work synchronously during suspend to avoid
  tasks running after PCI device teardown.
v3: https://lore.kernel.org/netdev/20260904085216.43469-1-mengyuanlou@net-swift.com/
- Fix DMA-after-free and UAF races in wxvf_suspend() by invoking
  wxvf_close() when running, ensuring DMA engines are stopped and
  NAPI/IRQs/rings are cleanly freed before clearing PCI state.
- Replace manual resource allocation in wxvf_resume() with deferred
  reopening via WX_FLAG_NEED_REOPEN, scheduling wxvf_reopen_subtask() to
  run full wxvf_open() sequence under rtnl_lock().
- Move timer/work synchronization after rtnl_lock() release in
  wxvf_suspend() to prevent service tasks from re-arming the timer.
- Add error unwinding in wxvf_reopen_subtask() to set WX_STATE_DOWN,
  clear bus mastering, and log errors if reopen fails.
- Skip wxvf_link_config_subtask() when WX_STATE_DOWN is set to prevent
  link updates on downed interfaces.
v2: https://lore.kernel.org/netdev/20260829091423.83097-1-mengyuanlou@net-swift.com/
- Refactored the suspend and resume logic to eliminate full netdevice
  close/open cycles in favor of lightweight interrupt and queue manipulation:
  * In wxvf_suspend(), replaced wxvf_close() with explicit queue stopping
    (netif_tx_disable), carrier drop, NAPI disabling (wx_napi_disable_all), IRQ
    releasing (wx_free_irq), and resource freeing (wx_free_resources).
  * In wxvf_resume(), replaced wxvf_open() with granular resource allocation
    (wx_setup_resources), MSI-X IRQ requesting (wx_request_msix_irqs_vf),
    and deferred hardware reconfiguration via WX_FLAG_NEED_DO_RESET flag.
- Dropped the addition of device_link_add() to parent PF in ngbevf and txgbevf
  probe() paths to keep the patch focused strictly on libwx PM suspend/resume flow.
v1: https://lore.kernel.org/netdev/20260826095243.16939-1-mengyuanlou@net-swift.com/
---
 .../net/ethernet/wangxun/libwx/wx_ethtool.c   |   3 +
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   |   3 +-
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |   1 +
 drivers/net/ethernet/wangxun/libwx/wx_vf.c    |   4 +-
 drivers/net/ethernet/wangxun/libwx/wx_vf.h    |   2 +
 .../net/ethernet/wangxun/libwx/wx_vf_common.c | 123 ++++++++++++++++--
 .../net/ethernet/wangxun/libwx/wx_vf_common.h |   2 +-
 7 files changed, 126 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
index 940d2e59876c..729b1ec65bd7 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
@@ -790,6 +790,9 @@ static int wx_set_ringparam_vf(struct net_device *netdev,
 	struct wx_ring *temp_ring;
 	int i, err = 0;
 
+	if (!wx->num_q_vectors)
+		return -EBUSY;
+
 	new_tx_count = clamp_t(u32, ring->tx_pending, WX_MIN_TXD, WX_MAX_TXD);
 	new_tx_count = ALIGN(new_tx_count, WX_REQ_TX_DESCRIPTOR_MULTIPLE);
 
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index dcbf5811046e..701d239ab13c 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -3402,7 +3402,8 @@ EXPORT_SYMBOL(wx_set_ring);
 
 void wx_service_event_schedule(struct wx *wx)
 {
-	if (!test_bit(WX_STATE_DOWN, wx->state) &&
+	if ((!test_bit(WX_STATE_DOWN, wx->state) ||
+	     test_bit(WX_FLAG_NEED_REOPEN, wx->flags)) &&
 	    !test_and_set_bit(WX_STATE_SERVICE_SCHED, wx->state))
 		queue_work(system_power_efficient_wq, &wx->service_task);
 }
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index a7f3c48e9603..66b21bde962e 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -1288,6 +1288,7 @@ enum wx_flags {
 	WX_FLAG_NEED_MODULE_RESET,
 	WX_FLAG_NEED_UPDATE_LINK,
 	WX_FLAG_NEED_DO_RESET,
+	WX_FLAG_NEED_REOPEN,
 	WX_FLAG_RX_MERGE_ENABLED,
 	WX_FLAG_TXHEAD_WB_ENABLED,
 	WX_FLAG_NEED_PCIE_RECOVERY,
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf.c b/drivers/net/ethernet/wangxun/libwx/wx_vf.c
index 7567216a005f..213d13d26070 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_vf.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_vf.c
@@ -38,8 +38,8 @@ void wx_init_hw_vf(struct wx *wx)
 }
 EXPORT_SYMBOL(wx_init_hw_vf);
 
-static int wx_mbx_write_and_read_reply(struct wx *wx, u32 *req_buf,
-				       u32 *resp_buf, u16 size)
+int wx_mbx_write_and_read_reply(struct wx *wx, u32 *req_buf,
+				u32 *resp_buf, u16 size)
 {
 	int ret;
 
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf.h b/drivers/net/ethernet/wangxun/libwx/wx_vf.h
index b64a4de089f2..e8001d5856a9 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_vf.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_vf.h
@@ -109,6 +109,8 @@ struct wx_link_reg_fields {
 };
 
 void wx_init_hw_vf(struct wx *wx);
+int wx_mbx_write_and_read_reply(struct wx *wx, u32 *req_buf,
+				u32 *resp_buf, u16 size);
 int wx_reset_hw_vf(struct wx *wx);
 void wx_get_mac_addr_vf(struct wx *wx, u8 *mac_addr);
 void wx_stop_adapter_vf(struct wx *wx);
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
index 26de78e9a69e..e274a77b3be9 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
@@ -16,9 +16,23 @@ int wxvf_suspend(struct device *dev_d)
 	struct pci_dev *pdev = to_pci_dev(dev_d);
 	struct wx *wx = pci_get_drvdata(pdev);
 
+	rtnl_lock();
 	netif_device_detach(wx->netdev);
+	if (netif_running(wx->netdev))
+		wxvf_close(wx->netdev);
+
+	clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);
 	wx_clear_interrupt_scheme(wx);
-	pci_disable_device(pdev);
+	set_bit(WX_STATE_RES_FREED, wx->state);
+	rtnl_unlock();
+
+	timer_delete_sync(&wx->service_timer);
+	cancel_work_sync(&wx->service_task);
+	clear_bit(WX_STATE_SERVICE_SCHED, wx->state);
+
+	pci_clear_master(pdev);
+	if (!test_and_set_bit(WX_STATE_DISABLED, wx->state))
+		pci_disable_device(pdev);
 
 	return 0;
 }
@@ -34,11 +48,20 @@ int wxvf_resume(struct device *dev_d)
 {
 	struct pci_dev *pdev = to_pci_dev(dev_d);
 	struct wx *wx = pci_get_drvdata(pdev);
+	int err;
 
-	pci_set_master(pdev);
-	wx_init_interrupt_scheme(wx);
-	netif_device_attach(wx->netdev);
+	err = pci_enable_device_mem(pdev);
+	if (err) {
+		dev_err(&pdev->dev, "Cannot enable PCI device from suspend\n");
+		return err;
+	}
 
+	clear_bit(WX_STATE_DISABLED, wx->state);
+	pci_set_master(pdev);
+	rtnl_lock();
+	mod_timer(&wx->service_timer, jiffies + HZ);
+	set_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+	rtnl_unlock();
 	return 0;
 }
 EXPORT_SYMBOL(wxvf_resume);
@@ -52,13 +75,15 @@ void wxvf_remove(struct pci_dev *pdev)
 	unregister_netdev(netdev);
 	timer_shutdown_sync(&wx->service_timer);
 	cancel_work_sync(&wx->service_task);
+	clear_bit(WX_STATE_SERVICE_SCHED, wx->state);
 	kfree(wx->vfinfo);
 	kfree(wx->rss_key);
 	kfree(wx->mac_table);
 	wx_clear_interrupt_scheme(wx);
 	pci_release_selected_regions(pdev,
 				     pci_select_bars(pdev, IORESOURCE_MEM));
-	pci_disable_device(pdev);
+	if (!test_and_set_bit(WX_STATE_DISABLED, wx->state))
+		pci_disable_device(pdev);
 }
 EXPORT_SYMBOL(wxvf_remove);
 
@@ -137,7 +162,7 @@ void wx_negotiate_api_vf(struct wx *wx)
 }
 EXPORT_SYMBOL(wx_negotiate_api_vf);
 
-void wx_reset_vf(struct wx *wx)
+int wx_reset_vf(struct wx *wx)
 {
 	struct net_device *netdev = wx->netdev;
 	int ret = 0;
@@ -150,6 +175,8 @@ void wx_reset_vf(struct wx *wx)
 		eth_hw_addr_set(netdev, wx->mac.addr);
 		ether_addr_copy(netdev->perm_addr, wx->mac.addr);
 	}
+
+	return ret;
 }
 EXPORT_SYMBOL(wx_reset_vf);
 
@@ -279,6 +306,7 @@ void wxvf_up_complete(struct wx *wx)
 	wx_configure_msix_vf(wx);
 	smp_mb__before_atomic();
 	clear_bit(WX_STATE_DOWN, wx->state);
+	clear_bit(WX_STATE_RES_FREED, wx->state);
 	wx_napi_enable_all(wx);
 
 	/* clear any pending interrupts, may auto mask */
@@ -376,6 +404,9 @@ int wxvf_close(struct net_device *netdev)
 {
 	struct wx *wx = netdev_priv(netdev);
 
+	if (test_and_set_bit(WX_STATE_RES_FREED, wx->state))
+		return 0;
+
 	wxvf_down(wx);
 	wx_free_irq(wx);
 	wx_free_resources(wx);
@@ -388,25 +419,101 @@ static void wxvf_link_config_subtask(struct wx *wx)
 {
 	struct net_device *netdev = wx->netdev;
 
+	rtnl_lock();
+	if (test_bit(WX_STATE_DOWN, wx->state) ||
+	    test_bit(WX_FLAG_NEED_REOPEN, wx->flags))
+		goto out;
+
 	wxvf_watchdog_update_link(wx);
 	if (wx->link) {
 		if (netif_carrier_ok(netdev))
-			return;
+			goto out;
 		netif_carrier_on(netdev);
 		netdev_info(netdev, "Link is Up - %s\n",
 			    phy_speed_to_str(wx->speed));
 	} else {
 		if (!netif_carrier_ok(netdev))
-			return;
+			goto out;
 		netif_carrier_off(netdev);
 		netdev_info(netdev, "Link is Down\n");
 	}
+
+out:
+	rtnl_unlock();
+}
+
+static void wxvf_reopen_subtask(struct wx *wx)
+{
+	u32 ping[2] = { WX_VF_GET_FW_VERSION };
+	int ret;
+
+	if (!test_bit(WX_FLAG_NEED_REOPEN, wx->flags))
+		return;
+
+	rtnl_lock();
+	if (!test_bit(WX_FLAG_NEED_REOPEN, wx->flags) ||
+	    test_bit(WX_STATE_DISABLED, wx->state) ||
+	    wx->netdev->reg_state != NETREG_REGISTERED) {
+		rtnl_unlock();
+		return;
+	}
+
+	/* Serialize the ping with other VF mailbox accesses.
+	 * The helper sends the request and waits for the PF reply.
+	 */
+	spin_lock_bh(&wx->mbx.mbx_lock);
+	ret = wx_mbx_write_and_read_reply(wx, ping, ping, ARRAY_SIZE(ping));
+	spin_unlock_bh(&wx->mbx.mbx_lock);
+	if (ret) {
+		rtnl_unlock();
+		return;
+	}
+
+	ret = wx_reset_vf(wx);
+	if (ret) {
+		rtnl_unlock();
+		return;
+	}
+
+	if (wx_init_interrupt_scheme(wx)) {
+		dev_err(&wx->pdev->dev, "Failed to initialize interrupt scheme\n");
+		goto err_disable_device;
+	}
+
+	if (netif_running(wx->netdev) && wxvf_open(wx->netdev))
+		goto err_disable_device;
+
+	clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+	if (!netif_running(wx->netdev))
+		timer_delete_sync(&wx->service_timer);
+	netif_device_attach(wx->netdev);
+	rtnl_unlock();
+	return;
+err_disable_device:
+	/* Do not retry interrupt setup or reopen from the service task. Keep the
+	 * netdev detached and down, and disable PCI.
+	 * Retry by rebinding the driver or through a later PM resume; bring the
+	 * interface up after recovery.
+	 */
+	timer_delete_sync(&wx->service_timer);
+	wx_clear_interrupt_scheme(wx);
+	clear_bit(WX_FLAG_NEED_REOPEN, wx->flags);
+	set_bit(WX_STATE_DOWN, wx->state);
+
+	if (netif_running(wx->netdev))
+		dev_close(wx->netdev);
+
+	rtnl_unlock();
+	if (!test_and_set_bit(WX_STATE_DISABLED, wx->state))
+		pci_disable_device(wx->pdev);
+	dev_err(&wx->pdev->dev, "Failed to fully resume device\n");
 }
 
 static void wxvf_service_task(struct work_struct *work)
 {
 	struct wx *wx = container_of(work, struct wx, service_task);
 
+	wxvf_reopen_subtask(wx);
 	wxvf_link_config_subtask(wx);
 	wxvf_reset_subtask(wx);
 	wx_service_event_complete(wx);
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.h b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.h
index d45d5d8ac3ab..5bfeab6fdde6 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.h
@@ -10,7 +10,7 @@ int wxvf_resume(struct device *dev_d);
 void wxvf_remove(struct pci_dev *pdev);
 int wx_request_msix_irqs_vf(struct wx *wx);
 void wx_negotiate_api_vf(struct wx *wx);
-void wx_reset_vf(struct wx *wx);
+int wx_reset_vf(struct wx *wx);
 void wx_set_rx_mode_vf(struct net_device *netdev);
 void wx_configure_vf(struct wx *wx);
 int wx_set_mac_vf(struct net_device *netdev, void *p);
-- 
2.30.1

Re: [PATCH net v8] net: libwx: fix VF PM suspend/resume flow

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v8] net: libwx: fix VF PM suspend/resume flow

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · 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] wxvf_link_config_subtask()
  (drivers/net/ethernet/wangxun/libwx/wx_vf_common.c) now calls…
- [Low] The new `if (!wx->num_q_vectors) return -EBUSY;` guard in
  wx_set_ringparam_vf()…
- [Low] The new PF-readiness probe in wxvf_reopen_subtask() checks only
  the return value of wx_mbx_write_and_read_reply().

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the new resume
  recovery path depends on it.

--

net: libwx: fix VF PM suspend/resume flow

This commit reworks the shared txgbevf/ngbevf PM callbacks. On suspend, a
running VF is now closed under RTNL, and the service timer and work are
drained before PCI is disabled. On resume, PCI is re-enabled and the PF
readiness check, VF reset, interrupt setup and reopen are deferred to the
service task.

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> index 940d2e59876c7..729b1ec65bd75 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> @@ -790,6 +790,9 @@ static int wx_set_ringparam_vf(struct net_device *netdev,
>  	struct wx_ring *temp_ring;
>  	int i, err = 0;
>  
> +	if (!wx->num_q_vectors)
> +		return -EBUSY;
> +

[Severity: Low]
Can this check in wx_set_ringparam_vf() ever be reached? Both ethtool entry
points reject a detached device before they call into the driver:

net/ethtool/ioctl.c:__dev_ethtool() {
    ...
	if (!netif_device_present(dev)) {
		rc = -ENODEV;
		goto out;
	}
    ...
}

net/ethtool/netlink.c:ethnl_ops_begin() {
    ...
	if (!netif_device_present(dev) ||
	    dev->reg_state >= NETREG_UNREGISTERING) {
		ret = -ENODEV;
    ...
}

As far as I can tell, the netdev stays detached under RTNL whenever
num_q_vectors is zero:

- wxvf_suspend() detaches before wx_clear_interrupt_scheme().
- wxvf_resume() does not reattach.
- wxvf_reopen_subtask() calls netif_device_attach() only after
  wx_init_interrupt_scheme() and wxvf_open() succeed.
- The err_disable_device path leaves the device detached.

The VF drivers also don't register a PCI err_handler that could reattach
the device.

wx_get_coalesce() and wx_set_coalesce(), in the same wx_ethtool_ops_vf
table, don't have this guard and dereference q_vector[0] directly:

	if (wx->q_vector[0]->tx.count && wx->q_vector[0]->rx.count)

If the state can't be reached, should the check be dropped, along with the
commit message line "Reject ring-parameter changes while queue vectors are
unavailable."? If it can be reached, wouldn't the coalesce ops need the
same guard?

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
> index 26de78e9a69ef..e274a77b3be9d 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_vf_common.c

[ ... ]

> @@ -388,25 +419,101 @@ static void wxvf_link_config_subtask(struct wx *wx)
>  {
>  	struct net_device *netdev = wx->netdev;
>  
> +	rtnl_lock();
> +	if (test_bit(WX_STATE_DOWN, wx->state) ||
> +	    test_bit(WX_FLAG_NEED_REOPEN, wx->flags))
> +		goto out;
> +
>  	wxvf_watchdog_update_link(wx);

[Severity: Medium]
Does this make every running VF take the global RTNL mutex every two
seconds, even when there is nothing to do?

wxvf_link_config_subtask() now takes rtnl_lock() before it checks any of
WX_STATE_DOWN, WX_FLAG_NEED_REOPEN or WX_FLAG_NEED_UPDATE_LINK. Meanwhile
wx_service_timer() re-arms itself every 2 * HZ:

wx_service_timer()
    mod_timer(&wx->service_timer, next_event_offset + jiffies);
    wx_service_event_schedule()
        -> wxvf_service_task()
            -> wxvf_link_config_subtask()
                -> rtnl_lock()

wx_msix_misc_vf() sets WX_FLAG_NEED_UPDATE_LINK on every misc interrupt,
and wxvf_up_complete() sets it too. When it is set,
wxvf_watchdog_update_link() now runs its mailbox exchange under RTNL. That
exchange holds mbx_lock with BH disabled and polls with
readx_poll_timeout_atomic().

wxvf_reopen_subtask() behaves the same way while a reopen is pending and
the PF does not answer. It takes RTNL every two seconds with no back-off or
retry limit, and each time it busy-waits on the GET_FW_VERSION ping.

Could the lockless flag checks be done before taking RTNL?

Also, this change to steady-state link polling is only mentioned in the v5
changelog below the "---" line, so it won't appear in git history. Would it
be worth describing it in the commit message?

[ ... ]

> +static void wxvf_reopen_subtask(struct wx *wx)
> +{
> +	u32 ping[2] = { WX_VF_GET_FW_VERSION };
> +	int ret;

[ ... ]

> +	spin_lock_bh(&wx->mbx.mbx_lock);
> +	ret = wx_mbx_write_and_read_reply(wx, ping, ping, ARRAY_SIZE(ping));
> +	spin_unlock_bh(&wx->mbx.mbx_lock);
> +	if (ret) {
> +		rtnl_unlock();
> +		return;
> +	}

[Severity: Low]
Is a zero return here enough to conclude that the PF is ready? ping[0] is
never checked for a WX_VF_GET_FW_VERSION reply with WX_VT_MSGTYPE_ACK or
WX_VT_MSGTYPE_NACK set.

If VF MMIO reads return all-ones, every poll in the mailbox transport
succeeds at once:

- wx_obtain_mbx_lock_vf() sees WX_VXMAILBOX_VFU.
- wx_poll_for_ack() sees WX_VXMAILBOX_PFACK.
- wx_poll_for_msg() sees WX_VXMAILBOX_PFSTS.
- wx_read_mbx_vf() copies 0xffffffff into ping[0] and returns 0.

wx_reset_vf() then writes to the unresponsive function. It only fails later,
with -EINVAL from the reply check in wx_reset_hw_vf():

	if (msgbuf[0] != (WX_VF_RESET | WX_VT_MSGTYPE_ACK) &&
	    msgbuf[0] != (WX_VF_RESET | WX_VT_MSGTYPE_NACK))
		return -EINVAL;

Would validating ping[0] in the same way, for example checking that
(ping[0] & 0xFFFF) == WX_VF_GET_FW_VERSION with ACK or NACK set, make this
a real readiness check?

[ ... ]

> +	if (netif_running(wx->netdev) && wxvf_open(wx->netdev))
> +		goto err_disable_device;

[Severity: High]
This isn't a bug introduced by this patch, but the new resume recovery
depends on it. wxvf_reopen_subtask() assumes wxvf_open() leaves the device
quiesced when it fails. However, the wxvf_open() error unwinding frees the
rings before it stops the hardware queues:

wxvf_open() {
    ...
err_free_irq:
	wx_free_irq(wx);
err_free_resources:
	wx_free_resources(wx);
err_reset:
	wx_reset_vf(wx);
	return err;
}

wx_configure_vf() has already enabled reception before either
wx_request_msix_irqs_vf() or netif_set_real_num_{tx,rx}_queues() can fail:

- wx_configure_rx_ring_vf() sets WX_VXRXDCTL_ENABLE, calls
  wx_enable_rx_queue() and posts DMA-mapped page_pool buffers.
- Through wx_rlpml_set_vf(), the PF has set this VF's bit in WX_RDM_VF_RE.

On failure, wx_free_resources()->wx_free_rx_resources() calls
dma_free_coherent() on the descriptor ring and then page_pool_destroy().
WX_VXRXDCTL_ENABLE is cleared only afterwards, in
wx_reset_vf()->wx_reset_hw_vf()->wx_stop_adapter_vf().

wxvf_resume() has already called pci_set_master() at this point. Can the VF
write back descriptors or received frames into the freed memory during that
window, before err_disable_device reaches pci_disable_device()?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109101442.84605-1-mengyuanlou%40net-swift.com

Re: [PATCH net v8] net: libwx: fix VF PM suspend/resume flow

[Jakub Kicinski]

On Mon, 09 Nov 2026 10:21:39 +0000 netdev-bot+sinfo@kernel.org wrote:
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
> 
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
> 
>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.

v8 and you still can't include the answers to this questions in the
commit msg? SMH

Re: [PATCH net v8] net: libwx: fix VF PM suspend/resume flow

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  9 Nov 2026 18:14:42 +0800 you wrote:
> The shared PM callbacks used by txgbevf and ngbevf free queue vectors and
> disable PCI without first stopping a running VF or draining service
> activity. Resume does not re-enable PCI or restore the queues and IRQs of
> a running VF. Service work can therefore access torn-down resources, and a
> running interface is not restored correctly after resume.
> 
> Stop a running VF under RTNL before freeing its interrupt resources,
> preserving its administrative running state. Drain the service timer and
> work before disabling PCI.
> 
> [...]

Here is the summary with links:
  - [net,v8] net: libwx: fix VF PM suspend/resume flow
    https://git.kernel.org/netdev/net-next/c/a6c3e4c1f703

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html