[PATCH 01/11] imx: implement get_effective_memsize

Peng Fan peng.fan at oss.nxp.com
Wed Nov 16 16:55:19 CET 2022


Hi Stefan,

On 11/16/2022 2:03 PM, Stefan Roese wrote:
> Hi Peng,
> 
> please find one late comment from me below...
> 
> On 16.11.22 03:43, Peng Fan wrote:
>>
>> +Tom
>>
>> On 11/9/2022 3:40 PM, Pali Rohár wrote:
>>> On Wednesday 09 November 2022 09:48:53 Peng Fan wrote:
>>>> On 11/8/2022 4:03 PM, Pali Rohár wrote:
>>>>> On Tuesday 08 November 2022 07:56:59 Peng Fan wrote:
>>>>>>> Subject: Re: [PATCH 01/11] imx: implement get_effective_memsize
>>>>>>>
>>>>>>> On Tuesday 08 November 2022 09:38:01 Peng Fan wrote:
>>>>>>>> On 11/7/2022 3:55 PM, Pali Rohár wrote:
>>>>>>>>> On Monday 07 November 2022 16:00:06 Peng Fan (OSS) wrote:
>>>>>>>>>> From: Peng Fan <peng.fan at nxp.com>
>>>>>>>>>>
>>>>>>>>>> To i.MX6/7 which has 2GB memory, the upper 4KB cut off, will cause
>>>>>>>>>> the top 1MB not mapped as normal memory, because ARMV7-A use
>>>>>>>>>> section mapping. So implement i.MX6/7 specific
>>>>>>>>>> get_effective_memsize to fix the issue.
>>>>>>>>>>
>>>>>>>>>> Fixes: 777aaaa706bc("common/memsize.c: Fix get_effective_memsize()
>>>>>>>>>> to check for overflow")
>>>>>>>>>> Signed-off-by: Peng Fan <peng.fan at nxp.com>
>>>>>>>>>
>>>>>>>>> Should not just configuring CONFIG_MAX_MEM_MAPPED properly avoid
>>>>>>> that issue?
>>>>>>>>
>>>>>>>> No, unless I decrease PHYS_SDRAM_SIZE.
>>>>>>>
>>>>>>> So, what is the issue? I just do not see what happens after
>>>>>>> 777aaaa706bc
>>>>>>> that RAM size is calculated incorrectly in your case. I did not
>>>>>>> catch the
>>>>>>> description from commit message. What are your gd->ram_size and
>>>>>>> CONFIG_MAX_MEM_MAPPED values that current code does not work?
>>>>>>
>>>>>> The base is 2GB, the size is 2GB. With CONFIG_MAX_MEM_MAPPED,
>>>>>> the ram_size already decreased by 4KB.
>>>>>
>>>>> If base is 2GB and size is 2GB then ram_top is 4GB which cannot be
>>>>> represented in 32-bit phys_size_t type and hence some of other u-boot
>>>>> functions use 0 as ram_top. Mentioned commit tries to fix this issue.
>>>>> I guess that you have some other hidden problem and my change just
>>>>> showed implication of that.
>>>>
>>>> The issue is with higher 4KB cut off, the MMU mapping will cut off
>>>> 1MB or 2MB(section mapping), so after MMU enabled, the PC instruction
>>>> will
>>>> not able to fetch instruction in the higher 1MB area because of U-Boot
>>>> relocated to
>>>> top of DRAM.
>>>
>>> But it is not possible to represent whole 4GB of RAM size in 32-bit
>>> phys_size_t type. So some cut-off for this storage is required.
>>
>> With your commit
>> Ram size: 80000000
>> Ram top: FFFFF000
> 
> Looks good to me.
> 
>> Your patch would break all i.MX6/7 boards that 2GB memory with
>> base starts at 0x80000000.
> 
> Why is this the case?
> 
>> After apply this patch, It works, even if RAM top is 0.
>> Ram size: 80000000
>> Ram top: 00000000
> 
> Ughhh. This looks broken to me. Address 0x0000.0000 is a valid address
> and points to 0! In this case the code just seems to work (as seen
> below) as 0 is wrapped around since it's a 32bit value.
> 
>> full log below:
>> U-Boot 2022.10-00346-gd80f7fc140a-dirty (Nov 16 2022 - 11:24:56 +0800)
>>
>> initcall: 8780f549
>> U-Boot code: 87800000 -> 878562BC  BSS: -> 8785D998
>> initcall: 8780f43d
>> initcall: 87801fe1
>> CPU:   Freescale i.MX6SLL rev1.0 at 792MHz
>> CPU:   Commercial temperature grade (0C to 95C)
>> Reset cause: POR
>> initcall: 8780fb21
>> Model: Freescale i.MX6SLL EVK Board
>> Board: MX6SLL EVK
>> initcall: 8780f5b9
>> initcall: 8780f5a9
>> DRAM:  initcall: 8780356d
>> initcall: 8780f87d
>> Monitor len: 0005D998
>> Ram size: 80000000
>> Ram top: 00000000
>> initcall: 8780f429
>> initcall: 87801d17
>> initcall: 8780f5eb
>> initcall: 8780f5ef
>> initcall: 8780f501
>> Reserving 374k for U-Boot at: fff92000
> 
> Here we've got the wrap around.
> 
> So why does it not work with the Pali's original code leading to a
> ram-top of 0xFFFFF000? Where does it fail? Could you please post
> this bootlog as well?

Not has board at hand for now.

0xFFFFF000 only has the top 4KB cut-off, but when we
set up MMU mapping, we use section mapping, so the top 1MB will
be cut off when do the mmu mapping.

         for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT;
              i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) +
                  (bd->bi_dram[bank].size >> MMU_SECTION_SHIFT);
              i++)
                 set_section_dcache(i, DCACHE_DEFAULT_OPTION);

