Thread (37 messages) 37 messages, 6 authors, 2d ago

Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED

From: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
Date: 2026-07-10 19:10:16
Also in: linux-arm-kernel, linux-coco, linux-iommu, linux-s390, lkml

Jason Gunthorpe [off-list ref] writes:
On Fri, Jul 10, 2026 at 10:52:49AM +0530, Aneesh Kumar K.V wrote:
quoted
quoted
quoted
quoted
	/*
	 * 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:
To follow how the rest of the decrypted/encrypted stuff works the MMIO
has to be flaged with CC_SHARED for HOST_MEM_ENCRYPT too, just like
the PTEs.
quoted
@@ -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;
force_dma_unencrypted() says nothing about the properties of the
address passed in, this was nonsense :\
quoted
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.
It never worked. When we added ATTR_MMIO it started to have a chance
to work but prior to that it was always broken anyhow. I don't see
there is much merit in preserving the narrow window when we
inadvertantly had a half working ATTR_MMIO.

But if you really want to it should be
cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) *only* and get rid of the wrong
force_dma_unencrypted().
Ok, I will update the changes to so we preserve the previous behaviour
for SME?

	if (attrs & DMA_ATTR_MMIO) {
		/*
		 * For host memory encryption treat MMIO memory as shared
		 */
		if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
			attrs |= DMA_ATTR_CC_SHARED;
	}

Do let me know if you feel strongly that the above change should not be
included and that the SME P2P case should instead be handled in a future
patch.

But IMHO, I'd rather this series treat ATTR_MMIO as private MMIO and
ATTR_MMIO|CC_SHARED as shared MMIO and that's the right and correct
thing for the DMA API.
If you think the rest of the series is ready for upstream, could you
please ack it so it can be picked up for the next merge window? I'll
repost v8, rebased on top of the pKVM topic branch.

-aneesh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help