app_update: fix intermittent failure with firmware updates

Routine `spi_flash_cache2phys` sometimes return incorrect value,
resulting in failure in getting currently running ota partition.
This in turn aborts firmware update process.This issue was more
prominent with SPIRAM enabled cases.

Fix ensures proper cache guards during `spi_flash_cache2phys`, and
also for few other similar APIs.

In addition, `esp_ota_get_running_partition` has also been optimized
to save currently running partition for subsequent invocations.

Fixes https://github.com/espressif/esp-idf/issues/2451

diff --git a/components/app_update/esp_ota_ops.c b/components/app_update/esp_ota_ops.c
index 5e4dcb424822ba73fab4985e1c51d618ca750327..0fbd5ace9f822d9b3c0c5fad1e6d07b358fc6135 100644 (file)
--- a/components/app_update/esp_ota_ops.c
+++ b/components/app_update/esp_ota_ops.c
@@ -494,9 +494,18 @@ const esp_partition_t *esp_ota_get_boot_partition(void)
 
 const esp_partition_t* esp_ota_get_running_partition(void)
 {
+    static const esp_partition_t *curr_partition = NULL;
+
+    /*
+     * Currently running partition is unlikely to change across reset cycle,
+     * so it can be cached here, and avoid lookup on every flash write operation.
+     */
+    if (curr_partition != NULL) {
+        return curr_partition;
+    }
+
     /* Find the flash address of this exact function. By definition that is part
        of the currently running firmware. Then find the enclosing partition. */
-
     size_t phys_offs = spi_flash_cache2phys(esp_ota_get_running_partition);
 
     assert (phys_offs != SPI_FLASH_CACHE2PHYS_FAIL); /* indicates cache2phys lookup is buggy */
@@ -510,6 +519,7 @@ const esp_partition_t* esp_ota_get_running_partition(void)
         const esp_partition_t *p = esp_partition_get(it);
         if (p->address <= phys_offs && p->address + p->size > phys_offs) {
             esp_partition_iterator_release(it);
+            curr_partition = p;
             return p;
         }
         it = esp_partition_next(it);

diff --git a/components/esp32/include/esp_attr.h b/components/esp32/include/esp_attr.h
index 72f13d364b758c8830a0b9e8beee3626b4dcba5d..6d2d5ea84d45dce799c73aca8348bf04d30f4990 100644 (file)
--- a/components/esp32/include/esp_attr.h
+++ b/components/esp32/include/esp_attr.h
@@ -68,4 +68,7 @@
 // after restart or during a deep sleep / wake cycle.
 #define RTC_NOINIT_ATTR  __attribute__((section(".rtc_noinit")))
 
+// Forces to not inline function
+#define NOINLINE_ATTR __attribute__((noinline))
+
 #endif /* __ESP_ATTR_H__ */

diff --git a/components/spi_flash/flash_mmap.c b/components/spi_flash/flash_mmap.c
index f4249799dc653c5ab34beef68388d83be1fdd8d3..d649a74603b89c0b56711802e69e3a23da6b84a9 100644 (file)
--- a/components/spi_flash/flash_mmap.c
+++ b/components/spi_flash/flash_mmap.c
@@ -286,23 +286,41 @@ void IRAM_ATTR spi_flash_munmap(spi_flash_mmap_handle_t handle)
     free(it);
 }
 
-void spi_flash_mmap_dump()
+static void IRAM_ATTR NOINLINE_ATTR spi_flash_protected_mmap_init()
 {
+    spi_flash_disable_interrupts_caches_and_other_cpu();
     spi_flash_mmap_init();
+    spi_flash_enable_interrupts_caches_and_other_cpu();
+}
+
+static uint32_t IRAM_ATTR NOINLINE_ATTR spi_flash_protected_read_mmu_entry(int index)
+{
+    uint32_t value;
+    spi_flash_disable_interrupts_caches_and_other_cpu();
+    value = DPORT_REG_READ((uint32_t)&DPORT_PRO_FLASH_MMU_TABLE[index]);
+    spi_flash_enable_interrupts_caches_and_other_cpu();
+    return value;
+}
+
+void spi_flash_mmap_dump()
+{
+    spi_flash_protected_mmap_init();
+
     mmap_entry_t* it;
     for (it = LIST_FIRST(&s_mmap_entries_head); it != NULL; it = LIST_NEXT(it, entries)) {
         printf("handle=%d page=%d count=%d\n", it->handle, it->page, it->count);
     }
     for (int i = 0; i < REGIONS_COUNT * PAGES_PER_REGION; ++i) {
         if (s_mmap_page_refcnt[i] != 0) {
-            printf("page %d: refcnt=%d paddr=%d\n",
-                    i, (int) s_mmap_page_refcnt[i], DPORT_REG_READ((uint32_t)&DPORT_PRO_FLASH_MMU_TABLE[i]));
+            uint32_t paddr = spi_flash_protected_read_mmu_entry(i);
+            printf("page %d: refcnt=%d paddr=%d\n", i, (int) s_mmap_page_refcnt[i], paddr);
         }
     }
 }
 
