On 08.10.2026 16:32, Aneesh Kumar K.V wrote: > Catalin Marinas <[email protected]> writes: >> On Thu, Oct 08, 2026 at 11:03:27AM +0530, Aneesh Kumar K.V wrote: >>> Aneesh Kumar K.V <[email protected]> writes: >>>> Will Deacon <[email protected]> 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); > } > > Marek, > > Patch 3 is a fix, so you may want to take it even if we drop the rest of > the series. Perhaps the first three patches could be taken together?
Okay, I will take only the first 3 patches to dma-mapping-for-next branch now and wait for the remaining patches until they gets sorted out. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
