[PATCH v2 6/6] common: Add an option to relocate on ram top

[Ilias Apalodimas]

On Thu, 23 Apr 2026 at 19:06, Michal Simek <michal.simek at amd.com> wrote:
>
>
>
> On 4/23/26 17:28, Ilias Apalodimas wrote:
> > Hi Michal
> >
> >
> >>>    {
> >>>        debug("Monitor len: %08x\n", gd->mon_len);
> >>> -#if CONFIG_VAL(SYS_MEM_TOP_HIDE)
> >>> +
> >>> +     if (CONFIG_IS_ENABLED(RELOC_ADDR_TOP)) {
> >>> +             int bank;
> >>> +             phys_size_t total_size = 0;
> >>> +
> >>> +             for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> >>> +                     if (gd->ram_top <= gd->bi_dram[bank].start)
> >>> +                             gd->ram_top = gd->bi_dram[bank].start +
> >>> +                                             gd->bi_dram[bank].size;
> >>> +                     total_size += gd->bi_dram[bank].size;
> >>
> >> Here you have one problem. If the latest bank is defined with size = 0
> >> then your code won't ignore it and just use it. I think you should check at
> >> least that size is not zero.
> >
> > I don't think this checks belongs here. IIRC the only reason to do
> > that is have some previous stage bootloader, or earlier u-boot code
> > update the zero length with a valid region. We already do similar
> > checks in fdt_fixup_memory_banks(). We should add similar ones to the
> > dram_init_banksize() variants. Most of the board specific functions
> > call fdtdec_setup_memory_banksize() anyway, which I plan to fix as
> > well and get rid of board code that's not needed anymore.
> >
> >>
> >> Obviously size needs to be big enough to fit u-boot itself.
> >
> > Yea that makes sense, I can add a check for this.
> >
> >>
> >> And as I said this is one configuration. Versal Gen 2 code has by default
> >> configuration that it stays in a bank where it runs.
> >> board/amd/versal2/board.c: dram_init()
> >>
> >> Added by commit 3aebac5a759b ("arm64: versal2: Populate DRAM banks before page
> >> table size calculation")
> >> Focus on text variable.
> >>
> >> That's why I can imagine that above code can be extended by one more
> >> configuration to stay inside bank where actually u-boot is now.
> >
> > With having the dram bank in an array we can go an extra step in the
> > future and have a Kconfig option with an int, representing the array
> > position of the bank we want to relocate. That should cover what you
> > ask here?
>
> Position in an array won't help because we have generic configuration which can
> run with different amount of MCs/banks and it is not practical to say which one
> it is. That's why we use bank where u-boot runs before relocation.

Ok, then we'll need to add a Kconfig option for that o nce someone extends it.

>
> Another part which we were handling was actually also dealing with reserved
> memory nodes. Did you test it? I mean configuration where you have more banks
> and the last bank is all reserved. Then U-Boot shouldn't relocate to it. That's
> what it is handled today too.

Apart from some xilinx boards calling get_mem_top() I am not aware of
any core code handling this. The current patch only depends on what
dram_init_banksize() does. But then I don't think dram_init_banksize()
should be responsible for parsing the reserved memory nodes and
adjusting ram top, reloc addresses etc.
What I can change in the patch though is call get_mem_top() instead of
the for loop that adjusts ram_top. That function already takes the
reserved regions into account.

Thanks
/Ilias

> Thanks,
> Michal