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
