esp_flash: fix the coredump issue

During coredump, dangerous-area-checking should be disabled, and cache
disabling should be replaced by a safer version.

Dangerous-area-checking used to be in the HAL, but it seems to be more
fit to os functions. So it's moved to os functions. Interfaces are
provided to switch between os functions during coredump.

components/espcoredump/src/core_dump_flash.c

@@ -15,6 +15,7 @@
 #include "esp32/rom/crc.h"
 #include "esp_partition.h"
 #include "esp_core_dump_priv.h"
+#include "esp_flash_internal.h"
 
 const static DRAM_ATTR char TAG[] __attribute__((unused)) = "esp_core_dump_flash";
 
@@ -50,7 +51,7 @@ static inline core_dump_crc_t esp_core_dump_calc_flash_config_crc(void)
     return crc32_le(0, (uint8_t const *)&s_core_flash_config.partition, sizeof(s_core_flash_config.partition));
 }
 
-void esp_core_dump_flash_init() 
+void esp_core_dump_flash_init(void)
 {
     const esp_partition_t *core_part;
 
@@ -216,6 +217,7 @@ void esp_core_dump_to_flash(XtExcFrame *frame)
 
     /* init non-OS flash access critical section */
     spi_flash_guard_set(&g_flash_guard_no_os_ops);
+    esp_flash_app_disable_protect(true);
 
     memset(&wr_cfg, 0, sizeof(wr_cfg));
     wr_cfg.prepare = esp_core_dump_flash_write_prepare;

components/soc/include/hal/spi_flash_types.h

@@ -142,11 +142,6 @@ struct spi_flash_host_driver_t {
      * modified, the cache needs to be flushed. Left NULL if not supported.
      */
     esp_err_t (*flush_cache)(spi_flash_host_driver_t* driver, uint32_t addr, uint32_t size);
-    /**
-     * Check if the given region is protected (e.g. is the bootloader). Left
-     * NULL if current host doesn't need protection.
-     */
-    bool (*region_protected)(spi_flash_host_driver_t* driver, uint32_t addr, uint32_t size);
 };
 
 #ifdef __cplusplus

components/spi_flash/esp_flash_api.c

@@ -22,6 +22,7 @@
 #include "esp_log.h"
 #include "sdkconfig.h"
 #include "esp_heap_caps.h"
+#include "esp_flash_internal.h"
 
 static const char TAG[] = "spi_flash";
 
@@ -42,7 +43,7 @@ static const char TAG[] = "spi_flash";
 #define CHECK_WRITE_ADDRESS(CHIP, ADDR, SIZE)
 #else /* FAILS or ABORTS */
 #define CHECK_WRITE_ADDRESS(CHIP, ADDR, SIZE) do {                            \
-        if (CHIP && CHIP->host->region_protected && CHIP->host->region_protected(CHIP->host, ADDR, SIZE)) {                       \
+        if (CHIP && CHIP->os_func->region_protected && CHIP->os_func->region_protected(CHIP->os_func_data, ADDR, SIZE)) {                       \
             UNSAFE_WRITE_ADDRESS;                                 \
         }                                                               \
     } while(0)
@@ -600,6 +601,17 @@ inline static IRAM_ATTR bool regions_overlap(uint32_t a_start, uint32_t a_len,ui
 }
 
 
+#ifndef CONFIG_SPI_FLASH_USE_LEGACY_IMPL
+esp_err_t esp_flash_app_disable_protect(bool disable)
+{
+    if (disable) {
+        return esp_flash_app_disable_os_functions(esp_flash_default_chip);
+    } else {
+        return esp_flash_app_init_os_functions(esp_flash_default_chip);
+    }
+}
+#endif
+
 /*------------------------------------------------------------------------------
     Adapter layer to original api before IDF v4.0
 ------------------------------------------------------------------------------*/

components/spi_flash/esp_flash_spi_init.c

@@ -22,6 +22,7 @@
 #include "esp_heap_caps.h"
 #include "hal/spi_types.h"
 #include "driver/spi_common.h"
+#include "esp_flash_internal.h"
 
 __attribute__((unused)) static const char TAG[] = "spi_flash";
 
@@ -207,7 +208,7 @@ esp_err_t esp_flash_init_default_chip()
 
 esp_err_t esp_flash_app_init()
 {
-    return esp_flash_init_os_functions(&default_chip, 0);
+    return esp_flash_app_init_os_functions(&default_chip);
 }
 
 #endif

components/spi_flash/include/esp_flash.h

@@ -45,6 +45,9 @@ typedef struct {
     /** Called after completing any flash operation. */
     esp_err_t (*end)(void *arg);
 
+    /** Called before any erase/write operations to check whether the region is limited by the OS */
+    esp_err_t (*region_protected)(void* arg, size_t start_addr, size_t size);
+
     /** Delay for at least 'ms' milliseconds. Called in between 'start' and 'end'. */
     esp_err_t (*delay_ms)(void *arg, unsigned ms);
 } esp_flash_os_functions_t;

components/spi_flash/include/esp_flash_internal.h

@@ -50,6 +50,19 @@ esp_err_t esp_flash_init_default_chip(void);
 esp_err_t esp_flash_app_init(void);
 #endif
 
+/**
+ *  Disable OS-level SPI flash protections in IDF
+ *
+ *  Called by the IDF internal code (e.g. coredump). You do not need to call this in your own applications.
+ *
+ * @return always ESP_OK.
+ */
+#ifdef CONFIG_SPI_FLASH_USE_LEGACY_IMPL
+#define esp_flash_app_disable_protect(...) ({ESP_OK;})
+#else
+esp_err_t esp_flash_app_disable_protect(bool disable);
+#endif
+
 /**
  *  Initialize OS-level functions for a specific chip.
  *
@@ -62,6 +75,25 @@ esp_err_t esp_flash_app_init(void);
  */
 esp_err_t esp_flash_init_os_functions(esp_flash_t *chip, int host_id);
 
+/**
+ *  Initialize OS-level functions for the main flash chip.
+ *
+ * @param chip The chip to init os functions. Only pointer to the default chip is supported now.
+ *
+ * @return always ESP_OK
+ */
+esp_err_t esp_flash_app_init_os_functions(esp_flash_t* chip);
+
+/**
+ *  Disable OS-level functions for the main flash chip during special phases (e.g. coredump)
+ *
+ * @param chip The chip to init os functions. Only "esp_flash_default_chip" is supported now.
+ *
+ * @return always ESP_OK
+ */
+esp_err_t esp_flash_app_disable_os_functions(esp_flash_t* chip);
+
+
 
 #ifdef __cplusplus
 }

