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

Mostafa Saleh
2026-03-30T20:49:37Z

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