Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33759 )
Change subject: soc/amd/picasso: Create a hybrid romstage to begin in DRAM ......................................................................
Patch Set 9:
(1 comment)
This looks exactly like bootblock c env but for romstage?
Good observation. Yes, but with cleanup of comments and items that don't necessarily apply to AMD. However, I tried experiment of replacing 100% of reset_vector.S, up to the cld instruction, with prologue.inc, entry16.inc, reset16.inc, entry32.inc. I needed to make the following changes for this to work: * Exclude gdt_init.S from arch//Makefile.inc, as it's included in entry32.inc instead * Remove _start from entry32.inc (i.e. extend the #if !ENV_BOOTBLOCK line) * Rewrote my romstage.ld to relocate _ROMTOP similar to contents of reset16.ld. Need a clean way to replace the ASSERT. Brought in the calculation helpers like gdtptr16_offset from entry16.ld. * Added type==R_386_16 to the eligible relocatable types in rmodule.c. nullidt_offset and gdtptr16_offset were preventing romstage from getting added to cbfs otherwise.
I sort of like this approach better, i.e. of using the 4 .inc files instead of rewriting functionality. It means needing to touch more non-Picasso code, however -- I'm happy to make the changes if it gets us moving again.
https://review.coreboot.org/c/coreboot/+/33759/9/src/soc/amd/picasso/reset_v... File src/soc/amd/picasso/reset_vector.S:
https://review.coreboot.org/c/coreboot/+/33759/9/src/soc/amd/picasso/reset_v... PS9, Line 111: .byte 0x00, 0x9b, 0xaf, 0x00
We're duplicating the descriptor tables?
This was to ensure the x86 can "reach" the descriptor tables immediately after reset, but allows a romstage that is larger than 64K, i.e. as the code sits the one in gdt_init.S could get linked too far away to be addressable right out of reset.
-
Marshall Dawson (Code Review)