[coreboot-gerrit] New patch to review for coreboot: vboot: Cleanup vboot code
Furquan Shaikh (furquan@google.com)
gerrit at coreboot.org
Fri Jul 22 18:36:36 CEST 2016
Furquan Shaikh (furquan at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15799
-gerrit
commit 863942ebb95ce1efbd0a90238687f88f9ff778d1
Author: Furquan Shaikh <furquan at google.com>
Date: Fri Jul 22 06:59:40 2016 -0700
vboot: Cleanup vboot code
1. Remove unused functions/structures.
2. Add checks for NULL return values.
3. Change prefixes to vb2 instead of vboot for functions used internally
within vboot2/
4. Get rid of vboot_handoff.h file and move the structure definition to
vboot_common.h
5. Rename all functions using handoff structure to have prefix
vboot_handoff_*. All the handoff functions can be run _only_ after cbmem
is online.
6. Organize vboot_common.h content according to different functionalities.
BUG=chrome-os-partner:55431
Change-Id: I4c07d50327d88cddbdfbb0b6f82c264e2b8620eb
Signed-off-by: Furquan Shaikh <furquan at google.com>
---
src/lib/bootmode.c | 10 +--
src/mainboard/google/rush_ryu/mainboard.c | 2 +-
src/soc/intel/skylake/fsp_reset.c | 4 +-
src/vendorcode/google/chromeos/chromeos.c | 2 -
src/vendorcode/google/chromeos/chromeos.h | 2 -
src/vendorcode/google/chromeos/elog.c | 2 +-
src/vendorcode/google/chromeos/gnvs.c | 2 +-
src/vendorcode/google/chromeos/vboot2/common.c | 18 +++--
src/vendorcode/google/chromeos/vboot2/misc.h | 4 +-
.../google/chromeos/vboot2/vboot_handoff.c | 1 -
.../google/chromeos/vboot2/vboot_loader.c | 6 +-
src/vendorcode/google/chromeos/vboot_common.c | 50 +++++++------
src/vendorcode/google/chromeos/vboot_common.h | 83 +++++++++++++++-------
13 files changed, 106 insertions(+), 80 deletions(-)
diff --git a/src/lib/bootmode.c b/src/lib/bootmode.c
index 15f7a5a..c695026 100644
--- a/src/lib/bootmode.c
+++ b/src/lib/bootmode.c
@@ -23,7 +23,7 @@ int developer_mode_enabled(void)
if (get_developer_mode_switch())
return 1;
#if CONFIG_VBOOT_VERIFY_FIRMWARE
- if (vboot_enable_developer())
+ if (vboot_handoff_check_developer_flag())
return 1;
#endif
return 0;
@@ -36,7 +36,7 @@ int developer_mode_enabled(void)
*
* If shared recovery reason is set:
* - before VbInit then get_recovery_mode_from_vbnv() is true
- * - after VbInit then vboot_enable_recovery() is true
+ * - after VbInit then vboot_handoff_check_recovery_flag() is true
*
* Otherwise the mainboard handler for get_recovery_mode_switch()
* will detect recovery mode initiated by the EC.
@@ -50,7 +50,7 @@ int recovery_mode_enabled(void)
return 1;
#endif
#if CONFIG_VBOOT_VERIFY_FIRMWARE
- if (vboot_enable_recovery())
+ if (vboot_handoff_check_recovery_flag())
return 1;
#endif
return 0;
@@ -75,9 +75,9 @@ void gfx_set_init_done(int done)
int display_init_required(void)
{
- /* For Chrome OS always honor vboot_skip_display_init(). */
+ /* For Chrome OS always honor vboot_handoff_skip_display_init(). */
if (IS_ENABLED(CONFIG_CHROMEOS))
- return !vboot_skip_display_init();
+ return !vboot_handoff_skip_display_init();
/* By default always initialize display. */
return 1;
diff --git a/src/mainboard/google/rush_ryu/mainboard.c b/src/mainboard/google/rush_ryu/mainboard.c
index e0479f3..85034b3 100644
--- a/src/mainboard/google/rush_ryu/mainboard.c
+++ b/src/mainboard/google/rush_ryu/mainboard.c
@@ -35,8 +35,8 @@
#include <vendorcode/google/chromeos/cros_vpd.h>
#if IS_ENABLED(CONFIG_CHROMEOS)
#include <vboot_struct.h>
-#include <vendorcode/google/chromeos/vboot_handoff.h>
#include <vendorcode/google/chromeos/vboot2/misc.h>
+#include <vendorcode/google/chromeos/vboot_common.h>
#endif
#include "gpio.h"
diff --git a/src/soc/intel/skylake/fsp_reset.c b/src/soc/intel/skylake/fsp_reset.c
index 4751872..ca93399 100644
--- a/src/soc/intel/skylake/fsp_reset.c
+++ b/src/soc/intel/skylake/fsp_reset.c
@@ -13,7 +13,7 @@
* GNU General Public License for more details.
*/
#include <bootstate.h>
-#include <vendorcode/google/chromeos/vboot_handoff.h>
+#include <vendorcode/google/chromeos/vboot_common.h>
static int is_recovery; /* flag to identify recovery mode */
@@ -36,7 +36,7 @@ static void set_recovery_request(void *unused)
* & store recovery request into VBNV
*/
if (is_recovery)
- set_recovery_mode_into_vbnv(vboot_recovery_reason());
+ set_recovery_mode_into_vbnv(vboot_handoff_get_recovery_reason());
}
diff --git a/src/vendorcode/google/chromeos/chromeos.c b/src/vendorcode/google/chromeos/chromeos.c
index 926ed39..563f6fd 100644
--- a/src/vendorcode/google/chromeos/chromeos.c
+++ b/src/vendorcode/google/chromeos/chromeos.c
@@ -23,7 +23,6 @@ int __attribute__((weak)) clear_recovery_mode_switch(void)
return 0;
}
-#ifdef __ROMSTAGE__
void __attribute__((weak)) save_chromeos_gpios(void)
{
// Can be implemented by a mainboard
@@ -34,4 +33,3 @@ int __attribute__((weak)) get_sw_write_protect_state(void)
// Can be implemented by a platform / mainboard
return 0;
}
-#endif
diff --git a/src/vendorcode/google/chromeos/chromeos.h b/src/vendorcode/google/chromeos/chromeos.h
index 57a2f71..ad9ef2f 100644
--- a/src/vendorcode/google/chromeos/chromeos.h
+++ b/src/vendorcode/google/chromeos/chromeos.h
@@ -24,9 +24,7 @@
#include "vboot_common.h"
#include "vboot2/misc.h"
-#if ENV_ROMSTAGE
void save_chromeos_gpios(void);
-#endif
#if CONFIG_CHROMEOS
/* functions implemented in elog.c */
diff --git a/src/vendorcode/google/chromeos/elog.c b/src/vendorcode/google/chromeos/elog.c
index bb71da3..710d6f6 100644
--- a/src/vendorcode/google/chromeos/elog.c
+++ b/src/vendorcode/google/chromeos/elog.c
@@ -18,7 +18,7 @@
#include <elog.h>
#include <vendorcode/google/chromeos/chromeos.h>
#if CONFIG_VBOOT_VERIFY_FIRMWARE
-#include "vboot_handoff.h"
+#include "vboot_common.h"
#include <vboot_struct.h>
#endif
diff --git a/src/vendorcode/google/chromeos/gnvs.c b/src/vendorcode/google/chromeos/gnvs.c
index 3443188..48668e8 100644
--- a/src/vendorcode/google/chromeos/gnvs.c
+++ b/src/vendorcode/google/chromeos/gnvs.c
@@ -24,7 +24,7 @@
#include "chromeos.h"
#include "gnvs.h"
#if CONFIG_VBOOT_VERIFY_FIRMWARE
-#include "vboot_handoff.h"
+#include "vboot_common.h"
#include <vboot_struct.h>
#endif
diff --git a/src/vendorcode/google/chromeos/vboot2/common.c b/src/vendorcode/google/chromeos/vboot2/common.c
index 749f328..58ea95d 100644
--- a/src/vendorcode/google/chromeos/vboot2/common.c
+++ b/src/vendorcode/google/chromeos/vboot2/common.c
@@ -13,6 +13,7 @@
* GNU General Public License for more details.
*/
+#include <assert.h>
#include <cbfs.h>
#include <cbmem.h>
#include <console/console.h>
@@ -21,7 +22,7 @@
#include <vb2_api.h>
#include "../chromeos.h"
#include "../symbols.h"
-#include "../vboot_handoff.h"
+#include "../vboot_common.h"
#include "misc.h"
struct selected_region {
@@ -134,19 +135,20 @@ int vb2_get_selected_region(struct region *region)
void vb2_set_selected_region(const struct region *region)
{
struct selected_region *reg = vb2_selected_region();
+
+ assert(reg != NULL);
+
reg->offset = region_offset(region);
reg->size = region_sz(region);
}
-int vboot_is_slot_selected(void)
+int vb2_is_slot_selected(void)
{
const struct selected_region *reg = vb2_selected_region();
- return reg->size > 0;
-}
-int vboot_is_readonly_path(void)
-{
- return !vboot_is_slot_selected();
+ assert(reg != NULL);
+
+ return reg->size > 0;
}
void vb2_store_selected_region(void)
@@ -160,6 +162,8 @@ void vb2_store_selected_region(void)
sel_reg = cbmem_add(CBMEM_ID_VBOOT_SEL_REG, sizeof(*sel_reg));
+ assert(sel_reg != NULL);
+
sel_reg->offset = wd->selected_region.offset;
sel_reg->size = wd->selected_region.size;
}
diff --git a/src/vendorcode/google/chromeos/vboot2/misc.h b/src/vendorcode/google/chromeos/vboot2/misc.h
index fc9d254..ca64b37 100644
--- a/src/vendorcode/google/chromeos/vboot2/misc.h
+++ b/src/vendorcode/google/chromeos/vboot2/misc.h
@@ -29,8 +29,8 @@ struct vb2_shared_data *vb2_get_shared_data(void);
/* Returns 0 on success. < 0 on failure. */
int vb2_get_selected_region(struct region *region);
void vb2_set_selected_region(const struct region *region);
-int vboot_is_slot_selected(void);
-int vboot_is_readonly_path(void);
+int vb2_is_slot_selected(void);
+int vb2_logic_executed(void);
/* Store the selected region in cbmem for later use. */
void vb2_store_selected_region(void);
diff --git a/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c b/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c
index c89e9a0..0e2cb84 100644
--- a/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c
+++ b/src/vendorcode/google/chromeos/vboot2/vboot_handoff.c
@@ -29,7 +29,6 @@
#include <vb2_api.h>
#include <vboot_struct.h>
#include "../chromeos.h"
-#include "../vboot_handoff.h"
#include "misc.h"
/**
diff --git a/src/vendorcode/google/chromeos/vboot2/vboot_loader.c b/src/vendorcode/google/chromeos/vboot2/vboot_loader.c
index f6efe0f..7541518 100644
--- a/src/vendorcode/google/chromeos/vboot2/vboot_loader.c
+++ b/src/vendorcode/google/chromeos/vboot2/vboot_loader.c
@@ -21,7 +21,7 @@
#include <rules.h>
#include <string.h>
#include "misc.h"
-#include "../vboot_handoff.h"
+#include "../vboot_common.h"
#include "../symbols.h"
/* The stage loading code is compiled and entered from multiple stages. The
@@ -61,7 +61,7 @@ static int verstage_should_load(void)
static int vboot_executed CAR_GLOBAL;
-static int vboot_logic_executed(void)
+int vb2_logic_executed(void)
{
/* If this stage is supposed to run the vboot logic ensure it has been
* executed. */
@@ -139,7 +139,7 @@ static int vboot_locate(struct cbfs_props *props)
struct region selected_region;
/* Don't honor vboot results until the vboot logic has run. */
- if (!vboot_logic_executed())
+ if (!vb2_logic_executed())
return -1;
if (vb2_get_selected_region(&selected_region))
diff --git a/src/vendorcode/google/chromeos/vboot_common.c b/src/vendorcode/google/chromeos/vboot_common.c
index 28135a0..451e8ed 100644
--- a/src/vendorcode/google/chromeos/vboot_common.c
+++ b/src/vendorcode/google/chromeos/vboot_common.c
@@ -26,72 +26,69 @@
#include "chromeos.h"
#include "vboot_common.h"
-#include "vboot_handoff.h"
int vboot_named_region_device(const char *name, struct region_device *rdev)
{
return fmap_locate_area_as_rdev(name, rdev);
}
-int vboot_region_device(const struct region *reg, struct region_device *rdev)
-{
- return boot_device_ro_subregion(reg, rdev);
-}
-
+/* ========================== VBOOT HANDOFF APIs =========================== */
int vboot_get_handoff_info(void **addr, uint32_t *size)
{
- struct vboot_handoff *vboot_handoff;
-
- /* No flags are available in a separate verstage or bootblock because
- * cbmem only comes online when dram does. */
- if ((ENV_VERSTAGE && IS_ENABLED(CONFIG_VBOOT_STARTS_IN_BOOTBLOCK)) ||
- ENV_BOOTBLOCK)
+ /*
+ * vboot_handoff is present only after cbmem comes online. If we are in
+ * pre-ram stage, then bail out early.
+ */
+ if (VBOOT_PRE_CBMEM)
return -1;
+ struct vboot_handoff *vboot_handoff;
vboot_handoff = cbmem_find(CBMEM_ID_VBOOT_HANDOFF);
if (vboot_handoff == NULL)
return -1;
*addr = vboot_handoff;
- *size = sizeof(*vboot_handoff);
+
+ if (*size)
+ *size = sizeof(*vboot_handoff);
return 0;
}
-static int vboot_handoff_flag(uint32_t flag)
+static int vboot_get_handoff_flag(uint32_t flag)
{
struct vboot_handoff *vbho;
- uint32_t size;
- if (vboot_get_handoff_info((void **)&vbho, &size))
+ /*
+ * If vboot_handoff cannot be found, return default value of flag as 0.
+ */
+ if (vboot_get_handoff_info((void **)&vbho, NULL))
return 0;
return !!(vbho->init_params.out_flags & flag);
}
-int vboot_skip_display_init(void)
+int vboot_handoff_skip_display_init(void)
{
- return !vboot_handoff_flag(VB_INIT_OUT_ENABLE_DISPLAY);
+ return !vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_DISPLAY);
}
-int vboot_enable_developer(void)
+int vboot_handoff_check_developer_flag(void)
{
- return vboot_handoff_flag(VB_INIT_OUT_ENABLE_DEVELOPER);
+ return vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_DEVELOPER);
}
-int vboot_enable_recovery(void)
+int vboot_handoff_check_recovery_flag(void)
{
- return vboot_handoff_flag(VB_INIT_OUT_ENABLE_RECOVERY);
+ return vboot_get_handoff_flag(VB_INIT_OUT_ENABLE_RECOVERY);
}
-int vboot_recovery_reason(void)
+int vboot_handoff_get_recovery_reason(void)
{
struct vboot_handoff *vbho;
VbSharedDataHeader *sd;
- vbho = cbmem_find(CBMEM_ID_VBOOT_HANDOFF);
-
- if (vbho == NULL)
+ if (vboot_get_handoff_info((void **)&vbho, NULL))
return 0;
sd = (VbSharedDataHeader *)vbho->shared_data;
@@ -99,6 +96,7 @@ int vboot_recovery_reason(void)
return sd->recovery_reason;
}
+/* ============================ VBOOT REBOOT ============================== */
void __attribute__((weak)) vboot_platform_prepare_reboot(void)
{
}
diff --git a/src/vendorcode/google/chromeos/vboot_common.h b/src/vendorcode/google/chromeos/vboot_common.h
index 250b0e5..0125550 100644
--- a/src/vendorcode/google/chromeos/vboot_common.h
+++ b/src/vendorcode/google/chromeos/vboot_common.h
@@ -15,37 +15,66 @@
#ifndef VBOOT_COMMON_H
#define VBOOT_COMMON_H
-#include <stdint.h>
#include <commonlib/region.h>
+#include <stdint.h>
+#include <vboot_api.h>
+#include <vboot_struct.h>
-/* The FW areas consist of multiple components. At the beginning of
- * each area is the number of total compoments as well as the size and
- * offset for each component. One needs to caculate the total size of the
- * signed firmware region based off of the embedded metadata. */
-struct vboot_component_entry {
- uint32_t offset;
- uint32_t size;
-} __attribute__((packed));
+#include "chromeos.h"
-struct vboot_components {
- uint32_t num_components;
- struct vboot_component_entry entries[0];
-} __attribute__((packed));
+#define VBOOT_PRE_CBMEM (ENV_BOOTBLOCK || \
+ (ENV_VERSTAGE && \
+ IS_ENABLED(VBOOT_STARTS_IN_BOOTBLOCK)))
-/* The following functions return 0 on success, < 0 on error. */
+
+/* Locate vboot area by name. Returns 0 on success and -1 on error. */
int vboot_named_region_device(const char *name, struct region_device *rdev);
-int vboot_region_device(const struct region *reg, struct region_device *rdev);
-int vboot_get_handoff_info(void **addr, uint32_t *size);
-/* The following functions return 1 for true and 0 for false. */
-int vboot_skip_display_init(void);
-int vboot_enable_recovery(void);
-int vboot_enable_developer(void);
+/* ========================== VBOOT HANDOFF APIs =========================== */
+/*
+ * The vboot_handoff structure contains the data to be consumed by downstream
+ * firmware after firmware selection has been completed. Namely it provides
+ * vboot shared data as well as the flags from VbInit.
+ */
+struct vboot_handoff {
+ VbInitParams init_params;
+ uint32_t selected_firmware;
+ char shared_data[VB_SHARED_DATA_MIN_SIZE];
+} __attribute__((packed));
-int vboot_recovery_reason(void);
+/*
+ * vboot_get_handoff_info returns pointer to the vboot_handoff structure if
+ * available. vboot_handoff is available only after CBMEM comes online. If size
+ * is not NULL, size of the vboot_handoff structure is returned in it.
+ * Returns 0 on success and -1 on error.
+ */
+int vboot_get_handoff_info(void **addr, uint32_t *size);
+/*
+ * The following functions read vboot_handoff structure to obtain requested
+ * information. If vboot handoff is not available, 0 is returned by default.
+ * If vboot handoff is available:
+ * Returns 1 for flag if true
+ * Returns 0 for flag if false
+ * Returns value read for other fields
+ */
+int vboot_handoff_skip_display_init(void);
+int vboot_handoff_check_recovery_flag(void);
+int vboot_handoff_check_developer_flag(void);
+int vboot_handoff_get_recovery_reason(void);
+
+/* ============================ VBOOT REBOOT ============================== */
+/*
+ * vboot_reboot handles the reboot requests made by vboot_reference library. It
+ * allows the platform to run any preparation steps before the reboot and then
+ * does a hard reset.
+ */
void vboot_reboot(void);
+/* Allow the platform to do any clean up work when vboot requests a reboot. */
+void vboot_platform_prepare_reboot(void);
+
+/* ============================ VBOOT RESUME ============================== */
/*
* Save the provided hash digest to a secure location to check against in
* the resume path. Returns 0 on success, < 0 on error.
@@ -64,13 +93,13 @@ int vboot_retrieve_hash(void *digest, size_t digest_size);
*/
int vboot_platform_is_resuming(void);
-/* Allow the platform to do any clean up work when vboot requests a reboot. */
-void vboot_platform_prepare_reboot(void);
-
-/* Main logic for verified boot. verstage() is the stage entry point
- * while the verstage_main() is just the core logic. */
+/* ============================= VERSTAGE ================================== */
+/*
+ * Main logic for verified boot. verstage() is the stage entry point
+ * while the verstage_main() is just the core logic.
+ */
void verstage_main(void);
-void verstage_mainboard_init(void);
void verstage(void);
+void verstage_mainboard_init(void);
#endif /* VBOOT_COMMON_H */
More information about the coreboot-gerrit
mailing list