From 44c388fb33858cc2bd749b64d5ce74d4caa590b7 Mon Sep 17 00:00:00 2001 From: Mark Holden Date: Tue, 10 May 2022 10:59:51 -0700 Subject: [PATCH] coredump: drivers: Add coredump device Add a pseudo device diver with device tree bindings for coredump. The device tree bindings exposes memory address/size values to be included in any dump. And the driver exposes an API to add/remove dump memory regions at runtime. Signed-off-by: Mark Holden --- MAINTAINERS.yml | 12 ++ doc/hardware/peripherals/coredump.rst | 27 +++ doc/hardware/peripherals/index.rst | 1 + doc/services/debugging/coredump.rst | 3 + drivers/CMakeLists.txt | 1 + drivers/Kconfig | 2 + drivers/coredump/CMakeLists.txt | 6 + drivers/coredump/Kconfig | 22 +++ drivers/coredump/coredump_impl.c | 184 +++++++++++++++++++++ dts/bindings/coredump/zephyr,coredump.yaml | 36 ++++ include/zephyr/drivers/coredump.h | 167 +++++++++++++++++++ subsys/debug/coredump/coredump_core.c | 19 +++ 12 files changed, 480 insertions(+) create mode 100644 doc/hardware/peripherals/coredump.rst create mode 100644 drivers/coredump/CMakeLists.txt create mode 100644 drivers/coredump/Kconfig create mode 100644 drivers/coredump/coredump_impl.c create mode 100644 dts/bindings/coredump/zephyr,coredump.yaml create mode 100644 include/zephyr/drivers/coredump.h diff --git a/MAINTAINERS.yml b/MAINTAINERS.yml index f408646de63..072fbce5f67 100644 --- a/MAINTAINERS.yml +++ b/MAINTAINERS.yml @@ -363,6 +363,7 @@ Debug: collaborators: - dcpleung - chen-png + - mrkhldn files: - include/zephyr/debug/ - subsys/debug/ @@ -545,6 +546,17 @@ Documentation: labels: - "area: Console" +"Drivers: Coredump": + status: maintained + maintainers: + - mrkhldn + files: + - drivers/coredump/ + - dts/bindings/coredump/ + - include/zephyr/drivers/coredump.h + labels: + - "area: Coredump" + "Drivers: Counter": status: maintained maintainers: diff --git a/doc/hardware/peripherals/coredump.rst b/doc/hardware/peripherals/coredump.rst new file mode 100644 index 00000000000..9072a972403 --- /dev/null +++ b/doc/hardware/peripherals/coredump.rst @@ -0,0 +1,27 @@ +.. _coredump_device_api: + +Coredump Device +#################### + +Overview +******** + +The coredump device is a pseudo-device driver with two types.A COREDUMP_TYPE_MEMCPY +type exposes device tree bindings for memory address/size values to be included in +any dump. And the driver exposes an API to add/remove dump memory regions at runtime. +A COREDUMP_TYPE_CALLBACK device requires exactly one entry in the memory-regions +array with a size of 0 and a desired size. The driver will statically allocate memory +of the desired size and provide an API to register a callback function to fill that +memory when a dump occurs. + +Configuration Options +********************* + +Related configuration options: + +* :kconfig:option:`CONFIG_COREDUMP_DEVICE` + +API Reference +************* + +.. doxygengroup:: coredump_device_interface diff --git a/doc/hardware/peripherals/index.rst b/doc/hardware/peripherals/index.rst index 475cab18d21..2226a3bf38c 100644 --- a/doc/hardware/peripherals/index.rst +++ b/doc/hardware/peripherals/index.rst @@ -9,6 +9,7 @@ Peripherals adc.rst audio/index.rst canbus/index.rst + coredump.rst counter.rst clock_control.rst dac.rst diff --git a/doc/services/debugging/coredump.rst b/doc/services/debugging/coredump.rst index 57714d943a0..74fdde3a7af 100644 --- a/doc/services/debugging/coredump.rst +++ b/doc/services/debugging/coredump.rst @@ -28,6 +28,9 @@ Here are the choices regarding memory dump: walking the stack in the debugger. Use this only if absolute minimum of data dump is desired. +Additional memory can be included in a dump (even with the "DEBUG_COREDUMP_MEMORY_DUMP_MIN" +config selected) through one or more :ref:`coredump devices ` + Usage ***** diff --git a/drivers/CMakeLists.txt b/drivers/CMakeLists.txt index 1d6b83e7301..5bb60897a4d 100644 --- a/drivers/CMakeLists.txt +++ b/drivers/CMakeLists.txt @@ -70,3 +70,4 @@ add_subdirectory_ifdef(CONFIG_MBOX mbox) add_subdirectory_ifdef(CONFIG_BOARD_XENVM xen) add_subdirectory_ifdef(CONFIG_MM_DRV mm) add_subdirectory_ifdef(CONFIG_RESET reset) +add_subdirectory_ifdef(CONFIG_COREDUMP_DEVICE coredump) diff --git a/drivers/Kconfig b/drivers/Kconfig index 5eeb5162baf..d2ce721b172 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -141,4 +141,6 @@ source "drivers/reset/Kconfig" source "drivers/mipi_dsi/Kconfig" +source "drivers/coredump/Kconfig" + endmenu diff --git a/drivers/coredump/CMakeLists.txt b/drivers/coredump/CMakeLists.txt new file mode 100644 index 00000000000..df95fde17d2 --- /dev/null +++ b/drivers/coredump/CMakeLists.txt @@ -0,0 +1,6 @@ +# Copyright Meta Platforms, Inc. and its affiliates. +# SPDX-License-Identifier: Apache-2.0 + +zephyr_library() + +zephyr_library_sources(coredump_impl.c) diff --git a/drivers/coredump/Kconfig b/drivers/coredump/Kconfig new file mode 100644 index 00000000000..d27dd9c63a1 --- /dev/null +++ b/drivers/coredump/Kconfig @@ -0,0 +1,22 @@ +# Copyright Meta Platforms, Inc. and its affiliates. +# SPDX-License-Identifier: Apache-2.0 + +menuconfig COREDUMP_DEVICE + bool "Coredump pseudo-device driver" + help + Enable support for a pseudo-device to help capturing + desired data into core dumps. + +if COREDUMP_DEVICE + +config COREDUMP_DEVICE_INIT_PRIORITY + int "Coredump device init priority" + default KERNEL_INIT_PRIORITY_DEVICE + help + Coredump pseudo-device driver initialization priority. + +module = COREDUMP_DEVICE +module-str = coredump device +source "subsys/logging/Kconfig.template.log_config" + +endif # COREDUMP_DEVICE diff --git a/drivers/coredump/coredump_impl.c b/drivers/coredump/coredump_impl.c new file mode 100644 index 00000000000..34da690017f --- /dev/null +++ b/drivers/coredump/coredump_impl.c @@ -0,0 +1,184 @@ +/* + * Copyright Meta Platforms, Inc. and its affiliates. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +#define DT_DRV_COMPAT zephyr_coredump + +enum COREDUMP_TYPE { + COREDUMP_TYPE_MEMCPY = 0, + COREDUMP_TYPE_CALLBACK = 1, +}; + +struct coredump_config { + /* Type of coredump device */ + enum COREDUMP_TYPE type; + + /* Length of memory_regions array */ + int length; + + /* Memory regions specified in device tree */ + size_t memory_regions[]; +}; + +struct coredump_data { + /* Memory regions registered at run time */ + sys_slist_t region_list; + + /* Callback to be invoked at time of dump */ + coredump_dump_callback_t dump_callback; +}; + +static void coredump_impl_dump(const struct device *dev) +{ + const struct coredump_config *config = dev->config; + struct coredump_data *data = dev->data; + + if (config->type == COREDUMP_TYPE_CALLBACK) { + if (data->dump_callback) { + uintptr_t start_address = config->memory_regions[0]; + size_t size = config->memory_regions[1]; + + /* Invoke callback to allow consumer to fill array with desired data */ + data->dump_callback(start_address, size); + coredump_memory_dump(start_address, start_address + size); + } + } else { /* COREDUMP_TYPE_MEMCPY */ + /* + * Add each memory region specified in device tree to the core dump, + * the memory_regions array should contain two entries per region + * containing the start address and size. + */ + if ((config->length > 0) && ((config->length % 2) == 0)) { + for (int i = 0; i < config->length; i += 2) { + uintptr_t start_address = config->memory_regions[i]; + size_t size = config->memory_regions[i+1]; + + coredump_memory_dump(start_address, start_address + size); + } + } + + sys_snode_t *node; + + /* Add each memory region registered at runtime to the core dump */ + SYS_SLIST_FOR_EACH_NODE(&data->region_list, node) { + struct coredump_mem_region_node *region; + + region = CONTAINER_OF(node, struct coredump_mem_region_node, node); + coredump_memory_dump(region->start, region->start + region->size); + } + } +} + +static bool coredump_impl_register_memory(const struct device *dev, + struct coredump_mem_region_node *region) +{ + const struct coredump_config *config = dev->config; + + if (config->type == COREDUMP_TYPE_CALLBACK) { + return false; + } + + struct coredump_data *data = dev->data; + + sys_slist_append(&data->region_list, ®ion->node); + return true; +} + +static bool coredump_impl_unregister_memory(const struct device *dev, + struct coredump_mem_region_node *region) +{ + const struct coredump_config *config = dev->config; + + if (config->type == COREDUMP_TYPE_CALLBACK) { + return false; + } + + struct coredump_data *data = dev->data; + + return sys_slist_find_and_remove(&data->region_list, ®ion->node); +} + +static bool coredump_impl_register_callback(const struct device *dev, + coredump_dump_callback_t callback) +{ + const struct coredump_config *config = dev->config; + + if (config->type == COREDUMP_TYPE_MEMCPY) { + return false; + } + + struct coredump_data *data = dev->data; + + data->dump_callback = callback; + return true; +} + +static int coredump_init(const struct device *dev) +{ + struct coredump_data *data = dev->data; + + sys_slist_init(&data->region_list); + return 0; +} + +static const struct coredump_driver_api coredump_api = { + .dump = coredump_impl_dump, + .register_memory = coredump_impl_register_memory, + .unregister_memory = coredump_impl_unregister_memory, + .register_callback = coredump_impl_register_callback, +}; + +#define INIT_REGION(node_id, prop, idx) DT_PROP_BY_IDX(node_id, prop, idx), +#define DT_INST_COREDUMP_IF_TYPE_CALLBACK(n, a, b) \ + COND_CODE_1(DT_INST_ENUM_IDX(n, coredump_type), a, b) + +#define CREATE_COREDUMP_DEVICE(n) \ + /* Statially allocate desired memory for the callback type device */ \ + DT_INST_COREDUMP_IF_TYPE_CALLBACK(n, \ + ( \ + BUILD_ASSERT(DT_INST_PROP_LEN(n, memory_regions) == 2, \ + "Allow exactly one entry (address and size) in memory_regions"); \ + BUILD_ASSERT(DT_INST_PROP_BY_IDX(n, memory_regions, 0) == 0, \ + "Verify address is set to 0"); \ + static uint8_t coredump_bytes[DT_INST_PROP_BY_IDX(n, memory_regions, 1)] \ + __aligned(4); \ + ), ()) \ + static struct coredump_data coredump_data_##n; \ + static const struct coredump_config coredump_config##n = { \ + .type = DT_INST_STRING_TOKEN_OR(n, coredump_type, COREDUMP_TYPE_MEMCPY), \ + COND_CODE_1(DT_INST_NODE_HAS_PROP(n, memory_regions), \ + ( \ + .length = DT_INST_PROP_LEN(n, memory_regions), \ + DT_INST_COREDUMP_IF_TYPE_CALLBACK(n, \ + ( \ + /* Callback type device has one entry in memory_regions array */ \ + .memory_regions = { \ + (size_t)&coredump_bytes[0], \ + DT_INST_PROP_BY_IDX(n, memory_regions, 1), \ + }, \ + ), \ + ( \ + .memory_regions = { \ + DT_INST_FOREACH_PROP_ELEM(n, memory_regions, INIT_REGION) \ + }, \ + )) \ + ), \ + ( \ + .length = 0, \ + )) \ + }; \ + DEVICE_DT_INST_DEFINE(n, \ + coredump_init, \ + NULL, \ + &coredump_data_##n, \ + &coredump_config##n, \ + PRE_KERNEL_1, \ + CONFIG_COREDUMP_DEVICE_INIT_PRIORITY, \ + &coredump_api); + +DT_INST_FOREACH_STATUS_OKAY(CREATE_COREDUMP_DEVICE) diff --git a/dts/bindings/coredump/zephyr,coredump.yaml b/dts/bindings/coredump/zephyr,coredump.yaml new file mode 100644 index 00000000000..2fcb460cff8 --- /dev/null +++ b/dts/bindings/coredump/zephyr,coredump.yaml @@ -0,0 +1,36 @@ +# Copyright Meta Platforms, Inc. and its affiliates. +# SPDX-License-Identifier: Apache-2.0 + +include: base.yaml + +compatible: "zephyr,coredump" + +description: Pseudo-device to help capturing desired data into core dumps + +properties: + label: + required: true + + memory-regions: + type: array + required: false + description: Start address and size of memory regions to be collected in a core dump + + coredump-type: + required: true + type: string + description: | + Designate which type of coredump device this will be. + A device of type COREDUMP_TYPE_MEMCPY can directly memcpy the provided memory-regions + into the coredump. The memory-regions array can contain 0 or more entries, and more + regions can be added at runtime through the coredump_device_register_memory API. + A device of type COREDUMP_TYPE_CALLBACK must specify exactly one entry in the + memory-regions array with a size of 0 and a desired size. The coredump device will statically + allocate a block of memory of the desired size and provide a callback with a pointer to + that memory which will be invoked at the time of a dump. This allows a consumer to add data + into the coredump that may not be directly accessible through a memcpy and/or provides an + opportunity to manipulate data for inclusion in the dump. The coredump_device_register_memory + API is not available for a device of this type. + enum: + - "COREDUMP_TYPE_MEMCPY" + - "COREDUMP_TYPE_CALLBACK" diff --git a/include/zephyr/drivers/coredump.h b/include/zephyr/drivers/coredump.h new file mode 100644 index 00000000000..2fd55134736 --- /dev/null +++ b/include/zephyr/drivers/coredump.h @@ -0,0 +1,167 @@ +/* + * Copyright Meta Platforms, Inc. and its affiliates. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @file + * @brief Public APIs for coredump pseudo-device driver + */ + +#ifndef INCLUDE_ZEPHYR_DRIVERS_COREDUMP_H_ +#define INCLUDE_ZEPHYR_DRIVERS_COREDUMP_H_ + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Coredump pseudo-device driver APIs + * @defgroup coredump_device_interface Coredump pseudo-device driver APIs + * @ingroup io_interfaces + * @{ + */ + +/** + * @brief Structure describing a region in memory that may be + * stored in core dump at the time it is generated + * + * Instances of this are passed to the coredump_device_register_memory() and + * coredump_device_unregister_memory() functions to indicate addition and removal + * of memory regions to be captured + */ +struct coredump_mem_region_node { + /** Node of single-linked list, do not modify */ + sys_snode_t node; + + /** Address of start of memory region */ + uintptr_t start; + + /** Size of memory region */ + size_t size; +}; + +/** + * @brief Callback that occurs at dump time, data copied into dump_area will + * be included in the dump that is generated + * + * @param dump_area Pointer to area to copy data into for inclusion in dump + * @param dump_area_size Size of available memory at dump_area + */ +typedef void (*coredump_dump_callback_t)(uintptr_t dump_area, size_t dump_area_size); + +/** + * @cond INTERNAL_HIDDEN + * + * For internal use only, skip these in public documentation. + */ + +/* + * Type definition of coredump API function for adding specified + * data into coredump + */ +typedef void (*coredump_device_dump_t)(const struct device *dev); + +/* + * Type definition of coredump API function for registering a memory + * region + */ +typedef bool (*coredump_device_register_memory_t)(const struct device *dev, + struct coredump_mem_region_node *region); + +/* + * Type definition of coredump API function for unregistering a memory + * region + */ +typedef bool (*coredump_device_unregister_memory_t)(const struct device *dev, + struct coredump_mem_region_node *region); + +/* + * Type definition of coredump API function for registering a dump + * callback + */ +typedef bool (*coredump_device_register_callback_t)(const struct device *dev, + coredump_dump_callback_t callback); + +/* + * API which a coredump pseudo-device driver should expose + */ +__subsystem struct coredump_driver_api { + coredump_device_dump_t dump; + coredump_device_register_memory_t register_memory; + coredump_device_unregister_memory_t unregister_memory; + coredump_device_register_callback_t register_callback; +}; + +/** + * @endcond + */ + +/** + * @brief Register a region of memory to be stored in core dump at the + * time it is generated + * + * @param dev Pointer to the device structure for the driver instance. + * @param region Struct describing memory to be collected + * + * @return true if registration succeeded + * @return false if registration failed + */ +static inline bool coredump_device_register_memory(const struct device *dev, + struct coredump_mem_region_node *region) +{ + const struct coredump_driver_api *api = + (const struct coredump_driver_api *)dev->api; + + return api->register_memory(dev, region); +} + +/** + * @brief Unregister a region of memory to be stored in core dump at the + * time it is generated + * + * @param dev Pointer to the device structure for the driver instance. + * @param region Struct describing memory to be collected + * + * @return true if unregistration succeeded + * @return false if unregistration failed + */ +static inline bool coredump_device_unregister_memory(const struct device *dev, + struct coredump_mem_region_node *region) +{ + const struct coredump_driver_api *api = + (const struct coredump_driver_api *)dev->api; + + return api->unregister_memory(dev, region); +} + +/** + * @brief Register a callback to be invoked at dump time + * + * @param dev Pointer to the device structure for the driver instance. + * @param callback Callback to be invoked at dump time + * + * @return true if registration succeeded + * @return false if registration failed + */ +static inline bool coredump_device_register_callback(const struct device *dev, + coredump_dump_callback_t callback) +{ + const struct coredump_driver_api *api = + (const struct coredump_driver_api *)dev->api; + + return api->register_callback(dev, callback); +} + +/** + * @} + */ + +#ifdef __cplusplus +} +#endif + +#endif /* INCLUDE_ZEPHYR_DRIVERS_COREDUMP_H_ */ diff --git a/subsys/debug/coredump/coredump_core.c b/subsys/debug/coredump/coredump_core.c index 5d82cb4bf79..6f04de1efcd 100644 --- a/subsys/debug/coredump/coredump_core.c +++ b/subsys/debug/coredump/coredump_core.c @@ -29,6 +29,11 @@ static struct coredump_backend_api #error "Need to select a coredump backend" #endif +#if defined(CONFIG_COREDUMP_DEVICE) +#include +#define DT_DRV_COMPAT zephyr_coredump +#endif + static void dump_header(unsigned int reason) { struct coredump_hdr_t hdr = { @@ -75,6 +80,15 @@ static void dump_thread(struct k_thread *thread) #endif } +#if defined(CONFIG_COREDUMP_DEVICE) +static void process_coredump_dev_memory(const struct device *dev) +{ + struct coredump_driver_api *api = (struct coredump_driver_api *)dev->api; + + api->dump(dev); +} +#endif + void process_memory_region_list(void) { #ifdef CONFIG_DEBUG_COREDUMP_MEMORY_DUMP_LINKER_RAM @@ -93,6 +107,11 @@ void process_memory_region_list(void) idx++; } #endif + +#if defined(CONFIG_COREDUMP_DEVICE) +#define MY_FN(inst) process_coredump_dev_memory(DEVICE_DT_INST_GET(inst)); + DT_INST_FOREACH_STATUS_OKAY(MY_FN) +#endif } void coredump(unsigned int reason, const z_arch_esf_t *esf,