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

[Tom Lendacky <thomas.lendacky@amd.com>]

On 7/1/26 20:04, Alexey Kardashevskiy wrote:
> 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,

With swiotlb=force all DMA would be bounce buffered through SWIOTLB. My
understanding (which could be completely wrong) for this setting is that
it is used mainly to validate a driver implementation (race conditions
between device memory access and driver memory access) and validate the
SWIOTLB implementation itself. I'm not sure why you would want to run
with swiotlb=force other than that.

In that situation, with SME enabled on the host, the SWIOTLB must be
mapped by the kernel without the encryption bit set and DMA addresses
must be provided to devices without the encryption bit set. This is
because if SWIOTLB is mapped encrypted, requiring the encryption bit be
part of the DMA address, any device that cannot perform DMA at the
address width where the encryption bit exists will fail to DMA properly.

Thanks,
Tom

> 
>