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

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Robin Murphy <robin.murphy@arm.com> writes:

> On 21/09/2026 7:36 am, Aneesh Kumar K.V (Arm) wrote:
> [...]
>> 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();
	}
}

>
>> +		ppc_swiotlb_flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
>> +	}
>>   }
>>   
>>   static int __init check_swiotlb_enabled(void)
> [...]
>> 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