[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8b8c2b0a-3629-4cde-afa4-18bc20e0f5df@oss.qualcomm.com>
Date: Tue, 9 Dec 2025 16:29:00 -0800
From: Oreoluwa Babatunde <oreoluwa.babatunde@....qualcomm.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>, Ye Li <ye.li@....nxp.com>,
robh@...nel.org, robin.murphy@....com
Cc: saravanak@...gle.com, quic_obabatun@...cinc.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
iommu@...ts.linux.dev, william.zhang@...adcom.com,
kernel@....qualcomm.com, will@...nel.org, djakov@...nel.org,
aisheng.dong@....com, joy.zou@....com, frank.li@....com,
jason.hui.liu@....com
Subject: Re: [PATCH v4] of: reserved_mem: Restructure call site for
dma_contiguous_early_fixup()
On 12/1/2025 10:54 PM, Oreoluwa Babatunde wrote:
>
>
> On 11/30/2025 11:51 PM, Marek Szyprowski wrote:
>> On 01.12.2025 07:31, Oreoluwa Babatunde wrote:
>>> On 11/28/2025 4:43 AM, Marek Szyprowski wrote:
>>>> On 26.11.2025 02:37, Ye Li wrote:
>>>>> On 8/11/2025 7:07 PM, Marek Szyprowski wrote:
>>>>>> On 06.08.2025 19:24, Oreoluwa Babatunde wrote:
>>>>>>> Restructure the call site for dma_contiguous_early_fixup() to
>>>>>>> where the reserved_mem nodes are being parsed from the DT so that
>>>>>>> dma_mmu_remap[] is populated before dma_contiguous_remap() is called.
>>>>>>>
>>>>>>> Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved
>>>>>>> memory regions are processed")
>>>>>>> Signed-off-by: Oreoluwa Babatunde <oreoluwa.babatunde@....qualcomm.com>
>>>>>>> Tested-by: William Zhang <william.zhang@...adcom.com>
>>>>>> Thanks, applied to dma-mapping-fixes branch.
>>>>>>
>>>>>> Best regards
>>>>> Hi Oreoluwa,
>>>>>
>>>>> We observed this patch causing kernel boot hang on iMX6 (armv7)
>>>>> platforms if using "cma=" kernel parameter. It only happens when the
>>>>> size assigned in
>>>>> "cma=" parameter is smaller than cma default size in dts.
>>>>>
>>>>> For example, we use "cma=96M" in command line and below reserved
>>>>> memory node (160M) in dts.
>>>>>
>>>>> reserved-memory {
>>>>> #address-cells = <1>;
>>>>> #size-cells = <1>;
>>>>> ranges;
>>>>>
>>>>> linux,cma {
>>>>> compatible = "shared-dma-pool";
>>>>> reusable;
>>>>> size = <0xa000000>;
>>>>> linux,cma-default;
>>>>> };
>>>>> };
>>>>>
>>>>> The root cause is this patch moving the dma_contiguous_early_fixup
>>>>> from rmem_cma_setup to __reserved_mem_alloc_size. rmem_cma_setup can
>>>>> skip the cma reserved memory if command line has cma parameter.
>>>>> However, the __reserved_mem_alloc_size won't do it. So this leads to
>>>>> have two cma regions added to dma_mmu_remap, one from dts, the other
>>>>> from command line. But the reserved memory of memblock that only
>>>>> records the cma from command line is inconsistent with dma_mmu_remap.
>>>>> The dma_contiguous_remap clears the MMU paging for the region of
>>>>> dma_mmu_remap firstly, then create a new mapping by iotable_init. For
>>>>> the cma from dts, this causes incorrect memory mapping cleared. Then
>>>>> any allocation from memblock in iotable_init hitting to the area will
>>>>> meet MMU mapping issue.
>>>>>
>>> Hi Ye Li,
>>>
>>> Thanks for pointing this out. From what I see in the code, if "cma="
>>> kernel parameter is being used to configure the default cma region, then we
>>> should skip adding the DT defined region to dma_mmu_remap array.
>>>
>>> I will work on a fix which does this and share here when it is done.
>>
>> I wonder how to avoid adding more such checks to
>> drivers/of/of_reserved_mem.c and making this code even more tangled and
>> spaghetti-like... I've briefly scanned that code and it is already quite
>> hard to follow, especially after commits 8a6e02d0c00e ("of:
>> reserved_mem: Restructure how the reserved memory regions are
>> processed") and 2c223f7239f3 ("of: reserved_mem: Restructure call site
>> for dma_contiguous_early_fixup()")... I wonder how many reserved memory
>> regions are used on real machines? Maybe instead of complicating this
>> code even more it is enough to make this configurable via Kconfig and
>> restore pre-8a6e02d0c00e version?
>
> Hi Marek,
>
> There was a change which attempted a simpler approach of increasing the
> size of the static array:
> https://lore.kernel.org/all/1650488954-26662-1-git-send-email-quic_pdaly@quicinc.com/
>
> The comment from Rob at the time was to revive another thread which attempted
> to dynamically allocate the reserved_mem array like we are doing now.
> Dynamic allocation gives more flexibility because we only use the exact
> amount of memory that is needed. This can save some memory if that ends
> up being smaller than what is specified in MAX_RESERVED_REGIONS.
>
> I do agree that adding another check in of_reserved_mem.c might not be the best
> in terms of code complexity, so I'm exploring other options on how to keep things
> simpler.
>
> Regards,
> Oreoluwa
>
Hi all,
Apologies for the delay in pushing a fix for this.
I've uploaded a fix now:
https://lore.kernel.org/all/20251210002027.1171519-1-oreoluwa.babatunde@oss.qualcomm.com/
I ended up adding some checks in the reserved_mem code to skip initializing
the default cma region when the "cma=" kernel param is being used. This seems
to be the most straight forward way of fixing this issue.
Also, this way makes sense because it allows the reserved_mem code to skip the
default cma node and not add it in the reserved_mem array since it will not be
used as a cma region eventually.
Please help me review and give some feedback.
Thanks,
Oreoluwa
Powered by blists - more mailing lists