Convert vboot hash calculation from task to deferred function

Vboot hash calculation takes ~350 ms during EC boot.  Since the hash
task is higher priority than the hook task, this starves all the hooks
during boot.

We could, in theory, fix that simply by swapping the priority of the
hook and hash tasks.  But then watchdog detection (in the hook task)
wouldn't detect hangs in the hash task.

A better fix (implemented here) is to convert the hashing operation to
a series of deferred function calls.  This gets rid of the hash task
entirely, and allows all pending hooks and other deferred function
calls to take place between each chunk of hashing.

On STM32-based boards, we need to bump up the hook task stack size,
since hashing is called from several layers deep in the hook task
instead of at the top of its own task, but this is still a net win of
several hundred bytes of SRAM.

BUG=None
Original-BUG=chrome-os-partner:24892
BRANCH=None
TEST=Boot EC; look for "hash start" and "hash done" debug output.
     'taskinfo' shows at least 32 bytes of unused stack for HOOKS task.
     'hash ro' runs properly from EC console.

Change-Id: I8ec26619ed48a7f60f512d06d3dbc13b1bbf12a2
Original-Change-Id: I7921749c95906c30655d456cee9a4555a7d2a88c
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/181954
Reviewed-on: https://chromium-review.googlesource.com/182248
Commit-Queue: Dave Parker <dparker@chromium.org>
Tested-by: Dave Parker <dparker@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/182421
Reviewed-by: Dave Parker <dparker@chromium.org>

10 files changed, 99 insertions, 87 deletions

diff --git a/board/glimmer/board.h b/board/glimmer/board.h
index e8a522ae64..190befbb65 100644
--- a/board/glimmer/board.h
+++ b/board/glimmer/board.h
@@ -38,6 +38,7 @@
 #define CONFIG_TEMP_SENSOR_TMP432
 #define CONFIG_USB_PORT_POWER_SMART
 #define CONFIG_USB_PORT_POWER_SMART_SIMPLE
+#define CONFIG_VBOOT_HASH
 #define CONFIG_WIRELESS
 
 #ifndef __ASSEMBLER__
diff --git a/board/glimmer/ec.tasklist b/board/glimmer/ec.tasklist
index 51a3cdc017..934b936894 100644
--- a/board/glimmer/ec.tasklist
+++ b/board/glimmer/ec.tasklist
@@ -19,7 +19,6 @@
  */
 #define CONFIG_TASK_LIST \
 	TASK_ALWAYS(HOOKS, hook_task, NULL, LARGER_TASK_STACK_SIZE) \
-	TASK_NOTEST(VBOOTHASH, vboot_hash_task, NULL, LARGER_TASK_STACK_SIZE) \
 	/* TASK_ALWAYS(CHARGER, charger_task, NULL, TASK_STACK_SIZE) */ \
 	TASK_NOTEST(CHIPSET, chipset_task, NULL, TASK_STACK_SIZE) \
 	TASK_NOTEST(KEYPROTO, keyboard_protocol_task, NULL, TASK_STACK_SIZE) \
diff --git a/board/rambi/board.h b/board/rambi/board.h
index 1cde39f89b..420d072620 100644
--- a/board/rambi/board.h
+++ b/board/rambi/board.h
@@ -36,6 +36,7 @@
 #define CONFIG_TEMP_SENSOR_TMP432
 #define CONFIG_USB_PORT_POWER_SMART
 #define CONFIG_USB_PORT_POWER_SMART_SIMPLE
+#define CONFIG_VBOOT_HASH
 #define CONFIG_WIRELESS
 
 #ifndef __ASSEMBLER__
diff --git a/board/rambi/ec.tasklist b/board/rambi/ec.tasklist
index c8284ec561..8b4485818f 100644
--- a/board/rambi/ec.tasklist
+++ b/board/rambi/ec.tasklist
@@ -19,7 +19,6 @@
  */
 #define CONFIG_TASK_LIST \
 	TASK_ALWAYS(HOOKS, hook_task, NULL, LARGER_TASK_STACK_SIZE) \
-	TASK_NOTEST(VBOOTHASH, vboot_hash_task, NULL, LARGER_TASK_STACK_SIZE) \
 	TASK_ALWAYS(CHARGER, charger_task, NULL, TASK_STACK_SIZE) \
 	TASK_NOTEST(CHIPSET, chipset_task, NULL, TASK_STACK_SIZE) \
 	TASK_NOTEST(KEYPROTO, keyboard_protocol_task, NULL, TASK_STACK_SIZE) \
diff --git a/chip/stm32/config_chip.h b/chip/stm32/config_chip.h
index 2d439a800e..afc2afe32c 100644
--- a/chip/stm32/config_chip.h
+++ b/chip/stm32/config_chip.h
@@ -36,6 +36,9 @@
 /* Default task stack size */
 #define TASK_STACK_SIZE 488
 
+/* Larger task stack size, for hook task */
+#define LARGER_TASK_STACK_SIZE 640
+
 /* Interval between HOOK_TICK notifications */
 #define HOOK_TICK_INTERVAL (500 * MSEC)
 
