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>
(cherry picked from commit 160f0f9f632e5a64527753d8c17399ab1ae0ff2b)

Change-Id: I2e15e3b960fa57e29f7a566298cfe8ed5ab14ef7
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2242511
Tested-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-by: Mary Ruthven <mruthven@chromium.org>
Commit-Queue: Mary Ruthven <mruthven@chromium.org>
(cherry picked from commit 7862370635beccf577c088950d1867aa821bffc6)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2261307

5 files changed, 38 insertions, 8 deletions

diff --git a/chip/g/flash.c b/chip/g/flash.c
index 464ba42b8f..f9ce5d580e 100644
--- a/chip/g/flash.c
+++ b/chip/g/flash.c
@@ -462,3 +462,8 @@ void flash_open_ro_window(uint32_t offset, size_t size_b)
 	GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, RD_EN, 1);
 	GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, WR_EN, 1);
 }
+
+void flash_close_ro_window(void)
+{
+	GWRITE_FIELD(GLOBALSEC, FLASH_REGION6_CTRL, WR_EN, 0);
+}
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 35d0c54549..7f15574e3b 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) &&