[RFC PATCH 0/7] Cleanup the LMB subsystem
The LMB subsystem was used opportunistically for a number of years. A while back Sughosh merged it with the EFI subsystem in order to have a common allocator and avoid subsystems overwriting memory they shouldn't.
This is an initial cleanup of all the crud we gathered over the years. There's no functional change expected from the patches as they just cleanup some abstraction functions and rename a few variables to make more sense.
This has to be rebased on top of Sam's [0] changes but that should be trivial.
I plan to make even bigger changes -- e.g I don't see the point of having *_alloc() and *_reserve() versions of the functions since they mostly do the same thing and just cause confusion. lmb_alloc_addr_flags() returning the base address on success makes little sense since we already *request* the address on the function arguments, etc. But since this patchset grew enough already, I'd like to get it in before more refactoring happens.
It's worth noting that although some patches slightly increase the code size due to an extra flags argument being carried around, the final result is eventually smaller.
add/remove: 2/5 grow/shrink: 17/3 up/down: 479/-524 (-45) Function old new delta lmb_alloc_base_flags - 299 +299 lmb_alloc_addr_flags - 90 +90 test_alloc_addr 2933 2963 +30 lib_test_lmb_overlapping_reserve 1018 1030 +12 test_multi_alloc.constprop 3034 3042 +8 test_get_unreserved_size 1032 1038 +6 boot_relocate_fdt 599 605 +6 lmb_alloc 4 9 +5 wget_handler 1530 1533 +3 tftp_handler 1190 1192 +2 test_noreserved 1207 1209 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_spi_flash 3150 3152 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 _fs_read.lto_priv 331 333 +2 lmb_reserve 4 - -4 lmb_alloc_addr 4 - -4 lmb_overlaps_region 62 52 -10 lmb_add_region_flags 600 568 -32 lmb_alloc_base 48 - -48 efi_allocate_pages.part 303 249 -54 _lmb_alloc_addr.lto_priv 92 - -92 _lmb_alloc_base.lto_priv 280 - -280 Total: Before=2492742, After=2492697, chg -0.00%
[0] https://lore.kernel.org/u-boot/20241208002121.31887-1-semen.protsenko@linaro...
Ilias Apalodimas (7): lmb: Replace lmb_reserve() with lmb_reserve_flags() lmb: Simplify lmb_addrs_overlap() lmb: Rename free_mem to available_mem lmb: Remove lmb_add_region() lmb: Replace lmb_alloc_addr() with lmb_alloc_addr_flags() lmb: Replace lmb_alloc_base() with lmb_alloc_base_flags() lmb: Rename _lmb_alloc_addr_flags()
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 +- boot/image-board.c | 24 +++++--- boot/image-fdt.c | 11 ++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- fs/fs.c | 2 +- include/lmb.h | 11 ++-- lib/lmb.c | 111 +++++++++++----------------------- test/cmd/bdinfo.c | 2 +- test/lib/lmb.c | 68 +++++++++++---------- 13 files changed, 105 insertions(+), 137 deletions(-)
-- 2.45.2
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/mp.c b/arch/powerpc/cpu/mpc85xx/mp.c index bed465cb2cba..22252c1448a8 100644 --- a/arch/powerpc/cpu/mpc85xx/mp.c +++ b/arch/powerpc/cpu/mpc85xx/mp.c @@ -412,7 +412,7 @@ void cpu_mp_lmb_reserve(void) { u32 bootpg = determine_mp_bootpg(NULL);
- lmb_reserve(bootpg, 4096); + lmb_reserve_flags(bootpg, 4096, LMB_NONE); }
void setup_mp(void) diff --git a/arch/powerpc/lib/misc.c b/arch/powerpc/lib/misc.c index 4cd23b3406d1..c08d4b35118b 100644 --- a/arch/powerpc/lib/misc.c +++ b/arch/powerpc/lib/misc.c @@ -40,7 +40,7 @@ int arch_misc_init(void)
printf("WARNING: adjusting available memory from 0x%lx to 0x%llx\n", size, (unsigned long long)bootm_size); - lmb_reserve(base, bootm_size - size); + lmb_reserve_flags(base, bootm_size - size, LMB_NONE); }
#ifdef CONFIG_MP diff --git a/boot/bootm.c b/boot/bootm.c index 16a43d519a8a..c8662442e403 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -696,7 +696,8 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress) }
if (CONFIG_IS_ENABLED(LMB)) - lmb_reserve(images->os.load, (load_end - images->os.load)); + lmb_reserve_flags(images->os.load, (load_end - images->os.load), + LMB_NONE);
return 0; } diff --git a/boot/image-board.c b/boot/image-board.c index b726bd6b3035..c4d914fd6074 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -562,7 +562,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, debug(" in-place initrd\n"); *initrd_start = rd_data; *initrd_end = rd_data + rd_len; - lmb_reserve(rd_data, rd_len); + lmb_reserve_flags(rd_data, rd_len, LMB_NONE); } else { if (initrd_high) *initrd_start = (ulong)lmb_alloc_base(rd_len, diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 3d5b6f9e2dc7..fd68b8594325 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -184,7 +184,8 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size) if (desired_addr == ~0UL) { /* All ones means use fdt in place */ of_start = fdt_blob; - lmb_reserve(map_to_sysmem(of_start), of_len); + lmb_reserve_flags(map_to_sysmem(of_start), of_len, + LMB_NONE); disable_relocation = 1; } else if (desired_addr) { addr = lmb_alloc_base(of_len, 0x1000, desired_addr); @@ -675,7 +676,7 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb)
/* Create a new LMB reservation */ if (CONFIG_IS_ENABLED(LMB) && lmb) - lmb_reserve(map_to_sysmem(blob), of_size); + lmb_reserve_flags(map_to_sysmem(blob), of_size, LMB_NONE);
#if defined(CONFIG_ARCH_KEYSTONE) if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) diff --git a/cmd/booti.c b/cmd/booti.c index 43e79e87201b..58d355726cc5 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -87,7 +87,7 @@ static int booti_start(struct bootm_info *bmi) images->os.start = relocated_addr; images->os.end = relocated_addr + image_size;
- lmb_reserve(images->ep, le32_to_cpu(image_size)); + lmb_reserve_flags(images->ep, le32_to_cpu(image_size), LMB_NONE);
/* * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not diff --git a/cmd/bootz.c b/cmd/bootz.c index 787203f5bd70..ab2bf12eb8b0 100644 --- a/cmd/bootz.c +++ b/cmd/bootz.c @@ -56,7 +56,7 @@ static int bootz_start(struct cmd_tbl *cmdtp, int flag, int argc, if (ret != 0) return 1;
- lmb_reserve(images->ep, zi_end - zi_start); + lmb_reserve_flags(images->ep, zi_end - zi_start, LMB_NONE);
/* * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not diff --git a/cmd/load.c b/cmd/load.c index 20d802502ae6..dfbb6d2db0c9 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -179,7 +179,7 @@ static ulong load_serial(long offset) { void *dst;
- ret = lmb_reserve(store_addr, binlen); + ret = lmb_reserve_flags(store_addr, binlen, LMB_NONE); if (ret) { printf("\nCannot overwrite reserved area (%08lx..%08lx)\n", store_addr, store_addr + binlen); diff --git a/include/lmb.h b/include/lmb.h index f221f0cce8f7..62882464f866 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -82,7 +82,6 @@ int lmb_init(void); void lmb_add_memory(void);
long lmb_add(phys_addr_t base, phys_size_t size); -long lmb_reserve(phys_addr_t base, phys_size_t size); /** * lmb_reserve_flags - Reserve one region with a specific flags bitfield. * diff --git a/lib/lmb.c b/lib/lmb.c index b03237bc06cb..a7ecbb58831f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -698,11 +698,6 @@ long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags) return lmb_map_update_notify(base, size, MAP_OP_RESERVE, flags); }
-long lmb_reserve(phys_addr_t base, phys_size_t size) -{ - return lmb_reserve_flags(base, size, LMB_NONE); -} - static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, enum lmb_flags flags) { diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 0bd29e2a4fe7..8af5dcad2ecc 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -117,7 +117,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, }
/* reserve 64KiB somewhere */ - ret = lmb_reserve(alloc_64k_addr, 0x10000); + ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, 0, 0, 1, alloc_64k_addr, 0x10000, 0, 0, 0, 0); @@ -264,7 +264,7 @@ static int test_bigblock(struct unit_test_state *uts, const phys_addr_t ram) ut_asserteq(ret, 0);
/* reserve 64KiB in the middle of RAM */ - ret = lmb_reserve(alloc_64k_addr, 0x10000); + ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, alloc_64k_addr, 0x10000, 0, 0, 0, 0); @@ -466,35 +466,35 @@ static int lib_test_lmb_overlapping_reserve(struct unit_test_state *uts) ret = lmb_add(ram, ram_size); ut_asserteq(ret, 0);
- ret = lmb_reserve(0x40010000, 0x10000); + ret = lmb_reserve_flags(0x40010000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x10000, 0, 0, 0, 0);
/* allocate overlapping region should return the coalesced count */ - ret = lmb_reserve(0x40011000, 0x10000); + ret = lmb_reserve_flags(0x40011000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x11000, 0, 0, 0, 0); /* allocate 3nd region */ - ret = lmb_reserve(0x40030000, 0x10000); + ret = lmb_reserve_flags(0x40030000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40010000, 0x11000, 0x40030000, 0x10000, 0, 0); /* allocate 2nd region , This should coalesced all region into one */ - ret = lmb_reserve(0x40020000, 0x10000); + ret = lmb_reserve_flags(0x40020000, 0x10000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x30000, 0, 0, 0, 0);
/* allocate 2nd region, which should be added as first region */ - ret = lmb_reserve(0x40000000, 0x8000); + ret = lmb_reserve_flags(0x40000000, 0x8000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x8000, 0x40010000, 0x30000, 0, 0);
/* allocate 3rd region, coalesce with first and overlap with second */ - ret = lmb_reserve(0x40008000, 0x10000); + ret = lmb_reserve_flags(0x40008000, 0x10000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40000000, 0x40000, 0, 0, 0, 0); @@ -550,11 +550,11 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) ut_asserteq(ret, 0);
/* reserve 3 blocks */ - ret = lmb_reserve(alloc_addr_a, 0x10000); + ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_b, 0x10000); + ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_c, 0x10000); + ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); @@ -680,11 +680,11 @@ static int test_get_unreserved_size(struct unit_test_state *uts, ut_asserteq(ret, 0);
/* reserve 3 blocks */ - ret = lmb_reserve(alloc_addr_a, 0x10000); + ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_b, 0x10000); + ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_c, 0x10000); + ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); @@ -789,7 +789,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts) ut_asserteq(lmb_is_nomap(&used[1]), 0);
/* test that old API use LMB_NONE */ - ret = lmb_reserve(0x40040000, 0x10000); + ret = lmb_reserve_flags(0x40040000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x30000, 0x40030000, 0x20000, 0, 0);
On Sun, Dec 08, 2024 at 12:52:04PM +0200, Ilias Apalodimas wrote:
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Reviewed-by: Tom Rini trini@konsulko.com
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Fwiw, I had attempted this earlier on similar lines -- have a single API, which would take the flag as a parameter [1], but was asked not to take this approach as part of the review [2].
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/mp.c b/arch/powerpc/cpu/mpc85xx/mp.c index bed465cb2cba..22252c1448a8 100644 --- a/arch/powerpc/cpu/mpc85xx/mp.c +++ b/arch/powerpc/cpu/mpc85xx/mp.c @@ -412,7 +412,7 @@ void cpu_mp_lmb_reserve(void) { u32 bootpg = determine_mp_bootpg(NULL);
lmb_reserve(bootpg, 4096);
lmb_reserve_flags(bootpg, 4096, LMB_NONE);}
void setup_mp(void) diff --git a/arch/powerpc/lib/misc.c b/arch/powerpc/lib/misc.c index 4cd23b3406d1..c08d4b35118b 100644 --- a/arch/powerpc/lib/misc.c +++ b/arch/powerpc/lib/misc.c @@ -40,7 +40,7 @@ int arch_misc_init(void)
printf("WARNING: adjusting available memory from 0x%lx to 0x%llx\n", size, (unsigned long long)bootm_size);
lmb_reserve(base, bootm_size - size);
lmb_reserve_flags(base, bootm_size - size, LMB_NONE); }#ifdef CONFIG_MP diff --git a/boot/bootm.c b/boot/bootm.c index 16a43d519a8a..c8662442e403 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -696,7 +696,8 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress) }
if (CONFIG_IS_ENABLED(LMB))
lmb_reserve(images->os.load, (load_end - images->os.load));
lmb_reserve_flags(images->os.load, (load_end - images->os.load),LMB_NONE); return 0;} diff --git a/boot/image-board.c b/boot/image-board.c index b726bd6b3035..c4d914fd6074 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -562,7 +562,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, debug(" in-place initrd\n"); *initrd_start = rd_data; *initrd_end = rd_data + rd_len;
lmb_reserve(rd_data, rd_len);
lmb_reserve_flags(rd_data, rd_len, LMB_NONE); } else { if (initrd_high) *initrd_start = (ulong)lmb_alloc_base(rd_len,diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 3d5b6f9e2dc7..fd68b8594325 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -184,7 +184,8 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size) if (desired_addr == ~0UL) { /* All ones means use fdt in place */ of_start = fdt_blob;
lmb_reserve(map_to_sysmem(of_start), of_len);
lmb_reserve_flags(map_to_sysmem(of_start), of_len,LMB_NONE); disable_relocation = 1; } else if (desired_addr) { addr = lmb_alloc_base(of_len, 0x1000, desired_addr);@@ -675,7 +676,7 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb)
/* Create a new LMB reservation */ if (CONFIG_IS_ENABLED(LMB) && lmb)
lmb_reserve(map_to_sysmem(blob), of_size);
lmb_reserve_flags(map_to_sysmem(blob), of_size, LMB_NONE);#if defined(CONFIG_ARCH_KEYSTONE) if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) diff --git a/cmd/booti.c b/cmd/booti.c index 43e79e87201b..58d355726cc5 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -87,7 +87,7 @@ static int booti_start(struct bootm_info *bmi) images->os.start = relocated_addr; images->os.end = relocated_addr + image_size;
lmb_reserve(images->ep, le32_to_cpu(image_size));
lmb_reserve_flags(images->ep, le32_to_cpu(image_size), LMB_NONE); /* * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do notdiff --git a/cmd/bootz.c b/cmd/bootz.c index 787203f5bd70..ab2bf12eb8b0 100644 --- a/cmd/bootz.c +++ b/cmd/bootz.c @@ -56,7 +56,7 @@ static int bootz_start(struct cmd_tbl *cmdtp, int flag, int argc, if (ret != 0) return 1;
lmb_reserve(images->ep, zi_end - zi_start);
lmb_reserve_flags(images->ep, zi_end - zi_start, LMB_NONE); /* * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do notdiff --git a/cmd/load.c b/cmd/load.c index 20d802502ae6..dfbb6d2db0c9 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -179,7 +179,7 @@ static ulong load_serial(long offset) { void *dst;
ret = lmb_reserve(store_addr, binlen);
ret = lmb_reserve_flags(store_addr, binlen, LMB_NONE); if (ret) { printf("\nCannot overwrite reserved area (%08lx..%08lx)\n", store_addr, store_addr + binlen);diff --git a/include/lmb.h b/include/lmb.h index f221f0cce8f7..62882464f866 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -82,7 +82,6 @@ int lmb_init(void); void lmb_add_memory(void);
long lmb_add(phys_addr_t base, phys_size_t size); -long lmb_reserve(phys_addr_t base, phys_size_t size); /**
- lmb_reserve_flags - Reserve one region with a specific flags bitfield.
diff --git a/lib/lmb.c b/lib/lmb.c index b03237bc06cb..a7ecbb58831f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -698,11 +698,6 @@ long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags) return lmb_map_update_notify(base, size, MAP_OP_RESERVE, flags); }
-long lmb_reserve(phys_addr_t base, phys_size_t size) -{
return lmb_reserve_flags(base, size, LMB_NONE);-}
static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, enum lmb_flags flags) { diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 0bd29e2a4fe7..8af5dcad2ecc 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -117,7 +117,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, }
/* reserve 64KiB somewhere */
ret = lmb_reserve(alloc_64k_addr, 0x10000);
ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, 0, 0, 1, alloc_64k_addr, 0x10000, 0, 0, 0, 0);@@ -264,7 +264,7 @@ static int test_bigblock(struct unit_test_state *uts, const phys_addr_t ram) ut_asserteq(ret, 0);
/* reserve 64KiB in the middle of RAM */
ret = lmb_reserve(alloc_64k_addr, 0x10000);
ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, alloc_64k_addr, 0x10000, 0, 0, 0, 0);@@ -466,35 +466,35 @@ static int lib_test_lmb_overlapping_reserve(struct unit_test_state *uts) ret = lmb_add(ram, ram_size); ut_asserteq(ret, 0);
ret = lmb_reserve(0x40010000, 0x10000);
ret = lmb_reserve_flags(0x40010000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x10000, 0, 0, 0, 0); /* allocate overlapping region should return the coalesced count */
ret = lmb_reserve(0x40011000, 0x10000);
ret = lmb_reserve_flags(0x40011000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x11000, 0, 0, 0, 0); /* allocate 3nd region */
ret = lmb_reserve(0x40030000, 0x10000);
ret = lmb_reserve_flags(0x40030000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40010000, 0x11000, 0x40030000, 0x10000, 0, 0); /* allocate 2nd region , This should coalesced all region into one */
ret = lmb_reserve(0x40020000, 0x10000);
ret = lmb_reserve_flags(0x40020000, 0x10000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x30000, 0, 0, 0, 0); /* allocate 2nd region, which should be added as first region */
ret = lmb_reserve(0x40000000, 0x8000);
ret = lmb_reserve_flags(0x40000000, 0x8000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x8000, 0x40010000, 0x30000, 0, 0); /* allocate 3rd region, coalesce with first and overlap with second */
ret = lmb_reserve(0x40008000, 0x10000);
ret = lmb_reserve_flags(0x40008000, 0x10000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40000000, 0x40000, 0, 0, 0, 0);@@ -550,11 +550,11 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) ut_asserteq(ret, 0);
/* reserve 3 blocks */
ret = lmb_reserve(alloc_addr_a, 0x10000);
ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); ut_asserteq(ret, 0);
ret = lmb_reserve(alloc_addr_b, 0x10000);
ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); ut_asserteq(ret, 0);
ret = lmb_reserve(alloc_addr_c, 0x10000);
ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);@@ -680,11 +680,11 @@ static int test_get_unreserved_size(struct unit_test_state *uts, ut_asserteq(ret, 0);
/* reserve 3 blocks */
ret = lmb_reserve(alloc_addr_a, 0x10000);
ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); ut_asserteq(ret, 0);
ret = lmb_reserve(alloc_addr_b, 0x10000);
ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); ut_asserteq(ret, 0);
ret = lmb_reserve(alloc_addr_c, 0x10000);
ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);@@ -789,7 +789,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts) ut_asserteq(lmb_is_nomap(&used[1]), 0);
/* test that old API use LMB_NONE */
ret = lmb_reserve(0x40040000, 0x10000);
ret = lmb_reserve_flags(0x40040000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x30000, 0x40030000, 0x20000, 0, 0);-- 2.45.2
Hi,
On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Fwiw, I had attempted this earlier on similar lines -- have a single API, which would take the flag as a parameter [1], but was asked not to take this approach as part of the review [2].
You missed that sandbox has LTO enabled. With that disabled (like many boards):
sandbox: (for 1/1 boards) all -32.0 text -32.0
We just had a patch and discussion to remove a single function call to save 8 bytes[3]
For firefly-rk3399 this adds 36 bytes:
aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0
You could run buildman on all arm64 boards to check it, perhaps.
The other thing is that there is actually only one place apart from tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). Boards don't use that flag. It is the flags themselves which are confusing.
Regards, Simon
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)
[3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f608060...
Hi Simon,
On Tue, 10 Dec 2024 at 18:16, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Fwiw, I had attempted this earlier on similar lines -- have a single API, which would take the flag as a parameter [1], but was asked not to take this approach as part of the review [2].
You missed that sandbox has LTO enabled. With that disabled (like many boards):
sandbox: (for 1/1 boards) all -32.0 text -32.0
We just had a patch and discussion to remove a single function call to save 8 bytes[3]
For firefly-rk3399 this adds 36 bytes:
aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0
Did you include DM tests in that config? Because applying the series says otherwise
for firefly-rk3399_defconfig add/remove: 0/5 grow/shrink: 14/0 up/down: 480/-544 (-64) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 version_string 50 70 +20 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1304 1308 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 380 384 +4 do_spi_flash 2164 2168 +4 do_load 732 736 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=691587, After=691523, chg -0.01%
You could run buildman on all arm64 boards to check it, perhaps.
I did test on non-LTO platforms as well, building for all is kind of pointless no? It's either LTO or no LTO. FWIW you need to apply the entire series, not just a single patch.
rpi_arm64_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 516 536 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1552 1556 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 456 460 +4 do_load 728 732 +4 do_booti 520 524 +4 bootm_run_states 1824 1828 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=542062, After=541970, chg -0.02%
qemu_arm64_lwip_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 load_serial 548 552 +4 lmb_alloc 8 12 +4 image_setup_libfdt 368 372 +4 do_load 728 732 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=1020122, After=1020030, chg -0.01%
The other thing is that there is actually only one place apart from tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). Boards don't use that flag. It is the flags themselves which are confusing.
I still think it's way easier to read without two levels of abstraction. Since the series decreases the size -- unless you include all DM Tests, I'd like to keep the flags as is for now, so I can send the next round which is the actual cleanup. We can re-introduce them if someone finds a 'size problem'. But since it's only when you enable DM I doubt anyone wants to have them working in SPL or production firmware.
/Ilias
Regards, Simon
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)
[3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f608060...
Hi Ilias,
On Tue, 10 Dec 2024 at 11:00, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Tue, 10 Dec 2024 at 18:16, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Fwiw, I had attempted this earlier on similar lines -- have a single API, which would take the flag as a parameter [1], but was asked not to take this approach as part of the review [2].
You missed that sandbox has LTO enabled. With that disabled (like many boards):
sandbox: (for 1/1 boards) all -32.0 text -32.0
We just had a patch and discussion to remove a single function call to save 8 bytes[3]
For firefly-rk3399 this adds 36 bytes:
aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0
Did you include DM tests in that config? Because applying the series says otherwise
for firefly-rk3399_defconfig add/remove: 0/5 grow/shrink: 14/0 up/down: 480/-544 (-64) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 version_string 50 70 +20 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1304 1308 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 380 384 +4 do_spi_flash 2164 2168 +4 do_load 732 736 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=691587, After=691523, chg -0.01%
OK good
You could run buildman on all arm64 boards to check it, perhaps.
I did test on non-LTO platforms as well, building for all is kind of pointless no? It's either LTO or no LTO.
Right
FWIW you need to apply the entire series, not just a single patch.
Yes, understood, I was just replying to that one patch.
rpi_arm64_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 516 536 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1552 1556 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 456 460 +4 do_load 728 732 +4 do_booti 520 524 +4 bootm_run_states 1824 1828 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=542062, After=541970, chg -0.02%
qemu_arm64_lwip_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 load_serial 548 552 +4 lmb_alloc 8 12 +4 image_setup_libfdt 368 372 +4 do_load 728 732 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=1020122, After=1020030, chg -0.01%
The other thing is that there is actually only one place apart from tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). Boards don't use that flag. It is the flags themselves which are confusing.
I still think it's way easier to read without two levels of abstraction. Since the series decreases the size -- unless you include all DM Tests, I'd like to keep the flags as is for now, so I can send the next round which is the actual cleanup. We can re-introduce them if someone finds a 'size problem'. But since it's only when you enable DM I doubt anyone wants to have them working in SPL or production firmware.
OK, well I don't mind, if there is no growth. I wonder if you could do better overall without this patch, but perhaps not.
/Ilias
Regards, Simon
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)
[3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f608060...
Regards, SImon
Thanks Simon,
On Tue, 10 Dec 2024 at 20:58, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Tue, 10 Dec 2024 at 11:00, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Tue, 10 Dec 2024 at 18:16, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Fwiw, I had attempted this earlier on similar lines -- have a single API, which would take the flag as a parameter [1], but was asked not to take this approach as part of the review [2].
You missed that sandbox has LTO enabled. With that disabled (like many boards):
sandbox: (for 1/1 boards) all -32.0 text -32.0
We just had a patch and discussion to remove a single function call to save 8 bytes[3]
For firefly-rk3399 this adds 36 bytes:
aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0
Did you include DM tests in that config? Because applying the series says otherwise
for firefly-rk3399_defconfig add/remove: 0/5 grow/shrink: 14/0 up/down: 480/-544 (-64) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 version_string 50 70 +20 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1304 1308 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 380 384 +4 do_spi_flash 2164 2168 +4 do_load 732 736 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=691587, After=691523, chg -0.01%
OK good
You could run buildman on all arm64 boards to check it, perhaps.
I did test on non-LTO platforms as well, building for all is kind of pointless no? It's either LTO or no LTO.
Right
FWIW you need to apply the entire series, not just a single patch.
Yes, understood, I was just replying to that one patch.
rpi_arm64_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 516 536 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1552 1556 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 456 460 +4 do_load 728 732 +4 do_booti 520 524 +4 bootm_run_states 1824 1828 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=542062, After=541970, chg -0.02%
qemu_arm64_lwip_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 load_serial 548 552 +4 lmb_alloc 8 12 +4 image_setup_libfdt 368 372 +4 do_load 728 732 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=1020122, After=1020030, chg -0.01%
The other thing is that there is actually only one place apart from tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). Boards don't use that flag. It is the flags themselves which are confusing.
I still think it's way easier to read without two levels of abstraction. Since the series decreases the size -- unless you include all DM Tests, I'd like to keep the flags as is for now, so I can send the next round which is the actual cleanup. We can re-introduce them if someone finds a 'size problem'. But since it's only when you enable DM I doubt anyone wants to have them working in SPL or production firmware.
OK, well I don't mind, if there is no growth. I wonder if you could do better overall without this patch, but perhaps not.
Probably yes, but we are not looking into anything more than a few bytes -- probably under 4 as well. TBH if we all agree that removing the non-flag variants is fine, I can name the functions without it.
/Ilias
/Ilias
Regards, Simon
-sughosh
[1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)
[3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f608060...
Regards, SImon
There's no point subtracting -1 from the calculated addresses and then check for a <= b. Just remove the -1 and check for a < b.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/lmb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index a7ecbb58831f..c7bf5120696f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, phys_addr_t base2, phys_size_t size2) { - const phys_addr_t base1_end = base1 + size1 - 1; - const phys_addr_t base2_end = base2 + size2 - 1; + const phys_addr_t base1_end = base1 + size1; + const phys_addr_t base2_end = base2 + size2;
- return ((base1 <= base2_end) && (base2 <= base1_end)); + return ((base1 < base2_end) && (base2 < base1_end)); }
static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
Am 8. Dezember 2024 11:52:05 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
There's no point subtracting -1 from the calculated addresses and then check for a <= b. Just remove the -1 and check for a < b.
I once thought that, too. But it makes a difference for end= U(L)LONG_MAX.
Best regards
Heinrich
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/lmb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index a7ecbb58831f..c7bf5120696f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, phys_addr_t base2, phys_size_t size2) {
- const phys_addr_t base1_end = base1 + size1 - 1;
- const phys_addr_t base2_end = base2 + size2 - 1;
- const phys_addr_t base1_end = base1 + size1;
- const phys_addr_t base2_end = base2 + size2;
- return ((base1 <= base2_end) && (base2 <= base1_end));
- return ((base1 < base2_end) && (base2 < base1_end));
}
static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
Hi Heinrich
On Sun, 8 Dec 2024 at 13:31, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. Dezember 2024 11:52:05 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
There's no point subtracting -1 from the calculated addresses and then check for a <= b. Just remove the -1 and check for a < b.
I once thought that, too. But it makes a difference for end= U(L)LONG_MAX.
If those addresses overflow then we have to add more checks -- e.g if (base2_end < base2) { Do something wrong }
In which case, I prefer modifying the function and add those checks
Thoughts? /Ilias
Best regards
Heinrich
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/lmb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index a7ecbb58831f..c7bf5120696f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, phys_addr_t base2, phys_size_t size2) {
const phys_addr_t base1_end = base1 + size1 - 1;const phys_addr_t base2_end = base2 + size2 - 1;
const phys_addr_t base1_end = base1 + size1;const phys_addr_t base2_end = base2 + size2;
return ((base1 <= base2_end) && (base2 <= base1_end));
return ((base1 < base2_end) && (base2 < base1_end));}
static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
On Sun, 8 Dec 2024 at 14:23, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Heinrich
On Sun, 8 Dec 2024 at 13:31, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. Dezember 2024 11:52:05 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
There's no point subtracting -1 from the calculated addresses and then check for a <= b. Just remove the -1 and check for a < b.
I once thought that, too. But it makes a difference for end= U(L)LONG_MAX.
If those addresses overflow then we have to add more checks -- e.g if (base2_end < base2) { Do something wrong }
In which case, I prefer modifying the function and add those checks
Thoughts? /Ilias
I'm replying to myself here, but the check above isn't needed it's already implied by the return checks. I'll drop this in v2
Cheers /Ilias
Best regards
Heinrich
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/lmb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index a7ecbb58831f..c7bf5120696f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, phys_addr_t base2, phys_size_t size2) {
const phys_addr_t base1_end = base1 + size1 - 1;const phys_addr_t base2_end = base2 + size2 - 1;
const phys_addr_t base1_end = base1 + size1;const phys_addr_t base2_end = base2 + size2;
return ((base1 <= base2_end) && (base2 <= base1_end));
return ((base1 < base2_end) && (base2 < base1_end));}
static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
free_mem is a misnomer. We never update that with the actual free memory. Instead this field describes all available memory and it's checked against used_mem to decide if a memory area is free or not. So rename it to something that better describes its usage.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- include/lmb.h | 4 ++-- lib/lmb.c | 34 +++++++++++++++++----------------- test/cmd/bdinfo.c | 2 +- test/lib/lmb.c | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index 62882464f866..54af5b0e4b44 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -47,12 +47,12 @@ struct lmb_region { /** * struct lmb - The LMB structure * - * @free_mem: List of free memory regions + * @free_mem: List of all available memory * @used_mem: List of used/reserved memory regions * @test: Is structure being used for LMB tests */ struct lmb { - struct alist free_mem; + struct alist available_mem; struct alist used_mem; bool test; }; diff --git a/lib/lmb.c b/lib/lmb.c index c7bf5120696f..ffdd23d87b9b 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -350,7 +350,7 @@ int io_lmb_setup(struct lmb *io_lmb) { int ret;
- ret = alist_init(&io_lmb->free_mem, sizeof(struct lmb_region), + ret = alist_init(&io_lmb->available_mem, sizeof(struct lmb_region), (uint)LMB_ALIST_INITIAL_SIZE); if (!ret) { log_debug("Unable to initialise the list for LMB free IOVA\n"); @@ -371,13 +371,13 @@ int io_lmb_setup(struct lmb *io_lmb)
void io_lmb_teardown(struct lmb *io_lmb) { - alist_uninit(&io_lmb->free_mem); + alist_uninit(&io_lmb->available_mem); alist_uninit(&io_lmb->used_mem); }
long io_lmb_add(struct lmb *io_lmb, phys_addr_t base, phys_size_t size) { - return lmb_add_region_flags(&io_lmb->free_mem, base, size, LMB_NONE); + return lmb_add_region_flags(&io_lmb->available_mem, base, size, LMB_NONE); }
/* derived and simplified from _lmb_alloc_base() */ @@ -387,9 +387,9 @@ phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong align) phys_addr_t base = 0; phys_addr_t res_base; struct lmb_region *lmb_used = io_lmb->used_mem.data; - struct lmb_region *lmb_memory = io_lmb->free_mem.data; + struct lmb_region *lmb_memory = io_lmb->available_mem.data;
- for (i = io_lmb->free_mem.count - 1; i >= 0; i--) { + for (i = io_lmb->available_mem.count - 1; i >= 0; i--) { phys_addr_t lmbbase = lmb_memory[i].base; phys_size_t lmbsize = lmb_memory[i].size;
@@ -514,7 +514,7 @@ static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name) void lmb_dump_all_force(void) { printf("lmb_dump_all:\n"); - lmb_dump_region(&lmb.free_mem, "memory"); + lmb_dump_region(&lmb.available_mem, "memory"); lmb_dump_region(&lmb.used_mem, "reserved"); }
@@ -650,7 +650,7 @@ static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, long lmb_add(phys_addr_t base, phys_size_t size) { long ret; - struct alist *lmb_rgn_lst = &lmb.free_mem; + struct alist *lmb_rgn_lst = &lmb.available_mem;
ret = lmb_add_region(lmb_rgn_lst, base, size); if (ret) @@ -706,9 +706,9 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t base = 0; phys_addr_t res_base; struct lmb_region *lmb_used = lmb.used_mem.data; - struct lmb_region *lmb_memory = lmb.free_mem.data; + struct lmb_region *lmb_memory = lmb.available_mem.data;
- for (i = lmb.free_mem.count - 1; i >= 0; i--) { + for (i = lmb.available_mem.count - 1; i >= 0; i--) { phys_addr_t lmbbase = lmb_memory[i].base; phys_size_t lmbsize = lmb_memory[i].size;
@@ -800,10 +800,10 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, enum lmb_flags flags) { long rgn; - struct lmb_region *lmb_memory = lmb.free_mem.data; + struct lmb_region *lmb_memory = lmb.available_mem.data;
/* Check if the requested address is in one of the memory regions */ - rgn = lmb_overlaps_region(&lmb.free_mem, base, size); + rgn = lmb_overlaps_region(&lmb.available_mem, base, size); if (rgn >= 0) { /* * Check if the requested end address is in the same memory @@ -854,10 +854,10 @@ phys_size_t lmb_get_free_size(phys_addr_t addr) int i; long rgn; struct lmb_region *lmb_used = lmb.used_mem.data; - struct lmb_region *lmb_memory = lmb.free_mem.data; + struct lmb_region *lmb_memory = lmb.available_mem.data;
/* check if the requested address is in the memory regions */ - rgn = lmb_overlaps_region(&lmb.free_mem, addr, 1); + rgn = lmb_overlaps_region(&lmb.available_mem, addr, 1); if (rgn >= 0) { for (i = 0; i < lmb.used_mem.count; i++) { if (addr < lmb_used[i].base) { @@ -871,8 +871,8 @@ phys_size_t lmb_get_free_size(phys_addr_t addr) } } /* if we come here: no reserved ranges above requested addr */ - return lmb_memory[lmb.free_mem.count - 1].base + - lmb_memory[lmb.free_mem.count - 1].size - addr; + return lmb_memory[lmb.available_mem.count - 1].base + + lmb_memory[lmb.available_mem.count - 1].size - addr; } return 0; } @@ -895,7 +895,7 @@ static int lmb_setup(bool test) { bool ret;
- ret = alist_init(&lmb.free_mem, sizeof(struct lmb_region), + ret = alist_init(&lmb.available_mem, sizeof(struct lmb_region), (uint)LMB_ALIST_INITIAL_SIZE); if (!ret) { log_debug("Unable to initialise the list for LMB free memory\n"); @@ -967,7 +967,7 @@ int lmb_push(struct lmb *store)
void lmb_pop(struct lmb *store) { - alist_uninit(&lmb.free_mem); + alist_uninit(&lmb.available_mem); alist_uninit(&lmb.used_mem); lmb = *store; } diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c index bb419ab2394e..e5b422ab2ba8 100644 --- a/test/cmd/bdinfo.c +++ b/test/cmd/bdinfo.c @@ -131,7 +131,7 @@ static int lmb_test_dump_all(struct unit_test_state *uts) struct lmb *lmb = lmb_get();
ut_assert_nextline("lmb_dump_all:"); - ut_assertok(lmb_test_dump_region(uts, &lmb->free_mem, "memory")); + ut_assertok(lmb_test_dump_region(uts, &lmb->available_mem, "memory")); ut_assertok(lmb_test_dump_region(uts, &lmb->used_mem, "reserved"));
return 0; diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 8af5dcad2ecc..0f7052224c10 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -65,7 +65,7 @@ static int setup_lmb_test(struct unit_test_state *uts, struct lmb *store,
ut_assertok(lmb_push(store)); lmb = lmb_get(); - *mem_lstp = &lmb->free_mem; + *mem_lstp = &lmb->available_mem; *used_lstp = &lmb->used_mem;
return 0; -- 2.45.2
Am 8. Dezember 2024 11:52:06 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
free_mem is a misnomer. We never update that with the actual free memory. Instead this field describes all available memory and it's checked against used_mem to decide if a memory area is free or not. So rename it to something that better describes its usage.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/lmb.h | 4 ++-- lib/lmb.c | 34 +++++++++++++++++----------------- test/cmd/bdinfo.c | 2 +- test/lib/lmb.c | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index 62882464f866..54af5b0e4b44 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -47,12 +47,12 @@ struct lmb_region { /**
- struct lmb - The LMB structure
- @free_mem: List of free memory regions
- @free_mem: List of all available memory
%s/free/available/
- @used_mem: List of used/reserved memory regions
- @test: Is structure being used for LMB tests
*/ struct lmb {
- struct alist free_mem;
- struct alist available_mem; struct alist used_mem; bool test;
}; diff --git a/lib/lmb.c b/lib/lmb.c index c7bf5120696f..ffdd23d87b9b 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -350,7 +350,7 @@ int io_lmb_setup(struct lmb *io_lmb) { int ret;
- ret = alist_init(&io_lmb->free_mem, sizeof(struct lmb_region),
- ret = alist_init(&io_lmb->available_mem, sizeof(struct lmb_region), (uint)LMB_ALIST_INITIAL_SIZE); if (!ret) { log_debug("Unable to initialise the list for LMB free IOVA\n");
@@ -371,13 +371,13 @@ int io_lmb_setup(struct lmb *io_lmb)
void io_lmb_teardown(struct lmb *io_lmb) {
- alist_uninit(&io_lmb->free_mem);
- alist_uninit(&io_lmb->available_mem); alist_uninit(&io_lmb->used_mem);
}
long io_lmb_add(struct lmb *io_lmb, phys_addr_t base, phys_size_t size) {
- return lmb_add_region_flags(&io_lmb->free_mem, base, size, LMB_NONE);
- return lmb_add_region_flags(&io_lmb->available_mem, base, size, LMB_NONE);
}
/* derived and simplified from _lmb_alloc_base() */ @@ -387,9 +387,9 @@ phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong align) phys_addr_t base = 0; phys_addr_t res_base; struct lmb_region *lmb_used = io_lmb->used_mem.data;
- struct lmb_region *lmb_memory = io_lmb->free_mem.data;
- struct lmb_region *lmb_memory = io_lmb->available_mem.data;
- for (i = io_lmb->free_mem.count - 1; i >= 0; i--) {
- for (i = io_lmb->available_mem.count - 1; i >= 0; i--) { phys_addr_t lmbbase = lmb_memory[i].base; phys_size_t lmbsize = lmb_memory[i].size;
@@ -514,7 +514,7 @@ static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name) void lmb_dump_all_force(void) { printf("lmb_dump_all:\n");
- lmb_dump_region(&lmb.free_mem, "memory");
- lmb_dump_region(&lmb.available_mem, "memory"); lmb_dump_region(&lmb.used_mem, "reserved");
}
@@ -650,7 +650,7 @@ static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, long lmb_add(phys_addr_t base, phys_size_t size) { long ret;
- struct alist *lmb_rgn_lst = &lmb.free_mem;
struct alist *lmb_rgn_lst = &lmb.available_mem;
ret = lmb_add_region(lmb_rgn_lst, base, size); if (ret)
@@ -706,9 +706,9 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t base = 0; phys_addr_t res_base; struct lmb_region *lmb_used = lmb.used_mem.data;
- struct lmb_region *lmb_memory = lmb.free_mem.data;
- struct lmb_region *lmb_memory = lmb.available_mem.data;
- for (i = lmb.free_mem.count - 1; i >= 0; i--) {
- for (i = lmb.available_mem.count - 1; i >= 0; i--) { phys_addr_t lmbbase = lmb_memory[i].base; phys_size_t lmbsize = lmb_memory[i].size;
@@ -800,10 +800,10 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, enum lmb_flags flags) { long rgn;
- struct lmb_region *lmb_memory = lmb.free_mem.data;
struct lmb_region *lmb_memory = lmb.available_mem.data;
/* Check if the requested address is in one of the memory regions */
- rgn = lmb_overlaps_region(&lmb.free_mem, base, size);
- rgn = lmb_overlaps_region(&lmb.available_mem, base, size); if (rgn >= 0) { /*
- Check if the requested end address is in the same memory
@@ -854,10 +854,10 @@ phys_size_t lmb_get_free_size(phys_addr_t addr) int i; long rgn; struct lmb_region *lmb_used = lmb.used_mem.data;
- struct lmb_region *lmb_memory = lmb.free_mem.data;
struct lmb_region *lmb_memory = lmb.available_mem.data;
/* check if the requested address is in the memory regions */
- rgn = lmb_overlaps_region(&lmb.free_mem, addr, 1);
- rgn = lmb_overlaps_region(&lmb.available_mem, addr, 1); if (rgn >= 0) { for (i = 0; i < lmb.used_mem.count; i++) { if (addr < lmb_used[i].base) {
@@ -871,8 +871,8 @@ phys_size_t lmb_get_free_size(phys_addr_t addr) } } /* if we come here: no reserved ranges above requested addr */
return lmb_memory[lmb.free_mem.count - 1].base +lmb_memory[lmb.free_mem.count - 1].size - addr;
return lmb_memory[lmb.available_mem.count - 1].base +lmb_memory[lmb.available_mem.count - 1].size - addr;} @@ -895,7 +895,7 @@ static int lmb_setup(bool test) { bool ret;
- ret = alist_init(&lmb.free_mem, sizeof(struct lmb_region),
- ret = alist_init(&lmb.available_mem, sizeof(struct lmb_region), (uint)LMB_ALIST_INITIAL_SIZE); if (!ret) { log_debug("Unable to initialise the list for LMB free memory\n");
@@ -967,7 +967,7 @@ int lmb_push(struct lmb *store)
void lmb_pop(struct lmb *store) {
- alist_uninit(&lmb.free_mem);
- alist_uninit(&lmb.available_mem); alist_uninit(&lmb.used_mem); lmb = *store;
} diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c index bb419ab2394e..e5b422ab2ba8 100644 --- a/test/cmd/bdinfo.c +++ b/test/cmd/bdinfo.c @@ -131,7 +131,7 @@ static int lmb_test_dump_all(struct unit_test_state *uts) struct lmb *lmb = lmb_get();
ut_assert_nextline("lmb_dump_all:");
- ut_assertok(lmb_test_dump_region(uts, &lmb->free_mem, "memory"));
ut_assertok(lmb_test_dump_region(uts, &lmb->available_mem, "memory")); ut_assertok(lmb_test_dump_region(uts, &lmb->used_mem, "reserved"));
return 0;
diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 8af5dcad2ecc..0f7052224c10 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -65,7 +65,7 @@ static int setup_lmb_test(struct unit_test_state *uts, struct lmb *store,
ut_assertok(lmb_push(store)); lmb = lmb_get();
- *mem_lstp = &lmb->free_mem;
*mem_lstp = &lmb->available_mem; *used_lstp = &lmb->used_mem;
return 0;
-- 2.45.2
On Sun, 8 Dec 2024 at 13:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. Dezember 2024 11:52:06 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
free_mem is a misnomer. We never update that with the actual free memory. Instead this field describes all available memory and it's checked against used_mem to decide if a memory area is free or not. So rename it to something that better describes its usage.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/lmb.h | 4 ++-- lib/lmb.c | 34 +++++++++++++++++----------------- test/cmd/bdinfo.c | 2 +- test/lib/lmb.c | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index 62882464f866..54af5b0e4b44 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -47,12 +47,12 @@ struct lmb_region { /**
- struct lmb - The LMB structure
- @free_mem: List of free memory regions
- @free_mem: List of all available memory
%s/free/available/
Sure, did you find anything else wrong with this patch?
Thanks /Ilias
- @used_mem: List of used/reserved memory regions
- @test: Is structure being used for LMB tests
*/ struct lmb {
struct alist free_mem;
struct alist available_mem; struct alist used_mem; bool test;}; diff --git a/lib/lmb.c b/lib/lmb.c index c7bf5120696f..ffdd23d87b9b 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -350,7 +350,7 @@ int io_lmb_setup(struct lmb *io_lmb) { int ret;
ret = alist_init(&io_lmb->free_mem, sizeof(struct lmb_region),
ret = alist_init(&io_lmb->available_mem, sizeof(struct lmb_region), (uint)LMB_ALIST_INITIAL_SIZE); if (!ret) { log_debug("Unable to initialise the list for LMB free IOVA\n");@@ -371,13 +371,13 @@ int io_lmb_setup(struct lmb *io_lmb)
void io_lmb_teardown(struct lmb *io_lmb) {
alist_uninit(&io_lmb->free_mem);
alist_uninit(&io_lmb->available_mem); alist_uninit(&io_lmb->used_mem);}
long io_lmb_add(struct lmb *io_lmb, phys_addr_t base, phys_size_t size) {
return lmb_add_region_flags(&io_lmb->free_mem, base, size, LMB_NONE);
return lmb_add_region_flags(&io_lmb->available_mem, base, size, LMB_NONE);}
/* derived and simplified from _lmb_alloc_base() */ @@ -387,9 +387,9 @@ phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong align) phys_addr_t base = 0; phys_addr_t res_base; struct lmb_region *lmb_used = io_lmb->used_mem.data;
struct lmb_region *lmb_memory = io_lmb->free_mem.data;
struct lmb_region *lmb_memory = io_lmb->available_mem.data;
for (i = io_lmb->free_mem.count - 1; i >= 0; i--) {
for (i = io_lmb->available_mem.count - 1; i >= 0; i--) { phys_addr_t lmbbase = lmb_memory[i].base; phys_size_t lmbsize = lmb_memory[i].size;@@ -514,7 +514,7 @@ static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name) void lmb_dump_all_force(void) { printf("lmb_dump_all:\n");
lmb_dump_region(&lmb.free_mem, "memory");
lmb_dump_region(&lmb.available_mem, "memory"); lmb_dump_region(&lmb.used_mem, "reserved");}
@@ -650,7 +650,7 @@ static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, long lmb_add(phys_addr_t base, phys_size_t size) { long ret;
struct alist *lmb_rgn_lst = &lmb.free_mem;
struct alist *lmb_rgn_lst = &lmb.available_mem; ret = lmb_add_region(lmb_rgn_lst, base, size); if (ret)@@ -706,9 +706,9 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t base = 0; phys_addr_t res_base; struct lmb_region *lmb_used = lmb.used_mem.data;
struct lmb_region *lmb_memory = lmb.free_mem.data;
struct lmb_region *lmb_memory = lmb.available_mem.data;
for (i = lmb.free_mem.count - 1; i >= 0; i--) {
for (i = lmb.available_mem.count - 1; i >= 0; i--) { phys_addr_t lmbbase = lmb_memory[i].base; phys_size_t lmbsize = lmb_memory[i].size;@@ -800,10 +800,10 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, enum lmb_flags flags) { long rgn;
struct lmb_region *lmb_memory = lmb.free_mem.data;
struct lmb_region *lmb_memory = lmb.available_mem.data; /* Check if the requested address is in one of the memory regions */
rgn = lmb_overlaps_region(&lmb.free_mem, base, size);
rgn = lmb_overlaps_region(&lmb.available_mem, base, size); if (rgn >= 0) { /* * Check if the requested end address is in the same memory@@ -854,10 +854,10 @@ phys_size_t lmb_get_free_size(phys_addr_t addr) int i; long rgn; struct lmb_region *lmb_used = lmb.used_mem.data;
struct lmb_region *lmb_memory = lmb.free_mem.data;
struct lmb_region *lmb_memory = lmb.available_mem.data; /* check if the requested address is in the memory regions */
rgn = lmb_overlaps_region(&lmb.free_mem, addr, 1);
rgn = lmb_overlaps_region(&lmb.available_mem, addr, 1); if (rgn >= 0) { for (i = 0; i < lmb.used_mem.count; i++) { if (addr < lmb_used[i].base) {@@ -871,8 +871,8 @@ phys_size_t lmb_get_free_size(phys_addr_t addr) } } /* if we come here: no reserved ranges above requested addr */
return lmb_memory[lmb.free_mem.count - 1].base +lmb_memory[lmb.free_mem.count - 1].size - addr;
return lmb_memory[lmb.available_mem.count - 1].base +lmb_memory[lmb.available_mem.count - 1].size - addr; } return 0;} @@ -895,7 +895,7 @@ static int lmb_setup(bool test) { bool ret;
ret = alist_init(&lmb.free_mem, sizeof(struct lmb_region),
ret = alist_init(&lmb.available_mem, sizeof(struct lmb_region), (uint)LMB_ALIST_INITIAL_SIZE); if (!ret) { log_debug("Unable to initialise the list for LMB free memory\n");@@ -967,7 +967,7 @@ int lmb_push(struct lmb *store)
void lmb_pop(struct lmb *store) {
alist_uninit(&lmb.free_mem);
alist_uninit(&lmb.available_mem); alist_uninit(&lmb.used_mem); lmb = *store;} diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c index bb419ab2394e..e5b422ab2ba8 100644 --- a/test/cmd/bdinfo.c +++ b/test/cmd/bdinfo.c @@ -131,7 +131,7 @@ static int lmb_test_dump_all(struct unit_test_state *uts) struct lmb *lmb = lmb_get();
ut_assert_nextline("lmb_dump_all:");
ut_assertok(lmb_test_dump_region(uts, &lmb->free_mem, "memory"));
ut_assertok(lmb_test_dump_region(uts, &lmb->available_mem, "memory")); ut_assertok(lmb_test_dump_region(uts, &lmb->used_mem, "reserved")); return 0;diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 8af5dcad2ecc..0f7052224c10 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -65,7 +65,7 @@ static int setup_lmb_test(struct unit_test_state *uts, struct lmb *store,
ut_assertok(lmb_push(store)); lmb = lmb_get();
*mem_lstp = &lmb->free_mem;
*mem_lstp = &lmb->available_mem; *used_lstp = &lmb->used_mem; return 0;-- 2.45.2
There's no point defining a function that's called only once just to avoid passing the flags. Remove the wrapper and just call lmb_add_region_flags().
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/lmb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index ffdd23d87b9b..56e005308dde 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -640,19 +640,13 @@ void lmb_add_memory(void) } }
-static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base, - phys_size_t size) -{ - return lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE); -} - /* This routine may be called with relocation disabled. */ long lmb_add(phys_addr_t base, phys_size_t size) { long ret; struct alist *lmb_rgn_lst = &lmb.available_mem;
- ret = lmb_add_region(lmb_rgn_lst, base, size); + ret = lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE); if (ret) return ret;
Am 8. Dezember 2024 11:52:07 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
There's no point defining a function that's called only once just to avoid passing the flags. Remove the wrapper and just call lmb_add_region_flags().
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/lmb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index ffdd23d87b9b..56e005308dde 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -640,19 +640,13 @@ void lmb_add_memory(void) } }
-static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base,
phys_size_t size)-{
- return lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE);
-}
/* This routine may be called with relocation disabled. */
This comment should be moved to lmb.h.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
long lmb_add(phys_addr_t base, phys_size_t size) { long ret; struct alist *lmb_rgn_lst = &lmb.available_mem;
- ret = lmb_add_region(lmb_rgn_lst, base, size);
- ret = lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE); if (ret) return ret;
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
There's no point defining a function that's called only once just to avoid passing the flags. Remove the wrapper and just call lmb_add_region_flags().
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Acked-by: Sughosh Ganu sughosh.ganu@linaro.org
-sughosh
lib/lmb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c index ffdd23d87b9b..56e005308dde 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -640,19 +640,13 @@ void lmb_add_memory(void) } }
-static long lmb_add_region(struct alist *lmb_rgn_lst, phys_addr_t base,
phys_size_t size)-{
return lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE);-}
/* This routine may be called with relocation disabled. */ long lmb_add(phys_addr_t base, phys_size_t size) { long ret; struct alist *lmb_rgn_lst = &lmb.available_mem;
ret = lmb_add_region(lmb_rgn_lst, base, size);
ret = lmb_add_region_flags(lmb_rgn_lst, base, size, LMB_NONE); if (ret) return ret;-- 2.45.2
lmb_alloc_addr() is just calling lmb_alloc_addr_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 0/1 grow/shrink: 5/0 up/down: 33/-4 (29) Function old new delta test_alloc_addr 2939 2963 +24 wget_handler 1530 1533 +3 tftp_handler 1190 1192 +2 do_spi_flash 3150 3152 +2 _fs_read.lto_priv 331 333 +2 lmb_alloc_addr 4 - -4 Total: Before=2492734, After=2492763, chg +0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- fs/fs.c | 2 +- include/lmb.h | 3 +-- lib/lmb.c | 9 --------- test/lib/lmb.c | 28 +++++++++++++++------------- 4 files changed, 17 insertions(+), 25 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c index 21a23efd932d..b7d422eaf0d1 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -554,7 +554,7 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
lmb_dump_all();
- if (lmb_alloc_addr(addr, read_len) == addr) + if (lmb_alloc_addr_flags(addr, read_len, LMB_NONE) == addr) return 0;
log_err("** Reading file would overwrite reserved memory **\n"); diff --git a/include/lmb.h b/include/lmb.h index 54af5b0e4b44..4e8e4f23e279 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -94,7 +94,6 @@ long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags); phys_addr_t lmb_alloc(phys_size_t size, ulong align); phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr); -phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size); phys_size_t lmb_get_free_size(phys_addr_t addr);
phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, @@ -152,7 +151,7 @@ void lmb_pop(struct lmb *store);
static inline int lmb_read_check(phys_addr_t addr, phys_size_t len) { - return lmb_alloc_addr(addr, len) == addr ? 0 : -1; + return lmb_alloc_addr_flags(addr, len, LMB_NONE) == addr ? 0 : -1; }
/** diff --git a/lib/lmb.c b/lib/lmb.c index 56e005308dde..6dbdd81bd7d8 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -815,15 +815,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, return 0; }
-/* - * Try to allocate a specific address range: must be in defined memory but not - * reserved - */ -phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size) -{ - return _lmb_alloc_addr(base, size, LMB_NONE); -} - /** * lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes * @base: Base Address requested diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 0f7052224c10..49857cb3fd4b 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -560,22 +560,24 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);
/* allocate blocks */ - a = lmb_alloc_addr(ram, alloc_addr_a - ram); + a = lmb_alloc_addr_flags(ram, alloc_addr_a - ram, LMB_NONE); ut_asserteq(a, ram); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, ram, 0x8010000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); - b = lmb_alloc_addr(alloc_addr_a + 0x10000, - alloc_addr_b - alloc_addr_a - 0x10000); + b = lmb_alloc_addr_flags(alloc_addr_a + 0x10000, + alloc_addr_b - alloc_addr_a - 0x10000, + LMB_NONE); ut_asserteq(b, alloc_addr_a + 0x10000); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, ram, 0x10010000, alloc_addr_c, 0x10000, 0, 0); - c = lmb_alloc_addr(alloc_addr_b + 0x10000, - alloc_addr_c - alloc_addr_b - 0x10000); + c = lmb_alloc_addr_flags(alloc_addr_b + 0x10000, + alloc_addr_c - alloc_addr_b - 0x10000, + LMB_NONE); ut_asserteq(c, alloc_addr_b + 0x10000); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram, 0x18010000, 0, 0, 0, 0); - d = lmb_alloc_addr(alloc_addr_c + 0x10000, - ram_end - alloc_addr_c - 0x10000); + d = lmb_alloc_addr_flags(alloc_addr_c + 0x10000, + ram_end - alloc_addr_c - 0x10000, LMB_NONE); ut_asserteq(d, alloc_addr_c + 0x10000); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram, ram_size, 0, 0, 0, 0); @@ -591,7 +593,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
/* allocate at 3 points in free range */
- d = lmb_alloc_addr(ram_end - 4, 4); + d = lmb_alloc_addr_flags(ram_end - 4, 4, LMB_NONE); ut_asserteq(d, ram_end - 4); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, ram, 0x18010000, d, 4, 0, 0); @@ -600,7 +602,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram, 0x18010000, 0, 0, 0, 0);
- d = lmb_alloc_addr(ram_end - 128, 4); + d = lmb_alloc_addr_flags(ram_end - 128, 4, LMB_NONE); ut_asserteq(d, ram_end - 128); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, ram, 0x18010000, d, 4, 0, 0); @@ -609,7 +611,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram, 0x18010000, 0, 0, 0, 0);
- d = lmb_alloc_addr(alloc_addr_c + 0x10000, 4); + d = lmb_alloc_addr_flags(alloc_addr_c + 0x10000, 4, LMB_NONE); ut_asserteq(d, alloc_addr_c + 0x10000); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram, 0x18010004, 0, 0, 0, 0); @@ -624,18 +626,18 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram + 0x8000000, 0x10010000, 0, 0, 0, 0);
- d = lmb_alloc_addr(ram, 4); + d = lmb_alloc_addr_flags(ram, 4, LMB_NONE); ut_asserteq(d, ram); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, d, 4, ram + 0x8000000, 0x10010000, 0, 0);
/* check that allocating outside memory fails */ if (ram_end != 0) { - ret = lmb_alloc_addr(ram_end, 1); + ret = lmb_alloc_addr_flags(ram_end, 1, LMB_NONE); ut_asserteq(ret, 0); } if (ram != 0) { - ret = lmb_alloc_addr(ram - 1, 1); + ret = lmb_alloc_addr_flags(ram - 1, 1, LMB_NONE); ut_asserteq(ret, 0); }
lmb_alloc_base() is just calling lmb_alloc_base_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow.
The code size increase is minimal - e.g for sandbox which includes all of the LMB tests
add/remove: 1/2 grow/shrink: 4/1 up/down: 316/-382 (-66) Function old new delta lmb_alloc_base_flags - 299 +299 test_multi_alloc.constprop 3036 3042 +6 lmb_alloc 4 9 +5 boot_relocate_fdt 601 605 +4 test_noreserved 1207 1209 +2 lmb_alloc_base 48 - -48 efi_allocate_pages.part 303 249 -54 _lmb_alloc_base.lto_priv 280 - -280 Total: Before=2492763, After=2492697, chg -0.00%
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- boot/image-board.c | 22 +++++++++++++--------- boot/image-fdt.c | 6 ++++-- include/lmb.h | 1 - lib/lmb.c | 15 +-------------- test/lib/lmb.c | 8 ++++---- 5 files changed, 22 insertions(+), 30 deletions(-)
diff --git a/boot/image-board.c b/boot/image-board.c index c4d914fd6074..d84104ab5c9f 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -565,9 +565,11 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, lmb_reserve_flags(rd_data, rd_len, LMB_NONE); } else { if (initrd_high) - *initrd_start = (ulong)lmb_alloc_base(rd_len, - 0x1000, - initrd_high); + *initrd_start = + (ulong)lmb_alloc_base_flags(rd_len, + 0x1000, + initrd_high, + LMB_NONE); else *initrd_start = (ulong)lmb_alloc(rd_len, 0x1000); @@ -838,8 +840,9 @@ int boot_get_cmdline(ulong *cmd_start, ulong *cmd_end) return 0;
barg = IF_ENABLED_INT(CONFIG_SYS_BOOT_GET_CMDLINE, CONFIG_SYS_BARGSIZE); - cmdline = (char *)(ulong)lmb_alloc_base(barg, 0xf, - env_get_bootm_mapsize() + env_get_bootm_low()); + cmdline = (char *)(ulong)lmb_alloc_base_flags(barg, 0xf, + env_get_bootm_mapsize() + env_get_bootm_low(), + LMB_NONE); if (!cmdline) return -1;
@@ -871,10 +874,11 @@ int boot_get_cmdline(ulong *cmd_start, ulong *cmd_end) */ int boot_get_kbd(struct bd_info **kbd) { - *kbd = (struct bd_info *)(ulong)lmb_alloc_base(sizeof(struct bd_info), - 0xf, - env_get_bootm_mapsize() + - env_get_bootm_low()); + *kbd = (struct bd_info *)(ulong)lmb_alloc_base_flags(sizeof(struct bd_info), + 0xf, + env_get_bootm_mapsize() + + env_get_bootm_low(), + LMB_NONE); if (!*kbd) return -1;
diff --git a/boot/image-fdt.c b/boot/image-fdt.c index fd68b8594325..a4ef01a8a470 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -188,7 +188,8 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size) LMB_NONE); disable_relocation = 1; } else if (desired_addr) { - addr = lmb_alloc_base(of_len, 0x1000, desired_addr); + addr = lmb_alloc_base_flags(of_len, 0x1000, desired_addr, + LMB_NONE); of_start = map_sysmem(addr, of_len); if (of_start == NULL) { puts("Failed using fdt_high value for Device Tree"); @@ -217,7 +218,8 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size) * for LMB allocation. */ usable = min(start + size, low + mapsize); - addr = lmb_alloc_base(of_len, 0x1000, usable); + addr = lmb_alloc_base_flags(of_len, 0x1000, usable, + LMB_NONE); of_start = map_sysmem(addr, of_len); /* Allocation succeeded, use this block. */ if (of_start != NULL) diff --git a/include/lmb.h b/include/lmb.h index 4e8e4f23e279..a35a69d69f77 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -93,7 +93,6 @@ long lmb_add(phys_addr_t base, phys_size_t size); long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags); phys_addr_t lmb_alloc(phys_size_t size, ulong align); -phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr); phys_size_t lmb_get_free_size(phys_addr_t addr);
phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, diff --git a/lib/lmb.c b/lib/lmb.c index 6dbdd81bd7d8..c09f1a1277ad 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -747,20 +747,7 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align,
phys_addr_t lmb_alloc(phys_size_t size, ulong align) { - return lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE); -} - -phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr) -{ - phys_addr_t alloc; - - alloc = _lmb_alloc_base(size, align, max_addr, LMB_NONE); - - if (alloc == 0) - printf("ERROR: Failed to allocate 0x%lx bytes below 0x%lx.\n", - (ulong)size, (ulong)max_addr); - - return alloc; + return lmb_alloc_base_flags(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); }
/** diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 49857cb3fd4b..df61b2fd5a0c 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -128,7 +128,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, ASSERT_LMB(mem_lst, used_lst, 0, 0, 2, alloc_64k_addr, 0x10000, ram_end - 4, 4, 0, 0); /* alloc below end of reserved region -> below reserved region */ - b = lmb_alloc_base(4, 1, alloc_64k_end); + b = lmb_alloc_base_flags(4, 1, alloc_64k_end, LMB_NONE); ut_asserteq(b, alloc_64k_addr - 4); ASSERT_LMB(mem_lst, used_lst, 0, 0, 2, alloc_64k_addr - 4, 0x10000 + 4, ram_end - 4, 4, 0, 0); @@ -138,7 +138,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, ut_asserteq(c, ram_end - 8); ASSERT_LMB(mem_lst, used_lst, 0, 0, 2, alloc_64k_addr - 4, 0x10000 + 4, ram_end - 8, 8, 0, 0); - d = lmb_alloc_base(4, 1, alloc_64k_end); + d = lmb_alloc_base_flags(4, 1, alloc_64k_end, LMB_NONE); ut_asserteq(d, alloc_64k_addr - 8); ASSERT_LMB(mem_lst, used_lst, 0, 0, 2, alloc_64k_addr - 8, 0x10000 + 8, ram_end - 8, 8, 0, 0); @@ -163,7 +163,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, alloc_64k_addr - 8, 4, alloc_64k_addr, 0x10000, ram_end - 8, 4); /* allocate again to ensure we get the same address */ - b2 = lmb_alloc_base(4, 1, alloc_64k_end); + b2 = lmb_alloc_base_flags(4, 1, alloc_64k_end, LMB_NONE); ut_asserteq(b, b2); ASSERT_LMB(mem_lst, used_lst, 0, 0, 2, alloc_64k_addr - 8, 0x10000 + 8, ram_end - 8, 4, 0, 0); @@ -363,7 +363,7 @@ static int test_noreserved(struct unit_test_state *uts, const phys_addr_t ram, ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 0, 0, 0, 0, 0, 0, 0);
/* allocate a block with base*/ - b = lmb_alloc_base(alloc_size, align, ram_end); + b = lmb_alloc_base_flags(alloc_size, align, ram_end, LMB_NONE); ut_assert(a == b); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, ram + ram_size - alloc_size_aligned,
After all the cleanups of the previous patches there's not point anymore to define lmb_alloc_addr_flags() and _lmb_alloc_addr_flags().
Fix the incocistency of the flags type, which one function was definining as uintn and the other as an enum lmb_flags and rename _lmb_alloc_addr_flags() to lmb_alloc_addr_flags()
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- include/lmb.h | 2 +- lib/lmb.c | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index a35a69d69f77..4f3c861b15a6 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -111,7 +111,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, * Return: base address on success, 0 on error */ phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size, - uint flags); + enum lmb_flags flags);
/** * lmb_is_reserved_flags() - test if address is in reserved region with flag bits set diff --git a/lib/lmb.c b/lib/lmb.c index c09f1a1277ad..67bde60b74be 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -777,8 +777,20 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, return alloc; }
-static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, - enum lmb_flags flags) +/** + * lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes + * @base: Base Address requested + * @size: Size of the region requested + * @flags: Memory region attributes to be set + * + * Allocate a region of memory with the attributes specified through the + * parameter. The base parameter is used to specify the base address + * of the requested region. + * + * Return: base address on success, 0 on error + */ +phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size, + enum lmb_flags flags) { long rgn; struct lmb_region *lmb_memory = lmb.available_mem.data; @@ -802,24 +814,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, return 0; }
-/** - * lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes - * @base: Base Address requested - * @size: Size of the region requested - * @flags: Memory region attributes to be set - * - * Allocate a region of memory with the attributes specified through the - * parameter. The base parameter is used to specify the base address - * of the requested region. - * - * Return: base address on success, 0 on error - */ -phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size, - uint flags) -{ - return _lmb_alloc_addr(base, size, flags); -} - /* Return number of bytes from a given address that are free */ phys_size_t lmb_get_free_size(phys_addr_t addr) {
Am 8. Dezember 2024 11:52:10 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
After all the cleanups of the previous patches there's not point anymore to define lmb_alloc_addr_flags() and _lmb_alloc_addr_flags().
Fix the incocistency of the flags type, which one function was definining as uintn and the other as an enum lmb_flags and rename _lmb_alloc_addr_flags() to lmb_alloc_addr_flags()
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/lmb.h | 2 +- lib/lmb.c | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index a35a69d69f77..4f3c861b15a6 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -111,7 +111,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
- Return: base address on success, 0 on error
*/ phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags);
enum lmb_flags flags);
This change implies that flags IS NOT a bitfield. Flags may only hold a SINGLE value from the enum.
This looks wrong looking at the definition of the constants via BIT().
Defining the constants of a bitfield in an enum was wrong (or at best misleading) from the beginning.
Best regards
Heinrich
/**
- lmb_is_reserved_flags() - test if address is in reserved region with flag bits set
diff --git a/lib/lmb.c b/lib/lmb.c index c09f1a1277ad..67bde60b74be 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -777,8 +777,20 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, return alloc; }
-static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
enum lmb_flags flags)+/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
+phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
enum lmb_flags flags){ long rgn; struct lmb_region *lmb_memory = lmb.available_mem.data; @@ -802,24 +814,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, return 0; }
-/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
-phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags)-{
- return _lmb_alloc_addr(base, size, flags);
-}
/* Return number of bytes from a given address that are free */ phys_size_t lmb_get_free_size(phys_addr_t addr) {
Hi Heinrich
On Sun, 8 Dec 2024 at 14:05, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. Dezember 2024 11:52:10 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
After all the cleanups of the previous patches there's not point anymore to define lmb_alloc_addr_flags() and _lmb_alloc_addr_flags().
Fix the incocistency of the flags type, which one function was definining as uintn and the other as an enum lmb_flags and rename _lmb_alloc_addr_flags() to lmb_alloc_addr_flags()
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/lmb.h | 2 +- lib/lmb.c | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index a35a69d69f77..4f3c861b15a6 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -111,7 +111,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
- Return: base address on success, 0 on error
*/ phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags);
enum lmb_flags flags);This change implies that flags IS NOT a bitfield. Flags may only hold a SINGLE value from the enum.
This looks wrong looking at the definition of the constants via BIT().
Defining the constants of a bitfield in an enum was wrong (or at best misleading) from the beginning.
Ah yes correct, I'll just leave it to uint and change the function prototype
Thanks /Ilias
Best regards
Heinrich
/**
- lmb_is_reserved_flags() - test if address is in reserved region with flag bits set
diff --git a/lib/lmb.c b/lib/lmb.c index c09f1a1277ad..67bde60b74be 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -777,8 +777,20 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, return alloc; }
-static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
enum lmb_flags flags)+/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
+phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
enum lmb_flags flags){ long rgn; struct lmb_region *lmb_memory = lmb.available_mem.data; @@ -802,24 +814,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, return 0; }
-/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
-phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags)-{
return _lmb_alloc_addr(base, size, flags);-}
/* Return number of bytes from a given address that are free */ phys_size_t lmb_get_free_size(phys_addr_t addr) {
On Sun, 8 Dec 2024 at 14:12, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Heinrich
On Sun, 8 Dec 2024 at 14:05, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. Dezember 2024 11:52:10 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
After all the cleanups of the previous patches there's not point anymore to define lmb_alloc_addr_flags() and _lmb_alloc_addr_flags().
Fix the incocistency of the flags type, which one function was definining as uintn and the other as an enum lmb_flags and rename _lmb_alloc_addr_flags() to lmb_alloc_addr_flags()
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/lmb.h | 2 +- lib/lmb.c | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index a35a69d69f77..4f3c861b15a6 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -111,7 +111,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
- Return: base address on success, 0 on error
*/ phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags);
enum lmb_flags flags);This change implies that flags IS NOT a bitfield. Flags may only hold a SINGLE value from the enum.
This looks wrong looking at the definition of the constants via BIT().
Defining the constants of a bitfield in an enum was wrong (or at best misleading) from the beginning.
Ah yes correct, I'll just leave it to uint and change the function prototype
Hmm thinking about it again. Those flags are never used as bitfield. They are always used as discrete values. So I think we should keep it as is
Thanks /Ilias
Thanks /Ilias
Best regards
Heinrich
/**
- lmb_is_reserved_flags() - test if address is in reserved region with flag bits set
diff --git a/lib/lmb.c b/lib/lmb.c index c09f1a1277ad..67bde60b74be 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -777,8 +777,20 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, return alloc; }
-static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
enum lmb_flags flags)+/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
+phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
enum lmb_flags flags){ long rgn; struct lmb_region *lmb_memory = lmb.available_mem.data; @@ -802,24 +814,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, return 0; }
-/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
-phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags)-{
return _lmb_alloc_addr(base, size, flags);-}
/* Return number of bytes from a given address that are free */ phys_size_t lmb_get_free_size(phys_addr_t addr) {
On Sun, 8 Dec 2024 at 14:17, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Sun, 8 Dec 2024 at 14:12, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Heinrich
On Sun, 8 Dec 2024 at 14:05, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 8. Dezember 2024 11:52:10 MEZ schrieb Ilias Apalodimas ilias.apalodimas@linaro.org:
After all the cleanups of the previous patches there's not point anymore to define lmb_alloc_addr_flags() and _lmb_alloc_addr_flags().
Fix the incocistency of the flags type, which one function was definining as uintn and the other as an enum lmb_flags and rename _lmb_alloc_addr_flags() to lmb_alloc_addr_flags()
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
include/lmb.h | 2 +- lib/lmb.c | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/include/lmb.h b/include/lmb.h index a35a69d69f77..4f3c861b15a6 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -111,7 +111,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
- Return: base address on success, 0 on error
*/ phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags);
enum lmb_flags flags);This change implies that flags IS NOT a bitfield. Flags may only hold a SINGLE value from the enum.
This looks wrong looking at the definition of the constants via BIT().
Defining the constants of a bitfield in an enum was wrong (or at best misleading) from the beginning.
Ah yes correct, I'll just leave it to uint and change the function prototype
Hmm thinking about it again. Those flags are never used as bitfield. They are always used as discrete values. So I think we should keep it as is
Ok, the above is not correct, we do indeed them use as a bitmask. So I'll just change the type to unit
Thanks /Ilias
Thanks /Ilias
Thanks /Ilias
Best regards
Heinrich
/**
- lmb_is_reserved_flags() - test if address is in reserved region with flag bits set
diff --git a/lib/lmb.c b/lib/lmb.c index c09f1a1277ad..67bde60b74be 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -777,8 +777,20 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, return alloc; }
-static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
enum lmb_flags flags)+/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
+phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
enum lmb_flags flags){ long rgn; struct lmb_region *lmb_memory = lmb.available_mem.data; @@ -802,24 +814,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size, return 0; }
-/**
- lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
- @base: Base Address requested
- @size: Size of the region requested
- @flags: Memory region attributes to be set
- Allocate a region of memory with the attributes specified through the
- parameter. The base parameter is used to specify the base address
- of the requested region.
- Return: base address on success, 0 on error
- */
-phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
uint flags)-{
return _lmb_alloc_addr(base, size, flags);-}
/* Return number of bytes from a given address that are free */ phys_size_t lmb_get_free_size(phys_addr_t addr) {
+CC Sam
Apologies, I forgot to CC you, I guess you can find the rest of the series on lore?
On Sun, 8 Dec 2024 at 12:52, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
The LMB subsystem was used opportunistically for a number of years. A while back Sughosh merged it with the EFI subsystem in order to have a common allocator and avoid subsystems overwriting memory they shouldn't.
This is an initial cleanup of all the crud we gathered over the years. There's no functional change expected from the patches as they just cleanup some abstraction functions and rename a few variables to make more sense.
This has to be rebased on top of Sam's [0] changes but that should be trivial.
I plan to make even bigger changes -- e.g I don't see the point of having *_alloc() and *_reserve() versions of the functions since they mostly do the same thing and just cause confusion. lmb_alloc_addr_flags() returning the base address on success makes little sense since we already *request* the address on the function arguments, etc. But since this patchset grew enough already, I'd like to get it in before more refactoring happens.
It's worth noting that although some patches slightly increase the code size due to an extra flags argument being carried around, the final result is eventually smaller.
add/remove: 2/5 grow/shrink: 17/3 up/down: 479/-524 (-45) Function old new delta lmb_alloc_base_flags - 299 +299 lmb_alloc_addr_flags - 90 +90 test_alloc_addr 2933 2963 +30 lib_test_lmb_overlapping_reserve 1018 1030 +12 test_multi_alloc.constprop 3034 3042 +8 test_get_unreserved_size 1032 1038 +6 boot_relocate_fdt 599 605 +6 lmb_alloc 4 9 +5 wget_handler 1530 1533 +3 tftp_handler 1190 1192 +2 test_noreserved 1207 1209 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_spi_flash 3150 3152 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 _fs_read.lto_priv 331 333 +2 lmb_reserve 4 - -4 lmb_alloc_addr 4 - -4 lmb_overlaps_region 62 52 -10 lmb_add_region_flags 600 568 -32 lmb_alloc_base 48 - -48 efi_allocate_pages.part 303 249 -54 _lmb_alloc_addr.lto_priv 92 - -92 _lmb_alloc_base.lto_priv 280 - -280 Total: Before=2492742, After=2492697, chg -0.00%
[0] https://lore.kernel.org/u-boot/20241208002121.31887-1-semen.protsenko@linaro...
Ilias Apalodimas (7): lmb: Replace lmb_reserve() with lmb_reserve_flags() lmb: Simplify lmb_addrs_overlap() lmb: Rename free_mem to available_mem lmb: Remove lmb_add_region() lmb: Replace lmb_alloc_addr() with lmb_alloc_addr_flags() lmb: Replace lmb_alloc_base() with lmb_alloc_base_flags() lmb: Rename _lmb_alloc_addr_flags()
arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 +- boot/image-board.c | 24 +++++--- boot/image-fdt.c | 11 ++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- fs/fs.c | 2 +- include/lmb.h | 11 ++-- lib/lmb.c | 111 +++++++++++----------------------- test/cmd/bdinfo.c | 2 +- test/lib/lmb.c | 68 +++++++++++---------- 13 files changed, 105 insertions(+), 137 deletions(-)
-- 2.45.2
participants (5)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Simon Glass -
Sughosh Ganu -
Tom Rini