Hi, On Mon, 24 Feb 2025 at 08:16, Caleb Connolly <caleb.conno...@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 <swb...@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
