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

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

[Aneesh Kumar K.V (Arm)]

Commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool
helper") changed dma_direct_alloc_from_pool() to return the CPU address
from dma_alloc_from_pool(). That fits dma_direct_alloc(), but
dma_direct_alloc_pages() also uses the helper and expects a struct page *.

Fix this by making dma_direct_alloc_from_pool() return the struct page *
again, and pass the CPU address back through an out-parameter for the
dma_direct_alloc() caller.

Fixes: 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper")
Cc: stable@vger.kernel.org
Tested-by: Michael Kelley <mhklinux@outlook.com>
Tested-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
 kernel/dma/direct.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index d8219efe3273..363d984d90e7 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -164,22 +164,21 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
 	return !gfpflags_allow_blocking(gfp) && !is_swiotlb_for_alloc(dev);
 }
 
-static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
-		dma_addr_t *dma_handle, gfp_t gfp)
+static struct page *dma_direct_alloc_from_pool(struct device *dev, size_t size,
+		dma_addr_t *dma_handle, void **cpu_addr, gfp_t gfp)
 {
 	struct page *page;
 	u64 phys_limit;
-	void *ret;
 
 	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_COHERENT_POOL)))
 		return NULL;
 
 	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
-	page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
+	page = dma_alloc_from_pool(dev, size, cpu_addr, gfp, dma_coherent_ok);
 	if (!page)
 		return NULL;
 	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
-	return ret;
+	return page;
 }
 
 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;
+	}
 
 	/* we always manually zero the memory once we are done */
 	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
@@ -357,7 +359,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 	void *ret;
 
 	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
-		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
+		return dma_direct_alloc_from_pool(dev, size, dma_handle, &ret, gfp);
 
 	page = __dma_direct_alloc_pages(dev, size, gfp, false);
 	if (!page)
-- 
2.43.0

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

[Leon Romanovsky]

On Fri, Jul 17, 2026 at 11:34:19PM +0530, Aneesh Kumar K.V (Arm) wrote:
> Commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool
> helper") changed dma_direct_alloc_from_pool() to return the CPU address
> from dma_alloc_from_pool(). That fits dma_direct_alloc(), but
> dma_direct_alloc_pages() also uses the helper and expects a struct page *.
> 
> Fix this by making dma_direct_alloc_from_pool() return the struct page *
> again, and pass the CPU address back through an out-parameter for the
> dma_direct_alloc() caller.
> 
> Fixes: 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper")
> Cc: stable@vger.kernel.org
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>  kernel/dma/direct.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index d8219efe3273..363d984d90e7 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -164,22 +164,21 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
>  	return !gfpflags_allow_blocking(gfp) && !is_swiotlb_for_alloc(dev);
>  }
>  
> -static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> -		dma_addr_t *dma_handle, gfp_t gfp)
> +static struct page *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> +		dma_addr_t *dma_handle, void **cpu_addr, gfp_t gfp)
>  {
>  	struct page *page;
>  	u64 phys_limit;
> -	void *ret;
>  
>  	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_COHERENT_POOL)))
>  		return NULL;
>  
>  	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
> -	page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
> +	page = dma_alloc_from_pool(dev, size, cpu_addr, gfp, dma_coherent_ok);
>  	if (!page)
>  		return NULL;
>  	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
> -	return ret;
> +	return page;
>  }
>  
>  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 

> +	}
>  
>  	/* we always manually zero the memory once we are done */
>  	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
> @@ -357,7 +359,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>  	void *ret;
>  
>  	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
> -		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
> +		return dma_direct_alloc_from_pool(dev, size, dma_handle, &ret, gfp);
>  
>  	page = __dma_direct_alloc_pages(dev, size, gfp, false);
>  	if (!page)
> -- 
> 2.43.0
> 
> 

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

[Aneesh Kumar K.V]

Leon Romanovsky <leon@kernel.org> writes:

