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@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@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@2020000 Out: serial@2020000 Err: serial@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