diff --git a/components/bt/common/ble_log/Kconfig.in b/components/bt/common/ble_log/Kconfig.in index 22d3ab6e7d0..f0ba28e06be 100644 --- a/components/bt/common/ble_log/Kconfig.in +++ b/components/bt/common/ble_log/Kconfig.in @@ -77,12 +77,12 @@ if BLE_LOG_ENABLED bool "Enable BLE HCI Logging" default y help - Enable HCI packet logging captured on the Host side - (Bluedroid / NimBLE HCI HAL). This is the single HCI - logging switch for both Host and Controller traffic: - the controller no longer maintains its own internal - LL HCI log, and controller-side HCI records are not - emitted separately. + Enable HCI packet logging. Bluedroid and legacy VHCI NimBLE + capture on the Host side and suppress duplicate Controller + HCI records. Other configurations, including non-legacy + NimBLE, retain Controller HCI records through the LL callback + and require BLE_LOG_LL_ENABLED. Disabling this option + suppresses HCI records from both capture paths. config BLE_LOG_HOST_SIDE_HCI_LOG_ENABLED bool "Enable BLE Host side HCI Logging" diff --git a/components/bt/common/ble_log/README.md b/components/bt/common/ble_log/README.md index 35a654ae8c1..cb6fbda3288 100644 --- a/components/bt/common/ble_log/README.md +++ b/components/bt/common/ble_log/README.md @@ -100,12 +100,14 @@ console batch). `ble_log_init()` resets all three sequences, and its required `FLUSH` within that epoch. Callers must not write until `ble_log_init()` returns, so the `INIT` snapshot is submitted first. -Controller-side HCI records are not emitted by BLE Log, and the controller no -longer maintains its own internal LL HCI log. Host-side Bluedroid and NimBLE -HCI capture (`CONFIG_BLE_LOG_HCI_LOG_ENABLED`) is the single HCI logging -switch and the authoritative `HCI` stream for both Host and Controller -traffic. Its direction bit continues to use HCI payload byte 0 bit 7; this is -independent of the source metadata bit. +`CONFIG_BLE_LOG_HCI_LOG_ENABLED` controls HCI logging. Bluedroid and legacy +VHCI NimBLE retain Host-side capture and suppress duplicate Controller HCI +records. Other configurations, including non-legacy NimBLE, retain Controller +HCI records through the LL callback (requires `CONFIG_BLE_LOG_LL_ENABLED`). +Controller flags preserve their source mapping: `HCI` to `LL_HCI` and +`HCI_UPSTREAM` to `HCI`, with ISR precedence. Controller payloads are forwarded +unchanged. Host-side capture retains its direction bit in HCI payload byte 0 +bit 7. Disabling HCI logging suppresses records from both capture paths. ## Internal Snapshot @@ -212,7 +214,7 @@ canceled and counted as lost. | `CONFIG_BLE_LOG_POOL_NON_YIELD_RESERVE_CNT` | 1 | ISR/critical reserve count | | `CONFIG_BLE_LOG_POOL_TRANS_SIZE` | 640 | Bytes per shared transport; SPI builds require a multiple of four | | `CONFIG_BLE_LOG_LL_ENABLED` | target dependent | Controller LL logging | -| `CONFIG_BLE_LOG_HCI_LOG_ENABLED` | target dependent | Host-side HCI capture | +| `CONFIG_BLE_LOG_HCI_LOG_ENABLED` | y | HCI capture from Host or Controller, selected by transport | | `CONFIG_BLE_LOG_TS_SYNC_TOGGLE_IO_ENABLED` | n | Build the optional analyzer GPIO toggle | | `CONFIG_BLE_LOG_TS_ENABLED` | n | Deprecated compatibility entry selecting the GPIO toggle | @@ -234,6 +236,6 @@ idf.py build ``` `ble_log_test` validates golden v7 bytes, the consolidated Internal Snapshot, -source/HCI metadata, pool exhaustion and reserve use, snapshot busy loss, -stale claims, and enable/disable/deinit races. `ble_log_rt_test` covers batched +source/HCI metadata and capture selection, pool exhaustion and reserve use, +snapshot busy loss, stale claims, and enable/disable/deinit races. `ble_log_rt_test` covers batched dispatch, timer behavior, inflight statistics, and repeated deinit races. diff --git a/components/bt/common/ble_log/src/ble_log_lbm_v2.c b/components/bt/common/ble_log/src/ble_log_lbm_v2.c index c21ba964df9..ef33c795b64 100644 --- a/components/bt/common/ble_log/src/ble_log_lbm_v2.c +++ b/components/bt/common/ble_log/src/ble_log_lbm_v2.c @@ -1145,10 +1145,14 @@ BLE_LOG_IRAM_ATTR void ble_log_write_hex_ll(uint32_t len, const uint8_t *addr, uint32_t len_append, const uint8_t *addr_append, uint32_t flag) { - /* Controller-side HCI logging duplicates the retained Host-side HCI path. */ - if ((flag & BIT(BLE_LOG_LL_FLAG_HCI)) || - (flag & BIT(BLE_LOG_LL_FLAG_HCI_UPSTREAM)) || - (!addr && len) || (!addr_append && len_append)) { +#if !CONFIG_BLE_LOG_HCI_LOG_ENABLED || CONFIG_BT_BLUEDROID_ENABLED || \ + CONFIG_BT_NIMBLE_LEGACY_VHCI_ENABLE + /* Bluedroid and legacy NimBLE already capture HCI on the Host side. */ + if (flag & (BIT(BLE_LOG_LL_FLAG_HCI) | BIT(BLE_LOG_LL_FLAG_HCI_UPSTREAM))) { + return; + } +#endif + if ((!addr && len) || (!addr_append && len_append)) { return; } @@ -1160,11 +1164,17 @@ void ble_log_write_hex_ll(uint32_t len, const uint8_t *addr, * ties between equal-timestamp records from different sources. */ uint32_t frame_sn = BLE_LOG_GET_GLOBAL_SN(); - /* Controller-side HCI records are dropped above, so only the LL task - * and ISR identities remain: ISR-flagged records keep their own - * LL_ISR source ID, everything else is LL_TASK. */ - ble_log_src_t src_code = (flag & BIT(BLE_LOG_LL_FLAG_ISR)) ? - BLE_LOG_SRC_LL_ISR : BLE_LOG_SRC_LL_TASK; + /* Preserve the controller flag-to-source mapping and ISR precedence. */ + ble_log_src_t src_code; + if (flag & BIT(BLE_LOG_LL_FLAG_ISR)) { + src_code = BLE_LOG_SRC_LL_ISR; + } else if (flag & BIT(BLE_LOG_LL_FLAG_HCI)) { + src_code = BLE_LOG_SRC_LL_HCI; + } else if (flag & BIT(BLE_LOG_LL_FLAG_HCI_UPSTREAM)) { + src_code = BLE_LOG_SRC_HCI; + } else { + src_code = BLE_LOG_SRC_LL_TASK; + } bool is_isr = BLE_LOG_IN_ISR(); bool can_yield = !is_isr && xPortCanYield() && diff --git a/components/bt/common/ble_log/test_apps/ble_log_test/main/test_ble_log_rt.c b/components/bt/common/ble_log/test_apps/ble_log_test/main/test_ble_log_rt.c index f79c10a1f51..dc6cc952e39 100644 --- a/components/bt/common/ble_log/test_apps/ble_log_test/main/test_ble_log_rt.c +++ b/components/bt/common/ble_log/test_apps/ble_log_test/main/test_ble_log_rt.c @@ -403,6 +403,8 @@ typedef struct { bool critical_frame; bool hci_downstream_frame; bool hci_upstream_frame; + int ll_hci_frame_cnt; + int ll_hci_upstream_frame_cnt; bool claimed_frame; bool stale_protected_frame; int encode_frame_cnt; @@ -431,6 +433,18 @@ static void capture_frame_meta(const test_ble_log_frame_t *frame, void *ctx) capture->claimed_frame = true; } else if (frame->src == BLE_LOG_SRC_ENCODE && marker == 0x44) { capture->stale_protected_frame = true; + } else if (marker == 0x51 || marker == 0x52) { + static const uint8_t controller_timestamp[] = {0x78, 0x56, 0x34, 0x12}; + TEST_ASSERT_EQUAL_size_t(sizeof(controller_timestamp) + 1, frame->payload_len); + TEST_ASSERT_EQUAL_MEMORY(controller_timestamp, frame->payload, + sizeof(controller_timestamp)); + if (marker == 0x51) { + TEST_ASSERT_EQUAL_UINT8(BLE_LOG_SRC_LL_HCI, frame->src); + capture->ll_hci_frame_cnt++; + } else { + TEST_ASSERT_EQUAL_UINT8(BLE_LOG_SRC_HCI, frame->src); + capture->ll_hci_upstream_frame_cnt++; + } } } @@ -468,6 +482,18 @@ TEST_CASE("BLE Log writes from critical sections and commits claimed payload", * input at the validating API instead. */ TEST_ASSERT_FALSE(ble_log_write_hex(BLE_LOG_SRC_HCI, NULL, 1)); +#if CONFIG_BLE_LOG_LL_ENABLED + /* Controller records keep their existing payload, including timestamp. + * Exercise both contiguous and appended payloads at the real LL entry. */ + static const uint8_t ll_hci_payload[] = {0x78, 0x56, 0x34, 0x12, 0x51}; + const uint8_t ll_hci_upstream = 0x52; + ble_log_write_hex_ll(sizeof(ll_hci_payload), ll_hci_payload, 0, NULL, + BIT(BLE_LOG_LL_FLAG_HCI)); + ble_log_write_hex_ll(sizeof(ll_hci_payload) - 1, ll_hci_payload, + sizeof(ll_hci_upstream), &ll_hci_upstream, + BIT(BLE_LOG_LL_FLAG_HCI_UPSTREAM)); +#endif + uint32_t handle; uint8_t *claimed = ble_log_claim(BLE_LOG_SRC_ENCODE, 8, &handle); TEST_ASSERT_NOT_NULL(claimed); @@ -511,6 +537,15 @@ TEST_CASE("BLE Log writes from critical sections and commits claimed payload", TEST_ASSERT_TRUE(capture.critical_frame); TEST_ASSERT_TRUE(capture.hci_downstream_frame); TEST_ASSERT_TRUE(capture.hci_upstream_frame); +#if CONFIG_BLE_LOG_LL_ENABLED && CONFIG_BLE_LOG_HCI_LOG_ENABLED && \ + !CONFIG_BT_BLUEDROID_ENABLED && !CONFIG_BT_NIMBLE_LEGACY_VHCI_ENABLE + TEST_ASSERT_EQUAL(1, capture.ll_hci_frame_cnt); + TEST_ASSERT_EQUAL(1, capture.ll_hci_upstream_frame_cnt); +#else + /* Disabled HCI or an existing Host capture must suppress duplicates. */ + TEST_ASSERT_EQUAL(0, capture.ll_hci_frame_cnt); + TEST_ASSERT_EQUAL(0, capture.ll_hci_upstream_frame_cnt); +#endif TEST_ASSERT_TRUE(capture.claimed_frame); TEST_ASSERT_TRUE(capture.stale_protected_frame); /* Exactly the two committed claims: a stale commit that slipped past