Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33759 )
Change subject: soc/amd/picasso: Support reset vector in romstage ......................................................................
Patch Set 17:
(5 comments)
https://review.coreboot.org/c/coreboot/+/33759/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33759/13//COMMIT_MSG@22 PS13, Line 22: Remove all postcar support.
At first appearance >50% of code line changes is about MTRRs and not about the entry to romstage l […]
Ack
https://review.coreboot.org/c/coreboot/+/33759/16/src/soc/amd/picasso/includ... File src/soc/amd/picasso/include/soc/romstage.h:
https://review.coreboot.org/c/coreboot/+/33759/16/src/soc/amd/picasso/includ... PS16, Line 21: #include <arch/cpu.h>
Is this needed here for the function definitions?
Done. Looks like PS12 used asmlinkage but it's no longer necessary.
https://review.coreboot.org/c/coreboot/+/33759/16/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/33759/16/src/soc/amd/picasso/romsta... PS16, Line 53: : static void romstage_soc_init(int s3_resume) : { : fch_early_init(); : }
Call it directly? Is the intention to make the first_stage_entry() an architectural default with cal […]
Redone
https://review.coreboot.org/c/coreboot/+/33759/16/src/soc/amd/picasso/romsta... PS16, Line 100: BIT(23)
8 * MiB would be more readable
Done. It was primarily to reflect that only 47:23 are valid, and not a relevant size. 22:0 are supposed to be RAZ, but the align_down is simply to be safe about it.
https://review.coreboot.org/c/coreboot/+/33759/16/src/soc/amd/picasso/romsta... PS16, Line 101: backup_top_of_low_cacheable(top_of_mem);
Is using this API really necessary if it's fetched from just one MSR? Note CB:36144 where you only n […]
This is only a temporary solution to demonstrate a somewhat simple romstage implementation. The real one relies on AGESA's reporting (due to UMA memory may fall below TOM), and is implemented in CB:34423.