[PATCH net] net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode

[PATCH net] net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode

[Jiawen Wu]

In txgbe_misc_irq_thread_fn(), the driver unmasks the miscellaneous
interrupt at the end of the handler using TXGBE_INTR_MISC(wx) (which
resolves to BIT(wx->num_q_vectors)). While this is correct for MSI-X
mode, it is incorrect for legacy INTx or single MSI modes.

Due to hardware behavior, the WX_PX_MISC_IVAR register is completely
ignored by the hardware when MSI-X is disabled. In non-MSI-X mode, the
hardware forcibly merges all interrupt causes (both Queue and MISC) into
a single bit: BIT(0) of the interrupt register.

Unconditionally unmasking TXGBE_INTR_MISC(wx) (e.g., BIT(1)) in non-MSI-X
mode means the actual MISC interrupt bit (BIT(0)) is not unmasked
promptly at the end of the MISC thread. Instead, it remains masked until
NAPI completes its polling and unmasks the shared BIT(0). This delays the
assertion of subsequent MISC interrupts, preventing timely handling of
events like link state changes.

Fix this by explicitly checking `pdev->msix_enabled` and falling back
to BIT(0) as the interrupt mask for the MISC cause when MSI-X is disabled.

Fixes: e37546ad1f9b ("net: wangxun: revert the adjustment of the IRQ vector sequence")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
index 8746318ad3bc..05b318f4e6ed 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
@@ -164,6 +164,7 @@ static irqreturn_t txgbe_misc_irq_thread_fn(int irq, void *data)
 	struct wx *wx = txgbe->wx;
 	unsigned int nhandled = 0;
 	unsigned int sub_irq;
+	u64 misc_mask;
 	u32 eicr;
 
 	eicr = txgbe->eicr;
@@ -183,7 +184,8 @@ static irqreturn_t txgbe_misc_irq_thread_fn(int irq, void *data)
 		nhandled++;
 	}
 
-	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
+	misc_mask = wx->pdev->msix_enabled ? TXGBE_INTR_MISC(wx) : BIT(0);
+	wx_intr_enable(wx, misc_mask);
 	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
 }
 
-- 
2.51.0

Re: [PATCH net] net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode

[Simon Horman]

On Thu, Aug 13, 2026 at 03:33:05PM +0800, Jiawen Wu wrote:
> In txgbe_misc_irq_thread_fn(), the driver unmasks the miscellaneous
> interrupt at the end of the handler using TXGBE_INTR_MISC(wx) (which
> resolves to BIT(wx->num_q_vectors)). While this is correct for MSI-X
> mode, it is incorrect for legacy INTx or single MSI modes.
> 
> Due to hardware behavior, the WX_PX_MISC_IVAR register is completely
> ignored by the hardware when MSI-X is disabled. In non-MSI-X mode, the
> hardware forcibly merges all interrupt causes (both Queue and MISC) into
> a single bit: BIT(0) of the interrupt register.
> 
> Unconditionally unmasking TXGBE_INTR_MISC(wx) (e.g., BIT(1)) in non-MSI-X
> mode means the actual MISC interrupt bit (BIT(0)) is not unmasked
> promptly at the end of the MISC thread. Instead, it remains masked until
> NAPI completes its polling and unmasks the shared BIT(0). This delays the
> assertion of subsequent MISC interrupts, preventing timely handling of
> events like link state changes.
> 
> Fix this by explicitly checking `pdev->msix_enabled` and falling back
> to BIT(0) as the interrupt mask for the MISC cause when MSI-X is disabled.
> 
> Fixes: e37546ad1f9b ("net: wangxun: revert the adjustment of the IRQ vector sequence")
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net] net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode

The patch changes the trailing unmask in txgbe_misc_irq_thread_fn() so that
it uses BIT(0) instead of TXGBE_INTR_MISC(wx) when MSI-X is not enabled,
because the hardware merges all interrupt causes into BIT(0) in MSI/INTx
mode. This is meant to avoid delaying subsequent MISC interrupts until NAPI
polling completes.

> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> index 8746318ad3bcb..05b318f4e6ed6 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c

[ ... ]

> @@ -183,7 +184,8 @@ static irqreturn_t txgbe_misc_irq_thread_fn(int irq, void *data)
>  		nhandled++;
>  	}
>  
> -	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
> +	misc_mask = wx->pdev->msix_enabled ? TXGBE_INTR_MISC(wx) : BIT(0);
> +	wx_intr_enable(wx, misc_mask);
>  	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
>  }

[Severity: Medium]
Should this unmask be guarded by WX_STATE_DOWN now that it touches the live
BIT(0) cause?

In non-MSI-X mode this write becomes a real WX_PX_IMC(0) = BIT(0), which
re-arms the single merged queue+MISC cause. Before the change the value was
BIT(wx->num_q_vectors) = BIT(1) with num_q_vectors == 1 in this mode, i.e. a
bit the hardware does not use here, so the write left no hardware residue.

wx_irq_disable() in libwx/wx_hw.c masks everything before it waits for
in-flight handlers:

void wx_irq_disable(struct wx *wx)
{
	struct pci_dev *pdev = wx->pdev;

	wr32(wx, WX_PX_MISC_IEN, 0);
	wx_intr_disable(wx, WX_INTR_ALL);

	if (pdev->msix_enabled) {
		...
	} else {
		synchronize_irq(pdev->irq);
	}
}

