[coreboot] Runtime config

[bartek.pastudzki 3mdeb.com]

Hello again,

on Fri, 13 Apr 2018 18:35:58, Nico Huber <nico.h at gmx.de> wrote:
> I think our primary focus should be to define a common interface, first,
> for different implementations. There's already coreboot's "option table"
> that is stored in RTC/CMOS RAM. Marcello has started some work to use
> VPD as a backend [1]. There's your implementation; and finally, Felix
> and me discussed a storage format specifically for dependable updates on
> NOR flashes [2].

Thanks for the information, those patches are useful. I agree we should
define a more flexible interface. There is even FIXME about it in <option.h>
header. :) I've seen a solution from VPD based implementation ([1]) and
adapted ours to implement similar interface. Generally, I think it doesn't
need to change much, however:

1. get_option() function gets destination pointer (void*), but no maximum size,
I think we should add size argument to avoid buffer overflow. I assume that
we want to allow reading more than one byte. We could also consider adding
functions that would also expect specific type of value (boolean, integer) and
return suitable type.

2. In [1], I see there is get_option() made as a wrapper to get_option_cmos()
function. Personaly I'd do it so:

in option.h:

#if CONFIG_USE_OPTION_TABLE

enum cb_err get_option(void *dest, size_t max_size, const char *key);

#else

inline enum cb_err get_option(void *dest, size_t max_size, const char *key) {
return CB_OTABLE_DISABLED;
}

#endif

I've changed get_option() prototype. I removed CMOS from the return value, because
I think we should keep them implementation independent. With such a header we
could link module of choice with proper implementation depending on constant
like CONFIG_OPTION_TABLE_BACKEND. It would simplify headers and let us avoid
many substitutions. It would also simplify testing if the feature is on. Testing
many options at the same time would be inconvenient.

On the other hand, having backend functions with different names could make code
searching a little bit easier. And perhaps I miss something important too?


> Personally, I don't care about the space needed. I would wait with the
> implementation until somebody really needs it packed tight.

I didn't make myself clear. I wrote that because, somewhere in the discussion,
fragmentation issue was raised. Keeping small file with static schema would help
avoid that problem.


on Fri, 13 Apr 2018 21:55:15, jwerner at google.com wrote:
> This is a very important part to keep in mind when we are talking UART.
> There has been an attempt inside the Chrome OS team before to make the UART
> runtime-configurable, and it fell flat on its face because of this.

Perhaps we should provide in Kconfig that CBFS implementation is possible.

Best regards
Bartek Pastudzki
3mdeb
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot/attachments/20180417/b49a92af/attachment.html>