Re: [RFC PATCH v2 2/2] dma: swiotlb: Initialize and size shared default pools for memory encryption

Catalin Marinas <[email protected]> writes:

> On Thu, Aug 13, 2026 at 03:55:21PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 9f5b366d2086..c3188ca878f3 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -338,12 +338,8 @@ void __init arch_setup_zero_pages(void)
>>  void __init arch_mm_preinit(void)
>>  {
>>  	unsigned int flags = SWIOTLB_VERBOSE;
>> -	/* pKVM uses restricted-dma-pool */
>> -	bool cc_guest = is_realm_world();
>>  
>> -	if (cc_guest)
>> -		flags |= SWIOTLB_INIT_CC_SHARED;
>> -	else if (max_pfn > PFN_DOWN(arm64_dma_phys_limit))
>> +	if (max_pfn > PFN_DOWN(arm64_dma_phys_limit))
>>  		flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
>>  
>>  	swiotlb_init(flags);
>
> This looks fine. As I mentioned on patch 1, we might as well move this
> hunk over there and avoid the flag definition.
>
>> @@ -102,9 +101,6 @@ void __init mem_encrypt_init(void)
>>  
>>  void __init mem_encrypt_setup_arch(void)
>>  {
>> -	phys_addr_t total_mem = memblock_phys_mem_size();
>> -	unsigned long size;
>> -
>>  	/*
>>  	 * Do RMP table fixups after the e820 tables have been setup by
>>  	 * e820__memory_setup().
>> @@ -112,33 +108,9 @@ void __init mem_encrypt_setup_arch(void)
>>  	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
>>  		snp_fixup_e820_tables();
>>  
>> -	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> -		x86_swiotlb_flags |= SWIOTLB_INIT_CC_SHARED;
>> -
>>  	if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>>  		return;
>>  
>> -	/*
>> -	 * For SEV and TDX, all DMA has to occur via shared/unencrypted pages.
>> -	 * Kernel uses SWIOTLB to make this happen without changing device
>> -	 * drivers. However, depending on the workload being run, the
>> -	 * default 64MB of SWIOTLB may not be enough and SWIOTLB may
>> -	 * run out of buffers for DMA, resulting in I/O errors and/or
>> -	 * performance degradation especially with high I/O workloads.
>> -	 *
>> -	 * Adjust the default size of SWIOTLB using a percentage of guest
>> -	 * memory for SWIOTLB buffers. Also, as the SWIOTLB bounce buffer
>> -	 * memory is allocated from low memory, ensure that the adjusted size
>> -	 * is within the limits of low available memory.
>> -	 *
>> -	 * The percentage of guest memory used here for SWIOTLB buffers
>> -	 * is more of an approximation of the static adjustment which
>> -	 * 64MB for <1G, and ~128M to 256M for 1G-to-4G, i.e., the 6%
>> -	 */
>> -	size = total_mem * 6 / 100;
>> -	size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
>> -	swiotlb_adjust_size(size);
>> -
>>  	/* Set restricted memory access for virtio. */
>>  	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
>>  }
>
> Credit to claude, it noticed a slight change in behaviour for x86 w.r.t.
> the crash kernel reservation. crash_low_size_default() reads the swiotlb
> size but the resizing now happens after arch_reserve_crashkernel().
> Maybe not an issue.
>
> Alternatively, we could build the sizing logic into
> swiotlb_size_or_default() but I haven't checked whether we have the
> right information when this function is called.
>

IIUC, the current code can still get a different value from
crash_low_size_default() than the final swiotlb size we end up using.
This is because crash_low_size_default() is computed early, before
default_nareas, which is derived from num_possible_cpus(), has been set.

If we are okay with keeping this consistent with the existing behavior,
moving sizing logic to swiotlb_adjusted_size() looks like a clean option.

>
>> @@ -382,12 +374,54 @@ static void __init *swiotlb_memblock_alloc(unsigned long nslabs,
>>  	return tlb;
>>  }

...

>> +static bool __init swiotlb_default_pool_needs_cc_shared(void)
>> +{
>> +	/* A restricted DMA pool provides the shared buffers instead. */
>> +	return cc_platform_has(CC_ATTR_MEM_ENCRYPT) &&
>> +	       !restricted_dma_pool_present;
>> +}
>
> I don't think restricted_dma_pool_present should change the cc_shared
> attribute. The rmem pool is all about sizing the swiotlb, not disabling
> sharing.
>
> Thinking some more, if other archs don't like rmem pool influencing the
> default swiotlb size, we could add a flag (SWIOTLB_SKIP_IF_RMEM_POOL or
> some better name). But only if people dislike the heuristics.
>
>> @@ -437,9 +469,11 @@ void __init swiotlb_init_remap(unsigned int flags,
>>  		io_tlb_default_mem.phys_limit = ARCH_LOW_ADDRESS_LIMIT;
>>  #endif
>>  
>> -	if (!(flags & (SWIOTLB_INIT_ADDRESSING_LIMIT |
>> -		       SWIOTLB_INIT_CC_SHARED)) &&
>> -	    swiotlb_kmalloc_needs_bounce()) {
>> +	if (swiotlb_default_pool_needs_cc_shared()) {
>> +		io_tlb_default_mem.cc_shared = true;
>> +		swiotlb_adjust_cc_attributes();
>> +	} else if (!(flags & SWIOTLB_INIT_ADDRESSING_LIMIT) &&
>> +		   swiotlb_kmalloc_needs_bounce()) {
>
> I think at a high level, we need (i.e. separate attributed from sizing):
>
> 	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> 		io_tlb_default_mem.cc_shared = true;
>
> 	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> 		if (!restricted_dma_pool_present)
> 			swiotlb_adjust_cc_size();	/* 6%, clamped */
> 	} else if (!(flags & SWIOTLB_INIT_ADDRESSING_LIMIT) &&
> 		   swiotlb_kmalloc_needs_bounce()) {
> 		swiotlb_shrink_for_kmalloc();		/* 1MB per 1GB */
> 	}
>

But pKVM wants to reduce the swiotlb size based on
kmalloc_needs_bounce() when it is using a restricted-dma-pool.

ie,
	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
		io_tlb_default_mem.cc_shared = true;
..

	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
	    !restricted_dma_pool_present) {
		swiotlb_adjust_cc_attributes();
	} else if (!(flags & SWIOTLB_INIT_ADDRESSING_LIMIT) &&
		   swiotlb_kmalloc_needs_bounce()) {

-aneesh