components/spi_flash/include/memspi_host_driver.h

@@ -35,7 +35,6 @@
         .configure_host_read_mode = spi_flash_hal_configure_host_read_mode, \
         .poll_cmd_done = spi_flash_hal_poll_cmd_done, \
         .flush_cache = memspi_host_flush_cache, \
-        .region_protected = memspi_region_protected, \
 }
 
 /// configuration for the memspi host
@@ -100,14 +99,3 @@ esp_err_t memspi_host_read_status_hs(spi_flash_host_driver_t *driver, uint8_t *o
  * @return always ESP_OK.
  */
 esp_err_t memspi_host_flush_cache(spi_flash_host_driver_t* driver, uint32_t addr, uint32_t size);
-
-/**
- * Check if the given region is protected.
- *
- * @param driver The driver context.
- * @param addr Start address of the region.
- * @param size Size of the region to check.
- *
- * @return true if protected, otherwise false.
- */
-bool memspi_region_protected(spi_flash_host_driver_t* driver, uint32_t addr, uint32_t size);

components/spi_flash/memspi_host_driver.c

@@ -34,7 +34,6 @@ esp_err_t memspi_host_init_pointers(spi_flash_host_driver_t *host, memspi_host_d
     //some functions are not required if not SPI1
     if (data->spi != &SPI1) {
         host->flush_cache = NULL;
-        host->region_protected = NULL;
     }
     return ESP_OK;
 }
@@ -87,15 +86,4 @@ esp_err_t memspi_host_flush_cache(spi_flash_host_driver_t* driver, uint32_t addr
         spi_flash_check_and_flush_cache(addr, size);
     }
     return ESP_OK;
-}
-
-bool memspi_region_protected(spi_flash_host_driver_t* driver, uint32_t addr, uint32_t size)
-{
-    if (((memspi_host_data_t*)(driver->driver_data))->spi != &SPI1) {
-        return false;
-    }
-    if (!esp_partition_main_flash_region_safe(addr, size)) {
-        return true;
-    }
-    return false;
 }

components/spi_flash/spi_flash_os_func_app.c

@@ -17,6 +17,8 @@
 #include "esp_attr.h"
 #include "esp_spi_flash.h"   //for ``g_flash_guard_default_ops``
 #include "esp_flash.h"
