Re: [PATCH net v3 2/2] net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986

From: AI Reviewer <[email protected]>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
--- Patch [2]: [PATCH net v3 2/2] net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986 ---
commit 30cd2d1bc8bf876e5d201d631684431920a3f165
Author: Zhi-Jun You <[email protected]>

net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986

This commit fixes an issue on non-DBDC MT7986 where WDMA_RING_TX(0) is
never set up because idx is 1, even though it is required for WED RX to
function. It addresses this by explicitly calling mtk_wed_wdma_tx_ring_setup
if the WED version is v2 and rx_ring[0] is unallocated.

Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
[ ... ]
> @@ -2334,6 +2334,13 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
>  		if (!dev->rx_wdma[i].desc)
>  			mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);

[Severity: High]
This is a pre-existing issue, but does this code ignore the return value
from mtk_wed_wdma_rx_ring_setup()? 

If mtk_wed_ring_alloc() fails due to memory pressure, it returns an error
without configuring the hardware base registers. Because the error is
ignored, when DMA is later enabled in mtk_wed_start(), could the hardware
attempt to fetch from unmapped physical addresses and cause an IOMMU fault?

>  
> +	/*
> +	 * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
> +	 * but tx_wdma[0] is also needed for WED to function.
> +	 */
> +	if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)
> +		mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);

[Severity: High]
Does this new call to mtk_wed_wdma_tx_ring_setup() also need to check its
return value? 

Similar to the RX ring above, if mtk_wed_ring_alloc() fails to allocate
memory, the hardware base registers won't be configured. Since the caller
proceeds to enable hardware DMA regardless, might this lead to the WED
processing descriptors from unmapped memory and causing memory corruption
or a system crash?

> +
>  	if (dev->wlan.hw_rro) {
>  		for (i = 0; i < MTK_WED_RX_PAGE_QUEUES; i++) {
-- 
This is an AI-generated review.