nocturne_fp: Add fix for legacy reset loop

This patch mitigates an infinite reset loop caused by an RO bug.
The reset occurs in RO when hardware write protect (wp_gpio_asserted)
is disabled, but software write protect (ro_now) is still enabled.
This can be seen by disabling hardware write protect and issuing
a soft reset.

There is one case where RO will forgo issuing this system reset.
That is when it detects a power on reset. Furthermore, it retrieves
its reset flags from the main system_get_reset_flags function, which
combines hardware reset registers AND a special RTC backup register
designed to preserve reset flags.

We exploit this reset backup register mechanism to inject a fake power-on
flag before resetting. As an added bonus, we also inject an ap-off flag
so that we can determine on startup if the power-on flag is real
or forged by this mechanism. If we detect that the power-on flag was
forged, we print a warning and fix the current reset flags.

In order to ensure that a power-on will be forged when
a spurious reset happens (exception or pin reset), we keep
the backup register loaded with the power-on and ap-off reset flags,
when the hardware write protect is disabled.

In order to keep the typical code path (HW+SW WP enabled)
clear of complexity and false power-on reports, we only forge
the power-on flag when hardware write protect is disabled.
Thus, we conditionally setup the forge on startup and setup an
interrupt handler to catch changes to the hardware write protect status.

It is safe to use ap-off flag for our nefarious purposes, since
the fingerprint controller has no functionality to control an AP
and has no included code that uses this reset flag.

Review:
* Normal power on reset --> The ap-off flag should be cleared
* Forged power on reset --> We set the ap-off flag

Scenarios covered:
* True power on      --> No reset loop and ap-off would not be set
* HW reset pulse     --> We preloaded ap-off and power-on flags in
                         the reset backup register
* Exception/Watchdog --> Same as above
* System reboot      --> We modified the system_reset function to
                         add ap-off and power-on to reset backup
                         register

