Re: [RFC PATCH v2 3/5] dma-mapping: Decrypt memory on remap

[Mostafa Saleh <smostafa@google.com>]

On Mon, Mar 30, 2026 at 12:19:00PM -0300, Jason Gunthorpe wrote:
> On Mon, Mar 30, 2026 at 02:50:41PM +0000, Mostafa Saleh wrote:
> 
> > @@ -265,6 +266,9 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >  		set_uncached = false;
> >  	}
> >  
> > +	if (dma_set_decrypted(dev, page_address(page), size))
> > +		goto out_leak_pages;
> > +
> >  	if (remap) {
> >  		pgprot_t prot = dma_pgprot(dev, PAGE_KERNEL, attrs);
> >
> >                if (force_dma_unencrypted(dev))
> >                        prot = pgprot_decrypted(prot);
> 
> It seems confusing, why do we unconditionally call something called
> dma_set_decrypted() and then conditionally call pgprot_decrypted()?

dma_set_decrypted() will call force_dma_unencrypted() so that check
is consistent.

> 
> So, I think the same remark, lets not sprinkle these tests all over
> the place and risk them becoming inconsistent. It should be much more
> direct, like:

I agree we shouldn’t be sprinkling all these random calls all over the code,
that’s why I was trying to consolidate the logic in the next patch.

> 
>     page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO,
> 				    allow_highmem, &flags);
> 
>     if (!dev_can_dma_from_encrypted(dev) && !(flags & FLAG_DECRYPTED)) {
>        dma_set_decrypted(dev, page_address(page));
>        flags = FLAG_DECRYPTED;
>     }
> 
>     if (flags & FLAG_DECRYPTED)
>        prot = pgprot_decrypted(prot);
> 
> And so on.
> 
> The one place we should see a force_dma_unencrypted() is directly
> before setting the flag.

I will look more into this, but my main worry would be
phys_to_dma_direct() and it's callers as I am not sure if is possible to
preserve the alloction origin in all contexts.

Thanks,
Mostafa



> 
> Jason