esp_hw_support: Fix invalid system time if s_esp_rtc_time_us & s_rtc_…

…last_ticks were moved around

The commit fixes the case:
If variables in RTC RAM have been moved around by the linker,
they will be filled with garbage data. Any reset other than OTA would work fine
because the variables would still be initialized from the initial bootup.

So now system time will be valid even after OTA.

Closes #9448

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>
 
@@ -52,9 +53,30 @@ extern uint32_t g_ticks_per_us_app;
 
 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)
 {
@@ -102,13 +124,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
@@ -127,11 +160,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);
@@ -145,7 +180,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);
@@ -172,7 +207,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 */