LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events
@ 2020-05-15 18:44 Kees Cook
  2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

Hello!

I wanted to get the pstore tree nailed down, so here's the v4 of
Pavel's series, tweaked for the feedback during v3 review.

-Kees

v4:
- rebase on pstore tree
- collapse shutdown types into a single dump reason
  https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
- fix dump_oops vs max_reason module params
  https://lore.kernel.org/lkml/20200512233504.GA118720@sequoia/
- typos
  https://lore.kernel.org/lkml/4cdeaa2af2fe0d6cc2ca8ce3a37608340799df8a.camel@perches.com/
- rename DT parsing routines ..._size -> ..._u32
  https://lore.kernel.org/lkml/CA+CK2bCu8eFomiU+NeBjVn-o2dbuECxwRfssNjB3ys3caCbXeA@mail.gmail.com/
v3: https://lore.kernel.org/lkml/20200506211523.15077-1-keescook@chromium.org/
v2: https://lore.kernel.org/lkml/20200505154510.93506-1-pasha.tatashin@soleen.com
v1: https://lore.kernel.org/lkml/20200502143555.543636-1-pasha.tatashin@soleen.com

Kees Cook (3):
  printk: Collapse shutdown types into a single dump reason
  printk: Introduce kmsg_dump_reason_str()
  pstore/ram: Introduce max_reason and convert dump_oops

Pavel Tatashin (3):
  printk: honor the max_reason field in kmsg_dumper
  pstore/platform: Pass max_reason to kmesg dump
  ramoops: Add max_reason optional field to ramoops DT node

 Documentation/admin-guide/ramoops.rst         | 14 +++--
 .../bindings/reserved-memory/ramoops.txt      | 13 ++++-
 arch/powerpc/kernel/nvram_64.c                |  4 +-
 drivers/platform/chrome/chromeos_pstore.c     |  2 +-
 fs/pstore/platform.c                          | 26 ++-------
 fs/pstore/ram.c                               | 58 +++++++++++++------
 include/linux/kmsg_dump.h                     | 12 +++-
 include/linux/pstore.h                        |  7 +++
 include/linux/pstore_ram.h                    |  2 +-
 kernel/printk/printk.c                        | 32 ++++++++--
 kernel/reboot.c                               |  6 +-
 11 files changed, 114 insertions(+), 62 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
@ 2020-05-15 18:44 ` Kees Cook
  2020-05-15 19:17   ` Pavel Tatashin
                     ` (2 more replies)
  2020-05-15 18:44 ` [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper Kees Cook
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
distinguish between them, so there's no need to, as discussed here:
https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/powerpc/kernel/nvram_64.c | 4 +---
 fs/pstore/platform.c           | 8 ++------
 include/linux/kmsg_dump.h      | 4 +---
 kernel/reboot.c                | 6 +++---
 4 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index fb4f61096613..0cd1c88bfc8b 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -655,9 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
 	int rc = -1;
 
 	switch (reason) {
-	case KMSG_DUMP_RESTART:
-	case KMSG_DUMP_HALT:
-	case KMSG_DUMP_POWEROFF:
+	case KMSG_DUMP_SHUTDOWN:
 		/* These are almost always orderly shutdowns. */
 		return;
 	case KMSG_DUMP_OOPS:
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 072440457c08..90d74ebaa70a 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -144,12 +144,8 @@ static const char *get_reason_str(enum kmsg_dump_reason reason)
 		return "Oops";
 	case KMSG_DUMP_EMERG:
 		return "Emergency";
-	case KMSG_DUMP_RESTART:
-		return "Restart";
-	case KMSG_DUMP_HALT:
-		return "Halt";
-	case KMSG_DUMP_POWEROFF:
-		return "Poweroff";
+	case KMSG_DUMP_SHUTDOWN:
+		return "Shutdown";
 	default:
 		return "Unknown";
 	}
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 2e7a1e032c71..3f82b5cb2d82 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -25,9 +25,7 @@ enum kmsg_dump_reason {
 	KMSG_DUMP_PANIC,
 	KMSG_DUMP_OOPS,
 	KMSG_DUMP_EMERG,
-	KMSG_DUMP_RESTART,
-	KMSG_DUMP_HALT,
-	KMSG_DUMP_POWEROFF,
+	KMSG_DUMP_SHUTDOWN,
 };
 
 /**
diff --git a/kernel/reboot.c b/kernel/reboot.c
index c4d472b7f1b4..491f1347bf43 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -250,7 +250,7 @@ void kernel_restart(char *cmd)
 		pr_emerg("Restarting system\n");
 	else
 		pr_emerg("Restarting system with command '%s'\n", cmd);
-	kmsg_dump(KMSG_DUMP_RESTART);
+	kmsg_dump(KMSG_DUMP_SHUTDOWN);
 	machine_restart(cmd);
 }
 EXPORT_SYMBOL_GPL(kernel_restart);
@@ -274,7 +274,7 @@ void kernel_halt(void)
 	migrate_to_reboot_cpu();
 	syscore_shutdown();
 	pr_emerg("System halted\n");
-	kmsg_dump(KMSG_DUMP_HALT);
+	kmsg_dump(KMSG_DUMP_SHUTDOWN);
 	machine_halt();
 }
 EXPORT_SYMBOL_GPL(kernel_halt);
@@ -292,7 +292,7 @@ void kernel_power_off(void)
 	migrate_to_reboot_cpu();
 	syscore_shutdown();
 	pr_emerg("Power down\n");
-	kmsg_dump(KMSG_DUMP_POWEROFF);
+	kmsg_dump(KMSG_DUMP_SHUTDOWN);
 	machine_power_off();
 }
 EXPORT_SYMBOL_GPL(kernel_power_off);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
  2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
@ 2020-05-15 18:44 ` Kees Cook
  2020-05-22 16:51   ` Petr Mladek
  2020-05-15 18:44 ` [PATCH v4 3/6] printk: Introduce kmsg_dump_reason_str() Kees Cook
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

From: Pavel Tatashin <pasha.tatashin@soleen.com>

kmsg_dump() allows to dump kmesg buffer for various system events: oops,
panic, reboot, etc. It provides an interface to register a callback call
for clients, and in that callback interface there is a field "max_reason"
which gets ignored unless always_kmsg_dump is passed as kernel parameter.

Allow clients to decide max_reason, and keep the current behavior when
max_reason is not set.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/lkml/20200506211523.15077-2-keescook@chromium.org/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/kmsg_dump.h |  1 +
 kernel/printk/printk.c    | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 3f82b5cb2d82..9826014771ab 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -26,6 +26,7 @@ enum kmsg_dump_reason {
 	KMSG_DUMP_OOPS,
 	KMSG_DUMP_EMERG,
 	KMSG_DUMP_SHUTDOWN,
+	KMSG_DUMP_MAX
 };
 
 /**
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 9a9b6156270b..a121c2255737 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3157,12 +3157,19 @@ void kmsg_dump(enum kmsg_dump_reason reason)
 	struct kmsg_dumper *dumper;
 	unsigned long flags;
 
-	if ((reason > KMSG_DUMP_OOPS) && !always_kmsg_dump)
-		return;
-
 	rcu_read_lock();
 	list_for_each_entry_rcu(dumper, &dump_list, list) {
-		if (dumper->max_reason && reason > dumper->max_reason)
+		enum kmsg_dump_reason max_reason = dumper->max_reason;
+
+		/*
+		 * If client has not provided a specific max_reason, default
+		 * to KMSG_DUMP_OOPS, unless always_kmsg_dump was set.
+		 */
+		if (max_reason == KMSG_DUMP_UNDEF) {
+			max_reason = always_kmsg_dump ? KMSG_DUMP_MAX :
+							KMSG_DUMP_OOPS;
+		}
+		if (reason > max_reason)
 			continue;
 
 		/* initialize iterator with data about the stored records */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 3/6] printk: Introduce kmsg_dump_reason_str()
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
  2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
  2020-05-15 18:44 ` [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper Kees Cook
@ 2020-05-15 18:44 ` Kees Cook
  2020-05-15 19:23   ` Pavel Tatashin
  2020-05-15 18:44 ` [PATCH v4 4/6] pstore/platform: Pass max_reason to kmesg dump Kees Cook
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Sergey Senozhatsky, Anton Vorontsov,
	Colin Cross, Tony Luck, Jonathan Corbet, Benson Leung,
	Rob Herring, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Enric Balletbo i Serra, Steven Rostedt,
	linux-doc, linux-kernel, devicetree, linuxppc-dev

