Re: [PATCH v2 6/8] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED

[Aneesh Kumar K . V]

Jason Gunthorpe <jgg@xxxxxxxx> writes:

> On Mon, Apr 20, 2026 at 11:44:13AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Teach dma_direct_map_phys() to select the DMA address encoding based on
>> DMA_ATTR_CC_SHARED.
>> 
>> Use phys_to_dma_unencrypted() for decrypted mappings and
>> phys_to_dma_encrypted() otherwise. If a device requires unencrypted DMA
>> but the source physical address is still encrypted, force the mapping
>> through swiotlb so the DMA address and backing memory attributes remain
>> consistent.
>> 
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
>> ---
>>  kernel/dma/direct.h | 25 ++++++++++++++-----------
>>  1 file changed, 14 insertions(+), 11 deletions(-)
>> 
>> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
>> index 7140c208c123..928671ef01e9 100644
>> --- a/kernel/dma/direct.h
>> +++ b/kernel/dma/direct.h
>> @@ -86,9 +86,14 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
>>  		phys_addr_t phys, size_t size, enum dma_data_direction dir,
>>  		unsigned long attrs, bool flush)
>>  {
>> +	bool force_swiotlb_map = false;
>>  	dma_addr_t dma_addr;
>>  
>> -	if (is_swiotlb_force_bounce(dev)) {
>> +	/* if phys addr attribute is encrypted but the device is forcing an encrypted dma addr */
>> +	if (!(attrs & DMA_ATTR_CC_SHARED) && force_dma_unencrypted(dev))
>> +		force_swiotlb_map = true;
>
> continuing my prior email.. This is really in the wrong spot, it
> should be in dma_capable()
>
>> +	if (is_swiotlb_force_bounce(dev) || force_swiotlb_map) {
>>  		if (!(attrs & DMA_ATTR_CC_SHARED)) {
>>  			if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
>>  				return DMA_MAPPING_ERROR;
>> @@ -105,18 +110,16 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
>>  			goto err_overflow;
>>  	} else if (attrs & DMA_ATTR_CC_SHARED) {
>>  		dma_addr = phys_to_dma_unencrypted(dev, phys);
>> -		if (unlikely(!dma_capable(dev, dma_addr, size, false)))
>> -			goto err_overflow;
>>  	} else {
>> -		dma_addr = phys_to_dma(dev, phys);
>> -		if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
>> -		    dma_kmalloc_needs_bounce(dev, size, dir)) {
>
> here.
>
> swiotlb because the device can't reach a high address and swiotlb
> because the device doesn't have T=1 are really the same thing and
> should have the same code flow.
>
> Add attrs to dma_capable() and check force_dma_unencrypted(dev)
> inside.
>

will update in the next revision

>
>> -			if (is_swiotlb_active(dev) &&
>> -			    !(attrs & DMA_ATTR_REQUIRE_COHERENT))
>> -				return swiotlb_map(dev, phys, size, dir, attrs);
>> +		dma_addr = phys_to_dma_encrypted(dev, phys);
>> +	}
>>  
>> -			goto err_overflow;
>> -		}
>> +	if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
>> +	    dma_kmalloc_needs_bounce(dev, size, dir)) {
>> +		if (is_swiotlb_active(dev) &&
>> +		    !(attrs & DMA_ATTR_REQUIRE_COHERENT))
>> +			return swiotlb_map(dev, phys, size, dir, attrs);
>> +		goto err_overflow;
>>  	}
>
> Then this movement shouldn't be needed?

I am still not clear about the use of DMA_ATTR_CC_SHARED here. If the
resulting DMA address is not dma_capable, I was expecting that we should
fall back to swiotlb_map(). That was the intention behind this change.
However, the other email thread suggests that DMA_ATTR_CC_SHARED is
always used with swiotlb_force_bounce(). I think we should address that.
If we do, the goal here would be to check dma_capable for both shared
and private addresses.

For private/protected addresses, swiotlb_map() will currently fail with
DMA_MAPPING_ERROR because the default io_tlb_mem (dev->dma_io_tlb_mem)
is decrypted by default

-aneesh