netdev - Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2cb86f7-c16a-44d2-a7b9-eb379784ff83@intel.com>
Date: Tue, 4 Jun 2024 15:25:41 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Frank Wunderlich <linux@...web.de>, Felix Fietkau <nbd@....name>, "Sean
 Wang" <sean.wang@...iatek.com>, Mark Lee <Mark-MC.Lee@...iatek.com>, "Lorenzo
 Bianconi" <lorenzo@...nel.org>, "David S. Miller" <davem@...emloft.net>,
	"Eric Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
 Abeni <pabeni@...hat.com>, Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: Frank Wunderlich <frank-w@...lic-files.de>, John Crispin
	<john@...ozen.org>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-mediatek@...ts.infradead.org>,
	<bc-bocun.chen@...iatek.com>, Daniel Golle <daniel@...rotopia.org>
Subject: Re: [net v3] net: ethernet: mtk_eth_soc: handle dma buffer size soc
 specific



On 6/3/2024 12:25 PM, Frank Wunderlich wrote:
> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct mtk_eth *eth)
>  						       cnt * soc->tx.desc_size,
>  						       &eth->phy_scratch_ring,
>  						       GFP_KERNEL);
> +
>  	if (unlikely(!eth->scratch_ring))
>  		return -ENOMEM;
>  
> -	eth->scratch_head = kcalloc(cnt, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
> -	if (unlikely(!eth->scratch_head))
> -		return -ENOMEM;
> +	phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
>  
> -	dma_addr = dma_map_single(eth->dma_dev,
> -				  eth->scratch_head, cnt * MTK_QDMA_PAGE_SIZE,
> -				  DMA_FROM_DEVICE);
> -	if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> -		return -ENOMEM;
> +	for (j = 0; j < DIV_ROUND_UP(soc->tx.fq_dma_size, MTK_FQ_DMA_LENGTH); j++) {
> +		len = min_t(int, cnt - j * MTK_FQ_DMA_LENGTH, MTK_FQ_DMA_LENGTH);
> +		eth->scratch_head[j] = kcalloc(len, MTK_QDMA_PAGE_SIZE, GFP_KERNEL);
>  
> -	phy_ring_tail = eth->phy_scratch_ring + soc->tx.desc_size * (cnt - 1);
> +		if (unlikely(!eth->scratch_head[j]))
> +			return -ENOMEM;
>  
> -	for (i = 0; i < cnt; i++) {
> -		dma_addr_t addr = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> -		struct mtk_tx_dma_v2 *txd;
> +		dma_addr = dma_map_single(eth->dma_dev,
> +					  eth->scratch_head[j], len * MTK_QDMA_PAGE_SIZE,
> +					  DMA_FROM_DEVICE);
>  
> -		txd = eth->scratch_ring + i * soc->tx.desc_size;
> -		txd->txd1 = addr;
> -		if (i < cnt - 1)
> -			txd->txd2 = eth->phy_scratch_ring +
> -				    (i + 1) * soc->tx.desc_size;
> +		if (unlikely(dma_mapping_error(eth->dma_dev, dma_addr)))
> +			return -ENOMEM;
>  
> -		txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> -		if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> -			txd->txd3 |= TX_DMA_PREP_ADDR64(addr);
> -		txd->txd4 = 0;
> -		if (mtk_is_netsys_v2_or_greater(eth)) {
> -			txd->txd5 = 0;
> -			txd->txd6 = 0;
> -			txd->txd7 = 0;
> -			txd->txd8 = 0;
> +		for (i = 0; i < cnt; i++) {
> +			struct mtk_tx_dma_v2 *txd;
> +
> +			txd = eth->scratch_ring + (j * MTK_FQ_DMA_LENGTH + i) * soc->tx.desc_size;
> +			txd->txd1 = dma_addr + i * MTK_QDMA_PAGE_SIZE;
> +			if (j * MTK_FQ_DMA_LENGTH + i < cnt)
> +				txd->txd2 = eth->phy_scratch_ring +
> +					    (j * MTK_FQ_DMA_LENGTH + i + 1) * soc->tx.desc_size;
> +
> +			txd->txd3 = TX_DMA_PLEN0(MTK_QDMA_PAGE_SIZE);
> +			if (MTK_HAS_CAPS(soc->caps, MTK_36BIT_DMA))
> +				txd->txd3 |= TX_DMA_PREP_ADDR64(dma_addr + i * MTK_QDMA_PAGE_SIZE);
> +
> +			txd->txd4 = 0;
> +			if (mtk_is_netsys_v2_or_greater(eth)) {
> +				txd->txd5 = 0;
> +				txd->txd6 = 0;
> +				txd->txd7 = 0;
> +				txd->txd8 = 0;
> +			}

This block of change was a bit hard to understand what was going on, but
I think I get the result is that you end up allocating different set of
scratch_head per size vs the original only having one scratch_head per
device?

Perhaps you can explain, but we're now allocating a bunch of different
scratch_head pointers.. However, in the patch, the only places that we
modify scratch_head appear to be the allocation path and the free path..
but I can't seem to understand how that would impact the users of
scratch head? I guess it changes the dma_addr which then changes the txd
values we program?

Ok.

I sort of understand whats going on here, but it was a fair bit to fully
grok this flow.

Overall, I'm no expert on the part or DMA here, but:

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>