> On Fri, Jul 17, 2026 at 11:34:19PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool
>> helper") changed dma_direct_alloc_from_pool() to return the CPU address
>> from dma_alloc_from_pool(). That fits dma_direct_alloc(), but
>> dma_direct_alloc_pages() also uses the helper and expects a struct page *.
>> 
>> Fix this by making dma_direct_alloc_from_pool() return the struct page *
>> again, and pass the CPU address back through an out-parameter for the
>> dma_direct_alloc() caller.
>> 
>> Fixes: 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper")
>> Cc: stable@vger.kernel.org
>> Tested-by: Michael Kelley <mhklinux@outlook.com>
>> Tested-by: Mostafa Saleh <smostafa@google.com>
>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>>  kernel/dma/direct.c | 18 ++++++++++--------
>>  1 file changed, 10 insertions(+), 8 deletions(-)
>> 
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index d8219efe3273..363d984d90e7 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -164,22 +164,21 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
>>  	return !gfpflags_allow_blocking(gfp) && !is_swiotlb_for_alloc(dev);
>>  }
>>  
>> -static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
>> -		dma_addr_t *dma_handle, gfp_t gfp)
>> +static struct page *dma_direct_alloc_from_pool(struct device *dev, size_t size,
>> +		dma_addr_t *dma_handle, void **cpu_addr, gfp_t gfp)
>>  {
>>  	struct page *page;
>>  	u64 phys_limit;
>> -	void *ret;
>>  
>>  	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_COHERENT_POOL)))
>>  		return NULL;
>>  
>>  	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
>> -	page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
>> +	page = dma_alloc_from_pool(dev, size, cpu_addr, gfp, dma_coherent_ok);
>>  	if (!page)
>>  		return NULL;
>>  	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
>> -	return ret;
>> +	return page;
>>  }
>>  
>>  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.

-aneesh

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

[Leon Romanovsky]

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:
> >> Commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool
> >> helper") changed dma_direct_alloc_from_pool() to return the CPU address
> >> from dma_alloc_from_pool(). That fits dma_direct_alloc(), but
> >> dma_direct_alloc_pages() also uses the helper and expects a struct page *.
> >> 
> >> Fix this by making dma_direct_alloc_from_pool() return the struct page *
> >> again, and pass the CPU address back through an out-parameter for the
> >> dma_direct_alloc() caller.
> >> 
> >> Fixes: 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper")
> >> Cc: stable@vger.kernel.org
> >> Tested-by: Michael Kelley <mhklinux@outlook.com>
> >> Tested-by: Mostafa Saleh <smostafa@google.com>
> >> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> >> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> >> ---
> >>  kernel/dma/direct.c | 18 ++++++++++--------
> >>  1 file changed, 10 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> >> index d8219efe3273..363d984d90e7 100644
> >> --- a/kernel/dma/direct.c
> >> +++ b/kernel/dma/direct.c
> >> @@ -164,22 +164,21 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
> >>  	return !gfpflags_allow_blocking(gfp) && !is_swiotlb_for_alloc(dev);
> >>  }
> >>  
> >> -static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> >> -		dma_addr_t *dma_handle, gfp_t gfp)
> >> +static struct page *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> >> +		dma_addr_t *dma_handle, void **cpu_addr, gfp_t gfp)
> >>  {
> >>  	struct page *page;
> >>  	u64 phys_limit;
> >> -	void *ret;
> >>  
> >>  	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_COHERENT_POOL)))
> >>  		return NULL;
> >>  
> >>  	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
> >> -	page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
> >> +	page = dma_alloc_from_pool(dev, size, cpu_addr, gfp, dma_coherent_ok);
> >>  	if (!page)
> >>  		return NULL;
> >>  	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
> >> -	return ret;
> >> +	return page;
> >>  }
> >>  
> >>  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.

Thanks

> 
> -aneesh
> 

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

[Aneesh Kumar K.V]

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


-aneesh

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

[Leon Romanovsky]

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
> 

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

[Jason Gunthorpe]

On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote:

> 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
> }

I don't like this, we don't know for sure the addr will be in a vmap
and this will fail if it isn't.

Jason

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

[Leon Romanovsky]

On Wed, Jul 22, 2026 at 04:59:12PM -0300, Jason Gunthorpe wrote:
> On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote:
> 
> > 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
> > }
> 
> I don't like this, we don't know for sure the addr will be in a vmap
> and this will fail if it isn't.

Of course we know. The existing "#ifdef CONFIG_DMA_DIRECT_REMAP" is
relevant for addresses acquired from pool.

dma_phys_to_page() is a bad name for some low-level conversion function.
It needs to be internal to DMA logic, in the level when we convert from
phys to page.

Thanks

> 
> Jason
> 

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

[Jason Gunthorpe]