Then the code/data between 0xFF000000 - 0xFFFFF000 are not mapped,
cause boot hang.

Thanks,
Peng.

> 
> Thanks,
> Stefan
> 
>> initcall: 8780f611
>> Reserving 16392k for malloc() at: fef90000
>> initcall: 8780f56d
>> Reserving 68 Bytes for Board Info at: fef8ffb0
>> initcall: 8780f63d
>> Reserving 248 Bytes for Global Data at: fef8feb0
>> initcall: 8780f669
>> Reserving 25600 Bytes for FDT at: fef89ab0
>> initcall: 8780f5f3
>> initcall: 8780f5f7
>> initcall: 8780f607
>> initcall: 8780f8e1
>> initcall: 8780f441
>> initcall: 8780f6c1
>>
>> RAM Configuration:
>> Bank #0: 80000000 2 GiB
>>
>> DRAM:  2 GiB
>> initcall: 8780f8f3
>> initcall: 8780f4ed
>> New Stack Pointer is: fef89a90
>> initcall: 8780f45b
>> initcall: 8780f5fb
>> initcall: 8780f5ff
>> initcall: 8780f48d
>> Relocation Offset is: 78792000
>> Relocating to fff92000, new gd at fef8feb0, sp at fef89a90
>> initcall: 8780f60b
>> initcall: 8780f5e3
>> Core:  74 devices, 16 uclasses, devicetree: separate
>> MMC:   FSL_SDHC: 0, FSL_SDHC: 2
>> Loading Environment from MMC... MMC: no card present
>> *** Warning - No block device, using default environment
>>
>> In:    serial at 2020000
>> Out:   serial at 2020000
>> Err:   serial at 2020000
>> Net:   No ethernet found.
>> Hit any key to stop autoboot:  0
>>
>>
>>>
>>>> Regards,
>>>> Peng.
>>>>
>>>> Could you check how is gd->ram_top
>>>>> configured with and without this change?
>>>
>>> Could you check this?
>>>
>>>>>> Regards,
>>>>>> Peng.
>>>>>>
>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Peng.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>>      arch/arm/mach-imx/cache.c | 14 ++++++++++++++
>>>>>>>>>>      1 file changed, 14 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/mach-imx/cache.c b/arch/arm/mach-imx/cache.c
>>>>>>>>>> index ab9b621a2a6..69a085abee7 100644
>>>>>>>>>> --- a/arch/arm/mach-imx/cache.c
>>>>>>>>>> +++ b/arch/arm/mach-imx/cache.c
>>>>>>>>>> @@ -7,10 +7,24 @@
>>>>>>>>>>      #include <cpu_func.h>
>>>>>>>>>>      #include <asm/armv7.h>
>>>>>>>>>>      #include <asm/cache.h>
>>>>>>>>>> +#include <asm/global_data.h>
>>>>>>>>>>      #include <asm/pl310.h>
>>>>>>>>>>      #include <asm/io.h>
>>>>>>>>>>      #include <asm/mach-imx/sys_proto.h>
>>>>>>>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>>>>>>> +
>>>>>>>>>> +phys_size_t get_effective_memsize(void) { #ifndef
>>>>>>>>>> +CONFIG_MAX_MEM_MAPPED
>>>>>>>>>> +    return gd->ram_size;
>>>>>>>>>> +#else
>>>>>>>>>> +    /* limit stack to what we can reasonable map */
>>>>>>>>>> +    return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
>>>>>>>>>> +        CONFIG_MAX_MEM_MAPPED : gd->ram_size); #endif }
>>>>>>>>>> +
>>>>>>>>>>      void enable_ca7_smp(void)
>>>>>>>>>>      {
>>>>>>>>>>          u32 val;
>>>>>>>>>> -- 
>>>>>>>>>> 2.36.0
>>>>>>>>>>
> 
> Viele Grüße,
> Stefan Roese
> 


More information about the U-Boot mailing list