Re: [PATCH 03/12] lmb: Add generic arch_lmb_reserve_generic()

Am Freitag, dem 10.09.2021 um 22:47 +0200 schrieb Marek Vasut:

The arc/arm/m68k/microblaze/mips/ppc arch_lmb_reserve() implementations are all mostly the same, except for a couple of details. Implement a generic arch_lmb_reserve_generic() function which can be parametrized enough to cater for those differences between architectures. This can also be parametrized enough so it can handle cases where U-Boot is not relocated to the end of DRAM e.g. because there is some other reserved memory past U-Boot (e.g. unmovable firmware for coprocessor), it is not relocated at all, and other such use cases.

Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Alexey Brodkin alexey.brodkin@synopsys.com Cc: Angelo Dureghello angelo@sysam.it Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Eugeniy Paltsev Eugeniy.Paltsev@synopsys.com Cc: Hai Pham hai.pham.ud@renesas.com Cc: Michal Simek monstr@monstr.eu Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Cc: Tom Rini trini@konsulko.com Cc: Wolfgang Denk wd@denx.de

---

V2: Reword code comment

include/lmb.h | 1 + lib/lmb.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)

diff --git a/include/lmb.h b/include/lmb.h index 3c4afdf9f0..1984291132 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -122,6 +122,7 @@ lmb_size_bytes(struct lmb_region *type, unsigned long region_nr)

void board_lmb_reserve(struct lmb *lmb); void arch_lmb_reserve(struct lmb *lmb); +void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align);

/* Low level functions */

diff --git a/lib/lmb.c b/lib/lmb.c index 7bd1255f7a..793647724c 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -12,6 +12,10 @@ #include <log.h> #include <malloc.h>

+#include <asm/global_data.h>

+DECLARE_GLOBAL_DATA_PTR;

#define LMB_ALLOC_ANYWHERE 0

static void lmb_dump_region(struct lmb_region *rgn, char *name) @@ -113,6 +117,37 @@ void lmb_init(struct lmb *lmb) lmb->reserved.cnt = 0; }

+void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align) +{

• ulong bank_end;
• int bank;
• /*

• * Reserve memory from aligned address below the bottom of U-
  

Boot stack

• * until end of U-Boot area using LMB to prevent U-Boot from
  

overwriting

• * that memory.
  

• */
  

• debug("## Current stack ends at 0x%08lx ", sp);
• /* adjust sp by 4K to be safe */

nit: comment doesn't fit anymore

• sp -= align;
• for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {

• if (!gd->bd->bi_dram[bank].size ||
  

•     sp < gd->bd->bi_dram[bank].start)
  

• 	continue;
  

• /* Watch out for RAM at end of address space! */
  

• bank_end = gd->bd->bi_dram[bank].start +
  

• 	gd->bd->bi_dram[bank].size - 1;
  

• if (sp > bank_end)
  

• 	continue;
  

• if (bank_end > end)
  

• 	bank_end = end - 1;
  

isn't that all already taken care of when initializing gd->ram_top as well as gd->relocaddr and gd->start_addr_sp (based on gd->ram_top)? So gd->ram_top is already guaranteed to be less or equal some DRAM bank end address. AFAIK arch_lmb_reserve() should just protect the U-Boot area from gd->start_addr_sp up to gd->ram_top as well as the stack area from the current stack pointer up to the initial stack pointer in gd-

start_addr_sp. Also you changed all callers to always pass gd->ram_top

in "ulong end". Thus I think arch_lmb_reserve_generic() could omit those redundant checks and could be simplified to:

sp -= align; lmb_reserve(lmb, sp, gd->ram_top - sp);

Or am I overlooking something? Is that a valid use case to have U-Boot area and stack area in different memory banks? If yes, shouldn't then lmb_reserve() be called twice by arch_lmb_reserve() to protect stack area AND U-Boot area?

• lmb_reserve(lmb, sp, bank_end - sp + 1);
  

• break;
  

• }

+}

static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob) { arch_lmb_reserve(lmb);