Re: [PATCH net-next v3 1/2] net: stmmac: Introduce group arguments for enable/disable_dma_irq

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: stmmac: hardcoded generic OTHER masks break variant-specific IRQs
--

commit 6b53d33b472c115af2d55abc75c5663ee8048b05
Author: Daniel Thompson <daniel@riscstar.com>

net: stmmac: Introduce group arguments for enable/disable_dma_irq

The enable_dma_irq/disable_dma_irq functions take two booleans, rx and tx,
that identify which interrupts should be disabled. Replace these with a single
bitflag argument, group. This allows a third group to be added which is used to
enable/disable other sources of DMA interrupt.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> index a0249715fafaf..7c5881ca61e8b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
[ ... ]
> @@ -109,30 +109,39 @@ void dwmac4_set_rx_ring_len(struct stmmac_priv *priv, void __iomem *ioaddr,
>  	writel(len, ioaddr + DMA_CHAN_RX_RING_LEN(dwmac4_addrs, chan));
>  }
>  
> +static u32 dwmac4_enable_dma_mask(u32 group)
> +{
> +	u32 mask = 0;
> +
> +	if (group & STMMAC_DMA_IRQ_RX)
> +		mask |= DMA_CHAN_INTR_ENA_RIE;
> +	if (group & STMMAC_DMA_IRQ_TX)
> +		mask |= DMA_CHAN_INTR_ENA_TIE;
> +	if (group & STMMAC_DMA_IRQ_OTHER)
> +		mask |= DMA_CHAN_INTR_DEFAULT_MASK &
> +			~(DMA_CHAN_INTR_ENA_RIE | DMA_CHAN_INTR_ENA_TIE);
> +
> +	return mask;
> +}

[Severity: High]
Does hardcoding DMA_CHAN_INTR_DEFAULT_MASK here cause a regression for
hardware variants that require different bit layouts for summary and abnormal
interrupts?

For example, 4.10a uses a different mask DMA_CHAN_INTR_DEFAULT_MASK_4_10
during channel initialization, where AIE is BIT 14 rather than BIT 15. Since
the generic mask is used here, an attempt to disable STMMAC_DMA_IRQ_OTHER
interrupts could fail to clear the AIE bit on 4.10a hardware, potentially
causing an interrupt storm or use-after-free if unmasked interrupts fire
during XDP ring reconfiguration.

[ ... ]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
> index a0383f9486c2d..d746527198546 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
[ ... ]
> +static u32 dwmac_enable_dma_mask(u32 group)
> +{
> +	u32 mask = 0;
> +
> +	if (group & STMMAC_DMA_IRQ_RX)
> +		mask |= DMA_INTR_DEFAULT_RX;
> +	if (group & STMMAC_DMA_IRQ_TX)
> +		mask |= DMA_INTR_DEFAULT_TX;
> +	if (group & STMMAC_DMA_IRQ_OTHER)
> +		mask |= DMA_INTR_DEFAULT_MASK &
> +			~(DMA_INTR_DEFAULT_RX | DMA_INTR_DEFAULT_TX);

[Severity: High]
Could this hardcoded mask also cause a regression for Loongson MACs?

Loongson hardware uses a custom mask layout DMA_INTR_DEFAULT_MASK_LOONGSON
because it has specific bits like NIS_TX (BIT 18) and NIS_RX (BIT 17). This
generic mask does not cover those bits, meaning they might remain enabled
when attempting to mask STMMAC_DMA_IRQ_OTHER interrupts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-tc956x-stmmac-no_irq_teardown-v3-0-8189870be545@riscstar.com?part=1