Merge branch 'bugfix/incorrect_time_after_ota_v5.1' into 'release/v5.1'

esp_hw_support: Fix invalid system time if s_esp_rtc_time_us & s_rtc_last_ticks were moved around (v5.1)

See merge request espressif/esp-idf!24408

components/bootloader_support/.build-test-rules.yml

@@ -5,7 +5,3 @@ components/bootloader_support/test_apps/rtc_custom_section:
     - if: IDF_TARGET == "esp32c2"
       temporary: false
       reason: esp32c2 does not have RTC memory
-  disable_test:
-    - if: IDF_TARGET == "esp32h2"
-      temporary: true
-      reason: target esp32h2 is not supported yet

components/bootloader_support/src/bootloader_common_loader.c

@@ -121,52 +121,38 @@ int bootloader_common_select_otadata(const esp_ota_select_entry_t *two_otadata,
 
 #if CONFIG_BOOTLOADER_RESERVE_RTC_MEM
 
-#define RTC_RETAIN_MEM_ADDR (SOC_RTC_DRAM_HIGH - sizeof(rtc_retain_mem_t))
-
-_Static_assert(RTC_RETAIN_MEM_ADDR >= SOC_RTC_DRAM_LOW, "rtc_retain_mem_t structure size is bigger than the RTC memory size. Consider reducing RTC reserved memory size.");
-
-rtc_retain_mem_t *const rtc_retain_mem = (rtc_retain_mem_t *)RTC_RETAIN_MEM_ADDR;
-
-#ifndef BOOTLOADER_BUILD
-#include "heap_memory_layout.h"
-/* The app needs to be told this memory is reserved, important if configured to use RTC memory as heap.
-
-   Note that keeping this macro here only works when other symbols in this file are referenced by the app, as
-   this feature is otherwise 100% part of the bootloader. However this seems to happen in all apps.
- */
-SOC_RESERVE_MEMORY_REGION(RTC_RETAIN_MEM_ADDR, RTC_RETAIN_MEM_ADDR + sizeof(rtc_retain_mem_t), rtc_retain_mem);
-#endif
-
 static uint32_t rtc_retain_mem_size(void) {
 #ifdef CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC
     /* A custom memory has been reserved by the user, do not consider this memory into CRC calculation as it may change without
      * the have the user updating the CRC. Return the offset of the custom field, which is equivalent to size of the structure
      * minus the size of everything after (including) `custom` */
     return offsetof(rtc_retain_mem_t, custom);
 #else
-    return sizeof(rtc_retain_mem_t) - sizeof(rtc_retain_mem->crc);
+    return sizeof(rtc_retain_mem_t) - sizeof(bootloader_common_get_rtc_retain_mem()->crc);
 #endif
 }
 
 static bool is_retain_mem_valid(void)
 {
+    rtc_retain_mem_t* rtc_retain_mem = bootloader_common_get_rtc_retain_mem();
     return esp_rom_crc32_le(UINT32_MAX, (uint8_t*)rtc_retain_mem, rtc_retain_mem_size()) == rtc_retain_mem->crc && rtc_retain_mem->crc != UINT32_MAX;
 }
 
 static void update_rtc_retain_mem_crc(void)
 {
+    rtc_retain_mem_t* rtc_retain_mem = bootloader_common_get_rtc_retain_mem();
     rtc_retain_mem->crc = esp_rom_crc32_le(UINT32_MAX, (uint8_t*)rtc_retain_mem, rtc_retain_mem_size());
 }
 
 NOINLINE_ATTR void bootloader_common_reset_rtc_retain_mem(void)
 {
-    hal_memset(rtc_retain_mem, 0, sizeof(rtc_retain_mem_t));
+    hal_memset(bootloader_common_get_rtc_retain_mem(), 0, sizeof(rtc_retain_mem_t));
 }
 
 uint16_t bootloader_common_get_rtc_retain_mem_reboot_counter(void)
 {
     if (is_retain_mem_valid()) {
-        return rtc_retain_mem->reboot_counter;
+        return bootloader_common_get_rtc_retain_mem()->reboot_counter;
     }
     return 0;
 }
@@ -176,12 +162,13 @@ void bootloader_common_set_rtc_retain_mem_factory_reset_state(void)
     if (!is_retain_mem_valid()) {
         bootloader_common_reset_rtc_retain_mem();
     }
-    rtc_retain_mem->flags.factory_reset_state = true;
+    bootloader_common_get_rtc_retain_mem()->flags.factory_reset_state = true;
     update_rtc_retain_mem_crc();
 }
 
 bool bootloader_common_get_rtc_retain_mem_factory_reset_state(void)
 {
+    rtc_retain_mem_t* rtc_retain_mem = bootloader_common_get_rtc_retain_mem();
     if (is_retain_mem_valid()) {
         bool factory_reset_state = rtc_retain_mem->flags.factory_reset_state;
         if (factory_reset_state == true) {
@@ -196,13 +183,14 @@ bool bootloader_common_get_rtc_retain_mem_factory_reset_state(void)
 esp_partition_pos_t* bootloader_common_get_rtc_retain_mem_partition(void)
 {
     if (is_retain_mem_valid()) {
-        return &rtc_retain_mem->partition;
+        return &bootloader_common_get_rtc_retain_mem()->partition;
     }
     return NULL;
 }
 
 void bootloader_common_update_rtc_retain_mem(esp_partition_pos_t* partition, bool reboot_counter)
 {
+    rtc_retain_mem_t* rtc_retain_mem = bootloader_common_get_rtc_retain_mem();
     if (reboot_counter) {
         if (!is_retain_mem_valid()) {
             bootloader_common_reset_rtc_retain_mem();
@@ -224,7 +212,14 @@ void bootloader_common_update_rtc_retain_mem(esp_partition_pos_t* partition, boo
 
 rtc_retain_mem_t* bootloader_common_get_rtc_retain_mem(void)
 {
-    return rtc_retain_mem;
+#ifdef BOOTLOADER_BUILD
+    #define RTC_RETAIN_MEM_ADDR (SOC_RTC_DRAM_HIGH - sizeof(rtc_retain_mem_t))
+    static rtc_retain_mem_t *const s_bootloader_retain_mem = (rtc_retain_mem_t *)RTC_RETAIN_MEM_ADDR;
+    return s_bootloader_retain_mem;
+#else
+    static __attribute__((section(".bootloader_data_rtc_mem"))) rtc_retain_mem_t s_bootloader_retain_mem;
+    return &s_bootloader_retain_mem;
+#endif // !BOOTLOADER_BUILD
 }
 
 #endif // CONFIG_BOOTLOADER_RESERVE_RTC_MEM

components/bootloader_support/test_apps/rtc_custom_section/pytest_rtc_mem.py

@@ -9,6 +9,7 @@
 @pytest.mark.esp32
 @pytest.mark.esp32c3
 @pytest.mark.esp32c6
+@pytest.mark.esp32h2
 @pytest.mark.esp32s2
 @pytest.mark.esp32s3
 def test_rtc_reserved_memory(dut: Dut) -> None:

components/esp_hw_support/esp_clk.c

@@ -1,10 +1,11 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #include <stdint.h>
+#include <string.h>
 #include <sys/param.h>
 #include <sys/lock.h>
 
@@ -47,9 +48,30 @@ extern uint32_t g_ticks_per_us_pro;
 
 static portMUX_TYPE s_esp_rtc_time_lock = portMUX_INITIALIZER_UNLOCKED;
 
-#if SOC_RTC_FAST_MEM_SUPPORTED
-static RTC_NOINIT_ATTR uint64_t s_esp_rtc_time_us, s_rtc_last_ticks;
-#endif
+#if SOC_RTC_MEM_SUPPORTED
+typedef struct {
+    uint64_t rtc_time_us;
+    uint64_t rtc_last_ticks;
+    uint32_t reserve;
+    uint32_t checksum;
+} retain_mem_t;
+_Static_assert(sizeof(retain_mem_t) == 24, "retain_mem_t must be 24 bytes");
+_Static_assert(offsetof(retain_mem_t, checksum) == sizeof(retain_mem_t) - sizeof(uint32_t), "Wrong offset for checksum field in retain_mem_t structure");
+
+static __attribute__((section(".rtc_timer_data_in_rtc_mem"))) retain_mem_t s_rtc_timer_retain_mem;
+
+static uint32_t calc_checksum(void)
+{
+    uint32_t checksum = 0;
+    uint32_t *data = (uint32_t*) &s_rtc_timer_retain_mem;
+
+    for (uint32_t i = 0; i < (sizeof(retain_mem_t) - sizeof(s_rtc_timer_retain_mem.checksum)) / 4; i++) {
+        checksum = ((checksum << 5) - checksum) ^ data[i];
+    }
+    return checksum;
+}
+#define IS_RETAIN_MEM_VALID() (s_rtc_timer_retain_mem.checksum == calc_checksum())
+#endif // SOC_RTC_MEM_SUPPORTED
 
 inline static int IRAM_ATTR s_get_cpu_freq_mhz(void)
 {
@@ -84,13 +106,24 @@ uint64_t esp_rtc_get_time_us(void)
 {
     portENTER_CRITICAL_SAFE(&s_esp_rtc_time_lock);
     const uint32_t cal = esp_clk_slowclk_cal_get();
-#if SOC_RTC_FAST_MEM_SUPPORTED
-    if (cal == 0) {
-        s_esp_rtc_time_us = 0;
-        s_rtc_last_ticks = 0;
+#if SOC_RTC_MEM_SUPPORTED
+    static bool first_call = true;
+    if (cal == 0 || (first_call && !IS_RETAIN_MEM_VALID())) {
+        /*
+            If cal is 0, then this is the first power-up. Cal is keeping valid
+            after reboot and deepsleep. If s_rtc_timer_retain_mem is invalid, it means
+            that something unexpected happened (the structure was moved around
+            after OTA update). To keep the system time valid even after OTA we
+            reset the s_rtc_timer_retain_mem. But the resetting can also lead to some
+            drift of the system time, because only the last current calibration
+            value will be applied to all rtc ticks. To mitigate this effect you
+            might need updating of the system time (via SNTP).
+        */
+        memset(&s_rtc_timer_retain_mem, 0, sizeof(retain_mem_t));
     }
+    first_call = false;
     const uint64_t rtc_this_ticks = rtc_time_get();
-    const uint64_t ticks = rtc_this_ticks - s_rtc_last_ticks;
+    const uint64_t ticks = rtc_this_ticks - s_rtc_timer_retain_mem.rtc_last_ticks;
 #else
     const uint64_t ticks = rtc_time_get();
 #endif
@@ -109,11 +142,13 @@ uint64_t esp_rtc_get_time_us(void)
     const uint64_t ticks_high = ticks >> 32;
     const uint64_t delta_time_us = ((ticks_low * cal) >> RTC_CLK_CAL_FRACT) +
                                    ((ticks_high * cal) << (32 - RTC_CLK_CAL_FRACT));
-#if SOC_RTC_FAST_MEM_SUPPORTED
-    s_esp_rtc_time_us += delta_time_us;
-    s_rtc_last_ticks = rtc_this_ticks;
+#if SOC_RTC_MEM_SUPPORTED
+    s_rtc_timer_retain_mem.rtc_time_us += delta_time_us;
+    s_rtc_timer_retain_mem.rtc_last_ticks = rtc_this_ticks;
+    s_rtc_timer_retain_mem.checksum = calc_checksum();
+    uint64_t esp_rtc_time_us = s_rtc_timer_retain_mem.rtc_time_us;
     portEXIT_CRITICAL_SAFE(&s_esp_rtc_time_lock);
-    return s_esp_rtc_time_us;
+    return esp_rtc_time_us;
 #else
     uint64_t esp_rtc_time_us = delta_time_us + clk_ll_rtc_slow_load_rtc_fix_us();
     portEXIT_CRITICAL_SAFE(&s_esp_rtc_time_lock);
@@ -127,7 +162,7 @@ void esp_clk_slowclk_cal_set(uint32_t new_cal)
     /* To force monotonic time values even when clock calibration value changes,
      * we adjust esp_rtc_time
      */
-#if SOC_RTC_FAST_MEM_SUPPORTED
+#if SOC_RTC_MEM_SUPPORTED
     esp_rtc_get_time_us();
 #else
     portENTER_CRITICAL_SAFE(&s_esp_rtc_time_lock);
@@ -154,7 +189,7 @@ void esp_clk_slowclk_cal_set(uint32_t new_cal)
         clk_ll_rtc_slow_store_rtc_fix_us(new_fix_us);
     }
     portEXIT_CRITICAL_SAFE(&s_esp_rtc_time_lock);
-#endif // SOC_RTC_FAST_MEM_SUPPORTED
+#endif // SOC_RTC_MEM_SUPPORTED
 #endif // CONFIG_ESP_TIME_FUNCS_USE_RTC_TIMER
     clk_ll_rtc_slow_store_cal(new_cal);
 }

components/esp_hw_support/sleep_modes.c

@@ -1578,12 +1578,12 @@ static uint32_t get_power_down_flags(void)
 
 #if SOC_PM_SUPPORT_RTC_SLOW_MEM_PD && SOC_ULP_SUPPORTED
     // Labels are defined in the linker script
-    extern int _rtc_slow_length;
+    extern int _rtc_slow_length, _rtc_reserved_length;
     /**
      * Compiler considers "(size_t) &_rtc_slow_length > 0" to always be true.
      * So use a volatile variable to prevent compiler from doing this optimization.
      */
-    volatile size_t rtc_slow_mem_used = (size_t)&_rtc_slow_length;
+    volatile size_t rtc_slow_mem_used = (size_t)&_rtc_slow_length + (size_t)&_rtc_reserved_length;
 
     if ((s_config.domain[ESP_PD_DOMAIN_RTC_SLOW_MEM].pd_option == ESP_PD_OPTION_AUTO) &&
             (rtc_slow_mem_used > 0 || (s_config.wakeup_triggers & RTC_ULP_TRIG_EN))) {

components/esp_system/ld/esp32/memory.ld.in

@@ -27,16 +27,6 @@
 #define CONFIG_BTDM_RESERVE_DRAM 0
 #endif
 
-#if CONFIG_BOOTLOADER_RESERVE_RTC_MEM
-#ifdef CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC
-#define ESP_BOOTLOADER_RESERVE_RTC (CONFIG_BOOTLOADER_RESERVE_RTC_SIZE + CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC_SIZE)
-#else
-#define ESP_BOOTLOADER_RESERVE_RTC (CONFIG_BOOTLOADER_RESERVE_RTC_SIZE)
-#endif // not CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC
-#else
-#define ESP_BOOTLOADER_RESERVE_RTC 0
-#endif // not CONFIG_BOOTLOADER_RESERVE_RTC_MEM
-
 #if defined(CONFIG_ESP32_USE_FIXED_STATIC_RAM_SIZE)
 
 ASSERT((CONFIG_ESP32_FIXED_STATIC_RAM_SIZE <= 0x2c200),
@@ -98,24 +88,39 @@ MEMORY
   /* (See iram0_2_seg for meaning of 0x20 offset in the above.) */
 #endif // CONFIG_APP_BUILD_USE_FLASH_SECTIONS
 
-  /* RTC fast memory (executable). Persists over deep sleep.
-   */
+  /* RTC fast memory (executable). Persists over deep sleep. */
   rtc_iram_seg(RWX) :                org = 0x400C0000, len = 0x2000 - ESP_BOOTLOADER_RESERVE_RTC
 
-  /* RTC fast memory (same block as above), viewed from data bus */
+  /* RTC fast memory (same block as above, rtc_iram_seg), viewed from data bus */
   rtc_data_seg(RW) :                 org = 0x3ff80000, len = 0x2000 - ESP_BOOTLOADER_RESERVE_RTC
 
+  /* We reduced the size of rtc_iram_seg and rtc_data_seg by ESP_BOOTLOADER_RESERVE_RTC value.
+     It reserves the amount of RTC fast memory that we use for this memory segment.
+     This segment is intended for keeping bootloader rtc data (s_bootloader_retain_mem, when a Kconfig option is on).
+     The aim of this is to keep data that will not be moved around and have a fixed address.
+     org = 0x3ff80000 + 0x2000 - ESP_BOOTLOADER_RESERVE_RTC == SOC_RTC_DRAM_HIGH - sizeof(rtc_retain_mem_t)
+  */
+  rtc_fast_reserved_seg(RW) :        org = 0x3ff80000 + 0x2000 - ESP_BOOTLOADER_RESERVE_RTC, len = ESP_BOOTLOADER_RESERVE_RTC
+
   /* RTC slow memory (data accessible). Persists over deep sleep.
 
      Start of RTC slow memory is reserved for ULP co-processor code + data, if enabled.
   */
 #if CONFIG_ULP_COPROC_ENABLED
   rtc_slow_seg(RW)  :                org = 0x50000000 + CONFIG_ULP_COPROC_RESERVE_MEM,
-                                     len = 0x2000 - CONFIG_ULP_COPROC_RESERVE_MEM
+                                     len = 0x2000 - CONFIG_ULP_COPROC_RESERVE_MEM - RESERVE_RTC_MEM
 #else
-  rtc_slow_seg(RW)  :                org = 0x50000000, len = 0x2000
+  rtc_slow_seg(RW)  :                org = 0x50000000, len = 0x2000 - RESERVE_RTC_MEM
 #endif // CONFIG_ULP_COPROC_ENABLED
 
+  /* We reduced the size of rtc_slow_seg by RESERVE_RTC_MEM value.
+     It reserves the amount of RTC slow memory that we use for this memory segment.
+     This segment is intended for keeping rtc timer data (s_rtc_timer_retain_mem, see esp_clk.c files).
+     The aim of this is to keep data that will not be moved around and have a fixed address.
+     org = 0x50000000 + 0x2000 - RESERVE_RTC_MEM
+  */
+  rtc_slow_reserved_seg(RW) :        org = 0x50000000 + 0x2000 - RESERVE_RTC_MEM, len = RESERVE_RTC_MEM
+
   /* external memory */
   extern_ram_seg(RWX)  :             org = 0x3F800000,
                                      len = 0x400000

components/esp_system/ld/esp32/sections.ld.in

@@ -109,6 +109,46 @@ SECTIONS
     _rtc_force_slow_end = ABSOLUTE(.);
   } > rtc_slow_seg
 
+  /**
+   * This section holds RTC FAST data that should have fixed addresses.
+   * The data are not initialized at power-up and are retained during deep sleep.
+   */
+  .rtc_fast_reserved (NOLOAD):
+  {
+    . = ALIGN(4);
+    _rtc_fast_reserved_start = ABSOLUTE(.);
+    /* New data can only be added here to ensure existing data are not moved.
+       Because data have adhered to the end of the segment and code is relied on it.
+       >> put new data here << */
+    KEEP(*(.bootloader_data_rtc_mem .bootloader_data_rtc_mem.*))
+    _rtc_fast_reserved_end = ABSOLUTE(.);
+  } > rtc_fast_reserved_seg
+
+  _rtc_fast_reserved_length = _rtc_fast_reserved_end - _rtc_fast_reserved_start;
+  ASSERT((_rtc_fast_reserved_length <= LENGTH(rtc_fast_reserved_seg)),
+          "RTC FAST reserved segment data does not fit.")
+
+  /**
+   * This section holds RTC SLOW data that should have fixed addresses.
+   * The data are not initialized at power-up and are retained during deep sleep.
+   */
+  .rtc_slow_reserved (NOLOAD):
+  {
+    . = ALIGN(4);
+    _rtc_slow_reserved_start = ABSOLUTE(.);
+    /* New data can only be added here to ensure existing data are not moved.
+       Because data have adhered to the end of the segment and code is relied on it.
+       >> put new data here << */
+
+    *(.rtc_timer_data_in_rtc_mem .rtc_timer_data_in_rtc_mem.*)
+    _rtc_slow_reserved_end = ABSOLUTE(.);
+  } > rtc_slow_reserved_seg
+
+  _rtc_slow_reserved_length = _rtc_slow_reserved_end - _rtc_slow_reserved_start;
+  _rtc_reserved_length = _rtc_slow_reserved_length;
+  ASSERT((_rtc_slow_reserved_length <= LENGTH(rtc_slow_reserved_seg)),
+          "RTC SLOW reserved segment data does not fit.")
+
   /* Get size of rtc slow data based on rtc_data_location alias */
   _rtc_slow_length = (ORIGIN(rtc_slow_seg) == ORIGIN(rtc_data_location))
                         ? (_rtc_force_slow_end - _rtc_data_start)

components/esp_system/ld/esp32c3/memory.ld.in

@@ -15,16 +15,6 @@
 #include "sdkconfig.h"
 #include "ld.common"
 
-#if CONFIG_BOOTLOADER_RESERVE_RTC_MEM
-#ifdef CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC
-#define ESP_BOOTLOADER_RESERVE_RTC (CONFIG_BOOTLOADER_RESERVE_RTC_SIZE + CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC_SIZE)
-#else
-#define ESP_BOOTLOADER_RESERVE_RTC (CONFIG_BOOTLOADER_RESERVE_RTC_SIZE)
-#endif // not CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC
-#else
-#define ESP_BOOTLOADER_RESERVE_RTC 0
-#endif // not CONFIG_BOOTLOADER_RESERVE_RTC_MEM
-
 /**
  * physical memory is mapped twice to the vritual address (IRAM and DRAM).
  * `I_D_SRAM_OFFSET` is the offset between the two locations of the same physical memory
@@ -82,7 +72,16 @@ MEMORY
   /**
    * RTC fast memory (executable). Persists over deep sleep.
    */
-  rtc_iram_seg(RWX) :                org = 0x50000000, len = 0x2000 - ESP_BOOTLOADER_RESERVE_RTC
+  rtc_iram_seg(RWX) :                org = 0x50000000, len = 0x2000 - RESERVE_RTC_MEM
+
+  /* We reduced the size of rtc_iram_seg by RESERVE_RTC_MEM value.
+     It reserves the amount of RTC fast memory that we use for this memory segment.
+     This segment is intended for keeping:
+       - (lower addr) rtc timer data (s_rtc_timer_retain_mem, see esp_clk.c files).
+       - (higher addr) bootloader rtc data (s_bootloader_retain_mem, when a Kconfig option is on).
+     The aim of this is to keep data that will not be moved around and have a fixed address.
+  */
+  rtc_reserved_seg(RW) :        org = 0x50000000 + 0x2000 - RESERVE_RTC_MEM, len = RESERVE_RTC_MEM
 }
 
 /* Heap ends at top of dram0_0_seg */