From d3ffeb40a74a711070d9fbce0f7f1af82f99e2ff Mon Sep 17 00:00:00 2001 From: Tiago Medicci Serrano Date: Mon, 21 Nov 2022 15:04:32 -0300 Subject: [PATCH] libc/machine/xtensa: make longjmp safe against context switch In order to turn longjmp context-switch safe, it's necessary to disable interrupts before modifying windowbase and windowstart. Otherwise, after a context switch, windowstart and windowbase would be different, leading to a wrongly set windowstart bit due to longjmp writing it based on the windowbase before the context switch. This corrupts the registers at the next window overflow reaching that wrongly set bit. *Background:* This PR is related to an issue first observed on ESP-IDF https://github.com/espressif/esp-idf/issues/5229 and it was, then, checked on NuttX using a test application. *The test application:* To check if the problem affects ESP32, ESP32-S2 and ESP32-S3 on NuttX, it was created an application based on: https://en.cppreference.com/w/c/program/longjmp The application creates 16 tasks (`#define NUMBER_OF_TASKS 16`) that implements the following daemon: ``` static int setjmp_longjmp_daemon(int argc, char *argv[]) { for (int i = 0; i < NUMBER_OF_TASKS * 2; i++) { jmp_buf env; volatile int count = 0; if (setjmp(env) != UINT16_MAX) { foo(&env, ++count); } } sem_post(&g_sem); return EXIT_SUCCESS; } ``` The main function also initializes a semaphore to avoid application exiting before tasks return successfully: ``` sem_init(&g_sem, 0, -NUMBER_OF_TASKS); ``` Finally, the round-robin interval was lowered to 1ms to raise the chances of the longjmp being interrupted by a context switch (`CONFIG_RR_INTERVAL=1). This setup was able to reproduce the problem prior to this patch being applied. --- arch/xtensa/include/esp32/core-isa.h | 1 + arch/xtensa/include/esp32s2/core-isa.h | 1 + arch/xtensa/include/esp32s3/core-isa.h | 1 + .../esp32-devkitc/configs/knsh/defconfig | 1 + .../esp32-devkitc/configs/ostest/defconfig | 1 + .../esp32/esp32-devkitc/configs/smp/defconfig | 1 + .../esp32-devkitc/configs/wapi_smp/defconfig | 1 + .../esp32s2-saola-1/configs/ostest/defconfig | 46 +++++++++++++++++++ .../esp32s3-devkit/configs/smp/defconfig | 1 + libs/libc/machine/xtensa/arch_setjmp.S | 12 +++++ 10 files changed, 66 insertions(+) create mode 100644 boards/xtensa/esp32s2/esp32s2-saola-1/configs/ostest/defconfig diff --git a/arch/xtensa/include/esp32/core-isa.h b/arch/xtensa/include/esp32/core-isa.h index fd52300e4a..f26fa9ca68 100644 --- a/arch/xtensa/include/esp32/core-isa.h +++ b/arch/xtensa/include/esp32/core-isa.h @@ -554,6 +554,7 @@ * 0 == XEAX (extern) or TX */ #define XCHAL_HAVE_XEA1 0 /* Exception Architecture 1 */ #define XCHAL_HAVE_XEA2 1 /* Exception Architecture 2 */ +#define XCHAL_HAVE_XEA3 0 /* Exception Architecture 3 */ #define XCHAL_HAVE_XEAX 0 /* External Exception Arch. */ #define XCHAL_HAVE_EXCEPTIONS 1 /* exception option */ #define XCHAL_HAVE_HALT 0 /* halt architecture option */ diff --git a/arch/xtensa/include/esp32s2/core-isa.h b/arch/xtensa/include/esp32s2/core-isa.h index fa88a96416..45ba21cc13 100644 --- a/arch/xtensa/include/esp32s2/core-isa.h +++ b/arch/xtensa/include/esp32s2/core-isa.h @@ -610,6 +610,7 @@ #define XCHAL_HAVE_XEA1 0 /* Exception Architecture 1 */ #define XCHAL_HAVE_XEA2 1 /* Exception Architecture 2 */ +#define XCHAL_HAVE_XEA3 0 /* Exception Architecture 3 */ #define XCHAL_HAVE_XEAX 0 /* External Exception Arch. */ #define XCHAL_HAVE_EXCEPTIONS 1 /* exception option */ #define XCHAL_HAVE_HALT 0 /* halt architecture option */ diff --git a/arch/xtensa/include/esp32s3/core-isa.h b/arch/xtensa/include/esp32s3/core-isa.h index ac954f19cb..e956321e20 100644 --- a/arch/xtensa/include/esp32s3/core-isa.h +++ b/arch/xtensa/include/esp32s3/core-isa.h @@ -568,6 +568,7 @@ #define XCHAL_HAVE_XEA1 0 /* Exception Architecture 1 */ #define XCHAL_HAVE_XEA2 1 /* Exception Architecture 2 */ +#define XCHAL_HAVE_XEA3 0 /* Exception Architecture 3 */ #define XCHAL_HAVE_XEAX 0 /* External Exception Arch. */ #define XCHAL_HAVE_EXCEPTIONS 1 /* exception option */ #define XCHAL_HAVE_HALT 0 /* halt architecture option */ diff --git a/boards/xtensa/esp32/esp32-devkitc/configs/knsh/defconfig b/boards/xtensa/esp32/esp32-devkitc/configs/knsh/defconfig index ae24f7dcbb..cf3abae7bc 100644 --- a/boards/xtensa/esp32/esp32-devkitc/configs/knsh/defconfig +++ b/boards/xtensa/esp32/esp32-devkitc/configs/knsh/defconfig @@ -17,6 +17,7 @@ CONFIG_ARCH_CHIP="esp32" CONFIG_ARCH_CHIP_ESP32=y CONFIG_ARCH_CHIP_ESP32WROVER=y CONFIG_ARCH_INTERRUPTSTACK=2048 +CONFIG_ARCH_SETJMP_H=y CONFIG_ARCH_STACKDUMP=y CONFIG_ARCH_XTENSA=y CONFIG_BOARD_LOOPSPERMSEC=16717 diff --git a/boards/xtensa/esp32/esp32-devkitc/configs/ostest/defconfig b/boards/xtensa/esp32/esp32-devkitc/configs/ostest/defconfig index 6b2a1cc235..6e2bd500e6 100644 --- a/boards/xtensa/esp32/esp32-devkitc/configs/ostest/defconfig +++ b/boards/xtensa/esp32/esp32-devkitc/configs/ostest/defconfig @@ -16,6 +16,7 @@ CONFIG_ARCH_BOARD_ESP32_DEVKITC=y CONFIG_ARCH_CHIP="esp32" CONFIG_ARCH_CHIP_ESP32=y CONFIG_ARCH_CHIP_ESP32WROVER=y +CONFIG_ARCH_SETJMP_H=y CONFIG_ARCH_STACKDUMP=y CONFIG_ARCH_XTENSA=y CONFIG_BOARD_LOOPSPERMSEC=16717 diff --git a/boards/xtensa/esp32/esp32-devkitc/configs/smp/defconfig b/boards/xtensa/esp32/esp32-devkitc/configs/smp/defconfig index 97ccbfa889..e0c400a832 100644 --- a/boards/xtensa/esp32/esp32-devkitc/configs/smp/defconfig +++ b/boards/xtensa/esp32/esp32-devkitc/configs/smp/defconfig @@ -17,6 +17,7 @@ CONFIG_ARCH_CHIP="esp32" CONFIG_ARCH_CHIP_ESP32=y CONFIG_ARCH_CHIP_ESP32WROVER=y CONFIG_ARCH_INTERRUPTSTACK=2048 +CONFIG_ARCH_SETJMP_H=y CONFIG_ARCH_STACKDUMP=y CONFIG_ARCH_XTENSA=y CONFIG_BOARD_LOOPSPERMSEC=16717 diff --git a/boards/xtensa/esp32/esp32-devkitc/configs/wapi_smp/defconfig b/boards/xtensa/esp32/esp32-devkitc/configs/wapi_smp/defconfig index a05a58b769..61c1833899 100644 --- a/boards/xtensa/esp32/esp32-devkitc/configs/wapi_smp/defconfig +++ b/boards/xtensa/esp32/esp32-devkitc/configs/wapi_smp/defconfig @@ -18,6 +18,7 @@ CONFIG_ARCH_CHIP="esp32" CONFIG_ARCH_CHIP_ESP32=y CONFIG_ARCH_CHIP_ESP32WROVER=y CONFIG_ARCH_INTERRUPTSTACK=2048 +CONFIG_ARCH_SETJMP_H=y CONFIG_ARCH_STACKDUMP=y CONFIG_ARCH_XTENSA=y CONFIG_BOARD_LOOPSPERMSEC=16717 diff --git a/boards/xtensa/esp32s2/esp32s2-saola-1/configs/ostest/defconfig b/boards/xtensa/esp32s2/esp32s2-saola-1/configs/ostest/defconfig new file mode 100644 index 0000000000..be378f9dc0 --- /dev/null +++ b/boards/xtensa/esp32s2/esp32s2-saola-1/configs/ostest/defconfig @@ -0,0 +1,46 @@ +# +# This file is autogenerated: PLEASE DO NOT EDIT IT. +# +# You can use "make menuconfig" to make any modifications to the installed .config file. +# You can then do "make savedefconfig" to generate a new defconfig file that includes your +# modifications. +# +# CONFIG_ARCH_LEDS is not set +# CONFIG_NSH_ARGCAT is not set +# CONFIG_NSH_CMDOPT_HEXDUMP is not set +# CONFIG_NSH_CMDPARMS is not set +CONFIG_ARCH="xtensa" +CONFIG_ARCH_BOARD="esp32s2-saola-1" +CONFIG_ARCH_BOARD_COMMON=y +CONFIG_ARCH_BOARD_ESP32S2_SAOLA_1=y +CONFIG_ARCH_CHIP="esp32s2" +CONFIG_ARCH_CHIP_ESP32S2=y +CONFIG_ARCH_CHIP_ESP32S2WROVER=y +CONFIG_ARCH_SETJMP_H=y +CONFIG_ARCH_STACKDUMP=y +CONFIG_ARCH_XTENSA=y +CONFIG_BOARD_LOOPSPERMSEC=16717 +CONFIG_BUILTIN=y +CONFIG_ESP32S2_UART0=y +CONFIG_FS_PROCFS=y +CONFIG_HAVE_CXX=y +CONFIG_HAVE_CXXINITIALIZE=y +CONFIG_IDLETHREAD_STACKSIZE=3072 +CONFIG_INIT_ENTRYPOINT="nsh_main" +CONFIG_INTELHEX_BINARY=y +CONFIG_NSH_ARCHINIT=y +CONFIG_NSH_BUILTIN_APPS=y +CONFIG_NSH_FILEIOSIZE=512 +CONFIG_NSH_LINELEN=64 +CONFIG_NSH_READLINE=y +CONFIG_PREALLOC_TIMERS=4 +CONFIG_RAM_SIZE=114688 +CONFIG_RAM_START=0x20000000 +CONFIG_RR_INTERVAL=200 +CONFIG_SCHED_WAITPID=y +CONFIG_START_DAY=6 +CONFIG_START_MONTH=12 +CONFIG_START_YEAR=2011 +CONFIG_SYSTEM_NSH=y +CONFIG_TESTING_OSTEST=y +CONFIG_UART0_SERIAL_CONSOLE=y diff --git a/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig b/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig index 4fb19b6810..d74ad0ae4a 100644 --- a/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig +++ b/boards/xtensa/esp32s3/esp32s3-devkit/configs/smp/defconfig @@ -17,6 +17,7 @@ CONFIG_ARCH_CHIP="esp32s3" CONFIG_ARCH_CHIP_ESP32S3=y CONFIG_ARCH_CHIP_ESP32S3WROOM1=y CONFIG_ARCH_INTERRUPTSTACK=2048 +CONFIG_ARCH_SETJMP_H=y CONFIG_ARCH_STACKDUMP=y CONFIG_ARCH_XTENSA=y CONFIG_BOARD_LOOPSPERMSEC=16717 diff --git a/libs/libc/machine/xtensa/arch_setjmp.S b/libs/libc/machine/xtensa/arch_setjmp.S index 16087eb4be..b60add35dc 100644 --- a/libs/libc/machine/xtensa/arch_setjmp.S +++ b/libs/libc/machine/xtensa/arch_setjmp.S @@ -258,6 +258,14 @@ longjmp: /* Using this register is more efficient; it triggers less overflows. */ # define AR_WB a5 # endif + /* Deactivate interrupts in order to modify WindowBase + and WindowStart. */ + rsr a7, PS /* to be restored after SPILL_ALL_WINDOWS */ + movi a5, XCHAL_PS_EXCM_MASK /* PS_INTLEVEL_MASK */ + or a5, a7, a5 /* get the current INTLEVEL */ + wsr a5, PS + rsync + /* Invalidate all but the current window; set WindowStart to (1 << WindowBase). */ rsr AR_WB, WINDOWBASE @@ -267,6 +275,10 @@ longjmp: wsr a4, WINDOWSTART rsync + /* Activate interrupts again after modifying WindowBase and WindowStart. */ + wsr a7, PS + rsync + /* 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. */