Hi Marek,
On 1/5/23 13:11, Marek Vasut wrote:
On 1/5/23 11:03, Patrick DELAUNAY wrote:
Hi,
Hi,
On 1/5/23 02:22, Marek Vasut wrote:
Do not access gd->ram_size and assume this is actual valid RAM size.
Since commit
777aaaa706b ("common/memsize.c: Fix get_effective_memsize() to check
for overflow")
the RAM size may be less than gd->ram_size , call
get_effective_memsize() to get
the limited value instead.
The aforementioned commit makes STM32MP15xx boards with 1 GiB of DRAM
at 0xc0000000 hang on boot, which is a grave defect.
Signed-off-by: Marek Vasut <ma...@denx.de>
---
Cc: Pali Rohar <p...@kernel.org>
Cc: Patrice Chotard <patrice.chot...@foss.st.com>
Cc: Patrick Delaunay <patrick.delau...@foss.st.com>
Cc: Tom Rini <tr...@konsulko.com>
---
arch/arm/mach-stm32mp/dram_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-stm32mp/dram_init.c
b/arch/arm/mach-stm32mp/dram_init.c
index 9346fa8546d..80ba5c27741 100644
--- a/arch/arm/mach-stm32mp/dram_init.c
+++ b/arch/arm/mach-stm32mp/dram_init.c
@@ -51,7 +51,7 @@ phys_size_t board_get_usable_ram_top(phys_size_t
total_size)
/* found enough not-reserved memory to relocated U-Boot */
lmb_init(&lmb);
- lmb_add(&lmb, gd->ram_base, gd->ram_size);
+ lmb_add(&lmb, gd->ram_base, get_effective_memsize());
boot_fdt_add_mem_rsv_regions(&lmb, (void *)gd->fdt_blob);
/* add 8M for reserved memory for display, fdt, gd,... */
size = ALIGN(SZ_8M + CONFIG_SYS_MALLOC_LEN + total_size,
MMU_SECTION_SIZE),
Thanks for the patch
Reviewed-by: Patrick Delaunay <patrick.delau...@foss.st.com>
Thank you.
Even if I don't uderstood where the code blocked here...
I need to cros-check it.
Please do, thank you.
Tested, see after....
But I think it is more a regression introduced by
777aaaa706b ("common/memsize.c: Fix get_effective_memsize() to check
for overflow")
I agree that commit likely breaks more systems than just STM32MP15xx ,
and I am tempted to say, it should be reverted before release.
Tom ?
in this patch I don't understood the assumption:
* It is required that ram_base + ram_size must be representable by
* phys_size_t type and must be aligned by direct access
why add now this limitation ?
why phys_size_t here and not phys_addr_t or unsigned long, the
ram_base type ?
why align on 4kB and not on MMU_SECTION_SIZE ?
until now on ARM 32bits U-Boot is working with
ram_base = 0xC0000000 (unsigned long)
ram_size = 0x40000000 (phys_size_t)
overflow for ram_top (phys_addr_t) = ram_base +
get_effective_memsize() was previously managed in the code
as we work only on base address + size (in board_f.c, in lmb library)
I correct in the code overflow issues, for example in commit
54be09cd8f6e ("arm: caches: manage phys
addr_t overflow in mmu_set_region_dcache_behaviour").
moreover for me it is strange to reduce the size of the DDR banks by
default
(see resutl of bdinfo command) only just to avoid overflow:
common/board_f.c:267: gd->bd->bi_dram[0].size =
get_effective_memsize();
For me the real assumption is on the end of RAM:
"ram_base + ram_size - 1" must be representable by 'ram_base' type
(phys_addr_t or unsigned long ?)
+
+ /*
+ * Check for overflow and limit ram size.
+ * It is required that ram_base + ram_size - 1 must be
representable
+ * by ram_base type, unsigned long.
+ */
+ if (gd->ram_base - 1 + ram_size < gd->ram_base)
+ ram_size = ((unsigned long)~0x0ULL) - gd->ram_base;
=> no need to remove 4KB is here... base + size can reach the MAX
value for the type + 1
Note that this -4 kiB is exactly what triggers the problem , i.e. if
get_effective_memsize() returns 0x4000_0000 , all is fine, if it
returns 0x3fff0000 then the system hangs. I also tried subtracting
MMU_SECTION_SIZE instead of 4k and that does not help, system still
hangs.
ok
I tested on STM32MP157C-EV1 on my side...
with 1GiB mermory size
U-Boot is booting on next TOP (for trusted boot with TF-A and OP-TEE)
U-Boot 2023.01-rc4-00386-gb429e78942de (Jan 05 2023 - 17:44:04 +0100)
stm32_smc: Failed to exec svc=82001003 op=1 in secure mode (err = -1)
stm32_smc: Failed to exec svc=82001003 op=1 in secure mode (err = -1)
CPU: STM32MP157C?? Rev.Z
Model: STMicroelectronics STM32MP157C eval daughter on eval mother
Board: stm32mp1 in trusted mode (st,stm32mp157c-ev1)
stm32_smc: Failed to exec svc=82001003 op=1 in secure mode (err = -1)
DRAM: 1 GiB
....
STM32MP> bdinfo
boot_params = 0x00000000
DRAM bank = 0x00000000
-> start = 0xc0000000
-> size = 0x3ffff000 <<<<<< the patch is applied - 4kiB here !!!
flashstart = 0x00000000
flashsize = 0x00000000
flashoffset = 0x00000000
baudrate = 115200 bps
relocaddr = 0xfdb1d000
reloc off = 0x3da1d000
Build = 32-bit
current eth = ethernet@5800a000
ethaddr = 00:80:e1:01:60:da
IP addr = <NULL>
fdt_blob = 0xfbb040a0
new_fdt = 0xfbb040a0
fdt_size = 0x00016de0
Video = display-controller@5a001000 inactive
lmb_dump_all:
memory.cnt = 0x1
memory[0] [0xc0000000-0xffffefff], 0x3ffff000 bytes flags: 0
<<<<
reserved.cnt = 0x6
reserved[0] [0x10000000-0x10045fff], 0x00046000 bytes flags: 4
reserved[1] [0x30000000-0x3003ffff], 0x00040000 bytes flags: 4
reserved[2] [0x38000000-0x3800ffff], 0x00010000 bytes flags: 4
reserved[3] [0xe8000000-0xefffffff], 0x08000000 bytes flags: 4
reserved[4] [0xfbaffbb8-0xfdffffff], 0x02500448 bytes flags: 0
reserved[5] [0xfe000000-0xffffffff], 0x02000000 bytes flags: 4 <<<<
devicetree = board
arch_number = 0x00000000
TLB addr = 0xfdff0000
irq_sp = 0xfbb03e10
sp start = 0xfbb03e00
Early malloc usage: 2924 / 20000
STM32MP>
But we have strange LMB reserved memory [0xfe000000-0xffffffff] from DT
=> it is outside of memory[0] [0xc0000000-0xffffefff]
I think the -4 kiB should be removed at least....
I test also with basic boot = SPL and U-Boot, no issue on my side
Patrick
