[PATCH 04/12] coreboot: Move coreboot logic out of x86 to a generic place

[Simon Glass]

Hi,

On Mon, 24 Feb 2025 at 08:16, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
>
>
> On 2/22/25 01:17, Stephen Boyd wrote:
> > Quoting Tom Rini (2025-02-20 17:06:58)
> >> On Thu, Feb 20, 2025 at 12:58:46PM -0800, Stephen Boyd wrote:
> >>
> >>> Lay the groundwork to run U-Boot as a payload on ARM coreboot based
> >>> devices. Move the coreboot table parsing code out of arch/x86 into
> >>> lib/coreboot. The headers like cb_sysinfo.h and coreboot_tables.h need
> >>> to be globally accessible, so move them into the top level include
> >>> directory. Introduce helper functions like
> >>> board_get_usable_ram_top_from_coreboot(), dram_init_from_coreboot(), and
> >>> dram_init_banksize_from_coreboot() so that boards can still override
> >>> these common functions while also supporting booting as a coreboot
> >>> payload.
> >>>
> >>> Signed-off-by: Stephen Boyd <swboyd at chromium.org>
> >>
> >> I see how the abstractions get used later on. The rest of the examples
> >> in tree today are of the form foo_dram_init / foo_dram_init_banksize, so
> >> we should follow that form instead. And I'd be inclined, but I'd like to
> >> hear Caleb or Simon's thoughts too, on just overloading the weak
>
> This adds a 6th (!!!) possible way that U-Boot can be loaded on a
> Qualcomm platform, on top of:
>
> 1. Android chainload
> 2. in-XBL (replacing EDK2)
> 3. as-hyp (EL2)
> 4. as-tz (EL3 with TF-A boot info passed in)
> 5. EFI stub
>
> While (4) and (5) aren't supported upstream yet, if/when we do add them
> we'll have 5 different ways of retrieving the DRAM layout
>
> a. builtin FDT
> b. SMEM (qcom memory database)
> c. TF-A boot info
> d. EFI memory map
> e. coreboot boot info
>
> So to this suggestion (and Stephens answer below): The way Stephen has
> composed this coreboot support makes sense to me since I can just look
> at mach-snapdragon and all the codepaths are there. I'd rather not wrap
> the whole functions in mach-snapdragon in an #ifndef, it impacts
> readability and also makes it quite hard for someone not familiar with
> U-Boot to figure out a) why this is done, and b) what functions get
> called instead.
>
> I would eventually prefer it to be a runtime check rather than compile
> time, and the current proposal seems closest to that.
>
> >> function we already have and when we don't, #ifndef CONFIG_SYS_COREBOOT
> >> the existing snapdragon function. I suspect we might longer term need a
> >> bit different abstraction here to make it easier to override things when
> >> U-Boot is a payload and getting this information elsewhere.
>
> Agreed, I think I saw a proposal recently to some info to the /chosen
> node for this. Not sure if that's the correct approach but I do agree
> that we should have some kind of API in U-Boot to let us collect info
> from previous stages and metadata so that we use it later to inform
> decision making.
>
> >>
> >
> > Ok. I struggled for a bit trying to understand the design of the weak
> > functions. I'll rename it and have it overload when nothing is
> > different. That means board_get_usable_ram_top() can override all the
> > time. The dram_init() and dram_init_banksize() functions likely need to
> > stick around though and be called explicitly? Maybe we can have some
>
> I think Tom is proposing to wrap the entire functions in mach-snapdragon
> with #ifndef CONFIG_SYS_COREBOOT. Which as above I'm not super in favour of.
>
> > sort of 'payload' level event hook so that depending on the payload
> > that's detected we can call a function to setup memory.
>
> It really depends how far we want to go for a generic build. It would be
> nice if we could source DRAM info from all available sources (since
> there might be multiple) and then pick our favourite. I think this would
> let us do a much better job at defining and documenting all the various
> combos we have in mach-snapdragon too.

I too favour a more dynamic / run-time solution, with a more generic
board. If we can get some of these boards into a lab and connected to
gitlab, so much the better.

Regards,
Simon