[coreboot] The fallacy of the CONFIG_* plague

mrnuke mr.nuke.me at gmail.com
Wed Feb 12 19:30:11 CET 2014


=== START_TLDR /* Skip to END_TLDR if you're short on time */ ===

It seems that we're using "#if CONFIG_" like free coffee and cookies in a 
conference. Without thinking too much of it. And we're using very much of this 
in generic code, which is, as I will point out why shortly, a very bad idea.

It wouldn't matter as much if we only used CONFIGs to control the build 
process, or some mundane board-specific options. One problem is, that we've 
started using them excessively in generic code. Generic code should be just 
that, generic. If we need it to behave one way or another, we should do so by 
the vales we pass it not by board-specific Kconfigs.

Take lib/ for example. We should be able to compile, the entire directory into 
a set of static libraries:
* libcorebootlib_rom.i386.a
* libcorebootlib_ram.i386.a
* libcorebootlib_bootblock.armv7.a
* libcorebootlib_rom.armv7.a
* libcorebootlib_ram.armv7.a
And use those libraries as-are, in every single board and component, without 
their behavior depending on Kconfigs. I am allowing for stage-specific 
versioning here due to the joys early init may throw at us.

The same reasoning applies to chip-generic code -- basically most things _not_ 
under src/mainboard/.

Case in point: http://review.coreboot.org/#/c/5199/
The intent is to use CONFIG_, not for the purpose of solving a practical 
problem, but just for the sake of fixing a build failure. It feels like a "if 
it compiles, let's ship it" mindset.

Who cares, really? There is a very practical aspect to lib-izing generic code, 
and that is the abuild step. We can speed that up tremendously by being smart. 
ccache helps a bit, but only so much.
Another practical aspect is eliminating confusion relating to "what does the 
code do now, whith _this_ option instead of _that_ option?".

=== END_TLDR ===

So, instead of having a generic function doing
 void function(void)
 {
 #if IS_ENABLED(CONFIG_HAVE_LOLLIPOP)
	printk(BIOS_YUMMY, "Sucking on a lollipop\n");
 #else
	printk(BIOS_SUCKY, "Meanie!!\n");
 #endif
 }

We make it dynamically polymorphic:
 void function(bool has_lollipop)
 {
 	if (has_lollipop)
		printk(BIOS_YUMMY, "Sucking on a lollipop\n");
 	else
		printk(BIOS_INFO, "Mister stole my lollipop\n");
 }

Let the mainboard pass the lollipop option instead of Kconfig-izing it. The 
only place where code flow/values should depend on anything CONFIG_* should be 
in (tentative list warning):
* board-specific code
* enabling debug options not used in production code (irrelevant for abuild)
* linker scripts
* very early init (assembly part of the bootblock)

And of course, we use the linker to garbage-collect unused functions for the 
sake of space.

Q.E.D.

Thoughts, comments, concerns, paint ideas?

Alex








More information about the coreboot mailing list