From 0bdec67fc7778ee55b7dbdacece2d81022fac1b3 Mon Sep 17 00:00:00 2001 From: Duncan Laurie Date: Thu, 8 Mar 2018 19:23:01 -0800 Subject: [PATCH] ec_sync: Go to recovery on aux fw update failure If an aux firmware update fails enter recovery with a specific reason code so we can identify systems that fail. Also handle the case where the update succeeds and requests a cold reset of the EC, first clearing the oprom flag if necessary in order to prevent a second reset. Unit test was added to check recovery reason for aux firmware update failure. BUG=b:74336712 BRANCH=eve TEST=manual: force update to fail and ensure it goes to recovery mode, and after successful update check that the option rom flag is cleared before the EC reset happens. Unit tests udpated and 'make runtests' passes. Change-Id: I35a93892a0f8bb16eac0925ada5dfbc5c3144f8d Signed-off-by: Duncan Laurie Reviewed-on: https://chromium-review.googlesource.com/959671 Reviewed-by: Caveh Jalali Reviewed-by: Furquan Shaikh --- firmware/lib/ec_sync.c | 14 ++++++++++ firmware/lib/ec_sync_all.c | 51 ++++++++++++++++++++++------------ firmware/lib/include/ec_sync.h | 8 ++++++ tests/ec_sync_tests.c | 9 +++++- 4 files changed, 63 insertions(+), 19 deletions(-) diff --git a/firmware/lib/ec_sync.c b/firmware/lib/ec_sync.c index c19e5d203b..3c70ae1524 100644 --- a/firmware/lib/ec_sync.c +++ b/firmware/lib/ec_sync.c @@ -438,6 +438,20 @@ VbError_t ec_sync_check_aux_fw(struct vb2_context *ctx, return VbExCheckAuxFw(severity); } +VbError_t ec_sync_update_aux_fw(struct vb2_context *ctx) +{ + VbError_t rv = VbExUpdateAuxFw(); + if (rv) { + if (rv == VBERROR_EC_REBOOT_TO_RO_REQUIRED) { + VB2_DEBUG("AUX firmware update requires RO reboot.\n"); + } else { + VB2_DEBUG("AUX firmware update/protect failed.\n"); + request_recovery(ctx, VB2_RECOVERY_AUX_FW_UPDATE); + } + } + return rv; +} + VbError_t ec_sync_phase2(struct vb2_context *ctx) { if (!ec_sync_allowed(ctx)) diff --git a/firmware/lib/ec_sync_all.c b/firmware/lib/ec_sync_all.c index 68e0a9af75..36d9e72757 100644 --- a/firmware/lib/ec_sync_all.c +++ b/firmware/lib/ec_sync_all.c @@ -17,6 +17,28 @@ #include "vboot_display.h" #include "vboot_kernel.h" +static VbError_t ec_sync_unload_oprom(struct vb2_context *ctx, + VbSharedDataHeader *shared, + int need_wait_screen) +{ + /* + * Reboot to unload VGA Option ROM if: + * - we displayed the wait screen + * - the system has slow EC update flag set + * - the VGA Option ROM was needed and loaded + * - the system is NOT in developer mode (that'll also need the ROM) + */ + if (need_wait_screen && + (shared->flags & VBSD_OPROM_MATTERS) && + (shared->flags & VBSD_OPROM_LOADED) && + !(shared->flags & VBSD_BOOT_DEV_SWITCH_ON)) { + VB2_DEBUG("Reboot to unload VGA Option ROM\n"); + vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 0); + return VBERROR_VGA_OPROM_MISMATCH; + } + return VBERROR_SUCCESS; +} + VbError_t ec_sync_all(struct vb2_context *ctx) { struct vb2_shared_data *sd = vb2_get_sd(ctx); @@ -66,28 +88,21 @@ VbError_t ec_sync_all(struct vb2_context *ctx) return rv; /* - * Do software sync for devices tunneled through the EC. + * Do Aux FW software sync and protect devices tunneled through the EC. + * Aux FW update may request RO reboot to force EC cold reset so also + * unload the option ROM if needed to prevent a second reboot. */ - rv = VbExUpdateAuxFw(); + rv = ec_sync_update_aux_fw(ctx); + if (rv) { + ec_sync_unload_oprom(ctx, shared, need_wait_screen); + return rv; + } + + /* Reboot to unload VGA Option ROM if needed */ + rv = ec_sync_unload_oprom(ctx, shared, need_wait_screen); if (rv) return rv; - /* - * Reboot to unload VGA Option ROM if: - * - we displayed the wait screen - * - the system has slow EC update flag set - * - the VGA Option ROM was needed and loaded - * - the system is NOT in developer mode (that'll also need the ROM) - */ - if (need_wait_screen && - (shared->flags & VBSD_OPROM_MATTERS) && - (shared->flags & VBSD_OPROM_LOADED) && - !(shared->flags & VBSD_BOOT_DEV_SWITCH_ON)) { - VB2_DEBUG("Reboot to unload VGA Option ROM\n"); - vb2_nv_set(ctx, VB2_NV_OPROM_NEEDED, 0); - return VBERROR_VGA_OPROM_MISMATCH; - } - /* Phase 3; Completes sync and handles battery cutoff */ rv = ec_sync_phase3(ctx); if (rv) diff --git a/firmware/lib/include/ec_sync.h b/firmware/lib/include/ec_sync.h index 573c94cbf3..dfb8ec9cc4 100644 --- a/firmware/lib/include/ec_sync.h +++ b/firmware/lib/include/ec_sync.h @@ -48,6 +48,14 @@ int ec_will_update_slowly(struct vb2_context *ctx); VbError_t ec_sync_check_aux_fw(struct vb2_context *ctx, VbAuxFwUpdateSeverity_t *severity); +/** + * Update and protect auxiliary firmware. + * + * @param ctx Vboot2 context + * @return VBERROR_SUCCESS or non-zero error code. + */ +VbError_t ec_sync_update_aux_fw(struct vb2_context *ctx); + /** * EC sync, phase 2 * diff --git a/tests/ec_sync_tests.c b/tests/ec_sync_tests.c index eceae78fa4..4c1775f2a9 100644 --- a/tests/ec_sync_tests.c +++ b/tests/ec_sync_tests.c @@ -56,6 +56,7 @@ static struct vb2_shared_data *sd; static uint32_t screens_displayed[8]; static uint32_t screens_count = 0; +static VbError_t ec_aux_fw_retval; static int ec_aux_fw_update_req; static VbAuxFwUpdateSeverity_t ec_aux_fw_mock_severity; static VbAuxFwUpdateSeverity_t ec_aux_fw_update_severity; @@ -109,6 +110,7 @@ static void ResetMocks(void) memset(screens_displayed, 0, sizeof(screens_displayed)); screens_count = 0; + ec_aux_fw_retval = VBERROR_SUCCESS; ec_aux_fw_mock_severity = VB_AUX_FW_NO_UPDATE; ec_aux_fw_update_severity = VB_AUX_FW_NO_UPDATE; ec_aux_fw_update_req = 0; @@ -219,7 +221,7 @@ VbError_t VbExUpdateAuxFw() { ec_aux_fw_update_req = ec_aux_fw_update_severity != VB_AUX_FW_NO_UPDATE; ec_aux_fw_protected = 1; - return VBERROR_SUCCESS; + return ec_aux_fw_retval; } static void test_ssync(VbError_t retval, int recovery_reason, const char *desc) @@ -428,6 +430,11 @@ static void VbSoftwareSyncTest(void) TEST_EQ(ec_aux_fw_protected, 1, " aux fw protected"); TEST_EQ(screens_displayed[0], VB_SCREEN_WAIT, " wait screen forced"); + + ResetMocks(); + ec_aux_fw_retval = VBERROR_UNKNOWN; + test_ssync(VBERROR_UNKNOWN, VB2_RECOVERY_AUX_FW_UPDATE, + "Error updating AUX firmware"); } int main(void)