-uint32_t spi_flash_mmap_get_free_pages(spi_flash_mmap_memory_t memory)
+uint32_t IRAM_ATTR spi_flash_mmap_get_free_pages(spi_flash_mmap_memory_t memory)
 {
+    spi_flash_disable_interrupts_caches_and_other_cpu();
     spi_flash_mmap_init();
     int count = 0;
     int region_begin;   // first page to check
@@ -315,7 +333,8 @@ uint32_t spi_flash_mmap_get_free_pages(spi_flash_mmap_memory_t memory)
             count++;
         }
     }
-   DPORT_INTERRUPT_RESTORE();
+    DPORT_INTERRUPT_RESTORE();
+    spi_flash_enable_interrupts_caches_and_other_cpu();
     return count;
 }
 
@@ -384,7 +403,6 @@ static inline IRAM_ATTR bool update_written_pages(size_t start_addr, size_t leng
     return false;
 }
 
-
 uint32_t spi_flash_cache2phys(const void *cached)
 {
     intptr_t c = (intptr_t)cached;
@@ -405,7 +423,7 @@ uint32_t spi_flash_cache2phys(const void *cached)
         /* cached address was not in IROM or DROM */
         return SPI_FLASH_CACHE2PHYS_FAIL;
     }
-    uint32_t phys_page = DPORT_REG_READ((uint32_t)&DPORT_PRO_FLASH_MMU_TABLE[cache_page]);
+    uint32_t phys_page = spi_flash_protected_read_mmu_entry(cache_page);
     if (phys_page == INVALID_ENTRY_VAL) {
         /* page is not mapped */
         return SPI_FLASH_CACHE2PHYS_FAIL;
@@ -414,8 +432,7 @@ uint32_t spi_flash_cache2phys(const void *cached)
     return phys_offs | (c & (SPI_FLASH_MMU_PAGE_SIZE-1));
 }
 
-
-const void *spi_flash_phys2cache(uint32_t phys_offs, spi_flash_mmap_memory_t memory)
+const void *IRAM_ATTR spi_flash_phys2cache(uint32_t phys_offs, spi_flash_mmap_memory_t memory)
 {
     uint32_t phys_page = phys_offs / SPI_FLASH_MMU_PAGE_SIZE;
     int start, end, page_delta;
@@ -432,15 +449,18 @@ const void *spi_flash_phys2cache(uint32_t phys_offs, spi_flash_mmap_memory_t mem
         base = VADDR1_START_ADDR;
         page_delta = 64;
     }
+    spi_flash_disable_interrupts_caches_and_other_cpu();
     DPORT_INTERRUPT_DISABLE();
     for (int i = start; i < end; i++) {
         if (DPORT_SEQUENCE_REG_READ((uint32_t)&DPORT_PRO_FLASH_MMU_TABLE[i]) == phys_page) {
             i -= page_delta;
             intptr_t cache_page =  base + (SPI_FLASH_MMU_PAGE_SIZE * i);
             DPORT_INTERRUPT_RESTORE();
+            spi_flash_enable_interrupts_caches_and_other_cpu();
             return (const void *) (cache_page | (phys_offs & (SPI_FLASH_MMU_PAGE_SIZE-1)));
         }
     }
     DPORT_INTERRUPT_RESTORE();
+    spi_flash_enable_interrupts_caches_and_other_cpu();
     return NULL;
 }

diff --git a/components/spi_flash/include/esp_spi_flash.h b/components/spi_flash/include/esp_spi_flash.h
index 7977e116fed87f70a42fad5f36f6ad0809882cf5..aa6eeea7539d64b86af88c83e3687760d85f9b0b 100644 (file)
--- a/components/spi_flash/include/esp_spi_flash.h
+++ b/components/spi_flash/include/esp_spi_flash.h
@@ -244,15 +244,13 @@ void spi_flash_mmap_dump();
 /**
  * @brief get free pages number which can be mmap
  *
- * This function will return free page number of the mmu table which can mmap,
- * when you want to call spi_flash_mmap to mmap an ranger of flash data to Dcache or Icache
- * memmory region, maybe the size of  MMU table will exceed,so if you are not sure the
- * size need mmap is ok, can call the interface and watch how many MMU table page can be
- * mmaped.
+ * This function will return number of free pages available in mmu table. This could be useful
+ * before calling actual spi_flash_mmap (maps flash range to DCache or ICache memory) to check
+ * if there is sufficient space available for mapping.
  *
- * @param memory  memmory type of MMU table free page
+ * @param memory memory type of MMU table free page
  *
- * @return  number of free pages which can be mmaped
+ * @return number of free pages which can be mmaped
  */
 uint32_t spi_flash_mmap_get_free_pages(spi_flash_mmap_memory_t memory);