The pstore subsystem already had a private version of this function.
With the coming addition of the pstore/zone driver, this needs to be
shared. As it really should live with printk, move it there instead.

Link: https://lore.kernel.org/lkml/20200510202436.63222-8-keescook@chromium.org/
Acked-by: Petr Mladek <pmladek@suse.com>
Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/platform.c      | 18 +-----------------
 include/linux/kmsg_dump.h |  7 +++++++
 kernel/printk/printk.c    | 17 +++++++++++++++++
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 90d74ebaa70a..5e6c6022deb9 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -135,22 +135,6 @@ enum pstore_type_id pstore_name_to_type(const char *name)
 }
 EXPORT_SYMBOL_GPL(pstore_name_to_type);
 
-static const char *get_reason_str(enum kmsg_dump_reason reason)
-{
-	switch (reason) {
-	case KMSG_DUMP_PANIC:
-		return "Panic";
-	case KMSG_DUMP_OOPS:
-		return "Oops";
-	case KMSG_DUMP_EMERG:
-		return "Emergency";
-	case KMSG_DUMP_SHUTDOWN:
-		return "Shutdown";
-	default:
-		return "Unknown";
-	}
-}
-
 static void pstore_timer_kick(void)
 {
 	if (pstore_update_ms < 0)
@@ -403,7 +387,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
 	unsigned int	part = 1;
 	int		ret;
 
-	why = get_reason_str(reason);
+	why = kmsg_dump_reason_str(reason);
 
 	if (down_trylock(&psinfo->buf_lock)) {
 		/* Failed to acquire lock: give up if we cannot wait. */
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 9826014771ab..3378bcbe585e 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -70,6 +70,8 @@ void kmsg_dump_rewind(struct kmsg_dumper *dumper);
 int kmsg_dump_register(struct kmsg_dumper *dumper);
 
 int kmsg_dump_unregister(struct kmsg_dumper *dumper);
+
+const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason);
 #else
 static inline void kmsg_dump(enum kmsg_dump_reason reason)
 {
@@ -111,6 +113,11 @@ static inline int kmsg_dump_unregister(struct kmsg_dumper *dumper)
 {
 	return -EINVAL;
 }
+
+static inline const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
+{
+	return "Disabled";
+}
 #endif
 
 #endif /* _LINUX_KMSG_DUMP_H */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a121c2255737..14ca4d05d902 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3144,6 +3144,23 @@ EXPORT_SYMBOL_GPL(kmsg_dump_unregister);
 static bool always_kmsg_dump;
 module_param_named(always_kmsg_dump, always_kmsg_dump, bool, S_IRUGO | S_IWUSR);
 
+const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
+{
+	switch (reason) {
+	case KMSG_DUMP_PANIC:
+		return "Panic";
+	case KMSG_DUMP_OOPS:
+		return "Oops";
+	case KMSG_DUMP_EMERG:
+		return "Emergency";
+	case KMSG_DUMP_SHUTDOWN:
+		return "Shutdown";
+	default:
+		return "Unknown";
+	}
+}
+EXPORT_SYMBOL_GPL(kmsg_dump_reason_str);
+
 /**
  * kmsg_dump - dump kernel log to kernel message dumpers.
  * @reason: the reason (oops, panic etc) for dumping
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 4/6] pstore/platform: Pass max_reason to kmesg dump
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
                   ` (2 preceding siblings ...)
  2020-05-15 18:44 ` [PATCH v4 3/6] printk: Introduce kmsg_dump_reason_str() Kees Cook
@ 2020-05-15 18:44 ` Kees Cook
  2020-05-15 18:44 ` [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops Kees Cook
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

From: Pavel Tatashin <pasha.tatashin@soleen.com>

Add a new member to struct pstore_info for passing information about
kmesg dump maximum reason. This allows a finer control of what kmesg
dumps are sent to pstore storage backends.

Those backends that do not explicitly set this field (keeping it equal to
0), get the default behavior: store only Oopses and Panics, or everything
if the printk.always_kmsg_dump boot param is set.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/lkml/20200506211523.15077-3-keescook@chromium.org/
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/platform.c   | 4 +++-
 include/linux/pstore.h | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 5e6c6022deb9..a9e297eefdff 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -595,8 +595,10 @@ int pstore_register(struct pstore_info *psi)
 
 	pstore_get_records(0);
 
-	if (psi->flags & PSTORE_FLAGS_DMESG)
+	if (psi->flags & PSTORE_FLAGS_DMESG) {
+		pstore_dumper.max_reason = psinfo->max_reason;
 		pstore_register_kmsg();
+	}
 	if (psi->flags & PSTORE_FLAGS_CONSOLE)
 		pstore_register_console();
 	if (psi->flags & PSTORE_FLAGS_FTRACE)
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index f6f22b13e04f..eb93a54cff31 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -96,6 +96,12 @@ struct pstore_record {
  *
  * @read_mutex:	serializes @open, @read, @close, and @erase callbacks
  * @flags:	bitfield of frontends the backend can accept writes for
+ * @max_reason:	Used when PSTORE_FLAGS_DMESG is set. Contains the
+ *		kmsg_dump_reason enum value. KMSG_DUMP_UNDEF means
+ *		"use existing kmsg_dump() filtering, based on the
+ *		printk.always_kmsg_dump boot param" (which is either
+ *		KMSG_DUMP_OOPS when false, or KMSG_DUMP_MAX when
+ *		true); see printk.always_kmsg_dump for more details.
  * @data:	backend-private pointer passed back during callbacks
  *
  * Callbacks:
@@ -179,6 +185,7 @@ struct pstore_info {
 	struct mutex	read_mutex;
 
 	int		flags;
+	int		max_reason;
 	void		*data;
 
 	int		(*open)(struct pstore_info *psi);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
                   ` (3 preceding siblings ...)
  2020-05-15 18:44 ` [PATCH v4 4/6] pstore/platform: Pass max_reason to kmesg dump Kees Cook
@ 2020-05-15 18:44 ` Kees Cook
  2020-05-15 19:30   ` Pavel Tatashin
  2020-05-15 19:40   ` Pavel Tatashin
  2020-05-15 18:44 ` [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node Kees Cook
  2020-05-15 19:13 ` [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Pavel Tatashin
  6 siblings, 2 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

Now that pstore_register() can correctly pass max_reason to the kmesg
dump facility, introduce a new "max_reason" module parameter and
"max-reason" Device Tree field.

The "dump_oops" module parameter and "dump-oops" Device
Tree field are now considered deprecated, but are now automatically
converted to their corresponding max_reason values when present, though
the new max_reason setting has precedence.

For struct ramoops_platform_data, the "dump_oops" member is entirely
replaced by a new "max_reason" member, with the only existing user
updated in place.

Additionally remove the "reason" filter logic from ramoops_pstore_write(),
as that is not specifically needed anymore, though technically
this is a change in behavior for any ramoops users also setting the
printk.always_kmsg_dump boot param, which will cause ramoops to behave as
if max_reason was set to KMSG_DUMP_MAX.

Co-developed-by: Pavel Tatashin <pasha.tatashin@soleen.com>
Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/lkml/20200506211523.15077-5-keescook@chromium.org/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Documentation/admin-guide/ramoops.rst     | 14 ++++--
 drivers/platform/chrome/chromeos_pstore.c |  2 +-
 fs/pstore/ram.c                           | 58 +++++++++++++++--------
 include/linux/pstore_ram.h                |  2 +-
 4 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/Documentation/admin-guide/ramoops.rst b/Documentation/admin-guide/ramoops.rst
index 6dbcc5481000..a60a96218ba9 100644
--- a/Documentation/admin-guide/ramoops.rst
+++ b/Documentation/admin-guide/ramoops.rst
@@ -32,11 +32,17 @@ memory to be mapped strongly ordered, and atomic operations on strongly ordered
 memory are implementation defined, and won't work on many ARMs such as omaps.
 
 The memory area is divided into ``record_size`` chunks (also rounded down to
-power of two) and each oops/panic writes a ``record_size`` chunk of
+power of two) and each kmesg dump writes a ``record_size`` chunk of
 information.
 
-Dumping both oopses and panics can be done by setting 1 in the ``dump_oops``
-variable while setting 0 in that variable dumps only the panics.
+Limiting which kinds of kmsg dumps are stored can be controlled via
+the ``max_reason`` value, as defined in include/linux/kmsg_dump.h's
+``enum kmsg_dump_reason``. For example, to store both Oopses and Panics,
+``max_reason`` should be set to 2 (KMSG_DUMP_OOPS), to store only Panics
+``max_reason`` should be set to 1 (KMSG_DUMP_PANIC). Setting this to 0
+(KMSG_DUMP_UNDEF), means the reason filtering will be controlled by the
+``printk.always_kmsg_dump`` boot param: if unset, it'll be KMSG_DUMP_OOPS,
+otherwise KMSG_DUMP_MAX.
 
 The module uses a counter to record multiple dumps but the counter gets reset
 on restart (i.e. new dumps after the restart will overwrite old ones).
@@ -90,7 +96,7 @@ Setting the ramoops parameters can be done in several different manners:
         .mem_address            = <...>,
         .mem_type               = <...>,
         .record_size            = <...>,
-        .dump_oops              = <...>,
+        .max_reason             = <...>,
         .ecc                    = <...>,
   };
 
diff --git a/drivers/platform/chrome/chromeos_pstore.c b/drivers/platform/chrome/chromeos_pstore.c
index d13770785fb5..fa51153688b4 100644
--- a/drivers/platform/chrome/chromeos_pstore.c
+++ b/drivers/platform/chrome/chromeos_pstore.c
@@ -57,7 +57,7 @@ static struct ramoops_platform_data chromeos_ramoops_data = {
 	.record_size	= 0x40000,
 	.console_size	= 0x20000,
 	.ftrace_size	= 0x20000,
-	.dump_oops	= 1,
+	.max_reason	= KMSG_DUMP_OOPS,
 };
 
 static struct platform_device chromeos_ramoops = {
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 31f277633beb..f6eace1dbf7e 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -58,10 +58,10 @@ module_param(mem_type, uint, 0400);
 MODULE_PARM_DESC(mem_type,
 		"set to 1 to try to use unbuffered memory (default 0)");
 
-static int dump_oops = 1;
-module_param(dump_oops, int, 0400);
-MODULE_PARM_DESC(dump_oops,
-		"set to 1 to dump oopses, 0 to only dump panics (default 1)");
+static int ramoops_max_reason = -1;
+module_param_named(max_reason, ramoops_max_reason, int, 0400);
+MODULE_PARM_DESC(max_reason,
+		 "maximum reason for kmsg dump (default 2: Oops and Panic) ");
 
 static int ramoops_ecc;
 module_param_named(ecc, ramoops_ecc, int, 0400);
@@ -70,6 +70,11 @@ MODULE_PARM_DESC(ramoops_ecc,
 		"ECC buffer size in bytes (1 is a special value, means 16 "
 		"bytes ECC)");
 
+static int ramoops_dump_oops = -1;
+module_param_named(dump_oops, ramoops_dump_oops, int, 0400);
+MODULE_PARM_DESC(dump_oops,
+		 "(deprecated: use max_reason instead) set to 1 to dump oopses & panics, 0 to only dump panics");
+
 struct ramoops_context {
 	struct persistent_ram_zone **dprzs;	/* Oops dump zones */
 	struct persistent_ram_zone *cprz;	/* Console zone */
@@ -82,7 +87,6 @@ struct ramoops_context {
 	size_t console_size;
 	size_t ftrace_size;
 	size_t pmsg_size;
-	int dump_oops;
 	u32 flags;
 	struct persistent_ram_ecc_info ecc_info;
 	unsigned int max_dump_cnt;
@@ -336,16 +340,14 @@ static int notrace ramoops_pstore_write(struct pstore_record *record)
 		return -EINVAL;
 
 	/*
-	 * Out of the various dmesg dump types, ramoops is currently designed
-	 * to only store crash logs, rather than storing general kernel logs.
+	 * We could filter on record->reason here if we wanted to (which
+	 * would duplicate what happened before the "max_reason" setting
+	 * was added), but that would defeat the purpose of a system
+	 * changing printk.always_kmsg_dump, so instead log everything that
+	 * the kmsg dumper sends us, since it should be doing the filtering
+	 * based on the combination of printk.always_kmsg_dump and our
+	 * requested "max_reason".
 	 */
-	if (record->reason != KMSG_DUMP_OOPS &&
-	    record->reason != KMSG_DUMP_PANIC)
-		return -EINVAL;
-
-	/* Skip Oopes when configured to do so. */
-	if (record->reason == KMSG_DUMP_OOPS && !cxt->dump_oops)
-		return -EINVAL;
 
 	/*
 	 * Explicitly only take the first part of any new crash.
@@ -647,7 +649,14 @@ static int ramoops_parse_dt(struct platform_device *pdev,
 	pdata->mem_size = resource_size(res);
 	pdata->mem_address = res->start;
 	pdata->mem_type = of_property_read_bool(of_node, "unbuffered");
-	pdata->dump_oops = !of_property_read_bool(of_node, "no-dump-oops");
+	/*
+	 * Setting "no-dump-oops" is deprecated and will be ignored if
+	 * "max_reason" is also specified.
+	 */
+	if (of_property_read_bool(of_node, "no-dump-oops"))
+		pdata->max_reason = KMSG_DUMP_PANIC;
+	else
+		pdata->max_reason = KMSG_DUMP_OOPS;
 
 #define parse_u32(name, field, default_value) {				\
 		ret = ramoops_parse_dt_u32(pdev, name, default_value,	\
@@ -663,6 +672,7 @@ static int ramoops_parse_dt(struct platform_device *pdev,
 	parse_u32("pmsg-size", pdata->pmsg_size, 0);
 	parse_u32("ecc-size", pdata->ecc_info.ecc_size, 0);
 	parse_u32("flags", pdata->flags, 0);
+	parse_u32("max-reason", pdata->max_reason, pdata->max_reason);
 
 #undef parse_size
 
@@ -746,7 +756,6 @@ static int ramoops_probe(struct platform_device *pdev)
 	cxt->console_size = pdata->console_size;
 	cxt->ftrace_size = pdata->ftrace_size;
 	cxt->pmsg_size = pdata->pmsg_size;
-	cxt->dump_oops = pdata->dump_oops;
 	cxt->flags = pdata->flags;
 	cxt->ecc_info = pdata->ecc_info;
 
@@ -789,8 +798,10 @@ static int ramoops_probe(struct platform_device *pdev)
 	 * the single region size is how to check.
 	 */
 	cxt->pstore.flags = 0;
-	if (cxt->max_dump_cnt)
+	if (cxt->max_dump_cnt) {
 		cxt->pstore.flags |= PSTORE_FLAGS_DMESG;
+		cxt->pstore.max_reason = pdata->max_reason;
+	}
 	if (cxt->console_size)
 		cxt->pstore.flags |= PSTORE_FLAGS_CONSOLE;
 	if (cxt->max_ftrace_cnt)
@@ -826,7 +837,7 @@ static int ramoops_probe(struct platform_device *pdev)
 	mem_size = pdata->mem_size;
 	mem_address = pdata->mem_address;
 	record_size = pdata->record_size;
-	dump_oops = pdata->dump_oops;
+	ramoops_max_reason = pdata->max_reason;
 	ramoops_console_size = pdata->console_size;
 	ramoops_pmsg_size = pdata->pmsg_size;
 	ramoops_ftrace_size = pdata->ftrace_size;
@@ -909,7 +920,16 @@ static void __init ramoops_register_dummy(void)
 	pdata.console_size = ramoops_console_size;
 	pdata.ftrace_size = ramoops_ftrace_size;
 	pdata.pmsg_size = ramoops_pmsg_size;
-	pdata.dump_oops = dump_oops;
+	/* If "max_reason" is set, its value has priority over "dump_oops". */
+	if (ramoops_max_reason != -1)
+		pdata.max_reason = ramoops_max_reason;
+	/* Otherwise, if "dump_oops" is set, parse it into "max_reason". */
+	else if (ramoops_dump_oops != -1)
+		pdata.max_reason = ramoops_dump_oops ? KMSG_DUMP_OOPS
+						     : KMSG_DUMP_PANIC;
+	/* And if neither are explicitly set, use the default. */
+	else
+		pdata.max_reason = KMSG_DUMP_OOPS;
 	pdata.flags = RAMOOPS_FLAG_FTRACE_PER_CPU;
 
 	/*
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 9cb9b9067298..9f16afec7290 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -133,7 +133,7 @@ struct ramoops_platform_data {
 	unsigned long	console_size;
 	unsigned long	ftrace_size;
 	unsigned long	pmsg_size;
-	int		dump_oops;
+	int		max_reason;
 	u32		flags;
 	struct persistent_ram_ecc_info ecc_info;
 };
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
                   ` (4 preceding siblings ...)
  2020-05-15 18:44 ` [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops Kees Cook
@ 2020-05-15 18:44 ` Kees Cook
  2020-05-18 22:45   ` Rob Herring
  2020-05-15 19:13 ` [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Pavel Tatashin
  6 siblings, 1 reply; 20+ messages in thread
From: Kees Cook @ 2020-05-15 18:44 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

From: Pavel Tatashin <pasha.tatashin@soleen.com>

Currently, it is possible to dump kmsges for panic, or oops.
With max_reason it is possible to dump messages for other
kmesg_dump events, for example reboot, halt, shutdown, kexec.

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
Link: https://lore.kernel.org/lkml/20200506211523.15077-6-keescook@chromium.org/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 .../devicetree/bindings/reserved-memory/ramoops.txt | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
index 0eba562fe5c6..b7886fea368c 100644
--- a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
@@ -30,7 +30,7 @@ Optional properties:
 - ecc-size: enables ECC support and specifies ECC buffer size in bytes
   (defaults to 0: no ECC)
 
-- record-size: maximum size in bytes of each dump done on oops/panic
+- record-size: maximum size in bytes of each kmsg dump.
   (defaults to 0: disabled)
 
 - console-size: size in bytes of log buffer reserved for kernel messages
@@ -45,7 +45,16 @@ Optional properties:
 - unbuffered: if present, use unbuffered mappings to map the reserved region
   (defaults to buffered mappings)
 
-- no-dump-oops: if present, only dump panics (defaults to panics and oops)
+- max-reason: if present, sets maximum type of kmsg dump reasons to store
+  (defaults to 2: log Oopses and Panics). This can be set to INT_MAX to
+  store all kmsg dumps. See include/linux/kmsg_dump.h KMSG_DUMP_* for other
+  kmsg dump reason values. Setting this to 0 (KMSG_DUMP_UNDEF), means the
+  reason filtering will be controlled by the printk.always_kmsg_dump boot
+  param: if unset, it will be KMSG_DUMP_OOPS, otherwise KMSG_DUMP_MAX.
+
+- no-dump-oops: deprecated, use max_reason instead. If present, and
+  max_reason is not specified, it is equivalent to max_reason = 1
+  (KMSG_DUMP_PANIC).
 
 - flags: if present, pass ramoops behavioral flags (defaults to 0,
   see include/linux/pstore_ram.h RAMOOPS_FLAG_* for flag values).
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events
  2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
                   ` (5 preceding siblings ...)
  2020-05-15 18:44 ` [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node Kees Cook
@ 2020-05-15 19:13 ` Pavel Tatashin
  6 siblings, 0 replies; 20+ messages in thread
From: Pavel Tatashin @ 2020-05-15 19:13 UTC (permalink / raw)
  To: Kees Cook
  Cc: Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List, LKML,
	devicetree, linuxppc-dev

On Fri, May 15, 2020 at 2:44 PM Kees Cook <keescook@chromium.org> wrote:
>
> Hello!
>
> I wanted to get the pstore tree nailed down, so here's the v4 of
> Pavel's series, tweaked for the feedback during v3 review.

Hi Kees,

Thank you, I was planning to send a new version of this series later
today. Let me quickly review it.

Pasha

>
> -Kees
>
> v4:
> - rebase on pstore tree
> - collapse shutdown types into a single dump reason
>   https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
> - fix dump_oops vs max_reason module params
>   https://lore.kernel.org/lkml/20200512233504.GA118720@sequoia/
> - typos
>   https://lore.kernel.org/lkml/4cdeaa2af2fe0d6cc2ca8ce3a37608340799df8a.camel@perches.com/
> - rename DT parsing routines ..._size -> ..._u32
>   https://lore.kernel.org/lkml/CA+CK2bCu8eFomiU+NeBjVn-o2dbuECxwRfssNjB3ys3caCbXeA@mail.gmail.com/
> v3: https://lore.kernel.org/lkml/20200506211523.15077-1-keescook@chromium.org/
> v2: https://lore.kernel.org/lkml/20200505154510.93506-1-pasha.tatashin@soleen.com
> v1: https://lore.kernel.org/lkml/20200502143555.543636-1-pasha.tatashin@soleen.com
>
> Kees Cook (3):
>   printk: Collapse shutdown types into a single dump reason
>   printk: Introduce kmsg_dump_reason_str()
>   pstore/ram: Introduce max_reason and convert dump_oops
>
> Pavel Tatashin (3):
>   printk: honor the max_reason field in kmsg_dumper
>   pstore/platform: Pass max_reason to kmesg dump
>   ramoops: Add max_reason optional field to ramoops DT node
>
>  Documentation/admin-guide/ramoops.rst         | 14 +++--
>  .../bindings/reserved-memory/ramoops.txt      | 13 ++++-
>  arch/powerpc/kernel/nvram_64.c                |  4 +-
>  drivers/platform/chrome/chromeos_pstore.c     |  2 +-
>  fs/pstore/platform.c                          | 26 ++-------
>  fs/pstore/ram.c                               | 58 +++++++++++++------
>  include/linux/kmsg_dump.h                     | 12 +++-
>  include/linux/pstore.h                        |  7 +++
>  include/linux/pstore_ram.h                    |  2 +-
>  kernel/printk/printk.c                        | 32 ++++++++--
>  kernel/reboot.c                               |  6 +-
>  11 files changed, 114 insertions(+), 62 deletions(-)
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
  2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
@ 2020-05-15 19:17   ` Pavel Tatashin
  2020-05-22 16:21   ` Petr Mladek
  2020-05-23 11:16   ` Michael Ellerman
  2 siblings, 0 replies; 20+ messages in thread
From: Pavel Tatashin @ 2020-05-15 19:17 UTC (permalink / raw)
  To: Kees Cook
  Cc: Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List, LKML,
	devicetree, linuxppc-dev

On Fri, May 15, 2020 at 2:44 PM Kees Cook <keescook@chromium.org> wrote:
>
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Maybe it makes sense to mention in the commit log that for all three
merged cases there is a pr_emerg() message logged right before the
kmsg_dump(), thus the reason is distinguishable from the dmesg log
itself.

Reviewed-by: Pavel Tatashin <pasha.tatashin@soleen.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/6] printk: Introduce kmsg_dump_reason_str()
  2020-05-15 18:44 ` [PATCH v4 3/6] printk: Introduce kmsg_dump_reason_str() Kees Cook
@ 2020-05-15 19:23   ` Pavel Tatashin
  0 siblings, 0 replies; 20+ messages in thread
From: Pavel Tatashin @ 2020-05-15 19:23 UTC (permalink / raw)
  To: Kees Cook
  Cc: Petr Mladek, Sergey Senozhatsky, Anton Vorontsov, Colin Cross,
	Tony Luck, Jonathan Corbet, Benson Leung, Rob Herring,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Enric Balletbo i Serra, Steven Rostedt, Linux Doc Mailing List,
	LKML, devicetree, linuxppc-dev

On Fri, May 15, 2020 at 2:44 PM Kees Cook <keescook@chromium.org> wrote:
>
> The pstore subsystem already had a private version of this function.
> With the coming addition of the pstore/zone driver, this needs to be
> shared. As it really should live with printk, move it there instead.
>
> Link: https://lore.kernel.org/lkml/20200510202436.63222-8-keescook@chromium.org/
> Acked-by: Petr Mladek <pmladek@suse.com>
> Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Reviewed-by: Pavel Tatashin <pasha.tatashin@soleen.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops
  2020-05-15 18:44 ` [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops Kees Cook
@ 2020-05-15 19:30   ` Pavel Tatashin
  2020-05-15 19:48     ` Kees Cook
  2020-05-15 19:40   ` Pavel Tatashin
  1 sibling, 1 reply; 20+ messages in thread
From: Pavel Tatashin @ 2020-05-15 19:30 UTC (permalink / raw)
  To: Kees Cook
  Cc: Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List, LKML,
	devicetree, linuxppc-dev

>  #define parse_u32(name, field, default_value) {                                \
>                 ret = ramoops_parse_dt_u32(pdev, name, default_value,   \

The series seems to be missing the patch where ramoops_parse_dt_size
-> ramoops_parse_dt_u32 get renamed, and updated to handle default
value.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops
  2020-05-15 18:44 ` [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops Kees Cook
  2020-05-15 19:30   ` Pavel Tatashin
@ 2020-05-15 19:40   ` Pavel Tatashin
  2020-05-15 19:53     ` Kees Cook
  1 sibling, 1 reply; 20+ messages in thread
From: Pavel Tatashin @ 2020-05-15 19:40 UTC (permalink / raw)
  To: Kees Cook
  Cc: Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List, LKML,
	devicetree, linuxppc-dev

 pdata.dump_oops = dump_oops;
> +       /* If "max_reason" is set, its value has priority over "dump_oops". */
> +       if (ramoops_max_reason != -1)
> +               pdata.max_reason = ramoops_max_reason;

 (ramoops_max_reason >= 0) might make more sense here, we do not want
negative max_reason even if it was provided by the user.

Otherwise the series looks good.

Thank you,
Pasha

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops
  2020-05-15 19:30   ` Pavel Tatashin
@ 2020-05-15 19:48     ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-15 19:48 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List, LKML,
	devicetree, linuxppc-dev

On Fri, May 15, 2020 at 03:30:27PM -0400, Pavel Tatashin wrote:
> >  #define parse_u32(name, field, default_value) {                                \
> >                 ret = ramoops_parse_dt_u32(pdev, name, default_value,   \
> 
> The series seems to be missing the patch where ramoops_parse_dt_size
> -> ramoops_parse_dt_u32 get renamed, and updated to handle default
> value.

Oops! Sorry, I cut the line in the wrong place for sending out the delta
on top of the pstore tree. :)

It's unchanged from:
https://lore.kernel.org/lkml/20200506211523.15077-4-keescook@chromium.org/

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops
  2020-05-15 19:40   ` Pavel Tatashin
@ 2020-05-15 19:53     ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-15 19:53 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List, LKML,
	devicetree, linuxppc-dev

On Fri, May 15, 2020 at 03:40:14PM -0400, Pavel Tatashin wrote:
>  pdata.dump_oops = dump_oops;
> > +       /* If "max_reason" is set, its value has priority over "dump_oops". */
> > +       if (ramoops_max_reason != -1)
> > +               pdata.max_reason = ramoops_max_reason;
> 
>  (ramoops_max_reason >= 0) might make more sense here, we do not want
> negative max_reason even if it was provided by the user.

Yeah, that's a good point. I'll tweak that. Thanks!

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node
  2020-05-15 18:44 ` [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node Kees Cook
@ 2020-05-18 22:45   ` Rob Herring
  2020-05-18 23:04     ` Kees Cook
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2020-05-18 22:45 UTC (permalink / raw)
  To: Kees Cook
  Cc: Pavel Tatashin, Petr Mladek, Anton Vorontsov, Colin Cross,
	Tony Luck, Jonathan Corbet, Benson Leung, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List,
	linux-kernel, devicetree, linuxppc-dev

On Fri, May 15, 2020 at 12:44 PM Kees Cook <keescook@chromium.org> wrote:
>
> From: Pavel Tatashin <pasha.tatashin@soleen.com>

Subject still has 'max_reason'.

>
> Currently, it is possible to dump kmsges for panic, or oops.
> With max_reason it is possible to dump messages for other

And here.

> kmesg_dump events, for example reboot, halt, shutdown, kexec.
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> Link: https://lore.kernel.org/lkml/20200506211523.15077-6-keescook@chromium.org/
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  .../devicetree/bindings/reserved-memory/ramoops.txt | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> index 0eba562fe5c6..b7886fea368c 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> +++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> @@ -30,7 +30,7 @@ Optional properties:
>  - ecc-size: enables ECC support and specifies ECC buffer size in bytes
>    (defaults to 0: no ECC)
>
> -- record-size: maximum size in bytes of each dump done on oops/panic
> +- record-size: maximum size in bytes of each kmsg dump.
>    (defaults to 0: disabled)
>
>  - console-size: size in bytes of log buffer reserved for kernel messages
> @@ -45,7 +45,16 @@ Optional properties:
>  - unbuffered: if present, use unbuffered mappings to map the reserved region
>    (defaults to buffered mappings)
>
> -- no-dump-oops: if present, only dump panics (defaults to panics and oops)
> +- max-reason: if present, sets maximum type of kmsg dump reasons to store
> +  (defaults to 2: log Oopses and Panics). This can be set to INT_MAX to
> +  store all kmsg dumps. See include/linux/kmsg_dump.h KMSG_DUMP_* for other
> +  kmsg dump reason values. Setting this to 0 (KMSG_DUMP_UNDEF), means the
> +  reason filtering will be controlled by the printk.always_kmsg_dump boot
> +  param: if unset, it will be KMSG_DUMP_OOPS, otherwise KMSG_DUMP_MAX.
> +
> +- no-dump-oops: deprecated, use max_reason instead. If present, and
> +  max_reason is not specified, it is equivalent to max_reason = 1

And here (3 times).

> +  (KMSG_DUMP_PANIC).
>
>  - flags: if present, pass ramoops behavioral flags (defaults to 0,
>    see include/linux/pstore_ram.h RAMOOPS_FLAG_* for flag values).
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node
  2020-05-18 22:45   ` Rob Herring
@ 2020-05-18 23:04     ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-18 23:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pavel Tatashin, Petr Mladek, Anton Vorontsov, Colin Cross,
	Tony Luck, Jonathan Corbet, Benson Leung, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, Linux Doc Mailing List,
	linux-kernel, devicetree, linuxppc-dev

On Mon, May 18, 2020 at 04:45:32PM -0600, Rob Herring wrote:
> On Fri, May 15, 2020 at 12:44 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > From: Pavel Tatashin <pasha.tatashin@soleen.com>
> 
> Subject still has 'max_reason'.
> 
> >
> > Currently, it is possible to dump kmsges for panic, or oops.
> > With max_reason it is possible to dump messages for other
> 
> And here.

Ah yeah, this was, I think, describing the internal field name, but I
see it would be less confusing to refer to this by the DT name. I will
adjust it. Thanks!

-Kees

> 
> > kmesg_dump events, for example reboot, halt, shutdown, kexec.
> >
> > Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> > Link: https://lore.kernel.org/lkml/20200506211523.15077-6-keescook@chromium.org/
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  .../devicetree/bindings/reserved-memory/ramoops.txt | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> > index 0eba562fe5c6..b7886fea368c 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> > +++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> > @@ -30,7 +30,7 @@ Optional properties:
> >  - ecc-size: enables ECC support and specifies ECC buffer size in bytes
> >    (defaults to 0: no ECC)
> >
> > -- record-size: maximum size in bytes of each dump done on oops/panic
> > +- record-size: maximum size in bytes of each kmsg dump.
> >    (defaults to 0: disabled)
> >
> >  - console-size: size in bytes of log buffer reserved for kernel messages
> > @@ -45,7 +45,16 @@ Optional properties:
> >  - unbuffered: if present, use unbuffered mappings to map the reserved region
> >    (defaults to buffered mappings)
> >
> > -- no-dump-oops: if present, only dump panics (defaults to panics and oops)
> > +- max-reason: if present, sets maximum type of kmsg dump reasons to store
> > +  (defaults to 2: log Oopses and Panics). This can be set to INT_MAX to
> > +  store all kmsg dumps. See include/linux/kmsg_dump.h KMSG_DUMP_* for other
> > +  kmsg dump reason values. Setting this to 0 (KMSG_DUMP_UNDEF), means the
> > +  reason filtering will be controlled by the printk.always_kmsg_dump boot
> > +  param: if unset, it will be KMSG_DUMP_OOPS, otherwise KMSG_DUMP_MAX.
> > +
> > +- no-dump-oops: deprecated, use max_reason instead. If present, and
> > +  max_reason is not specified, it is equivalent to max_reason = 1
> 
> And here (3 times).
> 
> > +  (KMSG_DUMP_PANIC).
> >
> >  - flags: if present, pass ramoops behavioral flags (defaults to 0,
> >    see include/linux/pstore_ram.h RAMOOPS_FLAG_* for flag values).
> > --
> > 2.20.1
> >

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
  2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
  2020-05-15 19:17   ` Pavel Tatashin
@ 2020-05-22 16:21   ` Petr Mladek
  2020-05-23 11:16   ` Michael Ellerman
  2 siblings, 0 replies; 20+ messages in thread
From: Petr Mladek @ 2020-05-22 16:21 UTC (permalink / raw)
  To: Kees Cook
  Cc: Pavel Tatashin, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

On Fri 2020-05-15 11:44:29, Kees Cook wrote:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
  2020-05-15 18:44 ` [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper Kees Cook
@ 2020-05-22 16:51   ` Petr Mladek
  2020-05-22 17:34     ` Kees Cook
  0 siblings, 1 reply; 20+ messages in thread
From: Petr Mladek @ 2020-05-22 16:51 UTC (permalink / raw)
  To: Kees Cook
  Cc: Pavel Tatashin, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> From: Pavel Tatashin <pasha.tatashin@soleen.com>
> 
> kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> panic, reboot, etc. It provides an interface to register a callback call
> for clients, and in that callback interface there is a field "max_reason"
> which gets ignored unless always_kmsg_dump is passed as kernel parameter.

Strictly speaking, this is not fully true. "max_reason" field is not
ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.

It should be something like:

"which gets ignored for reason higher than KMSG_DUMP_OOPS unless
always_kmsg_dump is passed as kernel parameter".

Heh, I wonder if anyone will be able to parse this ;-)


Otherwise, it looks good to me. With the updated commit message:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
  2020-05-22 16:51   ` Petr Mladek
@ 2020-05-22 17:34     ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2020-05-22 17:34 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Pavel Tatashin, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

On Fri, May 22, 2020 at 06:51:20PM +0200, Petr Mladek wrote:
> On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> > From: Pavel Tatashin <pasha.tatashin@soleen.com>
> > 
> > kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> > panic, reboot, etc. It provides an interface to register a callback call
> > for clients, and in that callback interface there is a field "max_reason"
> > which gets ignored unless always_kmsg_dump is passed as kernel parameter.
> 
> Strictly speaking, this is not fully true. "max_reason" field is not
> ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.
> 
> It should be something like:
> 
> "which gets ignored for reason higher than KMSG_DUMP_OOPS unless
> always_kmsg_dump is passed as kernel parameter".
> 
> Heh, I wonder if anyone will be able to parse this ;-)

Ah yeah, good point. I've reworded things like this:


    kmsg_dump() allows to dump kmesg buffer for various system events: oops,
    panic, reboot, etc. It provides an interface to register a callback
    call for clients, and in that callback interface there is a field
    "max_reason", but it was getting ignored when set to any "reason"
    higher than KMSG_DUMP_OOPS unless "always_kmsg_dump" was passed as
    kernel parameter.

    Allow clients to actually control their "max_reason", and keep the
    current behavior when "max_reason" is not set.

> Otherwise, it looks good to me. With the updated commit message:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Thanks!

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
  2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
  2020-05-15 19:17   ` Pavel Tatashin
  2020-05-22 16:21   ` Petr Mladek
@ 2020-05-23 11:16   ` Michael Ellerman
  2 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2020-05-23 11:16 UTC (permalink / raw)
  To: Kees Cook, Pavel Tatashin
  Cc: Kees Cook, Petr Mladek, Anton Vorontsov, Colin Cross, Tony Luck,
	Jonathan Corbet, Benson Leung, Rob Herring,
	Benjamin Herrenschmidt, Paul Mackerras, Enric Balletbo i Serra,
	Sergey Senozhatsky, Steven Rostedt, linux-doc, linux-kernel,
	devicetree, linuxppc-dev

Kees Cook <keescook@chromium.org> writes:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/powerpc/kernel/nvram_64.c | 4 +---
>  fs/pstore/platform.c           | 8 ++------
>  include/linux/kmsg_dump.h      | 4 +---
>  kernel/reboot.c                | 6 +++---
>  4 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index fb4f61096613..0cd1c88bfc8b 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -655,9 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
>  	int rc = -1;
>  
>  	switch (reason) {
> -	case KMSG_DUMP_RESTART:
> -	case KMSG_DUMP_HALT:
> -	case KMSG_DUMP_POWEROFF:
> +	case KMSG_DUMP_SHUTDOWN:
>  		/* These are almost always orderly shutdowns. */
>  		return;
>  	case KMSG_DUMP_OOPS:

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, back to index

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 18:44 [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Kees Cook
2020-05-15 18:44 ` [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason Kees Cook
2020-05-15 19:17   ` Pavel Tatashin
2020-05-22 16:21   ` Petr Mladek
2020-05-23 11:16   ` Michael Ellerman
2020-05-15 18:44 ` [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper Kees Cook
2020-05-22 16:51   ` Petr Mladek
2020-05-22 17:34     ` Kees Cook
2020-05-15 18:44 ` [PATCH v4 3/6] printk: Introduce kmsg_dump_reason_str() Kees Cook
2020-05-15 19:23   ` Pavel Tatashin
2020-05-15 18:44 ` [PATCH v4 4/6] pstore/platform: Pass max_reason to kmesg dump Kees Cook
2020-05-15 18:44 ` [PATCH v4 5/6] pstore/ram: Introduce max_reason and convert dump_oops Kees Cook
2020-05-15 19:30   ` Pavel Tatashin
2020-05-15 19:48     ` Kees Cook
2020-05-15 19:40   ` Pavel Tatashin
2020-05-15 19:53     ` Kees Cook
2020-05-15 18:44 ` [PATCH v4 6/6] ramoops: Add max_reason optional field to ramoops DT node Kees Cook
2020-05-18 22:45   ` Rob Herring
2020-05-18 23:04     ` Kees Cook
2020-05-15 19:13 ` [PATCH v4 0/6] allow ramoops to collect all kmesg_dump events Pavel Tatashin

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git