Re: [PATCH v5 1/6] dma: swiotlb: Centralize default pool policy selection

Robin Murphy [off-list ref] writes:

On 21/09/2026 7:36 am, Aneesh Kumar K.V (Arm) wrote:
[...]

quoted

diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
index ba256c37bcc0..97fffa46f05a 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c

@@ -14,8 +14,10 @@ unsigned int ppc_swiotlb_flags;
  
  void __init swiotlb_detect_4g(void)
  {
-	if ((memblock_end_of_DRAM() - 1) > 0xffffffff)
+	if ((memblock_end_of_DRAM() - 1) > 0xffffffff) {
  		ppc_swiotlb_enable = 1;

Nit: Perhaps it's beyond the functional scope of this patch, but I do 
wonder if these <arch>_swiotlb_enable conditions become redundant and 
could just be replaced with "if (<arch>_swiotlb_flags != 0)" too.

Possibly. However, x86 has a case where x86_swiotlb_enable is cleared
and used to free the swiotlb pool during initialization.

pci_iommu_init() {
	if (x86_swiotlb_enable) {
		pr_info("PCI-DMA: Using software bounce buffering for IO (SWIOTLB)\n");
		swiotlb_print_info();
	} else {
		swiotlb_exit();
	}
}

quoted

+		ppc_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+	}
  }
  
  static int __init check_swiotlb_enabled(void)

[...]

quoted

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 75cf8f6ae8cd..0cff255827ba 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c

@@ -44,8 +44,10 @@ static unsigned int x86_swiotlb_flags;
  static void __init pci_swiotlb_detect(void)
  {
  	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
-	if (!no_iommu && max_possible_pfn > MAX_DMA32_PFN)
+	if (!no_iommu && max_possible_pfn > MAX_DMA32_PFN) {
  		x86_swiotlb_enable = true;
+		x86_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
+	}
  
  	/*
  	 * Set swiotlb to 1 so that bounce buffers are allocated and used for

@@ -81,8 +83,10 @@ static void __init pci_xen_swiotlb_init(void)
  	if (!xen_swiotlb_enabled())
  		return;
  	x86_swiotlb_enable = true;
-	x86_swiotlb_flags |= SWIOTLB_ANY;
-	swiotlb_init_remap(true, x86_swiotlb_flags, xen_swiotlb_fixup);
+	/* Xen can use a SWIOTLB pool anywhere in directly mapped memory. */
+	x86_swiotlb_flags &= ~SWIOTLB_INIT_ADDRESSING_LIMIT;
+	x86_swiotlb_flags |= SWIOTLB_INIT_REMAP | SWIOTLB_ANY;

Similarly, I see how having a SWIOTLB_INIT_REMAP flag keeps 
swiotlb_select_pool_policy() neat, but couldn't we technically already 
infer that from the fact that a non-NULL remap function was passed?

I thought deriving the entire pool policy from the flags made the code
easier to follow. For example, if flags == 0 and remap != NULL, we would
also need to pass remap to swiotlb_select_pool_policy() to determine the
policy.

Nits aside, the overall change seems reasonable to me;

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Thanks
-aneesh