Can a misc thread that is already running execute its trailing
wx_intr_enable(wx, BIT(0)) after those mask writes, so that synchronize_irq()
returns with the only active cause unmasked while the driver considers
interrupts off?

The window in txgbe_disable_device() spans wx_irq_disable() through
wx_napi_disable_all(), timer_delete_sync(), cancel_work_sync() and the TX ring
shutdown, and the mask is only restored later via txgbe_reset() ->
txgbe_reset_hw() -> wx_stop_adapter(). Since the TX rings and TX DMA engine
are stopped after wx_irq_disable(), a TX completion cause could assert on the
re-unmasked BIT(0) during that window; on a shared INTx line the hard handler
then takes the eicr == 0 path, returns IRQ_NONE and does not write
WX_PX_INTA.

Every other unmask site in this driver family wraps the same operation in the
down check, for example wx_poll() in libwx/wx_lib.c:

	if (!test_bit(WX_STATE_DOWN, wx->state))
		wx_intr_enable(wx, WX_INTR_Q(q_vector->v_idx));

and ngbe_intr() in ngbe/ngbe_main.c:

	if (!test_bit(WX_STATE_DOWN, wx->state))
		ngbe_irq_enable(wx, false);

txgbe_misc_irq_handle() itself uses the guard on its shared interrupt path.
Would something like the following be more consistent?

	misc_mask = wx->pdev->msix_enabled ? TXGBE_INTR_MISC(wx) : BIT(0);
	if (!test_bit(WX_STATE_DOWN, wx->state))
		wx_intr_enable(wx, misc_mask);
-- 
pw-bot: cr

RE: [PATCH net] net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode

[Jiawen Wu]

On Tue, Aug 18, 2026 4:58 AM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: txgbe: fix interrupt mask for MISC cause in non-MSI-X mode
> 
> The patch changes the trailing unmask in txgbe_misc_irq_thread_fn() so that
> it uses BIT(0) instead of TXGBE_INTR_MISC(wx) when MSI-X is not enabled,
> because the hardware merges all interrupt causes into BIT(0) in MSI/INTx
> mode. This is meant to avoid delaying subsequent MISC interrupts until NAPI
> polling completes.
> 
> > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > index 8746318ad3bcb..05b318f4e6ed6 100644
> > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> 
> [ ... ]
> 
> > @@ -183,7 +184,8 @@ static irqreturn_t txgbe_misc_irq_thread_fn(int irq, void *data)
> >  		nhandled++;
> >  	}
> >
> > -	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
> > +	misc_mask = wx->pdev->msix_enabled ? TXGBE_INTR_MISC(wx) : BIT(0);
> > +	wx_intr_enable(wx, misc_mask);
> >  	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
> >  }
> 
> [Severity: Medium]
> Should this unmask be guarded by WX_STATE_DOWN now that it touches the live
> BIT(0) cause?
> 
> In non-MSI-X mode this write becomes a real WX_PX_IMC(0) = BIT(0), which
> re-arms the single merged queue+MISC cause. Before the change the value was
> BIT(wx->num_q_vectors) = BIT(1) with num_q_vectors == 1 in this mode, i.e. a
> bit the hardware does not use here, so the write left no hardware residue.
> 
> wx_irq_disable() in libwx/wx_hw.c masks everything before it waits for
> in-flight handlers:
> 
> void wx_irq_disable(struct wx *wx)
> {
> 	struct pci_dev *pdev = wx->pdev;
> 
> 	wr32(wx, WX_PX_MISC_IEN, 0);
> 	wx_intr_disable(wx, WX_INTR_ALL);
> 
> 	if (pdev->msix_enabled) {
> 		...
> 	} else {
> 		synchronize_irq(pdev->irq);
> 	}
> }
> 
> Can a misc thread that is already running execute its trailing
> wx_intr_enable(wx, BIT(0)) after those mask writes, so that synchronize_irq()
> returns with the only active cause unmasked while the driver considers
> interrupts off?
> 
> The window in txgbe_disable_device() spans wx_irq_disable() through
> wx_napi_disable_all(), timer_delete_sync(), cancel_work_sync() and the TX ring
> shutdown, and the mask is only restored later via txgbe_reset() ->
> txgbe_reset_hw() -> wx_stop_adapter(). Since the TX rings and TX DMA engine
> are stopped after wx_irq_disable(), a TX completion cause could assert on the
> re-unmasked BIT(0) during that window; on a shared INTx line the hard handler
> then takes the eicr == 0 path, returns IRQ_NONE and does not write
> WX_PX_INTA.
> 
> Every other unmask site in this driver family wraps the same operation in the
> down check, for example wx_poll() in libwx/wx_lib.c:
> 
> 	if (!test_bit(WX_STATE_DOWN, wx->state))
> 		wx_intr_enable(wx, WX_INTR_Q(q_vector->v_idx));
> 
> and ngbe_intr() in ngbe/ngbe_main.c:
> 
> 	if (!test_bit(WX_STATE_DOWN, wx->state))
> 		ngbe_irq_enable(wx, false);
> 
> txgbe_misc_irq_handle() itself uses the guard on its shared interrupt path.
> Would something like the following be more consistent?
> 
> 	misc_mask = wx->pdev->msix_enabled ? TXGBE_INTR_MISC(wx) : BIT(0);
> 	if (!test_bit(WX_STATE_DOWN, wx->state))
> 		wx_intr_enable(wx, misc_mask);

Right, I'll fix it in V2.