BRANCH=nocturne,hatch
BUG=b:146428434
TEST=make buildall -j
TEST=Checked all of the scenarios mentioned above
     in the [SW-WP off + HW-WP off], [SW-WP on + HW-WP on],
     and [SW-WP on + HW-WP off] situations using the nucleo-h743zi
     board (https://crrev.com/c/1994624).
TEST=Checked all of the previous using nocturne_fp board on nucleo-h743zi
TEST=Checked stable RO+fixed-RW on Kohaku

Change-Id: I89361fa95be8eafe78c80c30f5b3195d7a724f81
Signed-off-by: Craig Hesling <hesling@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1992740
Reviewed-by: Tom Hughes <tomhughes@chromium.org>

4 files changed, 139 insertions, 1 deletions

diff --git a/board/nocturne_fp/board.h b/board/nocturne_fp/board.h
index f1975d582d..1774e8483f 100644
--- a/board/nocturne_fp/board.h
+++ b/board/nocturne_fp/board.h
@@ -167,6 +167,13 @@
 #define CONFIG_CMD_FLASH
 #define CONFIG_CMD_SPI_XFER
 
+#ifdef SECTION_IS_RW
+/*
+ * Mitigating the effects of b/146428434.
+ */
+#define APPLY_RESET_LOOP_FIX
+#endif
+
 #ifndef __ASSEMBLER__
 
 /* Timer selection */
@@ -177,6 +184,9 @@
 
 void fps_event(enum gpio_signal signal);
 
+/* Defined in ro_workarounds.c */
+void wp_event(enum gpio_signal signal);
+
 #endif /* !__ASSEMBLER__ */
 
 #endif /* __BOARD_H */
diff --git a/board/nocturne_fp/build.mk b/board/nocturne_fp/build.mk
index 1116b15149..9874357472 100644
--- a/board/nocturne_fp/build.mk
+++ b/board/nocturne_fp/build.mk
@@ -9,6 +9,7 @@ CHIP:=stm32
 CHIP_FAMILY:=stm32h7
 CHIP_VARIANT:=stm32h7x3
 
+board-rw=ro_workarounds.o
 board-y=board.o fpsensor_detect.o
 
 test-list-y=aes sha256 sha256_unrolled
diff --git a/board/nocturne_fp/gpio.inc b/board/nocturne_fp/gpio.inc
index 7e0faa1b36..a76f82cf94 100644
--- a/board/nocturne_fp/gpio.inc
+++ b/board/nocturne_fp/gpio.inc
@@ -10,10 +10,16 @@ GPIO_INT(SPI1_NSS,        PIN(A, 4), GPIO_INPUT, spi_event)
 
 GPIO_INT(PCH_SLP_S0_L,    PIN(D,13), GPIO_INT_BOTH, slp_event)
 GPIO_INT(PCH_SLP_S3_L,    PIN(A,11), GPIO_INT_BOTH, slp_event)
+
+#if defined(APPLY_RESET_LOOP_FIX) && defined(SECTION_IS_RW)
+GPIO_INT(WP,              PIN(B, 7), GPIO_INT_BOTH, wp_event)
+#endif
+
 GPIO(PCH_SLP_S4_L,        PIN(D, 8), GPIO_INPUT)
 GPIO(PCH_SLP_SUS_L,       PIN(D, 3), GPIO_INPUT)
-
+#if !(defined(APPLY_RESET_LOOP_FIX) && defined(SECTION_IS_RW))
 GPIO(WP,                  PIN(B, 7), GPIO_INPUT)
+#endif
 
 /* Outputs */
 GPIO(EC_INT_L,            PIN(A, 1),  GPIO_OUT_HIGH)
diff --git a/board/nocturne_fp/ro_workarounds.c b/board/nocturne_fp/ro_workarounds.c
new file mode 100644
index 0000000000..333f1ed9da
--- /dev/null
+++ b/board/nocturne_fp/ro_workarounds.c
@@ -0,0 +1,121 @@
+/* Copyright 2020 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/* A place to organize workarounds for legacy RO */
+
+#include <assert.h>
+#include <stdbool.h>
+
+#include "bkpdata.h"
+#include "common.h"
+#include "console.h"
+#include "ec_commands.h" /* Reset cause */
+#include "gpio.h"
+#include "hooks.h"
+#include "system.h"
+#include "task.h"
+#include "watchdog.h"
+
+/* Console output macros */
+#define CPRINTS(format, args...) cprints(CC_SYSTEM, format, ## args)
+
+/*
+ * We only patch RW to ensure that future ROs have correct behavior.
+ */
+#if defined(APPLY_RESET_LOOP_FIX) && defined(SECTION_IS_RW)
+
+/*
+ * Add in ap-off flag to be able to detect on next boot.
+ * No other code in this build uses this ap-off reset flag.
+ */
+#define FORGE_PORFLAG_FLAGS (EC_RESET_FLAG_POWER_ON|EC_RESET_FLAG_AP_OFF)
+
+static void wp_change_deferred(void)
+{
+	/*
+	 * The normal state of the reset backup register is 0, but
+	 * we know that our override version of bkpdata_write_reset_flags
+	 * will adjust it based on GPIO_WP's status.
+	 */
+	bkpdata_write_reset_flags(0);
+}
+DECLARE_DEFERRED(wp_change_deferred);
+
+/*
+ * We respond to changes in the hardware write protect line in order to
+ * ensure this workaround is installed when it is needed and uninstalled
+ * when it is not needed. This ensures that we are protected during
+ * unexpected resets, such as pin resets or double faults.
+ *
+ * Furthermore, installing and uninstalling when needed minimizes the
+ * difference between our normal operating conditions and normal operating
+ * conditions with this ro_workaround source being included. That is to say,
+ * the system behavior is only altered in the less likely state, when hardware
+ * write protect deasserted.
+ */
+void wp_event(enum gpio_signal signal)
+{
+	/*
+	 * We must use a deferred function to call bkpdata_write_reset_flags,
+	 * since the underlying bkpdata_write uses a mutex.
+	 */
+	hook_call_deferred(&wp_change_deferred_data, 0);
+}
+
+/*
+ * We intercept all changes to the reset backup register to ensure that
+ * our reset loop patch stays in place.
+ *
+ * This function will be called once in check_reset_cause during
+ * startup, which ensures proper behavior even when unexpected
+ * resets occurs (pin reset or exception).
+ *
+ * This function is also called from system_reset to set the final save
+ * reset flags, before an actual planned reset.
+ */
+__override
+void bkpdata_write_reset_flags(uint32_t save_flags)
+{
+	/* Preserve flags in case a reset pulse occurs */
+	if (!gpio_get_level(GPIO_WP))
+		save_flags |= FORGE_PORFLAG_FLAGS;
+
+#ifdef CONFIG_STM32_RESET_FLAGS_EXTENDED
+	bkpdata_write(BKPDATA_INDEX_SAVED_RESET_FLAGS, save_flags & 0xffff);
+	bkpdata_write(BKPDATA_INDEX_SAVED_RESET_FLAGS_2, save_flags >> 16);
+#else
+	/* Reset flags are 32-bits, but BBRAM entry is only 16 bits. */
+	ASSERT(!(save_flags >> 16));
+	bkpdata_write(BKPDATA_INDEX_SAVED_RESET_FLAGS, save_flags);
+#endif
+}
+
+/*
+ * We do not need to explicitly invoke bkpdata_write_reset_flags
+ * on boot, since check_reset_cause will already invoke it once on boot.
+ */
+static void board_init_workarounds(void)
+{
+	gpio_disable_interrupt(GPIO_WP);
+	gpio_clear_pending_interrupt(GPIO_WP);
+
+	/*
+	 * Detect our forged power-on flag and correct the current
+	 * system reset flags.
+	 * This does not ensure that all init functions will see
+	 * the corrected system reset flags, so care should be taken.
+	 */
+	if ((system_get_reset_flags() & FORGE_PORFLAG_FLAGS) ==
+	    FORGE_PORFLAG_FLAGS) {
+		CPRINTS("WARNING: Reset flags power-on + ap-off were forged.");
+		system_clear_reset_flags(FORGE_PORFLAG_FLAGS);
+	}
+
+	gpio_enable_interrupt(GPIO_WP);
+}
+/* Run one priority level higher than the main board_init in board.c */
+DECLARE_HOOK(HOOK_INIT, board_init_workarounds, HOOK_PRIO_DEFAULT - 1);
+
+#endif /* APPLY_RESET_LOOP_FIX && SECTION_IS_RW */