Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/esp: fix computation of coprocessor save area pointer in initial thread context #11461

Merged
merged 1 commit into from
Apr 29, 2019

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR fixes a problem that was figured out in issue #11443.

With commit 28d9599, the following change was introduced:

--- a/cpu/esp32/thread_arch.c
+++ b/cpu/esp32/thread_arch.c
@@ -186,1 +186,1 @@ char* thread_stack_init(thread_task_func_t task_func, void *arg, void *stack_sta
-    p = (uint32_t *)(((uint32_t)(top_of_stack + 1) - XT_CP_SIZE));
+    p = (uint32_t *)(((uint32_t)(top_of_stack + 1) - XT_CP_SIZE) & ~0xf);

top_of_stack isn't aligned down to the previous 16 byte aligned address. Furthermore, top_of_stack as well as XT_CP_SIZE are used unaligned in cpu/esp_common/vendor/xtensa/portasm.S in address computation for the coprocessor save area, .

Aligning pointer p down to the previous 16 byte aligned address results in a wrong address of the coprocessor save area during the initialization of the thread context. This leads to wrong values and wrong positions of these values in the coprocessor save area in inital thread context.

Since ESP8266 doesn't have a coprocessor, this bug affects only ESP32.

Testing procedure

The only known application suffering from this problem is examples/lorawan on one of the new LoRa32 board definitions in PR #11265 and #11418. Therefore, it is suggested to test this PR with one of these boards.

make BOARD=esp32-heltec-lora32-v2 -C examples/lorawan flash

Before the fix, the application hangs in function semtech_loramac_init and ends within a boot loop.

...
Starting RIOT kernel on PRO cpu
I (247) [main_trampoline]: main(): This is RIOT! (Version: 2019.07-devel-141-g203c-boards/esp32-heltec-lora32)
LoRaWAN Class A low-power application
=====================================
ets Jun  8 2016 00:22:57

rst:0x7 (TG0WDT_SYS_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
...

With the fix, function semtech_loramac_init works and the application prints the message "Starting join procedure".

...
Starting RIOT kernel on PRO cpu
I (247) [main_trampoline]: main(): This is RIOT! (Version: 2019.07-devel-141-g203c-boards/esp32-heltec-lora32)
LoRaWAN Class A low-power application
=====================================
Starting join procedure
...

Issues/PRs references

Fixes #11443

`top_of_stack` isn't aligned down to the previous 16 byte aligned address. Furthermore, `top_of_stack` as well as `XT_CP_SIZE` are used unaligned in `cpu/esp_common/vendor/xtensa/portasm.S` in the address computation for the coprocessor save area, .

Aligning pointer `p` down to the previous 16 byte aligned address results in a wrong address of the coprocessor save area during the initialization of the thread context. This leads to wrong values and wrong positions of these values in the coprocessor save area in inital thread context.

Since ESP8266 doesn't have a coprocessor, this bug affects only ESP32.
@gschorcht gschorcht added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Apr 27, 2019
@yegorich
Copy link
Contributor

Tested on TTGO T-Beam board with both examples/lorawan and a custom application. Bugfix confirmed.

@gschorcht gschorcht requested a review from smlng April 29, 2019 07:50
@gschorcht
Copy link
Contributor Author

@sming May I kindly ask you to review it since @yegorich isn't maintainer? Thanks.

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines labels Apr 29, 2019
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, trust @yegorich testing

@fcgdam
Copy link

fcgdam commented Apr 29, 2019

Can confirm that this fix also works and solves the issue for me.

@smlng smlng merged commit 972ae2e into RIOT-OS:master Apr 29, 2019
@gschorcht
Copy link
Contributor Author

@smlng Thanks for reviewing
@yegorich Thanks for figuring out the problem
@fcgdam Thanks for testing
😄

@yegorich
Copy link
Contributor

@gschorcht thanks for a quick fix and let's start merging the boards :-)

@gschorcht gschorcht deleted the cpu/esp/issue_11443 branch May 2, 2019 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cpu/esp32: WDT reset in example/lorawan
4 participants