Bug #668
openmb/asus/p8x7x-series abuild broken
0%
Description
abuild for this family of boards breaks if CONFIG_BOOTBLOCK_CONSOLE is disabled.
The error is:
/usr/src/coreboot/util/crossgcc/xgcc/bin/i386-elf-ld.bfd: coreboot-builds/ASUS_P8Z77_V_LE_PLUS/bootblock/superio/nuvoton/common/early_serial.o: in function `nuvoton_enable_serial':
/usr/src/coreboot/src/superio/nuvoton/common/early_serial.c:17:(.text.nuvoton_enable_serial+0xf): undefined reference to `console_serial_enabled'
It looks like a link time error, because console/serial_console.c is not being compiled into bootblock in this case. This unit is a new addition of commit 605cba2fd137 ("console: allow enabling serial console at runtime").
To reproduce, from root of coreboot source tree:
- Create a text file
nobootconsolewith this content:
CONFIG_BOOTBLOCK_CONSOLE=n
- Run
util/abuild/abuild -p none -c 8 -t asus/p8x7x-series -S -K nobootconsole
This error isn't immediately obvious because CONFIG_BOOTBLOCK_CONSOLE defaults to enabled (for good reasons), but it is a user Kconfig option so none can guarantee it's always enabled.
Whether CONFIG_SEPARATE_ROMSTAGE is chosen or not does not seem to matter. Build still fails.
I should check with Matt before I start working around this by deferring nuvoton_enable_serial() to romstage at mainboard level, as my workaround plan can be tricky as I need to find a romstage hook before console init to make this call in.
Ideally, if a board has serial console support and bootblock console is disabled, no serial and console init should happen in bootblock and should instead happen early in romstage.
MD Updated by Matt DeVillier 20 days ago
must have missed that use case when implementing the runtime serial option, https://review.coreboot.org/c/coreboot/+/96589 should resolve it
PR Updated by Patrick Rudolph 20 days ago
The Super I/O code should not use console_serial_enabled() at all. Quote "Whether the serial console should be used this stage.".
It should use "CONFIG(CONSOLE_SERIAL) || CONFIG(CONSOLE_SERIAL_RUNTIME)" since the call to nuvoton_enable_serial() is missing in other stages. (Ideally this would be weak and called from console_init code, then console_serial_enabled() could be used).
KH Updated by Keith Hui 20 days ago
Patrick Rudolph wrote in #note-2:
The Super I/O code should not use
console_serial_enabled()at all. Quote "Whether the serial console should be used this stage.".It should use "CONFIG(CONSOLE_SERIAL) || CONFIG(CONSOLE_SERIAL_RUNTIME)" since the call to
nuvoton_enable_serial()is missing in other stages. (Ideally this would be weak and called from console_init code, thenconsole_serial_enabled()could be used).
As I said before, ideally nuvoton_enable_serial() should act just before serial console is needed. But I took a quick look at the romstage code flow, and console_init() is the very first thing done without a hook for platform or mainboard. There is also the assumption of:
/* Assumes the hardware was set up during the bootblock */
console_init();
(arch/x86/romstage.c:13)
So to minimize breakage nuvoton_enable_serial() (or its counterparts of other SIO chips) should run unconditionally during bootblock. That is remove the condition from superio/nuvoton/common/early_serial.c:17:
if (!console_serial_enabled())
return;
Another way is to connect nuvoton_enable_serial() et al to console_init() directly with a new hook, which would be the perfect ideal solution. Here I assume console_init() is called at the first stage with enabled console.
Having said this, CB:96589 did fix the build. So how should we go forward?