Re: [PATCH net-next v2 11/18] net: macb: single dma_alloc_coherent() for DMA descriptors

["Théo Lebrun" <theo.lebrun@bootlin.com>]

Hello Sean,

Thanks for the review! I'll reply only to questions (or comments about
which I have questions).

On Tue Jul 1, 2025 at 6:32 PM CEST, Sean Anderson wrote:
> On 6/27/25 05:08, Théo Lebrun wrote:
>> Move from two (Tx/Rx) dma_alloc_coherent() for DMA descriptor rings *per
>> queue* to two dma_alloc_coherent() overall.
>> 
>> Issue is with how all queues share the same register for configuring the
>> upper 32-bits of Tx/Rx descriptor rings. For example, with Tx, notice
>> how TBQPH does *not* depend on the queue index:
>> 
>> 	#define GEM_TBQP(hw_q)		(0x0440 + ((hw_q) << 2))
>> 	#define GEM_TBQPH(hw_q)		(0x04C8)
>> 
>> 	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
>> 	#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> 	if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>> 		queue_writel(queue, TBQPH, upper_32_bits(queue->tx_ring_dma));
>> 	#endif
>> 
>> To maxime our chances of getting valid DMA addresses, we do a single
>
> maximize
>
>> dma_alloc_coherent() across queues.
>
> Is there really any chance involved (other than avoiding ENOMEM)?

If we land in the the page allocator codepath of dma_alloc_coherent(),
then we get natural alignment guarantees, see alloc_pages() comment [0].

[0]: https://elixir.bootlin.com/linux/v6.16/source/mm/mempolicy.c#L2499-L2502

However, we cannot be certain we land in that path. If we have an
IOMMU, then I don't think the API provides strong enough guarantees.

Same for custom `struct dma_map_ops`, be it per-device or arch-specific.
I am not aware (is anything documented on that?) of any alignment
guarantees.

Even if those give us page-aligned allocations, that isn't enough. For
example let's say we want 256KiB. We get 0xFFFF0000 from an allocator.
That is page aligned, but:

   upper_32_bits(START)      != upper_32_bits(START + SIZE - 1)
   upper_32_bits(0xFFFF0000) != upper_32_bits(0xFFFF0000 + 0x40000 - 1)
   0x0                       != 0x1

Thanks!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com