Re: [PATCH v5 4/6] dma: swiotlb: Centralize memory-encryption pool sizing

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Robin Murphy <robin.murphy@arm.com> writes:

> On 21/09/2026 7:36 am, Aneesh Kumar K.V (Arm) wrote:
>> Memory-encrypted guests use shared or unencrypted memory for DMA and may
>> route all DMA through SWIOTLB. The default pool can therefore be too
>> small for I/O-intensive workloads.
>> 

 [ ... 133 lines skipped ... ] 

>> +/**
>> + * swiotlb_adjusted_size() - get the prospective adjusted SWIOTLB size
>> + *
>> + * Return the size that confidential-computing guest sizing would select for
>> + * the default pool, without changing the configured SWIOTLB size. An
>> + * explicit swiotlb= size is always preserved. An explicit area count is
>> + * included in the size calculation. Automatic area sizing is initialized
>> + * later from the running kernel's possible CPU map and any resulting size
>> + * adjustment is therefore not reflected in the returned size.
>> + */
>> +unsigned long __init swiotlb_adjusted_size(void)
>
> This is yet another misleadingly ambiguous name.
>

Do you have any suggestions for a better approach? This returns the
swiotlb size adjusted according to the existing heuristics.

>> +{
>> +	unsigned long nslabs, size = swiotlb_size_or_default();
>> +
>> +	if (swiotlb_default_size_changed() ||
>> +	    !cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>> +		return size;
>
> And this makes for another needlessly convoluted calling convention.
>

I have updated this to

+static unsigned long __init swiotlb_adjusted_size(void)
+{
+	unsigned long nslabs;
+	u64 size = swiotlb_default_pool_size();
+
+	if (!swiotlb_cmdline_size_set &&
+	    cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
+		/*
+		 * For SEV and TDX and CCA, 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.
+		 *
+		 * 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 = div_u64((u64)memblock_phys_mem_size() * 6, 100);
+		size = clamp_val(size, IO_TLB_DEFAULT_SIZE, SZ_1G);
+	}
+
+	nslabs = swiotlb_calc_nslabs(size, default_nareas);
+
+	return nslabs << IO_TLB_SHIFT;
+}
+

But as shown above, we apply the CoCo sizing heuristic only when no
explicit swiotlb= size was specified and guest memory encryption is
enabled.

>
>> +	/*
>> +	 * For SEV and TDX and CCA, 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.
>> +	 *
>> +	 * 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 = memblock_phys_mem_size() * 6 / 100;
>
> But mostly I fail to see how this makes any sense for the x86 
> crash_low_size_default() case anyway. This calculation is based on the 
> *total* system memory, of which 6% is likely comparable to (or perhaps 
> even more than) the *entire* amount of memory reserved for the crash 
> kernel itself. What's more, if the low memory and size restrictions are 
> lifted for regular CoCo SWIOTLB as people want, then it becomes even 
> more utterly nonsensical to tie crashkernel_low to this.
>
> Yes, this happens to be the behaviour that falls out of how two 
> different parts of the existing code interact, but I highly doubt it was 
> ever intentional, so I'm not convinced that complicating SWIOTLB 
> interfaces to blindly preserve it is the right thing to do.
>


I agree with the concern, but what would be the right solution? When the
crash kernel boots, it will try to allocate the SWIOTLB pool according
to this sizing heuristic, and the allocation may fail if insufficient
memory was reserved.

I have split the x86 crash-kernel changes into a separate patch, which
we can drop depending on the conclusion here. I must admit that I do not
understand the crash-kernel memory restrictions and allocation details
well enough to propose a solution.

-aneesh