ap_ro_integrity_check: call keep_ec_in_reset in do_ap_ro_checkstabilize-14385.B-cr50_stabfactory-kukui-14374.B-cr50_stab

Move the keep_ec_in_reset call into do_ap_ro_check, so AP RO
verification will hold the EC in reset when it's triggered from the AP.

This change removes the ap_ro_verification_failed_ variable, so all of
the AP RO verification is included in ap_ro_info. ap_ro_ver_state isn't
needed anymore, so this CL removes it.

BUG=b:207545621
TEST=make clobber ; make buildall -j

Change-Id: Id0b2e04b042d48f2b8a9dae021e762369ca5f3eb
Signed-off-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3300174
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Vadim Bendebury <vbendeb@chromium.org>

3 files changed, 44 insertions, 87 deletions

diff --git a/board/cr50/power_button.c b/board/cr50/power_button.c
index a13f450031..640fd970eb 100644
--- a/board/cr50/power_button.c
+++ b/board/cr50/power_button.c
@@ -59,71 +59,6 @@ static void power_button_press_enable_interrupt(int enable)
 #ifdef CONFIG_AP_RO_VERIFICATION
 
 /*
- * A hook used to keep the EC in reset, no matter what keys the user presses,
- * the only way out is the Cr50 reboot, most likely through power cycle by
- * battery cutoff.
- *
- * Cr50 console over SuzyQ would still be available in case the user has the
- * cable and wants to see what happens with the system. The easiest way to see
- * the system is in this state to run the 'flog' command and examine the flash
- * log.
- */
-static void keep_ec_in_reset(void);
-static bool ap_ro_verification_failed_;
-
-DECLARE_DEFERRED(keep_ec_in_reset);
-
-static void keep_ec_in_reset(void)
-{
-	if (!ap_ro_verification_failed_) {
-		enable_sleep(SLEEP_MASK_AP_RO_VERIFICATION);
-		return;
-	}
-
-	assert_ec_rst();
-	hook_call_deferred(&keep_ec_in_reset_data, 100 * MSEC);
-}
-
-static int ap_ro_ver_state_cmd(int argc, char **argv)
-{
-#ifdef CR50_DEV
-	int const max_args = 2;
-#else
-	int const max_args = 1;
-#endif
-
-	if (argc > max_args)
-		return EC_ERROR_PARAM_COUNT;
-#ifdef CR50_DEV
-	if (argc == max_args) {
-		if (strcasecmp(argv[1], "clear"))
-			return EC_ERROR_PARAM1;
-		if (ap_ro_verification_failed_) {
-			ap_ro_verification_failed_ = false;
-			deassert_ec_rst();
-		}
-	}
-#endif
-	ccprintf("%sAP RO verification failure detected\n",
-		 ap_ro_verification_failed_ ? "" : "NO ");
-
-	return EC_SUCCESS;
-}
-DECLARE_SAFE_CONSOLE_COMMAND(ap_ro_verstate, ap_ro_ver_state_cmd,
-#ifdef CR50_DEV
-			     "[clear]",
-			     "Display and/or clear AP RO validation state"
-#else
-			     "", "Display AP RO validation state"
-#endif
-);
-
-int ec_rst_override(void)
-{
-	return ap_ro_verification_failed_;
-}
-
-/*
  * Implement sequence detecting trigger for starting AP RO verification.
  *
  * 'RCTD' is short for 'RO check trigger detection'.
@@ -234,12 +169,7 @@ static int rctd_poll_handler(void)
 	CPRINTS("RO Validation triggered");
 	ap_ro_add_flash_event(APROF_CHECK_TRIGGERED);
 
-	if (validate_ap_ro() == EC_ERROR_CRC) {
-		/* Validation failed, no go. */
-		ap_ro_verification_failed_ = true;
-		disable_sleep(SLEEP_MASK_AP_RO_VERIFICATION);
-		keep_ec_in_reset();
-	}
+	validate_ap_ro();
 
 	return 0;
 }
@@ -399,4 +329,3 @@ static enum vendor_cmd_rc vc_get_pwr_btn(enum vendor_cmd_cc code,
 	return VENDOR_RC_SUCCESS;
 }
 DECLARE_VENDOR_COMMAND(VENDOR_CC_GET_PWR_BTN, vc_get_pwr_btn);
-
diff --git a/common/ap_ro_integrity_check.c b/common/ap_ro_integrity_check.c
index 6889ae9361..14fdb0f876 100644
--- a/common/ap_ro_integrity_check.c
+++ b/common/ap_ro_integrity_check.c
@@ -18,6 +18,7 @@
 #include "shared_mem.h"
 #include "stddef.h"
 #include "stdint.h"
