system: Use stored size in image_data for determining image_used

Image used size is now part of the image_data struct present in all
images at a fixed offset, so use it rather than scanning from the end of
the image.

BUG=chromium:577915
TEST=Verify on kevin + lars + lars_pd that system_get_image_used() returns
the same value as the old implementation, for both RO and RW images.
BRANCH=None

Change-Id: I35f0aa87f5ab1371dbd8b132f22b9d0044358223
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/450859
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

4 files changed, 25 insertions, 89 deletions

diff --git a/common/system.c b/common/system.c
index 4caf7b799d..72d642f0f4 100644
--- a/common/system.c
+++ b/common/system.c
@@ -126,7 +126,7 @@ uintptr_t get_program_memory_addr(enum system_image_copy_t copy)
 /**
  * Return the size of the image copy, or 0 if error.
  */
-static uint32_t get_size(enum system_image_copy_t copy)
+static uint32_t __attribute__((unused)) get_size(enum system_image_copy_t copy)
 {
 	/* Ensure we return aligned sizes. */
 	BUILD_ASSERT(CONFIG_RO_SIZE % SPI_FLASH_MAX_WRITE_SIZE == 0);
@@ -364,59 +364,6 @@ test_mockable enum system_image_copy_t system_get_image_copy(void)
 #endif
 }
 
-/*
- * TODO(crbug.com/577915): Store image used size at build time and simply
- * read it back.
- */
-int system_get_image_used(enum system_image_copy_t copy)
-{
-#ifdef CONFIG_EXTERNAL_STORAGE
-	static uint8_t buf[SPI_FLASH_MAX_WRITE_SIZE];
-#endif
-	int image_offset;
-	const uint8_t *image;
-	int size;
-	size = get_size(copy);
-	if (size <= 0)
-		return 0;
-
-	/*
-	 * Scan backwards looking for 0xea byte, which is by definition the
-	 * last byte of the image.  See ec.lds.S for how this is inserted at
-	 * the end of the image.
-	 */
-	image_offset = (copy == SYSTEM_IMAGE_RW) ?
-			CONFIG_EC_WRITABLE_STORAGE_OFF + CONFIG_RW_STORAGE_OFF :
-			CONFIG_EC_PROTECTED_STORAGE_OFF + CONFIG_RO_STORAGE_OFF;
-#ifdef CONFIG_EXTERNAL_STORAGE
-	image = buf;
-
-	do {
-		if (image == buf) {
-			/* No valid image found? */
-			if (size < SPI_FLASH_MAX_WRITE_SIZE)
-				return 0;
-
-			flash_read(image_offset + size -
-				SPI_FLASH_MAX_WRITE_SIZE,
-				SPI_FLASH_MAX_WRITE_SIZE, buf);
-			image = buf + SPI_FLASH_MAX_WRITE_SIZE;
-		}
-
-		image--, size--;
-
-	} while (*image != 0xea);
-#else
-	image = (const uint8_t *)(image_offset + CONFIG_MAPPED_STORAGE_BASE);
-	flash_lock_mapped_storage(1);
-	for (size--; size > 0 && image[size] != 0xea; size--)
-		;
-	flash_lock_mapped_storage(0);
-#endif
-
-	return size ? size + 1 : 0;  /* 0xea byte IS part of the image */
-}
-
 test_mockable int system_unsafe_to_overwrite(uint32_t offset, uint32_t size)
 {
 	uint32_t r_offset;
@@ -613,8 +560,8 @@ int system_run_image_copy(enum system_image_copy_t copy)
 	return EC_ERROR_UNKNOWN;
 }
 