diff --git a/common/build.mk b/common/build.mk
index b1ac341891..21814a5d83 100644
--- a/common/build.mk
+++ b/common/build.mk
@@ -54,11 +54,11 @@ common-$(CONFIG_SWITCH)+=switch.o
 common-$(CONFIG_TEMP_SENSOR)+=temp_sensor.o thermal.o
 common-$(CONFIG_USB_PORT_POWER_DUMB)+=usb_port_power_dumb.o
 common-$(CONFIG_USB_PORT_POWER_SMART)+=usb_port_power_smart.o
+common-$(CONFIG_VBOOT_HASH)+=sha256.o vboot_hash.o
 common-$(CONFIG_WIRELESS)+=wireless.o
 common-$(HAS_TASK_CHIPSET)+=chipset.o throttle_ap.o
 common-$(HAS_TASK_CONSOLE)+=console.o
 common-$(HAS_TASK_HOSTCMD)+=acpi.o host_command.o host_event_commands.o
 common-$(HAS_TASK_KEYSCAN)+=keyboard_scan.o
 common-$(HAS_TASK_LIGHTBAR)+=lightbar.o
-common-$(HAS_TASK_VBOOTHASH)+=sha256.o vboot_hash.o
 common-$(TEST_BUILD)+=test_util.o
diff --git a/common/flash.c b/common/flash.c
index ccb0827806..10dda3768b 100644
--- a/common/flash.c
+++ b/common/flash.c
@@ -132,7 +132,7 @@ int flash_write(int offset, int size, const char *data)
 	if (flash_dataptr(offset, size, CONFIG_FLASH_WRITE_SIZE, NULL) < 0)
 		return EC_ERROR_INVAL;  /* Invalid range */
 
-#ifdef HAS_TASK_VBOOTHASH
+#ifdef CONFIG_VBOOT_HASH
 	vboot_hash_invalidate(offset, size);
 #endif
 
@@ -144,7 +144,7 @@ int flash_erase(int offset, int size)
 	if (flash_dataptr(offset, size, CONFIG_FLASH_ERASE_SIZE, NULL) < 0)
 		return EC_ERROR_INVAL;  /* Invalid range */
 
-#ifdef HAS_TASK_VBOOTHASH
+#ifdef CONFIG_VBOOT_HASH
 	vboot_hash_invalidate(offset, size);
 #endif
 
diff --git a/common/vboot_hash.c b/common/vboot_hash.c
index abc67ebe00..bd80fbde2d 100644
--- a/common/vboot_hash.c
+++ b/common/vboot_hash.c
@@ -28,7 +28,9 @@ struct vboot_hash_tag {
 
 #define VBOOT_HASH_SYSJUMP_TAG 0x5648 /* "VH" */
 #define VBOOT_HASH_SYSJUMP_VERSION 1
-#define CHUNK_SIZE 1024
+
+#define CHUNK_SIZE 1024       /* Bytes to hash per deferred call */
+#define WORK_INTERVAL_US 100  /* Delay between deferred calls */
 
 static uint32_t data_offset;
 static uint32_t data_size;
@@ -39,10 +41,65 @@ static int in_progress;
 
 static struct sha256_ctx ctx;
 
-/*
+/**
+ * Abort hash currently in progress, and invalidate any completed hash.
+ */
+static void vboot_hash_abort(void)
+{
+	if (in_progress) {
+		want_abort = 1;
+	} else {
+		CPRINTF("[%T hash abort]\n");
+		want_abort = 0;
+		data_size = 0;
+		hash = NULL;
+	}
+}
+
+/**
+ * Do next chunk of hashing work, if any.
+ */
+static void vboot_hash_next_chunk(void)
+{
+	int size;
+
+	/* Handle abort */
+	if (want_abort) {
+		in_progress = 0;
+		vboot_hash_abort();
+		return;
+	}
+
+	/* Compute the next chunk of hash */
+	size = MIN(CHUNK_SIZE, data_size - curr_pos);
+	SHA256_update(&ctx, (const uint8_t *)(CONFIG_FLASH_BASE +
+					      data_offset + curr_pos), size);
+
+	curr_pos += size;
+	if (curr_pos >= data_size) {
+		/* Store the final hash */
+		hash = SHA256_final(&ctx);
+		CPRINTF("[%T hash done %.*h]\n", SHA256_DIGEST_SIZE, hash);
+
+		in_progress = 0;
+
+		/* Handle receiving abort during finalize */
+		if (want_abort)
+			vboot_hash_abort();
+
+		return;
+	}
+
+	/* If we're still here, more work to do; come back later */
+	hook_call_deferred(vboot_hash_next_chunk, WORK_INTERVAL_US);
+}
+DECLARE_DEFERRED(vboot_hash_next_chunk);
+
+/**
  * Start computing a hash of <size> bytes of data at flash offset <offset>.
- * If nonce_size is non-zero, prefixes the <nonce> onto the data to be
- * hashed.  Returns non-zero if error.
+ *
+ * If nonce_size is non-zero, prefixes the <nonce> onto the data to be hashed.
+ * Returns non-zero if error.
  */
 static int vboot_hash_start(uint32_t offset, uint32_t size,
 			    const uint8_t *nonce, int nonce_size)
@@ -74,25 +131,34 @@ static int vboot_hash_start(uint32_t offset, uint32_t size,
 	if (nonce_size)
 		SHA256_update(&ctx, nonce, nonce_size);
 
-	/* Wake the hash task */
-	task_wake(TASK_ID_VBOOTHASH);
+	hook_call_deferred(vboot_hash_next_chunk, 0);
 
 	return EC_SUCCESS;
 }
 
-/* Abort hash currently in progress, and invalidate any completed hash. */
-static void vboot_hash_abort(void)
+int vboot_hash_invalidate(int offset, int size)
 {
-	if (in_progress) {
-		want_abort = 1;
-	} else {
-		CPRINTF("[%T hash abort]\n");
-		want_abort = 0;
-		data_size = 0;
-		hash = NULL;
-	}
+	/* Don't invalidate if passed an invalid region */
+	if (offset < 0 || size <= 0 || offset + size < 0)
+		return 0;
+
+	/* Don't invalidate if hash is already invalid */
+	if (!hash)
+		return 0;
+
+	/* No overlap if passed region is off either end of hashed region */
+	if (offset + size <= data_offset || offset >= data_offset + data_size)
+		return 0;
+
+	/* Invalidate the hash */
+	CPRINTF("[%T hash invalidated 0x%08x 0x%08x]\n", offset, size);
+	vboot_hash_abort();
+	return 1;
 }
 
+/*****************************************************************************/
+/* Hooks */
+
 static void vboot_hash_init(void)
 {
 #ifdef CONFIG_SAVE_VBOOT_HASH
@@ -117,72 +183,7 @@ static void vboot_hash_init(void)
 				 NULL, 0);
 	}
 }
