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

[Caleb Connolly]

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.

-- 
Caleb (they/them)