Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36211 )
Change subject: cpu/intel/car: Add EC software sync to Intel romstage ......................................................................
Patch Set 1:
Would a common romstage state machine make sense? You'd have pre_raminit (console init if done in bootblock, stack guards, ), early/raminit, post_raminit (prepare postcar, ...) callbacks?
It's a little more complicated than that given the new AMD boot architecture. But I currently don't see a need that necessitates more infrastructure aside placing calls at the correct point. One could put in a generic callback at various points in that file and hook in like that. But is that necessary when we already know Kconfig is honoring dependencies. The compiler will omit the call sequences through dead code elimination.
Ack. I was just thinking that now that romstage bootflows are becoming more aligned it might be a good idea to unify things in general. This idea does not particularly pertain to this patch per se but it would fit in a bit more nicely assuming that doing the EC sync early is not something you only needs to be done on Intel targets if I may assume that?
Right now we're starting w/ Intel platforms, yes. But as I noted Kconfig dependencies handle the situation when it isn't needed, I think. Though, we may need to tighten the dependencies correctly as vboot doesn't necessarily mean EC sw sync.
All that said, I would like to place the calls/support to be in common code paths because then those features easily apply everywhere vs needing to touch more code just to enable something after the Kconfig flip.
-
Aaron Durbin (Code Review)