g: Improve version info for dual RO & RW images

The SoC looks for two RO images at reset, and is typically
configured for two RW images as well. This CL reports version
strings for all those images, as well as identifying the active
RO and RW copies.

Since the RO image doesn't contain a version string, we create
one using the epoch_, major_, minor_, and img_chk_ members of its
signed header.

BUG=chrome-os-partner:55558
BRANCH=none
TEST=make buildall; run on Cr50 hardware

The "version" command now includes information like this:

  RO_A:  * 0.0.2/a3c3d5ea
  RO_B:    0.0.2/8895c9eb
  RW_A:    cr50_v1.1.4965-a6c1c73-dirty
  RW_B:  * cr50_v1.1.4959-2f49d5c

The '*' indicates the active image.

The test/tpm_test/tpmtest.py program has been updated to request
the version information at startup, and it also now reports
similar information, just all on one line:

  RO_A:* 0.0.2/a3c3d5ea RO_B: 0.0.2/8895c9eb RW_A: cr50_v1.1 ...

The active images are marked with a '*' following the ':', so
that the same regexp can match either format:

  ($ro, $rw) = m/RO_[AB]:\s*\*\s+(\S+).*RW_[AB]:\s*\*\s+(\S+)/s;

Change-Id: Ic27e295d9122045b2ec5a638933924b65ecc8e43
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/362861
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

6 files changed, 215 insertions, 60 deletions

diff --git a/chip/g/config_chip.h b/chip/g/config_chip.h
index 99b9195b8b..97cc56ba94 100644
--- a/chip/g/config_chip.h
+++ b/chip/g/config_chip.h
@@ -81,6 +81,9 @@
  * The following macros try to make this all work.
  */
 
+/* This isn't optional, since the bootrom will always look for both */
+#define CHIP_HAS_RO_B
+
 /* It's easier for us to consider each half as having its own RO and RW */
 #define CFG_FLASH_HALF (CONFIG_FLASH_SIZE >> 1)
 
@@ -96,11 +99,15 @@
 
 /* The RO images start at the very beginning of each flash half */
 #define CONFIG_RO_MEM_OFF 0
+#define CHIP_RO_B_MEM_OFF CFG_FLASH_HALF
 
 /* Size reserved for each RO image */
 #define CONFIG_RO_SIZE 0x4000
 
-/* RW images start right after the reserved-for-RO areas in each half */
+/*
+ * RW images start right after the reserved-for-RO areas in each half, but only
+ * because that's where the RO images look for them. It's not a HW constraint.
+ */
 #define CONFIG_RW_MEM_OFF CONFIG_RO_SIZE
 #define CONFIG_RW_B_MEM_OFF (CFG_FLASH_HALF + CONFIG_RW_MEM_OFF)
 
diff --git a/chip/g/system.c b/chip/g/system.c
index bf1c39e805..2bbd5d0301 100644
--- a/chip/g/system.c
+++ b/chip/g/system.c
@@ -3,10 +3,14 @@
  * found in the LICENSE file.
  */
 
+#include "config.h"
 #include "cpu.h"
+#include "printf.h"
 #include "registers.h"
+#include "signed_header.h"
 #include "system.h"
 #include "task.h"
