[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Frédéric Danis]

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

-- 
2.18.0

[PATCH v3 1/3] cmd: Add command to display or save Linux PStore dumps

[Frédéric Danis]

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 /
-- 
2.18.0

[PATCH v3 2/3] test: Add PStore command tests

[Frédéric Danis]

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))
+
+ at 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)
+
+ at 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)
+
+ at 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 at 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 at lNt}`hF%c=vQY{D}JqApUVz+M^#mQS38q21kR=dA1k
z656*dwDxHrn#gIb!N!=1-E&>xgDwrjz_Af2FP at z4lvdzaX=YhgZ%XBT48sLX{ZLm_
zPDKnyPbK0<-l>$;Z%Z$c>pK{J at 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 at YOcuqU(?+|)pqi=961Pu$5^ROr3u5<e
zapyekE1qT#SQ)0Cy>>IHSVMt?8z_X9<$KXq6bOmEx2Ld5{qrht!K;=+w26WfMvl7`
z1>cRy|3%?>6 at pC^h-aB;+hPDB68mpU;l%*dCB+f#>I%E4o7RHnOt%9;Ht`58$JuI-
zLo0}bx8-35VtqH2im9T at M1}KRN-nA;A$J3z$nv)=j3E at 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 at 6nx
zvRFM<JuoF$g~G at 3@z}1a1P?4aUqcn2pi;Vd-OVlR6`bFp)``o9*<C`Z8m#qv09<_=
zTzok at +n=A!2BeRA4SVDCVuFDWhr`PcU(S#B&X2B!M_=!*FEFDB&Wa)D;t{w8A&m%1
z=xpJE^g`lz7M?3(mi@q|u{~>SuS at 8HeUg^Ce_ZD|{ut+SirtG_j?6(l^{c0)Tij6y
z4&r)XA3h$QpB#M;E at e3LOn;9qvWai`w}3mY{Vg_+p8W at 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 at 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 at 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 at 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 at 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 at 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 at b~N
z5m at 4J?}{$Zsc1;G1m at 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_ at gw{OZfg-wic-|*4#3XrNL}l!ODqvW>)UK*vH=I at lwgT4h
z3u;=WUt at olMN3~3zm+mFvB+X at bbt_X5nj;?r#qoje!E-rb_p9NS>$G9lXyW0wiz2^
zng6#3%P-<A47Rjz+1|&LB#}rQz5 at Ox9usUE87D_?$ps2v$8#`j%PoI16ehgy;Tx-5
zf at eGYh{xRP at b#Xi at 0wT~4O`do7B=5B!W=uoTI(4N&UC@R;}GAIeu7mo*7ot|yhqEO
zNvLA1Y;Wr(5~?USE?T8>0a)&ZSpd%68NVNdj at l;YJA8kepWN&4eUp1TSlwsC-{W^v
z%aQ+0!$&pQ=STQ)C>_2Je8lEk5BRR(T#_|X=C+GJ3~}1Kscpl?vhvVq<frfHCo;qj
za7VyCH+k^k{O3s at Tj<h`7_b#K4Ib@gv|qImeiC>czWil;5%@`FY_!c!G$7KJQC^?A
zV2B?^GQplPx#Pd)f at 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 at SKQ}AdtWx9dxJfAFlT)ub1Uk
z%@qusho@<O{&xB}O*41?dWoxWyUS_JC2p_1hr3P3=wa>-*3-8&E(Gp|h;zQJi|cD!
z!-b6DB64+i at 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 at LM{XqWM{8baKe(+4Q4zCt!4qI8Oe-
gK1K6im&c6#eu`;b{uY4j*Km4t at 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 at v4W6_m@=ygnlbA96oGcSU*`=6U(|OOlnVWwV7j
z>f at 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 at 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!@C>rz;!~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 at 7NBfseSaif)3z4cr+8qtH|IGePv
zqZ2)7V6 at Hgafu!bx~|RhF}r at 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

-- 
2.18.0

[PATCH v3 3/3] cmd: Fixup DT to pass PStore Ramoops parameters

[Frédéric Danis]

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 */
-- 
2.18.0

[PATCH v3 3/3] cmd: Fixup DT to pass PStore Ramoops parameters

[Heinrich Schuchardt]

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 */
>

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I'm what passes for a Unix guru in my office. This is  a  frightening
concept. - Lee Ann Goldstein, in <3k55ba$c43@butch.lmsc.lockheed.com>

[PATCH v3 2/3] test: Add PStore command tests

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
That's their goal, remember, a goal that's really contrary to that of
the programmer or administrator. We just want to get our  jobs  done.
$Bill  just  wants  to  become  $$Bill. These aren't even marginallly
congruent.
         -- Tom Christiansen in <6jhtqk$qls$1@csnews.cs.colorado.edu>

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Heinrich Schuchardt]

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 at 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
>

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[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 at 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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ken Dodd?s dad?s dog?s dead.

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Heiko Schocher]

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 at 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...

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/kernel/printk/printk.c#n368

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/printk/printk.c#n1114

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Frédéric Danis]

Hi Wolfgang, Heinrich, Heiko,

On 20/03/2020 09:08, Wolfgang Denk wrote:
> 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 at 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?

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

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Panic, n.: The second time you can't do it the first time.

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Frédéric Danis]

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

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Be wiser than other people if you can, but do not tell them so.
                                       -- Philip Earl of Chesterfield

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Simon Glass]

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

[PATCH v3 0/3] Add command to display or save Linux PStore dumps

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200323/1b2573d1/attachment.sig>