Re: [RFC PATCH v2 1/5] dma-mapping: Avoid double decrypting with DMA_RESTRICTED_POOL

[Aneesh Kumar K . V]

Mostafa Saleh <smostafa@xxxxxxxxxx> writes:

> On Mon, Mar 30, 2026 at 12:06:54PM -0300, Jason Gunthorpe wrote:
>> On Mon, Mar 30, 2026 at 02:50:39PM +0000, Mostafa Saleh wrote:
>> > In case a device have a restricted DMA pool, it will be decrypted
>> > by default.
>> > 
>> > However, in the path of dma_direct_alloc() memory can be allocated
>> > from this pool using, __dma_direct_alloc_pages() =>
>> > dma_direct_alloc_swiotlb()
>> > 
>> > After that from the same function, it will attempt to decrypt it
>> > using dma_set_decrypted() if force_dma_unencrypted().
>> > 
>> > Which results in the memory being decrypted twice.
>> > 
>> > It's not clear how the does realm world/hypervisors deal with that,
>> > for example:
>> > - CCA: Clear a bit in the page table and call realm IPA_STATE_SET.
>> > - TDX: Issue a hypercall.
>> > - pKVM: Which doesn't implement force_dma_unencrypted() at the moment,
>> >   uses a share hypercall.
>> > 
>> > Change that to only encrypt/decrypt memory that are not allocated
>> > from the restricted dma pools.
>> > 
>> > Fixes: f4111e39a52a ("swiotlb: Add restricted DMA alloc/free support")
>> > Signed-off-by: Mostafa Saleh <smostafa@xxxxxxxxxx>
>> > ---
>> >  kernel/dma/direct.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> > index 8f43a930716d..27d804f0473f 100644
>> > --- a/kernel/dma/direct.c
>> > +++ b/kernel/dma/direct.c
>> > @@ -79,7 +79,7 @@ bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
>> >  
>> >  static int dma_set_decrypted(struct device *dev, void *vaddr, size_t size)
>> >  {
>> > -	if (!force_dma_unencrypted(dev))
>> > +	if (!force_dma_unencrypted(dev) || is_swiotlb_for_alloc(dev))
>> >  		return 0;
>> 
>> This seems really obtuse, I would expect the decryption state of the
>> memory to be known by the caller. If dma_direct_alloc_swiotlb() can
>> return decrypted or encrypted memory it needs to return a flag saying
>> that. It shouldn't be deduced by checking dev flags in random places
>> like this.
>
> At the moment restricted dma is always decrypted, also it’s per device
> so we don’t have to check this per allocation.
> I can change the signature for __dma_direct_alloc_pages() to make it
> return an extra flag but that feels more complicated as it changes
> dma_direct_alloc_swiotlb() , swiotlb_alloc() with its callers.
>
> I can investigate this approach further.
>

I know there is a v3, but I’m commenting here to check whether we really
need to do this now. In my opinion, we can keep this much simpler by
treating all swiotlb allocations as decrypted allocations. We also need
this to simplify the IPA change alignment updates I posted

https://lore.kernel.org/all/20260309102625.2315725-2-aneesh.kumar@xxxxxxxxxx

-aneesh