-
-int vboot_hash_invalidate(int offset, int size)
-{
-	/* Don't invalidate if passed an invalid region */
-	if (offset < 0 || size <= 0 || offset + size < 0)
-		return 0;
-
-	/* Don't invalidate if hash is already invalid */
-	if (!hash)
-		return 0;
-
-	/* No overlap if passed region is off either end of hashed region */
-	if (offset + size <= data_offset || offset >= data_offset + data_size)
-		return 0;
-
-	/* Invalidate the hash */
-	CPRINTF("[%T hash invalidated 0x%08x 0x%08x]\n", offset, size);
-	vboot_hash_abort();
-	return 1;
-}
-
-void vboot_hash_task(void)
-{
-	vboot_hash_init();
-
-	while (1) {
-		if (!in_progress) {
-			/* Nothing to do, so go back to sleep */
-			task_wait_event(-1);
-		} else if (want_abort) {
-			/* Abort hash computation currently in progress */
-			in_progress = 0;
-			vboot_hash_abort();
-		} else {
-			/* Compute the next chunk of hash */
-			int size = MIN(CHUNK_SIZE, data_size - curr_pos);
-
-			SHA256_update(&ctx,
-				      (const uint8_t *)(CONFIG_FLASH_BASE +
-							data_offset + curr_pos),
-				      size);
-			curr_pos += size;
-			if (curr_pos >= data_size) {
-				/* Store the final hash */
-				hash = SHA256_final(&ctx);
-				CPRINTF("[%T hash done %.*h]\n",
-					SHA256_DIGEST_SIZE, hash);
-
-				in_progress = 0;
-
-				/* Handle receiving abort during finalize */
-				if (want_abort)
-					vboot_hash_abort();
-			}
-
-			/*
-			 * Let other tasks (really, just the watchdog task)
-			 * run for a bit.
-			 */
-			usleep(100);
-		}
-	}
-}
-
-/*****************************************************************************/
-/* Hooks */
+DECLARE_HOOK(HOOK_INIT, vboot_hash_init, HOOK_PRIO_DEFAULT);
 
 #ifdef CONFIG_SAVE_VBOOT_HASH
 
diff --git a/include/config.h b/include/config.h
index 4d4c908e01..f3035c2fa9 100644
--- a/include/config.h
+++ b/include/config.h
@@ -744,6 +744,11 @@
 #undef CONFIG_USB_SWITCH_TSU6721
 
 /*****************************************************************************/
+
+/* Support computing hash of code for verified boot */
+#undef CONFIG_VBOOT_HASH
+
+/*****************************************************************************/
 /* Watchdog config */
 
 /*
diff --git a/test/test_config.h b/test/test_config.h
index 70a3ec72a3..4767d034be 100644
--- a/test/test_config.h
+++ b/test/test_config.h
@@ -8,6 +8,9 @@
 #ifndef __CROS_EC_TEST_CONFIG_H
 #define __CROS_EC_TEST_CONFIG_H
 
+/* Don't compile vboot hash support unless specifically testing for it */
+#undef CONFIG_VBOOT_HASH
+
 #ifdef TEST_ADAPTER
 #define CONFIG_CHIPSET_CAN_THROTTLE
 #define CONFIG_EXTPOWER_FALCO