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

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Jul 08, 2026 at 11:28:14PM +0530, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
> 
> > On Wed, Jul 01, 2026 at 11:19:20AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> index 97987f850a33..acf67c7064db 100644
> >> --- a/arch/arm64/mm/init.c
> >> +++ b/arch/arm64/mm/init.c
> >> @@ -338,10 +338,8 @@ void __init arch_mm_preinit(void)
> >>  	unsigned int flags = SWIOTLB_VERBOSE;
> >>  	bool swiotlb = max_pfn > PFN_DOWN(arm64_dma_phys_limit);
> >>  
> >> -	if (is_realm_world()) {
> >> +	if (is_realm_world())
> >>  		swiotlb = true;
> >> -		flags |= SWIOTLB_FORCE;
> >> -	}
> >
> > For this part:
> >
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> >> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
> >> index e05dc7649366..f3fc28f352ba 100644
> >> --- a/kernel/dma/direct.h
> >> +++ b/kernel/dma/direct.h
> >> @@ -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;
> >> +
> >>  	if (is_swiotlb_force_bounce(dev)) {
> >> -		if (!(attrs & DMA_ATTR_CC_SHARED)) {
> >> -			if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> >> -				return DMA_MAPPING_ERROR;
> >> +		if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> >> +			return DMA_MAPPING_ERROR;
> >>  
> >> -			return swiotlb_map(dev, phys, size, dir, attrs);
> >> -		}
> >> -	} else if (attrs & DMA_ATTR_CC_SHARED) {
> >> -		return DMA_MAPPING_ERROR;
> >> +		return swiotlb_map(dev, phys, size, dir, attrs);
> >>  	}
> >>  
> >> -	if (attrs & DMA_ATTR_MMIO) {
> >> -		dma_addr = phys;
> >> -		if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
> >> -			goto err_overflow;
> >> -	} else if (attrs & DMA_ATTR_CC_SHARED) {
> >> +	if (attrs & DMA_ATTR_CC_SHARED)
> >>  		dma_addr = phys_to_dma_unencrypted(dev, phys);
> >> +	else
> >> +		dma_addr = phys_to_dma_encrypted(dev, phys);
> >
> > 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).

-- 
Catalin