Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34929 )
Change subject: arch/x86: Fix spinlocks for cases of __PRE_RAM__ ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34929/4/src/console/printk.c File src/console/printk.c:
https://review.coreboot.org/c/coreboot/+/34929/4/src/console/printk.c@29 PS4, Line 29: if (ENV_STAGE_HAS_DATA_SECTION) : spin_lock(&console_lock); : else if (ENV_ROMSTAGE && CONFIG(HAVE_ROMSTAGE_CONSOLE_SPINLOCK)) : spin_lock(romstage_console_lock());
Just curious: You are not using a helper function here to get a pointer to the spinlock because the […]
Would you rephrase the comment (or was there a question)?
Use of the helper function (present in original work already) was forced by:
1. CAR_GLOBAL_MIGRATION=y, romstage needs to take the form spin_lock(car_get_var_ptr(&console_lock)) instead. 2. Lack of .data means DECLARE_SPIN_LOCK() does not work for stages in CAR. Those spinlock variables are explicitly defined as CAR_GLOBALs and unlocked in mainboard/ romstage.c files (yuck).
The two boards (asus/kcma-d8,kgpe-d16) with any 'select HAVE_ROMSTAGE_xxx_SPINLOCK' will hit 3 of 3 deprecation criteria on next release. Can we just drop current implementation in all its uglyness, together with these Kconfigs, and have a more generic solution?
https://review.coreboot.org/c/coreboot/+/34929/4/src/cpu/amd/pi/00730F01/mic... File src/cpu/amd/pi/00730F01/microcode_fam16h.c:
PS4: Added in CB:29792 but since CB:29793 was abandoned we only need this file built for ramstage. And the platform in question does not declare romstage spinlocks.