From b42798174d9fc5d192d6aca7b29ded69ed155481 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Thu, 29 Feb 2024 11:30:05 +0530 Subject: [PATCH] [ESP32] Provide core dump summary in response to crash diagnostic log requests (#32331) * [ESP32] Provide core dump summary in response to crash diagnostic log requests. * use esp_core_dump_image_check when core dump size is requested --- .../esp32/README.md | 15 ++-- .../esp32/main/CMakeLists.txt | 2 +- ...diagnostic-logs-provider-delegate-impl.cpp | 70 +++++++++---------- .../diagnostic-logs-provider-delegate-impl.h | 28 +++++--- .../esp32/partitions.csv | 2 +- 5 files changed, 56 insertions(+), 61 deletions(-) diff --git a/examples/temperature-measurement-app/esp32/README.md b/examples/temperature-measurement-app/esp32/README.md index 99347fc6ead9f4..71210102d932bd 100644 --- a/examples/temperature-measurement-app/esp32/README.md +++ b/examples/temperature-measurement-app/esp32/README.md @@ -34,7 +34,6 @@ contains three files: ``` main/diagnostic_logs -├── crash.log ├── end_user_support.log └── network_diag.log ``` @@ -53,8 +52,8 @@ chip-tool diagnosticlogs retrieve-logs-request 0 0 1 0 # Read network diagnostic using BDX protocol chip-tool interactive start > diagnosticlogs retrieve-logs-request 1 1 1 0 --TransferFileDesignator network-diag.log -# Retrieve crash over BDX -> diagnosticlogs retrieve-logs-request 1 1 1 0 --TransferFileDesignator crash.bin +# Retrieve crash summary over BDX +> diagnosticlogs retrieve-logs-request 2 1 1 0 --TransferFileDesignator crash-summary.bin ``` esp-idf supports storing and retrieving @@ -73,15 +72,9 @@ This example's partition table and sdkconfig.default are already modified - Retrieve the core dump using diagnostic logs cluster ``` - # Read crash logs over BDX + # Read crash summary over BDX chip-tool interactive start - > diagnosticlogs retrieve-logs-request 1 1 1 0 --TransferFileDesignator crash.bin - ``` - -- Decode the crash logs, using espcoredump.py - ``` - espcoredump.py --chip (CHIP) info_corefile --core /tmp/crash.bin \ - --core-format elf build/chip-temperature-measurement-app.elf + > diagnosticlogs retrieve-logs-request 2 1 1 0 --TransferFileDesignator crash-summary.bin ``` ## Optimization diff --git a/examples/temperature-measurement-app/esp32/main/CMakeLists.txt b/examples/temperature-measurement-app/esp32/main/CMakeLists.txt index 062266652dc303..89fa5e9f755455 100644 --- a/examples/temperature-measurement-app/esp32/main/CMakeLists.txt +++ b/examples/temperature-measurement-app/esp32/main/CMakeLists.txt @@ -55,7 +55,7 @@ set(SRC_DIRS_LIST "${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/providers" ) -set(PRIV_REQUIRES_LIST chip QRCode bt nvs_flash bootloader_support espcoredump) +set(PRIV_REQUIRES_LIST chip QRCode bt nvs_flash espcoredump) if (CONFIG_ENABLE_PW_RPC) # Append additional directories for RPC build diff --git a/examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp b/examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp index 5034e3f928788f..55336ea76030dd 100644 --- a/examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp +++ b/examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp @@ -21,10 +21,6 @@ #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) #include -#include -// Its a bit hackish but we need this in order to pull in the sizeof(core_dump_header_t) -// we can even use the static 20 but, what if that gets chagned? -#include "../include_core_dump/esp_core_dump_types.h" #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) using namespace chip; @@ -95,34 +91,16 @@ size_t LogProvider::GetCrashSize() size_t outSize = 0; #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) - size_t unusedOutAddr; - esp_err_t esp_err = esp_core_dump_image_get(&unusedOutAddr, &outSize); - VerifyOrReturnValue(esp_err == ESP_OK, 0, ChipLogError(DeviceLayer, "Failed to get core dump image, esp_err:%d", esp_err)); + // Verify that the crash is present and sane + esp_err_t esp_err = esp_core_dump_image_check(); + VerifyOrReturnValue(esp_err == ESP_OK, 0, ChipLogError(DeviceLayer, "Core dump image check failed, esp_err:%d", esp_err)); + + outSize = sizeof(esp_core_dump_summary_t); #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) return outSize; } -CHIP_ERROR LogProvider::MapCrashPartition(CrashLogContext * context) -{ -#if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) - size_t outAddr, outSize; - esp_err_t esp_err = esp_core_dump_image_get(&outAddr, &outSize); - VerifyOrReturnError(esp_err == ESP_OK, CHIP_ERROR(ChipError::Range::kPlatform, esp_err), - ChipLogError(DeviceLayer, "Failed to get core dump image, esp_err:%d", esp_err)); - - /* map the full core dump parition, including the checksum. */ - esp_err = spi_flash_mmap(outAddr, outSize, SPI_FLASH_MMAP_DATA, &context->mappedAddress, &context->mappedHandle); - VerifyOrReturnError(esp_err == ESP_OK, CHIP_ERROR(ChipError::Range::kPlatform, esp_err), - ChipLogError(DeviceLayer, "Failed to mmap the crash partition, esp_err:%d", esp_err)); - - context->crashSize = static_cast(outSize); - return CHIP_NO_ERROR; -#else - return CHIP_ERROR_NOT_FOUND; -#endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) -} - CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentEnum intent) { context->intent = intent; @@ -146,11 +124,24 @@ CHIP_ERROR LogProvider::PrepareLogContextForIntent(LogContext * context, IntentE sCrashLogContext.Reset(); context->Crash.logContext = &sCrashLogContext; - CHIP_ERROR err = MapCrashPartition(context->Crash.logContext); - VerifyOrReturnError(err == CHIP_NO_ERROR, err, context->Crash.logContext = nullptr); + size_t crashSize = GetCrashSize(); + VerifyOrReturnError(crashSize > 0, CHIP_ERROR_NOT_FOUND); + + esp_core_dump_summary_t * summary = + reinterpret_cast(Platform::MemoryCalloc(1, sizeof(esp_core_dump_summary_t))); + VerifyOrReturnError(summary != nullptr, CHIP_ERROR_NO_MEMORY); + + esp_err_t esp_err = esp_core_dump_get_summary(summary); + if (esp_err != ESP_OK) + { + ChipLogError(DeviceLayer, "Failed to get core dump image, esp_err:%d", esp_err); + Platform::MemoryFree(summary); + return CHIP_ERROR_NOT_FOUND; + } - context->Crash.logContext->readOffset = sizeof(core_dump_header_t); - context->Crash.logContext->isMapped = true; + context->Crash.logContext->crashSize = crashSize; + context->Crash.logContext->readOffset = 0; + context->Crash.logContext->summary = summary; #else return CHIP_ERROR_NOT_FOUND; #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) @@ -177,7 +168,7 @@ void LogProvider::CleanupLogContextForIntent(LogContext * context) case IntentEnum::kCrashLogs: { #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) CrashLogContext * logContext = context->Crash.logContext; - spi_flash_munmap(logContext->mappedHandle); + // Reset() frees the summary if allocated logContext->Reset(); #endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) } @@ -227,12 +218,15 @@ CHIP_ERROR LogProvider::GetDataForIntent(LogContext * context, MutableByteSpan & case IntentEnum::kCrashLogs: { #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) CrashLogContext * logContext = context->Crash.logContext; - size_t dataSize = logContext->crashSize - logContext->readOffset; - auto count = std::min(dataSize, outBuffer.size()); + + VerifyOrReturnError(logContext->readOffset < logContext->crashSize, CHIP_ERROR_INCORRECT_STATE, outBuffer.reduce_size(0)); + + size_t dataSize = logContext->crashSize - logContext->readOffset; + auto count = std::min(dataSize, outBuffer.size()); VerifyOrReturnError(CanCastTo(count), CHIP_ERROR_INVALID_ARGUMENT, outBuffer.reduce_size(0)); - const uint8_t * readAddr = reinterpret_cast(logContext->mappedAddress) + logContext->readOffset; + const uint8_t * readAddr = reinterpret_cast(logContext->summary) + logContext->readOffset; memcpy(outBuffer.data(), readAddr, count); outBuffer.reduce_size(count); @@ -257,12 +251,14 @@ CHIP_ERROR LogProvider::StartLogCollection(IntentEnum intent, LogSessionHandle & { VerifyOrReturnValue(IsValidIntent(intent), CHIP_ERROR_INVALID_ARGUMENT); +#if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) // In case of crash logs we can only mmap at max once, so check before doing anything if (intent == IntentEnum::kCrashLogs) { - VerifyOrReturnError(sCrashLogContext.isMapped == false, CHIP_ERROR_INCORRECT_STATE, - ChipLogError(DeviceLayer, "Crash partition already mapped")); + VerifyOrReturnError(sCrashLogContext.summary == nullptr, CHIP_ERROR_INCORRECT_STATE, + ChipLogError(DeviceLayer, "Crash summary already allocated")); } +#endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) LogContext * context = reinterpret_cast(Platform::MemoryCalloc(1, sizeof(LogContext))); VerifyOrReturnValue(context != nullptr, CHIP_ERROR_NO_MEMORY); diff --git a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h index c3d1bd389fb39b..3431a54adc86a8 100644 --- a/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h +++ b/examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h @@ -20,7 +20,10 @@ #include #include -#include + +#if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) +#include +#endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) namespace chip { namespace app { @@ -57,20 +60,23 @@ class LogProvider : public DiagnosticLogsProviderDelegate struct CrashLogContext { - spi_flash_mmap_handle_t mappedHandle = 0; - const void * mappedAddress = nullptr; - uint32_t crashSize = 0; - uint32_t readOffset = 0; - bool isMapped = 0; +#if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) + uint32_t crashSize = 0; + uint32_t readOffset = 0; + esp_core_dump_summary_t * summary = nullptr; void Reset() { - this->mappedHandle = 0; - this->mappedAddress = nullptr; - this->crashSize = 0; - this->readOffset = 0; - this->isMapped = 0; + this->crashSize = 0; + this->readOffset = 0; + + if (this->summary) + { + Platform::MemoryFree(this->summary); + this->summary = nullptr; + } } +#endif // defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF) }; static CrashLogContext sCrashLogContext; diff --git a/examples/temperature-measurement-app/esp32/partitions.csv b/examples/temperature-measurement-app/esp32/partitions.csv index 35c776d19e80fa..487ac3e70dd993 100644 --- a/examples/temperature-measurement-app/esp32/partitions.csv +++ b/examples/temperature-measurement-app/esp32/partitions.csv @@ -4,4 +4,4 @@ nvs, data, nvs, , 0xC000, phy_init, data, phy, , 0x1000, # Factory partition size about 1.5MB factory, app, factory, , 1536K, -coredump, data, coredump,, 64K +coredump, data, coredump,, 64K, encrypted