Merge ram_console into pstore

Pstore doesn't support logging kernel messages in run-time, it only dumps dmesg when kernel oopses/panics. This makes pstore useless for debugging hangs caused by HW issues or improper use of HW (e.g. weird device inserted -> driver tried to write a reserved bits -> SoC hanged. In that case we don't get any messages in the pstore.

Therefore, let's add a runtime logging support: PSTORE_TYPE_CONSOLE.

Signed-off-by Anton Vorontsov <[email protected]>

---
 fs/pstore/Kconfig      |    7 +++++++ fs/pstore/inode.c      |    3 +++ fs/pstore/platform.c   |   25 +++++++++++++++++++++++++ include/linux/pstore.h |    1 +

 4 files changed, 36 insertions(+)

fs/pstore/Kconfig +0 -0

index 23ade26..d044de6 100644
@@ -12,6 +12,13 @@ config PSTORE
           If you don't have a platform persistent store driver,

          say N.

fs/pstore/Kconfig +7 -0

+config PSTORE_CONSOLE
+       bool "Log kernel console messages"
+       depends on PSTORE
+       help
+         When the option is enabled, pstore will log all kernel
+         messages, even if no oops or panic happened.
+

 config PSTORE_RAM
       tristate "Log panic/oops to a RAM buffer"
       depends on PSTORE

fs/pstore/inode.c +0 -0

index 1950788..97d35ee 100644
@@ -212,6 +212,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id,

       case PSTORE_TYPE_DMESG:
               sprintf(name, "dmesg-%s-%lld", psname, id); break;
+       case PSTORE_TYPE_CONSOLE:
+               sprintf(name, "console-%s", psname);
+               break;
       case PSTORE_TYPE_MCE:
               sprintf(name, "mce-%s-%lld", psname, id); break;

fs/pstore/platform.c +2 -0

index 82c585f..3a384c8 100644
@@ -1,6 +1,7 @@
  /*
  * Persistent Storage - platform driver interface parts.
  *
+ * Copyright (C) 2007-2008 Google, Inc.
  * Copyright (C) 2010 Intel Corporation <[email protected]>
  *
  *  This program is free software; you can redistribute it and/or modify

@@ -22,6 +23,7 @@
  #include <linux/errno.h>
  #include <linux/init.h>
  #include <linux/kmsg_dump.h>
+#include <linux/console.h>
  #include <linux/module.h>
  #include <linux/pstore.h>
  #include <linux/string.h>

fs/pstore/platform.c static struct kmsg_dumper pstore_dumper = { · line 164

Quoted 4 lines

fs/pstore/platform.c +4 -0

+#ifdef CONFIG_PSTORE_CONSOLE
+static void pstore_console_write(struct console *con, const char *s, unsigned c)
+{
+       strncpy(psinfo->buf, s, c);

The size of psinfo->buf needs to be the length argument to strncpy, not the size of "s". If "s" isn't NULL terminated, then the min of c and buf's size should be used. And if this should be NULL terminated in buf, that needs to be added manually since strncpy won't do it if it hits the max length argument.

On Wed, May 16, 2012 at 09:49:11AM -0700, Kees Cook wrote:
[...]

Quoted 10 lines

fs/pstore/platform.c static struct kmsg_dumper pstore_dumper = { · line 164

Quoted 4 lines

fs/pstore/platform.c +4 -0

+#ifdef CONFIG_PSTORE_CONSOLE
+static void pstore_console_write(struct console *con, const char *s, unsigned c)
+{
+       strncpy(psinfo->buf, s, c);

The size of psinfo->buf needs to be the length argument to strncpy, not the size of "s". If "s" isn't NULL terminated, then the min of c and buf's size should be used. And if this should be NULL terminated in buf, that needs to be added manually since strncpy won't do it if it hits the max length argument.

Whoops. Will fix it, thanks!

This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.

Quoted 3 lines · Anton Vorontsov

This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.

What about the saving /proc/last_kmsg? Are you not bringing that feature from ramconsole? Sorry if I missed it in reading the patches.

This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.

Signed-off-by Anton Vorontsov <[email protected]>

---
 fs/pstore/ram.c |   39 ++++++++++++++++++++++++++++++---------

 1 file changed, 30 insertions(+), 9 deletions(-)

fs/pstore/ram.c +0 -0

index 9123cce..54213eb 100644
[...]

@@ -142,7 +148,12 @@ static int ramoops_pstore_write(enum pstore_type_id type,
        struct persistent_ram_zone *prz = cxt->przs[cxt->count];
        size_t hlen;

Quoted 18 lines · Anton Vorontsov

This is all straightforward: we just use the last region for console logging. If there's just one region, we fall-back to the old behaviour: just a oops/dumps logging.

Signed-off-by Anton Vorontsov <[email protected]>

---
 fs/pstore/ram.c |   39 ++++++++++++++++++++++++++++++---------

 1 file changed, 30 insertions(+), 9 deletions(-)

fs/pstore/ram.c +0 -0

index 9123cce..54213eb 100644
[...]

@@ -142,7 +148,12 @@ static int ramoops_pstore_write(enum pstore_type_id type,
        struct persistent_ram_zone *prz = cxt->przs[cxt->count];
        size_t hlen;
 

fs/pstore/ram.c static int ramoops_pstore_write(enum pstore_type_id type, · line 158

Quoted 9 lines

fs/pstore/ram.c +6 -1

-       /* Currently ramoops is designed to only store dmesg dumps. */
+       if (type == PSTORE_TYPE_CONSOLE) {
+               if (!cxt->cprz)
+                       return -ENOMEM;
+               persistent_ram_write(cxt->cprz, cxt->pstore.buf, size);
+       }
+
        if (type != PSTORE_TYPE_DMESG)
                return -EINVAL;

