Keith Short has uploaded this change for review.

View Change

security/vboot: Add VBNV flags to save the Cr50 recovery switch state

Add flags to save the Cr50 recovery switch state. This ensures that the
Cr50 recovery switch state is only read during verstage.

BUG=b:123360379
BRANCH=none
TEST=build coreboot on sarien and arcada. Test normal boot and recovery
boot on arcada - confirm that that tpm transaction errors are gone.

Change-Id: Id30a7b203e5aac8631971eb102986427b8362a71
Signed-off-by: Keith Short <keithshort@chromium.org>
---
M src/mainboard/google/sarien/chromeos.c
M src/security/vboot/vbnv.c
M src/security/vboot/vbnv.h
M src/security/vboot/vbnv_layout.h
4 files changed, 71 insertions(+), 20 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/31800/1
diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c
index 1e363fd..308b682 100644
--- a/src/mainboard/google/sarien/chromeos.c
+++ b/src/mainboard/google/sarien/chromeos.c
@@ -20,18 +20,12 @@
#include <variant/gpio.h>
#include <vendorcode/google/chromeos/chromeos.h>
#include <security/tpm/tss.h>
+#include <security/vboot/vbnv.h>
#include <device/device.h>
#include <intelblocks/pmclib.h>
#include <soc/pmc.h>
#include <soc/pci_devs.h>

-enum rec_mode_state {
- REC_MODE_UNINITIALIZED,
- REC_MODE_NOT_REQUESTED,
- REC_MODE_REQUESTED,
-};
-static enum rec_mode_state saved_rec_mode;
-
void fill_lb_gpios(struct lb_gpios *gpios)
{
struct lb_gpio chromeos_gpios[] = {
@@ -84,30 +78,33 @@

int get_recovery_mode_switch(void)
{
- enum rec_mode_state state = saved_rec_mode;
+ int rec_switch;
uint8_t recovery_button_state = 0;

- /* Check the global variable first. */
- if (state == REC_MODE_NOT_REQUESTED)
- return 0;
- else if (state == REC_MODE_REQUESTED)
- return 1;
+ /*
+ * Only verstage performs a real check of the Cr50 recovery switch.
+ * The recovery switch state is cleared on the first access by the AP
+ * so there's no point in querying the Cr50 at later stages. All other
+ * stages use the state saved in VBNV.
+ */
+ if (!ENV_VERSTAGE &&
+ !get_recovery_switch_from_vbnv(&rec_switch))
+ return rec_switch;

- state = REC_MODE_NOT_REQUESTED;
+ rec_switch = 0;

/* Read state from the GPIO controlled by servo. */
if (cros_get_gpio_value(CROS_GPIO_REC))
- state = REC_MODE_REQUESTED;
+ rec_switch = 1;
/* Read one-time recovery request from cr50. */
else if (tlcl_cr50_get_recovery_button(&recovery_button_state)
== TPM_SUCCESS)
- state = recovery_button_state ?
- REC_MODE_REQUESTED : REC_MODE_NOT_REQUESTED;
+ rec_switch = !!recovery_button_state;

/* Store the state in case this is called again in verstage. */
- saved_rec_mode = state;
+ set_recovery_switch_into_vbnv(rec_switch);

- return state == REC_MODE_REQUESTED;
+ return rec_switch;
}

int get_lid_switch(void)
diff --git a/src/security/vboot/vbnv.c b/src/security/vboot/vbnv.c
index 636e5e3..8156fc5 100644
--- a/src/security/vboot/vbnv.c
+++ b/src/security/vboot/vbnv.c
@@ -140,6 +140,42 @@
return vbnv_data(RECOVERY_OFFSET);
}

+/* Save the recovery switch state into VBNV. */
+void set_recovery_switch_into_vbnv(int recovery_switch)
+{
+ uint8_t vbnv_copy[VBOOT_VBNV_BLOCK_SIZE];
+
+ read_vbnv(vbnv_copy);
+
+ vbnv_copy[MISC_FLAGS_OFFSET] |= MISC_FLAGS_RECOVERY_SWITCH_VALID_MASK;
+ if (recovery_switch)
+ vbnv_copy[MISC_FLAGS_OFFSET] |=
+ MISC_FLAGS_RECOVERY_SWITCH_STATE_MASK;
+ else
+ vbnv_copy[MISC_FLAGS_OFFSET] &=
+ ~MISC_FLAGS_RECOVERY_SWITCH_STATE_MASK;
+
+ vbnv_copy[CRC_OFFSET] = crc8_vbnv(vbnv_copy, CRC_OFFSET);
+
+ save_vbnv(vbnv_copy);
+}
+
+/* Read the recovery switch state from VBNV. */
+int get_recovery_switch_from_vbnv(int *recovery_switch)
+{
+ uint8_t misc_flags;
+ vbnv_setup();
+ misc_flags = vbnv_data(MISC_FLAGS_OFFSET);
+
+ if (!(misc_flags & MISC_FLAGS_RECOVERY_SWITCH_VALID_MASK))
+ return -1;
+
+ *recovery_switch =
+ !!(misc_flags & MISC_FLAGS_RECOVERY_SWITCH_STATE_MASK);
+
+ return 0;
+}
+
/* Read the BOOT_OPROM_NEEDED flag from VBNV. */
int vboot_wants_oprom(void)
{
diff --git a/src/security/vboot/vbnv.h b/src/security/vboot/vbnv.h
index c8e689f..367a376 100644
--- a/src/security/vboot/vbnv.h
+++ b/src/security/vboot/vbnv.h
@@ -25,6 +25,22 @@
void regen_vbnv_crc(uint8_t *vbnv_copy);
int get_recovery_mode_from_vbnv(void);
void set_recovery_mode_into_vbnv(int recovery_reason);
+
+/**
+ * Save the recovery switch state into VBNV
+ *
+ * @param recovery_switch Current state of the recovery switch.
+ */
+void set_recovery_switch_into_vbnv(int recovery_switch);
+/**
+ * Get the recovery switch date from VBNV
+ *
+ * @param recovery_switch On success, set to the saved recovery switch state.
+ *
+ * @return 0 on success, !=0 if recovery switch state not saved.
+ */
+int get_recovery_switch_from_vbnv(int *recovery_switch);
+
int vboot_wants_oprom(void);

/* Read the USB Device Controller(UDC) enable flag from VBNV. */
diff --git a/src/security/vboot/vbnv_layout.h b/src/security/vboot/vbnv_layout.h
index a9326e4..322fcf7 100644
--- a/src/security/vboot/vbnv_layout.h
+++ b/src/security/vboot/vbnv_layout.h
@@ -43,7 +43,9 @@
#define DEV_ENABLE_UDC 0x40

#define MISC_FLAGS_OFFSET 8
-#define MISC_FLAGS_BATTERY_CUTOFF_MASK 0x08
+#define MISC_FLAGS_BATTERY_CUTOFF_MASK 0x08
+#define MISC_FLAGS_RECOVERY_SWITCH_VALID_MASK 0x10
+#define MISC_FLAGS_RECOVERY_SWITCH_STATE_MASK 0x20

#define KERNEL_FIELD_OFFSET 11
#define CRC_OFFSET 15

To view, visit change 31800. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id30a7b203e5aac8631971eb102986427b8362a71
Gerrit-Change-Number: 31800
Gerrit-PatchSet: 1
Gerrit-Owner: Keith Short <keithshort@chromium.org>
Gerrit-MessageType: newchange