ite83xx: Remove false-alarm EC panic reports

The pre-watchdog warning interrupt handler currently invokes
get_panic_data_write() to retrieve a pointer to panic data. This has a
side-effect of initializing the panic data with PANIC_MAGIC and a panic
data size without other pertient fields set. As a result, the EC reports
malformed panic to the AP causing an unparsable EC crash to be reported.

Revert the ITE watchdog warning interrupt usage of
get_panic_data_write() to using the global PANIC_DATA_PTR pointer to
panic data.

BRANCH=none
BUG=b:247100203
TEST=On drawcia, dead battery boot, invoke waitms 1100 (pre-wdt warning),
invoke ectool panicinfo to check for well-formed panic data
TEST=On drawcia, invoke crash watchdog, invoke ectool panicinfo to check
for well-formed panic data.

Change-Id: I5a75bec0efa79d00c358edc0645a0a0d39a879d2
Signed-off-by: Aaron Massey <aaronmassey@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4358752
Reviewed-by: Boris Mittelberg <bmbm@google.com>
Reviewed-by: Rob Barnes <robbarnes@google.com>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>

2 files changed, 27 insertions, 7 deletions

diff --git a/chip/it83xx/watchdog.c b/chip/it83xx/watchdog.c
index 4acea9d1b8..b67987d899 100644
--- a/chip/it83xx/watchdog.c
+++ b/chip/it83xx/watchdog.c
@@ -38,8 +38,23 @@ static void watchdog_set_warning_timer(int32_t ms, int init)
 
 void watchdog_warning_irq(void)
 {
-	struct panic_data *const pdata_ptr = get_panic_data_write();
-
+	/*
+	 * Why we directly use the PANIC_DATA_PTR:
+	 *
+	 * We don't get the panic data pointer from get_panic_data_write()
+	 * because this interrupt fires on a warning and we don't want to
+	 * initialize a panic with only a warning. We also don't use
+	 * panic_get_data() because that function returns a NULL pointer if
+	 * panic data was never prior initialized.
+	 *
+	 * Why we fill ipc/mepc here in the watchdog bark/warning interrupt:
+	 *
+	 * In ITE, a full watchdog bite results in an EC reset that bypasses all
+	 * exception handlers. We save the program counter now (during a
+	 * warning) before a full watchdog bite occurs so it is accessible after
+	 * the bite.
+	 */
+	struct panic_data *const pdata_ptr = PANIC_DATA_PTR;
 #if defined(CHIP_CORE_NDS32)
 	pdata_ptr->nds_n8.ipc = get_ipc();
 #elif defined(CHIP_CORE_RISCV)
diff --git a/include/panic.h b/include/panic.h
index 3c769720c7..e3323e7cc7 100644
--- a/include/panic.h
+++ b/include/panic.h
@@ -174,11 +174,16 @@ uintptr_t get_panic_data_start(void);
 struct panic_data *test_get_panic_data_pointer(void);
 #endif
 
-/*
- * Return a pointer to panic_data structure that can be safely written.
- * Please note that this function can move jump data and jump tags.
- * It can also delete panic data from previous boot, so this function
- * should be used when we are sure that we don't need it.
+/**
+ * Return a pointer to panic_data structure that can be safely written.  Please
+ * note that this function can move jump data and jump tags.  It can also delete
+ * panic data from previous boot, so this function should be used when we are
+ * sure that we don't need it.
+ *
+ * NOTE: Invoking this function without subsequently setting the rest of the
+ * panic data is unsafe because it leaves the panic data in an unfinished state
+ * that may be inappropriately reported to the AP.
+ * TODO(b/274661193): Finalize panic data with panic magic.
  *
  * @param pointer to panic_data structure that can be safely written
  */