Re: [PATCH net-next v6 5/5] net: wangxun: add pcie error handler

[Simon Horman <horms@kernel.org>]

On Wed, Jun 10, 2026 at 02:09:17PM +0800, Jiawen Wu wrote:
> Support AER driver to handle the PCIe errors. Sometimes netdev watchdog
> Tx timeout happens before the AER error report when a PCIe error occurs,
> CPU blocking would be caused by MMIO during the reset process. To
> prevent it, check PCIe error status in .ndo_tx_timeout. The current
> function of ngbe is not yet fully developed, it will be completed in the
> future.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Hi Jiawen,

There is AI-generated review of this patch-set available on both
https://sashiko.dev and https://netdev-ai.bots.linux.dev/sashiko/

I've tried to filter out only those that seem strictly relevant
to the progress of this patch and have included them inline.

I'd appreciate it if you could take a look over this.

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_err.c b/drivers/net/ethernet/wangxun/libwx/wx_err.c

...

> +/**
> + * wx_io_slot_reset - called after the pci bus has been reset.
> + * @pdev: Pointer to PCI device
> + *
> + * Return: pci_ers_result_t.
> + *
> + * Restart the card from scratch, as if from a cold-boot.
> + */
> +static pci_ers_result_t wx_io_slot_reset(struct pci_dev *pdev)
> +{
> +	struct wx *wx = pci_get_drvdata(pdev);
> +	pci_ers_result_t result;
> +
> +	if (pci_enable_device_mem(pdev)) {
> +		wx_err(wx, "Cannot re-enable PCI device after reset.\n");
> +		result = PCI_ERS_RESULT_DISCONNECT;
> +	} else {
> +		/* make all memory operations done before clearing the flag */
> +		smp_mb__before_atomic();
> +		clear_bit(WX_STATE_DISABLED, wx->state);
> +		clear_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags);
> +		pci_set_master(pdev);
> +		pci_restore_state(pdev);
> +		pci_wake_from_d3(pdev, false);
> +
> +		rtnl_lock();
> +		if (netif_running(wx->netdev) && wx->down_suspend)
> +			wx->down_suspend(wx);
> +		if (wx->do_reset)
> +			wx->do_reset(wx->netdev, false);

The AI-generated review on sashiko.dev flags the following.

	"Does this properly initialize the hardware MAC during PCIe AER
	 slot reset recovery for the ngbe driver?

	"For txgbe, this correctly maps to txgbe_reset(), which calls
	 txgbe_reset_hw().  However, for ngbe, wx->do_reset maps to
	 ngbe_do_reset() which calls ngbe_reset().

	"Looking at ngbe_reset():

	"drivers/net/ethernet/wangxun/ngbe/ngbe_main.c:ngbe_reset() {
	    if (test_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags))
	        return;
	    wx_flush_sw_mac_table(wx);
	    wx_mac_set_default_filter(wx, wx->mac.addr);
	    if (test_bit(WX_STATE_PTP_RUNNING, wx->state))
	        wx_ptp_reset(wx);

	"This skips ngbe_reset_hw() and fails to fully reset the hardware.
	 Since a PCIe slot reset clears the device's internal state, when
	 wx_io_resume() subsequently calls ndo_open(), could the device fail
	 to function because its hardware registers and PHY state were never
	 reset?

> +		rtnl_unlock();
> +		result = PCI_ERS_RESULT_RECOVERED;
> +	}
> +
> +	pci_aer_clear_nonfatal_status(pdev);
> +
> +	return result;
> +}

...

> +static void wx_tx_timeout_recovery(struct wx *wx)
> +{
> +	/*
> +	 * When a PCIe hardware error occurs, the driver should initiate a PCIe
> +	 * recovery mechanism. However, this recovery flow relies on the AER
> +	 * driver for current kernel policy. Therefore, a self-contained
> +	 * recovery mechanism is not implemented yet.
> +	 */
> +	set_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags);
> +	wx_err(wx, "PCIe error detected during tx timeout\n");
> +	queue_work(wx->reset_wq, &wx->reset_task);
> +}

The AI-generated review on https://netdev-ai.bots.linux.dev/sashiko/ flags:

	"What clears WX_FLAG_NEED_PCIE_RECOVERY if AER never follows up?
	 wx_tx_timeout_recovery() sets the flag and queues reset_task.
	 reset_task only calls wx_soft_quiesce() and does not clear the
	 flag:

	    if (test_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags))
	        wx_soft_quiesce(wx);

	"The flag is cleared only on the success branch of
	 wx_io_slot_reset(). If the PCIe error never escalates to AER (AER
	 disabled in firmware or kernel, error not reportable, slot_reset
	 returns PCI_ERS_RESULT_DISCONNECT), the flag remains set
	 indefinitely.

	"Both ngbe_reset() and txgbe_reset() now early-return on this flag:

	    static void ngbe_reset(struct wx *wx)
	    {
	        if (test_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags))
	            return;
	        ...
	    }

	"so any subsequent reset path (ethtool reset, ndo_set_features
	 triggered reset, watchdog timeout reset, FDIR reinit,
	 ngbe_reinit_locked / txgbe_reinit_locked) is silently a no-op. The
	 hardware reset capability is effectively disabled until the module
	 is reloaded.

	"Should wx_tx_timeout_recovery() (or reset_task after the quiesce)
	 clear the flag once the soft-quiesce completes, so the driver's
	 reset machinery remains functional when AER does not follow up?

...