Re: [PATCH v3 4/9] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED

[Aneesh Kumar K . V]

Catalin Marinas <catalin.marinas@xxxxxxx> writes:

> On Mon, Apr 27, 2026 at 11:25:04AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> @@ -1408,6 +1429,17 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
>>  	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>>  		pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
>>  
>> +	/*
>> +	 * if we are trying to swiotlb map a decrypted paddr or the paddr is encrypted
>> +	 * but the device is forcing decryption, use decrypted io_tlb_mem
>> +	 */
>> +	if ((attrs & DMA_ATTR_CC_SHARED) ||
>> +	    (!(attrs & DMA_ATTR_CC_SHARED) && force_dma_unencrypted(dev)))
>> +		require_decrypted = true;
>
> Nit: just this should do:
>
> 	if ((attrs & DMA_ATTR_CC_SHARED) || force_dma_unencrypted(dev))
>

I will update this in the next version.

>> +	if (require_decrypted != mem->decrypted)
>> +		return (phys_addr_t)DMA_MAPPING_ERROR;
>
> I wonder whether io_tlb_mem should store the attrs that were used when
> created (just DMA_ATTR_CC_SHARED for now) and use that to check here. In
> patch 7, this hunk in swiotlb_map() confused me:
>

We already added io_tlb_mem->decrypted. Are you suggesting that this
should instead be io_tlb_mem->attrs and use DMA_ATTR_CC_SHARED? Do we
foresee the need to use any other attributes (other than shared) with
respect to io_tlb_mem?

>
> 	if (dev->dma_io_tlb_mem->decrypted) {
> 		dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
> 		attrs |= DMA_ATTR_CC_SHARED;
> 	} else {
> 		dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
> 	}
>
> as I thought we'd not update the attributes on the streaming API path.
> But what you meant here is for dma_capable() to be checked against the
> device with the actual io_tlb_mem attributes.
>

if we allocated/mapped from a decrypted iot_tlb_mem, we should return an
unencrypted dma_addr_t. 

> Anyway, the new swiotlb_tbl_map_single() rejects kmalloc-minalign
> bouncing if the device is private while the bounce buffer is shared.
> Unlikely we'll need such bouncing if the devices are coherent but it's
> good as a safety check.
>

-aneesh