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

[Alexey Kardashevskiy <aik@amd.com>]

On 9/7/26 03:58, 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?


Normally both encrypted and unencrypted DMA addresses do not have Cbit, with the only exception of "iommu=pt" (which is not the default afaik). And in this case, having Cbit in DMA handles only makes sense if p2p trafic goes via the root port (== IOMMU in passthrough mode, and I am not sure even about if the root port will convert this Cbit to T=1 MMIO in this mode) but if it goes via some PCI bridge - then Cbit won't mean encryption for sure. But I do not know much about p2p (never touched). But in any case force_dma_unencrypted() seems to make no sense here.


> 
> 	/*
> 	 * 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;
> 	}
> 
> 
>>
>> In the other case, for an arm CCA guest, if the MMIO is shared we end up
>> setting the shared attribute but that's fine, it's only an IPA address.

IPA == guest physical address? Thanks,


>>
>> -- 
>> Catalin
> 
> -aneesh

-- 
Alexey