Re: [PATCH v6 6/8] dma: swiotlb: Centralize memory-encryption pool sizing

[Aneesh Kumar K . V]

Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxx> writes:

> Will Deacon <will@xxxxxxxxxx> writes:
>
>> On Wed, Oct 07, 2026 at 11:04:05AM +0100, Catalin Marinas wrote:
>>> On Tue, Oct 06, 2026 at 10:49:17PM +0100, Will Deacon wrote:
>>> > On Thu, Sep 24, 2026 at 11:37:54AM +0530, Aneesh Kumar K.V (Arm) wrote:
>>> > > @@ -496,7 +516,8 @@ swiotlb_select_pool_policy(unsigned int flags)
>>> > >  	if (swiotlb_force_disable)
>>> > >  		return SWIOTLB_POOL_NONE;
>>> > >  
>>> > > -	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>>> > > +	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
>>> > > +	    !restricted_dma_pool_present)
>>> > >  		return SWIOTLB_POOL_CC_GUEST;
>>> > 
>>> > I think this check on the restricted DMA pool is too general -- the pool
>>> > could be tied to a specific DMA-capable peripheral and so treating its
>>> > presence as a global property isn't right.
>>> 
>>> I agree it's a hack but that was the simplest way to avoid the pVMs
>>> getting a bounce buffer after this patch. More than happy to leave it
>>> out and reduce the buffer on cmdline or we come up with some better
>>> heuristics.
>>
>> Hrm, that does mean that reverting just this part will regress pVMs
>> because they'll suddenly be allocating a tonne more memory for an
>> entirely unused swiotlb buffer. So I think I'd prefer to drop the entire
>> series until this has been worked out properly.
>>
>>> Another option could be the arch code passing another flag that it
>>> doesn't want an encrypted pool (e.g. when running in a pKVM guest) but I
>>> don't particularly this either. The arch code doesn't know whether
>>> there's an alternative pool.
>>
>> At that point, the default size may as well be driven by the
>> drivers/virt/coco driver.
>>
>>> That said, such heuristics should have been a separate patch to make it
>>> easier to review/drop.
>>
>> I think the only right way to get a semi-accurate heuristic is to take
>> into account the set of dma-capable devices that will use the swiotlb
>> pool, but that's fiddly and should probably be tackled as a separate
>> series. Maybe a simpler hack in that direction would be to take the
>> SWIOTLB_POOL_CC_GUEST if _any_ device is going to use swiotlb? You'll
>> run into the usual problem of not being able to tell if a device is
>> DMA-capable or not, but you could probably look for a global restricted
>> DMA pool and, if that doesn't exist, check for per-device restricted pools
>> on dma-coherent devices (since restricted DMA isn't supported by ACPI) as
>> a reasonable approximation.
>
> So, something like this?
>
> 	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
> 	    swiotlb_cc_guest_needs_default_pool())
> 		return SWIOTLB_POOL_CC_GUEST;
>

Detecting a DMA-capable device is not straightforward, and if we get it
wrong, we will enable SWIOTLB_POOL_CC_GUEST unnecessarily. Would the
code below be a reasonable approximation of what you suggested?

Another option would be to make swiotlb_cc_guest_needs_default_pool() a
weak function that architectures can override. arm64 pKVM could then use
a different scheme (for this patch series default to false). Would that
be preferable?

#ifdef CONFIG_DMA_RESTRICTED_POOL
static bool __init swiotlb_of_dma_candidate(struct device_node *np)
{
	struct device_node *node __free(device_node) = of_node_get(np);

	/*
	 * Ignore nodes that don't have compatible and reg property
	 * So we don't wrongly consider a node as device node.
	 */
	if (!of_property_present(np, "compatible") ||
	    !of_property_present(np, "reg"))
		return false;

	while (node) {
		if (!of_device_is_available(node))
			return false;
		/*
		 * Ignore reserved-memory nodes because that have compatible
		 * and reg property
		 */
		if (node->parent == of_root &&
		    of_node_name_eq(node, "reserved-memory"))
			return false;
		node = of_get_next_parent(node);
	}
	return of_dma_is_coherent(np);
}

static bool __init swiotlb_of_dma_needs_default_pool(struct device_node *np)
{
	if (!of_dma_get_restricted_pool(np))
		return true;

	return false;
}
#endif

static bool __init swiotlb_cc_guest_needs_default_pool(void)
{
#ifdef CONFIG_DMA_RESTRICTED_POOL
	struct device_node *np;
	bool found = false;

	if (!of_root)
		return true;
	/*
	 * DMA capability is not explicitly described for every DT device.
	 * Use coherent, addressed device nodes as an approximation, and keep
	 * guest sizing unless every candidate has an initialized restricted
	 * pool.
	 */
	for_each_of_allnodes(np) {
		if (!swiotlb_of_dma_candidate(np))
			continue;
		found = true;
		if (swiotlb_of_dma_needs_default_pool(np))
			return true;
	}

	return !found;
#else
	return true;
#endif
}