Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths

[Alexey Kardashevskiy <aik@amd.com>]

On 1/7/26 03:42, Jason Gunthorpe wrote:
> On Mon, Jun 29, 2026 at 12:16:30PM +0530, Aneesh Kumar K.V wrote:
>>>> Thinking about this more, I guess we should mark the swiotlb as
>>>> cc_shared only with  CC_ATTR_GUEST_MEM_ENCRYPT instead of
>>>> CC_ATTR_MEM_ENCRYPT as we have below.
>>>
>>> The name cc_shared should be used for GUEST scenarios only.
>>>
>>> I guess there is some merit in keeping swiotlb using "decrypted" to
>>> mean it usinig pgprot_decrypted and set_memory_decyped() which AMD
>>> gives meaning to on both host and guest.
>>
>> Are you suggesting to change the struct io_tlb_mem::cc_shared back to
>> struct io_tlb_mem::unencrypted?.
> 
> Yes
> 
>>> IDK what AMD should do on the host by default. I guess it should setup
>>> a swiotlb pool of low dma addrs "unencrypted", but not "cc_shared"?
>>>
>>
>> If by low DMA address you mean using an address with the C-bit
>> cleared.
> 
> Yes
> 
>> The current code already does this and uses the swiotlb pool correctly
>> on SME.
> 
> Well, through the force_dma_unencrypted() hack...
> 
>> The challenge arises when we want to force SWIOTLB
>> bouncing even for devices that can handle encrypted DMA addresses (more
>> on that below). For such a config force_dma_uencrypted(dev) will return
>> false and swiotlb will be marked cc_shared/decrypted = true; This trip
>> the new check we added.
> 
> Yes, because cc_shared (guest) and unencrypted (host) are very
> different things and we've mixed them:
> 
>> 	if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
> 
> I'm aruging force_dma_unencrypted should mean cc_shared and be
> guest_only, but the SME hack breaks this.
> 
>> We can also do
>>
>> 	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
>> 		/* swiotlb pool is incorrect for this device */
>> 		if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
>> 			return (phys_addr_t)DMA_MAPPING_ERROR;
>>
>> 		/* Force attrs to match the kind of memory in the pool */
>> 		if (mem->cc_shared)
>> 			*attrs |= DMA_ATTR_CC_SHARED;
>> 		else
>> 			*attrs &= ~DMA_ATTR_CC_SHARED;
>> 	} else {
>> 		/*
>> 		 * Host memory encryption where device requires an
>> 		 * unencrypted dma_addr_t due to dma mask limit
>>      		 */
>> 		if (force_dma_unencrypted(dev))
>> 			*attrs |= DMA_ATTR_CC_SHARED;
>> 		else
>> 			*attrs &= ~DMA_ATTR_CC_SHARED;
>> 	}
> 
> If we do this I would like to split the force_dma_.. functions into
> guest and host, ie force_dma_cc_shared() and force_host_decrypted()

imho force_dma_unencrypted() should not look at the mask at all (the mask should tell the DMA layer to use swiotlb, encrypted or not), instead, when we set up swiotlb - we could make it unencrypted if iommu=pt, otherwise encrypted (although this means IOMMU and defeats the purpose of swiotlb). But at least this patchset has enough plumbing to have swiotlb encrypted, right?

> To make it clear there are two very different things here.
> 
>> Here I see value in having DMA_ATTR_UNENCRYPTED. The question is do we
>> need to split this into two flags and introduce the resulting code
>> duplication.
> 
> The external flag name should be DMA_ATTR_CC_SHARED and only used on
> CC guest. Internally that turns into using set_memory_decrypted()
> which works on guest and host for AMD. I don't know how to make the
> host only case clearer and still keep the code efficient..
> 
>>> The dma api has to detect, after the driver sets the dma limit, that
>>> none of system memory is usable when:
>>>   - The direct path is being used
>>>   - phys to dma for 0 is outside the dma limit
>>>
>>> Then it should assume the arch has setup a swiotlb pool for it to use
>>> to fix the high memory problem.
>>>
>>> Similar hackery would be needed in the dma alloc path to know that
>>> decrypted can be used to fix the high memory problem like for GUEST.
>>>
>>> I guess some 'dev_cannot_reach_memory(dev)' sort of test in a
>>> few key places? Setup with a static branch to be a nop on everything
>>> but AMD, compiled out on every other arch.
>>>
>>
>> If we are not able to reach the memory because of the memory encryption
>> bit, then isn't dev_cannot_reach_memory(dev) the same as
>> force_dma_unencrypted(dev)? If so, that is how it is already done.
> 
> Sort of yes, but it is properly named to its purpose and not confused
> with what should be a guest-only function.
> 
>> x86/dma: Disable forced SWIOTLB bouncing for SME IOMMU passthrough
> 
> Maybe as a crutch to get this series merged..
feels okay but I do not really know the true meaning of "swiotlb=force" so adding Tom to the thread. Thanks,


-- 
Alexey