close RO window after use

The code opening the RO windows to make it possible to update or
enable the RO images, leaves the window open, allowing subsequent
writes into the RO space. It has been acceptable until now, because RO
updates are usually followed up by reboots.

With introduction of the AP RO hash, there is a need to close the
window (specifically, disable write access) when not in use. This
patch adds a function for that and uses the new function everywhere
where flash_open_ro_window() is called.

BUG=b:153764696
TEST=verified successful Cr50 RO and AP RO hash  updates.

Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Change-Id: Ia595e5c7ce0beb1a67ef3513117984d18655a60c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2204973
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

5 files changed, 38 insertions, 8 deletions

diff --git a/chip/g/flash.c b/chip/g/flash.c
index 4829c986c8..7812bb52e3 100644
--- a/chip/g/flash.c
+++ b/chip/g/flash.c
@@ -464,6 +464,11 @@ void flash_open_ro_window(uint32_t offset, size_t size_b)
 	GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, WR_EN, 1);
 }
 
+void flash_close_ro_window(void)
+{
+	GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, WR_EN, 0);
+}
+
 #ifdef CR50_DEV
 /*
  * The seed is the first 32 bytes of the manufacture state space. That is all
diff --git a/chip/g/flash_info.h b/chip/g/flash_info.h
index 464eb70dbd..4267d8c4ea 100644
--- a/chip/g/flash_info.h
+++ b/chip/g/flash_info.h
@@ -36,6 +36,8 @@ void flash_info_write_disable(void);
 int flash_info_physical_write(int byte_offset, int num_bytes, const char *data);
 int flash_physical_info_read_word(int byte_offset, uint32_t *dst);
 
+/* Enable or disable write access to the backup RO section. */
 void flash_open_ro_window(uint32_t offset, size_t size_b);
+void flash_close_ro_window(void);
 
 #endif  /* ! __EC_CHIP_G_FLASH_INFO_H */
diff --git a/chip/g/upgrade.c b/chip/g/upgrade.c
index 03c6a0450a..ca0fe07a8f 100644
--- a/chip/g/upgrade.c
+++ b/chip/g/upgrade.c
@@ -36,6 +36,8 @@ static int header_restored(uint32_t offset)
 {
 	struct SignedHeader *header;
 	uint32_t new_size;
+	int rv;
+	bool ro_header;
 
 	header = (struct SignedHeader *)(CONFIG_PROGRAM_MEMORY_BASE + offset);
 
@@ -51,13 +53,20 @@ static int header_restored(uint32_t offset)
 	if (new_size > CONFIG_RW_SIZE)
 		return 0;
 
-	if ((offset == CONFIG_RO_MEM_OFF) || (offset == CHIP_RO_B_MEM_OFF))
+	ro_header = (offset == CONFIG_RO_MEM_OFF) ||
+		    (offset == CHIP_RO_B_MEM_OFF);
+	if (ro_header)
 		flash_open_ro_window(offset, sizeof(struct SignedHeader));
 
-	return flash_physical_write(offset + offsetof(struct SignedHeader,
-						      image_size),
-				    sizeof(header->image_size),
-				    (char *)&new_size) == EC_SUCCESS;
+	/* rv is set to TRUE on success. */
+	rv = flash_physical_write(offset + offsetof(struct SignedHeader,
+						    image_size),
+				  sizeof(header->image_size),
+				  (char *)&new_size) == EC_SUCCESS;
+	if (ro_header)
+		flash_close_ro_window();
+
+	return rv;
 }
 
 /*
diff --git a/chip/g/upgrade_fw.c b/chip/g/upgrade_fw.c
index f400b4f317..d78afb610d 100644
--- a/chip/g/upgrade_fw.c
+++ b/chip/g/upgrade_fw.c
@@ -114,13 +114,16 @@ static uint8_t check_update_chunk(uint32_t block_offset, size_t body_size)
 		if (block_offset == valid_sections.ro_base_offset) {
 			uint32_t base;
 			uint32_t size;
+			int rv;
 
 			base = valid_sections.ro_base_offset;
 			size = valid_sections.ro_top_offset -
 				valid_sections.ro_base_offset;
 			/* backup RO area write access needs to be enabled. */
 			flash_open_ro_window(base, size);
-			if (flash_physical_erase(base, size) != EC_SUCCESS) {
+			rv = flash_physical_erase(base, size);
+			flash_close_ro_window();
+			if (rv != EC_SUCCESS) {
 				CPRINTF("%s:%d erase failure of 0x%x..+0x%x\n",
 					__func__, __LINE__, base, size);
 				return UPGRADE_ERASE_FAILURE;
@@ -399,6 +402,7 @@ void fw_upgrade_command_handler(void *body,
 	uint8_t *error_code = body;  /* Cache the address for code clarity. */
 	size_t body_size;
 	uint32_t block_offset;
+	int rv;
 
 	*response_size = 1; /* One byte response unless this is a start PDU. */
 
@@ -506,8 +510,15 @@ void fw_upgrade_command_handler(void *body,
 	}
 
 	CPRINTF("at 0x%x\n", block_offset + CONFIG_PROGRAM_MEMORY_BASE);
-	if (flash_physical_write(block_offset, body_size, upgrade_data)
-	    != EC_SUCCESS) {
+	if (block_offset < valid_sections.ro_top_offset)
+		flash_open_ro_window(valid_sections.ro_base_offset,
+				     valid_sections.ro_top_offset -
+					     valid_sections.ro_base_offset);
+	rv = flash_physical_write(block_offset, body_size, upgrade_data);
+	if (block_offset < valid_sections.ro_top_offset)
+		flash_close_ro_window();
+
+	if (rv != EC_SUCCESS) {
 		*error_code = UPGRADE_WRITE_FAILURE;
 		CPRINTF("%s:%d upgrade write error\n",	__func__, __LINE__);
 		return;
diff --git a/common/ap_ro_integrity_check.c b/common/ap_ro_integrity_check.c
index 43c487cceb..0606415a62 100644
--- a/common/ap_ro_integrity_check.c
+++ b/common/ap_ro_integrity_check.c
@@ -142,6 +142,8 @@ static enum vendor_cmd_rc vc_seed_ap_ro_check(enum vendor_cmd_cc code,
 		rv = flash_physical_write(h1_flash_offset_ +
 						  sizeof(check_header),
 					  input_size, buf);
+	flash_close_ro_window();
+
 	if (rv != EC_SUCCESS) {
 		*response = ARCVE_FLASH_WRITE_FAILED;
 		return VENDOR_RC_WRITE_FLASH_FAIL;
@@ -258,6 +260,7 @@ static int ap_ro_info_cmd(int argc, char **argv)
 		 */
 		flash_open_ro_window(h1_flash_offset_, AP_RO_DATA_SPACE_SIZE);
 		flash_physical_erase(h1_flash_offset_, AP_RO_DATA_SPACE_SIZE);
+		flash_close_ro_window();
 	}
 #endif
 	if ((p_chk->header.num_ranges == (uint16_t)~0) &&