Re: [RFC PATCH v3 4/5] dma-mapping: Encapsulate memory state during allocation

[Jason Gunthorpe]

On Wed, Apr 08, 2026 at 07:47:41PM +0000, Mostafa Saleh wrote:
> Introduce a new dma-direct internal type dma_page which is
> "struct page" and a bit indicate whether the memory has been decrypted
> or not.
> This is useful to pass such information encapsulated through
> allocation functions, which is currently set from swiotlb_alloc().
> 
> No functional changes.
> 
> Signed-off-by: Mostafa Saleh <smostafa@xxxxxxxxxx>
> ---
>  kernel/dma/direct.c | 58 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 46 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index de63e0449700..204bc566480c 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -16,6 +16,33 @@
>  #include <linux/pci-p2pdma.h>
>  #include "direct.h"
>  
> +/*
> + * Represent DMA allocation and 1 bit flag for it's state
> + */

I'd explain this wrappers a pointer and uses the low PAGE_SHIFT bits
for flags..

> +struct dma_page {
> +	unsigned long val;

unintptr_t ?

> @@ -103,20 +130,21 @@ static void __dma_direct_free_pages(struct device *dev, struct page *page,
>  	dma_free_contiguous(dev, page, size);
>  }
>  
> -static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
> +static struct dma_page dma_direct_alloc_swiotlb(struct device *dev, size_t size)
>  {
> -	struct page *page = swiotlb_alloc(dev, size, NULL);
> +	enum swiotlb_page_state state;
> +	struct page *page = swiotlb_alloc(dev, size, &state);
>  
>  	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
>  		swiotlb_free(dev, page, size);
> -		return NULL;
> +		return DMA_PAGE_NULL;
>  	}
>  
> -	return page;
> +	return page_to_dma_page(page, state == SWIOTLB_PAGE_DECRYPTED);

Should the struct dma_page have been introduced earlier instead of the
swiotlb_page_state ? Seems a bit odd to have both

If these are actually internally allocated struct pages, could you use
the struct page memory itself to record the decrypted state? That
would require more significant changes to the allocator calls.

> @@ -184,9 +212,11 @@ static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
>  static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
>  		dma_addr_t *dma_handle, gfp_t gfp)
>  {
> +	struct dma_page dma_page;
>  	struct page *page;
>  
> -	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
> +	dma_page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
> +	page = dma_page_to_page(dma_page);
>  	if (!page)
>  		return NULL;

I would expect to see more usage of the dma_page here..

Like I don't think this is really right:

        *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));

Does page_to_phys(page) really work on decrypted memory? On CCA it
will return the protected alias which doesn't seem like something
useful?

static inline dma_addr_t phys_to_dma_direct(struct device *dev,
                phys_addr_t phys)
{
        if (force_dma_unencrypted(dev))
                return phys_to_dma_unencrypted(dev, phys);
        return phys_to_dma(dev, phys);

Above is all nonsense now that you have a direct indication of the
address is decrypted memory or not, it should also be used right here
directly.

if (is_dma_page_decrypted(dma_page))
   *dma_handle = phys_to_dma_unencrypted(..)
else
   *dma_handle = phys_to_dma(..);

The later patch just makes it worse by adding even more confusing
flags to phys_to_dma_direct().

I think it should work out that everyone already knows what memory
type they are working with before they call down to
phys_to_dma_direct() - the calls to force_dma_unecrypted() here are
just hacks because it previously did not.

Anyhow, I think this series is alot better than the previous one. If
you work a little harder to make it so there is only one
force_dma_unecrypted() per high level DMA API call that would be
perfect.

Jason