[ESP32] Provide core dump summary in response to crash diagnostic log

requests.
project-chip · Feb 27, 2024 · 821a477 · 821a477
1 parent 28c78af
commit 821a477
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 60 deletions.
diff --git 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
@@ -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
@@ -21,10 +21,6 @@
 
 #if defined(CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH) && defined(CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF)
 #include <esp_core_dump.h>
-#include <esp_flash_encrypt.h>
-// 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,17 @@ 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);
+    // Verify that the crash is present and sane
+    size_t unusedOutAddr, unusedOutSize;
+    esp_err_t esp_err = esp_core_dump_image_get(&unusedOutAddr, &unusedOutSize);
     VerifyOrReturnValue(esp_err == ESP_OK, 0, ChipLogError(DeviceLayer, "Failed to get core dump image, 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<uint32_t>(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 +125,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<esp_core_dump_summary_t *>(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 +169,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 +219,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<off_t>(count), CHIP_ERROR_INVALID_ARGUMENT, outBuffer.reduce_size(0));
 
-        const uint8_t * readAddr = reinterpret_cast<const uint8_t *>(logContext->mappedAddress) + logContext->readOffset;
+        const uint8_t * readAddr = reinterpret_cast<const uint8_t *>(logContext->summary) + logContext->readOffset;
         memcpy(outBuffer.data(), readAddr, count);
         outBuffer.reduce_size(count);
 
@@ -260,8 +255,8 @@ CHIP_ERROR LogProvider::StartLogCollection(IntentEnum intent, LogSessionHandle &
     // 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"));
     }
 
     LogContext * context = reinterpret_cast<LogContext *>(Platform::MemoryCalloc(1, sizeof(LogContext)));

diff --git a/...s/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h b/...s/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h
@@ -19,8 +19,8 @@
 #pragma once
 
 #include <app/clusters/diagnostic-logs-server/DiagnosticLogsProviderDelegate.h>
+#include <esp_core_dump.h>
 #include <map>
-#include <spi_flash_mmap.h>
 
 namespace chip {
 namespace app {
@@ -57,19 +57,20 @@ 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;
+        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;
+            }
         }
     };
 

diff --git 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