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

[Kohei Enju <enju.kohei@fujitsu.com>]

On 10/08 20:02, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
> 
> > On Thu, Oct 08, 2026 at 11:03:27AM +0530, Aneesh Kumar K.V wrote:
> >> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
> >> > Will Deacon <will@kernel.org> 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@kernel.org/#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.

Thanks,
Kohei