| Message ID | 20241208002121.31887-7-semen.protsenko@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | lmb: Fix reserving the same region multiple times | expand |
On Sun, 8 Dec 2024 at 02:21, Sam Protsenko <semen.protsenko@linaro.org> 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 <semen.protsenko@linaro.org> > --- > 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 <ilias.apalodimas@linaro.org>
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,
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 <semen.protsenko@linaro.org> --- boot/image-fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)