+#include "system.h"
 #include "timer.h"
 #include "tpm_registers.h"
 #include "usb_spi.h"
@@ -496,7 +497,7 @@ static int verify_keyblock(const struct kb_container *kbc,
 /* Clear validate_ap_ro_boot state. */
 void ap_ro_device_reset(void)
 {
-	if (apro_result == AP_RO_NOT_RUN)
+	if (apro_result == AP_RO_NOT_RUN || ec_rst_override())
 		return;
 	CPRINTS("%s: clear apro result", __func__);
 	apro_result = AP_RO_NOT_RUN;
@@ -1364,6 +1365,40 @@ static enum ap_ro_check_result validate_and_cache_ap_ro_v2_from_flash(void)
 	return ROV_NOT_FOUND;
 }
 
+/*
+ * A hook used to keep the EC in reset, no matter what keys the user presses,
+ * the only way out is the Cr50 reboot, most likely through power cycle by
+ * battery cutoff.
+ *
+ * Cr50 console over SuzyQ would still be available in case the user has the
+ * cable and wants to see what happens with the system. The easiest way to see
+ * the system is in this state to run the 'flog' command and examine the flash
+ * log.
+ */
+static void keep_ec_in_reset(void);
+
+DECLARE_DEFERRED(keep_ec_in_reset);
+
+static void keep_ec_in_reset(void)
+{
+	disable_sleep(SLEEP_MASK_AP_RO_VERIFICATION);
+	assert_ec_rst();
+	hook_call_deferred(&keep_ec_in_reset_data, 100 * MSEC);
+}
+
+static void release_ec_reset_override(void)
+{
+	hook_call_deferred(&keep_ec_in_reset_data, -1);
+	deassert_ec_rst();
+	enable_sleep(SLEEP_MASK_AP_RO_VERIFICATION);
+}
+
+int ec_rst_override(void)
+{
+	return apro_result == AP_RO_FAIL;
+}
+
+
 static uint8_t do_ap_ro_check(void)
 {
 	enum ap_ro_check_result rv;
@@ -1423,14 +1458,17 @@ static uint8_t do_ap_ro_check(void)
 		 * Both explicit failure to verify OR any error if cached
 		 * descriptor was found should block the booting.
 		 */
-		if ((rv == ROV_FAILED) || check_is_required())
+		if ((rv == ROV_FAILED) || check_is_required()) {
+			keep_ec_in_reset();
 			return EC_ERROR_CRC;
+		}
 		return EC_ERROR_UNIMPLEMENTED;
 	}
 
 	apro_result = AP_RO_PASS;
 	ap_ro_add_flash_event(APROF_CHECK_SUCCEEDED);
 	CPRINTS("AP RO verification SUCCEEDED!");
+	release_ec_reset_override();
 
 	return EC_SUCCESS;
 }
@@ -1459,7 +1497,7 @@ static enum vendor_cmd_rc ap_ro_check_callback(struct vendor_cmd_params *p)
 }
 DECLARE_VENDOR_COMMAND_P(VENDOR_CC_AP_RO_VALIDATE, ap_ro_check_callback);
 
-int validate_ap_ro(void)
+void validate_ap_ro(void)
 {
 	struct {
 		struct tpm_cmd_header tpmh;
@@ -1474,9 +1512,6 @@ int validate_ap_ro(void)
 	pack.tpmh.subcommand_code = htobe16(VENDOR_CC_AP_RO_VALIDATE);
 
 	tpm_alt_extension(&pack.tpmh, sizeof(pack));
-
-	/* The last byte is the response code. */
-	return pack.rv;
 }
 
 void ap_ro_add_flash_event(enum ap_ro_verification_ev event)
diff --git a/include/ap_ro_integrity_check.h b/include/ap_ro_integrity_check.h
index 12d701c44b..38e4c57ff4 100644
--- a/include/ap_ro_integrity_check.h
+++ b/include/ap_ro_integrity_check.h
@@ -18,16 +18,9 @@ enum ap_ro_status {
 };
 /*
  * validate_ap_ro: based on information saved in an H1 RO flash page verify
- * contents of the AP flash.
- *
- * Returns:
- *
- *  EC_SUCCESS if valid integrity check information was found and the AP flash
- *             check succeeded.
- *  EC_ERROR_INVAL in valid integrity check information was not found.
- *  EC_ERROR_CRC if information was found, but the AP flash check failed.
+ * contents of the AP flash. Hold the EC in reset if verification fails.
  */
-int validate_ap_ro(void);
+void validate_ap_ro(void);
 
 /*
  * ap_ro_add_flash_event: add a flash log event to keep track of AP RO