| Message ID | 20240607185240.1892031-9-sughosh.ganu@linaro.org |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Make U-Boot memory reservations coherent | expand |
On Sat, Jun 08, 2024 at 12:22:17AM +0530, Sughosh Ganu wrote: > With the changes to make the Logical Memory Block(LMB) allocations > persistent and with the common memory regions being reserved during > board init, the lmb_init_and_reserve() API can be removed and replaced > with a lmb_add_memory() API, which adds all the available memory to > the LMB pool. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > arch/arm/mach-apple/board.c | 2 +- > arch/arm/mach-snapdragon/board.c | 2 +- > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 2 +- > cmd/bdinfo.c | 2 +- > cmd/load.c | 2 +- > fs/fs.c | 2 +- > include/lmb.h | 12 +++++++++++- > lib/lmb.c | 15 +++++++++++---- > net/tftp.c | 2 +- > net/wget.c | 2 +- > test/cmd/bdinfo.c | 10 +--------- > 11 files changed, 31 insertions(+), 22 deletions(-) > > diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c > index c877c7b94c..2e72d03edd 100644 > --- a/arch/arm/mach-apple/board.c > +++ b/arch/arm/mach-apple/board.c > @@ -776,7 +776,7 @@ int board_late_init(void) > { > u32 status = 0; > > - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); > + lmb_add_memory(gd->bd); > > /* somewhat based on the Linux Kernel boot requirements: > * align by 2M and maximal FDT size 2M We already reserved gd->bd as part of the initr_lmb call. So I think this commit needs rethinking, or am I missing something?
On Mon, 10 Jun 2024 at 23:01, Tom Rini <trini@konsulko.com> wrote: > > On Sat, Jun 08, 2024 at 12:22:17AM +0530, Sughosh Ganu wrote: > > With the changes to make the Logical Memory Block(LMB) allocations > > persistent and with the common memory regions being reserved during > > board init, the lmb_init_and_reserve() API can be removed and replaced > > with a lmb_add_memory() API, which adds all the available memory to > > the LMB pool. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > arch/arm/mach-apple/board.c | 2 +- > > arch/arm/mach-snapdragon/board.c | 2 +- > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 2 +- > > cmd/bdinfo.c | 2 +- > > cmd/load.c | 2 +- > > fs/fs.c | 2 +- > > include/lmb.h | 12 +++++++++++- > > lib/lmb.c | 15 +++++++++++---- > > net/tftp.c | 2 +- > > net/wget.c | 2 +- > > test/cmd/bdinfo.c | 10 +--------- > > 11 files changed, 31 insertions(+), 22 deletions(-) > > > > diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c > > index c877c7b94c..2e72d03edd 100644 > > --- a/arch/arm/mach-apple/board.c > > +++ b/arch/arm/mach-apple/board.c > > @@ -776,7 +776,7 @@ int board_late_init(void) > > { > > u32 status = 0; > > > > - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); > > + lmb_add_memory(gd->bd); > > > > /* somewhat based on the Linux Kernel boot requirements: > > * align by 2M and maximal FDT size 2M > > We already reserved gd->bd as part of the initr_lmb call. So I think > this commit needs rethinking, or am I missing something? I believe the LMB memory API's also get called from SPL(not sure about TPL/VPL though). The memory that gets added in the other commit is for U-Boot main, post relocation. These calls will then be needed for prior stages of U-Boot that want to use LMB memory. -sughosh
On Tue, Jun 11, 2024 at 02:20:40PM +0530, Sughosh Ganu wrote: > On Mon, 10 Jun 2024 at 23:01, Tom Rini <trini@konsulko.com> wrote: > > > > On Sat, Jun 08, 2024 at 12:22:17AM +0530, Sughosh Ganu wrote: > > > With the changes to make the Logical Memory Block(LMB) allocations > > > persistent and with the common memory regions being reserved during > > > board init, the lmb_init_and_reserve() API can be removed and replaced > > > with a lmb_add_memory() API, which adds all the available memory to > > > the LMB pool. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > arch/arm/mach-apple/board.c | 2 +- > > > arch/arm/mach-snapdragon/board.c | 2 +- > > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 2 +- > > > cmd/bdinfo.c | 2 +- > > > cmd/load.c | 2 +- > > > fs/fs.c | 2 +- > > > include/lmb.h | 12 +++++++++++- > > > lib/lmb.c | 15 +++++++++++---- > > > net/tftp.c | 2 +- > > > net/wget.c | 2 +- > > > test/cmd/bdinfo.c | 10 +--------- > > > 11 files changed, 31 insertions(+), 22 deletions(-) > > > > > > diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c > > > index c877c7b94c..2e72d03edd 100644 > > > --- a/arch/arm/mach-apple/board.c > > > +++ b/arch/arm/mach-apple/board.c > > > @@ -776,7 +776,7 @@ int board_late_init(void) > > > { > > > u32 status = 0; > > > > > > - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); > > > + lmb_add_memory(gd->bd); > > > > > > /* somewhat based on the Linux Kernel boot requirements: > > > * align by 2M and maximal FDT size 2M > > > > We already reserved gd->bd as part of the initr_lmb call. So I think > > this commit needs rethinking, or am I missing something? > > I believe the LMB memory API's also get called from SPL(not sure about > TPL/VPL though). The memory that gets added in the other commit is for > U-Boot main, post relocation. These calls will then be needed for > prior stages of U-Boot that want to use LMB memory. We do likely make use of LMB at times in SPL, yes, for OS boot and that does remind me of cases people have had of loading image failures due to LMB-as-security-mechanism today. So we need to make a call to the new lmb init for real, once, rather than ad-hoc like this commit makes it.
diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index c877c7b94c..2e72d03edd 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -776,7 +776,7 @@ int board_late_init(void) { u32 status = 0; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); /* somewhat based on the Linux Kernel boot requirements: * align by 2M and maximal FDT size 2M diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index a63c8bec45..b3c9a21419 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -282,7 +282,7 @@ int board_late_init(void) { u32 status = 0; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); /* We need to be fairly conservative here as we support boards with just 1G of TOTAL RAM */ status |= env_set_hex("kernel_addr_r", addr_alloc(SZ_128M)); diff --git a/arch/arm/mach-stm32mp/stm32mp1/cpu.c b/arch/arm/mach-stm32mp/stm32mp1/cpu.c index a223297034..8e3f001f74 100644 --- a/arch/arm/mach-stm32mp/stm32mp1/cpu.c +++ b/arch/arm/mach-stm32mp/stm32mp1/cpu.c @@ -143,7 +143,7 @@ int mach_cpu_init(void) void enable_caches(void) { /* parse device tree when data cache is still activated */ - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); /* I-cache is already enabled in start.S: icache_enable() not needed */ diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index e08d3e2cf3..fc408e9820 100644 --- a/cmd/bdinfo.c +++ b/cmd/bdinfo.c @@ -163,7 +163,7 @@ static int bdinfo_print_all(struct bd_info *bd) bdinfo_print_num_l("multi_dtb_fit", (ulong)gd->multi_dtb_fit); #endif if (IS_ENABLED(CONFIG_LMB) && gd->fdt_blob) { - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); lmb_dump_all_force(); if (IS_ENABLED(CONFIG_OF_REAL)) printf("devicetree = %s\n", fdtdec_get_srcname()); diff --git a/cmd/load.c b/cmd/load.c index f019111991..9918523806 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -154,7 +154,7 @@ static ulong load_serial(long offset) int line_count = 0; long ret; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); while (read_record(record, SREC_MAXRECLEN + 1) >= 0) { type = srec_decode(record, &binlen, &addr, binbuf); diff --git a/fs/fs.c b/fs/fs.c index 3c54eaa366..f72d3962d1 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -549,7 +549,7 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset, if (len && len < read_len) read_len = len; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); lmb_dump_all(); if (lmb_alloc_addr(addr, read_len) == addr) diff --git a/include/lmb.h b/include/lmb.h index f3e3f716e5..03bce2a50c 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -84,7 +84,17 @@ struct lmb { struct lmb_region reserved; }; -void lmb_init_and_reserve(struct bd_info *bd, void *fdt_blob); +/** + * lmb_add_memory() - Add memory range for LMB allocations + * @bd: pointer to board info structure + * + * Add the entire available memory range to the pool of memory that + * can be used by the LMB module for allocations. + * + * Return: None + * + */ +void lmb_add_memory(struct bd_info *bd); long lmb_add(phys_addr_t base, phys_size_t size); long lmb_reserve(phys_addr_t base, phys_size_t size); /** diff --git a/lib/lmb.c b/lib/lmb.c index 5c056800c3..de5a2cf23b 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -240,8 +240,17 @@ void lmb_reserve_common(void *fdt_blob) efi_lmb_reserve(); } -/* Initialize the struct, add memory and call arch/board reserve functions */ -void lmb_init_and_reserve(struct bd_info *bd, void *fdt_blob) +/** + * lmb_add_memory() - Add memory range for LMB allocations + * @bd: pointer to board info structure + * + * Add the entire available memory range to the pool of memory that + * can be used by the LMB module for allocations. + * + * Return: None + * + */ +void lmb_add_memory(struct bd_info *bd) { int i; @@ -249,8 +258,6 @@ void lmb_init_and_reserve(struct bd_info *bd, void *fdt_blob) if (bd->bi_dram[i].size) lmb_add(bd->bi_dram[i].start, bd->bi_dram[i].size); } - - lmb_reserve_common(fdt_blob); } /* This routine called with relocation disabled. */ diff --git a/net/tftp.c b/net/tftp.c index 02a5ca0f9f..54ac5e1262 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -714,7 +714,7 @@ static int tftp_init_load_addr(void) #ifdef CONFIG_LMB phys_size_t max_size; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); max_size = lmb_get_free_size(image_load_addr); if (!max_size) diff --git a/net/wget.c b/net/wget.c index dbdc519946..87f2f33c8a 100644 --- a/net/wget.c +++ b/net/wget.c @@ -76,7 +76,7 @@ static int wget_init_load_size(void) { phys_size_t max_size; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); max_size = lmb_get_free_size(image_load_addr); if (!max_size) diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c index 31d5e28105..8ba785fc31 100644 --- a/test/cmd/bdinfo.c +++ b/test/cmd/bdinfo.c @@ -114,14 +114,6 @@ static int lmb_test_dump_region(struct unit_test_state *uts, end = base + size - 1; flags = rgn->region[i].flags; - /* - * this entry includes the stack (get_sp()) on many platforms - * so will different each time lmb_init_and_reserve() is called. - * We could instead have the bdinfo command put its lmb region - * in a known location, so we can check it directly, rather than - * calling lmb_init_and_reserve() to create a new (and hopefully - * identical one). But for now this seems good enough. - */ if (!IS_ENABLED(CONFIG_SANDBOX) && i == 3) { ut_assert_nextlinen(" %s[%d]\t[", name, i); continue; @@ -201,7 +193,7 @@ static int bdinfo_test_all(struct unit_test_state *uts) if (IS_ENABLED(CONFIG_LMB) && gd->fdt_blob) { struct lmb lmb; - lmb_init_and_reserve(gd->bd, (void *)gd->fdt_blob); + lmb_add_memory(gd->bd); ut_assertok(lmb_test_dump_all(uts, &lmb)); if (IS_ENABLED(CONFIG_OF_REAL)) ut_assert_nextline("devicetree = %s", fdtdec_get_srcname());
With the changes to make the Logical Memory Block(LMB) allocations persistent and with the common memory regions being reserved during board init, the lmb_init_and_reserve() API can be removed and replaced with a lmb_add_memory() API, which adds all the available memory to the LMB pool. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- arch/arm/mach-apple/board.c | 2 +- arch/arm/mach-snapdragon/board.c | 2 +- arch/arm/mach-stm32mp/stm32mp1/cpu.c | 2 +- cmd/bdinfo.c | 2 +- cmd/load.c | 2 +- fs/fs.c | 2 +- include/lmb.h | 12 +++++++++++- lib/lmb.c | 15 +++++++++++---- net/tftp.c | 2 +- net/wget.c | 2 +- test/cmd/bdinfo.c | 10 +--------- 11 files changed, 31 insertions(+), 22 deletions(-)