-__attribute__((weak))	   /* Weird chips may need their own implementations */
-const char *system_get_version(enum system_image_copy_t copy)
+static const struct image_data *system_get_image_data(
+					enum system_image_copy_t copy)
 {
 	static struct image_data data;
 
@@ -623,10 +570,9 @@ const char *system_get_version(enum system_image_copy_t copy)
 
 	/* Handle version of current image */
 	if (copy == active_copy || copy == SYSTEM_IMAGE_UNKNOWN)
-		return &current_image_data.version[0];
-
+		return &current_image_data;
 	if (active_copy == SYSTEM_IMAGE_UNKNOWN)
-		return "";
+		return NULL;
 
 	/*
 	 * The version string is always located after the reset vectors, so
@@ -651,16 +597,31 @@ const char *system_get_version(enum system_image_copy_t copy)
 #else
 	/* Read the version struct from flash into a buffer. */
 	if (flash_read(addr, sizeof(data), (char *)&data))
-		return "";
+		return NULL;
 #endif
 
 	/* Make sure the version struct cookies match before returning the
 	 * version string. */
 	if (data.cookie1 == current_image_data.cookie1 &&
 	    data.cookie2 == current_image_data.cookie2)
-		return data.version;
+		return &data;
+
+	return NULL;
+}
+
+__attribute__((weak))	   /* Weird chips may need their own implementations */
+const char *system_get_version(enum system_image_copy_t copy)
+{
+	const struct image_data *data = system_get_image_data(copy);
+
+	return data ? data->version : "";
+}
+
+int system_get_image_used(enum system_image_copy_t copy)
+{
+	const struct image_data *data = system_get_image_data(copy);
 
-	return "";
+	return data ? MAX((int)data->size, 0) : 0;
 }
 
 int system_get_board_version(void)
diff --git a/core/cortex-m/ec.lds.S b/core/cortex-m/ec.lds.S
index 77db031076..2deedbbd3d 100644
--- a/core/cortex-m/ec.lds.S
+++ b/core/cortex-m/ec.lds.S
@@ -325,14 +325,6 @@ SECTIONS
 	 * can expand to use all the remaining RAM. */
 	__shared_mem_buf = .;
 
-	/* Tag at end of firmware image so that we can find the image size.
-	 * This may be overwritten by the shared memory buffer; that's ok
-	 * because we only use it to find the image size in flash. */
-        . = ALIGN(4);
-        BYTE(0x45);
-        BYTE(0x4e);
-        BYTE(0x44);
-        BYTE(0xea);
 	/* NOTHING MAY GO AFTER THIS! */
     } > IRAM
 
diff --git a/core/cortex-m0/ec.lds.S b/core/cortex-m0/ec.lds.S
index 98840266e4..c03133b186 100644
--- a/core/cortex-m0/ec.lds.S
+++ b/core/cortex-m0/ec.lds.S
@@ -212,14 +212,6 @@ SECTIONS
 	 * can expand to use all the remaining RAM. */
 	__shared_mem_buf = .;
 
-	/* Tag at end of firmware image so that we can find the image size.
-	 * This may be overwritten by the shared memory buffer; that's ok
-	 * because we only use it to find the image size in flash. */
-        . = ALIGN(4);
-        BYTE(0x45);
-        BYTE(0x4e);
-        BYTE(0x44);
-        BYTE(0xea);
 	/* NOTHING MAY GO AFTER THIS! */
     } > IRAM
 
diff --git a/core/nds32/ec.lds.S b/core/nds32/ec.lds.S
index 0557cbca38..1451c5a17c 100644
--- a/core/nds32/ec.lds.S
+++ b/core/nds32/ec.lds.S
@@ -214,17 +214,8 @@ SECTIONS
 
     } > IRAM
 
-    .flash.tag : {
-        /* Tag at end of firmware image so that we can find the image size.
-         * This may be overwritten by the shared memory buffer; that's ok
-         * because we only use it to find the image size in flash. */
-        . = ALIGN(4);
-        BYTE(0x45);
-        BYTE(0x4e);
-        BYTE(0x44);
-        BYTE(0xea);
-        /* NOTHING MAY GO IN FLASH AFTER THIS! */
-    } >FLASH
+    /* TODO(b:36228568): Properly compute image size for nds32. */
+    __image_size = 0;
 
 #ifdef CONFIG_LPC
     .h2ram (NOLOAD) : {