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+rene...@gmail.com>
> Cc: Alexey Brodkin <alexey.brod...@synopsys.com>
> Cc: Angelo Dureghello <ang...@sysam.it>
> Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com>
> Cc: Eugeniy Paltsev <eugeniy.palt...@synopsys.com>
> Cc: Hai Pham <hai.pham...@renesas.com>
> Cc: Michal Simek <mon...@monstr.eu>
> Cc: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
> Cc: Tom Rini <tr...@konsulko.com>
> Cc: Wolfgang Denk <w...@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);
--
- Daniel
