Robin Murphy <[email protected]> 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 <[email protected]>
Thanks
-aneesh
