On Sun, 8 Dec 2024 at 02:21, Sam Protsenko <[email protected]> wrote: > > The boot_fdt_add_mem_rsv_regions() function can be called twice, e.g. > first time during the board init (as a part of LMB init), and then when > booting the OS with 'booti' command: > > lmb_add_region_flags > lmb_reserve_flags > boot_fdt_reserve_region > boot_fdt_add_mem_rsv_regions > ^ > | > +-----------------------+ > | (1) | (2) > lmb_reserve_common image_setup_linux > lmb_init ... > initr_lmb do_booti > board_init_r 'booti' > > That consequently leads to the attempt of reserving the same memory > areas (described in the 'reserved-memory' dts node) in LMB. The > lmb_add_region_flags() returns -EEXIST error code in such cases, but > boot_fdt_reserve_region() handles all negative error codes as a failure > to reserve fdt memory region, printing corresponding error messages, > which are essentially harmless, but misleading. For example, this is the > output of 'booti' command on E850-96 board: > > => booti $loadaddr - $fdtaddr > ... > ERROR: reserving fdt memory region failed > (addr=bab00000 size=5500000 flags=2) > ERROR: reserving fdt memory region failed > (addr=f0000000 size=200000 flags=4) > ... > Starting kernel ... > > The mentioned false positive error messages are observed starting with > commit 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory > areas with !LMB_NONE"), which removes the check for the already added > memory regions in lmb_add_region_flags(), making it return -1 for > !LMB_NONE cases. Another commit 827dee587b75 ("fdt: lmb: add reserved > regions as no-overwrite") changes flags used for reserving memory in > boot_fdt_add_mem_rsv_regions() from LMB_NONE to LMB_NOOVERWRITE. So > together with the patch mentioned earlier, it makes > lmb_add_region_flags() return -1 when called from > boot_fdt_reserve_region(). > > Since then, the different patch was implemented, adding the check back > and returning -EEXIST error code in described cases, which is: > > lmb: Return -EEXIST in lmb_add_region_flags() if region already added > > Handle -EEXIST error code as a normal (successful) case in > lmb_reserve_flags() and don't print any messages. > > Fixes: 1d9aa4a283da ("lmb: Fix the allocation of overlapping memory areas > with !LMB_NONE") > Signed-off-by: Sam Protsenko <[email protected]> > --- > boot/image-fdt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/boot/image-fdt.c b/boot/image-fdt.c > index 3d5b6f9e2dc7..73c43c30684f 100644 > --- a/boot/image-fdt.c > +++ b/boot/image-fdt.c > @@ -77,7 +77,7 @@ static void boot_fdt_reserve_region(u64 addr, u64 size, > enum lmb_flags flags) > debug(" reserving fdt memory region: addr=%llx size=%llx > flags=%x\n", > (unsigned long long)addr, > (unsigned long long)size, flags); > - } else { > + } else if (ret != -EEXIST) { > puts("ERROR: reserving fdt memory region failed "); > printf("(addr=%llx size=%llx flags=%x)\n", > (unsigned long long)addr, > -- > 2.39.5 >
Reviewed-by: Ilias Apalodimas <[email protected]>
