[PATCH v3 0/3] Add command to display or save Linux PStore dumps
This serie of patches adds a new pstore command allowing to display or save ramoops logs (oops, panic, console, ftrace and user) generated by a previous kernel crash. PStore parameters can be set in U-Boot configuration file, or at run-time using "pstore set" command. For kernel using Device Tree, the parameters are dynamically added to Device Tree. Records size should be the same as the ones used by kernel, and should be a power of 2.
Since v2: - Add default value for PStore memory size - Remove default value of PStore memory address - Update config entry helps - Replace calls to debug() by log_debug() - Update documentation - Replace 1M test file by 3 * 4K files and build pstore memory during test - Add fdt_fixup_pstore() to pass PStore/Ramoops parameters to kernel
Since v1: - Fix 64bit mode build warnings - Add documentation - Add function description comments - Replace calls to pr_debug() by debug() - Add CONFIG_CMD_PSTORE to sandbox and sandbox64 - Add unit tests
Frédéric Danis (3): cmd: Add command to display or save Linux PStore dumps test: Add PStore command tests cmd: Fixup DT to pass PStore Ramoops parameters
cmd/Kconfig | 71 +++ cmd/Makefile | 1 + cmd/pstore.c | 543 +++++++++++++++++++++ common/image-fdt.c | 4 + configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + doc/index.rst | 7 + doc/pstore.rst | 76 +++ include/fdt_support.h | 3 + test/py/tests/test_pstore.py | 73 +++ test/py/tests/test_pstore_data_console.hex | Bin 0 -> 4096 bytes test/py/tests/test_pstore_data_panic1.hex | Bin 0 -> 4096 bytes test/py/tests/test_pstore_data_panic2.hex | Bin 0 -> 4096 bytes 13 files changed, 780 insertions(+) create mode 100644 cmd/pstore.c create mode 100644 doc/pstore.rst create mode 100644 test/py/tests/test_pstore.py create mode 100644 test/py/tests/test_pstore_data_console.hex create mode 100644 test/py/tests/test_pstore_data_panic1.hex create mode 100644 test/py/tests/test_pstore_data_panic2.hex
This patch adds a new pstore command allowing to display or save ramoops logs (oops, panic, console, ftrace and user) generated by a previous kernel crash. PStore parameters can be set in U-Boot configuration file, or at run-time using "pstore set" command. Records size should be the same as the ones used by kernel, and should be a power of 2. This command allows: - to display uncompressed logs - to save compressed or uncompressed logs, compressed logs are saved as a compressed stream, it may need some work to be able to decompress it, e.g. adding a fake header: "printf "\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x00" | cat - dmesg-ramoops-0.enc.z | gzip -dc" - ECC part is not used to check memory corruption - only 1st FTrace log is displayed or saved
Signed-off-by: Frédéric Danis frederic.danis@collabora.com --- cmd/Kconfig | 71 +++++++ cmd/Makefile | 1 + cmd/pstore.c | 505 +++++++++++++++++++++++++++++++++++++++++++++++++ doc/index.rst | 7 + doc/pstore.rst | 74 ++++++++ 5 files changed, 658 insertions(+) create mode 100644 cmd/pstore.c create mode 100644 doc/pstore.rst
diff --git a/cmd/Kconfig b/cmd/Kconfig index 6403bc45a5..cd202bf7fb 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1736,6 +1736,77 @@ config CMD_QFW feature is to allow easy loading of files passed to qemu-system via -kernel / -initrd
+config CMD_PSTORE + bool "pstore" + help + This provides access to Linux PStore with Rammoops backend. The main + feature is to allow to display or save PStore records. + + See doc/pstore.rst for more information. + +if CMD_PSTORE + +config CMD_PSTORE_MEM_ADDR + hex "Memory Address" + depends on CMD_PSTORE + help + Base addr used for PStore ramoops memory, should be identical to + ramoops.mem_address parameter used by kernel + +config CMD_PSTORE_MEM_SIZE + hex "Memory size" + depends on CMD_PSTORE + default "0x10000" + help + Size of PStore ramoops memory, should be identical to ramoops.mem_size + parameter used by kernel, a power of 2 and larger than the sum of the + record sizes + +config CMD_PSTORE_RECORD_SIZE + hex "Dump record size" + depends on CMD_PSTORE + default "0x1000" + help + Size of each dump done on oops/panic, should be identical to + ramoops.record_size parameter used by kernel and a power of 2 + Must be non-zero + +config CMD_PSTORE_CONSOLE_SIZE + hex "Kernel console log size" + depends on CMD_PSTORE + default "0x1000" + help + Size of kernel console log, should be identical to + ramoops.console_size parameter used by kernel and a power of 2 + Must be non-zero + +config CMD_PSTORE_FTRACE_SIZE + hex "FTrace log size" + depends on CMD_PSTORE + default "0x1000" + help + Size of ftrace log, should be identical to ramoops.ftrace_size + parameter used by kernel and a power of 2 + +config CMD_PSTORE_PMSG_SIZE + hex "User space message log size" + depends on CMD_PSTORE + default "0x1000" + help + Size of user space message log, should be identical to + ramoops.pmsg_size parameter used by kernel and a power of 2 + +config CMD_PSTORE_ECC_SIZE + int "ECC size" + depends on CMD_PSTORE + default "0" + help + if non-zero, the option enables ECC support and specifies ECC buffer + size in bytes (1 is a special value, means 16 bytes ECC), should be + identical to ramoops.ramoops_ecc parameter used by kernel + +endif + source "cmd/mvebu/Kconfig"
config CMD_TERMINAL diff --git a/cmd/Makefile b/cmd/Makefile index f1dd513a4b..06d7ad7375 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -110,6 +110,7 @@ obj-$(CONFIG_CMD_PCI) += pci.o endif obj-$(CONFIG_CMD_PINMUX) += pinmux.o obj-$(CONFIG_CMD_PMC) += pmc.o +obj-$(CONFIG_CMD_PSTORE) += pstore.o obj-$(CONFIG_CMD_PXE) += pxe.o pxe_utils.o obj-$(CONFIG_CMD_WOL) += wol.o obj-$(CONFIG_CMD_QFW) += qfw.o diff --git a/cmd/pstore.c b/cmd/pstore.c new file mode 100644 index 0000000000..4e4d70d604 --- /dev/null +++ b/cmd/pstore.c @@ -0,0 +1,505 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright © 2019 Collabora Ltd + */ + +#include <config.h> +#include <common.h> +#include <fs.h> +#include <mapmem.h> +#include <memalign.h> +#include <part.h> + +struct persistent_ram_buffer { + u32 sig; + u32 start; + u32 size; + u8 data[0]; +}; + +#define PERSISTENT_RAM_SIG (0x43474244) /* DBGC */ +#define RAMOOPS_KERNMSG_HDR "====" + +#define PSTORE_TYPE_DMESG 0 +#define PSTORE_TYPE_CONSOLE 2 +#define PSTORE_TYPE_FTRACE 3 +#define PSTORE_TYPE_PMSG 7 +#define PSTORE_TYPE_ALL 255 + +static phys_addr_t pstore_addr = CONFIG_CMD_PSTORE_MEM_ADDR; +static phys_size_t pstore_length = CONFIG_CMD_PSTORE_MEM_SIZE; +static unsigned int pstore_record_size = CONFIG_CMD_PSTORE_RECORD_SIZE; +static unsigned int pstore_console_size = CONFIG_CMD_PSTORE_CONSOLE_SIZE; +static unsigned int pstore_ftrace_size = CONFIG_CMD_PSTORE_FTRACE_SIZE; +static unsigned int pstore_pmsg_size = CONFIG_CMD_PSTORE_PMSG_SIZE; +static unsigned int pstore_ecc_size = CONFIG_CMD_PSTORE_ECC_SIZE; +static unsigned int buffer_size; + + /** + * pstore_read_kmsg_hdr() - Check kernel header and get compression flag if + * available. + * @buffer: Kernel messages buffer. + * @compressed: Returns TRUE if kernel buffer is compressed, else FALSE. + * + * Check if buffer starts with a kernel header of the form: + * ====<secs>.<nsecs>[-<compression>]\n + * If <compression> is equal to 'C' then the buffer is compressed, else iter + * should be 'D'. + * + * Return: Length of kernel header. + */ +static int pstore_read_kmsg_hdr(char *buffer, bool *compressed) +{ + char *ptr = buffer; + *compressed = false; + + if (strncmp(RAMOOPS_KERNMSG_HDR, ptr, strlen(RAMOOPS_KERNMSG_HDR)) != 0) + return 0; + + ptr += strlen(RAMOOPS_KERNMSG_HDR); + + ptr = strchr(ptr, '\n'); + if (!ptr) + return 0; + + if (ptr[-2] == '-' && ptr[-1] == 'C') + *compressed = true; + + return ptr - buffer + 1; +} + +/** + * pstore_get_buffer() - Get unwrapped record buffer + * @sig: Signature to check + * @buffer: Buffer containing wrapped record + * @size: wrapped record size + * @dest: Buffer used to store unwrapped record + * + * The record starts with <signature><start><size> header. + * The signature is 'DBGC' for all records except for Ftrace's record(s) wich + * use LINUX_VERSION_CODE ^ 'DBGC'. + * Use 0 for @sig to prevent checking signature. + * Start and size are 4 bytes long. + * + * Return: record's length + */ +static u32 pstore_get_buffer(u32 sig, phys_addr_t buffer, u32 size, char *dest) +{ + struct persistent_ram_buffer *prb = + (struct persistent_ram_buffer *)map_sysmem(buffer, size); + u32 dest_size; + + if (sig == 0 || prb->sig == sig) { + if (prb->size == 0) { + log_debug("found existing empty buffer\n"); + return 0; + } + + if (prb->size > size) { + log_debug("found existing invalid buffer, size %u, start %u\n", + prb->size, prb->start); + return 0; + } + } else { + log_debug("no valid data in buffer (sig = 0x%08x)\n", prb->sig); + return 0; + } + + log_debug("found existing buffer, size %u, start %u\n", + prb->size, prb->start); + + memcpy(dest, &prb->data[prb->start], prb->size - prb->start); + memcpy(dest + prb->size - prb->start, &prb->data[0], prb->start); + + dest_size = prb->size; + unmap_sysmem(prb); + + return dest_size; +} + +/** + * pstore_init_buffer_size() - Init buffer size to largest record size + * + * Records, console, FTrace and user logs can use different buffer sizes. + * This function allows to retrieve the biggest one. + */ +static void pstore_init_buffer_size(void) +{ + if (pstore_record_size > buffer_size) + buffer_size = pstore_record_size; + + if (pstore_console_size > buffer_size) + buffer_size = pstore_console_size; + + if (pstore_ftrace_size > buffer_size) + buffer_size = pstore_ftrace_size; + + if (pstore_pmsg_size > buffer_size) + buffer_size = pstore_pmsg_size; +} + +/** + * pstore_set() - Initialize PStore settings from command line arguments + * @cmdtp: Command data struct pointer + * @flag: Command flag + * @argc: Command-line argument count + * @argv: Array of command-line arguments + * + * Set pstore reserved memory info, starting at 'addr' for 'len' bytes. + * Default length for records is 4K. + * Mandatory arguments: + * - addr: ramoops starting address + * - len: ramoops total length + * Optional arguments: + * - record-size: size of one panic or oops record ('dump' type) + * - console-size: size of the kernel logs record + * - ftrace-size: size of the ftrace record(s), this can be a single record or + * divided in parts based on number of CPUs + * - pmsg-size: size of the user space logs record + * - ecc-size: enables/disables ECC support and specifies ECC buffer size in + * bytes (0 disables it, 1 is a special value, means 16 bytes ECC) + * + * Return: zero on success, CMD_RET_USAGE in case of misuse and negative + * on error. + */ +static int pstore_set(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + if (argc < 3) + return CMD_RET_USAGE; + + /* Address is specified since argc > 2 + */ + pstore_addr = simple_strtoul(argv[1], NULL, 16); + + /* Length is specified since argc > 2 + */ + pstore_length = simple_strtoul(argv[2], NULL, 16); + + if (argc > 3) + pstore_record_size = simple_strtoul(argv[3], NULL, 16); + + if (argc > 4) + pstore_console_size = simple_strtoul(argv[4], NULL, 16); + + if (argc > 5) + pstore_ftrace_size = simple_strtoul(argv[5], NULL, 16); + + if (argc > 6) + pstore_pmsg_size = simple_strtoul(argv[6], NULL, 16); + + if (argc > 7) + pstore_ecc_size = simple_strtoul(argv[7], NULL, 16); + + if (pstore_length < (pstore_record_size + pstore_console_size + + pstore_ftrace_size + pstore_pmsg_size)) { + printf("pstore <len> should be larger than the sum of all records sizes\n"); + pstore_length = 0; + } + + log_debug("pstore set done: start 0x%08llx - length 0x%llx\n", + (unsigned long long)pstore_addr, + (unsigned long long)pstore_length); + + return 0; +} + +/** + * pstore_print_buffer() - Print buffer + * @type: buffer type + * @buffer: buffer to print + * @size: buffer size + * + * Print buffer type and content + */ +static void pstore_print_buffer(char *type, char *buffer, u32 size) +{ + u32 i = 0; + + printf("**** %s\n", type); + while (i < size && buffer[i] != 0) { + putc(buffer[i]); + i++; + } +} + +/** + * pstore_display() - Display existing records in pstore reserved memory + * @cmdtp: Command data struct pointer + * @flag: Command flag + * @argc: Command-line argument count + * @argv: Array of command-line arguments + * + * A 'record-type' can be given to only display records of this kind. + * If no 'record-type' is given, all valid records are dispayed. + * 'record-type' can be one of 'dump', 'console', 'ftrace' or 'user'. For 'dump' + * and 'ftrace' types, a 'nb' can be given to only display one record. + * + * Return: zero on success, CMD_RET_USAGE in case of misuse and negative + * on error. + */ +static int pstore_display(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + int type = PSTORE_TYPE_ALL; + phys_addr_t ptr; + char *buffer; + u32 size; + int header_len = 0; + bool compressed; + + if (argc > 1) { + if (!strcmp(argv[1], "dump")) + type = PSTORE_TYPE_DMESG; + else if (!strcmp(argv[1], "console")) + type = PSTORE_TYPE_CONSOLE; + else if (!strcmp(argv[1], "ftrace")) + type = PSTORE_TYPE_FTRACE; + else if (!strcmp(argv[1], "user")) + type = PSTORE_TYPE_PMSG; + else + return CMD_RET_USAGE; + } + + if (pstore_length == 0) { + printf("Please set PStore configuration\n"); + return CMD_RET_USAGE; + } + + if (buffer_size == 0) + pstore_init_buffer_size(); + + buffer = malloc_cache_aligned(buffer_size); + + if (type == PSTORE_TYPE_DMESG || type == PSTORE_TYPE_ALL) { + ptr = pstore_addr; + phys_addr_t ptr_end = ptr + pstore_length - pstore_pmsg_size + - pstore_ftrace_size - pstore_console_size; + + if (argc > 2) { + ptr += simple_strtoul(argv[2], NULL, 10) + * pstore_record_size; + ptr_end = ptr + pstore_record_size; + } + + while (ptr < ptr_end) { + size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, + pstore_record_size, buffer); + ptr += pstore_record_size; + + if (size == 0) + continue; + + header_len = pstore_read_kmsg_hdr(buffer, &compressed); + if (header_len == 0) { + log_debug("no valid kernel header\n"); + continue; + } + + if (compressed) { + printf("Compressed buffer, display not available\n"); + continue; + } + + pstore_print_buffer("Dump", buffer + header_len, + size - header_len); + } + } + + if (type == PSTORE_TYPE_CONSOLE || type == PSTORE_TYPE_ALL) { + ptr = pstore_addr + pstore_length - pstore_pmsg_size + - pstore_ftrace_size - pstore_console_size; + size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, + pstore_console_size, buffer); + if (size != 0) + pstore_print_buffer("Console", buffer, size); + } + + if (type == PSTORE_TYPE_FTRACE || type == PSTORE_TYPE_ALL) { + ptr = pstore_addr + pstore_length - pstore_pmsg_size + - pstore_ftrace_size; + /* The FTrace record(s) uses LINUX_VERSION_CODE ^ 'DBGC' + * signature, pass 0 to pstore_get_buffer to prevent + * checking it + */ + size = pstore_get_buffer(0, ptr, pstore_ftrace_size, buffer); + if (size != 0) + pstore_print_buffer("FTrace", buffer, size); + } + + if (type == PSTORE_TYPE_PMSG || type == PSTORE_TYPE_ALL) { + ptr = pstore_addr + pstore_length - pstore_pmsg_size; + size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, + pstore_pmsg_size, buffer); + if (size != 0) + pstore_print_buffer("User", buffer, size); + } + + free(buffer); + + return 0; +} + +/** + * pstore_save() - Save existing records from pstore reserved memory + * @cmdtp: Command data struct pointer + * @flag: Command flag + * @argc: Command-line argument count + * @argv: Array of command-line arguments + * + * the records are saved under 'directory path', which should already exist, + * to partition 'part' on device type 'interface' instance 'dev' + * Filenames are automatically generated, depending on record type, like in + * /sys/fs/pstore under Linux + * + * Return: zero on success, CMD_RET_USAGE in case of misuse and negative + * on error. + */ +static int pstore_save(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + phys_addr_t ptr, ptr_end; + char *buffer; + char *save_argv[6]; + char addr[19], length[19]; + char path[256]; + u32 size; + unsigned int index; + int header_len = 0; + bool compressed; + + if (argc < 4) + return CMD_RET_USAGE; + + if (pstore_length == 0) { + printf("Please set PStore configuration\n"); + return CMD_RET_USAGE; + } + + if (buffer_size == 0) + pstore_init_buffer_size(); + + buffer = malloc_cache_aligned(buffer_size); + sprintf(addr, "0x%p", buffer); + + save_argv[0] = argv[0]; + save_argv[1] = argv[1]; + save_argv[2] = argv[2]; + save_argv[3] = addr; + save_argv[4] = path; + save_argv[5] = length; + + /* Save all Dump records */ + ptr = pstore_addr; + ptr_end = ptr + pstore_length - pstore_pmsg_size - pstore_ftrace_size + - pstore_console_size; + index = 0; + while (ptr < ptr_end) { + size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, + pstore_record_size, buffer); + ptr += pstore_record_size; + + if (size == 0) + continue; + + header_len = pstore_read_kmsg_hdr(buffer, &compressed); + if (header_len == 0) { + log_debug("no valid kernel header\n"); + continue; + } + + sprintf(addr, "0x%08lx", (ulong)map_to_sysmem(buffer + header_len)); + sprintf(length, "0x%X", size - header_len); + sprintf(path, "%s/dmesg-ramoops-%u%s", argv[3], index, + compressed ? ".enc.z" : ""); + do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY); + index++; + } + + sprintf(addr, "0x%08lx", (ulong)map_to_sysmem(buffer)); + + /* Save Console record */ + size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, pstore_console_size, + buffer); + if (size != 0) { + sprintf(length, "0x%X", size); + sprintf(path, "%s/console-ramoops-0", argv[3]); + do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY); + } + ptr += pstore_console_size; + + /* Save FTrace record(s) + * The FTrace record(s) uses LINUX_VERSION_CODE ^ 'DBGC' signature, + * pass 0 to pstore_get_buffer to prevent checking it + */ + size = pstore_get_buffer(0, ptr, pstore_ftrace_size, buffer); + if (size != 0) { + sprintf(length, "0x%X", size); + sprintf(path, "%s/ftrace-ramoops-0", argv[3]); + do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY); + } + ptr += pstore_ftrace_size; + + /* Save Console record */ + size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, pstore_pmsg_size, + buffer); + if (size != 0) { + sprintf(length, "0x%X", size); + sprintf(path, "%s/pmsg-ramoops-0", argv[3]); + do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY); + } + + free(buffer); + + return 0; +} + +static cmd_tbl_t cmd_pstore_sub[] = { + U_BOOT_CMD_MKENT(set, 8, 0, pstore_set, "", ""), + U_BOOT_CMD_MKENT(display, 3, 0, pstore_display, "", ""), + U_BOOT_CMD_MKENT(save, 4, 0, pstore_save, "", ""), +}; + +static int do_pstore(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + cmd_tbl_t *c; + + if (argc < 2) + return CMD_RET_USAGE; + + /* Strip off leading argument */ + argc--; + argv++; + + c = find_cmd_tbl(argv[0], cmd_pstore_sub, ARRAY_SIZE(cmd_pstore_sub)); + + if (!c) + return CMD_RET_USAGE; + + return c->cmd(cmdtp, flag, argc, argv); +} + +U_BOOT_CMD(pstore, 10, 0, do_pstore, + "Manage Linux Persistent Storage", + "set <addr> <len> [record-size] [console-size] [ftrace-size] [pmsg_size] [ecc-size]\n" + "- Set pstore reserved memory info, starting at 'addr' for 'len' bytes.\n" + " Default length for records is 4K.\n" + " 'record-size' is the size of one panic or oops record ('dump' type).\n" + " 'console-size' is the size of the kernel logs record.\n" + " 'ftrace-size' is the size of the ftrace record(s), this can be a single\n" + " record or divided in parts based on number of CPUs.\n" + " 'pmsg-size' is the size of the user space logs record.\n" + " 'ecc-size' enables/disables ECC support and specifies ECC buffer size in\n" + " bytes (0 disables it, 1 is a special value, means 16 bytes ECC).\n" + "pstore display [record-type] [nb]\n" + "- Display existing records in pstore reserved memory. A 'record-type' can\n" + " be given to only display records of this kind. 'record-type' can be one\n" + " of 'dump', 'console', 'ftrace' or 'user'. For 'dump' and 'ftrace' types,\n" + " a 'nb' can be given to only display one record.\n" + "pstore save <interface> <dev[:part]> <directory-path>\n" + "- Save existing records in pstore reserved memory under 'directory path'\n" + " to partition 'part' on device type 'interface' instance 'dev'.\n" + " Filenames are automatically generated, depending on record type, like\n" + " in /sys/fs/pstore under Linux.\n" + " The 'directory-path' should already exist.\n" +); diff --git a/doc/index.rst b/doc/index.rst index cd98be6cc5..c556cdb607 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -98,6 +98,13 @@ Android-specific features available in U-Boot.
android/index
+Command line +------------ +.. toctree:: + :maxdepth: 2 + + pstore.rst + Indices and tables ==================
diff --git a/doc/pstore.rst b/doc/pstore.rst new file mode 100644 index 0000000000..90a8e1c2cb --- /dev/null +++ b/doc/pstore.rst @@ -0,0 +1,74 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +PStore command +============== + +Design +------ + +Linux PStore and Ramoops modules (Linux config options PSTORE and PSTORE_RAM) +allow to use memory to pass data from the dying breath of a crashing kernel to +its successor. This command allows to read those records from U-Boot command +line. + +Ramoops is an oops/panic logger that writes its logs to RAM before the system +crashes. It works by logging oopses and panics in a circular buffer. Ramoops +needs a system with persistent RAM so that the content of that area can survive +after a restart. + +Ramoops uses a predefined memory area to store the dump. + +Ramoops parameters can be passed as kernel parameters or through Device Tree, +i.e.:: + ramoops.mem_address=0x30000000 ramoops.mem_size=0x100000 ramoops.record_size=0x2000 ramoops.console_size=0x2000 memmap=0x100000$0x30000000 + +The same values should be set in U-Boot to be able to retrieve the records. +This values can be set at build time in U-Boot configuration file, or at runtime. + +The PStore configuration parameters are: + +======================= ========== + Name Default +======================= ========== +CMD_PSTORE_MEM_ADDR +CMD_PSTORE_MEM_SIZE 0x10000 +CMD_PSTORE_RECORD_SIZE 0x1000 +CMD_PSTORE_CONSOLE_SIZE 0x1000 +CMD_PSTORE_FTRACE_SIZE 0x1000 +CMD_PSTORE_PMSG_SIZE 0x1000 +CMD_PSTORE_ECC_SIZE 0 +======================= ========== + +Records sizes should be a power of 2. +The memory size and the record/console size must be non-zero. + +Multiple 'dump' records can be stored in the memory reserved for PStore. +The memory size has to be larger than the sum of the record sizes, i.e.:: + MEM_SIZE >= RECORD_SIZE * n + CONSOLE_SIZE + FTRACE_SIZE + PMSG_SIZE + +Usage +----- + +Generate kernel crash +~~~~~~~~~~~~~~~~~~~~~ + +For test purpose, you can generate a kernel crash by setting reboot timeout to +10 seconds and trigger a panic:: + $ sudo sh -c "echo 1 > /proc/sys/kernel/sysrq" + $ sudo sh -c "echo 10 > /proc/sys/kernel/panic" + $ sudo sh -c "echo c > /proc/sysrq-trigger" + +Retrieve logs in U-Boot +~~~~~~~~~~~~~~~~~~~~~~~ + +First of all, unless PStore parameters as been set during U-Boot configuration +and match kernel ramoops parameters, it needs to be set using 'pstore set', e.g.:: + => pstore set 0x30000000 0x100000 0x2000 0x2000 + +Then all available dumps can be displayed +using:: + => pstore display + +Or saved to an existing directory in an Ext2 or Ext4 partition, e.g. on root +directory of 1st partition of the 2nd MMC:: + => pstore save mmc 1:1 /
Add PStore command to sandbox and sandbox64 defconfigs. Add test checking: - 'pstore display' of all records - 'pstore display' only the 2nd dump record - 'pstore save' of all records
Signed-off-by: Frédéric Danis frederic.danis@collabora.com --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + test/py/tests/test_pstore.py | 73 +++++++++++++++++++++ test/py/tests/test_pstore_data_console.hex | Bin 0 -> 4096 bytes test/py/tests/test_pstore_data_panic1.hex | Bin 0 -> 4096 bytes test/py/tests/test_pstore_data_panic2.hex | Bin 0 -> 4096 bytes 6 files changed, 75 insertions(+) create mode 100644 test/py/tests/test_pstore.py create mode 100644 test/py/tests/test_pstore_data_console.hex create mode 100644 test/py/tests/test_pstore_data_panic1.hex create mode 100644 test/py/tests/test_pstore_data_panic2.hex
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 71a4d7fccb..c36cac1ffc 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -68,6 +68,7 @@ CONFIG_CMD_CBFS=y CONFIG_CMD_CRAMFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_MTDPARTS=y +CONFIG_CMD_PSTORE=y CONFIG_MAC_PARTITION=y CONFIG_AMIGA_PARTITION=y CONFIG_OF_CONTROL=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index f96891ecae..155190a3d3 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -77,6 +77,7 @@ CONFIG_CMD_CBFS=y CONFIG_CMD_CRAMFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_MTDPARTS=y +CONFIG_CMD_PSTORE=y CONFIG_MAC_PARTITION=y CONFIG_AMIGA_PARTITION=y CONFIG_OF_CONTROL=y diff --git a/test/py/tests/test_pstore.py b/test/py/tests/test_pstore.py new file mode 100644 index 0000000000..5c7e5cea74 --- /dev/null +++ b/test/py/tests/test_pstore.py @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2020, Collabora +# Author: Frédéric Danis frederic.danis@collabora.com + +import pytest +import u_boot_utils +import tempfile +import shutil + +PSTORE_ADDR=0x3000000 +PSTORE_LENGTH=0x100000 +PSTORE_PANIC1='test/py/tests/test_pstore_data_panic1.hex' +PSTORE_PANIC2='test/py/tests/test_pstore_data_panic2.hex' +PSTORE_CONSOLE='test/py/tests/test_pstore_data_console.hex' +ADDR=0x01000008 + +def load_pstore(u_boot_console): + """Load PStore records from sample files""" + + output = u_boot_console.run_command_list([ + 'host load hostfs - 0x%x %s' % (PSTORE_ADDR, PSTORE_PANIC1), + 'host load hostfs - 0x%x %s' % (PSTORE_ADDR + 4096, PSTORE_PANIC2), + 'host load hostfs - 0x%x %s' % (PSTORE_ADDR + 253 * 4096, PSTORE_CONSOLE), + 'pstore set 0x%x 0x%x' % (PSTORE_ADDR, PSTORE_LENGTH)]) + +def checkfile(u_boot_console, path, filesize, checksum): + """Check file against MD5 checksum""" + + output = u_boot_console.run_command_list([ + 'load hostfs - %x %s' % (ADDR, path), + 'printenv filesize']) + assert('filesize=%x' % (filesize) in ''.join(output)) + + output = u_boot_console.run_command_list([ + 'md5sum %x $filesize' % ADDR, + 'setenv filesize']) + assert(checksum in ''.join(output)) + +@pytest.mark.buildconfigspec('cmd_pstore') +def test_pstore_display_all_records(u_boot_console): + """Test that pstore displays all records.""" + + u_boot_console.run_command('') + load_pstore(u_boot_console) + response = u_boot_console.run_command('pstore display') + assert('**** Dump' in response) + assert('**** Console' in response) + +@pytest.mark.buildconfigspec('cmd_pstore') +def test_pstore_display_one_record(u_boot_console): + """Test that pstore displays only one record.""" + + u_boot_console.run_command('') + load_pstore(u_boot_console) + response = u_boot_console.run_command('pstore display dump 1') + assert('Panic#2 Part1' in response) + assert('**** Console' not in response) + +@pytest.mark.buildconfigspec('cmd_pstore') +def test_pstore_save_records(u_boot_console): + """Test that pstore saves all records.""" + + outdir = tempfile.mkdtemp() + + u_boot_console.run_command('') + load_pstore(u_boot_console) + u_boot_console.run_command('pstore save hostfs - %s' % (outdir)) + + checkfile(u_boot_console, '%s/dmesg-ramoops-0' % (outdir), 3798, '8059335ab4cfa62c77324c491659c503') + checkfile(u_boot_console, '%s/dmesg-ramoops-1' % (outdir), 4035, '3ff30df3429d81939c75d0070b5187b9') + checkfile(u_boot_console, '%s/console-ramoops-0' % (outdir), 4084, 'bb44de4a9b8ebd9b17ae98003287325b') + + shutil.rmtree(outdir) diff --git a/test/py/tests/test_pstore_data_console.hex b/test/py/tests/test_pstore_data_console.hex new file mode 100644 index 0000000000000000000000000000000000000000..e7f426e8928a2793457baced5f66ee165ef429f6 GIT binary patch literal 4096 zcmcgv%TnVw6lJ!(nP0e*4NyZ8zvNd{7IX(_s%e@b&{Z=OMcI}D8f?ie*$MgY`GC1f z?1ZL6Cktj4lpUjc?>YA&9Sz@~d<cKp+4<+!ogJz&$oSm@K6`cyiWPFg4nS8)#lU-a z1K>Db&-IDxkz1&BYW{HH_2@lNt}`hF%c=vQY{D}JqApUVz+M^#mQS38q21kR=dA1k z656*dwDxHrn#gIb!N!=1-E&>xgDwrjz_Af2FP@z4lvdzaX=YhgZ%XBT48sLX{ZLm_ zPDKnyPbK0<-l>$;Z%Z$c>pK{J@i~S|h6zy>7x$oN7_fK;cGUADG6$&=L1hs`0QKS( zril``cu(&`!L?=-Xw9IKpfIgDFSv1Kf*Ch~>qQYlPHM`l7^+#x2DWEeiw}D?30&Lu zqsYURQS9u;kd$Sj3aJL2(beJA^{4}~WayepG3b;^j(N`Ra+%N$G`|L&T41flTrm77 zl7bC7{aUMj%0)j*1m-ZU)vU{&ScXQN4qV@YOcuqU(?+|)pqi=961Pu$5^ROr3u5<e zapyekE1qT#SQ)0Cy>>IHSVMt?8z_X9<$KXq6bOmEx2Ld5{qrht!K;=+w26WfMvl7` z1>cRy|3%?>6@pC^h-aB;+hPDB68mpU;l%*dCB+f#>I%E4o7RHnOt%9;Ht`58$JuI- zLo0}bx8-35VtqH2im9T@M1}KRN-nA;A$J3z$nv)=j3E@hEur;=!7Ztj0?Bl_pzNtF zjlYSBYZKSnro{}C=xv$&q%>0FNuaB>)$ulQA_0lE+J8fr#J2Ueq+XO~-ehexHVu3f z+HSPfz=tG?ZTr7P-qfH4|I;(Wv3r4M`2m9e%rnJu_%!+}!XbeZexIbgf{TxTf@6nx zvRFM<JuoF$g~G@3@z}1a1P?4aUqcn2pi;Vd-OVlR6`bFp)``o9*<C`Z8m#qv09<_= zTzok@+n=A!2BeRA4SVDCVuFDWhr`PcU(S#B&X2B!M_=!*FEFDB&Wa)D;t{w8A&m%1 z=xpJE^g`lz7M?3(mi@q|u{~>SuS@8HeUg^Ce_ZD|{ut+SirtG_j?6(l^{c0)Tij6y z4&r)XA3h$QpB#M;E@e3LOn;9qvWai`w}3mY{Vg_+p8W@WJv%&yQC*f&RWPx8;UcQv zL!CC4SPSt+yMN1Ci6o~t4smZ|I8IP7(YZdu6hQM|Fft1IlSK7%dEalo_I>6?Nk~#^ z(a>XF5~1RxV-lsrVTlz)D65$2+NS<XM^b^3D5+7P9RTjbMo6L>G+G<5XJn0kW58}f z<v3faRAa?7LPcSXT`(_=<G8)R_Cnvg#kE!TFqgIM8y2*EMRiW+RmG#MY=kYE`mXI` z`rAY_LO2E|GAK#}f*pa`)|f>M{uAkHkc1BL2$tc6J8@%UR!1|D8KNeQiv=1`DMOme z?6J8p=$<K&xnfv8DlsIr-W3znxEh_FTpeA0UR0_`P$%I>l(?Bes(Zbaw+Qo(-P?M5 zQSeKaF&sa-bU0SBfQ!>1-YWK5n<h8=OQ$$|mKmxS3mUd7h@Vc+&Nf4IT-=slBm6N@ z6fY5`9~L!+U2~)PeffL$0C4-z*ic}#DRjJu82zyp-?b=YE#_C4d0#IUt@7`XD2%X_ zH^h#xc$at)Y?kl#cjv>`l_H-37ck}31Kv1HH*46{njYvf$=F?HcDDmvrxTgu4T3bU zgZ}O+g#jS}l74@6c|1a2V+niB)v+kDG=H>9bHTxz<s;0Uf^ll9jC&LlNs4pY(sdmt zFv@o=-9y}mo;O~-pwH+MitB`)7rtUXWJy->2~I4A*3+b1CCYVOyobGFXTr{xV}Cau yo`=L!&&H!;E#e_Fr-O-?OIiLfAGn|6h`eHJ!alb`>%~#CVvF0Qi-|XP{eJ;km?x0{
literal 0 HcmV?d00001
diff --git a/test/py/tests/test_pstore_data_panic1.hex b/test/py/tests/test_pstore_data_panic1.hex new file mode 100644 index 0000000000000000000000000000000000000000..988929d12c25fee4c4242776a2ed0d78cdc55948 GIT binary patch literal 4096 zcmb_f-;dn35$0X4!PN+gYmt|}4U(5LnsX>oqNGI<taBH~24eL5qeW5UAW$TAD|odl zOWN}#Pg|hqbN__&AMD@IkJ{C)WS@hgs2028kTc&5haZRBi*LU_|MzbgbKkG=cwr<w zPlzyjkZ>U-KfAd1$7R)g?$M>HTkqai!8fmnyeN7Q${^(Yil``WO<hv6FlnXB)Rf9D zu8V~w+b+~iaYN>9Ix}UFLa}f6+H_s5(nim$>3Z6tv^7;~VdrU6l-1l4pwri7lg{kC z!Wg_bhiL)38$sG?YL=^o(aYL)1MSS#rJ6fCvNy)wxL)}t)zvDm?HkIowE3|~b4-e= zw`H2msi@LcWpmrmZQoF<c9}N;Wo@0Kds)*@O>0tP^|GvL+cX<_C*9u8R5PQW=0ygj ze6CtcwJMBEt2cyBU2EZM+Pu<hyRdZuao(i*x?Zj-QZv0t5R|)i1&n-&No+d^<1<t1 zE{oeXt>#tHDYS~LLzrnpE2Gmc9gT5a*1bBbja<#!yrfl=bvk&LMY9<%{DB@dx+&6f zX-F+pyR6dMUat@nQc=x*Ov~FQg)P=oi*%-tOg(>{BB4|ky0-HHQKToh1X0c}|1=@+ z=*h)|g2<=yWm!%LS$nC9MQhE3zNh{<?;F7bFL}Vv<e93=vLf*C!e)h9P&Xi>GhZ*m zp)=nPV*iRBtJ=I(;KV|ec0!M8Ya6Y=r{clWzeswvQs&Exm74!hV5Lqk>`kF9y@b~N z5m@4J?}{$Zsc1;G1m@IlFx`cj+f>(B@;~rf!N0^$JhT%cjxpmOo^S0CJP53AkhcsL zzJ^6Gwun7nTv4TILa)BkJTjs3<8QFNi01XSQ+PIux1Ci}Pe^k2cw*=~2$0+>YAP66 zo#n<RF*fO5j3MTGQR$I&63OU_YL8E-1Lr&-suO^1MGi5Bs6Ah2$j{I^47m(o9qrY9 zsLQ<=K@4}I6cB@*$cObsN>4^Hz{Gys`SyD;5>E7b5#d4PJ5i3ZB4ZI?MIO!7bu*#G zU(audo;-in&~ica=*hP6bZIQT`15%x5cZQ759#6ar$jI8b!itZEtU&AoNW|_E?4Kv zMO!cD;5OHxh$BN_G|I_@gw{OZfg-wic-|*4#3XrNL}l!ODqvW>)UK*vH=I@lwgT4h z3u;=WUt@olMN3~3zm+mFvB+X@bbt_X5nj;?r#qoje!E-rb_p9NS>$G9lXyW0wiz2^ zng6#3%P-<A47Rjz+1|&LB#}rQz5@Ox9usUE87D_?$ps2v$8#`j%PoI16ehgy;Tx-5 zf@eGYh{xRP@b#Xi@0wT~4O`do7B=5B!W=uoTI(4N&UC@R;}GAIeu7mo*7ot|yhqEO zNvLA1Y;Wr(5~?USE?T8>0a)&ZSpd%68NVNdj@l;YJA8kepWN&4eUp1TSlwsC-{W^v z%aQ+0!$&pQ=STQ)C>_2Je8lEk5BRR(T#_|X=C+GJ3~}1Kscpl?vhvVq<frfHCo;qj za7VyCH+k^k{O3s@Tj<h`7_b#K4Ib@gv|qImeiC>czWil;5%@`FY_!c!G$7KJQC^?A zV2B?^GQplPx#Pd)f@9X3Xu*hn$y6Ud*s_TqwCoxp8N|QIzZ&HqwJYfG9sfIg2fG1@ z`ylU*wx`W*M{FA-envsyqw<)1&;Ps8Hpab2?z?OVU-FnTn<shBh3oOGRipgGKDL;{ zcl_T5fXk3Gi9Og#YgCvC*N`F#B>9~Z3i4KYm-b!H@SKQ}AdtWx9dxJfAFlT)ub1Uk z%@qusho@<O{&xB}O*41?dWoxWyUS_JC2p_1hr3P3=wa>-*3-8&E(Gp|h;zQJi|cD! z!-b6DB64+i@4<_4Q{H*_u=&8vrfN7h73#gaQKg+fc!=C2lCcND{Qy<YW$8nfL-4L- zY%+9{H#=nC#U6~2vAZJhvqQeiYy|6Shl3r0W`WDO9;1dn_G6Fwp4Au=wZLJ%My7X> zMh?`DF{R)ED#0dFS3$;B?|>%U4G%FrH+xTyRxS#-kGnvR`Hs;55_JK5c96Ag(_A&J zV+O8~bF7$KurL?_$D#0$*6(h$t=a-Ni|zpLkK|m%v^$aw_TI=g5ee@~wksL$Kbw7Z z_a^^^lCh&}!b22||9i>U<9kuyuDt(s$rzO5#Ft(I;+*65h3i7-u4*Wkc^Dx{0scp3 z*7!kS$O?s8jiFN;>Q3oC4Yp*0zpWBm^6c#F6<PdIY-@#Ey^6C~D6OI_;YM3dS5Uae z*6jH3U`p(meat@l-6v~e?C*!{XZDyK{S(cP86LuCjrW>;f!{-7CyY(mcWnC7^&YO- z!6n`&$0wcM$uT=PJUKamIeLzcpvsQ$IX-58gW8%M;?p%~&88>pfU!&Z)%y4lsCeAi z(fW{0r?@YEvSw_0#GW&vDO(d`KV$fa9-)Qj@LM{XqWM{8baKe(+4Q4zCt!4qI8Oe- gK1K6im&c6#eu`;b{uY4j*Km4t@E9X-&W;cM16+S2TL1t6
literal 0 HcmV?d00001
diff --git a/test/py/tests/test_pstore_data_panic2.hex b/test/py/tests/test_pstore_data_panic2.hex new file mode 100644 index 0000000000000000000000000000000000000000..8f9d56cbe01e38f5f8fac6d46491497c9d7bda8f GIT binary patch literal 4096 zcmcIn%Wm676lIDmnrzxn7$6%5h2?qNc?oG2vD(x?3%EcE6bKA;G#ti+ElQ=N*x6;# zzwFQS4qql?*)a;5fZ-uDbLR3s&bfE?=I!yn-z7=>`w<@|wO~vLX(phxVf=9R{L~e> zf5FMAE4S?VD|r2$5T?z9a>c=%k@v4W6_m@=ygnlbA96oGcSU*`=6U(|OOlnVWwV7j z>f@Gq{AIrR=zpNs${*2dOJDYJg;H9ak)N)8Sa0)HK{i|GFAnH+2-dPpP_kW-_D!3? zSpdcGEXz2Zl7~FBc?kYjoTorSrhEoq2!0mlg3`WiQfg%*J|KREPr!4;GfriiLA?mV zN3{5ucERFLnK2Ug&{V4ho~l2`_&&Z;7F5Jnh+l88-A=p+J?%`|Er2LWHKj~c#zceq zXj)0b+X&rq{~$Ps3NbjY^yhQG-pz9iAqEHx1_=DX$wt{5*u4jP&@KzWX@pKg=609c z3VH6`<t4(42rq{4)=-N+j(K&@;ocR(0MDw_N>{Djj7Maqpl>;&#)51d^5vl+pv<at zj-J5uE4RD#JQUmV6WUxMi$TsZW);v5_^!kKD;T$`vUwBysz_b=slI4_5y~P&-!Mhv zCzwHoGl8|Io14CZ(Ot)4f%^J7L~P#BGrTQk1{}-zj2xf-!KYYotMxiBKJ;>lfx#ND zHcSbU!O3h&C^=p&m$?1Pt=FME;^dvni)~1!<ZWGhGeVBrDufBM6MCo)UAbJY$qPni zVUfFnR6i(v$o)$8Z&cKB{L)`snO?avkisp)l>A(Vu)*vJB%l2Dry{@aTzWLyxyxVk zZ6F7;@G<v+yu)3C$ZM=_Xx%1_uUx+?uJR(CFOXigrSqY4Sgf;P>J-?VudZTf{6&m? z3<)15mBB!@vEOs52kHXHtyL}Tmg^355lJu|CY0r9LT&H9Q$0|D#ImSB8{R!RJ&BcN zqp89VvWH^~yVBirt`B$r6Y8OYzI!@Crz;!~hA{Y9z$HVqmhq7~tDb47lJ7Pz<vF zB?ejoDh-OkBHSSc9E4<;pxFb(0KOsy_yGek;1ZCoB7P$VxS<w<xU<uMZgIaDa0Qxm z5gJVlxRwH1K8o6OTSa<Fbq}A%l)Wtm++Z`%*1_FM(!1ipV!$!6#AAp776S0r(*_<< z3<Rf|vzFEF7nIILE&WFt+Ajt|U^QdWp%`EnvPTSr&>U*Dh$^sG4Ctt0ASC6Hzef=R zfg~Ke#Hw>;ql$shjLJ5t#u5Wzn6ynFH=-B_i<;k-?wDc#6o-hWgBx=)047Xy?rkxM z4LA#-nGCA8j1QC#`x|Tqv9rcqn6u3j7b>;VM54F8PxMtoeSde~pN0S{Mplyty}{qt z_1DJXh)^#8={p)9mF!k`-|L0|B&+e`WP=HuQ5@7NBfseSaif)3z4cr+8qtH|IGePv zqZ2)7V6@Hgafu!bx~|RhF}r@Sii*}H4<dS(Y0D~wEW-vTp(5X+?k%G3Ee1o7q(~$U zv+p_7k-29`Wz<x&N;Rk4j1Q1TSV^%@d-6GXk|fWP<e$Hi&q?ylGxFx!r%#jQ`~Lte C8DA&>
literal 0 HcmV?d00001
Dear Frédéric,
In message 20200319175737.10166-3-frederic.danis@collabora.com you wrote:
+++ b/test/py/tests/test_pstore.py @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: GPL-2.0
Can we please make new code always GPL-2.0+ ?
Thanks!
Best regards,
Wolfgang Denk
To simplify configuration and keep synchronized the PStore/Ramoops between U-Boot and the Linux kernel, this commit dynamically adds the Ramoops parameters defined in the U-Boot session to the Device Tree.
Signed-off-by: Frédéric Danis frederic.danis@collabora.com --- cmd/pstore.c | 38 ++++++++++++++++++++++++++++++++++++++ common/image-fdt.c | 4 ++++ doc/pstore.rst | 2 ++ include/fdt_support.h | 3 +++ 4 files changed, 47 insertions(+)
diff --git a/cmd/pstore.c b/cmd/pstore.c index 4e4d70d604..6cad635620 100644 --- a/cmd/pstore.c +++ b/cmd/pstore.c @@ -479,6 +479,44 @@ static int do_pstore(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return c->cmd(cmdtp, flag, argc, argv); }
+void fdt_fixup_pstore(void *blob) +{ + char node[32]; + int nodeoffset; /* node offset from libfdt */ + + nodeoffset = fdt_path_offset(blob, "/"); + if (nodeoffset < 0) { + /* Not found or something else bad happened. */ + log_debug("fdt_path_offset() returned %s\n", fdt_strerror(nodeoffset)); + return; + } + + nodeoffset = fdt_add_subnode(blob, nodeoffset, "reserved-memory"); + if (nodeoffset < 0) { + log_debug("Add 'reserved-memory' node failed: %s\n", + fdt_strerror(nodeoffset)); + return; + } + fdt_appendprop_u32(blob, nodeoffset, "#address-cells", 2); + fdt_appendprop_u32(blob, nodeoffset, "#size-cells", 2); + fdt_appendprop(blob, nodeoffset, "ranges", NULL, 0); + + sprintf(node, "ramoops@%llx", (unsigned long long)pstore_addr); + nodeoffset = fdt_add_subnode(blob, nodeoffset, node); + if (nodeoffset < 0) { + log_debug("Add '%s' node failed: %s\n", node, fdt_strerror(nodeoffset)); + return; + } + fdt_appendprop_string(blob, nodeoffset, "compatible", "ramoops"); + fdt_appendprop_u64(blob, nodeoffset, "reg", pstore_addr); + fdt_appendprop_u64(blob, nodeoffset, "reg", pstore_length); + fdt_appendprop_u32(blob, nodeoffset, "record-size", pstore_record_size); + fdt_appendprop_u32(blob, nodeoffset, "console-size", pstore_console_size); + fdt_appendprop_u32(blob, nodeoffset, "ftrace-size", pstore_ftrace_size); + fdt_appendprop_u32(blob, nodeoffset, "pmsg-size", pstore_pmsg_size); + fdt_appendprop_u32(blob, nodeoffset, "ecc-size", pstore_ecc_size); +} + U_BOOT_CMD(pstore, 10, 0, do_pstore, "Manage Linux Persistent Storage", "set <addr> <len> [record-size] [console-size] [ftrace-size] [pmsg_size] [ecc-size]\n" diff --git a/common/image-fdt.c b/common/image-fdt.c index 3002948b6b..491d55ad1a 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -547,6 +547,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, } /* Update ethernet nodes */ fdt_fixup_ethernet(blob); +#if CONFIG_IS_ENABLED(CMD_PSTORE) + /* Append PStore configuration */ + fdt_fixup_pstore(blob); +#endif if (IMAGE_OF_BOARD_SETUP) { fdt_ret = ft_board_setup(blob, gd->bd); if (fdt_ret) { diff --git a/doc/pstore.rst b/doc/pstore.rst index 3ed2e9e0fd..3c84b8b172 100644 --- a/doc/pstore.rst +++ b/doc/pstore.rst @@ -24,6 +24,8 @@ i.e.::
The same values should be set in U-Boot to be able to retrieve the records. This values can be set at build time in U-Boot configuration file, or at runtime. +U-Boot automatically patches the Device Tree to pass the Ramoops parameters to +the kernel.
The PStore configuration parameters are:
diff --git a/include/fdt_support.h b/include/fdt_support.h index ba14acd7f6..7afbdcfe37 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -350,4 +350,7 @@ int fdt_update_ethernet_dt(void *blob); #ifdef CONFIG_FSL_MC_ENET void fdt_fixup_board_enet(void *blob); #endif +#ifdef CONFIG_CMD_PSTORE +void fdt_fixup_pstore(void *blob); +#endif #endif /* ifndef __FDT_SUPPORT_H */
On 3/19/20 6:57 PM, Frédéric Danis wrote:
To simplify configuration and keep synchronized the PStore/Ramoops between U-Boot and the Linux kernel, this commit dynamically adds the Ramoops parameters defined in the U-Boot session to the Device Tree.
Signed-off-by: Frédéric Danis frederic.danis@collabora.com
cmd/pstore.c | 38 ++++++++++++++++++++++++++++++++++++++ common/image-fdt.c | 4 ++++ doc/pstore.rst | 2 ++ include/fdt_support.h | 3 +++ 4 files changed, 47 insertions(+)
diff --git a/cmd/pstore.c b/cmd/pstore.c index 4e4d70d604..6cad635620 100644 --- a/cmd/pstore.c +++ b/cmd/pstore.c @@ -479,6 +479,44 @@ static int do_pstore(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return c->cmd(cmdtp, flag, argc, argv); }
+void fdt_fixup_pstore(void *blob) +{
- char node[32];
- int nodeoffset; /* node offset from libfdt */
- nodeoffset = fdt_path_offset(blob, "/");
- if (nodeoffset < 0) {
/* Not found or something else bad happened. */log_debug("fdt_path_offset() returned %s\n", fdt_strerror(nodeoffset));
Thanks for implementing device tree support.
Should this be log_err()?
return;- }
- nodeoffset = fdt_add_subnode(blob, nodeoffset, "reserved-memory");
- if (nodeoffset < 0) {
log_debug("Add 'reserved-memory' node failed: %s\n",fdt_strerror(nodeoffset));return;- }
- fdt_appendprop_u32(blob, nodeoffset, "#address-cells", 2);
For adding the first value I guess it is preferable to use the fdt_set* functions.
fdt_setprop_u32()
- fdt_appendprop_u32(blob, nodeoffset, "#size-cells", 2);
fdt_setprop_u32()
- fdt_appendprop(blob, nodeoffset, "ranges", NULL, 0);
fdt_setprop_empty()?
- sprintf(node, "ramoops@%llx", (unsigned long long)pstore_addr);
- nodeoffset = fdt_add_subnode(blob, nodeoffset, node);
- if (nodeoffset < 0) {
log_debug("Add '%s' node failed: %s\n", node, fdt_strerror(nodeoffset));
Should this be log_err()?
return;- }
- fdt_appendprop_string(blob, nodeoffset, "compatible", "ramoops");
fdt_setprop_string()
- fdt_appendprop_u64(blob, nodeoffset, "reg", pstore_addr);
fdt_setprop_u64()
- fdt_appendprop_u64(blob, nodeoffset, "reg", pstore_length);
This one is ok.
- fdt_appendprop_u32(blob, nodeoffset, "record-size", pstore_record_size);
fdt_setprop_u32(), same below.
Best regards
Heinrich
- fdt_appendprop_u32(blob, nodeoffset, "console-size", pstore_console_size);
- fdt_appendprop_u32(blob, nodeoffset, "ftrace-size", pstore_ftrace_size);
- fdt_appendprop_u32(blob, nodeoffset, "pmsg-size", pstore_pmsg_size);
- fdt_appendprop_u32(blob, nodeoffset, "ecc-size", pstore_ecc_size);
+}
U_BOOT_CMD(pstore, 10, 0, do_pstore, "Manage Linux Persistent Storage", "set <addr> <len> [record-size] [console-size] [ftrace-size] [pmsg_size] [ecc-size]\n" diff --git a/common/image-fdt.c b/common/image-fdt.c index 3002948b6b..491d55ad1a 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -547,6 +547,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, } /* Update ethernet nodes */ fdt_fixup_ethernet(blob); +#if CONFIG_IS_ENABLED(CMD_PSTORE)
- /* Append PStore configuration */
- fdt_fixup_pstore(blob);
+#endif if (IMAGE_OF_BOARD_SETUP) { fdt_ret = ft_board_setup(blob, gd->bd); if (fdt_ret) { diff --git a/doc/pstore.rst b/doc/pstore.rst index 3ed2e9e0fd..3c84b8b172 100644 --- a/doc/pstore.rst +++ b/doc/pstore.rst @@ -24,6 +24,8 @@ i.e.::
The same values should be set in U-Boot to be able to retrieve the records. This values can be set at build time in U-Boot configuration file, or at runtime. +U-Boot automatically patches the Device Tree to pass the Ramoops parameters to +the kernel.
The PStore configuration parameters are:
diff --git a/include/fdt_support.h b/include/fdt_support.h index ba14acd7f6..7afbdcfe37 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -350,4 +350,7 @@ int fdt_update_ethernet_dt(void *blob); #ifdef CONFIG_FSL_MC_ENET void fdt_fixup_board_enet(void *blob); #endif +#ifdef CONFIG_CMD_PSTORE +void fdt_fixup_pstore(void *blob); +#endif #endif /* ifndef __FDT_SUPPORT_H */
Dear Frédéric,
In message 20200319175737.10166-1-frederic.danis@collabora.com you wrote:
This serie of patches adds a new pstore command allowing to display or save ramoops logs (oops, panic, console, ftrace and user) generated by a previous kernel crash. PStore parameters can be set in U-Boot configuration file, or at run-time using "pstore set" command. For kernel using Device Tree, the parameters are dynamically added to Device Tree. Records size should be the same as the ones used by kernel, and should be a power of 2.
I wonder if we are reinventing the wheel here again?
There is this feature in U-Boot which is called "shared log buffer"; a couple of years ago this was fully functional at least on Power and ARM architectures, but it was rarely used and probably has not been tested for years. A;so, the necessary tiny patch to have it supported in Linux as well never made it upstream (don't remember why, likely lack of time/interest).
The functionality we had then was the following:
- A memory area war reserved in U-Boot (typically at the upper end of memory) as a buffer that was shared between U-Boot and Linux. The format was as used for the kernel log buffer. - Upon boot, U-Boot would not re-initialize an existing log buffer, but keep it's content. That means, you could read and display the log buffer of the linux kernel that was running before the reset. After kernel crashes, pretty often this contained information that the kernel could not even print to the serial console any more. - In U-Boot, you could append log entries to that buffer. For example, this was used to record the results of the Power On Self Test (POST) routines (another feature that only few people still remember). - When booting Linux, the kernel syslog mechanism was used to extract the information from the log buffer in the usual way.
The interesting fact here was that the Linux kernel was able to extract and save the kernel panic messages etc. from the crash before, plus any messages logged by U-Boot.
To me this sounds very much like what you are adding here (plus a few features more). Does it make sense to unify such code?
Added Heiko to Cc:, as he is currently working on fixes to get shared logbuffer working again for another project.
Best regards,
Wolfgang Denk
On 3/19/20 9:37 PM, Wolfgang Denk wrote:
Dear Frédéric,
In message 20200319175737.10166-1-frederic.danis@collabora.com you wrote:
This serie of patches adds a new pstore command allowing to display or save ramoops logs (oops, panic, console, ftrace and user) generated by a previous kernel crash. PStore parameters can be set in U-Boot configuration file, or at run-time using "pstore set" command. For kernel using Device Tree, the parameters are dynamically added to Device Tree. Records size should be the same as the ones used by kernel, and should be a power of 2.
I wonder if we are reinventing the wheel here again?
There is this feature in U-Boot which is called "shared log buffer"; a couple of years ago this was fully functional at least on Power and ARM architectures, but it was rarely used and probably has not been tested for years. A;so, the necessary tiny patch to have it supported in Linux as well never made it upstream (don't remember why, likely lack of time/interest).
The functionality we had then was the following:
- A memory area war reserved in U-Boot (typically at the upper end of memory) as a buffer that was shared between U-Boot and Linux. The format was as used for the kernel log buffer.
- Upon boot, U-Boot would not re-initialize an existing log buffer, but keep it's content. That means, you could read and display the log buffer of the linux kernel that was running before the reset. After kernel crashes, pretty often this contained information that the kernel could not even print to the serial console any more.
- In U-Boot, you could append log entries to that buffer. For example, this was used to record the results of the Power On Self Test (POST) routines (another feature that only few people still remember).
- When booting Linux, the kernel syslog mechanism was used to extract the information from the log buffer in the usual way.
The interesting fact here was that the Linux kernel was able to extract and save the kernel panic messages etc. from the crash before, plus any messages logged by U-Boot.
To me this sounds very much like what you are adding here (plus a few features more). Does it make sense to unify such code?
It seems you are relating to https://lore.kernel.org/lkml/844oyrqvvb.fsf@sauna.l.org/t/
ramoops in Linux is exactly doing what was suggested in 2009. You can find the Documentation/admin-guide/ramoops.rst
git grep -GHrn 'shared log' finds nothing in U-Boot. So if any part of the old implementation in U-Boot exists, could you, please, point us to the coding.
If the original design never made it into Linux and there is an established Linux interface since 2011, I would plead to eliminate any remaining non-compliant coding from U-Boot should it exist.
Best regards
Heinrich
Added Heiko to Cc:, as he is currently working on fixes to get shared logbuffer working again for another project.
Best regards,
Wolfgang Denk
Dear Heinrich,
In message a1b7a877-e950-b09a-a0f6-2c9c9cb7e1fb@gmx.de you wrote:
To me this sounds very much like what you are adding here (plus a few features more). Does it make sense to unify such code?
It seems you are relating to https://lore.kernel.org/lkml/844oyrqvvb.fsf@sauna.l.org/t/
No, I'm not. I was talking of my own code from many, many years ago.
ramoops in Linux is exactly doing what was suggested in 2009. You can find the Documentation/admin-guide/ramoops.rst
We had this in U-Boot long before that time. It was a key requirement when we added POST support in 2002.
git grep -GHrn 'shared log' finds nothing in U-Boot. So if any part of the old implementation in U-Boot exists, could you, please, point us to the coding.
The shared log buffer support was added by commit 56f94be3ef63:
commit 56f94be3ef63732384063e110277ed89701b6471 (tag: LABEL_2002_11_05_1735) Author: Wolfgang Denk <wdenk> Date: Tue Nov 5 16:35:14 2002 +0000
* Add support for log buffer which can be passed to Linux kernel's syslog mechanism; used especially for POST results.
* Patch by Klaus Heydeck, 31 Oct 2002: Add initial support for kup4k board
The code was mainly in common/cmd_log.c, but this got heahily rewritten and is now in cmd/log.c ; apparently this got lost (like the original copyright, sic!) when Simon modified / rewrote this driver.
For history, try: git log --follow -- common/cmd_log.c
If the original design never made it into Linux and there is an established Linux interface since 2011, I would plead to eliminate any remaining non-compliant coding from U-Boot should it exist.
I understand what Linus has is one-way, only focussing on crash dump, i. e. it does not allow to pass information from U-Boot to Linux?
Also, my understanding is that the changes needed in Linux are pretty small.
Maybe Heiko can comment on that...
Best regards,
Wolfgang Denk
Hello Wolfgang, Heinrich,
Am 20.03.2020 um 09:08 schrieb Wolfgang Denk:
Dear Heinrich,
In message a1b7a877-e950-b09a-a0f6-2c9c9cb7e1fb@gmx.de you wrote:
To me this sounds very much like what you are adding here (plus a few features more). Does it make sense to unify such code?
It seems you are relating to https://lore.kernel.org/lkml/844oyrqvvb.fsf@sauna.l.org/t/
No, I'm not. I was talking of my own code from many, many years ago.
ramoops in Linux is exactly doing what was suggested in 2009. You can find the Documentation/admin-guide/ramoops.rst
We had this in U-Boot long before that time. It was a key requirement when we added POST support in 2002.
git grep -GHrn 'shared log' finds nothing in U-Boot. So if any part of the old implementation in U-Boot exists, could you, please, point us to the coding.
The shared log buffer support was added by commit 56f94be3ef63:
commit 56f94be3ef63732384063e110277ed89701b6471 (tag: LABEL_2002_11_05_1735) Author: Wolfgang Denk <wdenk> Date: Tue Nov 5 16:35:14 2002 +0000
* Add support for log buffer which can be passed to Linux kernel's syslog mechanism; used especially for POST results. * Patch by Klaus Heydeck, 31 Oct 2002: Add initial support for kup4k boardThe code was mainly in common/cmd_log.c, but this got heahily rewritten and is now in cmd/log.c ; apparently this got lost (like the original copyright, sic!) when Simon modified / rewrote this driver.
For history, try: git log --follow -- common/cmd_log.c
If the original design never made it into Linux and there is an established Linux interface since 2011, I would plead to eliminate any remaining non-compliant coding from U-Boot should it exist.
I understand what Linus has is one-way, only focussing on crash dump, i. e. it does not allow to pass information from U-Boot to Linux?
Also, my understanding is that the changes needed in Linux are pretty small.
Maybe Heiko can comment on that...
Since 2002 the logbuffer support in linux changed, so the old patch does not work anymore.
First there is now the struct "printk_log" [1]
Unfortunately the size of it is dependend on linux configuration.
Each log entry is prepended from such a struct, just missing the possibility to detect if entry is valid (as linux was the only user of it, this was not needed).
There is already a possibilty to set a log buffer length through kernel commandline parameter "log_buf_len" [2]. If this is passed, kernel already uses a new area (currently memblock_alloced) for log buffer storage!
So added now the following to linux logbuffer handling:
Pass the persistent logbuffer address and length through a kernel commandline parameter and use it instead memblock_alloc()
Add a magic field in [1] which I use to detect if there are valid entries and where is the end of logbuffer entries in the new logbuffer area (Not that easy as there was may a wrap around, but also this is detectable)
-> start and end in the new log area is now known.
Now copy the new logbuffer messages from __log_buf with the already existing functions in linux kernel to the end of the new logbuffer area ... and you have old linux bootlogs and the new ones after a reboot.
aditionally changes in U-Boot:
- enable PRAM to reserve RAM at the end of RAM for logbuffer so U-Boot does not use this memory space. Already in U-Boot.
- add common/log_buffer.c log backend, which uses the functions from kernel for analysing/writing content of the logbuffer area.
"log rec" U-Boot command appends now also U-Boot log messages to the logbuffer area.
- add "log show" command, with which you can show current content of logbuffer area
And now U-Boot log messages also shown in linux bootlog ...
bye Heiko [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kern...
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kern...
Hi Wolfgang, Heinrich, Heiko,
On 20/03/2020 09:08, Wolfgang Denk wrote:
Dear Heinrich,
In messagea1b7a877-e950-b09a-a0f6-2c9c9cb7e1fb@gmx.de you wrote:
To me this sounds very much like what you are adding here (plus a few features more). Does it make sense to unify such code?
It seems you are relating to https://lore.kernel.org/lkml/844oyrqvvb.fsf@sauna.l.org/t/
No, I'm not. I was talking of my own code from many, many years ago.
ramoops in Linux is exactly doing what was suggested in 2009. You can find the Documentation/admin-guide/ramoops.rst
We had this in U-Boot long before that time. It was a key requirement when we added POST support in 2002.
git grep -GHrn 'shared log' finds nothing in U-Boot. So if any part of the old implementation in U-Boot exists, could you, please, point us to the coding.
The shared log buffer support was added by commit 56f94be3ef63:
commit 56f94be3ef63732384063e110277ed89701b6471 (tag: LABEL_2002_11_05_1735) Author: Wolfgang Denk <wdenk> Date: Tue Nov 5 16:35:14 2002 +0000
* Add support for log buffer which can be passed to Linux kernel's syslog mechanism; used especially for POST results. * Patch by Klaus Heydeck, 31 Oct 2002: Add initial support for kup4k boardThe code was mainly in common/cmd_log.c, but this got heahily rewritten and is now in cmd/log.c ; apparently this got lost (like the original copyright, sic!) when Simon modified / rewrote this driver.
For history, try: git log --follow -- common/cmd_log.c
If the original design never made it into Linux and there is an established Linux interface since 2011, I would plead to eliminate any remaining non-compliant coding from U-Boot should it exist.
I understand what Linus has is one-way, only focussing on crash dump, i. e. it does not allow to pass information from U-Boot to Linux?
Yes, currently it is not possible to pass information from U-Boot to Linux. But, PStore does not only store crash dump, but also console, ftrace and user space logs.
It may be possible to add a "bootloader" storage space to PStore.
Best regards,
Frédéric Danis
Dear Frédéric,
In message 03674974-2a2c-3804-ce02-4f3d38ff01d9@collabora.com you wrote:
I understand what Linus has is one-way, only focussing on crash dump, i. e. it does not allow to pass information from U-Boot to Linux?
Yes, currently it is not possible to pass information from U-Boot to Linux. But, PStore does not only store crash dump, but also console, ftrace and user space logs.
It may be possible to add a "bootloader" storage space to PStore.
This does not make much sense to me. We should not try to push any possible functionality into U-Boot just because we can.
U-Boot is a boot loader, and should only contain necessary functionality and things, that are impossible or difficult to solve in other ways.
I would much rather see the opposite direction working (again): being able to pass information from U-Boot to Linux kernel and further into Linux user land in a standard way.
Best regards,
Wolfgang Denk
Dear Wolfgang,
On 20/03/2020 11:35, Wolfgang Denk wrote:
Dear Frédéric,
In message 03674974-2a2c-3804-ce02-4f3d38ff01d9@collabora.com you wrote:
I understand what Linus has is one-way, only focussing on crash dump, i. e. it does not allow to pass information from U-Boot to Linux?
Yes, currently it is not possible to pass information from U-Boot to Linux. But, PStore does not only store crash dump, but also console, ftrace and user space logs.
It may be possible to add a "bootloader" storage space to PStore.
This does not make much sense to me. We should not try to push any possible functionality into U-Boot just because we can.
Yes, I understand it. But, when debugging kernel crashes occurring either early in boot or hard crashes later, it can be helpful to be able to retrieve logs using the U-Boot session.
U-Boot is a boot loader, and should only contain necessary functionality and things, that are impossible or difficult to solve in other ways.
I would much rather see the opposite direction working (again): being able to pass information from U-Boot to Linux kernel and further into Linux user land in a standard way.
Best regards,
Wolfgang Denk
Best regards,
Frédéric Danis
Dear Frédéric,
In message 9b0b5776-cb47-dfb2-ab77-36c89dc64359@collabora.com you wrote:
But, when debugging kernel crashes occurring either early in boot or hard crashes later, it can be helpful to be able to retrieve logs using the U-Boot session.
Full agreement, and we had such a feature working and in mainline already 17+ years ago.
It's sad that people rather reinvent the wheel instead of using existing functionality (or reviving it, if it has been broken over time).
Best regards,
Wolfgang Denk
Hi,
On Fri, 20 Mar 2020 at 05:30, Wolfgang Denk wd@denx.de wrote:
Dear Frédéric,
In message 9b0b5776-cb47-dfb2-ab77-36c89dc64359@collabora.com you wrote:
But, when debugging kernel crashes occurring either early in boot or hard crashes later, it can be helpful to be able to retrieve logs using the U-Boot session.
Full agreement, and we had such a feature working and in mainline already 17+ years ago.
It's sad that people rather reinvent the wheel instead of using existing functionality (or reviving it, if it has been broken over time).
Indeed.
Let's make sure there are tests added to this new feature (which looks extremely useful to me), so it stays working.
Regards, Simon
On Mon, Mar 23, 2020 at 09:37:12AM -0600, Simon Glass wrote:
Hi,
On Fri, 20 Mar 2020 at 05:30, Wolfgang Denk wd@denx.de wrote:
Dear Frédéric,
In message 9b0b5776-cb47-dfb2-ab77-36c89dc64359@collabora.com you wrote:
But, when debugging kernel crashes occurring either early in boot or hard crashes later, it can be helpful to be able to retrieve logs using the U-Boot session.
Full agreement, and we had such a feature working and in mainline already 17+ years ago.
It's sad that people rather reinvent the wheel instead of using existing functionality (or reviving it, if it has been broken over time).
Indeed.
Let's make sure there are tests added to this new feature (which looks extremely useful to me), so it stays working.
I'll just chime in to say that I have now reached the point in my career where I've both been told "you're re-inventing the wheel (again)" and now seen the wheel be re-re...-re-invented.
participants (6)
-
Frédéric Danis -
Heiko Schocher -
Heinrich Schuchardt -
Simon Glass -
Tom Rini -
Wolfgang Denk