+#include "version.h"
 
 static void check_reset_cause(void)
 {
@@ -145,3 +149,89 @@ int system_set_vbnvcontext(const uint8_t *block)
 {
 	return 0;
 }
+
+enum system_image_copy_t system_get_ro_image_copy(void)
+{
+	/*
+	 * The bootrom protects the selected bootloader with REGION0,
+	 * so we should be able to identify the active RO by seeing which one
+	 * is protected.
+	 */
+	switch (GREG32(GLOBALSEC, FLASH_REGION0_BASE_ADDR)) {
+	case CONFIG_PROGRAM_MEMORY_BASE + CONFIG_RO_MEM_OFF:
+		return SYSTEM_IMAGE_RO;
+	case CONFIG_PROGRAM_MEMORY_BASE + CHIP_RO_B_MEM_OFF:
+		return SYSTEM_IMAGE_RO_B;
+	}
+
+	return SYSTEM_IMAGE_UNKNOWN;
+}
+
+/*
+ * The RW images contain version strings. The RO images don't, so we'll make
+ * some here.
+ */
+#define MAX_RO_VER_LEN 48
+static char ro_str[2][MAX_RO_VER_LEN];
+
+const char *system_get_version(enum system_image_copy_t copy)
+{
+	const struct version_struct *v;
+	const struct SignedHeader *h;
+	enum system_image_copy_t this_copy;
+	uintptr_t vaddr, delta;
+	int i;
+
+	switch (copy) {
+	case SYSTEM_IMAGE_RO:
+	case SYSTEM_IMAGE_RO_B:
+		/* The RO header is the first thing in each flash half */
+		vaddr = get_program_memory_addr(copy);
+		if (vaddr == INVALID_ADDR)
+			break;
+		h = (const struct SignedHeader *)vaddr;
+		i = (copy == SYSTEM_IMAGE_RO) ? 0 : 1;
+		/* Use some fields from the header for the version string */
+		snprintf(ro_str[i], MAX_RO_VER_LEN, "%d.%d.%d/%08x",
+			 h->epoch_, h->major_, h->minor_, h->img_chk_);
+		return ro_str[i];
+
+	case SYSTEM_IMAGE_RW:
+	case SYSTEM_IMAGE_RW_B:
+		/*
+		 * This function isn't part of any RO image, so we must be in a
+		 * RW image. If the current image is the one we're asked for,
+		 * we can just return our version string.
+		 */
+		this_copy = system_get_image_copy();
+		if (copy == this_copy)
+			return version_data.version;
+
+		/*
+		 * We want the version of the other RW image. The linker script
+		 * puts the version string right after the reset vectors, so
+		 * it's at the same relative offset. Measure that offset here.
+		 */
+		vaddr = get_program_memory_addr(this_copy);
+		delta = (uintptr_t)&version_data - vaddr;
+
+		/* Now look at that offset in the requested image */
+		vaddr = get_program_memory_addr(copy);
+		if (vaddr == INVALID_ADDR)
+			break;
+		vaddr += delta;
+		v = (const struct version_struct *)vaddr;
+
+		/*
+		 * Make sure the version struct cookies match before returning
+		 * the version string.
+		 */
+		if (v->cookie1 == version_data.cookie1 &&
+		    v->cookie2 == version_data.cookie2)
+			return v->version;
+	default:
+		break;
+	}
+
+	return "Error";
+}
diff --git a/common/system.c b/common/system.c
index 471144d24d..02d006097e 100644
--- a/common/system.c
+++ b/common/system.c
@@ -101,19 +101,23 @@ uint32_t sleep_mask;
  * begin. In the case of external storage, the image may or may not currently
  * reside at the location returned.
  */
-static uintptr_t get_program_memory_addr(enum system_image_copy_t copy)
+uintptr_t get_program_memory_addr(enum system_image_copy_t copy)
 {
 	switch (copy) {
 	case SYSTEM_IMAGE_RO:
 		return CONFIG_PROGRAM_MEMORY_BASE + CONFIG_RO_MEM_OFF;
 	case SYSTEM_IMAGE_RW:
 		return CONFIG_PROGRAM_MEMORY_BASE + CONFIG_RW_MEM_OFF;
+#ifdef CHIP_HAS_RO_B
+	case SYSTEM_IMAGE_RO_B:
+		return CONFIG_PROGRAM_MEMORY_BASE + CHIP_RO_B_MEM_OFF;
+#endif
 #ifdef CONFIG_RW_B
 	case SYSTEM_IMAGE_RW_B:
 		return CONFIG_PROGRAM_MEMORY_BASE + CONFIG_RW_B_MEM_OFF;
 #endif
 	default:
-		return 0xffffffff;
+		return INVALID_ADDR;
 	}
 }
 
@@ -128,13 +132,11 @@ static uint32_t get_size(enum system_image_copy_t copy)
 
 	switch (copy) {
 	case SYSTEM_IMAGE_RO:
+	case SYSTEM_IMAGE_RO_B:
 		return CONFIG_RO_SIZE;
 	case SYSTEM_IMAGE_RW:
-		return CONFIG_RW_SIZE;
-#ifdef CONFIG_RW_B
 	case SYSTEM_IMAGE_RW_B:
 		return CONFIG_RW_SIZE;
-#endif
 	default:
 		return 0;
 	}
@@ -344,6 +346,12 @@ test_mockable enum system_image_copy_t system_get_image_copy(void)
 	    my_addr < (CONFIG_RW_MEM_OFF + CONFIG_RW_SIZE))
 		return SYSTEM_IMAGE_RW;
 
