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

Mostafa Saleh posted 5 patches 1 day, 18 hours ago
[RFC PATCH v2 1/5] dma-mapping: Avoid double decrypting with DMA_RESTRICTED_POOL
Posted by Mostafa Saleh 1 day, 18 hours ago
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@google.com>
---
 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;
 	return set_memory_decrypted((unsigned long)vaddr, PFN_UP(size));
 }
@@ -88,7 +88,7 @@ static int dma_set_encrypted(struct device *dev, void *vaddr, size_t size)
 {
 	int ret;
 
-	if (!force_dma_unencrypted(dev))
+	if (!force_dma_unencrypted(dev) || is_swiotlb_for_alloc(dev))
 		return 0;
 	ret = set_memory_encrypted((unsigned long)vaddr, PFN_UP(size));
 	if (ret)
-- 
2.53.0.1185.g05d4b7b318-goog
Re: [RFC PATCH v2 1/5] dma-mapping: Avoid double decrypting with DMA_RESTRICTED_POOL
Posted by Jason Gunthorpe 1 day, 18 hours ago
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@google.com>
> ---
>  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.

Double decryption is certainly a bug, I do not expect that to work.

Jason
Re: [RFC PATCH v2 1/5] dma-mapping: Avoid double decrypting with DMA_RESTRICTED_POOL
Posted by Mostafa Saleh 1 day, 13 hours ago
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@google.com>
> > ---
> >  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.

Thanks,
Mostafa

> 
> Double decryption is certainly a bug, I do not expect that to work.
> 
> Jason
Re: [RFC PATCH v2 1/5] dma-mapping: Avoid double decrypting with DMA_RESTRICTED_POOL
Posted by Suzuki K Poulose 22 hours ago
On 30/03/2026 21:43, Mostafa Saleh wrote:
> 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@google.com>
>>> ---
>>>   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.

Doesn't the initial state depend on platform ? For CCA, the Realm must
decide how it wants to use a given region, which for the restricted DMA 
pool, it can be made decrypted. Could the VM OS decide to make this
decrypted at boot ?

Suzuki


> 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.
> 
> Thanks,
> Mostafa
> 
>>
>> Double decryption is certainly a bug, I do not expect that to work.
>>
>> Jason

Re: [RFC PATCH v2 1/5] dma-mapping: Avoid double decrypting with DMA_RESTRICTED_POOL
Posted by Mostafa Saleh 20 hours ago
On Tue, Mar 31, 2026 at 12:34:20PM +0100, Suzuki K Poulose wrote:
> On 30/03/2026 21:43, Mostafa Saleh wrote:
> > 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@google.com>
> > > > ---
> > > >   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.
> 
> Doesn't the initial state depend on platform ? For CCA, the Realm must
> decide how it wants to use a given region, which for the restricted DMA
> pool, it can be made decrypted. Could the VM OS decide to make this
> decrypted at boot ?
> 

At the moment no [1], the pool is decrypted unconditionally.
As mentioned in the cover letter under "Future work", I believe
giving the OS the ability to have undecrypted pools is important for
confidential DMA.

Initially, I thought that can be a per device property (so the
platform will keep the memory encrypted for physical devices and
decrypt it for emulated ones).

But, that might cause runtime issues as a pool can be shared between
multiple devices. So, I beleive it's better to have this as per-pool
property(from device-tree for ex) and then the platform can do any
validation of assumptions it needs in the runtime.

This is a bit hairy, as I mentioned it would be a good topic to
discuss in the next LPC.

Thanks,
Mostafa


[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/dma/swiotlb.c#n1847

> Suzuki
> 
> 
> > 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.
> > 
> > Thanks,
> > Mostafa
> > 
> > > 
> > > Double decryption is certainly a bug, I do not expect that to work.
> > > 
> > > Jason
>