+#include "esp_flash_partitions.h"
+
 
 /*
  * OS functions providing delay service and arbitration among chips, and with the cache.
@@ -29,6 +31,11 @@ typedef struct {
     int host_id;
 } app_func_arg_t;
 
+typedef struct {
+    int host_id;
+    bool no_protect;    //to decide whether to check protected region (for the main chip) or not.
+} spi1_app_func_arg_t;
+
 // in the future we will have arbitration among devices, including flash on the same flash bus
 static IRAM_ATTR esp_err_t spi_bus_acquire(int host_id)
 {
@@ -45,7 +52,7 @@ static IRAM_ATTR esp_err_t spi1_start(void *arg)
 {
     g_flash_guard_default_ops.start();
 
-    spi_bus_acquire(((app_func_arg_t *)arg)->host_id);
+    spi_bus_acquire(((spi1_app_func_arg_t *)arg)->host_id);
 
     return ESP_OK;
 }
@@ -53,7 +60,7 @@ static IRAM_ATTR esp_err_t spi1_end(void *arg)
 {
     g_flash_guard_default_ops.end();
 
-    spi_bus_release(((app_func_arg_t *)arg)->host_id);
+    spi_bus_release(((spi1_app_func_arg_t *)arg)->host_id);
 
     return ESP_OK;
 }
@@ -76,8 +83,24 @@ static IRAM_ATTR esp_err_t delay_ms(void *arg, unsigned ms)
     return ESP_OK;
 }
 
-static DRAM_ATTR app_func_arg_t spi1_arg = {
+static IRAM_ATTR esp_err_t main_flash_region_protected(void* arg, size_t start_addr, size_t size)
+{
+    if (((spi1_app_func_arg_t*)arg)->no_protect || esp_partition_main_flash_region_safe(start_addr, size)) {
+        //ESP_OK = 0, also means protected==0
+        return ESP_OK;
+    } else {
+        return ESP_ERR_NOT_SUPPORTED;
+    }
+}
+
+static DRAM_ATTR spi1_app_func_arg_t spi1_arg = {
     .host_id = 0,   //for SPI1,
+    .no_protect = true,
+};
+
+static DRAM_ATTR spi1_app_func_arg_t main_flash_arg = {
+    .host_id = 0,   //for SPI1,
+    .no_protect = false,
 };
 
 static app_func_arg_t spi2_arg = {
@@ -93,6 +116,7 @@ const DRAM_ATTR esp_flash_os_functions_t esp_flash_spi1_default_os_functions = {
     .start = spi1_start,
     .end = spi1_end,
     .delay_ms = delay_ms,
+    .region_protected = main_flash_region_protected,
 };
 
 const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = {
@@ -117,4 +141,11 @@ esp_err_t esp_flash_init_os_functions(esp_flash_t *chip, int host_id)
     return ESP_OK;
 }
 
+esp_err_t esp_flash_app_init_os_functions(esp_flash_t* chip)
+{
+    chip->os_func = &esp_flash_spi1_default_os_functions;
+    chip->os_func_data = &main_flash_arg;
+    return ESP_OK;
+}
+
 

components/spi_flash/spi_flash_os_func_noos.c

@@ -20,13 +20,15 @@
 #include "esp_attr.h"
 
 
-static esp_err_t start(void *arg)
+
+static IRAM_ATTR esp_err_t start(void *arg)
 {
     Cache_Read_Disable(0);
     Cache_Read_Disable(1);
     return ESP_OK;
 }
-static esp_err_t end(void *arg)
+
+static IRAM_ATTR esp_err_t end(void *arg)
 {
     Cache_Flush(0);
     Cache_Flush(1);
@@ -35,14 +37,21 @@ static esp_err_t end(void *arg)
     return ESP_OK;
 }
 
-static esp_err_t delay_ms(void *arg, unsigned ms)
+static IRAM_ATTR esp_err_t delay_ms(void *arg, unsigned ms)
 {
     ets_delay_us(1000 * ms);
     return ESP_OK;
 }
 
-const esp_flash_os_functions_t esp_flash_noos_functions = {
+const DRAM_ATTR esp_flash_os_functions_t esp_flash_noos_functions = {
     .start = start,
     .end = end,
     .delay_ms = delay_ms,
+    .region_protected = NULL,
 };
+
+esp_err_t IRAM_ATTR esp_flash_app_disable_os_functions(esp_flash_t* chip)
+{
+    chip->os_func = &esp_flash_noos_functions;
+    return ESP_OK;
+}