Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36914 )
Change subject: binaryPI: implement C bootblock ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... File src/drivers/amd/agesa/bootblock.c:
https://review.coreboot.org/c/coreboot/+/36914/9/src/drivers/amd/agesa/bootb... PS9, Line 58: !boot_cpu() You already branch in assembly to avoid changing %esp. Maybe just have a different C entry point for AP's (without timestamps)?
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... File src/drivers/amd/agesa/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 150: x1b LAPIC_BASE_MSR
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 152: 256 LAPIC_BASE_MSR_BOOTSTRAP_PROCESSOR
https://review.coreboot.org/c/coreboot/+/36914/8/src/drivers/amd/agesa/cache... PS8, Line 159: %esp Where are the AP's stack? You probably want some linker symbol region to assert that that it does not collide with other CAR symbols.