]> granicus.if.org Git - esp-idf/commitdiff
esp_flash: fix the coredump issue
authorMichael (XIAO Xufeng) <xiaoxufeng@espressif.com>
Wed, 11 Sep 2019 18:41:00 +0000 (02:41 +0800)
committerMichael (XIAO Xufeng) <xiaoxufeng@espressif.com>
Sat, 14 Sep 2019 09:01:36 +0000 (17:01 +0800)
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
components/soc/include/hal/spi_flash_types.h
components/spi_flash/esp_flash_api.c
components/spi_flash/esp_flash_spi_init.c
components/spi_flash/include/esp_flash.h
components/spi_flash/include/esp_flash_internal.h
components/spi_flash/include/memspi_host_driver.h
components/spi_flash/memspi_host_driver.c
components/spi_flash/spi_flash_os_func_app.c
components/spi_flash/spi_flash_os_func_noos.c

index 21e85731e09a1956f08170d890ca3c00c4537bdf..5705e94bb818b37330cd064530f973af29e77148 100644 (file)
@@ -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) 
+void esp_core_dump_flash_init(void)
 {
     const esp_partition_t *core_part;
 
@@ -214,10 +215,9 @@ void esp_core_dump_to_flash(XtExcFrame *frame)
         return;
     }
 
-#if CONFIG_SPI_FLASH_USE_LEGACY_IMPL
     // init non-OS flash access critical section
     spi_flash_guard_set(&g_flash_guard_no_os_ops);
-#endif
+    esp_flash_app_disable_protect(true);
 
     memset(&wr_cfg, 0, sizeof(wr_cfg));
     wr_cfg.prepare = esp_core_dump_flash_write_prepare;
index 7babeee0c93ee7b6bd9e92f97ef8d0c4e84c4772..c19eee1ed066dae5fc6674f3819fad106f0f8ced 100644 (file)
@@ -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
index 7f29d86fc4ebb9158e52cc7c3e708ef740023476..b062619dcd42a9771fc6ed5476d0be507989d24b 100644 (file)
@@ -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)
@@ -616,6 +617,17 @@ esp_err_t IRAM_ATTR esp_flash_read_encrypted(esp_flash_t *chip, uint32_t address
     return spi_flash_read_encrypted(address, out_buffer, length);
 }
 
+#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
 ------------------------------------------------------------------------------*/
index ba0d26b567363b567f72c1afac75e678bb246235..9938ab3fbeaf263a2ed6ecd95d2f8941abe8b2c4 100644 (file)
@@ -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(void)
 
 esp_err_t esp_flash_app_init(void)
 {
-    return esp_flash_init_os_functions(&default_chip, 0);
+    return esp_flash_app_init_os_functions(&default_chip);
 }
 
 #endif
index 77ffe31f58c08cc7fd00aa3dbf80942b04d2eabc..caca0759602bf87dd1780af3f0f5661162db2f60 100644 (file)
@@ -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;
index b0d35f371c915ec369e1e63583b2d75f771ac5f0..0496f5649c40c97fcac394b8ec64e3cdf4212505 100644 (file)
@@ -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
 }
index 434361d9700004ccd91c5d848d1a8f279c8898d6..cb0f3e9d9e7d70596f896478b53979f838297119 100644 (file)
@@ -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);
\ No newline at end of file
index 871616386fc596c68565aea13e1d0abf8c864a19..85b31998a03a22176efea99aab46922509265bcb 100644 (file)
@@ -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;
 }
\ No newline at end of file
index b2e13c8e44625c27a3eb60e94ccfd8f93db1c4c3..ad97c433c5a634310258ce156e598552165e2dd2 100644 (file)
@@ -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;
+}
+
 
index 4b494279b0cbad1a4c5ce0ad8fcc9defc5c4534e..910633f9b5cf8396550dab9410bf840e98e19bf3 100644 (file)
 #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;
+}