Jason Gunthorpe <[email protected]> writes:
> On Thu, Jul 09, 2026 at 12:13:19PM +0100, Catalin Marinas wrote:
>> > > For AMD/SME, on host with memory encryption we now end up setting the C
>> > > bit for DMA_ATTR_MMIO. This is fine for RAM but not sure whether
>> > > some other MMIO bus understands this attribute. Maybe we should stick to
>> > > something like __phys_to_dma() for the !CC_SHARED && MMIO path. Or,
>> > > since this is not universally defined, just use the old dma_addr = phys
>> > > if MMIO and ignore any unlikely DMA offsets.
>> > >
>> >
>> > Considering for AMD/SME system an unencrypted dma addr is one without C
>> > bit, will this be good?
>> >
>> > /*
>> > * For host memory encryption and device requiring unencrypted DMA,
>> > * MMIO memory is treated as shared by default.
>> > */
>> > if (attrs & DMA_ATTR_MMIO) {
>> > if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) ||
>> > force_dma_unencrypted(dev))
>> > attrs |= DMA_ATTR_CC_SHARED;
>> > }
>>
>> Yes, I think it does the trick, preserves the current semantics for AMD.
>> I guess you could use a single 'if' for all checks (up to you).
>
> Please don't change it, MMIO P2P is broken on CC systems today and it
> should stay broken. Passing DMA_ATTR_MMIO with DMA_ATTR_CC_SHARED is
> an error that we need to correct in the drivers not make work in the
> core code.
>
But the above changes are intended to handle HOST_MEM_ENCRYPT. In v7, we
had the following diff:
@@ -88,37 +88,40 @@ static inline dma_addr_t dma_direct_map_phys(struct device
*dev,
{
dma_addr_t dma_addr;
+ /*
+ * For a device requiring unencrypted DMA, MMIO memory is treated
+ * as shared by default.
+ */
+ if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
+ attrs |= DMA_ATTR_CC_SHARED;
+
This is now getting updated to
@@ -624,37 +626,44 @@ dma_addr_t dma_direct_map_phys(struct device *dev,
phys_addr_t phys,
{
dma_addr_t dma_addr;
+ if (attrs & DMA_ATTR_MMIO) {
+ /*
+ * For host memory encryption and device requiring
+ * unencrypted DMA, MMIO memory is treated as shared by
+ * default.
+ */
+ if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) ||
+ force_dma_unencrypted(dev))
+ attrs |= DMA_ATTR_CC_SHARED;
+ }
+
I agree that we need to move that DMA_ATTR_CC_SHARED setting to
modified drivers/pci/p2pdma.c
@@ -285,6 +285,11 @@ int pcim_p2pdma_init(struct pci_dev *pdev)
continue;
p2p->mem[i].owner = &pdev->dev;
+
+ p2p->mem[i].dma_mapping_flags = DMA_ATTR_MMIO;
+ if (force_dma_unencrypted(dev))
+ p2p->mem[i].dma_mapping_flags |= DMA_ATTR_CC_SHARED;
+
As we discussed [1], that can come in a later patch. In the meantime, adding
the HOST_MEM_ENCRYPT check preserves the previous behavior for SME.
[1] https://lore.kernel.org/all/[email protected]/
-aneesh