On Thu, Jul 23, 2026 at 10:57:04AM +0300, Leon Romanovsky wrote:
> On Wed, Jul 22, 2026 at 04:59:12PM -0300, Jason Gunthorpe wrote:
> > On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote:
> > 
> > > 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
> > > }
> > 
> > I don't like this, we don't know for sure the addr will be in a vmap
> > and this will fail if it isn't.
> 
> Of course we know. The existing "#ifdef CONFIG_DMA_DIRECT_REMAP" is
> relevant for addresses acquired from pool.

Yeah, but I still don't like it :) It is hard to follow if you make
those kinds of leaps, someone will call this new helper on something
they shouldn't

> dma_phys_to_page() is a bad name for some low-level conversion function.
> It needs to be internal to DMA logic, in the level when we convert from
> phys to page.

I think we should not convert from phys to page, that's also easy to
do wrong

Jason

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

[Leon Romanovsky]

On Sat, Jul 25, 2026 at 11:34:11AM -0300, Jason Gunthorpe wrote:
> On Thu, Jul 23, 2026 at 10:57:04AM +0300, Leon Romanovsky wrote:
> > On Wed, Jul 22, 2026 at 04:59:12PM -0300, Jason Gunthorpe wrote:
> > > On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote:
> > > 
> > > > 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
> > > > }
> > > 
> > > I don't like this, we don't know for sure the addr will be in a vmap
> > > and this will fail if it isn't.
> > 
> > Of course we know. The existing "#ifdef CONFIG_DMA_DIRECT_REMAP" is
> > relevant for addresses acquired from pool.
> 
> Yeah, but I still don't like it :) It is hard to follow if you make
> those kinds of leaps, someone will call this new helper on something
> they shouldn't

This issue is straightforward to address today. Limit the scope, use
an appropriate function name, and add a comment indicating that the
function is local to and specific to the pool. The latter helps
AI-based review tools flag incorrect usage outside the intended scope.

> 
> > dma_phys_to_page() is a bad name for some low-level conversion function.
> > It needs to be internal to DMA logic, in the level when we convert from
> > phys to page.
> 
> I think we should not convert from phys to page, that's also easy to
> do wrong

Perhaps I'll reiterate my complaint. DMA internals are already hard to
understand. Part of that complexity is necessary to support every
possible use case, but another part comes from a maze of types that is
entirely self-inflicted. I think this code only makes the latter worse.

Currently, the phys type universally describes memory and can be
reliably translated into any required representation. It is the most
fundamental type we have.

Thanks

> 
> Jason
> 

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

[Jason Gunthorpe]

On Sun, Jul 26, 2026 at 11:17:53AM +0300, Leon Romanovsky wrote:

> Currently, the phys type universally describes memory and can be
> reliably translated into any required representation. It is the most
> fundamental type we have.

You cannot go from phys to page reliably unless you make assumptions
about the phys, that's the whole issue here. If the callchain expects
and intends to use a page it should really stay in a page and not take
a side trip to phys.

I agree it would help to have some helpers that were more phys based
for the callchains that want that since we are mixing two fairly
different activities in these functions..

But making phys the only output out of the pool isn't going to be an
improvement.

BTW how does a struct page even work for decrypted memory? You can't
actually use it that way right? If you try to access it then you'll
get the wrong IPA and it should explode?

Maybe we should be blocking the struct page path entirely for CC
shared?

Jason

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

[Leon Romanovsky]

On Mon, Jul 27, 2026 at 01:23:57AM -0300, Jason Gunthorpe wrote:
> On Sun, Jul 26, 2026 at 11:17:53AM +0300, Leon Romanovsky wrote:
> 
> > Currently, the phys type universally describes memory and can be
> > reliably translated into any required representation. It is the most
> > fundamental type we have.
> 
> You cannot go from phys to page reliably unless you make assumptions
> about the phys, that's the whole issue here. If the callchain expects
> and intends to use a page it should really stay in a page and not take
> a side trip to phys.
> 
> I agree it would help to have some helpers that were more phys based
> for the callchains that want that since we are mixing two fairly
> different activities in these functions..

Yes, we are agreed here.

> 
> But making phys the only output out of the pool isn't going to be an
> improvement.

I don't know, phys was natural suggestion due to multiple
page_to_phys/phys_to_page conversions down the road.

> 
> BTW how does a struct page even work for decrypted memory? You can't
> actually use it that way right? If you try to access it then you'll
> get the wrong IPA and it should explode?
> 
> Maybe we should be blocking the struct page path entirely for CC
> shared?

