Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33142 )
Change subject: Kconfig: Create coreboot separate stage kconfigs ......................................................................
Patch Set 2:
Patch Set 2:
This is my point about the requirements. We need to be able to select which stages get put into the build itself. And we should be able to do that from platform or mainboard code. What you put up doesn't allow one to mix and match the stages selected. i.e. the 'current coreboot' doesn't meet the new requirements.
Okay, then what requirements are those? I can't really imagine a use case where an SoC or mainboard would want to omit a whole stage without any involvement of a toplevel Kconfig that changes behavior in some generic code. I mean, run_romstage() or run_ramstage() are called from generic code, a mainboard can't just decide on its own to not have a romstage for some reason. It would have to select an option that changes all the generic behavior which normally expects the existence of that stage.
If you're thinking about the new AMD stuff I think that should certainly get a new toplevel option to control omission of the pre-RAM stages somewhere. Other than that I'm not aware of another upcoming need for excluding stages. I doubt we're suddenly going to see so many that handling them all as toplevel Kconfigs would become a maintenance problem?
- Ramstage is not a tiny stage, it still takes around 200KB of cbfs space (RO, RW-A/B, 200*3=600KB)
- Adding to boot time, with RAMPAYLOAD is enable and having ramstage inside CBFS vs not inside CBFS is adding up to 100ms of boot time.
Just to clarify, I'm not saying you should build a ramstage and include it in CBFS. You should have ifeqs in the Makefile to exclude the ramstage ELF from the dependencies and the files included in CBFS. I'm just saying you should use CONFIG_RAMPAYLOAD to control that, not the toolchain options. Having the toolchain defined doesn't mean you actually need to build anything (and thanks to Makefile dependencies you shouldn't need to touch all the details about how the ramstage is built, just make the final image not depend on it).
[Subrata] I understand your point but i think its looking things differently from one to another, when i have started implementing, i was actually guarding things against RAMPAYLOAD and it will work but then Aaron bring another point that ARCH are nothing but telling what all different soc/vendor toolchain is supporting a stage , rather keeping things dependent on architecture for an example: COMPRESS_RAMSTAGE (shouldn't enable if all atleast 1/10 ARCH_RAMSTAGE is not enable), we should have a definition of stage, which was missing in coreboot today. Only stage definition exist today is for verstage (seperate_verstage).
And if we have stage definitions then only we could pick stages from mainboard or soc.