Re: [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16

[Jakub Raczynski <j.raczynski@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]

I will be rewriting this patch as discussed in other thread,
but will mention that IMO this AI review is generally invalid.
Below why.

On Thu, Jun 04, 2026 at 03:16:06PM +0200, Paolo Abeni wrote:
> From: AI Reviewer <ai@example.com>
> 
> 
> [Severity: High]
> Does increasing these limits to 16 cause undefined behavior and silent register
> truncation in the hardware configuration functions when queue >= 8?
> 
> In dwxgmac2_core.c:dwxgmac2_tx_queue_prio(), when queue >= 8, the logic
> only subtracts 4:
> 
>     if (queue >= 4)
>         queue -= 4;

This ignores previous check of queue > numtc, which is hardware returning
maximum value of 8 (0-7). This is not direct 'queue > 7 ' for AI so I get it,
but this is not possible, unless hardware would return wrong value.

> 
> Finally, dwmac4_core.c:dwmac4_map_mtl_dma() only maps queues up to 7 (using
> MAP0 and MAP1):
> 
>     if (queue < 4) {
>     ...
>     } else {
>         value = readl(ioaddr + MTL_RXQ_DMA_MAP1);
>         value &= ~MTL_RXQ_DMA_QXMDMACH_MASK(queue - 4);

Correct, would probably be good to not forget about dwmac, although
GMAC does not return number of available TC's so check would be artificial.
Although no function checks that currently, will be fixed by last comment and
probably checking this stuff in upper layer.

> 
> [Severity: Critical]
> This is a pre-existing issue, but does missing bounds checking on the device
> tree properties expose an out-of-bounds array access during device probe?
> 
> In stmmac_platform.c:stmmac_mtl_setup(), the device tree property
> snps,tx-queues-to-use is parsed and clamped only to 255 (U8_MAX):
> 
>     if (!of_property_read_u32(tx_node, "snps,tx-queues-to-use", &value)) {
>         if (value > U8_MAX)
>             value = U8_MAX;
>         plat->tx_queues_to_use = value;
>     }

'Net' material, not for this patch (net-next), will probably send it later,
as this is valid

> 
> Later in stmmac_main.c:stmmac_hw_setup(), it attempts to dynamically clamp
> this value to the hardware capabilities:
> 
>     if (priv->dma_cap.number_tx_queues &&
>         priv->plat->tx_queues_to_use > priv->dma_cap.number_tx_queues) {
>         priv->plat->tx_queues_to_use = priv->dma_cap.number_tx_queues;
>     }

Well, XGMAC will correct incorrect value, so why not do the same for GMAC
even though it would be artificial > MTL_MAX_TX_QUEUES.
Same for RX. Although same as for above, probably 'net' material

BR
Jakub Raczynski

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]