Call VbExEcRunningRW to set IN_RW flag
CL:693008 changed check_ac_active so that we ask CR50 to verify EC
is in RO. While this is the right decision, on some platforms ECs
can't reset EC_IN_RW. This causes check_ec_active to set IN_RW
wrongly when EC is in RO after reboot.
This patch replaces VbExTrustEC with VbExEcRunningRW. If RW is
owned it may say it's in RO. Then, the software sync will proceed
and flash RW while the EC is running RW copy.
It also removes redundant checks for VbExTrustEC() when deciding
whether to allow developer mode to be enabled from the INSERT
screen. The INSERT screen can only be reached by manual recovery,
which resets the EC, we don't need to check again before going to
TODEV.
BUG=b:67976359
BRANCH=none
TEST=make runtests
Change-Id: Ide722146ca8683411dd9072a39387aa9531f6cfc
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/740878
(cherry picked from commit e5e03c6d50fd4c4a0cd95b68eeb52f0c8e98bfc6)
Reviewed-on: https://chromium-review.googlesource.com/747001
Reviewed-by: Scott Collyer <scollyer@chromium.org>
Commit-Queue: Aaron Durbin <adurbin@chromium.org>
Tested-by: Aaron Durbin <adurbin@chromium.org>
Trybot-Ready: Aaron Durbin <adurbin@chromium.org>
diff --git a/firmware/lib/ec_sync.c b/firmware/lib/ec_sync.c
index baf3cfc..35fae2d 100644
--- a/firmware/lib/ec_sync.c
+++ b/firmware/lib/ec_sync.c
@@ -202,11 +202,29 @@
* @param ctx Vboot2 context
* @param devidx Which device (EC=0, PD=1)
*/
-static void check_ec_active(struct vb2_context *ctx, int devidx)
+static VbError_t check_ec_active(struct vb2_context *ctx, int devidx)
{
struct vb2_shared_data *sd = vb2_get_sd(ctx);
- if (!VbExTrustEC(devidx))
+ int in_rw = 0;
+ /*
+ * We don't use VbExTrustEC, which checks EC_IN_RW. It is controlled by
+ * cr50 but on some platforms, cr50 can't know when a EC resets. So, we
+ * trust what EC-RW says. If it lies it's in RO, we'll flash RW while
+ * it's in RW.
+ */
+ int rv = VbExEcRunningRW(devidx, &in_rw);
+
+ /* If we couldn't determine where the EC was, reboot to recovery. */
+ if (rv != VBERROR_SUCCESS) {
+ VB2_DEBUG("VbExEcRunningRW() returned %d\n", rv);
+ request_recovery(ctx, VB2_RECOVERY_EC_UNKNOWN_IMAGE);
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+ }
+
+ if (in_rw)
sd->flags |= IN_RW(devidx);
+
+ return VBERROR_SUCCESS;
}
#define RO_RETRIES 2 /* Maximum times to retry flashing RO */
@@ -341,9 +359,10 @@
#endif
/* Set IN_RW flags */
- check_ec_active(ctx, 0);
- if (do_pd_sync)
- check_ec_active(ctx, 1);
+ if (check_ec_active(ctx, 0))
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
+ if (do_pd_sync && check_ec_active(ctx, 1))
+ return VBERROR_EC_REBOOT_TO_RO_REQUIRED;
/* Check if we need to update RW. Failures trigger recovery mode. */
if (check_ec_hash(ctx, 0, VB_SELECT_FIRMWARE_EC_ACTIVE))
diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c
index 1c82666..60483b2 100644
--- a/firmware/lib/vboot_common.c
+++ b/firmware/lib/vboot_common.c
@@ -217,7 +217,12 @@
if (flags & VBSD_BOOT_DEV_SWITCH_ON)
return 1;
- /* If EC is in RW, it implies recovery wasn't manually requested. */
+ /*
+ * If EC is in RW, it implies recovery wasn't manually requested.
+ * On some platforms, EC_IN_RW can't be reset by the EC, thus, this may
+ * return false (=RW). That's ok because if recovery is manual, we will
+ * get the right signal and that's the case we care about.
+ */
if (!VbExTrustEC(0))
return 0;
diff --git a/firmware/lib/vboot_ui.c b/firmware/lib/vboot_ui.c
index c3a3e9e..a99945e 100644
--- a/firmware/lib/vboot_ui.c
+++ b/firmware/lib/vboot_ui.c
@@ -488,13 +488,11 @@
* - we can honor the virtual dev switch
* - not already in dev mode
* - user forced recovery mode
- * - EC isn't pwned
*/
if (key == 0x04 &&
shared->flags & VBSD_HONOR_VIRT_DEV_SWITCH &&
!(shared->flags & VBSD_BOOT_DEV_SWITCH_ON) &&
- (shared->flags & VBSD_BOOT_REC_SWITCH_ON) &&
- VbExTrustEC(0)) {
+ (shared->flags & VBSD_BOOT_REC_SWITCH_ON)) {
if (!(shared->flags &
VBSD_BOOT_REC_SWITCH_VIRTUAL) &&
VbExGetSwitches(
diff --git a/firmware/lib/vboot_ui_menu.c b/firmware/lib/vboot_ui_menu.c
index 82ef422..2e3b17b 100644
--- a/firmware/lib/vboot_ui_menu.c
+++ b/firmware/lib/vboot_ui_menu.c
@@ -1213,14 +1213,12 @@
* - we can honor the virtual dev switch
* - not already in dev mode
* - user forced recovery mode
- * - EC isn't pwned
*/
if (current_menu == VB_MENU_TO_DEV &&
current_menu_idx == 0 &&
shared->flags & VBSD_HONOR_VIRT_DEV_SWITCH &&
!(shared->flags & VBSD_BOOT_DEV_SWITCH_ON) &&
- (shared->flags & VBSD_BOOT_REC_SWITCH_ON) &&
- VbExTrustEC(0)) {
+ (shared->flags & VBSD_BOOT_REC_SWITCH_ON)) {
if (!(shared->flags &
VBSD_BOOT_REC_SWITCH_VIRTUAL) &&
VbExGetSwitches(
diff --git a/tests/ec_sync_tests.c b/tests/ec_sync_tests.c
index 56f3baa..1d5f8c1 100644
--- a/tests/ec_sync_tests.c
+++ b/tests/ec_sync_tests.c
@@ -33,6 +33,7 @@
static GoogleBinaryBlockHeader gbb;
static int mock_in_rw;
+static VbError_t in_rw_retval;
static int protect_retval;
static int ec_ro_protected;
static int ec_rw_protected;
@@ -94,6 +95,7 @@
ec_run_image = 0; /* 0 = RO, 1 = RW */
ec_ro_updated = 0;
ec_rw_updated = 0;
+ in_rw_retval = VBERROR_SUCCESS;
protect_retval = VBERROR_SUCCESS;
update_retval = VBERROR_SUCCESS;
run_retval = VBERROR_SUCCESS;
@@ -142,6 +144,12 @@
return !mock_in_rw;
}
+VbError_t VbExEcRunningRW(int devidx, int *in_rw)
+{
+ *in_rw = mock_in_rw;
+ return in_rw_retval;
+}
+
VbError_t VbExEcProtect(int devidx, enum VbSelectFirmware_t select)
{
if (select == VB_SELECT_FIRMWARE_READONLY)
@@ -159,6 +167,7 @@
VbError_t VbExEcJumpToRW(int devidx)
{
ec_run_image = 1;
+ mock_in_rw = 1;
return run_retval;
}
@@ -237,6 +246,12 @@
static void VbSoftwareSyncTest(void)
{
+ /* AP-RO cases */
+ ResetMocks();
+ in_rw_retval = VBERROR_SIMULATED;
+ test_ssync(VBERROR_EC_REBOOT_TO_RO_REQUIRED,
+ VBNV_RECOVERY_EC_UNKNOWN_IMAGE, "Unknown EC image");
+
/* Calculate hashes */
ResetMocks();
mock_ec_rw_hash_size = 0;