It will simplify things. Ultimately, this series demonstrates that
CC users must be aware of the memory attribute (CC_SHARED) and struct
page is too generic.

Thanks

> 
> Jason
> 

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

[Aneesh Kumar K.V]

Leon Romanovsky <leon@kernel.org> writes:

> On Sat, Jul 25, 2026 at 11:34:11AM -0300, Jason Gunthorpe wrote:
>> On Thu, Jul 23, 2026 at 10:57:04AM +0300, Leon Romanovsky wrote:
>> > On Wed, Jul 22, 2026 at 04:59:12PM -0300, Jason Gunthorpe wrote:
>> > > On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote:
>> > > 
>> > > > 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
>> > > > }
>> > > 
>> > > I don't like this, we don't know for sure the addr will be in a vmap
>> > > and this will fail if it isn't.
>> > 
>> > Of course we know. The existing "#ifdef CONFIG_DMA_DIRECT_REMAP" is
>> > relevant for addresses acquired from pool.
>> 
>> Yeah, but I still don't like it :) It is hard to follow if you make
>> those kinds of leaps, someone will call this new helper on something
>> they shouldn't
>
> This issue is straightforward to address today. Limit the scope, use
> an appropriate function name, and add a comment indicating that the
> function is local to and specific to the pool. The latter helps
> AI-based review tools flag incorrect usage outside the intended scope.
>
>> 
>> > dma_phys_to_page() is a bad name for some low-level conversion function.
>> > It needs to be internal to DMA logic, in the level when we convert from
>> > phys to page.
>> 
>> I think we should not convert from phys to page, that's also easy to
>> do wrong
>
> Perhaps I'll reiterate my complaint. DMA internals are already hard to
> understand. Part of that complexity is necessary to support every
> possible use case, but another part comes from a maze of types that is
> entirely self-inflicted. I think this code only makes the latter worse.
>
> Currently, the phys type universally describes memory and can be
> reliably translated into any required representation. It is the most
> fundamental type we have.
>

I think we can address the suggested changes as a follow-up patch series
on top of this.

How about we get this picked up for the next merge window first, so that
we can get the CC_SHARED related changes in? The follow-up changes
should be easier once we have agreement on the overall design, and can
then be done as incremental cleanups.

-aneesh

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

[Mostafa Saleh]

On Fri, Jul 17, 2026 at 11:34:19PM +0530, Aneesh Kumar K.V (Arm) wrote:
> Commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool
> helper") changed dma_direct_alloc_from_pool() to return the CPU address
> from dma_alloc_from_pool(). That fits dma_direct_alloc(), but
> dma_direct_alloc_pages() also uses the helper and expects a struct page *.
> 
> Fix this by making dma_direct_alloc_from_pool() return the struct page *
> again, and pass the CPU address back through an out-parameter for the
> dma_direct_alloc() caller.
> 
> Fixes: 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper")
> Cc: stable@vger.kernel.org
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa

> ---
>  kernel/dma/direct.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index d8219efe3273..363d984d90e7 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -164,22 +164,21 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
>  	return !gfpflags_allow_blocking(gfp) && !is_swiotlb_for_alloc(dev);
>  }
>  
> -static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> -		dma_addr_t *dma_handle, gfp_t gfp)
> +static struct page *dma_direct_alloc_from_pool(struct device *dev, size_t size,
> +		dma_addr_t *dma_handle, void **cpu_addr, gfp_t gfp)
>  {
>  	struct page *page;
>  	u64 phys_limit;
> -	void *ret;
>  
>  	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_COHERENT_POOL)))
>  		return NULL;
>  
>  	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
> -	page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
> +	page = dma_alloc_from_pool(dev, size, cpu_addr, gfp, dma_coherent_ok);
>  	if (!page)
>  		return NULL;
>  	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
> -	return ret;
> +	return page;
>  }
>  
>  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;
> +	}
>  
>  	/* we always manually zero the memory once we are done */
>  	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
> @@ -357,7 +359,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
>  	void *ret;
>  
>  	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
> -		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
> +		return dma_direct_alloc_from_pool(dev, size, dma_handle, &ret, gfp);
>  
>  	page = __dma_direct_alloc_pages(dev, size, gfp, false);
>  	if (!page)
> -- 
> 2.43.0
>