Doesn't this mean that type == PSTORE_TYPE_CONSOLE will write to the ram, but then fail with -EINVAL?

Suggested-by Shuah Khan <[email protected]>

Signed-off-by Anton Vorontsov <[email protected]>

---
 Documentation/ramoops.txt |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

Documentation/ramoops.txt +0 -0

index 4ba7db2..138823b 100644
@@ -40,6 +40,12 @@ corrupt, but usually it is restorable.
 Setting the ramoops parameters can be done in 2 different manners:

Documentation/ramoops.txt +1 -0

+
 3. Dump format
 
 The data dump begins with a header, currently defined as "====" followed by a

Suggested-by Shuah Khan <[email protected]>

Signed-off-by Anton Vorontsov <[email protected]>

---
 Documentation/ramoops.txt |   15 +++++++++++++++

 1 file changed, 15 insertions(+)

Documentation/ramoops.txt +0 -0

index 4ba7db2..138823b 100644
@@ -40,6 +40,12 @@ corrupt, but usually it is restorable.

 Setting the ramoops parameters can be done in 2 different manners:
 1. Use the module parameters (which have the names of the variables described as before).
+ For quick debugging, you can also reserve parts of memory during boot
+ and then use the reserved memory for ramoops. For example, assuming a machine
+ with > 128 MB of memory, the following kernel command line will tell the
+ kernel to use only the first 128 MB of memory, and place ECC-protected ramoops
+ region at 128 MB boundary:
+ "mem=128M ramoops.mem_address=0x8000000 ramoops.ecc=1"
 2. Use a platform device and set the platform data. The parameters can then be set through that platform data. An example of doing that is:

Documentation/ramoops.txt +6 -0

+e.g.:
+
+#include <linux/memblock.h>
+
+memblock_reserve(ramoops_data.mem_address, ramoops_data.mem_size);
+

 3. Dump format

 The data dump begins with a header, currently defined as "====" followed by a --
1.7.9.2

Documentation/ramoops.txt if (ret) { · line 81

Quoted 3 lines

Documentation/ramoops.txt +3 -0

+You can specify either RAM memory or peripheral devices' memory. However, when
+specifying RAM, be sure to reserve the memory by issuing memblock_reserve()
+very early in the architecture code, just before platform device registration,

Just before platform device registration is way too late. ARM provides a machine reserve callback to allow board files to call memblock_reserve inside arm_memblock_init() and before mm_init().

On Wed, May 16, 2012 at 10:56:09AM -0700, Colin Cross wrote:
[...]

Quoted 7 lines

Documentation/ramoops.txt if (ret) { · line 81

Quoted 3 lines

Documentation/ramoops.txt +3 -0

+You can specify either RAM memory or peripheral devices' memory. However, when
+specifying RAM, be sure to reserve the memory by issuing memblock_reserve()
+very early in the architecture code, just before platform device registration,

Just before platform device registration is way too late.  ARM provides a machine reserve callback to allow board files to call memblock_reserve inside arm_memblock_init() and before mm_init().

Yeah, and I guess that 'too late' is also true for all architectures, the platform_device_register and friends are most probably unusable before mm_init, and after that it's always too late.

So the word 'just' is misleading indeed, I'll remove it.