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

[Aneesh Kumar K . V]

Kohei Enju <enju.kohei@xxxxxxxxxxx> writes:

> On 10/08 20:02, Aneesh Kumar K.V wrote:
>> Catalin Marinas <catalin.marinas@xxxxxxx> writes:
>> 
>> > On Thu, Oct 08, 2026 at 11:03:27AM +0530, Aneesh Kumar K.V wrote:
>> >> 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?
>> >
>> > Even the rmem check for each device is still a hack that may bite us in
>> > the future (private devices for example would not need swiotlb). I'm
>> > thinking more and more of leaving the sizing an arch-specific decision,
>> > don't bother generalising it at all.
>> >
>> > On pKVM vs CCA guests, there's really nothing specific here to pKVM
>> > guests. The only difference is that confidential guests that so far have
>> > run without a swiotlb buffer will regress if their memory is tight. For
>> > confidential guests without dedicated rmem (either CCA or pKVM), I think
>> > our options are either command line swiotlb sizing or dynamic swiotlb.
>> >
>> > Could you respin your series while leaving out the generic sizing? IOW,
>> > no x86 code generalisation. We can discuss the best strategy on sizing
>> > later (I haven't checked how much of this series still makes sense
>> > without the generic sizing).
>> >
>> 
>> It would mostly consist of the first three cleanup patches, followed by
>> three patches that replace the addressing_limit argument with flags.
>> 
>> #define SWIOTLB_VERBOSE	(1 << 0) /* verbose initialization */
>> /* Initialize a pool for devices with limited DMA addressing. */
>> #define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 1)
>> /* Initialize a pool that requires architecture remapping. */
>> #define SWIOTLB_INIT_REMAP		(1 << 2)
>> /* Initialize a pool for DMA to memory-encrypted host or guest memory. */
>> #define SWIOTLB_INIT_MEM_ENCRYPT		(1 << 3)
>> /* Initialize a pool for unaligned kmalloc bouncing. */
>> #define SWIOTLB_INIT_KMALLOC		(1 << 4)
>> /* Do not initialize a pool unless SWIOTLB is explicitly required. */
>> #define SWIOTLB_INIT_DEFAULT_OFF		(1 << 5)
>> 
>> This results in the large change below. I'm not sure we want to do this
>> for no real benefit other than making swiotlb_should_init() slightly
>> easier to follow.
>> 
>> static bool __init swiotlb_should_init(unsigned int flags)
>> {
>> 	if (swiotlb_force_disable)
>> 		return false;
>> 
>> 	if (swiotlb_force_bounce)
>> 		return true;
>> 
>> 	if (flags & (SWIOTLB_INIT_REMAP | SWIOTLB_INIT_MEM_ENCRYPT)))
>> 		return true;
>> 
>> 	/* Explicit requirements override an architecture's default opt-out. */
>> 	if (flags & SWIOTLB_INIT_DEFAULT_OFF)
>> 		return false;
>> 
>> 	return flags & (SWIOTLB_INIT_ADDRESSING_LIMIT | SWIOTLB_INIT_KMALLOC);
>> }
>>  [...]
>
> Hi Aneesh,
>
> I can reproduce the CCA regression discussed in this thread:
>   https://lore.kernel.org/all/yq5azeyxyfol.fsf@xxxxxxxxxx/#t
>
> The guest runs out of SWIOTLB slots, causing disk I/O to fail and
> preventing the guest from booting.
>
> I would like to test the fix on real CCA hardware. Could you please Cc
> me when you respin the series? I follow LKML, but I may still miss the
> posting by chance.
>

The conclusion was to either use CONFIG_SWIOTLB_DYNAMIC or swiotlb kernel command line
option

-aneesh