Re: [PATCH v8 01/23] dma-direct: return struct page from dma_direct_alloc_from_pool()

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jul 21, 2026 at 08:40:08PM +0530, Aneesh Kumar K.V wrote:
> Leon Romanovsky <leon@kernel.org> writes:
> 
> > On Tue, Jul 21, 2026 at 07:50:10PM +0530, Aneesh Kumar K.V wrote:
> >> Leon Romanovsky <leon@kernel.org> writes:
> >> 
> >> > On Fri, Jul 17, 2026 at 11:34:19PM +0530, Aneesh Kumar K.V (Arm) wrote:
> ....
> >> >>  static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
> >> >> @@ -247,8 +246,11 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >> >>  	 * the atomic pools instead if we aren't allowed block.
> >> >>  	 */
> >> >>  	if ((remap || force_dma_unencrypted(dev)) &&
> >> >> -	    dma_direct_use_pool(dev, gfp))
> >> >> -		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
> >> >> +	    dma_direct_use_pool(dev, gfp)) {
> >> >> +		page = dma_direct_alloc_from_pool(dev, size, dma_handle,
> >> >> +						  &ret, gfp);
> >> >> +		return page ? ret : NULL;
> >> >
> >> > Sorry for joining the discussion late, but the line above caught my
> >> > attention.
> >> >
> >> > Why do we need both ret and page? We can derive cpu_addr from page and
> >> > vice versa. Do we really need the &ret parameter? Or, more generally, do
> >> > we really need "struct page *"?
> >> >
> >> > static struct page *__dma_alloc_from_pool(struct device *dev, size_t size,
> >> > 		struct gen_pool *pool, void **cpu_addr,
> >> > 		bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t))
> >> > {
> >> > ...
> >> > 	*cpu_addr = (void *)addr;
> >> > 	memset(*cpu_addr, 0, size);
> >> > 	return pfn_to_page(__phys_to_pfn(phys));
> >> > }
> >> >
> >> > Why
> >> >
> >> 
> >> With CONFIG_DMA_DIRECT_REMAP the cpu_addr can be different from
> >> page_address.
> >
> > Can you please point to the code there it can happen?
> > __dma_alloc_from_pool() has direct connection between physical address
> > and struct page.
> >
> 
> dma_direct_alloc -> 	remap = IS_ENABLED(CONFIG_DMA_DIRECT_REMAP);
> 	if ((remap && dma_direct_use_pool(dev, gfp)) {
> 		page = dma_direct_alloc_from_pool(dev, size,
> 
> ..
> __dma_alloc_from_pool -> 
> 	addr = gen_pool_alloc(pool, size);
> 	if (!addr)
> 
> 
> We expand the pool as below..
> 
> atomic_pool_expand ->
> 
> #ifdef CONFIG_DMA_DIRECT_REMAP
> 	addr = dma_common_contiguous_remap(page, pool_size,
> 			pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL)),
> 			__builtin_return_address(0));
> 	if (!addr)
> 		goto free_page;
> #else
> 	addr = page_to_virt(page);
> #endif

Right, and nothing prevents you from adding a small DMA helper
that translates mapped/direct addresses back to struct page.

Something like, but probably void* needs to be phys_addr_t:

static inline struct page *dma_phys_to_page(void *addr)
{
#ifdef CONFIG_DMA_DIRECT_REMAP
	return vmalloc_to_page(addr);
#else
	return virt_to_page(addr);
#endif
}

Architecture code already does this throughout the tree.

Thanks


> 
> 
> -aneesh
>