Re: [PATCH v4 5/5] dma: swiotlb: Remove SWIOTLB_ANY

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

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Subsystem guideline violation: Use of globally uninitialized high_memory in __init code
> --
>
> commit 61bbab3b84e605312f21e75e603675cd29154afc
> Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>
> dma: swiotlb: Remove SWIOTLB_ANY
>
> This patch replaces SWIOTLB_ANY with SWIOTLB_INIT_ADDRESSING_LIMIT to
> explicitly identify callers that need pools reachable by devices with
> limited DMA addressing. It changes the default behavior to allow static
> and dynamic pools anywhere in directly mapped memory when the limit flag
> is omitted.
>
> [ ... ]
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index 6695da682f961..f8a77926b8b09 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
> [ ... ]
>> @@ -604,10 +599,10 @@ void __init swiotlb_init_remap(unsigned int flags,
>>  #ifdef CONFIG_SWIOTLB_DYNAMIC
>>  	if (!remap)
>>  		io_tlb_default_mem.can_grow = true;
>> -	if (flags & SWIOTLB_ANY)
>> -		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
>> -	else
>> +	if (flags & SWIOTLB_INIT_ADDRESSING_LIMIT)
>>  		io_tlb_default_mem.phys_limit = ARCH_LOW_ADDRESS_LIMIT;
>> +	else
>> +		io_tlb_default_mem.phys_limit = virt_to_phys(high_memory - 1);
>
> [Severity: High]
> This is a pre-existing issue, but does this evaluation of high_memory in an
> __init function risk calculating an incorrect physical limit?
>
> The swiotlb_init_remap function in kernel/dma/swiotlb.c evaluates
> virt_to_phys(high_memory - 1) during early kernel boot. However, the
> high_memory global is uninitialized and evaluates to zero until
> free_area_init() runs. Calculating a physical limit using high_memory here
> can result in a bogus DMA boundary (such as zero or an underflowed limit),
> which could lead to memory allocation failures or data corruption during
> DMA mapping.
>
> Could this code use memblock_end_of_DRAM() instead of high_memory during
> early boot allocations?
>


But swiotlb_init() is called after set_high_memory()

start_kernel()
  |
  +--> mm_core_init_early()
  |      |
  |      +--> free_area_init()
  |             |
  |             +--> set_high_memory()
  |
  +--> mm_core_init()
         |
         +--> arch_mm_preinit()
                |
                +--> swiotlb_init()

-aneesh