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

BLE HOGP host cyw43 driver crash #2268

Open
Slion opened this issue Feb 14, 2025 · 12 comments
Open

BLE HOGP host cyw43 driver crash #2268

Slion opened this issue Feb 14, 2025 · 12 comments
Assignees
Milestone

Comments

@Slion
Copy link

Slion commented Feb 14, 2025

SDK 2.1.0 + develop branch
Boards: Pico 2 W and Pico W
btstack mode: I mostly use poll but I could also repro the issue with background mode

When trying to pair with HOGP from my app to a BLE TrackPoint Keyboard II I get a driver crash.
Though it works fine when pairing to that same device from the hog_host_demo.
My app can pair, connect and use a BLE HOGP Logitech LIFT mouse just fine.

It hits that line from crt0.S:

decl_isr_bkpt isr_hardfault

Here is the callstack:

isr_hardfault@0x10000114 (s:\Dev\Project\lib\pico-sdk\src\rp2_common\pico_crt0\crt0.S:170)
<signal handler called>@0xffffffe9 (Unknown Source:0)
??@0x00000000 (Unknown Source:0)
cyw43_write_reg_u8@0x1002e288 (s:\Dev\Project\lib\pico-sdk\src\rp2_common\pico_cyw43_driver\cyw43_bus_pio_spi.c:485)
??@0x00000000 (Unknown Source:0)

Here is a btstack-dev mailing list thread about it:
https://groups.google.com/g/btstack-dev/c/F-59UnNlvGE

Steps to reproduce

Use that hog_host_demo and enable stack guards by adding add_compile_definitions(PICO_USE_STACK_GUARDS=1) to cmake and try pairing to a Lenovo TrackPoint Keyboard II.
Not sure if other devices are affected. Not sure if the issue lies with the stack guards feature or is it that the cyw43 driver have some stack overflow issue when pairing with that specific device.

@Slion
Copy link
Author

Slion commented Feb 15, 2025

I disabled BLE TLV device database and used the memory database instead, same issue.
For it to compile you also need to comment out your NVM_NUM_DEVICE_DB_ENTRIES define from your btstack config.

diff --git a/src/rp2_common/pico_btstack/CMakeLists.txt b/src/rp2_common/pico_btstack/CMakeLists.txt
index 010cc9e..390ea23 100644
--- a/src/rp2_common/pico_btstack/CMakeLists.txt
+++ b/src/rp2_common/pico_btstack/CMakeLists.txt
@@ -97,7 +97,7 @@ if (EXISTS ${PICO_BTSTACK_PATH}/${BTSTACK_TEST_PATH})
         ${PICO_BTSTACK_PATH}/src/ble/gatt-service/ancs_client.c
         ${PICO_BTSTACK_PATH}/src/ble/gatt_client.c
         ${PICO_BTSTACK_PATH}/src/ble/le_device_db_memory.c
-        ${PICO_BTSTACK_PATH}/src/ble/le_device_db_tlv.c
+        #${PICO_BTSTACK_PATH}/src/ble/le_device_db_tlv.c
         ${PICO_BTSTACK_PATH}/src/ble/sm.c
     )
     pico_mirrored_target_link_libraries(pico_btstack_ble INTERFACE
diff --git a/src/rp2_common/pico_cyw43_driver/btstack_cyw43.c b/src/rp2_common/pico_cyw43_driver/btstack_cyw43.c
index 75061f1..58043f5 100644
--- a/src/rp2_common/pico_cyw43_driver/btstack_cyw43.c
+++ b/src/rp2_common/pico_cyw43_driver/btstack_cyw43.c
@@ -40,7 +40,7 @@ static void setup_tlv(void) {
     const btstack_link_key_db_t *btstack_link_key_db = btstack_link_key_db_tlv_get_instance(btstack_tlv_impl, &btstack_tlv_flash_bank_context);
     hci_set_link_key_db(btstack_link_key_db);
 #endif
-#ifdef ENABLE_BLE
+#if defined(ENABLE_BLE) && defined(NVM_NUM_DEVICE_DB_ENTRIES)
     // configure LE Device DB for TLV
     le_device_db_tlv_configure(btstack_tlv_impl, &btstack_tlv_flash_bank_context);
 #endif

@peterharperuk
Copy link
Contributor

That call stack makes no sense

@mringwal
Copy link
Contributor

@Slion Do you have master/2.1 of the Pico SDK or the current develop branch. If not develop, please switch to the develop version and repeat your test.

@Slion
Copy link
Author

Slion commented Feb 16, 2025

@Slion Do you have master/2.1 of the Pico SDK or the current develop branch. If not develop, please switch to the develop version and repeat your test.

Same issue on master 2.1.0 and develop from d7f6582 which is using btstack 1.6.2 I believe.

Between those two versions there is an update of the cyw43 driver and of btstack to v1.6.2. I don't think there is any relevant changes since on develop but still I could do a rebase and test it again.

@Slion
Copy link
Author

Slion commented Feb 16, 2025

That call stack makes no sense

Glad I'm not the only one then 🫣
I'll take any piece of advice to make progress on this issue.

@kilograham kilograham added this to the 2.1.2 milestone Feb 17, 2025
@Slion
Copy link
Author

Slion commented Feb 18, 2025

To reproduce the issue you can just use that hog_host_demo and enable stack guards by adding add_compile_definitions(PICO_USE_STACK_GUARDS=1) to cmake and try pairing to a Lenovo TrackPoint Keyboard II.
Not sure if other devices are affected. Not sure if the issue lies with the stack guards feature or is it that the cyw43 driver have some stack overflow issue when pairing with that specific device.

I've added this information to the first post.

@mringwal
Copy link
Contributor

I understand that PICO_USE_STACK_GUARDS write protects the last 32 bytes of the stack. Maybe the demo requires some of these bytes. In this case, increasing the stack by should allow to re-enable PICO_USE_STACK_GUARDS. Could you verify this hypothesis.

BTstack makes rather heavy use of the stack (as it avoids dynamic memory allocation ).
There are a few places where this could be reduced.
Would it be possible to increase default stack size from 2 kB to 3 kB if BTstack is used or similar?

@Slion
Copy link
Author

Slion commented Feb 19, 2025

In this case, increasing the stack by should allow to re-enable PICO_USE_STACK_GUARDS. Could you verify this hypothesis.

Good call!
If I set PICO_STACK_SIZE to 4KB, up from the default 2KB, pairing with that keyboard does work even with PICO_USE_STACK_GUARDS enabled.

@peterharperuk
Copy link
Contributor

@Slion Are you using both cores?

@Slion
Copy link
Author

Slion commented Feb 19, 2025

@Slion Are you using both cores?

I don't think so and neither should the hog_host_demo example.

@peterharperuk
Copy link
Contributor

I can see why this would fix the issue when PICO_USE_STACK_GUARDS is used. But if you don't use PICO_USE_STACK_GUARDS and are not using the other core, then I don't think a stack overflow will cause a crash. So I'm wondering if this is the cause of your original issue.

@Slion
Copy link
Author

Slion commented Feb 19, 2025

But if you don't use PICO_USE_STACK_GUARDS and are not using the other core, then I don't think a stack overflow will cause a crash

That's indeed what I observed:
2KB stack with PICO_USE_STACK_GUARDS => Crash
2KB stack without PICO_USE_STACK_GUARDS => No crash
4KB stack with PICO_USE_STACK_GUARDS => No crash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants