U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Protsenko <semen.protsenko@linaro.org>
To: Tom Rini <trini@konsulko.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Sughosh Ganu <sughosh.ganu@linaro.org>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Simon Glass <sjg@chromium.org>,
	Caleb Connolly <caleb.connolly@linaro.org>,
	Marek Vasut <marex@denx.de>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Patrick Delaunay <patrick.delaunay@foss.st.com>,
	u-boot@lists.denx.de
Subject: [PATCH v2 2/2] boot: fdt: Handle already reserved memory in boot_fdt_reserve_region()
Date: Tue, 10 Dec 2024 20:17:02 -0600	[thread overview]
Message-ID: <20241211021703.2333-3-semen.protsenko@linaro.org> (raw)
In-Reply-To: <20241211021703.2333-1-semen.protsenko@linaro.org>

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, 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>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes in v2:
  - Added R-b tag from Ilias
  - Reworded the commit message a bit, reflecting changes in the lmb patch

 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


  parent reply	other threads:[~2024-12-11  2:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-11  2:17 [PATCH v2 0/2] lmb: Fix reserving the same region multiple times Sam Protsenko
2024-12-11  2:17 ` [PATCH v2 1/2] lmb: Return -EEXIST in lmb_add_region_flags() if region already added Sam Protsenko
2024-12-11  6:15   ` Ilias Apalodimas
2024-12-11  2:17 ` Sam Protsenko [this message]
2024-12-12 14:40 ` [PATCH v2 0/2] lmb: Fix reserving the same region multiple times Michal Simek
2024-12-12 19:18 ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241211021703.2333-3-semen.protsenko@linaro.org \
    --to=semen.protsenko@linaro.org \
    --cc=caleb.connolly@linaro.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=marex@denx.de \
    --cc=patrick.delaunay@foss.st.com \
    --cc=sjg@chromium.org \
    --cc=sughosh.ganu@linaro.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox