As restricted dma pools are always decrypted, in swiotlb.c it uses
phys_to_dma_unencrypted() for address conversion.

However, in DMA-direct, calls to phys_to_dma_direct() with
force_dma_unencrypted() returning false, will fallback to
phys_to_dma() which is inconsistent for memory allocated from
restricted dma pools.

Fixes: f4111e39a52a ("swiotlb: Add restricted DMA alloc/free support")
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 kernel/dma/direct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 27d804f0473f..1a402bb956d9 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -26,7 +26,7 @@ u64 zone_dma_limit __ro_after_init = DMA_BIT_MASK(24);
 static inline dma_addr_t phys_to_dma_direct(struct device *dev,
 		phys_addr_t phys)
 {
-	if (force_dma_unencrypted(dev))
+	if (force_dma_unencrypted(dev) || is_swiotlb_for_alloc(dev))
 		return phys_to_dma_unencrypted(dev, phys);
 	return phys_to_dma(dev, phys);
 }
-- 
2.53.0.1185.g05d4b7b318-goog

On Mon, Mar 30, 2026 at 02:50:40PM +0000, Mostafa Saleh wrote:
> As restricted dma pools are always decrypted, in swiotlb.c it uses
> phys_to_dma_unencrypted() for address conversion.
> 
> However, in DMA-direct, calls to phys_to_dma_direct() with
> force_dma_unencrypted() returning false, will fallback to
> phys_to_dma() which is inconsistent for memory allocated from
> restricted dma pools.
> 
> Fixes: f4111e39a52a ("swiotlb: Add restricted DMA alloc/free support")
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>  kernel/dma/direct.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 27d804f0473f..1a402bb956d9 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -26,7 +26,7 @@ u64 zone_dma_limit __ro_after_init = DMA_BIT_MASK(24);
>  static inline dma_addr_t phys_to_dma_direct(struct device *dev,
>  		phys_addr_t phys)
>  {
> -	if (force_dma_unencrypted(dev))
> +	if (force_dma_unencrypted(dev) || is_swiotlb_for_alloc(dev))
>  		return phys_to_dma_unencrypted(dev, phys);

Same remark, I think the force_dma_unencrypted() was a hack to make up
for a flag here. In these lower layers we need to annotate if phys is
encrypted/decrypted and take the proper action universially.

The force_dma_unencrypted() should only be done way up the call chain
where we decide to get a phys that is decrypted. Once we have a
decrypted phys it should be carried with an annotation throughout all
the other places.

Then this is more like:

if (flags & FLAG_DECRYPTED)
   	return phys_to_dma_unencrypted(dev, phys);

Jason

On Mon, Mar 30, 2026 at 12:09:03PM -0300, Jason Gunthorpe wrote:
> On Mon, Mar 30, 2026 at 02:50:40PM +0000, Mostafa Saleh wrote:
> > As restricted dma pools are always decrypted, in swiotlb.c it uses
> > phys_to_dma_unencrypted() for address conversion.
> > 
> > However, in DMA-direct, calls to phys_to_dma_direct() with
> > force_dma_unencrypted() returning false, will fallback to
> > phys_to_dma() which is inconsistent for memory allocated from
> > restricted dma pools.
> > 
> > Fixes: f4111e39a52a ("swiotlb: Add restricted DMA alloc/free support")
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> >  kernel/dma/direct.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> > index 27d804f0473f..1a402bb956d9 100644
> > --- a/kernel/dma/direct.c
> > +++ b/kernel/dma/direct.c
> > @@ -26,7 +26,7 @@ u64 zone_dma_limit __ro_after_init = DMA_BIT_MASK(24);
> >  static inline dma_addr_t phys_to_dma_direct(struct device *dev,
> >  		phys_addr_t phys)
> >  {
> > -	if (force_dma_unencrypted(dev))
> > +	if (force_dma_unencrypted(dev) || is_swiotlb_for_alloc(dev))
> >  		return phys_to_dma_unencrypted(dev, phys);
> 
> Same remark, I think the force_dma_unencrypted() was a hack to make up
> for a flag here. In these lower layers we need to annotate if phys is
> encrypted/decrypted and take the proper action universially.
> 
> The force_dma_unencrypted() should only be done way up the call chain
> where we decide to get a phys that is decrypted. Once we have a
> decrypted phys it should be carried with an annotation throughout all
> the other places.

Can you please clarify what you mean by annotation in this context?
As I believe any tracking in the vmemmap is a big NO.

As replied to the first patch I can attempt to implement this approach
(by passing a flag around) and see how intrusive it would be.

Thanks,
Mostafa

> 
> Then this is more like:
> 
> if (flags & FLAG_DECRYPTED)
>    	return phys_to_dma_unencrypted(dev, phys);
> 



> Jason

On Mon, Mar 30, 2026 at 08:47:41PM +0000, Mostafa Saleh wrote:
> > The force_dma_unencrypted() should only be done way up the call chain
> > where we decide to get a phys that is decrypted. Once we have a
> > decrypted phys it should be carried with an annotation throughout all
> > the other places.
> 
> Can you please clarify what you mean by annotation in this context?
> As I believe any tracking in the vmemmap is a big NO.

It would have to be a flag pass along the phys, or phys & flag in a
struct or some kind of approach like that.

> As replied to the first patch I can attempt to implement this approach
> (by passing a flag around) and see how intrusive it would be.

I'm less concerned about intrusive and more about making this
understandable.

When we reach a function with a phys it should know what that phys is,
not call a bunch of random helpers to hopefully correctly guess what
it is, that's unmaintainable spaghetti even if it is fewer changes.

Jason