[coreboot-gerrit] Change in coreboot[master]: amd/pi: Add verstage support

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/18439 )

Change subject: amd/pi: Add verstage support
......................................................................


Patch Set 13:

(4 comments)

https://review.coreboot.org/#/c/18439/13/src/cpu/amd/pi/car.h
File src/cpu/amd/pi/car.h:

Line 21: void romstage_after_verstage(void);
> asmlinkage
Done


https://review.coreboot.org/#/c/18439/7/src/cpu/amd/pi/verstage.c
File src/cpu/amd/pi/verstage.c:

Line 19: void cache_as_ram_stage_main(void)
> > Same lack of CONFIG_C_ENVIRONMENT_BOOTBLOCK=y response as before.
We intend to rework the tree to be able to support C_ENVIRONMENT_BOOTBLOCK as part of an overhaul to upgrade to the modern way of doing things.

IIRC the CAR is coherent.  Each core gets its own stack area.  Perhaps we've been lucky so far because there only two cores in any of these SKUs (i.e. cores can't run these same code paths simultaneously).

At the moment and with current constraints, I'm not certain how to keep the AP from running through verstage.  Possibly duplicate bootblock_simple.c and change this so the AP always goes to romstage...
 #if CONFIG_VBOOT_SEPARATE_VERSTAGE
	 const char *target1 = "fallback/verstage";
 #else
 	 const char *target1 = "fallback/romstage";
 #endif

I recall you mentioning you'd changed Intel functionality to queue up an SMI while in wait-for-SIPI.  Is that part of how you got around this?


https://review.coreboot.org/#/c/18439/13/src/mainboard/amd/gardenia/Makefile.inc
File src/mainboard/amd/gardenia/Makefile.inc:

Line 16: verstage-y += early_mainboard.c
> Where is early_mainboard.c?
Done.  OBE after abandoning a different patch.


https://review.coreboot.org/#/c/18439/13/src/vendorcode/amd/pi/Makefile.inc
File src/vendorcode/amd/pi/Makefile.inc:

Line 143: verstage-libs += $(obj)/agesa/libagesa.a
> why is libagesa needed in verstage?
cpu//fixme.c amd_initmmio() still has calls to LibAmd*.


-- 
To view, visit https://review.coreboot.org/18439
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie43d87908c2d83b42b95b306419156e85993f7bd
Gerrit-PatchSet: 13
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd at gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes