Hello Martin Roth, Marc Jones, Johnny Lin, Angel Pons,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/45774
to review the following change.
Change subject: [RFC] Introduce vpd_get_option() ......................................................................
[RFC] Introduce vpd_get_option()
Allows to read options from VPD regions in flash. This is currently very limited as type information is missing both in the get_option() interface and the backend. Hence, every option to be queried needs to be added manually to vpd_get_option().
Storing options in VPD should only be an interim solution, VPD doesn't seem to be meant to be used like that. The lack of typing also makes this a bit ugly. But this way, we can keep that local.
Change-Id: I825388f767d1d87f44923eff5ae6680105fa801e Signed-off-by: Nico Huber nico.h@gmx.de --- M src/Kconfig M src/drivers/vpd/vpd.c M src/drivers/vpd/vpd.h M src/include/option.h 4 files changed, 58 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/45774/1
diff --git a/src/Kconfig b/src/Kconfig index 40310bb..40e275b 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -135,6 +135,46 @@ every boot. Use this if you want the NVRAM configuration to never be modified from its default values.
+config USE_VPD_OPTIONS + bool "Use VPD for configuration values" + depends on VPD + help + Enable this option if coreboot shall read options from the VPD + regions in flash. + This is currently very limited as type information is missing + both in the get_option() interface and the backend. Hence, every + option to be queried needs to be added manually to vpd_get_option(). + +if USE_VPD_OPTIONS + +choice + prompt "VPD region to query" + default VPD_OPTION_REGION_RW_THEN_RO + +config VPD_OPTION_REGION_RO + bool "RO" + +config VPD_OPTION_REGION_RW + bool "RW" + +config VPD_OPTION_REGION_RO_THEN_RW + bool "RO, then RW" + +config VPD_OPTION_REGION_RW_THEN_RO + bool "RW, then RO" + +endchoice + +# Keep in sync with `drivers/vpd/vpd.h`. +config VPD_OPTION_REGION + int + default 3 if VPD_OPTION_REGION_RW_THEN_RO + default 2 if VPD_OPTION_REGION_RO_THEN_RW + default 1 if VPD_OPTION_REGION_RW + default 0 + +endif # USE_VPD_OPTIONS + config USE_MAINBOARD_OPTIONS bool help diff --git a/src/drivers/vpd/vpd.c b/src/drivers/vpd/vpd.c index d3ff370..727a616 100644 --- a/src/drivers/vpd/vpd.c +++ b/src/drivers/vpd/vpd.c @@ -5,6 +5,7 @@ #include <cbmem.h> #include <ctype.h> #include <fmap.h> +#include <option.h> #include <program_loading.h> #include <string.h> #include <timestamp.h> @@ -297,4 +298,17 @@ return true; }
+enum cb_err vpd_get_option(void *const dest, const char *const name) +{ + if (strcmp(name, "debug_level") == 0) { + int i; + if (vpd_get_int("coreboot_log_level", CONFIG_VPD_OPTION_REGION, &i)) { + *(char *)dest = (char)i; + return CB_SUCCESS; + } + } + + return CB_CMOS_OPTION_NOT_FOUND; +} + ROMSTAGE_CBMEM_INIT_HOOK(cbmem_add_cros_vpd) diff --git a/src/drivers/vpd/vpd.h b/src/drivers/vpd/vpd.h index 817867a..5c2a80f 100644 --- a/src/drivers/vpd/vpd.h +++ b/src/drivers/vpd/vpd.h @@ -7,6 +7,7 @@
#define GOOGLE_VPD_2_0_OFFSET 0x600
+/* Keep in sync with Kconfig VPD_OPTION_REGION. */ enum vpd_region { VPD_RO, VPD_RW, diff --git a/src/include/option.h b/src/include/option.h index 86c8ff1..2822bcb 100644 --- a/src/include/option.h +++ b/src/include/option.h @@ -11,6 +11,7 @@ enum cb_err cmos_get_option(void *dest, const char *name);
enum cb_err mainboard_get_option(void *dest, const char *name); +enum cb_err vpd_get_option(void *dest, const char *name);
static inline enum cb_err set_option(const char *name, void *val) { @@ -26,6 +27,8 @@
if (CONFIG(USE_OPTION_TABLE)) ret = cmos_get_option(dest, name); + if (CONFIG(USE_VPD_OPTIONS) && ret != CB_SUCCESS) + ret = vpd_get_option(dest, name); if (CONFIG(USE_MAINBOARD_OPTIONS) && ret != CB_SUCCESS) ret = mainboard_get_option(dest, name);