+#ifdef CHIP_HAS_RO_B
+	if (my_addr >= CHIP_RO_B_MEM_OFF &&
+	    my_addr < (CHIP_RO_B_MEM_OFF + CONFIG_RO_SIZE))
+		return SYSTEM_IMAGE_RO_B;
+#endif
+
 #ifdef CONFIG_RW_B
 	if (my_addr >= CONFIG_RW_B_MEM_OFF &&
 	    my_addr < (CONFIG_RW_B_MEM_OFF + CONFIG_RW_SIZE))
@@ -443,11 +451,9 @@ const char *system_get_image_copy_string(void)
 
 const char *system_image_copy_t_to_string(enum system_image_copy_t copy)
 {
-	static const char * const image_names[] = {"unknown", "RO", "RW",
-#ifdef CONFIG_RW_B
-		"RW_B",
-#endif
-		};
+	static const char * const image_names[] = {
+		"unknown", "RO", "RW", "RO_B", "RW_B"
+	};
 	return image_names[copy < ARRAY_SIZE(image_names) ? copy : 0];
 }
 
@@ -603,6 +609,7 @@ 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)
 {
 #ifndef CONFIG_MAPPED_STORAGE
@@ -894,40 +901,37 @@ static int command_version(int argc, char **argv)
 	ccprintf("Chip:    %s %s %s\n", system_get_chip_vendor(),
 		 system_get_chip_name(), system_get_chip_revision());
 	ccprintf("Board:   %d\n", system_get_board_version());
+#ifdef CHIP_HAS_RO_B
+	{
+		enum system_image_copy_t active;
+
+		active = system_get_ro_image_copy();
+		ccprintf("RO_A:  %c %s\n",
+			 (active == SYSTEM_IMAGE_RO ? '*' : ' '),
+			 system_get_version(SYSTEM_IMAGE_RO));
+		ccprintf("RO_B:  %c %s\n",
+			 (active == SYSTEM_IMAGE_RO_B ? '*' : ' '),
+			 system_get_version(SYSTEM_IMAGE_RO_B));
+	}
+#else
 	ccprintf("RO:      %s\n", system_get_version(SYSTEM_IMAGE_RO));
+#endif
 #ifdef CONFIG_RW_B
-
-	/*
-	 * Code reporting the RW version fails to properly retrieve the
-	 * version of the non actively running RW image. The code always uses a
-	 * fixed offset into the flash memory, which is correct for RW, but
-	 * incorrect for RW_B.
-	 *
-	 * The RW and RW_B versions end up at different offsets into their
-	 * respective image halves, so it is impossible to find the RW_B
-	 * versoin offset by just adding another offset the the RW version
-	 * offset.
-	 *
-	 * To address this issue, when running an RW image, report the version
-	 * of the running part of the image explicitly.
-	 */
-
 	{
-		enum system_image_copy_t active_copy = system_get_image_copy();
-
-		if (active_copy == SYSTEM_IMAGE_RO) {
-			ccprintf("RW:      %s\n",
-				 system_get_version(SYSTEM_IMAGE_RW));
-		} else {
-			ccprintf("RW%s  %s\n",
-				 active_copy == SYSTEM_IMAGE_RW ? ":  " : "_B:",
-				 system_get_version(active_copy));
-		}
+		enum system_image_copy_t active;
+
+		active = system_get_image_copy();
+		ccprintf("RW_A:  %c %s\n",
+			 (active == SYSTEM_IMAGE_RW ? '*' : ' '),
+			 system_get_version(SYSTEM_IMAGE_RW));
+		ccprintf("RW_B:  %c %s\n",
+			 (active == SYSTEM_IMAGE_RW_B ? '*' : ' '),
+			 system_get_version(SYSTEM_IMAGE_RW_B));
 	}
 #else
 	ccprintf("RW:      %s\n", system_get_version(SYSTEM_IMAGE_RW));
 #endif
-	ccprintf("Build: %s\n", system_get_build_info());
+	ccprintf("Build:   %s\n", system_get_build_info());
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(version, command_version,
diff --git a/common/tpm_registers.c b/common/tpm_registers.c
index b2cbe22a8a..2d63c61a92 100644
--- a/common/tpm_registers.c
+++ b/common/tpm_registers.c
@@ -107,11 +107,32 @@ enum tpm_sts_bits {
 	response_retry = (1 << 1),
 };
 
-#define TPM_FW_VER_MAX_SIZE 64
 /* Used to count bytes read in version string */
 static int tpm_fw_ver_index;
- /* 50 bytes should be enough for the version strings. */
-static uint8_t tpm_fw_ver[50];
+static uint8_t tpm_fw_ver[180];
+
+/*
+ * We need to be able to report firmware version to the host, both RO and RW
+ * sections. This copies the information into a static string so that it can be
+ * passed to the host a little bit at a time.
+ */
+static void set_version_string(void)
+{
+	enum system_image_copy_t active_ro, active_rw;
+
+	active_ro = system_get_ro_image_copy();
+	active_rw = system_get_image_copy();
+	snprintf(tpm_fw_ver, sizeof(tpm_fw_ver),
+		 "RO_A:%s %s RO_B:%s %s RW_A:%s %s RW_B:%s %s",
+		 (active_ro == SYSTEM_IMAGE_RO ? "*" : ""),
+		 system_get_version(SYSTEM_IMAGE_RO),
+		 (active_ro == SYSTEM_IMAGE_RO_B ? "*" : ""),
+		 system_get_version(SYSTEM_IMAGE_RO_B),
+		 (active_rw == SYSTEM_IMAGE_RW ? "*" : ""),
+		 system_get_version(SYSTEM_IMAGE_RW),
+		 (active_rw == SYSTEM_IMAGE_RW_B ? "*" : ""),
+		 system_get_version(SYSTEM_IMAGE_RW_B));
+}
 
 static void set_tpm_state(enum tpm_states state)
 {
@@ -360,6 +381,8 @@ void tpm_register_put(uint32_t regaddr, const uint8_t *data, uint32_t data_size)
 		fifo_reg_write(data, data_size);
 		break;
 	case TPM_FW_VER:
+		/* Reload versions, in case something has been updated */
+		set_version_string();
 		/* Reset read byte count */
 		tpm_fw_ver_index = 0;
 		break;
@@ -425,7 +448,7 @@ void tpm_register_get(uint32_t regaddr, uint8_t *dest, uint32_t data_size)
 			 * Only read while the index remains less than the
 			 * maximum allowed version string size.
 			 */
-			if (tpm_fw_ver_index < TPM_FW_VER_MAX_SIZE) {
+			if (tpm_fw_ver_index < sizeof(tpm_fw_ver)) {
 				*dest++ = tpm_fw_ver[tpm_fw_ver_index];
 				/*
 				 * If reached end of string, then don't update
@@ -510,22 +533,6 @@ static void call_extension_command(struct tpm_cmd_header *tpmh,
 }
 #endif
 
-/*
- * We need to be able to report firmware version to the host, both RO and RW
- * sections. The first four bytes of the RO seciton's SHA are saved in the RO
- * header, which is mapped into the beginning of flash memory.
- */
-static void set_version_string(void)
-{
-	enum system_image_copy_t current_image = system_get_image_copy();
-	const struct SignedHeader *sh = (const struct SignedHeader *)
-		CONFIG_PROGRAM_MEMORY_BASE;
-
-	snprintf(tpm_fw_ver, sizeof(tpm_fw_ver), "RO: %08x RW%s: %s",
-		 sh->img_chk_, current_image == SYSTEM_IMAGE_RW_B ? "_B" : "",
-		 system_get_version(current_image));
-}
-
 void tpm_task(void)
 {
 	set_version_string();
diff --git a/include/system.h b/include/system.h
index 4362097345..39bdb95cd0 100644
--- a/include/system.h
+++ b/include/system.h
@@ -38,9 +38,9 @@ enum system_image_copy_t {
 	SYSTEM_IMAGE_UNKNOWN = 0,
 	SYSTEM_IMAGE_RO,
 	SYSTEM_IMAGE_RW,
-#ifdef CONFIG_RW_B
+	/* Some systems may have these too */
+	SYSTEM_IMAGE_RO_B,
 	SYSTEM_IMAGE_RW_B,
-#endif
 };
 
 /**
@@ -105,6 +105,21 @@ void system_disable_jump(void);
 enum system_image_copy_t system_get_image_copy(void);
 
 /**
+ * Return the active RO image copy so that if we're in RW, we can know how we
+ * got there. Only needed when there are multiple RO images.
+ */
+enum system_image_copy_t system_get_ro_image_copy(void);
+
+/**
+ * Return the program memory address where the image copy begins or should
+ * begin. In the case of external storage, the image may or may not currently
+ * reside at the location returned. Returns INVALID_ADDR if the image copy is
+ * not supported.
+ */
+uintptr_t get_program_memory_addr(enum system_image_copy_t copy);
+#define INVALID_ADDR ((uintptr_t)0xffffffff)
+
+/**
  * Return non-zero if the system has switched between image copies at least
  * once since the last real boot.
  */
diff --git a/test/tpm_test/ftdi_spi_tpm.c b/test/tpm_test/ftdi_spi_tpm.c
index 69b6cf57da..43ca83e3aa 100644
--- a/test/tpm_test/ftdi_spi_tpm.c
+++ b/test/tpm_test/ftdi_spi_tpm.c
@@ -25,6 +25,7 @@ static int ftdi_trace_enabled;
 #define TPM_DATA_FIFO_REG (TPM_LOCALITY_0_SPI_BASE + 0x24)
 #define TPM_DID_VID_REG   (TPM_LOCALITY_0_SPI_BASE + 0xf00)
 #define TPM_RID_REG       (TPM_LOCALITY_0_SPI_BASE + 0xf04)
+#define TPM_FW_VER        (TPM_LOCALITY_0_SPI_BASE + 0xf90)
 
 static struct swig_string_data empty_string_data = (struct swig_string_data){
 	.size = 0, .data = NULL
@@ -203,6 +204,35 @@ static uint32_t GetBurstCount(void)
 	return (status >> burstCountShift) & burstCountMask;
 }
 
+static void GetVersion(void)
+{
+	int chunk_count = 0;
+	uint32_t chunk = 0;
+	char vstr[sizeof(chunk) + 1];           /* room for 4 chars + zero */
+
+	/*
+	 * Does not really matter what's written, this just makes sure
+	 * the version is reported from the beginning.
+	 */
+	FtdiWriteReg(TPM_FW_VER, sizeof(chunk), &chunk);
+
+	/* Print it out in 4 byte chunks. */
+	vstr[sizeof(vstr) - 1] = 0;
+	do {
+		FtdiReadReg(TPM_FW_VER, sizeof(chunk), vstr);
+		printf("%s", vstr);
+
+		/*
+		 * While string is not over, and no more than 200
+		 * characters.
+		 * This is likely result in one extra printk()
+		 * invocation with an empty string, not a big deal.
+		 */
+	} while (vstr[0] && (chunk_count++ < (200 / sizeof(chunk))));
+
+	printf("\n");
+}
+
 int FtdiSpiInit(uint32_t freq, int enable_debug)
 {
 	uint32_t did_vid, status;
@@ -271,6 +301,8 @@ int FtdiSpiInit(uint32_t freq, int enable_debug)
 	printf("Connected to device vid:did:rid of %4.4x:%4.4x:%2.2x\n",
 	       did_vid & 0xffff, did_vid >> 16, cmd);
 
+	GetVersion();
+
 	return true;
 }