From 678bb4526ef19ef9458910d0231722b0de4c5ddf Mon Sep 17 00:00:00 2001 From: Mary Ruthven Date: Tue, 5 Mar 2019 19:30:27 -0800 Subject: cr50: use board_wipe_tpm to clear the tpm We were clearing the tpm in two different ways. There was one implementation in factory_mode.c and one in wp.c. This change merges the two, so there's only one board_wipe_tpm. While modifying the wipe tpm code from factory_mode.c I noticed the factory_enable_failed stuff is maybe a bit more complicated than necessary. I opened a bug for cleaning that up(b/129956462). It wont be addressed in this change. BUG=none BRANCH=none TEST=Run the processes that wipe the tpm open ccd. enable factory mode from vendor command. run rma open process Change-Id: Ia76df19f7d9e4f308f3f1a7175f130f1ef7249a2 Signed-off-by: Mary Ruthven Reviewed-on: https://chromium-review.googlesource.com/1535156 Reviewed-by: Vadim Bendebury --- board/cr50/board.c | 2 +- board/cr50/board.h | 3 ++- board/cr50/wp.c | 45 +++++++++++++++++++++++++++++---------------- common/ccd_config.c | 2 +- common/factory_mode.c | 37 ++++--------------------------------- include/flash_log.h | 1 + 6 files changed, 38 insertions(+), 52 deletions(-) diff --git a/board/cr50/board.c b/board/cr50/board.c index e7482a9577..5de77ddf35 100644 --- a/board/cr50/board.c +++ b/board/cr50/board.c @@ -957,7 +957,7 @@ void board_reboot_ap(void) /** * Reboot the EC */ -static void board_reboot_ec(void) +void board_reboot_ec(void) { if (board_uses_closed_loop_reset()) { board_closed_loop_reset(); diff --git a/board/cr50/board.h b/board/cr50/board.h index f6f2f5c9de..016cbc4607 100644 --- a/board/cr50/board.h +++ b/board/cr50/board.h @@ -341,8 +341,9 @@ int board_battery_is_present(void); int board_fwmp_allows_unlock(void); int board_vboot_dev_mode_enabled(void); void board_reboot_ap(void); +void board_reboot_ec(void); void board_closed_loop_reset(void); -int board_wipe_tpm(void); +int board_wipe_tpm(int reset_required); int board_is_first_factory_boot(void); int usb_i2c_board_enable(void); diff --git a/board/cr50/wp.c b/board/cr50/wp.c index b3d50e4dad..0e2a4020af 100644 --- a/board/cr50/wp.c +++ b/board/cr50/wp.c @@ -7,6 +7,7 @@ #include "console.h" #include "crc8.h" #include "extension.h" +#include "flash_log.h" #include "gpio.h" #include "hooks.h" #include "registers.h" @@ -303,23 +304,18 @@ void init_wp_state(void) /** * Wipe the TPM * + * @param reset_required: reset the system after wiping the TPM. + * * @return EC_SUCCESS, or non-zero if error. */ -int board_wipe_tpm(void) +int board_wipe_tpm(int reset_required) { int rc; - /* - * Blindly zapping the TPM space while the AP is awake and poking at - * it will bork the TPM task and the AP itself, so force the whole - * system off by holding the EC in reset. - */ - CPRINTS("%s: force EC off", __func__); - assert_ec_rst(); - /* Wipe the TPM's memory and reset the TPM task. */ rc = tpm_reset_request(1, 1); if (rc != EC_SUCCESS) { + flash_log_add_event(FE_LOG_TPM_WIPE_ERROR, 0, NULL); /* * If anything goes wrong (which is unlikely), we REALLY don't * want to unlock the console. It's possible to fail without @@ -332,22 +328,39 @@ int board_wipe_tpm(void) SYSTEM_RESET_HARD); /* - * That should never return, but if it did, release EC reset - * and pass through the error we got. + * That should never return, but if it did, reset the EC and + * through the error we got. */ - deassert_ec_rst(); + board_reboot_ec(); return rc; } + /* + * TPM was wiped out successfully, let's prevent further communications + * from the AP until next reboot. The reboot will be triggered below if + * a reset is requested. If we aren't resetting the system now, the TPM + * will stay disabled until the user resets the system. + * This should be done as soon as possible after tpm_reset_request + * completes. + */ + tpm_stop(); + CPRINTS("TPM is erased"); /* Tell the TPM task to re-enable NvMem commits. */ tpm_reinstate_nvmem_commits(); - /* Let the rest of the system boot. */ - CPRINTS("%s: release EC reset", __func__); - deassert_ec_rst(); - + /* + * Use board_reboot_ec to ensure the system resets instead of + * deassert_ec_reset. Some boards don't reset immediately when EC_RST_L + * is asserted. board_reboot_ec will ensure the system has actually + * reset before releasing it. If the system has a normal reset scheme, + * EC reset will be released immediately. + */ + if (reset_required) { + CPRINTS("%s: reset EC", __func__); + board_reboot_ec(); + } return EC_SUCCESS; } diff --git a/common/ccd_config.c b/common/ccd_config.c index 62f97892f4..5b5eb87876 100644 --- a/common/ccd_config.c +++ b/common/ccd_config.c @@ -569,7 +569,7 @@ static void ccd_open_done(int sync) if (sync) rv = tpm_sync_reset(1); else - rv = board_wipe_tpm(); + rv = board_wipe_tpm(1); if (rv != EC_SUCCESS) { CPRINTS("CCD open TPM wipe failed"); diff --git a/common/factory_mode.c b/common/factory_mode.c index 1555fc6833..f2ed77cae6 100644 --- a/common/factory_mode.c +++ b/common/factory_mode.c @@ -39,10 +39,8 @@ static void factory_enable_failed(void) ccd_hook_active = 0; CPRINTS("factory enable failed"); - if (reset_required_) { + if (reset_required_) reset_required_ = 0; - deassert_ec_rst(); - } } DECLARE_DEFERRED(factory_enable_failed); @@ -65,34 +63,8 @@ static void factory_enable_deferred(void) { int rv; - CPRINTS("%s: reset TPM\n", __func__); - if (reset_required_) - assert_ec_rst(); - - if (tpm_reset_request(1, 1) != EC_SUCCESS) { - CPRINTS("%s: TPM reset failed\n", __func__); - /* - * Attempt to reset TPM failed, let's reboot the device just - * in case. - */ - if (!reset_required_) - assert_ec_rst(); - deassert_ec_rst(); + if (board_wipe_tpm(reset_required_) != EC_SUCCESS) return; - } - - /* - * TPM was wiped out successfully, let's prevent further - * communications from the AP until next reboot. - */ - if (!reset_required_) - tpm_stop(); - - /* - * Need this to make sure that CCD state changes are saved in the - * NVMEM before reboot. - */ - tpm_reinstate_nvmem_commits(); CPRINTS("%s: TPM reset done, enabling factory mode", __func__); @@ -103,9 +75,8 @@ static void factory_enable_deferred(void) if (reset_required_) { /* - * Make sure we never end up with the EC held in reset, no - * matter what prevents the proper factory reset flow from - * succeeding. + * Cr50 will reset once factory mode is enabled. If it hasn't in + * TPM_RESET_TIME, declare factory enable failed. */ hook_call_deferred(&factory_enable_failed_data, TPM_RESET_TIME); } diff --git a/include/flash_log.h b/include/flash_log.h index b4882ebe36..cfdaf46ce8 100644 --- a/include/flash_log.h +++ b/include/flash_log.h @@ -18,6 +18,7 @@ enum flash_event_type { FE_LOG_OVERFLOWS = 3, /* A single byte, overflow counter. */ FE_LOG_LOCKS = 4, /* A single byte, lock failures counter. */ FE_LOG_NVMEM = 5, /* NVMEM failure, variable structure. */ + FE_LOG_TPM_WIPE_ERROR = 6, /* Failed to wipe the TPM */ /* * Fixed padding value makes it easier to parse log space -- cgit v1.2.1