General boot-flow document:
https://doc.coreboot.org/soc/amd/family17h.html

Thanks for providing that doc, it makes many things clearer. However, I still don't really follow why running a separate romstage (and apparently also postcar?) binary in DRAM is the best/easiest way to model this boot flow. The only real reason you provide is to run ROMSTAGE_CBMEM_INIT_HOOKs, but almost all those hooks exist to copy data from pre-RAM storage into CBMEM. You don't have that pre-RAM storage and you don't have any existing data you want to copy, so why do you want to run the hooks? Most code doing that (e.g. CBMEM console, timestamps) should be written robustly enough that it should "just work" if you only start running it in ramstage (because usually they just check if the CBMEM space exists and reinitialize it from scratch if it doesn't, no matter the stage), and if there's any code that doesn't it would probably be easier and cleaner to fix that than to build a fake romstage environment for it. The only hook I can find in common code that does anything you may actually need to run is some of the vboot stuff, and you have to do some work there anyway because you ran vboot on another processor with its own memory. (How are you passing vboot data from the verstage forward, anyway? Your doc doesn't address that, but it's definitely something you'll have to figure out somehow.)

If you're worried about initializing CBMEM itself, ramstage already does that (in lib/hardwaremain.c#main). It will reinitialize CBMEM as empty if there's no previous record in memory. (If you're worried about recovering stale CBMEM data, it's just one line that needs to be changed to cbmem_initialize_empty(), selected by a Kconfig.)

All the other stuff you're doing (e.g. switching to protected mode) is custom to your platform regardless (romstage normally starts in protected mode already). Inserting that at the start of ramstage shouldn't be any harder than inserting it at the start of romstage?

(I'm also curious about how verstage can interact with the rest of the PSP boot flow. Can an RW update be used to update the ABL code or the APCB data? That's not really clear from the doc.)

View Change

To view, visit change 32414. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I678b1f74546ea30abcc655a0daed795d6cfa0034
Gerrit-Change-Number: 32414
Gerrit-PatchSet: 5
Gerrit-Owner: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich@gmail.com>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-CC: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 01:09:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment