Explorar el Código

[esp_rom]: fixed S3 longjmp patch

* On S3, the placement of ROM functions is
  ECO-dependent. Hence, we don't jump into
  the middle of the longjmp function in ROM
  on S3 anymore.
  Instead, the whole longjump function is used
  in the patch.

* Also properly excluded the patch from
  bootloader build with Makefiles

Closes IDF-3391
Jakob Hasse hace 4 años
padre
commit
e32831033a

+ 2 - 4
components/esp_rom/CMakeLists.txt

@@ -4,8 +4,7 @@ set(sources "patches/esp_rom_crc.c"
             "patches/esp_rom_sys.c"
             "patches/esp_rom_uart.c")
 
-if(CONFIG_IDF_TARGET_ARCH_XTENSA AND NOT (target STREQUAL "esp32s3"))
-    # Temporarily disabled on S3 due to it breaking longjmp TODO ESP32S3 IDF-3391
+if(CONFIG_IDF_TARGET_ARCH_XTENSA)
     list(APPEND sources "patches/esp_rom_longjmp.S")
 endif()
 
@@ -103,8 +102,7 @@ else() # Regular app build
         endif()
     endif()
 
-    if(CONFIG_IDF_TARGET_ARCH_XTENSA AND NOT (target STREQUAL "esp32s3"))
-        # Temporarily disabled on S3 due to it breaking longjmp TODO ESP32S3 IDF-3391
+    if(CONFIG_IDF_TARGET_ARCH_XTENSA)
         target_link_libraries(${COMPONENT_LIB} INTERFACE "-Wl,--wrap=longjmp")
     endif()
 endif()

+ 8 - 2
components/esp_rom/component.mk

@@ -1,6 +1,10 @@
 COMPONENT_ADD_INCLUDEDIRS := include esp32 include/esp32
 COMPONENT_SRCDIRS := patches .
 
+ifdef IS_BOOTLOADER_BUILD
+COMPONENT_OBJEXCLUDE := patches/esp_rom_longjmp.o
+endif
+
 #Linker scripts used to link the final application.
 #Warning: These linker scripts are only used when the normal app is compiled; the bootloader
 #specifies its own scripts.
@@ -38,7 +42,9 @@ LINKER_SCRIPTS += esp32.rom.newlib-time.ld
 endif
 
 COMPONENT_ADD_LDFLAGS += -L $(COMPONENT_PATH)/esp32/ld \
-                         $(addprefix -T ,$(LINKER_SCRIPTS)) \
-                         -l$(COMPONENT_NAME) -Wl,--wrap=longjmp \
+                         $(addprefix -T ,$(LINKER_SCRIPTS))
+ifndef IS_BOOTLOADER_BUILD
+COMPONENT_ADD_LDFLAGS += -l$(COMPONENT_NAME) -Wl,--wrap=longjmp
+endif
 
 COMPONENT_ADD_LINKER_DEPS += $(addprefix esp32/ld/, $(LINKER_SCRIPTS))

+ 82 - 4
components/esp_rom/patches/esp_rom_longjmp.S

@@ -29,10 +29,9 @@
 */
 
 #include <xtensa/corebits.h>
+#include <sdkconfig.h>
 
-/*
-    Replacement of the first instructions of void longjmp (jmp_buf env, int val)
-*/
+/* void longjmp (jmp_buf env, int val) */
 
     .align  4
     .literal_position
@@ -59,7 +58,11 @@ __wrap_longjmp:
     /* Activate interrupts again after modifying WINDOWBASE and WINDOWSTART. */
     wsr     a7, PS
 
-    /* Jump back to original longjmp implementation.
+#if !CONFIG_IDF_TARGET_ESP32S3
+
+    /*
+        If not on S3, replacement of only the first instructions,
+        then jump back to original longjmp implementation.
         The jump target is the instrucion
     	    l32i	a0, a2, 64
         of the original code. Hence, the original code's entry instruction and windowstart modification are left
@@ -69,3 +72,78 @@ __wrap_longjmp:
     jx a0
 
     .size   __wrap_longjmp, . - __wrap_longjmp
+
+#else /* CONFIG_IDF_TARGET_ESP32S3 */
+    /*
+       If on S3, we replace the whole function for simplicity. The placement of longjmp in ROM is ECO-dependent
+       on S3.
+    */
+
+    /*
+	   Return to the return address of the setjmp, using the
+	   window size bits from the setjmp call so that the caller
+	   will be able to find the return value that we put in a2.  */
+
+	l32i	a0, a2, 64
+
+	/* Copy the first 4 saved registers from jmp_buf into the save area
+	   at the current sp so that the values will be restored to registers
+	   when longjmp returns.  */
+
+	addi	a7, a1, -16
+	l32i	a4, a2, 0
+	l32i	a5, a2, 4
+	s32i	a4, a7, 0
+	s32i	a5, a7, 4
+	l32i	a4, a2, 8
+	l32i	a5, a2, 12
+	s32i	a4, a7, 8
+	s32i	a5, a7, 12
+
+	/* Copy the remaining 0-8 saved registers.  */
+	extui	a7, a0, 30, 2
+	blti	a7, 2, .Lendlj
+	l32i	a8, a2, 52
+	slli	a4, a7, 4
+	sub	a6, a8, a4
+	addi	a5, a2, 16
+	addi	a8, a8, -16		// a8 = end of register overflow area
+.Lljloop:
+	l32i	a7, a5, 0
+	l32i	a4, a5, 4
+	s32i	a7, a6, 0
+	s32i	a4, a6, 4
+	l32i	a7, a5, 8
+	l32i	a4, a5, 12
+	s32i	a7, a6, 8
+	s32i	a4, a6, 12
+	addi	a5, a5, 16
+	addi	a6, a6, 16
+	blt	a6, a8, .Lljloop
+.Lendlj:
+
+	/* The 4 words saved from the register save area at the target's
+	   sp are copied back to the target procedure's save area.  The
+	   only point of this is to prevent a catastrophic failure in
+	   case the contents were moved by an alloca after calling
+	   setjmp.  This is a bit paranoid but it doesn't cost much.  */
+
+	l32i	a7, a2, 4		// load the target stack pointer
+	addi	a7, a7, -16		// find the destination save area
+	l32i	a4, a2, 48
+	l32i	a5, a2, 52
+	s32i	a4, a7, 0
+	s32i	a5, a7, 4
+	l32i	a4, a2, 56
+	l32i	a5, a2, 60
+	s32i	a4, a7, 8
+	s32i	a5, a7, 12
+
+	/* Return val ? val : 1.  */
+	movi	a2, 1
+	movnez	a2, a3, a3
+
+	retw
+    .size   __wrap_longjmp, . - __wrap_longjmp
+
+#endif /* CONFIG_IDF_TARGET_ESP32S3 */

+ 4 - 0
tools/test_apps/system/longjmp_test/main/component.mk

@@ -0,0 +1,4 @@
+#
+# "main" pseudo-component makefile.
+#
+# (Uses default behaviour of compiling all source files in directory, adding 'include' to include path.)

+ 1 - 1
tools/test_apps/system/longjmp_test/main/hello_world_main.c

@@ -110,7 +110,7 @@ void app_main(void)
     }
 
     vTaskDelay(10000);
-    printf("stopping timers...\n");
+    printf("stopping timer...\n");
     esp_timer_stop(crash_timer);
     esp_timer_delete(crash_timer);