Hi all,
There are currently two competing debug facilities to store kernel
messages in a persistent storage: a generic pstore and Google's
persistent_ram. Not so long ago (https://lkml.org/lkml/2012/3/8/252),
it was decided that we should fix this situation.
Recently ramoops has switched to pstore, which basically means that
it became a RAM backend for the pstore framework.
persistent_ram+ram_console and ramoops+pstore have almost the same
features, except:
1. Ramoops doesn't support ECC. Having ECC is useful when a hardware
reset was used to bring the machine back to life (i.e. a watchdog
triggered). In such cases, RAM may be somewhat corrupt, but
usually it is restorable.
2. 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.
These patches solve the first issue, plus move things to their
proper places. Patches that will fix the second issue are pending.
Thanks,
---
drivers/char/Kconfig | 9 -
drivers/char/Makefile | 1 -
drivers/char/ramoops.c | 362 --------------------
drivers/staging/android/Kconfig | 10 +-
drivers/staging/android/persistent_ram.c | 473 --------------------------
drivers/staging/android/persistent_ram.h | 78 -----
drivers/staging/android/ram_console.c | 2 +-
fs/pstore/Kconfig | 12 +
fs/pstore/Makefile | 1 +
fs/pstore/ram.c | 384 ++++++++++++++++++++++
fs/pstore/ram_core.c | 530 ++++++++++++++++++++++++++++++
include/linux/pstore_ram.h | 99 ++++++
include/linux/ramoops.h | 17 -
13 files changed, 1028 insertions(+), 950 deletions(-)
--
Anton Vorontsov
Email: [email protected]
The 'node' struct member is unused, so remove it.
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 2 --
drivers/staging/android/persistent_ram.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index 8407112..12444fd 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -407,8 +407,6 @@ struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
goto err;
}
- INIT_LIST_HEAD(&prz->node);
-
ret = persistent_ram_buffer_init(dev_name(dev), prz);
if (ret) {
pr_err("persistent_ram: failed to initialize buffer\n");
diff --git a/drivers/staging/android/persistent_ram.h b/drivers/staging/android/persistent_ram.h
index f41e208..5635355 100644
--- a/drivers/staging/android/persistent_ram.h
+++ b/drivers/staging/android/persistent_ram.h
@@ -38,7 +38,6 @@ struct persistent_ram {
};
struct persistent_ram_zone {
- struct list_head node;
void *vaddr;
struct persistent_ram_buffer *buffer;
size_t buffer_size;
--
1.7.9.2
This is a longstanding bug, almost unnoticeable when calling
persistent_ram_write() for small buffers.
But when called for large data buffers, the write routine behaves
incorrectly, as the size may never update: instead of clamping
the size to the maximum buffer size, buffer_size_add_clamp() returns
an error (which is never checked by the write routine, btw).
To fix this, we now use buffer_size_add() that actually clamps the
size to the max value.
Also remove buffer_size_add_clamp(), it is no longer needed.
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index 12444fd..13a12bc 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -79,23 +79,6 @@ static inline void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
} while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
}
-/* increase the size counter, retuning an error if it hits the max size */
-static inline ssize_t buffer_size_add_clamp(struct persistent_ram_zone *prz,
- size_t a)
-{
- size_t old;
- size_t new;
-
- do {
- old = atomic_read(&prz->buffer->size);
- new = old + a;
- if (new > prz->buffer_size)
- return -ENOMEM;
- } while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
-
- return 0;
-}
-
static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz,
uint8_t *data, size_t len, uint8_t *ecc)
{
@@ -300,7 +283,7 @@ int notrace persistent_ram_write(struct persistent_ram_zone *prz,
c = prz->buffer_size;
}
- buffer_size_add_clamp(prz, c);
+ buffer_size_add(prz, c);
start = buffer_start_add(prz, c);
--
1.7.9.2
Factor post init logic out of __persistent_ram_init(), we'll need
it for the new persistent_ram_new() routine.
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 44 ++++++++++++++++++------------
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index 13a12bc..ec23822 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -378,28 +378,15 @@ static int __init persistent_ram_buffer_init(const char *name,
return -EINVAL;
}
-static __init
-struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
+static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool ecc)
{
- struct persistent_ram_zone *prz;
- int ret = -ENOMEM;
-
- prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
- if (!prz) {
- pr_err("persistent_ram: failed to allocate persistent ram zone\n");
- goto err;
- }
-
- ret = persistent_ram_buffer_init(dev_name(dev), prz);
- if (ret) {
- pr_err("persistent_ram: failed to initialize buffer\n");
- goto err;
- }
+ int ret;
prz->ecc = ecc;
+
ret = persistent_ram_init_ecc(prz, prz->buffer_size);
if (ret)
- goto err;
+ return ret;
if (prz->buffer->sig == PERSISTENT_RAM_SIG) {
if (buffer_size(prz) > prz->buffer_size ||
@@ -422,6 +409,29 @@ struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
atomic_set(&prz->buffer->start, 0);
atomic_set(&prz->buffer->size, 0);
+ return 0;
+}
+
+static __init
+struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
+{
+ struct persistent_ram_zone *prz;
+ int ret = -ENOMEM;
+
+ prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
+ if (!prz) {
+ pr_err("persistent_ram: failed to allocate persistent ram zone\n");
+ goto err;
+ }
+
+ ret = persistent_ram_buffer_init(dev_name(dev), prz);
+ if (ret) {
+ pr_err("persistent_ram: failed to initialize buffer\n");
+ goto err;
+ }
+
+ persistent_ram_post_init(prz, ecc);
+
return prz;
err:
kfree(prz);
--
1.7.9.2
The routine just creates a persistent ram zone at a specified address.
For persistent_ram_init_ringbuffer() we'd need to add a
'struct persistent_ram' to the global list, and associate it with a
device. We don't need all this complexity in pstore_ram, so we introduce
the simple function.
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 26 ++++++++++++++++++++++++++
drivers/staging/android/persistent_ram.h | 4 ++++
2 files changed, 30 insertions(+)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index ec23822..c0c3d32 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -412,6 +412,32 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
return 0;
}
+struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
+ size_t size,
+ bool ecc)
+{
+ struct persistent_ram_zone *prz;
+ int ret = -ENOMEM;
+
+ prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
+ if (!prz) {
+ pr_err("persistent_ram: failed to allocate persistent ram zone\n");
+ goto err;
+ }
+
+ ret = persistent_ram_buffer_map(start, size, prz);
+ if (ret)
+ goto err;
+
+ persistent_ram_post_init(prz, ecc);
+ persistent_ram_update_header_ecc(prz);
+
+ return prz;
+err:
+ kfree(prz);
+ return ERR_PTR(ret);
+}
+
static __init
struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
{
diff --git a/drivers/staging/android/persistent_ram.h b/drivers/staging/android/persistent_ram.h
index 5635355..8154d15 100644
--- a/drivers/staging/android/persistent_ram.h
+++ b/drivers/staging/android/persistent_ram.h
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/types.h>
+#include <linux/init.h>
struct persistent_ram_buffer;
@@ -62,6 +63,9 @@ struct persistent_ram_zone {
int persistent_ram_early_init(struct persistent_ram *ram);
+struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
+ size_t size,
+ bool ecc);
struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
bool ecc);
--
1.7.9.2
Factor out vmap logic out of persistent_ram_buffer_map(), this will
make the code a bit more understandable when we'll add support for
non-bootmem memory.
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index c0c3d32..ab8bff1 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -318,14 +318,14 @@ void persistent_ram_free_old(struct persistent_ram_zone *prz)
prz->old_log_size = 0;
}
-static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
- struct persistent_ram_zone *prz)
+static void *persistent_ram_vmap(phys_addr_t start, size_t size)
{
struct page **pages;
phys_addr_t page_start;
unsigned int page_count;
pgprot_t prot;
unsigned int i;
+ void *vaddr;
page_start = start - offset_in_page(start);
page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE);
@@ -336,17 +336,26 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
if (!pages) {
pr_err("%s: Failed to allocate array for %u pages\n", __func__,
page_count);
- return -ENOMEM;
+ return NULL;
}
for (i = 0; i < page_count; i++) {
phys_addr_t addr = page_start + i * PAGE_SIZE;
pages[i] = pfn_to_page(addr >> PAGE_SHIFT);
}
- prz->vaddr = vmap(pages, page_count, VM_MAP, prot);
+ vaddr = vmap(pages, page_count, VM_MAP, prot);
kfree(pages);
+
+ return vaddr;
+}
+
+static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
+ struct persistent_ram_zone *prz)
+{
+ prz->vaddr = persistent_ram_vmap(start, size);
if (!prz->vaddr) {
- pr_err("%s: Failed to map %u pages\n", __func__, page_count);
+ pr_err("%s: Failed to map 0x%llx pages at 0x%llx\n", __func__,
+ (unsigned long long)size, (unsigned long long)start);
return -ENOMEM;
}
--
1.7.9.2
This includes devices' memory (e.g. framebuffers or memory mapped
EEPROMs on a local bus), as well as the normal RAM that we don't use
for the main memory.
For the normal (but unused) ram we could use kmaps, but this assumes
highmem support, so we don't bother and just use the memory via
ioremap.
As a side effect, the following hack is possible: when used together
with pstore_ram (new ramoops) module, we can limit the normal RAM region
with mem= and then point ramoops to use the rest of the memory, e.g.
mem=128M ramoops.mem_address=0x8000000
Sure, we could just reserve the region with memblock_reserve() early in
the arch/ code, and then register a pstore_ram platform device pointing
to the reserved region. It's still a viable option if platform wants
to do so.
Also, we might want to use IO accessors in case of a real device,
but for now we don't bother (the old ramoops wasn't using it either, so
at least we don't make things worse).
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index ab8bff1..c16d7c2 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -23,6 +23,7 @@
#include <linux/rslib.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
+#include <asm/page.h>
#include "persistent_ram.h"
struct persistent_ram_buffer {
@@ -349,10 +350,25 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size)
return vaddr;
}
+static void *persistent_ram_iomap(phys_addr_t start, size_t size)
+{
+ if (!request_mem_region(start, size, "persistent_ram")) {
+ pr_err("request mem region (0x%llx@0x%llx) failed\n",
+ (unsigned long long)size, (unsigned long long)start);
+ return NULL;
+ }
+
+ return ioremap(start, size);
+}
+
static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
struct persistent_ram_zone *prz)
{
- prz->vaddr = persistent_ram_vmap(start, size);
+ if (pfn_valid(start >> PAGE_SHIFT))
+ prz->vaddr = persistent_ram_vmap(start, size);
+ else
+ prz->vaddr = persistent_ram_iomap(start, size);
+
if (!prz->vaddr) {
pr_err("%s: Failed to map 0x%llx pages at 0x%llx\n", __func__,
(unsigned long long)size, (unsigned long long)start);
--
1.7.9.2
A corresponding function to persistent_ram_new().
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/persistent_ram.c | 15 +++++++++++++++
drivers/staging/android/persistent_ram.h | 3 +++
2 files changed, 18 insertions(+)
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
index c16d7c2..63481da 100644
--- a/drivers/staging/android/persistent_ram.c
+++ b/drivers/staging/android/persistent_ram.c
@@ -364,6 +364,9 @@ static void *persistent_ram_iomap(phys_addr_t start, size_t size)
static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
struct persistent_ram_zone *prz)
{
+ prz->paddr = start;
+ prz->size = size;
+
if (pfn_valid(start >> PAGE_SHIFT))
prz->vaddr = persistent_ram_vmap(start, size);
else
@@ -437,6 +440,18 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
return 0;
}
+void persistent_ram_free(struct persistent_ram_zone *prz)
+{
+ if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
+ vunmap(prz->vaddr);
+ } else {
+ iounmap(prz->vaddr);
+ release_mem_region(prz->paddr, prz->size);
+ }
+ persistent_ram_free_old(prz);
+ kfree(prz);
+}
+
struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
size_t size,
bool ecc)
diff --git a/drivers/staging/android/persistent_ram.h b/drivers/staging/android/persistent_ram.h
index 8154d15..d3b2b45 100644
--- a/drivers/staging/android/persistent_ram.h
+++ b/drivers/staging/android/persistent_ram.h
@@ -39,6 +39,8 @@ struct persistent_ram {
};
struct persistent_ram_zone {
+ phys_addr_t paddr;
+ size_t size;
void *vaddr;
struct persistent_ram_buffer *buffer;
size_t buffer_size;
@@ -66,6 +68,7 @@ int persistent_ram_early_init(struct persistent_ram *ram);
struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
size_t size,
bool ecc);
+void persistent_ram_free(struct persistent_ram_zone *prz);
struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
bool ecc);
--
1.7.9.2
Since ramoops was converted to pstore, it has nothing to do with character
devices nowadays. Instead, today it is just a RAM backend for pstore.
The patch just moves things around. There are a few changes were needed
because of the move:
1. Kconfig and Makefiles fixups, of course.
2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
is needed to keep user experience the same as with ramoops driver
(i.e. so that ramoops.foo kernel command line arguments would still
work).
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/char/Kconfig | 9 --
drivers/char/Makefile | 1 -
drivers/char/ramoops.c | 362 -------------------------------------------
fs/pstore/Kconfig | 9 ++
fs/pstore/Makefile | 1 +
fs/pstore/ram.c | 367 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pstore_ram.h | 17 ++
include/linux/ramoops.h | 17 --
8 files changed, 394 insertions(+), 389 deletions(-)
delete mode 100644 drivers/char/ramoops.c
create mode 100644 fs/pstore/ram.c
create mode 100644 include/linux/pstore_ram.h
delete mode 100644 include/linux/ramoops.h
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index fab778d4..ea6f632 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -585,15 +585,6 @@ config DEVPORT
source "drivers/s390/char/Kconfig"
-config RAMOOPS
- tristate "Log panic/oops to a RAM buffer"
- depends on HAS_IOMEM
- depends on PSTORE
- default n
- help
- This enables panic and oops messages to be logged to a circular
- buffer in RAM where it can be read back at some later point.
-
config MSM_SMD_PKT
bool "Enable device interface for some SMD packet ports"
default n
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index 0dc5d7c..d0b27a3 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -58,7 +58,6 @@ obj-$(CONFIG_HANGCHECK_TIMER) += hangcheck-timer.o
obj-$(CONFIG_TCG_TPM) += tpm/
obj-$(CONFIG_PS3_FLASH) += ps3flash.o
-obj-$(CONFIG_RAMOOPS) += ramoops.o
obj-$(CONFIG_JS_RTC) += js-rtc.o
js-rtc-y = rtc.o
diff --git a/drivers/char/ramoops.c b/drivers/char/ramoops.c
deleted file mode 100644
index b8b8542..0000000
--- a/drivers/char/ramoops.c
+++ /dev/null
@@ -1,362 +0,0 @@
-/*
- * RAM Oops/Panic logger
- *
- * Copyright (C) 2010 Marco Stornelli <[email protected]>
- * Copyright (C) 2011 Kees Cook <[email protected]>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
- *
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/kernel.h>
-#include <linux/err.h>
-#include <linux/module.h>
-#include <linux/pstore.h>
-#include <linux/time.h>
-#include <linux/io.h>
-#include <linux/ioport.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-#include <linux/ramoops.h>
-
-#define RAMOOPS_KERNMSG_HDR "===="
-#define MIN_MEM_SIZE 4096UL
-
-static ulong record_size = MIN_MEM_SIZE;
-module_param(record_size, ulong, 0400);
-MODULE_PARM_DESC(record_size,
- "size of each dump done on oops/panic");
-
-static ulong mem_address;
-module_param(mem_address, ulong, 0400);
-MODULE_PARM_DESC(mem_address,
- "start of reserved RAM used to store oops/panic logs");
-
-static ulong mem_size;
-module_param(mem_size, ulong, 0400);
-MODULE_PARM_DESC(mem_size,
- "size of reserved RAM used to store oops/panic logs");
-
-static int dump_oops = 1;
-module_param(dump_oops, int, 0600);
-MODULE_PARM_DESC(dump_oops,
- "set to 1 to dump oopses, 0 to only dump panics (default 1)");
-
-struct ramoops_context {
- void *virt_addr;
- phys_addr_t phys_addr;
- unsigned long size;
- size_t record_size;
- int dump_oops;
- unsigned int count;
- unsigned int max_count;
- unsigned int read_count;
- struct pstore_info pstore;
-};
-
-static struct platform_device *dummy;
-static struct ramoops_platform_data *dummy_data;
-
-static int ramoops_pstore_open(struct pstore_info *psi)
-{
- struct ramoops_context *cxt = psi->data;
-
- cxt->read_count = 0;
- return 0;
-}
-
-static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
- struct timespec *time,
- char **buf,
- struct pstore_info *psi)
-{
- ssize_t size;
- char *rambuf;
- struct ramoops_context *cxt = psi->data;
-
- if (cxt->read_count >= cxt->max_count)
- return -EINVAL;
- *id = cxt->read_count++;
- /* Only supports dmesg output so far. */
- *type = PSTORE_TYPE_DMESG;
- /* TODO(kees): Bogus time for the moment. */
- time->tv_sec = 0;
- time->tv_nsec = 0;
-
- rambuf = cxt->virt_addr + (*id * cxt->record_size);
- size = strnlen(rambuf, cxt->record_size);
- *buf = kmalloc(size, GFP_KERNEL);
- if (*buf == NULL)
- return -ENOMEM;
- memcpy(*buf, rambuf, size);
-
- return size;
-}
-
-static int ramoops_pstore_write(enum pstore_type_id type,
- enum kmsg_dump_reason reason,
- u64 *id,
- unsigned int part,
- size_t size, struct pstore_info *psi)
-{
- char *buf;
- size_t res;
- struct timeval timestamp;
- struct ramoops_context *cxt = psi->data;
- size_t available = cxt->record_size;
-
- /* Currently ramoops is designed to only store dmesg dumps. */
- if (type != PSTORE_TYPE_DMESG)
- return -EINVAL;
-
- /* Out of the various dmesg dump types, ramoops is currently designed
- * to only store crash logs, rather than storing general kernel logs.
- */
- if (reason != KMSG_DUMP_OOPS &&
- reason != KMSG_DUMP_PANIC)
- return -EINVAL;
-
- /* Skip Oopes when configured to do so. */
- if (reason == KMSG_DUMP_OOPS && !cxt->dump_oops)
- return -EINVAL;
-
- /* Explicitly only take the first part of any new crash.
- * If our buffer is larger than kmsg_bytes, this can never happen,
- * and if our buffer is smaller than kmsg_bytes, we don't want the
- * report split across multiple records.
- */
- if (part != 1)
- return -ENOSPC;
-
- buf = cxt->virt_addr + (cxt->count * cxt->record_size);
-
- res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
- buf += res;
- available -= res;
-
- do_gettimeofday(×tamp);
- res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
- buf += res;
- available -= res;
-
- if (size > available)
- size = available;
-
- memcpy(buf, cxt->pstore.buf, size);
- memset(buf + size, '\0', available - size);
-
- cxt->count = (cxt->count + 1) % cxt->max_count;
-
- return 0;
-}
-
-static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
- struct pstore_info *psi)
-{
- char *buf;
- struct ramoops_context *cxt = psi->data;
-
- if (id >= cxt->max_count)
- return -EINVAL;
-
- buf = cxt->virt_addr + (id * cxt->record_size);
- memset(buf, '\0', cxt->record_size);
-
- return 0;
-}
-
-static struct ramoops_context oops_cxt = {
- .pstore = {
- .owner = THIS_MODULE,
- .name = "ramoops",
- .open = ramoops_pstore_open,
- .read = ramoops_pstore_read,
- .write = ramoops_pstore_write,
- .erase = ramoops_pstore_erase,
- },
-};
-
-static int __init ramoops_probe(struct platform_device *pdev)
-{
- struct ramoops_platform_data *pdata = pdev->dev.platform_data;
- struct ramoops_context *cxt = &oops_cxt;
- int err = -EINVAL;
-
- /* Only a single ramoops area allowed at a time, so fail extra
- * probes.
- */
- if (cxt->max_count)
- goto fail_out;
-
- if (!pdata->mem_size || !pdata->record_size) {
- pr_err("The memory size and the record size must be "
- "non-zero\n");
- goto fail_out;
- }
-
- pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
- pdata->record_size = rounddown_pow_of_two(pdata->record_size);
-
- /* Check for the minimum memory size */
- if (pdata->mem_size < MIN_MEM_SIZE &&
- pdata->record_size < MIN_MEM_SIZE) {
- pr_err("memory size too small, minimum is %lu\n",
- MIN_MEM_SIZE);
- goto fail_out;
- }
-
- if (pdata->mem_size < pdata->record_size) {
- pr_err("The memory size must be larger than the "
- "records size\n");
- goto fail_out;
- }
-
- cxt->max_count = pdata->mem_size / pdata->record_size;
- cxt->count = 0;
- cxt->size = pdata->mem_size;
- cxt->phys_addr = pdata->mem_address;
- cxt->record_size = pdata->record_size;
- cxt->dump_oops = pdata->dump_oops;
-
- cxt->pstore.data = cxt;
- cxt->pstore.bufsize = cxt->record_size;
- cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
- spin_lock_init(&cxt->pstore.buf_lock);
- if (!cxt->pstore.buf) {
- pr_err("cannot allocate pstore buffer\n");
- goto fail_clear;
- }
-
- if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
- pr_err("request mem region (0x%lx@0x%llx) failed\n",
- cxt->size, (unsigned long long)cxt->phys_addr);
- err = -EINVAL;
- goto fail_buf;
- }
-
- cxt->virt_addr = ioremap(cxt->phys_addr, cxt->size);
- if (!cxt->virt_addr) {
- pr_err("ioremap failed\n");
- goto fail_mem_region;
- }
-
- err = pstore_register(&cxt->pstore);
- if (err) {
- pr_err("registering with pstore failed\n");
- goto fail_iounmap;
- }
-
- /*
- * Update the module parameter variables as well so they are visible
- * through /sys/module/ramoops/parameters/
- */
- mem_size = pdata->mem_size;
- mem_address = pdata->mem_address;
- record_size = pdata->record_size;
- dump_oops = pdata->dump_oops;
-
- pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
- cxt->size, (unsigned long long)cxt->phys_addr,
- cxt->max_count, cxt->record_size);
-
- return 0;
-
-fail_iounmap:
- iounmap(cxt->virt_addr);
-fail_mem_region:
- release_mem_region(cxt->phys_addr, cxt->size);
-fail_buf:
- kfree(cxt->pstore.buf);
-fail_clear:
- cxt->pstore.bufsize = 0;
- cxt->max_count = 0;
-fail_out:
- return err;
-}
-
-static int __exit ramoops_remove(struct platform_device *pdev)
-{
-#if 0
- /* TODO(kees): We cannot unload ramoops since pstore doesn't support
- * unregistering yet.
- */
- struct ramoops_context *cxt = &oops_cxt;
-
- iounmap(cxt->virt_addr);
- release_mem_region(cxt->phys_addr, cxt->size);
- cxt->max_count = 0;
-
- /* TODO(kees): When pstore supports unregistering, call it here. */
- kfree(cxt->pstore.buf);
- cxt->pstore.bufsize = 0;
-
- return 0;
-#endif
- return -EBUSY;
-}
-
-static struct platform_driver ramoops_driver = {
- .remove = __exit_p(ramoops_remove),
- .driver = {
- .name = "ramoops",
- .owner = THIS_MODULE,
- },
-};
-
-static int __init ramoops_init(void)
-{
- int ret;
- ret = platform_driver_probe(&ramoops_driver, ramoops_probe);
- if (ret == -ENODEV) {
- /*
- * If we didn't find a platform device, we use module parameters
- * building platform data on the fly.
- */
- pr_info("platform device not found, using module parameters\n");
- dummy_data = kzalloc(sizeof(struct ramoops_platform_data),
- GFP_KERNEL);
- if (!dummy_data)
- return -ENOMEM;
- dummy_data->mem_size = mem_size;
- dummy_data->mem_address = mem_address;
- dummy_data->record_size = record_size;
- dummy_data->dump_oops = dump_oops;
- dummy = platform_create_bundle(&ramoops_driver, ramoops_probe,
- NULL, 0, dummy_data,
- sizeof(struct ramoops_platform_data));
-
- if (IS_ERR(dummy))
- ret = PTR_ERR(dummy);
- else
- ret = 0;
- }
-
- return ret;
-}
-
-static void __exit ramoops_exit(void)
-{
- platform_driver_unregister(&ramoops_driver);
- kfree(dummy_data);
-}
-
-module_init(ramoops_init);
-module_exit(ramoops_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Marco Stornelli <[email protected]>");
-MODULE_DESCRIPTION("RAM Oops/Panic logger/driver");
diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index 8007ae7..ad6e594 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -11,3 +11,12 @@ config PSTORE
(e.g. ACPI_APEI on X86) which will select this for you.
If you don't have a platform persistent store driver,
say N.
+
+config PSTORE_RAM
+ tristate "Log panic/oops to a RAM buffer"
+ depends on HAS_IOMEM
+ depends on PSTORE
+ default n
+ help
+ This enables panic and oops messages to be logged to a circular
+ buffer in RAM where it can be read back at some later point.
diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
index 760f4bc..804e376 100644
--- a/fs/pstore/Makefile
+++ b/fs/pstore/Makefile
@@ -5,3 +5,4 @@
obj-y += pstore.o
pstore-objs += inode.o platform.o
+obj-$(CONFIG_PSTORE_RAM) += ram.o
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
new file mode 100644
index 0000000..b26b58e
--- /dev/null
+++ b/fs/pstore/ram.c
@@ -0,0 +1,367 @@
+/*
+ * RAM Oops/Panic logger
+ *
+ * Copyright (C) 2010 Marco Stornelli <[email protected]>
+ * Copyright (C) 2011 Kees Cook <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+#define pr_fmt(fmt) "ramoops: " fmt
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/pstore.h>
+#include <linux/time.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/pstore_ram.h>
+
+/* For historical reasons we name it ramoops when built-in. */
+#ifndef MODULE
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "ramoops."
+#endif
+
+#define RAMOOPS_KERNMSG_HDR "===="
+#define MIN_MEM_SIZE 4096UL
+
+static ulong record_size = MIN_MEM_SIZE;
+module_param(record_size, ulong, 0400);
+MODULE_PARM_DESC(record_size,
+ "size of each dump done on oops/panic");
+
+static ulong mem_address;
+module_param(mem_address, ulong, 0400);
+MODULE_PARM_DESC(mem_address,
+ "start of reserved RAM used to store oops/panic logs");
+
+static ulong mem_size;
+module_param(mem_size, ulong, 0400);
+MODULE_PARM_DESC(mem_size,
+ "size of reserved RAM used to store oops/panic logs");
+
+static int dump_oops = 1;
+module_param(dump_oops, int, 0600);
+MODULE_PARM_DESC(dump_oops,
+ "set to 1 to dump oopses, 0 to only dump panics (default 1)");
+
+struct ramoops_context {
+ void *virt_addr;
+ phys_addr_t phys_addr;
+ unsigned long size;
+ size_t record_size;
+ int dump_oops;
+ unsigned int count;
+ unsigned int max_count;
+ unsigned int read_count;
+ struct pstore_info pstore;
+};
+
+static struct platform_device *dummy;
+static struct ramoops_platform_data *dummy_data;
+
+static int ramoops_pstore_open(struct pstore_info *psi)
+{
+ struct ramoops_context *cxt = psi->data;
+
+ cxt->read_count = 0;
+ return 0;
+}
+
+static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
+ struct timespec *time,
+ char **buf,
+ struct pstore_info *psi)
+{
+ ssize_t size;
+ char *rambuf;
+ struct ramoops_context *cxt = psi->data;
+
+ if (cxt->read_count >= cxt->max_count)
+ return -EINVAL;
+ *id = cxt->read_count++;
+ /* Only supports dmesg output so far. */
+ *type = PSTORE_TYPE_DMESG;
+ /* TODO(kees): Bogus time for the moment. */
+ time->tv_sec = 0;
+ time->tv_nsec = 0;
+
+ rambuf = cxt->virt_addr + (*id * cxt->record_size);
+ size = strnlen(rambuf, cxt->record_size);
+ *buf = kmalloc(size, GFP_KERNEL);
+ if (*buf == NULL)
+ return -ENOMEM;
+ memcpy(*buf, rambuf, size);
+
+ return size;
+}
+
+static int ramoops_pstore_write(enum pstore_type_id type,
+ enum kmsg_dump_reason reason,
+ u64 *id,
+ unsigned int part,
+ size_t size, struct pstore_info *psi)
+{
+ char *buf;
+ size_t res;
+ struct timeval timestamp;
+ struct ramoops_context *cxt = psi->data;
+ size_t available = cxt->record_size;
+
+ /* Currently ramoops is designed to only store dmesg dumps. */
+ if (type != PSTORE_TYPE_DMESG)
+ return -EINVAL;
+
+ /* Out of the various dmesg dump types, ramoops is currently designed
+ * to only store crash logs, rather than storing general kernel logs.
+ */
+ if (reason != KMSG_DUMP_OOPS &&
+ reason != KMSG_DUMP_PANIC)
+ return -EINVAL;
+
+ /* Skip Oopes when configured to do so. */
+ if (reason == KMSG_DUMP_OOPS && !cxt->dump_oops)
+ return -EINVAL;
+
+ /* Explicitly only take the first part of any new crash.
+ * If our buffer is larger than kmsg_bytes, this can never happen,
+ * and if our buffer is smaller than kmsg_bytes, we don't want the
+ * report split across multiple records.
+ */
+ if (part != 1)
+ return -ENOSPC;
+
+ buf = cxt->virt_addr + (cxt->count * cxt->record_size);
+
+ res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
+ buf += res;
+ available -= res;
+
+ do_gettimeofday(×tamp);
+ res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
+ buf += res;
+ available -= res;
+
+ if (size > available)
+ size = available;
+
+ memcpy(buf, cxt->pstore.buf, size);
+ memset(buf + size, '\0', available - size);
+
+ cxt->count = (cxt->count + 1) % cxt->max_count;
+
+ return 0;
+}
+
+static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
+ struct pstore_info *psi)
+{
+ char *buf;
+ struct ramoops_context *cxt = psi->data;
+
+ if (id >= cxt->max_count)
+ return -EINVAL;
+
+ buf = cxt->virt_addr + (id * cxt->record_size);
+ memset(buf, '\0', cxt->record_size);
+
+ return 0;
+}
+
+static struct ramoops_context oops_cxt = {
+ .pstore = {
+ .owner = THIS_MODULE,
+ .name = "ramoops",
+ .open = ramoops_pstore_open,
+ .read = ramoops_pstore_read,
+ .write = ramoops_pstore_write,
+ .erase = ramoops_pstore_erase,
+ },
+};
+
+static int __init ramoops_probe(struct platform_device *pdev)
+{
+ struct ramoops_platform_data *pdata = pdev->dev.platform_data;
+ struct ramoops_context *cxt = &oops_cxt;
+ int err = -EINVAL;
+
+ /* Only a single ramoops area allowed at a time, so fail extra
+ * probes.
+ */
+ if (cxt->max_count)
+ goto fail_out;
+
+ if (!pdata->mem_size || !pdata->record_size) {
+ pr_err("The memory size and the record size must be "
+ "non-zero\n");
+ goto fail_out;
+ }
+
+ pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
+ pdata->record_size = rounddown_pow_of_two(pdata->record_size);
+
+ /* Check for the minimum memory size */
+ if (pdata->mem_size < MIN_MEM_SIZE &&
+ pdata->record_size < MIN_MEM_SIZE) {
+ pr_err("memory size too small, minimum is %lu\n",
+ MIN_MEM_SIZE);
+ goto fail_out;
+ }
+
+ if (pdata->mem_size < pdata->record_size) {
+ pr_err("The memory size must be larger than the "
+ "records size\n");
+ goto fail_out;
+ }
+
+ cxt->max_count = pdata->mem_size / pdata->record_size;
+ cxt->count = 0;
+ cxt->size = pdata->mem_size;
+ cxt->phys_addr = pdata->mem_address;
+ cxt->record_size = pdata->record_size;
+ cxt->dump_oops = pdata->dump_oops;
+
+ cxt->pstore.data = cxt;
+ cxt->pstore.bufsize = cxt->record_size;
+ cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
+ spin_lock_init(&cxt->pstore.buf_lock);
+ if (!cxt->pstore.buf) {
+ pr_err("cannot allocate pstore buffer\n");
+ goto fail_clear;
+ }
+
+ if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
+ pr_err("request mem region (0x%lx@0x%llx) failed\n",
+ cxt->size, (unsigned long long)cxt->phys_addr);
+ err = -EINVAL;
+ goto fail_buf;
+ }
+
+ cxt->virt_addr = ioremap(cxt->phys_addr, cxt->size);
+ if (!cxt->virt_addr) {
+ pr_err("ioremap failed\n");
+ goto fail_mem_region;
+ }
+
+ err = pstore_register(&cxt->pstore);
+ if (err) {
+ pr_err("registering with pstore failed\n");
+ goto fail_iounmap;
+ }
+
+ /*
+ * Update the module parameter variables as well so they are visible
+ * through /sys/module/ramoops/parameters/
+ */
+ mem_size = pdata->mem_size;
+ mem_address = pdata->mem_address;
+ record_size = pdata->record_size;
+ dump_oops = pdata->dump_oops;
+
+ pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
+ cxt->size, (unsigned long long)cxt->phys_addr,
+ cxt->max_count, cxt->record_size);
+
+ return 0;
+
+fail_iounmap:
+ iounmap(cxt->virt_addr);
+fail_mem_region:
+ release_mem_region(cxt->phys_addr, cxt->size);
+fail_buf:
+ kfree(cxt->pstore.buf);
+fail_clear:
+ cxt->pstore.bufsize = 0;
+ cxt->max_count = 0;
+fail_out:
+ return err;
+}
+
+static int __exit ramoops_remove(struct platform_device *pdev)
+{
+#if 0
+ /* TODO(kees): We cannot unload ramoops since pstore doesn't support
+ * unregistering yet.
+ */
+ struct ramoops_context *cxt = &oops_cxt;
+
+ iounmap(cxt->virt_addr);
+ release_mem_region(cxt->phys_addr, cxt->size);
+ cxt->max_count = 0;
+
+ /* TODO(kees): When pstore supports unregistering, call it here. */
+ kfree(cxt->pstore.buf);
+ cxt->pstore.bufsize = 0;
+
+ return 0;
+#endif
+ return -EBUSY;
+}
+
+static struct platform_driver ramoops_driver = {
+ .remove = __exit_p(ramoops_remove),
+ .driver = {
+ .name = "ramoops",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init ramoops_init(void)
+{
+ int ret;
+ ret = platform_driver_probe(&ramoops_driver, ramoops_probe);
+ if (ret == -ENODEV) {
+ /*
+ * If we didn't find a platform device, we use module parameters
+ * building platform data on the fly.
+ */
+ pr_info("platform device not found, using module parameters\n");
+ dummy_data = kzalloc(sizeof(struct ramoops_platform_data),
+ GFP_KERNEL);
+ if (!dummy_data)
+ return -ENOMEM;
+ dummy_data->mem_size = mem_size;
+ dummy_data->mem_address = mem_address;
+ dummy_data->record_size = record_size;
+ dummy_data->dump_oops = dump_oops;
+ dummy = platform_create_bundle(&ramoops_driver, ramoops_probe,
+ NULL, 0, dummy_data,
+ sizeof(struct ramoops_platform_data));
+
+ if (IS_ERR(dummy))
+ ret = PTR_ERR(dummy);
+ else
+ ret = 0;
+ }
+
+ return ret;
+}
+
+static void __exit ramoops_exit(void)
+{
+ platform_driver_unregister(&ramoops_driver);
+ kfree(dummy_data);
+}
+
+module_init(ramoops_init);
+module_exit(ramoops_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Marco Stornelli <[email protected]>");
+MODULE_DESCRIPTION("RAM Oops/Panic logger/driver");
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
new file mode 100644
index 0000000..484fef8
--- /dev/null
+++ b/include/linux/pstore_ram.h
@@ -0,0 +1,17 @@
+#ifndef __RAMOOPS_H
+#define __RAMOOPS_H
+
+/*
+ * Ramoops platform data
+ * @mem_size memory size for ramoops
+ * @mem_address physical memory address to contain ramoops
+ */
+
+struct ramoops_platform_data {
+ unsigned long mem_size;
+ unsigned long mem_address;
+ unsigned long record_size;
+ int dump_oops;
+};
+
+#endif
diff --git a/include/linux/ramoops.h b/include/linux/ramoops.h
deleted file mode 100644
index 484fef8..0000000
--- a/include/linux/ramoops.h
+++ /dev/null
@@ -1,17 +0,0 @@
-#ifndef __RAMOOPS_H
-#define __RAMOOPS_H
-
-/*
- * Ramoops platform data
- * @mem_size memory size for ramoops
- * @mem_address physical memory address to contain ramoops
- */
-
-struct ramoops_platform_data {
- unsigned long mem_size;
- unsigned long mem_address;
- unsigned long record_size;
- int dump_oops;
-};
-
-#endif
--
1.7.9.2
This is a first step for adding ECC support for pstore RAM backend: we
will use the persistent_ram routines, kindly provided by Google.
Basically, persistent_ram is a set of helper routines to deal with the
[optionally] ECC-protected persistent ram regions.
A bit of Makefile, Kconfig and header files adjustments were needed
because of the move.
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/staging/android/Kconfig | 10 +-
drivers/staging/android/persistent_ram.c | 530 ------------------------------
drivers/staging/android/persistent_ram.h | 84 -----
drivers/staging/android/ram_console.c | 2 +-
fs/pstore/Kconfig | 7 +-
fs/pstore/Makefile | 2 +-
fs/pstore/ram_core.c | 530 ++++++++++++++++++++++++++++++
include/linux/pstore_ram.h | 86 ++++-
8 files changed, 622 insertions(+), 629 deletions(-)
delete mode 100644 drivers/staging/android/persistent_ram.c
delete mode 100644 drivers/staging/android/persistent_ram.h
create mode 100644 fs/pstore/ram_core.c
diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index 42f0133..4bfcceb 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -25,17 +25,9 @@ config ANDROID_LOGGER
tristate "Android log driver"
default n
-config ANDROID_PERSISTENT_RAM
- bool
- depends on HAVE_MEMBLOCK
- select REED_SOLOMON
- select REED_SOLOMON_ENC8
- select REED_SOLOMON_DEC8
-
config ANDROID_RAM_CONSOLE
bool "Android RAM buffer console"
- depends on !S390 && !UML && HAVE_MEMBLOCK
- select ANDROID_PERSISTENT_RAM
+ depends on !S390 && !UML && HAVE_MEMBLOCK && PSTORE_RAM
default n
config ANDROID_TIMED_OUTPUT
diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
deleted file mode 100644
index 63481da..0000000
--- a/drivers/staging/android/persistent_ram.c
+++ /dev/null
@@ -1,530 +0,0 @@
-/*
- * Copyright (C) 2012 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- */
-
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/errno.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/list.h>
-#include <linux/memblock.h>
-#include <linux/rslib.h>
-#include <linux/slab.h>
-#include <linux/vmalloc.h>
-#include <asm/page.h>
-#include "persistent_ram.h"
-
-struct persistent_ram_buffer {
- uint32_t sig;
- atomic_t start;
- atomic_t size;
- uint8_t data[0];
-};
-
-#define PERSISTENT_RAM_SIG (0x43474244) /* DBGC */
-
-static __initdata LIST_HEAD(persistent_ram_list);
-
-static inline size_t buffer_size(struct persistent_ram_zone *prz)
-{
- return atomic_read(&prz->buffer->size);
-}
-
-static inline size_t buffer_start(struct persistent_ram_zone *prz)
-{
- return atomic_read(&prz->buffer->start);
-}
-
-/* increase and wrap the start pointer, returning the old value */
-static inline size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a)
-{
- int old;
- int new;
-
- do {
- old = atomic_read(&prz->buffer->start);
- new = old + a;
- while (unlikely(new > prz->buffer_size))
- new -= prz->buffer_size;
- } while (atomic_cmpxchg(&prz->buffer->start, old, new) != old);
-
- return old;
-}
-
-/* increase the size counter until it hits the max size */
-static inline void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
-{
- size_t old;
- size_t new;
-
- if (atomic_read(&prz->buffer->size) == prz->buffer_size)
- return;
-
- do {
- old = atomic_read(&prz->buffer->size);
- new = old + a;
- if (new > prz->buffer_size)
- new = prz->buffer_size;
- } while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
-}
-
-static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz,
- uint8_t *data, size_t len, uint8_t *ecc)
-{
- int i;
- uint16_t par[prz->ecc_size];
-
- /* Initialize the parity buffer */
- memset(par, 0, sizeof(par));
- encode_rs8(prz->rs_decoder, data, len, par, 0);
- for (i = 0; i < prz->ecc_size; i++)
- ecc[i] = par[i];
-}
-
-static int persistent_ram_decode_rs8(struct persistent_ram_zone *prz,
- void *data, size_t len, uint8_t *ecc)
-{
- int i;
- uint16_t par[prz->ecc_size];
-
- for (i = 0; i < prz->ecc_size; i++)
- par[i] = ecc[i];
- return decode_rs8(prz->rs_decoder, data, par, len,
- NULL, 0, NULL, 0, NULL);
-}
-
-static void notrace persistent_ram_update_ecc(struct persistent_ram_zone *prz,
- unsigned int start, unsigned int count)
-{
- struct persistent_ram_buffer *buffer = prz->buffer;
- uint8_t *buffer_end = buffer->data + prz->buffer_size;
- uint8_t *block;
- uint8_t *par;
- int ecc_block_size = prz->ecc_block_size;
- int ecc_size = prz->ecc_size;
- int size = prz->ecc_block_size;
-
- if (!prz->ecc)
- return;
-
- block = buffer->data + (start & ~(ecc_block_size - 1));
- par = prz->par_buffer + (start / ecc_block_size) * prz->ecc_size;
-
- do {
- if (block + ecc_block_size > buffer_end)
- size = buffer_end - block;
- persistent_ram_encode_rs8(prz, block, size, par);
- block += ecc_block_size;
- par += ecc_size;
- } while (block < buffer->data + start + count);
-}
-
-static void persistent_ram_update_header_ecc(struct persistent_ram_zone *prz)
-{
- struct persistent_ram_buffer *buffer = prz->buffer;
-
- if (!prz->ecc)
- return;
-
- persistent_ram_encode_rs8(prz, (uint8_t *)buffer, sizeof(*buffer),
- prz->par_header);
-}
-
-static void persistent_ram_ecc_old(struct persistent_ram_zone *prz)
-{
- struct persistent_ram_buffer *buffer = prz->buffer;
- uint8_t *block;
- uint8_t *par;
-
- if (!prz->ecc)
- return;
-
- block = buffer->data;
- par = prz->par_buffer;
- while (block < buffer->data + buffer_size(prz)) {
- int numerr;
- int size = prz->ecc_block_size;
- if (block + size > buffer->data + prz->buffer_size)
- size = buffer->data + prz->buffer_size - block;
- numerr = persistent_ram_decode_rs8(prz, block, size, par);
- if (numerr > 0) {
- pr_devel("persistent_ram: error in block %p, %d\n",
- block, numerr);
- prz->corrected_bytes += numerr;
- } else if (numerr < 0) {
- pr_devel("persistent_ram: uncorrectable error in block %p\n",
- block);
- prz->bad_blocks++;
- }
- block += prz->ecc_block_size;
- par += prz->ecc_size;
- }
-}
-
-static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
- size_t buffer_size)
-{
- int numerr;
- struct persistent_ram_buffer *buffer = prz->buffer;
- int ecc_blocks;
-
- if (!prz->ecc)
- return 0;
-
- prz->ecc_block_size = 128;
- prz->ecc_size = 16;
- prz->ecc_symsize = 8;
- prz->ecc_poly = 0x11d;
-
- ecc_blocks = DIV_ROUND_UP(prz->buffer_size, prz->ecc_block_size);
- prz->buffer_size -= (ecc_blocks + 1) * prz->ecc_size;
-
- if (prz->buffer_size > buffer_size) {
- pr_err("persistent_ram: invalid size %zu, non-ecc datasize %zu\n",
- buffer_size, prz->buffer_size);
- return -EINVAL;
- }
-
- prz->par_buffer = buffer->data + prz->buffer_size;
- prz->par_header = prz->par_buffer + ecc_blocks * prz->ecc_size;
-
- /*
- * first consecutive root is 0
- * primitive element to generate roots = 1
- */
- prz->rs_decoder = init_rs(prz->ecc_symsize, prz->ecc_poly, 0, 1,
- prz->ecc_size);
- if (prz->rs_decoder == NULL) {
- pr_info("persistent_ram: init_rs failed\n");
- return -EINVAL;
- }
-
- prz->corrected_bytes = 0;
- prz->bad_blocks = 0;
-
- numerr = persistent_ram_decode_rs8(prz, buffer, sizeof(*buffer),
- prz->par_header);
- if (numerr > 0) {
- pr_info("persistent_ram: error in header, %d\n", numerr);
- prz->corrected_bytes += numerr;
- } else if (numerr < 0) {
- pr_info("persistent_ram: uncorrectable error in header\n");
- prz->bad_blocks++;
- }
-
- return 0;
-}
-
-ssize_t persistent_ram_ecc_string(struct persistent_ram_zone *prz,
- char *str, size_t len)
-{
- ssize_t ret;
-
- if (prz->corrected_bytes || prz->bad_blocks)
- ret = snprintf(str, len, ""
- "\n%d Corrected bytes, %d unrecoverable blocks\n",
- prz->corrected_bytes, prz->bad_blocks);
- else
- ret = snprintf(str, len, "\nNo errors detected\n");
-
- return ret;
-}
-
-static void notrace persistent_ram_update(struct persistent_ram_zone *prz,
- const void *s, unsigned int start, unsigned int count)
-{
- struct persistent_ram_buffer *buffer = prz->buffer;
- memcpy(buffer->data + start, s, count);
- persistent_ram_update_ecc(prz, start, count);
-}
-
-static void __init
-persistent_ram_save_old(struct persistent_ram_zone *prz)
-{
- struct persistent_ram_buffer *buffer = prz->buffer;
- size_t size = buffer_size(prz);
- size_t start = buffer_start(prz);
- char *dest;
-
- persistent_ram_ecc_old(prz);
-
- dest = kmalloc(size, GFP_KERNEL);
- if (dest == NULL) {
- pr_err("persistent_ram: failed to allocate buffer\n");
- return;
- }
-
- prz->old_log = dest;
- prz->old_log_size = size;
- memcpy(prz->old_log, &buffer->data[start], size - start);
- memcpy(prz->old_log + size - start, &buffer->data[0], start);
-}
-
-int notrace persistent_ram_write(struct persistent_ram_zone *prz,
- const void *s, unsigned int count)
-{
- int rem;
- int c = count;
- size_t start;
-
- if (unlikely(c > prz->buffer_size)) {
- s += c - prz->buffer_size;
- c = prz->buffer_size;
- }
-
- buffer_size_add(prz, c);
-
- start = buffer_start_add(prz, c);
-
- rem = prz->buffer_size - start;
- if (unlikely(rem < c)) {
- persistent_ram_update(prz, s, start, rem);
- s += rem;
- c -= rem;
- start = 0;
- }
- persistent_ram_update(prz, s, start, c);
-
- persistent_ram_update_header_ecc(prz);
-
- return count;
-}
-
-size_t persistent_ram_old_size(struct persistent_ram_zone *prz)
-{
- return prz->old_log_size;
-}
-
-void *persistent_ram_old(struct persistent_ram_zone *prz)
-{
- return prz->old_log;
-}
-
-void persistent_ram_free_old(struct persistent_ram_zone *prz)
-{
- kfree(prz->old_log);
- prz->old_log = NULL;
- prz->old_log_size = 0;
-}
-
-static void *persistent_ram_vmap(phys_addr_t start, size_t size)
-{
- struct page **pages;
- phys_addr_t page_start;
- unsigned int page_count;
- pgprot_t prot;
- unsigned int i;
- void *vaddr;
-
- page_start = start - offset_in_page(start);
- page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE);
-
- prot = pgprot_noncached(PAGE_KERNEL);
-
- pages = kmalloc(sizeof(struct page *) * page_count, GFP_KERNEL);
- if (!pages) {
- pr_err("%s: Failed to allocate array for %u pages\n", __func__,
- page_count);
- return NULL;
- }
-
- for (i = 0; i < page_count; i++) {
- phys_addr_t addr = page_start + i * PAGE_SIZE;
- pages[i] = pfn_to_page(addr >> PAGE_SHIFT);
- }
- vaddr = vmap(pages, page_count, VM_MAP, prot);
- kfree(pages);
-
- return vaddr;
-}
-
-static void *persistent_ram_iomap(phys_addr_t start, size_t size)
-{
- if (!request_mem_region(start, size, "persistent_ram")) {
- pr_err("request mem region (0x%llx@0x%llx) failed\n",
- (unsigned long long)size, (unsigned long long)start);
- return NULL;
- }
-
- return ioremap(start, size);
-}
-
-static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
- struct persistent_ram_zone *prz)
-{
- prz->paddr = start;
- prz->size = size;
-
- if (pfn_valid(start >> PAGE_SHIFT))
- prz->vaddr = persistent_ram_vmap(start, size);
- else
- prz->vaddr = persistent_ram_iomap(start, size);
-
- if (!prz->vaddr) {
- pr_err("%s: Failed to map 0x%llx pages at 0x%llx\n", __func__,
- (unsigned long long)size, (unsigned long long)start);
- return -ENOMEM;
- }
-
- prz->buffer = prz->vaddr + offset_in_page(start);
- prz->buffer_size = size - sizeof(struct persistent_ram_buffer);
-
- return 0;
-}
-
-static int __init persistent_ram_buffer_init(const char *name,
- struct persistent_ram_zone *prz)
-{
- int i;
- struct persistent_ram *ram;
- struct persistent_ram_descriptor *desc;
- phys_addr_t start;
-
- list_for_each_entry(ram, &persistent_ram_list, node) {
- start = ram->start;
- for (i = 0; i < ram->num_descs; i++) {
- desc = &ram->descs[i];
- if (!strcmp(desc->name, name))
- return persistent_ram_buffer_map(start,
- desc->size, prz);
- start += desc->size;
- }
- }
-
- return -EINVAL;
-}
-
-static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool ecc)
-{
- int ret;
-
- prz->ecc = ecc;
-
- ret = persistent_ram_init_ecc(prz, prz->buffer_size);
- if (ret)
- return ret;
-
- if (prz->buffer->sig == PERSISTENT_RAM_SIG) {
- if (buffer_size(prz) > prz->buffer_size ||
- buffer_start(prz) > buffer_size(prz))
- pr_info("persistent_ram: found existing invalid buffer,"
- " size %zu, start %zu\n",
- buffer_size(prz), buffer_start(prz));
- else {
- pr_info("persistent_ram: found existing buffer,"
- " size %zu, start %zu\n",
- buffer_size(prz), buffer_start(prz));
- persistent_ram_save_old(prz);
- }
- } else {
- pr_info("persistent_ram: no valid data in buffer"
- " (sig = 0x%08x)\n", prz->buffer->sig);
- }
-
- prz->buffer->sig = PERSISTENT_RAM_SIG;
- atomic_set(&prz->buffer->start, 0);
- atomic_set(&prz->buffer->size, 0);
-
- return 0;
-}
-
-void persistent_ram_free(struct persistent_ram_zone *prz)
-{
- if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
- vunmap(prz->vaddr);
- } else {
- iounmap(prz->vaddr);
- release_mem_region(prz->paddr, prz->size);
- }
- persistent_ram_free_old(prz);
- kfree(prz);
-}
-
-struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
- size_t size,
- bool ecc)
-{
- struct persistent_ram_zone *prz;
- int ret = -ENOMEM;
-
- prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
- if (!prz) {
- pr_err("persistent_ram: failed to allocate persistent ram zone\n");
- goto err;
- }
-
- ret = persistent_ram_buffer_map(start, size, prz);
- if (ret)
- goto err;
-
- persistent_ram_post_init(prz, ecc);
- persistent_ram_update_header_ecc(prz);
-
- return prz;
-err:
- kfree(prz);
- return ERR_PTR(ret);
-}
-
-static __init
-struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
-{
- struct persistent_ram_zone *prz;
- int ret = -ENOMEM;
-
- prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
- if (!prz) {
- pr_err("persistent_ram: failed to allocate persistent ram zone\n");
- goto err;
- }
-
- ret = persistent_ram_buffer_init(dev_name(dev), prz);
- if (ret) {
- pr_err("persistent_ram: failed to initialize buffer\n");
- goto err;
- }
-
- persistent_ram_post_init(prz, ecc);
-
- return prz;
-err:
- kfree(prz);
- return ERR_PTR(ret);
-}
-
-struct persistent_ram_zone * __init
-persistent_ram_init_ringbuffer(struct device *dev, bool ecc)
-{
- return __persistent_ram_init(dev, ecc);
-}
-
-int __init persistent_ram_early_init(struct persistent_ram *ram)
-{
- int ret;
-
- ret = memblock_reserve(ram->start, ram->size);
- if (ret) {
- pr_err("Failed to reserve persistent memory from %08lx-%08lx\n",
- (long)ram->start, (long)(ram->start + ram->size - 1));
- return ret;
- }
-
- list_add_tail(&ram->node, &persistent_ram_list);
-
- pr_info("Initialized persistent memory from %08lx-%08lx\n",
- (long)ram->start, (long)(ram->start + ram->size - 1));
-
- return 0;
-}
diff --git a/drivers/staging/android/persistent_ram.h b/drivers/staging/android/persistent_ram.h
deleted file mode 100644
index d3b2b45..0000000
--- a/drivers/staging/android/persistent_ram.h
+++ /dev/null
@@ -1,84 +0,0 @@
-/*
- * Copyright (C) 2011 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- */
-
-#ifndef __LINUX_PERSISTENT_RAM_H__
-#define __LINUX_PERSISTENT_RAM_H__
-
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/list.h>
-#include <linux/types.h>
-#include <linux/init.h>
-
-struct persistent_ram_buffer;
-
-struct persistent_ram_descriptor {
- const char *name;
- phys_addr_t size;
-};
-
-struct persistent_ram {
- phys_addr_t start;
- phys_addr_t size;
-
- int num_descs;
- struct persistent_ram_descriptor *descs;
-
- struct list_head node;
-};
-
-struct persistent_ram_zone {
- phys_addr_t paddr;
- size_t size;
- void *vaddr;
- struct persistent_ram_buffer *buffer;
- size_t buffer_size;
-
- /* ECC correction */
- bool ecc;
- char *par_buffer;
- char *par_header;
- struct rs_control *rs_decoder;
- int corrected_bytes;
- int bad_blocks;
- int ecc_block_size;
- int ecc_size;
- int ecc_symsize;
- int ecc_poly;
-
- char *old_log;
- size_t old_log_size;
- size_t old_log_footer_size;
- bool early;
-};
-
-int persistent_ram_early_init(struct persistent_ram *ram);
-
-struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
- size_t size,
- bool ecc);
-void persistent_ram_free(struct persistent_ram_zone *prz);
-struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
- bool ecc);
-
-int persistent_ram_write(struct persistent_ram_zone *prz, const void *s,
- unsigned int count);
-
-size_t persistent_ram_old_size(struct persistent_ram_zone *prz);
-void *persistent_ram_old(struct persistent_ram_zone *prz);
-void persistent_ram_free_old(struct persistent_ram_zone *prz);
-ssize_t persistent_ram_ecc_string(struct persistent_ram_zone *prz,
- char *str, size_t len);
-
-#endif
diff --git a/drivers/staging/android/ram_console.c b/drivers/staging/android/ram_console.c
index ce140ff..82323bb 100644
--- a/drivers/staging/android/ram_console.c
+++ b/drivers/staging/android/ram_console.c
@@ -21,7 +21,7 @@
#include <linux/string.h>
#include <linux/uaccess.h>
#include <linux/io.h>
-#include "persistent_ram.h"
+#include <linux/pstore_ram.h>
#include "ram_console.h"
static struct persistent_ram_zone *ram_console_zone;
diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index ad6e594..139a07c 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -14,9 +14,12 @@ config PSTORE
config PSTORE_RAM
tristate "Log panic/oops to a RAM buffer"
- depends on HAS_IOMEM
depends on PSTORE
- default n
+ depends on HAS_IOMEM
+ depends on HAVE_MEMBLOCK
+ select REED_SOLOMON
+ select REED_SOLOMON_ENC8
+ select REED_SOLOMON_DEC8
help
This enables panic and oops messages to be logged to a circular
buffer in RAM where it can be read back at some later point.
diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
index 804e376..76d5284 100644
--- a/fs/pstore/Makefile
+++ b/fs/pstore/Makefile
@@ -5,4 +5,4 @@
obj-y += pstore.o
pstore-objs += inode.o platform.o
-obj-$(CONFIG_PSTORE_RAM) += ram.o
+obj-$(CONFIG_PSTORE_RAM) += ram.o ram_core.o
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
new file mode 100644
index 0000000..7b9556ba
--- /dev/null
+++ b/fs/pstore/ram_core.c
@@ -0,0 +1,530 @@
+/*
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/list.h>
+#include <linux/memblock.h>
+#include <linux/rslib.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/pstore_ram.h>
+#include <asm/page.h>
+
+struct persistent_ram_buffer {
+ uint32_t sig;
+ atomic_t start;
+ atomic_t size;
+ uint8_t data[0];
+};
+
+#define PERSISTENT_RAM_SIG (0x43474244) /* DBGC */
+
+static __initdata LIST_HEAD(persistent_ram_list);
+
+static inline size_t buffer_size(struct persistent_ram_zone *prz)
+{
+ return atomic_read(&prz->buffer->size);
+}
+
+static inline size_t buffer_start(struct persistent_ram_zone *prz)
+{
+ return atomic_read(&prz->buffer->start);
+}
+
+/* increase and wrap the start pointer, returning the old value */
+static inline size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a)
+{
+ int old;
+ int new;
+
+ do {
+ old = atomic_read(&prz->buffer->start);
+ new = old + a;
+ while (unlikely(new > prz->buffer_size))
+ new -= prz->buffer_size;
+ } while (atomic_cmpxchg(&prz->buffer->start, old, new) != old);
+
+ return old;
+}
+
+/* increase the size counter until it hits the max size */
+static inline void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
+{
+ size_t old;
+ size_t new;
+
+ if (atomic_read(&prz->buffer->size) == prz->buffer_size)
+ return;
+
+ do {
+ old = atomic_read(&prz->buffer->size);
+ new = old + a;
+ if (new > prz->buffer_size)
+ new = prz->buffer_size;
+ } while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
+}
+
+static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz,
+ uint8_t *data, size_t len, uint8_t *ecc)
+{
+ int i;
+ uint16_t par[prz->ecc_size];
+
+ /* Initialize the parity buffer */
+ memset(par, 0, sizeof(par));
+ encode_rs8(prz->rs_decoder, data, len, par, 0);
+ for (i = 0; i < prz->ecc_size; i++)
+ ecc[i] = par[i];
+}
+
+static int persistent_ram_decode_rs8(struct persistent_ram_zone *prz,
+ void *data, size_t len, uint8_t *ecc)
+{
+ int i;
+ uint16_t par[prz->ecc_size];
+
+ for (i = 0; i < prz->ecc_size; i++)
+ par[i] = ecc[i];
+ return decode_rs8(prz->rs_decoder, data, par, len,
+ NULL, 0, NULL, 0, NULL);
+}
+
+static void notrace persistent_ram_update_ecc(struct persistent_ram_zone *prz,
+ unsigned int start, unsigned int count)
+{
+ struct persistent_ram_buffer *buffer = prz->buffer;
+ uint8_t *buffer_end = buffer->data + prz->buffer_size;
+ uint8_t *block;
+ uint8_t *par;
+ int ecc_block_size = prz->ecc_block_size;
+ int ecc_size = prz->ecc_size;
+ int size = prz->ecc_block_size;
+
+ if (!prz->ecc)
+ return;
+
+ block = buffer->data + (start & ~(ecc_block_size - 1));
+ par = prz->par_buffer + (start / ecc_block_size) * prz->ecc_size;
+
+ do {
+ if (block + ecc_block_size > buffer_end)
+ size = buffer_end - block;
+ persistent_ram_encode_rs8(prz, block, size, par);
+ block += ecc_block_size;
+ par += ecc_size;
+ } while (block < buffer->data + start + count);
+}
+
+static void persistent_ram_update_header_ecc(struct persistent_ram_zone *prz)
+{
+ struct persistent_ram_buffer *buffer = prz->buffer;
+
+ if (!prz->ecc)
+ return;
+
+ persistent_ram_encode_rs8(prz, (uint8_t *)buffer, sizeof(*buffer),
+ prz->par_header);
+}
+
+static void persistent_ram_ecc_old(struct persistent_ram_zone *prz)
+{
+ struct persistent_ram_buffer *buffer = prz->buffer;
+ uint8_t *block;
+ uint8_t *par;
+
+ if (!prz->ecc)
+ return;
+
+ block = buffer->data;
+ par = prz->par_buffer;
+ while (block < buffer->data + buffer_size(prz)) {
+ int numerr;
+ int size = prz->ecc_block_size;
+ if (block + size > buffer->data + prz->buffer_size)
+ size = buffer->data + prz->buffer_size - block;
+ numerr = persistent_ram_decode_rs8(prz, block, size, par);
+ if (numerr > 0) {
+ pr_devel("persistent_ram: error in block %p, %d\n",
+ block, numerr);
+ prz->corrected_bytes += numerr;
+ } else if (numerr < 0) {
+ pr_devel("persistent_ram: uncorrectable error in block %p\n",
+ block);
+ prz->bad_blocks++;
+ }
+ block += prz->ecc_block_size;
+ par += prz->ecc_size;
+ }
+}
+
+static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
+ size_t buffer_size)
+{
+ int numerr;
+ struct persistent_ram_buffer *buffer = prz->buffer;
+ int ecc_blocks;
+
+ if (!prz->ecc)
+ return 0;
+
+ prz->ecc_block_size = 128;
+ prz->ecc_size = 16;
+ prz->ecc_symsize = 8;
+ prz->ecc_poly = 0x11d;
+
+ ecc_blocks = DIV_ROUND_UP(prz->buffer_size, prz->ecc_block_size);
+ prz->buffer_size -= (ecc_blocks + 1) * prz->ecc_size;
+
+ if (prz->buffer_size > buffer_size) {
+ pr_err("persistent_ram: invalid size %zu, non-ecc datasize %zu\n",
+ buffer_size, prz->buffer_size);
+ return -EINVAL;
+ }
+
+ prz->par_buffer = buffer->data + prz->buffer_size;
+ prz->par_header = prz->par_buffer + ecc_blocks * prz->ecc_size;
+
+ /*
+ * first consecutive root is 0
+ * primitive element to generate roots = 1
+ */
+ prz->rs_decoder = init_rs(prz->ecc_symsize, prz->ecc_poly, 0, 1,
+ prz->ecc_size);
+ if (prz->rs_decoder == NULL) {
+ pr_info("persistent_ram: init_rs failed\n");
+ return -EINVAL;
+ }
+
+ prz->corrected_bytes = 0;
+ prz->bad_blocks = 0;
+
+ numerr = persistent_ram_decode_rs8(prz, buffer, sizeof(*buffer),
+ prz->par_header);
+ if (numerr > 0) {
+ pr_info("persistent_ram: error in header, %d\n", numerr);
+ prz->corrected_bytes += numerr;
+ } else if (numerr < 0) {
+ pr_info("persistent_ram: uncorrectable error in header\n");
+ prz->bad_blocks++;
+ }
+
+ return 0;
+}
+
+ssize_t persistent_ram_ecc_string(struct persistent_ram_zone *prz,
+ char *str, size_t len)
+{
+ ssize_t ret;
+
+ if (prz->corrected_bytes || prz->bad_blocks)
+ ret = snprintf(str, len, ""
+ "\n%d Corrected bytes, %d unrecoverable blocks\n",
+ prz->corrected_bytes, prz->bad_blocks);
+ else
+ ret = snprintf(str, len, "\nNo errors detected\n");
+
+ return ret;
+}
+
+static void notrace persistent_ram_update(struct persistent_ram_zone *prz,
+ const void *s, unsigned int start, unsigned int count)
+{
+ struct persistent_ram_buffer *buffer = prz->buffer;
+ memcpy(buffer->data + start, s, count);
+ persistent_ram_update_ecc(prz, start, count);
+}
+
+static void __init
+persistent_ram_save_old(struct persistent_ram_zone *prz)
+{
+ struct persistent_ram_buffer *buffer = prz->buffer;
+ size_t size = buffer_size(prz);
+ size_t start = buffer_start(prz);
+ char *dest;
+
+ persistent_ram_ecc_old(prz);
+
+ dest = kmalloc(size, GFP_KERNEL);
+ if (dest == NULL) {
+ pr_err("persistent_ram: failed to allocate buffer\n");
+ return;
+ }
+
+ prz->old_log = dest;
+ prz->old_log_size = size;
+ memcpy(prz->old_log, &buffer->data[start], size - start);
+ memcpy(prz->old_log + size - start, &buffer->data[0], start);
+}
+
+int notrace persistent_ram_write(struct persistent_ram_zone *prz,
+ const void *s, unsigned int count)
+{
+ int rem;
+ int c = count;
+ size_t start;
+
+ if (unlikely(c > prz->buffer_size)) {
+ s += c - prz->buffer_size;
+ c = prz->buffer_size;
+ }
+
+ buffer_size_add(prz, c);
+
+ start = buffer_start_add(prz, c);
+
+ rem = prz->buffer_size - start;
+ if (unlikely(rem < c)) {
+ persistent_ram_update(prz, s, start, rem);
+ s += rem;
+ c -= rem;
+ start = 0;
+ }
+ persistent_ram_update(prz, s, start, c);
+
+ persistent_ram_update_header_ecc(prz);
+
+ return count;
+}
+
+size_t persistent_ram_old_size(struct persistent_ram_zone *prz)
+{
+ return prz->old_log_size;
+}
+
+void *persistent_ram_old(struct persistent_ram_zone *prz)
+{
+ return prz->old_log;
+}
+
+void persistent_ram_free_old(struct persistent_ram_zone *prz)
+{
+ kfree(prz->old_log);
+ prz->old_log = NULL;
+ prz->old_log_size = 0;
+}
+
+static void *persistent_ram_vmap(phys_addr_t start, size_t size)
+{
+ struct page **pages;
+ phys_addr_t page_start;
+ unsigned int page_count;
+ pgprot_t prot;
+ unsigned int i;
+ void *vaddr;
+
+ page_start = start - offset_in_page(start);
+ page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE);
+
+ prot = pgprot_noncached(PAGE_KERNEL);
+
+ pages = kmalloc(sizeof(struct page *) * page_count, GFP_KERNEL);
+ if (!pages) {
+ pr_err("%s: Failed to allocate array for %u pages\n", __func__,
+ page_count);
+ return NULL;
+ }
+
+ for (i = 0; i < page_count; i++) {
+ phys_addr_t addr = page_start + i * PAGE_SIZE;
+ pages[i] = pfn_to_page(addr >> PAGE_SHIFT);
+ }
+ vaddr = vmap(pages, page_count, VM_MAP, prot);
+ kfree(pages);
+
+ return vaddr;
+}
+
+static void *persistent_ram_iomap(phys_addr_t start, size_t size)
+{
+ if (!request_mem_region(start, size, "persistent_ram")) {
+ pr_err("request mem region (0x%llx@0x%llx) failed\n",
+ (unsigned long long)size, (unsigned long long)start);
+ return NULL;
+ }
+
+ return ioremap(start, size);
+}
+
+static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
+ struct persistent_ram_zone *prz)
+{
+ prz->paddr = start;
+ prz->size = size;
+
+ if (pfn_valid(start >> PAGE_SHIFT))
+ prz->vaddr = persistent_ram_vmap(start, size);
+ else
+ prz->vaddr = persistent_ram_iomap(start, size);
+
+ if (!prz->vaddr) {
+ pr_err("%s: Failed to map 0x%llx pages at 0x%llx\n", __func__,
+ (unsigned long long)size, (unsigned long long)start);
+ return -ENOMEM;
+ }
+
+ prz->buffer = prz->vaddr + offset_in_page(start);
+ prz->buffer_size = size - sizeof(struct persistent_ram_buffer);
+
+ return 0;
+}
+
+static int __init persistent_ram_buffer_init(const char *name,
+ struct persistent_ram_zone *prz)
+{
+ int i;
+ struct persistent_ram *ram;
+ struct persistent_ram_descriptor *desc;
+ phys_addr_t start;
+
+ list_for_each_entry(ram, &persistent_ram_list, node) {
+ start = ram->start;
+ for (i = 0; i < ram->num_descs; i++) {
+ desc = &ram->descs[i];
+ if (!strcmp(desc->name, name))
+ return persistent_ram_buffer_map(start,
+ desc->size, prz);
+ start += desc->size;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool ecc)
+{
+ int ret;
+
+ prz->ecc = ecc;
+
+ ret = persistent_ram_init_ecc(prz, prz->buffer_size);
+ if (ret)
+ return ret;
+
+ if (prz->buffer->sig == PERSISTENT_RAM_SIG) {
+ if (buffer_size(prz) > prz->buffer_size ||
+ buffer_start(prz) > buffer_size(prz))
+ pr_info("persistent_ram: found existing invalid buffer,"
+ " size %zu, start %zu\n",
+ buffer_size(prz), buffer_start(prz));
+ else {
+ pr_info("persistent_ram: found existing buffer,"
+ " size %zu, start %zu\n",
+ buffer_size(prz), buffer_start(prz));
+ persistent_ram_save_old(prz);
+ }
+ } else {
+ pr_info("persistent_ram: no valid data in buffer"
+ " (sig = 0x%08x)\n", prz->buffer->sig);
+ }
+
+ prz->buffer->sig = PERSISTENT_RAM_SIG;
+ atomic_set(&prz->buffer->start, 0);
+ atomic_set(&prz->buffer->size, 0);
+
+ return 0;
+}
+
+void persistent_ram_free(struct persistent_ram_zone *prz)
+{
+ if (pfn_valid(prz->paddr >> PAGE_SHIFT)) {
+ vunmap(prz->vaddr);
+ } else {
+ iounmap(prz->vaddr);
+ release_mem_region(prz->paddr, prz->size);
+ }
+ persistent_ram_free_old(prz);
+ kfree(prz);
+}
+
+struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
+ size_t size,
+ bool ecc)
+{
+ struct persistent_ram_zone *prz;
+ int ret = -ENOMEM;
+
+ prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
+ if (!prz) {
+ pr_err("persistent_ram: failed to allocate persistent ram zone\n");
+ goto err;
+ }
+
+ ret = persistent_ram_buffer_map(start, size, prz);
+ if (ret)
+ goto err;
+
+ persistent_ram_post_init(prz, ecc);
+ persistent_ram_update_header_ecc(prz);
+
+ return prz;
+err:
+ kfree(prz);
+ return ERR_PTR(ret);
+}
+
+static __init
+struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
+{
+ struct persistent_ram_zone *prz;
+ int ret = -ENOMEM;
+
+ prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
+ if (!prz) {
+ pr_err("persistent_ram: failed to allocate persistent ram zone\n");
+ goto err;
+ }
+
+ ret = persistent_ram_buffer_init(dev_name(dev), prz);
+ if (ret) {
+ pr_err("persistent_ram: failed to initialize buffer\n");
+ goto err;
+ }
+
+ persistent_ram_post_init(prz, ecc);
+
+ return prz;
+err:
+ kfree(prz);
+ return ERR_PTR(ret);
+}
+
+struct persistent_ram_zone * __init
+persistent_ram_init_ringbuffer(struct device *dev, bool ecc)
+{
+ return __persistent_ram_init(dev, ecc);
+}
+
+int __init persistent_ram_early_init(struct persistent_ram *ram)
+{
+ int ret;
+
+ ret = memblock_reserve(ram->start, ram->size);
+ if (ret) {
+ pr_err("Failed to reserve persistent memory from %08lx-%08lx\n",
+ (long)ram->start, (long)(ram->start + ram->size - 1));
+ return ret;
+ }
+
+ list_add_tail(&ram->node, &persistent_ram_list);
+
+ pr_info("Initialized persistent memory from %08lx-%08lx\n",
+ (long)ram->start, (long)(ram->start + ram->size - 1));
+
+ return 0;
+}
diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
index 484fef8..fa4d6e3 100644
--- a/include/linux/pstore_ram.h
+++ b/include/linux/pstore_ram.h
@@ -1,5 +1,87 @@
-#ifndef __RAMOOPS_H
-#define __RAMOOPS_H
+/*
+ * Copyright (C) 2010 Marco Stornelli <[email protected]>
+ * Copyright (C) 2011 Kees Cook <[email protected]>
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __LINUX_PSTORE_RAM_H__
+#define __LINUX_PSTORE_RAM_H__
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/types.h>
+#include <linux/init.h>
+
+struct persistent_ram_buffer;
+
+struct persistent_ram_descriptor {
+ const char *name;
+ phys_addr_t size;
+};
+
+struct persistent_ram {
+ phys_addr_t start;
+ phys_addr_t size;
+
+ int num_descs;
+ struct persistent_ram_descriptor *descs;
+
+ struct list_head node;
+};
+
+struct persistent_ram_zone {
+ phys_addr_t paddr;
+ size_t size;
+ void *vaddr;
+ struct persistent_ram_buffer *buffer;
+ size_t buffer_size;
+
+ /* ECC correction */
+ bool ecc;
+ char *par_buffer;
+ char *par_header;
+ struct rs_control *rs_decoder;
+ int corrected_bytes;
+ int bad_blocks;
+ int ecc_block_size;
+ int ecc_size;
+ int ecc_symsize;
+ int ecc_poly;
+
+ char *old_log;
+ size_t old_log_size;
+ size_t old_log_footer_size;
+ bool early;
+};
+
+int persistent_ram_early_init(struct persistent_ram *ram);
+
+struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
+ size_t size,
+ bool ecc);
+void persistent_ram_free(struct persistent_ram_zone *prz);
+struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
+ bool ecc);
+
+int persistent_ram_write(struct persistent_ram_zone *prz, const void *s,
+ unsigned int count);
+
+size_t persistent_ram_old_size(struct persistent_ram_zone *prz);
+void *persistent_ram_old(struct persistent_ram_zone *prz);
+void persistent_ram_free_old(struct persistent_ram_zone *prz);
+ssize_t persistent_ram_ecc_string(struct persistent_ram_zone *prz,
+ char *str, size_t len);
/*
* Ramoops platform data
--
1.7.9.2
The patch switches pstore RAM backend to use persistent_ram routines,
one step closer to the ECC support.
Signed-off-by: Anton Vorontsov <[email protected]>
---
fs/pstore/ram.c | 109 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 60 insertions(+), 49 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index b26b58e..cf0ad92 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -62,7 +62,7 @@ MODULE_PARM_DESC(dump_oops,
"set to 1 to dump oopses, 0 to only dump panics (default 1)");
struct ramoops_context {
- void *virt_addr;
+ struct persistent_ram_zone **przs;
phys_addr_t phys_addr;
unsigned long size;
size_t record_size;
@@ -90,39 +90,56 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
struct pstore_info *psi)
{
ssize_t size;
- char *rambuf;
struct ramoops_context *cxt = psi->data;
+ struct persistent_ram_zone *prz;
if (cxt->read_count >= cxt->max_count)
return -EINVAL;
+
*id = cxt->read_count++;
+ prz = cxt->przs[*id];
+
/* Only supports dmesg output so far. */
*type = PSTORE_TYPE_DMESG;
/* TODO(kees): Bogus time for the moment. */
time->tv_sec = 0;
time->tv_nsec = 0;
- rambuf = cxt->virt_addr + (*id * cxt->record_size);
- size = strnlen(rambuf, cxt->record_size);
+ size = persistent_ram_old_size(prz);
*buf = kmalloc(size, GFP_KERNEL);
if (*buf == NULL)
return -ENOMEM;
- memcpy(*buf, rambuf, size);
+ memcpy(*buf, persistent_ram_old(prz), size);
return size;
}
+static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz)
+{
+ char *hdr;
+ struct timeval timestamp;
+ size_t len;
+
+ do_gettimeofday(×tamp);
+ hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu\n",
+ (long)timestamp.tv_sec, (long)timestamp.tv_usec);
+ WARN_ON_ONCE(!hdr);
+ len = hdr ? strlen(hdr) : 0;
+ persistent_ram_write(prz, hdr, len);
+ kfree(hdr);
+
+ return len;
+}
+
static int ramoops_pstore_write(enum pstore_type_id type,
enum kmsg_dump_reason reason,
u64 *id,
unsigned int part,
size_t size, struct pstore_info *psi)
{
- char *buf;
- size_t res;
- struct timeval timestamp;
struct ramoops_context *cxt = psi->data;
- size_t available = cxt->record_size;
+ struct persistent_ram_zone *prz = cxt->przs[cxt->count];
+ size_t hlen;
/* Currently ramoops is designed to only store dmesg dumps. */
if (type != PSTORE_TYPE_DMESG)
@@ -147,22 +164,10 @@ static int ramoops_pstore_write(enum pstore_type_id type,
if (part != 1)
return -ENOSPC;
- buf = cxt->virt_addr + (cxt->count * cxt->record_size);
-
- res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
- buf += res;
- available -= res;
-
- do_gettimeofday(×tamp);
- res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
- buf += res;
- available -= res;
-
- if (size > available)
- size = available;
-
- memcpy(buf, cxt->pstore.buf, size);
- memset(buf + size, '\0', available - size);
+ hlen = ramoops_write_kmsg_hdr(prz);
+ if (size + hlen > prz->buffer_size)
+ size = prz->buffer_size - hlen;
+ persistent_ram_write(prz, cxt->pstore.buf, size);
cxt->count = (cxt->count + 1) % cxt->max_count;
@@ -172,14 +177,12 @@ static int ramoops_pstore_write(enum pstore_type_id type,
static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
struct pstore_info *psi)
{
- char *buf;
struct ramoops_context *cxt = psi->data;
if (id >= cxt->max_count)
return -EINVAL;
- buf = cxt->virt_addr + (id * cxt->record_size);
- memset(buf, '\0', cxt->record_size);
+ persistent_ram_free_old(cxt->przs[id]);
return 0;
}
@@ -200,6 +203,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
struct ramoops_platform_data *pdata = pdev->dev.platform_data;
struct ramoops_context *cxt = &oops_cxt;
int err = -EINVAL;
+ int i;
/* Only a single ramoops area allowed at a time, so fail extra
* probes.
@@ -237,32 +241,37 @@ static int __init ramoops_probe(struct platform_device *pdev)
cxt->record_size = pdata->record_size;
cxt->dump_oops = pdata->dump_oops;
+ cxt->przs = kzalloc(sizeof(*cxt->przs) * cxt->max_count, GFP_KERNEL);
+ if (!cxt->przs) {
+ pr_err("failed to initialize a prz array\n");
+ goto fail_przs;
+ }
+
+ for (i = 0; i < cxt->max_count; i++) {
+ size_t sz = cxt->record_size;
+ phys_addr_t start = cxt->phys_addr + sz * i;
+
+ cxt->przs[i] = persistent_ram_new(start, sz, 0);
+ if (IS_ERR(cxt->przs[i])) {
+ err = PTR_ERR(cxt->przs[i]);
+ pr_err("failed to initialize a prz\n");
+ goto fail_prz;
+ }
+ }
+
cxt->pstore.data = cxt;
- cxt->pstore.bufsize = cxt->record_size;
- cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
spin_lock_init(&cxt->pstore.buf_lock);
+ cxt->pstore.bufsize = cxt->przs[0]->buffer_size;
+ cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
if (!cxt->pstore.buf) {
pr_err("cannot allocate pstore buffer\n");
goto fail_clear;
}
- if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
- pr_err("request mem region (0x%lx@0x%llx) failed\n",
- cxt->size, (unsigned long long)cxt->phys_addr);
- err = -EINVAL;
- goto fail_buf;
- }
-
- cxt->virt_addr = ioremap(cxt->phys_addr, cxt->size);
- if (!cxt->virt_addr) {
- pr_err("ioremap failed\n");
- goto fail_mem_region;
- }
-
err = pstore_register(&cxt->pstore);
if (err) {
pr_err("registering with pstore failed\n");
- goto fail_iounmap;
+ goto fail_pstore;
}
/*
@@ -280,15 +289,17 @@ static int __init ramoops_probe(struct platform_device *pdev)
return 0;
-fail_iounmap:
- iounmap(cxt->virt_addr);
-fail_mem_region:
- release_mem_region(cxt->phys_addr, cxt->size);
-fail_buf:
+fail_pstore:
kfree(cxt->pstore.buf);
fail_clear:
cxt->pstore.bufsize = 0;
cxt->max_count = 0;
+fail_przs:
+ for (i = 0; cxt->przs[i]; i++)
+ persistent_ram_free(cxt->przs[i]);
+ kfree(cxt->przs);
+fail_prz:
+ kfree(cxt->pstore.buf);
fail_out:
return err;
}
--
1.7.9.2
This is now straightforward: just introduce a module parameter and pass
the needed value to persistent_ram_new().
Signed-off-by: Anton Vorontsov <[email protected]>
---
fs/pstore/ram.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index cf0ad92..eeb4e32 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -61,6 +61,11 @@ module_param(dump_oops, int, 0600);
MODULE_PARM_DESC(dump_oops,
"set to 1 to dump oopses, 0 to only dump panics (default 1)");
+static int ramoops_ecc;
+module_param_named(ecc, ramoops_ecc, int, 0600);
+MODULE_PARM_DESC(ramoops_ecc,
+ "set to 1 to enable ECC support");
+
struct ramoops_context {
struct persistent_ram_zone **przs;
phys_addr_t phys_addr;
@@ -251,7 +256,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
size_t sz = cxt->record_size;
phys_addr_t start = cxt->phys_addr + sz * i;
- cxt->przs[i] = persistent_ram_new(start, sz, 0);
+ cxt->przs[i] = persistent_ram_new(start, sz, ramoops_ecc);
if (IS_ERR(cxt->przs[i])) {
err = PTR_ERR(cxt->przs[i]);
pr_err("failed to initialize a prz\n");
@@ -283,9 +288,10 @@ static int __init ramoops_probe(struct platform_device *pdev)
record_size = pdata->record_size;
dump_oops = pdata->dump_oops;
- pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
+ pr_info("attached 0x%lx@0x%llx (%ux0x%zx), ecc: %s\n",
cxt->size, (unsigned long long)cxt->phys_addr,
- cxt->max_count, cxt->record_size);
+ cxt->max_count, cxt->record_size,
+ ramoops_ecc ? "on" : "off");
return 0;
--
1.7.9.2
On Fri, May 11, 2012 at 05:17:17PM -0700, Anton Vorontsov wrote:
> This is a longstanding bug, almost unnoticeable when calling
> persistent_ram_write() for small buffers.
>
> But when called for large data buffers, the write routine behaves
> incorrectly, as the size may never update: instead of clamping
> the size to the maximum buffer size, buffer_size_add_clamp() returns
> an error (which is never checked by the write routine, btw).
>
> To fix this, we now use buffer_size_add() that actually clamps the
> size to the max value.
>
> Also remove buffer_size_add_clamp(), it is no longer needed.
>
Say if you did notice it, what would that look like? It's just that
something gets lost instead of written to the screen right?
regards,
dan carpenter
On Sun, May 13, 2012 at 07:56:01PM +0300, Dan Carpenter wrote:
> On Fri, May 11, 2012 at 05:17:17PM -0700, Anton Vorontsov wrote:
> > This is a longstanding bug, almost unnoticeable when calling
> > persistent_ram_write() for small buffers.
> >
> > But when called for large data buffers, the write routine behaves
> > incorrectly, as the size may never update: instead of clamping
> > the size to the maximum buffer size, buffer_size_add_clamp() returns
> > an error (which is never checked by the write routine, btw).
> >
> > To fix this, we now use buffer_size_add() that actually clamps the
> > size to the max value.
> >
> > Also remove buffer_size_add_clamp(), it is no longer needed.
> >
>
> Say if you did notice it, what would that look like? It's just that
> something gets lost instead of written to the screen right?
Yep. Suppose the ring buffer size is 4096 bytes, when somebody tries to
write a data in a 2000 bytes chunk, the first write will succeed (buffer
size will be 2000), but the second now 3000-bytes write will left the
size equal to 2000, instead of clamping it to 4096.
When we had a large buffer but a small writes (e.g. ram_console usage
scenario), this is almost unnoticeable. But when we started using large
writes the bug showed up.
Thanks,
--
Anton Vorontsov
Email: [email protected]
On Fri, May 11, 2012 at 5:17 PM, Anton Vorontsov
<[email protected]> wrote:
> This is a longstanding bug, almost unnoticeable when calling
> persistent_ram_write() for small buffers.
>
> But when called for large data buffers, the write routine behaves
> incorrectly, as the size may never update: instead of clamping
> the size to the maximum buffer size, buffer_size_add_clamp() returns
> an error (which is never checked by the write routine, btw).
>
> To fix this, we now use buffer_size_add() that actually clamps the
> size to the max value.
>
> Also remove buffer_size_add_clamp(), it is no longer needed.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> ?drivers/staging/android/persistent_ram.c | ? 19 +------------------
> ?1 file changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
> index 12444fd..13a12bc 100644
> --- a/drivers/staging/android/persistent_ram.c
> +++ b/drivers/staging/android/persistent_ram.c
> @@ -79,23 +79,6 @@ static inline void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
> ? ? ? ?} while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
> ?}
>
> -/* increase the size counter, retuning an error if it hits the max size */
> -static inline ssize_t buffer_size_add_clamp(struct persistent_ram_zone *prz,
> - ? ? ? size_t a)
> -{
> - ? ? ? size_t old;
> - ? ? ? size_t new;
> -
> - ? ? ? do {
> - ? ? ? ? ? ? ? old = atomic_read(&prz->buffer->size);
> - ? ? ? ? ? ? ? new = old + a;
> - ? ? ? ? ? ? ? if (new > prz->buffer_size)
> - ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> - ? ? ? } while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
> -
> - ? ? ? return 0;
> -}
> -
> ?static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz,
> ? ? ? ?uint8_t *data, size_t len, uint8_t *ecc)
> ?{
> @@ -300,7 +283,7 @@ int notrace persistent_ram_write(struct persistent_ram_zone *prz,
> ? ? ? ? ? ? ? ?c = prz->buffer_size;
> ? ? ? ?}
>
> - ? ? ? buffer_size_add_clamp(prz, c);
> + ? ? ? buffer_size_add(prz, c);
>
> ? ? ? ?start = buffer_start_add(prz, c);
>
> --
> 1.7.9.2
>
Acked-by: Colin Cross <[email protected]>
This is a bug fix for a bug introduced in 3.4-rc1, it should go into
3.4 if there is another rc.
On Sun, May 13, 2012 at 08:23:58PM -0700, Colin Cross wrote:
> On Fri, May 11, 2012 at 5:17 PM, Anton Vorontsov
> <[email protected]> wrote:
> > This is a longstanding bug, almost unnoticeable when calling
> > persistent_ram_write() for small buffers.
> >
> > But when called for large data buffers, the write routine behaves
> > incorrectly, as the size may never update: instead of clamping
> > the size to the maximum buffer size, buffer_size_add_clamp() returns
> > an error (which is never checked by the write routine, btw).
> >
> > To fix this, we now use buffer_size_add() that actually clamps the
> > size to the max value.
> >
> > Also remove buffer_size_add_clamp(), it is no longer needed.
> >
> > Signed-off-by: Anton Vorontsov <[email protected]>
> > ---
> > ?drivers/staging/android/persistent_ram.c | ? 19 +------------------
> > ?1 file changed, 1 insertion(+), 18 deletions(-)
> >
> > diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
> > index 12444fd..13a12bc 100644
> > --- a/drivers/staging/android/persistent_ram.c
> > +++ b/drivers/staging/android/persistent_ram.c
> > @@ -79,23 +79,6 @@ static inline void buffer_size_add(struct persistent_ram_zone *prz, size_t a)
> > ? ? ? ?} while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
> > ?}
> >
> > -/* increase the size counter, retuning an error if it hits the max size */
> > -static inline ssize_t buffer_size_add_clamp(struct persistent_ram_zone *prz,
> > - ? ? ? size_t a)
> > -{
> > - ? ? ? size_t old;
> > - ? ? ? size_t new;
> > -
> > - ? ? ? do {
> > - ? ? ? ? ? ? ? old = atomic_read(&prz->buffer->size);
> > - ? ? ? ? ? ? ? new = old + a;
> > - ? ? ? ? ? ? ? if (new > prz->buffer_size)
> > - ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> > - ? ? ? } while (atomic_cmpxchg(&prz->buffer->size, old, new) != old);
> > -
> > - ? ? ? return 0;
> > -}
> > -
> > ?static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz,
> > ? ? ? ?uint8_t *data, size_t len, uint8_t *ecc)
> > ?{
> > @@ -300,7 +283,7 @@ int notrace persistent_ram_write(struct persistent_ram_zone *prz,
> > ? ? ? ? ? ? ? ?c = prz->buffer_size;
> > ? ? ? ?}
> >
> > - ? ? ? buffer_size_add_clamp(prz, c);
> > + ? ? ? buffer_size_add(prz, c);
> >
> > ? ? ? ?start = buffer_start_add(prz, c);
> >
> > --
> > 1.7.9.2
> >
>
> Acked-by: Colin Cross <[email protected]>
>
> This is a bug fix for a bug introduced in 3.4-rc1, it should go into
> 3.4 if there is another rc.
I'll queue it up for 3.4.1.
thanks,
greg k-h
On Fri, May 11, 2012 at 05:15:06PM -0700, Anton Vorontsov wrote:
> Hi all,
>
> There are currently two competing debug facilities to store kernel
> messages in a persistent storage: a generic pstore and Google's
> persistent_ram. Not so long ago (https://lkml.org/lkml/2012/3/8/252),
> it was decided that we should fix this situation.
>
> Recently ramoops has switched to pstore, which basically means that
> it became a RAM backend for the pstore framework.
>
> persistent_ram+ram_console and ramoops+pstore have almost the same
> features, except:
>
> 1. Ramoops doesn't support ECC. Having ECC is useful when a hardware
> reset was used to bring the machine back to life (i.e. a watchdog
> triggered). In such cases, RAM may be somewhat corrupt, but
> usually it is restorable.
>
> 2. 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.
>
> These patches solve the first issue, plus move things to their
> proper places. Patches that will fix the second issue are pending.
I've applied the first 7 patches, as they were localized to the
drivers/staging/android/ directory, but in order for me to apply the
rest, I need acks from the respective subsystem maintainers.
pstore developers, what do you say about these changes, are you ok with
them?
thanks,
greg k-h
On Mon, 2012-05-14 at 08:58 -0700, Greg Kroah-Hartman wrote:
> On Fri, May 11, 2012 at 05:15:06PM -0700, Anton Vorontsov wrote:
> > Hi all,
> >
> > There are currently two competing debug facilities to store kernel
> > messages in a persistent storage: a generic pstore and Google's
> > persistent_ram. Not so long ago (https://lkml.org/lkml/2012/3/8/252),
> > it was decided that we should fix this situation.
> >
> > Recently ramoops has switched to pstore, which basically means that
> > it became a RAM backend for the pstore framework.
> >
> > persistent_ram+ram_console and ramoops+pstore have almost the same
> > features, except:
> >
> > 1. Ramoops doesn't support ECC. Having ECC is useful when a hardware
> > reset was used to bring the machine back to life (i.e. a watchdog
> > triggered). In such cases, RAM may be somewhat corrupt, but
> > usually it is restorable.
> >
> > 2. 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.
> >
> > These patches solve the first issue, plus move things to their
> > proper places. Patches that will fix the second issue are pending.
>
> I've applied the first 7 patches, as they were localized to the
> drivers/staging/android/ directory, but in order for me to apply the
> rest, I need acks from the respective subsystem maintainers.
>
> pstore developers, what do you say about these changes, are you ok with
> them?
Good to see this work get done. Anton beat me to it. :) I have been
talking to pstore developers (Tony Luck) and ramoops maintainers (Kees
Cook) about this re-architecture work since I first floated this idea on
ce-android mailing list. I have been working on this rec-architecture
focusing on the second feature "Pstore doesn't support logging kernel
messages in run-time" and didn't get to ECC even though it is on my
feature list to do bring ramconsole features into ramoops.
Anton! Is it safe to assume you are planning to cover the second feature
as well, in which case I can drop my plans to get this work done.
-- Shuah
>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Mon, May 14, 2012 at 10:30:22AM -0600, Shuah Khan wrote:
[...]
> Anton! Is it safe to assume you are planning to cover the second feature
> as well, in which case I can drop my plans to get this work done.
Yep, absolutely. I'm fully committed to add runtime logging support
to pstore. Actually, I'll post patches pretty soon.
Thanks!
--
Anton Vorontsov
Email: [email protected]
On Mon, 2012-05-14 at 13:45 -0700, Anton Vorontsov wrote:
> On Mon, May 14, 2012 at 10:30:22AM -0600, Shuah Khan wrote:
> [...]
> > Anton! Is it safe to assume you are planning to cover the second feature
> > as well, in which case I can drop my plans to get this work done.
>
> Yep, absolutely. I'm fully committed to add runtime logging support
> to pstore. Actually, I'll post patches pretty soon.
>
> Thanks!
Cool. Do you need any testing help? I have an interest in seeing this
work on architectures and platforms I play with.
-- Shuah
On Fri, May 11, 2012 at 5:18 PM, Anton Vorontsov
<[email protected]> wrote:
> Since ramoops was converted to pstore, it has nothing to do with character
> devices nowadays. Instead, today it is just a RAM backend for pstore.
>
> The patch just moves things around. There are a few changes were needed
> because of the move:
>
> 1. Kconfig and Makefiles fixups, of course.
>
> 2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
> ? is needed to keep user experience the same as with ramoops driver
> ? (i.e. so that ramoops.foo kernel command line arguments would still
> ? work).
>
> Signed-off-by: Anton Vorontsov <[email protected]>
This consolidation seems good. I might prefer the move separated from
the changes, just to make review easier, but I have no idea what
that'll do to a bisect. :P
> --- /dev/null
> +++ b/fs/pstore/ram.c
"ram.ko" seems like an awfully generic modbule name. Should this be
called pstore_ram.* instead, like was done for the header file?
And unless anyone objects, I have no problem letting the built-in name
change too.
> --- /dev/null
> +++ b/include/linux/pstore_ram.h
> @@ -0,0 +1,17 @@
> +#ifndef __RAMOOPS_H
> +#define __RAMOOPS_H
This define should probably change just to avoid confusion.
-Kees
--
Kees Cook
Chrome OS Security
On Fri, May 11, 2012 at 5:18 PM, Anton Vorontsov
<[email protected]> wrote:
> This is a first step for adding ECC support for pstore RAM backend: we
> will use the persistent_ram routines, kindly provided by Google.
>
> Basically, persistent_ram is a set of helper routines to deal with the
> [optionally] ECC-protected persistent ram regions.
>
> A bit of Makefile, Kconfig and header files adjustments were needed
> because of the move.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kees Cook <[email protected]>
> ?include/linux/pstore_ram.h ? ? ? ? ? ? ? | ? 86 ++++-
This change includes the fix I suggested in the prior email, so yay. :)
-Kees
--
Kees Cook
Chrome OS Security
On Fri, May 11, 2012 at 5:18 PM, Anton Vorontsov
<[email protected]> wrote:
> The patch switches pstore RAM backend to use persistent_ram routines,
> one step closer to the ECC support.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
As mentioned, I'm all for this consolidation. That said, some notes below...
> ---
> ?fs/pstore/ram.c | ?109 ++++++++++++++++++++++++++++++-------------------------
> ?1 file changed, 60 insertions(+), 49 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index b26b58e..cf0ad92 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -62,7 +62,7 @@ MODULE_PARM_DESC(dump_oops,
> ? ? ? ? ? ? ? ?"set to 1 to dump oopses, 0 to only dump panics (default 1)");
>
> ?struct ramoops_context {
> - ? ? ? void *virt_addr;
> + ? ? ? struct persistent_ram_zone **przs;
> ? ? ? ?phys_addr_t phys_addr;
> ? ? ? ?unsigned long size;
> ? ? ? ?size_t record_size;
> @@ -90,39 +90,56 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct pstore_info *psi)
> ?{
> ? ? ? ?ssize_t size;
> - ? ? ? char *rambuf;
> ? ? ? ?struct ramoops_context *cxt = psi->data;
> + ? ? ? struct persistent_ram_zone *prz;
>
> ? ? ? ?if (cxt->read_count >= cxt->max_count)
> ? ? ? ? ? ? ? ?return -EINVAL;
> +
> ? ? ? ?*id = cxt->read_count++;
> + ? ? ? prz = cxt->przs[*id];
> +
> ? ? ? ?/* Only supports dmesg output so far. */
> ? ? ? ?*type = PSTORE_TYPE_DMESG;
> ? ? ? ?/* TODO(kees): Bogus time for the moment. */
> ? ? ? ?time->tv_sec = 0;
> ? ? ? ?time->tv_nsec = 0;
>
> - ? ? ? rambuf = cxt->virt_addr + (*id * cxt->record_size);
> - ? ? ? size = strnlen(rambuf, cxt->record_size);
> + ? ? ? size = persistent_ram_old_size(prz);
> ? ? ? ?*buf = kmalloc(size, GFP_KERNEL);
> ? ? ? ?if (*buf == NULL)
> ? ? ? ? ? ? ? ?return -ENOMEM;
> - ? ? ? memcpy(*buf, rambuf, size);
> + ? ? ? memcpy(*buf, persistent_ram_old(prz), size);
>
> ? ? ? ?return size;
> ?}
>
> +static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz)
> +{
> + ? ? ? char *hdr;
> + ? ? ? struct timeval timestamp;
> + ? ? ? size_t len;
> +
> + ? ? ? do_gettimeofday(×tamp);
> + ? ? ? hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu\n",
> + ? ? ? ? ? ? ? (long)timestamp.tv_sec, (long)timestamp.tv_usec);
> + ? ? ? WARN_ON_ONCE(!hdr);
> + ? ? ? len = hdr ? strlen(hdr) : 0;
> + ? ? ? persistent_ram_write(prz, hdr, len);
> + ? ? ? kfree(hdr);
> +
> + ? ? ? return len;
> +}
> +
> ?static int ramoops_pstore_write(enum pstore_type_id type,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?enum kmsg_dump_reason reason,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?u64 *id,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int part,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t size, struct pstore_info *psi)
> ?{
> - ? ? ? char *buf;
> - ? ? ? size_t res;
> - ? ? ? struct timeval timestamp;
> ? ? ? ?struct ramoops_context *cxt = psi->data;
> - ? ? ? size_t available = cxt->record_size;
> + ? ? ? struct persistent_ram_zone *prz = cxt->przs[cxt->count];
> + ? ? ? size_t hlen;
>
> ? ? ? ?/* Currently ramoops is designed to only store dmesg dumps. */
> ? ? ? ?if (type != PSTORE_TYPE_DMESG)
> @@ -147,22 +164,10 @@ static int ramoops_pstore_write(enum pstore_type_id type,
> ? ? ? ?if (part != 1)
> ? ? ? ? ? ? ? ?return -ENOSPC;
>
> - ? ? ? buf = cxt->virt_addr + (cxt->count * cxt->record_size);
> -
> - ? ? ? res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
> - ? ? ? buf += res;
> - ? ? ? available -= res;
> -
> - ? ? ? do_gettimeofday(×tamp);
> - ? ? ? res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
> - ? ? ? buf += res;
> - ? ? ? available -= res;
> -
> - ? ? ? if (size > available)
> - ? ? ? ? ? ? ? size = available;
> -
> - ? ? ? memcpy(buf, cxt->pstore.buf, size);
> - ? ? ? memset(buf + size, '\0', available - size);
> + ? ? ? hlen = ramoops_write_kmsg_hdr(prz);
> + ? ? ? if (size + hlen > prz->buffer_size)
> + ? ? ? ? ? ? ? size = prz->buffer_size - hlen;
> + ? ? ? persistent_ram_write(prz, cxt->pstore.buf, size);
>
> ? ? ? ?cxt->count = (cxt->count + 1) % cxt->max_count;
>
> @@ -172,14 +177,12 @@ static int ramoops_pstore_write(enum pstore_type_id type,
> ?static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct pstore_info *psi)
> ?{
> - ? ? ? char *buf;
> ? ? ? ?struct ramoops_context *cxt = psi->data;
>
> ? ? ? ?if (id >= cxt->max_count)
> ? ? ? ? ? ? ? ?return -EINVAL;
>
> - ? ? ? buf = cxt->virt_addr + (id * cxt->record_size);
> - ? ? ? memset(buf, '\0', cxt->record_size);
> + ? ? ? persistent_ram_free_old(cxt->przs[id]);
Hm, I don't think persistent_ram_free_old() is what's wanted here.
That appears to entirely release the region? I want to make sure the
memory is cleared first. And will this area come back on a write, or
does it stay released?
>
> ? ? ? ?return 0;
> ?}
> @@ -200,6 +203,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
> ? ? ? ?struct ramoops_platform_data *pdata = pdev->dev.platform_data;
> ? ? ? ?struct ramoops_context *cxt = &oops_cxt;
> ? ? ? ?int err = -EINVAL;
> + ? ? ? int i;
>
> ? ? ? ?/* Only a single ramoops area allowed at a time, so fail extra
> ? ? ? ? * probes.
> @@ -237,32 +241,37 @@ static int __init ramoops_probe(struct platform_device *pdev)
> ? ? ? ?cxt->record_size = pdata->record_size;
> ? ? ? ?cxt->dump_oops = pdata->dump_oops;
>
> + ? ? ? cxt->przs = kzalloc(sizeof(*cxt->przs) * cxt->max_count, GFP_KERNEL);
> + ? ? ? if (!cxt->przs) {
> + ? ? ? ? ? ? ? pr_err("failed to initialize a prz array\n");
> + ? ? ? ? ? ? ? goto fail_przs;
This should be fail_out.
> + ? ? ? }
> +
> + ? ? ? for (i = 0; i < cxt->max_count; i++) {
> + ? ? ? ? ? ? ? size_t sz = cxt->record_size;
> + ? ? ? ? ? ? ? phys_addr_t start = cxt->phys_addr + sz * i;
> +
> + ? ? ? ? ? ? ? cxt->przs[i] = persistent_ram_new(start, sz, 0);
persistent_ram_new() is marked as __init, so this is unsafe to call if
built as a module. I think persistent_ram_new() will need to lose the
__init marking, or I'm misunderstanding something.
> + ? ? ? ? ? ? ? if (IS_ERR(cxt->przs[i])) {
> + ? ? ? ? ? ? ? ? ? ? ? err = PTR_ERR(cxt->przs[i]);
> + ? ? ? ? ? ? ? ? ? ? ? pr_err("failed to initialize a prz\n");
Since neither persistent_ram_new() nor persistent_ram_buffer_map()
report the location of the failure, I'd like to keep the error report
(removed below "pr_err("request mem region (0x%lx@0x%llx)
failed\n",...") for failures, so there is something actionable in
dmesg when the platform data is mismatched for the hardware.
> + ? ? ? ? ? ? ? ? ? ? ? goto fail_prz;
This should be fail_przs.
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +
> ? ? ? ?cxt->pstore.data = cxt;
> - ? ? ? cxt->pstore.bufsize = cxt->record_size;
> - ? ? ? cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
> ? ? ? ?spin_lock_init(&cxt->pstore.buf_lock);
> + ? ? ? cxt->pstore.bufsize = cxt->przs[0]->buffer_size;
> + ? ? ? cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
I don't see a reason to re-order these (nothing can use buf yet
because we haven't registered it with pstore yet).
> ? ? ? ?if (!cxt->pstore.buf) {
> ? ? ? ? ? ? ? ?pr_err("cannot allocate pstore buffer\n");
> ? ? ? ? ? ? ? ?goto fail_clear;
> ? ? ? ?}
>
> - ? ? ? if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
> - ? ? ? ? ? ? ? pr_err("request mem region (0x%lx@0x%llx) failed\n",
> - ? ? ? ? ? ? ? ? ? ? ? cxt->size, (unsigned long long)cxt->phys_addr);
> - ? ? ? ? ? ? ? err = -EINVAL;
> - ? ? ? ? ? ? ? goto fail_buf;
> - ? ? ? }
> -
> - ? ? ? cxt->virt_addr = ioremap(cxt->phys_addr, ?cxt->size);
> - ? ? ? if (!cxt->virt_addr) {
> - ? ? ? ? ? ? ? pr_err("ioremap failed\n");
> - ? ? ? ? ? ? ? goto fail_mem_region;
> - ? ? ? }
> -
> ? ? ? ?err = pstore_register(&cxt->pstore);
> ? ? ? ?if (err) {
> ? ? ? ? ? ? ? ?pr_err("registering with pstore failed\n");
> - ? ? ? ? ? ? ? goto fail_iounmap;
> + ? ? ? ? ? ? ? goto fail_pstore;
This should be fail_buf.
> ? ? ? ?}
>
> ? ? ? ?/*
> @@ -280,15 +289,17 @@ static int __init ramoops_probe(struct platform_device *pdev)
>
> ? ? ? ?return 0;
>
> -fail_iounmap:
> - ? ? ? iounmap(cxt->virt_addr);
> -fail_mem_region:
> - ? ? ? release_mem_region(cxt->phys_addr, cxt->size);
> -fail_buf:
> +fail_pstore:
No reason to rename this from "fail_buf".
> ? ? ? ?kfree(cxt->pstore.buf);
> ?fail_clear:
> ? ? ? ?cxt->pstore.bufsize = 0;
> ? ? ? ?cxt->max_count = 0;
> +fail_przs:
> + ? ? ? for (i = 0; cxt->przs[i]; i++)
> + ? ? ? ? ? ? ? persistent_ram_free(cxt->przs[i]);
This can lead to a BUG, since persistent_ram_free() doesn't handle
NULL arguments.
> + ? ? ? kfree(cxt->przs);
> +fail_prz:
> + ? ? ? kfree(cxt->pstore.buf);
This target (fail_prz) should be removed, and the kfree is redundant
to fail_buf above.
> ?fail_out:
> ? ? ? ?return err;
> ?}
> --
> 1.7.9.2
>
-Kees
--
Kees Cook
Chrome OS Security
On Fri, May 11, 2012 at 5:18 PM, Anton Vorontsov
<[email protected]> wrote:
> This is now straightforward: just introduce a module parameter and pass
> the needed value to persistent_ram_new().
>
> Signed-off-by: Anton Vorontsov <[email protected]>
Acked-by: Kees Cook <[email protected]>
> ---
> ?fs/pstore/ram.c | ? 12 +++++++++---
> ?1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index cf0ad92..eeb4e32 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -61,6 +61,11 @@ module_param(dump_oops, int, 0600);
> ?MODULE_PARM_DESC(dump_oops,
> ? ? ? ? ? ? ? ?"set to 1 to dump oopses, 0 to only dump panics (default 1)");
>
> +static int ramoops_ecc;
> +module_param_named(ecc, ramoops_ecc, int, 0600);
> +MODULE_PARM_DESC(ramoops_ecc,
> + ? ? ? ? ? ? ? "set to 1 to enable ECC support");
> +
> ?struct ramoops_context {
> ? ? ? ?struct persistent_ram_zone **przs;
> ? ? ? ?phys_addr_t phys_addr;
> @@ -251,7 +256,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
> ? ? ? ? ? ? ? ?size_t sz = cxt->record_size;
> ? ? ? ? ? ? ? ?phys_addr_t start = cxt->phys_addr + sz * i;
>
> - ? ? ? ? ? ? ? cxt->przs[i] = persistent_ram_new(start, sz, 0);
> + ? ? ? ? ? ? ? cxt->przs[i] = persistent_ram_new(start, sz, ramoops_ecc);
> ? ? ? ? ? ? ? ?if (IS_ERR(cxt->przs[i])) {
> ? ? ? ? ? ? ? ? ? ? ? ?err = PTR_ERR(cxt->przs[i]);
> ? ? ? ? ? ? ? ? ? ? ? ?pr_err("failed to initialize a prz\n");
> @@ -283,9 +288,10 @@ static int __init ramoops_probe(struct platform_device *pdev)
> ? ? ? ?record_size = pdata->record_size;
> ? ? ? ?dump_oops = pdata->dump_oops;
>
> - ? ? ? pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
> + ? ? ? pr_info("attached 0x%lx@0x%llx (%ux0x%zx), ecc: %s\n",
> ? ? ? ? ? ? ? ?cxt->size, (unsigned long long)cxt->phys_addr,
> - ? ? ? ? ? ? ? cxt->max_count, cxt->record_size);
> + ? ? ? ? ? ? ? cxt->max_count, cxt->record_size,
> + ? ? ? ? ? ? ? ramoops_ecc ? "on" : "off");
>
> ? ? ? ?return 0;
>
> --
> 1.7.9.2
--
Kees Cook
Chrome OS Security
On Fri, May 11, 2012 at 5:17 PM, Anton Vorontsov
<[email protected]> wrote:
> The routine just creates a persistent ram zone at a specified address.
>
> For persistent_ram_init_ringbuffer() we'd need to add a
> 'struct persistent_ram' to the global list, and associate it with a
> device. We don't need all this complexity in pstore_ram, so we introduce
> the simple function.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> ?drivers/staging/android/persistent_ram.c | ? 26 ++++++++++++++++++++++++++
> ?drivers/staging/android/persistent_ram.h | ? ?4 ++++
> ?2 files changed, 30 insertions(+)
>
> diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
> index ec23822..c0c3d32 100644
> --- a/drivers/staging/android/persistent_ram.c
> +++ b/drivers/staging/android/persistent_ram.c
> @@ -412,6 +412,32 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
> ? ? ? ?return 0;
> ?}
>
> +struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t size,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bool ecc)
> +{
> + ? ? ? struct persistent_ram_zone *prz;
> + ? ? ? int ret = -ENOMEM;
> +
> + ? ? ? prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
> + ? ? ? if (!prz) {
> + ? ? ? ? ? ? ? pr_err("persistent_ram: failed to allocate persistent ram zone\n");
> + ? ? ? ? ? ? ? goto err;
> + ? ? ? }
> +
> + ? ? ? ret = persistent_ram_buffer_map(start, size, prz);
> + ? ? ? if (ret)
> + ? ? ? ? ? ? ? goto err;
> +
> + ? ? ? persistent_ram_post_init(prz, ecc);
> + ? ? ? persistent_ram_update_header_ecc(prz);
> +
> + ? ? ? return prz;
> +err:
> + ? ? ? kfree(prz);
> + ? ? ? return ERR_PTR(ret);
> +}
> +
> ?static ?__init
> ?struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
> ?{
> diff --git a/drivers/staging/android/persistent_ram.h b/drivers/staging/android/persistent_ram.h
> index 5635355..8154d15 100644
> --- a/drivers/staging/android/persistent_ram.h
> +++ b/drivers/staging/android/persistent_ram.h
> @@ -19,6 +19,7 @@
> ?#include <linux/kernel.h>
> ?#include <linux/list.h>
> ?#include <linux/types.h>
> +#include <linux/init.h>
>
> ?struct persistent_ram_buffer;
>
> @@ -62,6 +63,9 @@ struct persistent_ram_zone {
>
> ?int persistent_ram_early_init(struct persistent_ram *ram);
>
> +struct persistent_ram_zone * __init persistent_ram_new(phys_addr_t start,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t size,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bool ecc);
> ?struct persistent_ram_zone *persistent_ram_init_ringbuffer(struct device *dev,
> ? ? ? ? ? ? ? ?bool ecc);
>
> --
> 1.7.9.2
>
Overall I like this series, but I'm not sure I agree with
persistent_ram_new(). The point of persistent_ram_early_init with the
call to memblock_reserve and the persistent_ram_descriptor list was to
have a single pool of persistent memory that could be parcelled out to
whatever drivers needed it, keeping the code out of the board file.
With persistent_ram_new, the board file is now responsible for making
sure that the memory has been reserved with memblock_reserve(), or,
even worse, mem= from the bootloader. Mixing the two methods together
would be confusing. Either persistent_ram_early_init should be
removed completely (or replaced with something that is easier to
register ramoops into), or ramoops should use
persistent_ram_init_ringbuffer like ram_console does.
2012/5/14 Shuah Khan <[email protected]>:
> On Mon, 2012-05-14 at 08:58 -0700, Greg Kroah-Hartman wrote:
>> On Fri, May 11, 2012 at 05:15:06PM -0700, Anton Vorontsov wrote:
>> > Hi all,
>> >
>> > There are currently two competing debug facilities to store kernel
>> > messages in a persistent storage: a generic pstore and Google's
>> > persistent_ram. Not so long ago (https://lkml.org/lkml/2012/3/8/252),
>> > it was decided that we should fix this situation.
>> >
>> > Recently ramoops has switched to pstore, which basically means that
>> > it became a RAM backend for the pstore framework.
>> >
>> > persistent_ram+ram_console and ramoops+pstore have almost the same
>> > features, except:
>> >
>> > 1. Ramoops doesn't support ECC. Having ECC is useful when a hardware
>> > ? ?reset was used to bring the machine back to life (i.e. a watchdog
>> > ? ?triggered). In such cases, RAM may be somewhat corrupt, but
>> > ? ?usually it is restorable.
>> >
>> > 2. 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.
>> >
>> > These patches solve the first issue, plus move things to their
>> > proper places. Patches that will fix the second issue are pending.
>>
>> I've applied the first 7 patches, as they were localized to the
>> drivers/staging/android/ directory, but in order for me to apply the
>> rest, I need acks from the respective subsystem maintainers.
>>
>> pstore developers, what do you say about these changes, are you ok with
>> them?
>
> Good to see this work get done. Anton beat me to it. :) I have been
> talking to pstore developers (Tony Luck) and ramoops maintainers (Kees
> Cook) about this re-architecture work since I first floated this idea on
> ce-android mailing list. I have been working on this rec-architecture
> focusing on the second feature "Pstore doesn't support logging kernel
> messages in run-time" and didn't get to ECC even though it is on my
> feature list to do bring ramconsole features into ramoops.
>
> Anton! Is it safe to assume you are planning to cover the second feature
> as well, in which case I can drop my plans to get this work done.
>
> -- Shuah
>>
>> thanks,
>>
>> greg k-h
>> --
My ack for ramoops patches. You can add my acked-by.
Marco
On Fri, 2012-05-11 at 17:18 -0700, Anton Vorontsov wrote:
> Since ramoops was converted to pstore, it has nothing to do with character
> devices nowadays. Instead, today it is just a RAM backend for pstore.
>
> The patch just moves things around. There are a few changes were needed
> because of the move:
>
> 1. Kconfig and Makefiles fixups, of course.
>
> 2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
> is needed to keep user experience the same as with ramoops driver
> (i.e. so that ramoops.foo kernel command line arguments would still
> work).
Anton,
Could you please enhance Kconfig as well as ram.c with information with
the new functionality it supports. Also ram.c in my opinion doesn't
really reflect the feature it currently supports and its future plans.
ramoops doesn't either. ramdesg or ramkmsg probably are better suited.
Also leaving the ABI that ramoops specific might lead confusion in the
long run. It might make sense to update the ABI to reflect its new
features, if it doesn't impact existing ramoops users.
Would you be interested in adding a doc file for usage describing how
users can configure the driver - the details I would like to see are how
to pick a ram address especially when mem_address and mem_size are
passed in as module parameters.
Thanks,
-- Shuah
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> drivers/char/Kconfig | 9 --
> drivers/char/Makefile | 1 -
> drivers/char/ramoops.c | 362 -------------------------------------------
> fs/pstore/Kconfig | 9 ++
> fs/pstore/Makefile | 1 +
> fs/pstore/ram.c | 367 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pstore_ram.h | 17 ++
> include/linux/ramoops.h | 17 --
> 8 files changed, 394 insertions(+), 389 deletions(-)
> delete mode 100644 drivers/char/ramoops.c
> create mode 100644 fs/pstore/ram.c
> create mode 100644 include/linux/pstore_ram.h
> delete mode 100644 include/linux/ramoops.h
>
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index fab778d4..ea6f632 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -585,15 +585,6 @@ config DEVPORT
>
> source "drivers/s390/char/Kconfig"
>
> -config RAMOOPS
> - tristate "Log panic/oops to a RAM buffer"
> - depends on HAS_IOMEM
> - depends on PSTORE
> - default n
> - help
> - This enables panic and oops messages to be logged to a circular
> - buffer in RAM where it can be read back at some later point.
> -
> config MSM_SMD_PKT
> bool "Enable device interface for some SMD packet ports"
> default n
> diff --git a/drivers/char/Makefile b/drivers/char/Makefile
> index 0dc5d7c..d0b27a3 100644
> --- a/drivers/char/Makefile
> +++ b/drivers/char/Makefile
> @@ -58,7 +58,6 @@ obj-$(CONFIG_HANGCHECK_TIMER) += hangcheck-timer.o
> obj-$(CONFIG_TCG_TPM) += tpm/
>
> obj-$(CONFIG_PS3_FLASH) += ps3flash.o
> -obj-$(CONFIG_RAMOOPS) += ramoops.o
>
> obj-$(CONFIG_JS_RTC) += js-rtc.o
> js-rtc-y = rtc.o
> diff --git a/drivers/char/ramoops.c b/drivers/char/ramoops.c
> deleted file mode 100644
> index b8b8542..0000000
> --- a/drivers/char/ramoops.c
> +++ /dev/null
> @@ -1,362 +0,0 @@
> -/*
> - * RAM Oops/Panic logger
> - *
> - * Copyright (C) 2010 Marco Stornelli <[email protected]>
> - * Copyright (C) 2011 Kees Cook <[email protected]>
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - * General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> - * 02110-1301 USA
> - *
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/kernel.h>
> -#include <linux/err.h>
> -#include <linux/module.h>
> -#include <linux/pstore.h>
> -#include <linux/time.h>
> -#include <linux/io.h>
> -#include <linux/ioport.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> -#include <linux/ramoops.h>
> -
> -#define RAMOOPS_KERNMSG_HDR "===="
> -#define MIN_MEM_SIZE 4096UL
> -
> -static ulong record_size = MIN_MEM_SIZE;
> -module_param(record_size, ulong, 0400);
> -MODULE_PARM_DESC(record_size,
> - "size of each dump done on oops/panic");
> -
> -static ulong mem_address;
> -module_param(mem_address, ulong, 0400);
> -MODULE_PARM_DESC(mem_address,
> - "start of reserved RAM used to store oops/panic logs");
> -
> -static ulong mem_size;
> -module_param(mem_size, ulong, 0400);
> -MODULE_PARM_DESC(mem_size,
> - "size of reserved RAM used to store oops/panic logs");
> -
> -static int dump_oops = 1;
> -module_param(dump_oops, int, 0600);
> -MODULE_PARM_DESC(dump_oops,
> - "set to 1 to dump oopses, 0 to only dump panics (default 1)");
> -
> -struct ramoops_context {
> - void *virt_addr;
> - phys_addr_t phys_addr;
> - unsigned long size;
> - size_t record_size;
> - int dump_oops;
> - unsigned int count;
> - unsigned int max_count;
> - unsigned int read_count;
> - struct pstore_info pstore;
> -};
> -
> -static struct platform_device *dummy;
> -static struct ramoops_platform_data *dummy_data;
> -
> -static int ramoops_pstore_open(struct pstore_info *psi)
> -{
> - struct ramoops_context *cxt = psi->data;
> -
> - cxt->read_count = 0;
> - return 0;
> -}
> -
> -static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> - struct timespec *time,
> - char **buf,
> - struct pstore_info *psi)
> -{
> - ssize_t size;
> - char *rambuf;
> - struct ramoops_context *cxt = psi->data;
> -
> - if (cxt->read_count >= cxt->max_count)
> - return -EINVAL;
> - *id = cxt->read_count++;
> - /* Only supports dmesg output so far. */
> - *type = PSTORE_TYPE_DMESG;
> - /* TODO(kees): Bogus time for the moment. */
> - time->tv_sec = 0;
> - time->tv_nsec = 0;
> -
> - rambuf = cxt->virt_addr + (*id * cxt->record_size);
> - size = strnlen(rambuf, cxt->record_size);
> - *buf = kmalloc(size, GFP_KERNEL);
> - if (*buf == NULL)
> - return -ENOMEM;
> - memcpy(*buf, rambuf, size);
> -
> - return size;
> -}
> -
> -static int ramoops_pstore_write(enum pstore_type_id type,
> - enum kmsg_dump_reason reason,
> - u64 *id,
> - unsigned int part,
> - size_t size, struct pstore_info *psi)
> -{
> - char *buf;
> - size_t res;
> - struct timeval timestamp;
> - struct ramoops_context *cxt = psi->data;
> - size_t available = cxt->record_size;
> -
> - /* Currently ramoops is designed to only store dmesg dumps. */
> - if (type != PSTORE_TYPE_DMESG)
> - return -EINVAL;
> -
> - /* Out of the various dmesg dump types, ramoops is currently designed
> - * to only store crash logs, rather than storing general kernel logs.
> - */
> - if (reason != KMSG_DUMP_OOPS &&
> - reason != KMSG_DUMP_PANIC)
> - return -EINVAL;
> -
> - /* Skip Oopes when configured to do so. */
> - if (reason == KMSG_DUMP_OOPS && !cxt->dump_oops)
> - return -EINVAL;
> -
> - /* Explicitly only take the first part of any new crash.
> - * If our buffer is larger than kmsg_bytes, this can never happen,
> - * and if our buffer is smaller than kmsg_bytes, we don't want the
> - * report split across multiple records.
> - */
> - if (part != 1)
> - return -ENOSPC;
> -
> - buf = cxt->virt_addr + (cxt->count * cxt->record_size);
> -
> - res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
> - buf += res;
> - available -= res;
> -
> - do_gettimeofday(×tamp);
> - res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
> - buf += res;
> - available -= res;
> -
> - if (size > available)
> - size = available;
> -
> - memcpy(buf, cxt->pstore.buf, size);
> - memset(buf + size, '\0', available - size);
> -
> - cxt->count = (cxt->count + 1) % cxt->max_count;
> -
> - return 0;
> -}
> -
> -static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
> - struct pstore_info *psi)
> -{
> - char *buf;
> - struct ramoops_context *cxt = psi->data;
> -
> - if (id >= cxt->max_count)
> - return -EINVAL;
> -
> - buf = cxt->virt_addr + (id * cxt->record_size);
> - memset(buf, '\0', cxt->record_size);
> -
> - return 0;
> -}
> -
> -static struct ramoops_context oops_cxt = {
> - .pstore = {
> - .owner = THIS_MODULE,
> - .name = "ramoops",
> - .open = ramoops_pstore_open,
> - .read = ramoops_pstore_read,
> - .write = ramoops_pstore_write,
> - .erase = ramoops_pstore_erase,
> - },
> -};
> -
> -static int __init ramoops_probe(struct platform_device *pdev)
> -{
> - struct ramoops_platform_data *pdata = pdev->dev.platform_data;
> - struct ramoops_context *cxt = &oops_cxt;
> - int err = -EINVAL;
> -
> - /* Only a single ramoops area allowed at a time, so fail extra
> - * probes.
> - */
> - if (cxt->max_count)
> - goto fail_out;
> -
> - if (!pdata->mem_size || !pdata->record_size) {
> - pr_err("The memory size and the record size must be "
> - "non-zero\n");
> - goto fail_out;
> - }
> -
> - pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
> - pdata->record_size = rounddown_pow_of_two(pdata->record_size);
> -
> - /* Check for the minimum memory size */
> - if (pdata->mem_size < MIN_MEM_SIZE &&
> - pdata->record_size < MIN_MEM_SIZE) {
> - pr_err("memory size too small, minimum is %lu\n",
> - MIN_MEM_SIZE);
> - goto fail_out;
> - }
> -
> - if (pdata->mem_size < pdata->record_size) {
> - pr_err("The memory size must be larger than the "
> - "records size\n");
> - goto fail_out;
> - }
> -
> - cxt->max_count = pdata->mem_size / pdata->record_size;
> - cxt->count = 0;
> - cxt->size = pdata->mem_size;
> - cxt->phys_addr = pdata->mem_address;
> - cxt->record_size = pdata->record_size;
> - cxt->dump_oops = pdata->dump_oops;
> -
> - cxt->pstore.data = cxt;
> - cxt->pstore.bufsize = cxt->record_size;
> - cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
> - spin_lock_init(&cxt->pstore.buf_lock);
> - if (!cxt->pstore.buf) {
> - pr_err("cannot allocate pstore buffer\n");
> - goto fail_clear;
> - }
> -
> - if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
> - pr_err("request mem region (0x%lx@0x%llx) failed\n",
> - cxt->size, (unsigned long long)cxt->phys_addr);
> - err = -EINVAL;
> - goto fail_buf;
> - }
> -
> - cxt->virt_addr = ioremap(cxt->phys_addr, cxt->size);
> - if (!cxt->virt_addr) {
> - pr_err("ioremap failed\n");
> - goto fail_mem_region;
> - }
> -
> - err = pstore_register(&cxt->pstore);
> - if (err) {
> - pr_err("registering with pstore failed\n");
> - goto fail_iounmap;
> - }
> -
> - /*
> - * Update the module parameter variables as well so they are visible
> - * through /sys/module/ramoops/parameters/
> - */
> - mem_size = pdata->mem_size;
> - mem_address = pdata->mem_address;
> - record_size = pdata->record_size;
> - dump_oops = pdata->dump_oops;
> -
> - pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
> - cxt->size, (unsigned long long)cxt->phys_addr,
> - cxt->max_count, cxt->record_size);
> -
> - return 0;
> -
> -fail_iounmap:
> - iounmap(cxt->virt_addr);
> -fail_mem_region:
> - release_mem_region(cxt->phys_addr, cxt->size);
> -fail_buf:
> - kfree(cxt->pstore.buf);
> -fail_clear:
> - cxt->pstore.bufsize = 0;
> - cxt->max_count = 0;
> -fail_out:
> - return err;
> -}
> -
> -static int __exit ramoops_remove(struct platform_device *pdev)
> -{
> -#if 0
> - /* TODO(kees): We cannot unload ramoops since pstore doesn't support
> - * unregistering yet.
> - */
> - struct ramoops_context *cxt = &oops_cxt;
> -
> - iounmap(cxt->virt_addr);
> - release_mem_region(cxt->phys_addr, cxt->size);
> - cxt->max_count = 0;
> -
> - /* TODO(kees): When pstore supports unregistering, call it here. */
> - kfree(cxt->pstore.buf);
> - cxt->pstore.bufsize = 0;
> -
> - return 0;
> -#endif
> - return -EBUSY;
> -}
> -
> -static struct platform_driver ramoops_driver = {
> - .remove = __exit_p(ramoops_remove),
> - .driver = {
> - .name = "ramoops",
> - .owner = THIS_MODULE,
> - },
> -};
> -
> -static int __init ramoops_init(void)
> -{
> - int ret;
> - ret = platform_driver_probe(&ramoops_driver, ramoops_probe);
> - if (ret == -ENODEV) {
> - /*
> - * If we didn't find a platform device, we use module parameters
> - * building platform data on the fly.
> - */
> - pr_info("platform device not found, using module parameters\n");
> - dummy_data = kzalloc(sizeof(struct ramoops_platform_data),
> - GFP_KERNEL);
> - if (!dummy_data)
> - return -ENOMEM;
> - dummy_data->mem_size = mem_size;
> - dummy_data->mem_address = mem_address;
> - dummy_data->record_size = record_size;
> - dummy_data->dump_oops = dump_oops;
> - dummy = platform_create_bundle(&ramoops_driver, ramoops_probe,
> - NULL, 0, dummy_data,
> - sizeof(struct ramoops_platform_data));
> -
> - if (IS_ERR(dummy))
> - ret = PTR_ERR(dummy);
> - else
> - ret = 0;
> - }
> -
> - return ret;
> -}
> -
> -static void __exit ramoops_exit(void)
> -{
> - platform_driver_unregister(&ramoops_driver);
> - kfree(dummy_data);
> -}
> -
> -module_init(ramoops_init);
> -module_exit(ramoops_exit);
> -
> -MODULE_LICENSE("GPL");
> -MODULE_AUTHOR("Marco Stornelli <[email protected]>");
> -MODULE_DESCRIPTION("RAM Oops/Panic logger/driver");
> diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
> index 8007ae7..ad6e594 100644
> --- a/fs/pstore/Kconfig
> +++ b/fs/pstore/Kconfig
> @@ -11,3 +11,12 @@ config PSTORE
> (e.g. ACPI_APEI on X86) which will select this for you.
> If you don't have a platform persistent store driver,
> say N.
> +
> +config PSTORE_RAM
> + tristate "Log panic/oops to a RAM buffer"
> + depends on HAS_IOMEM
> + depends on PSTORE
> + default n
> + help
> + This enables panic and oops messages to be logged to a circular
> + buffer in RAM where it can be read back at some later point.
> diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
> index 760f4bc..804e376 100644
> --- a/fs/pstore/Makefile
> +++ b/fs/pstore/Makefile
> @@ -5,3 +5,4 @@
> obj-y += pstore.o
>
> pstore-objs += inode.o platform.o
> +obj-$(CONFIG_PSTORE_RAM) += ram.o
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> new file mode 100644
> index 0000000..b26b58e
> --- /dev/null
> +++ b/fs/pstore/ram.c
> @@ -0,0 +1,367 @@
> +/*
> + * RAM Oops/Panic logger
> + *
> + * Copyright (C) 2010 Marco Stornelli <[email protected]>
> + * Copyright (C) 2011 Kees Cook <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +#define pr_fmt(fmt) "ramoops: " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/pstore.h>
> +#include <linux/time.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/pstore_ram.h>
> +
> +/* For historical reasons we name it ramoops when built-in. */
> +#ifndef MODULE
> +#undef MODULE_PARAM_PREFIX
> +#define MODULE_PARAM_PREFIX "ramoops."
> +#endif
> +
> +#define RAMOOPS_KERNMSG_HDR "===="
> +#define MIN_MEM_SIZE 4096UL
> +
> +static ulong record_size = MIN_MEM_SIZE;
> +module_param(record_size, ulong, 0400);
> +MODULE_PARM_DESC(record_size,
> + "size of each dump done on oops/panic");
> +
> +static ulong mem_address;
> +module_param(mem_address, ulong, 0400);
> +MODULE_PARM_DESC(mem_address,
> + "start of reserved RAM used to store oops/panic logs");
> +
> +static ulong mem_size;
> +module_param(mem_size, ulong, 0400);
> +MODULE_PARM_DESC(mem_size,
> + "size of reserved RAM used to store oops/panic logs");
> +
> +static int dump_oops = 1;
> +module_param(dump_oops, int, 0600);
> +MODULE_PARM_DESC(dump_oops,
> + "set to 1 to dump oopses, 0 to only dump panics (default 1)");
> +
> +struct ramoops_context {
> + void *virt_addr;
> + phys_addr_t phys_addr;
> + unsigned long size;
> + size_t record_size;
> + int dump_oops;
> + unsigned int count;
> + unsigned int max_count;
> + unsigned int read_count;
> + struct pstore_info pstore;
> +};
> +
> +static struct platform_device *dummy;
> +static struct ramoops_platform_data *dummy_data;
> +
> +static int ramoops_pstore_open(struct pstore_info *psi)
> +{
> + struct ramoops_context *cxt = psi->data;
> +
> + cxt->read_count = 0;
> + return 0;
> +}
> +
> +static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
> + struct timespec *time,
> + char **buf,
> + struct pstore_info *psi)
> +{
> + ssize_t size;
> + char *rambuf;
> + struct ramoops_context *cxt = psi->data;
> +
> + if (cxt->read_count >= cxt->max_count)
> + return -EINVAL;
> + *id = cxt->read_count++;
> + /* Only supports dmesg output so far. */
> + *type = PSTORE_TYPE_DMESG;
> + /* TODO(kees): Bogus time for the moment. */
> + time->tv_sec = 0;
> + time->tv_nsec = 0;
> +
> + rambuf = cxt->virt_addr + (*id * cxt->record_size);
> + size = strnlen(rambuf, cxt->record_size);
> + *buf = kmalloc(size, GFP_KERNEL);
> + if (*buf == NULL)
> + return -ENOMEM;
> + memcpy(*buf, rambuf, size);
> +
> + return size;
> +}
> +
> +static int ramoops_pstore_write(enum pstore_type_id type,
> + enum kmsg_dump_reason reason,
> + u64 *id,
> + unsigned int part,
> + size_t size, struct pstore_info *psi)
> +{
> + char *buf;
> + size_t res;
> + struct timeval timestamp;
> + struct ramoops_context *cxt = psi->data;
> + size_t available = cxt->record_size;
> +
> + /* Currently ramoops is designed to only store dmesg dumps. */
> + if (type != PSTORE_TYPE_DMESG)
> + return -EINVAL;
> +
> + /* Out of the various dmesg dump types, ramoops is currently designed
> + * to only store crash logs, rather than storing general kernel logs.
> + */
> + if (reason != KMSG_DUMP_OOPS &&
> + reason != KMSG_DUMP_PANIC)
> + return -EINVAL;
> +
> + /* Skip Oopes when configured to do so. */
> + if (reason == KMSG_DUMP_OOPS && !cxt->dump_oops)
> + return -EINVAL;
> +
> + /* Explicitly only take the first part of any new crash.
> + * If our buffer is larger than kmsg_bytes, this can never happen,
> + * and if our buffer is smaller than kmsg_bytes, we don't want the
> + * report split across multiple records.
> + */
> + if (part != 1)
> + return -ENOSPC;
> +
> + buf = cxt->virt_addr + (cxt->count * cxt->record_size);
> +
> + res = sprintf(buf, "%s", RAMOOPS_KERNMSG_HDR);
> + buf += res;
> + available -= res;
> +
> + do_gettimeofday(×tamp);
> + res = sprintf(buf, "%lu.%lu\n", (long)timestamp.tv_sec, (long)timestamp.tv_usec);
> + buf += res;
> + available -= res;
> +
> + if (size > available)
> + size = available;
> +
> + memcpy(buf, cxt->pstore.buf, size);
> + memset(buf + size, '\0', available - size);
> +
> + cxt->count = (cxt->count + 1) % cxt->max_count;
> +
> + return 0;
> +}
> +
> +static int ramoops_pstore_erase(enum pstore_type_id type, u64 id,
> + struct pstore_info *psi)
> +{
> + char *buf;
> + struct ramoops_context *cxt = psi->data;
> +
> + if (id >= cxt->max_count)
> + return -EINVAL;
> +
> + buf = cxt->virt_addr + (id * cxt->record_size);
> + memset(buf, '\0', cxt->record_size);
> +
> + return 0;
> +}
> +
> +static struct ramoops_context oops_cxt = {
> + .pstore = {
> + .owner = THIS_MODULE,
> + .name = "ramoops",
> + .open = ramoops_pstore_open,
> + .read = ramoops_pstore_read,
> + .write = ramoops_pstore_write,
> + .erase = ramoops_pstore_erase,
> + },
> +};
> +
> +static int __init ramoops_probe(struct platform_device *pdev)
> +{
> + struct ramoops_platform_data *pdata = pdev->dev.platform_data;
> + struct ramoops_context *cxt = &oops_cxt;
> + int err = -EINVAL;
> +
> + /* Only a single ramoops area allowed at a time, so fail extra
> + * probes.
> + */
> + if (cxt->max_count)
> + goto fail_out;
> +
> + if (!pdata->mem_size || !pdata->record_size) {
> + pr_err("The memory size and the record size must be "
> + "non-zero\n");
> + goto fail_out;
> + }
> +
> + pdata->mem_size = rounddown_pow_of_two(pdata->mem_size);
> + pdata->record_size = rounddown_pow_of_two(pdata->record_size);
> +
> + /* Check for the minimum memory size */
> + if (pdata->mem_size < MIN_MEM_SIZE &&
> + pdata->record_size < MIN_MEM_SIZE) {
> + pr_err("memory size too small, minimum is %lu\n",
> + MIN_MEM_SIZE);
> + goto fail_out;
> + }
> +
> + if (pdata->mem_size < pdata->record_size) {
> + pr_err("The memory size must be larger than the "
> + "records size\n");
> + goto fail_out;
> + }
> +
> + cxt->max_count = pdata->mem_size / pdata->record_size;
> + cxt->count = 0;
> + cxt->size = pdata->mem_size;
> + cxt->phys_addr = pdata->mem_address;
> + cxt->record_size = pdata->record_size;
> + cxt->dump_oops = pdata->dump_oops;
> +
> + cxt->pstore.data = cxt;
> + cxt->pstore.bufsize = cxt->record_size;
> + cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
> + spin_lock_init(&cxt->pstore.buf_lock);
> + if (!cxt->pstore.buf) {
> + pr_err("cannot allocate pstore buffer\n");
> + goto fail_clear;
> + }
> +
> + if (!request_mem_region(cxt->phys_addr, cxt->size, "ramoops")) {
> + pr_err("request mem region (0x%lx@0x%llx) failed\n",
> + cxt->size, (unsigned long long)cxt->phys_addr);
> + err = -EINVAL;
> + goto fail_buf;
> + }
> +
> + cxt->virt_addr = ioremap(cxt->phys_addr, cxt->size);
> + if (!cxt->virt_addr) {
> + pr_err("ioremap failed\n");
> + goto fail_mem_region;
> + }
> +
> + err = pstore_register(&cxt->pstore);
> + if (err) {
> + pr_err("registering with pstore failed\n");
> + goto fail_iounmap;
> + }
> +
> + /*
> + * Update the module parameter variables as well so they are visible
> + * through /sys/module/ramoops/parameters/
> + */
> + mem_size = pdata->mem_size;
> + mem_address = pdata->mem_address;
> + record_size = pdata->record_size;
> + dump_oops = pdata->dump_oops;
> +
> + pr_info("attached 0x%lx@0x%llx (%ux0x%zx)\n",
> + cxt->size, (unsigned long long)cxt->phys_addr,
> + cxt->max_count, cxt->record_size);
> +
> + return 0;
> +
> +fail_iounmap:
> + iounmap(cxt->virt_addr);
> +fail_mem_region:
> + release_mem_region(cxt->phys_addr, cxt->size);
> +fail_buf:
> + kfree(cxt->pstore.buf);
> +fail_clear:
> + cxt->pstore.bufsize = 0;
> + cxt->max_count = 0;
> +fail_out:
> + return err;
> +}
> +
> +static int __exit ramoops_remove(struct platform_device *pdev)
> +{
> +#if 0
> + /* TODO(kees): We cannot unload ramoops since pstore doesn't support
> + * unregistering yet.
> + */
> + struct ramoops_context *cxt = &oops_cxt;
> +
> + iounmap(cxt->virt_addr);
> + release_mem_region(cxt->phys_addr, cxt->size);
> + cxt->max_count = 0;
> +
> + /* TODO(kees): When pstore supports unregistering, call it here. */
> + kfree(cxt->pstore.buf);
> + cxt->pstore.bufsize = 0;
> +
> + return 0;
> +#endif
> + return -EBUSY;
> +}
> +
> +static struct platform_driver ramoops_driver = {
> + .remove = __exit_p(ramoops_remove),
> + .driver = {
> + .name = "ramoops",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init ramoops_init(void)
> +{
> + int ret;
> + ret = platform_driver_probe(&ramoops_driver, ramoops_probe);
> + if (ret == -ENODEV) {
> + /*
> + * If we didn't find a platform device, we use module parameters
> + * building platform data on the fly.
> + */
> + pr_info("platform device not found, using module parameters\n");
> + dummy_data = kzalloc(sizeof(struct ramoops_platform_data),
> + GFP_KERNEL);
> + if (!dummy_data)
> + return -ENOMEM;
> + dummy_data->mem_size = mem_size;
> + dummy_data->mem_address = mem_address;
> + dummy_data->record_size = record_size;
> + dummy_data->dump_oops = dump_oops;
> + dummy = platform_create_bundle(&ramoops_driver, ramoops_probe,
> + NULL, 0, dummy_data,
> + sizeof(struct ramoops_platform_data));
> +
> + if (IS_ERR(dummy))
> + ret = PTR_ERR(dummy);
> + else
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +static void __exit ramoops_exit(void)
> +{
> + platform_driver_unregister(&ramoops_driver);
> + kfree(dummy_data);
> +}
> +
> +module_init(ramoops_init);
> +module_exit(ramoops_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Marco Stornelli <[email protected]>");
> +MODULE_DESCRIPTION("RAM Oops/Panic logger/driver");
> diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
> new file mode 100644
> index 0000000..484fef8
> --- /dev/null
> +++ b/include/linux/pstore_ram.h
> @@ -0,0 +1,17 @@
> +#ifndef __RAMOOPS_H
> +#define __RAMOOPS_H
> +
> +/*
> + * Ramoops platform data
> + * @mem_size memory size for ramoops
> + * @mem_address physical memory address to contain ramoops
> + */
> +
> +struct ramoops_platform_data {
> + unsigned long mem_size;
> + unsigned long mem_address;
> + unsigned long record_size;
> + int dump_oops;
> +};
> +
> +#endif
> diff --git a/include/linux/ramoops.h b/include/linux/ramoops.h
> deleted file mode 100644
> index 484fef8..0000000
> --- a/include/linux/ramoops.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -#ifndef __RAMOOPS_H
> -#define __RAMOOPS_H
> -
> -/*
> - * Ramoops platform data
> - * @mem_size memory size for ramoops
> - * @mem_address physical memory address to contain ramoops
> - */
> -
> -struct ramoops_platform_data {
> - unsigned long mem_size;
> - unsigned long mem_address;
> - unsigned long record_size;
> - int dump_oops;
> -};
> -
> -#endif
On Mon, May 14, 2012 at 01:45:31PM -0700, Anton Vorontsov wrote:
> On Mon, May 14, 2012 at 10:30:22AM -0600, Shuah Khan wrote:
> [...]
> > Anton! Is it safe to assume you are planning to cover the second feature
> > as well, in which case I can drop my plans to get this work done.
>
> Yep, absolutely. I'm fully committed to add runtime logging support
> to pstore. Actually, I'll post patches pretty soon.
Care to redo the last 4 in this series as well, based on Kees's
comments?
thanks,
greg k-h
Hello Kees,
On Mon, May 14, 2012 at 02:34:18PM -0700, Kees Cook wrote:
> On Fri, May 11, 2012 at 5:18 PM, Anton Vorontsov
> <[email protected]> wrote:
> > Since ramoops was converted to pstore, it has nothing to do with character
> > devices nowadays. Instead, today it is just a RAM backend for pstore.
> >
> > The patch just moves things around. There are a few changes were needed
> > because of the move:
> >
> > 1. Kconfig and Makefiles fixups, of course.
> >
> > 2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
> > is needed to keep user experience the same as with ramoops driver
> > (i.e. so that ramoops.foo kernel command line arguments would still
> > work).
> >
> > Signed-off-by: Anton Vorontsov <[email protected]>
>
> This consolidation seems good. I might prefer the move separated from
> the changes, just to make review easier, but I have no idea what
> that'll do to a bisect. :P
Yep, exactly, the point of making the changes together with the
move was to keep things bisectable.
> > --- /dev/null
> > +++ b/fs/pstore/ram.c
>
> "ram.ko" seems like an awfully generic modbule name. Should this be
> called pstore_ram.* instead, like was done for the header file?
Oh, right you are. Actually, if I'd change the module name via
Makefile (i.e. ramoops-objs = ram.o), we can get rid of
MODULE_PARAM_PREFIX hack. So, I'd just name the module ramoops.ko
name, but keep the ram.c source file name.
Thanks for the hint.
> And unless anyone objects, I have no problem letting the built-in name
> change too.
>
> > --- /dev/null
> > +++ b/include/linux/pstore_ram.h
> > @@ -0,0 +1,17 @@
> > +#ifndef __RAMOOPS_H
> > +#define __RAMOOPS_H
>
> This define should probably change just to avoid confusion.
Fixed, thanks!
--
Anton Vorontsov
Email: [email protected]
Hello Colin,
On Mon, May 14, 2012 at 05:37:56PM -0700, Colin Cross wrote:
[...]
> even worse, mem= from the bootloader. Mixing the two methods together
> would be confusing.
Yes, mixing is discouraged. The mem= hack is mostly useful for
developers, for hacking random kernels. Even on x86 it is
useful, when you want to grab an oops, but you don't have say
netconsole, or HW really screwed up and you don't have any
means to get the oops log, ramoops may become quite useful.
But in the Android phone scenario, if you want to have this
feature into production kernels, platforms should register
the ramoops platform driver, as they were doing before.
> Either persistent_ram_early_init should be
> removed completely (or replaced with something that is easier to
> register ramoops into), or ramoops should use
> persistent_ram_init_ringbuffer like ram_console does.
Yep, this was indeed my original idea: persistent_ram_early_init
should go.
Boards (or generic arch/ or arch/mach-* code that knows memory
layout) will have to just do two things:
1. Wisely and early call memblock_reserve().
2. Register a ramoops platform device pointing to the reserved
memory.
This is actually exactly the same as you were doing with
ram_console:
1. Platform were adding an entry to the global list of persistent
ram zones, and then were calling persistent_ram_early_init()
somewhere in the arch/ code (at least that's how I understood
the idea of the code, as there are currently no in-tree users).
2. Then platforms were registering a ram_console platform device,
and the driver would find out the needed zone by matching on
the device name.
Thinking about it, the whole thing was actually abusing
the device-driver model a little bit. So things are just easier
now.
Thanks!
--
Anton Vorontsov
Email: [email protected]
Hello Kees,
On Mon, May 14, 2012 at 03:21:17PM -0700, Kees Cook wrote:
[...]
> > - buf = cxt->virt_addr + (id * cxt->record_size);
> > - memset(buf, '\0', cxt->record_size);
> > + persistent_ram_free_old(cxt->przs[id]);
>
> Hm, I don't think persistent_ram_free_old() is what's wanted here.
> That appears to entirely release the region? I want to make sure the
> memory is cleared first. And will this area come back on a write, or
> does it stay released?
It just releases ECC-restored memory region (a copy). The original
(persistent) region is still fully reusable after that call.
(It is a pity that pstore internals can't use the restored copy
directly, as pstore expects that it will release the region itself
after pstore_mkfile(), so we somewhat duplicate the memory during
psi->read(). We'd better fix it some day, but it's a minor issue
so far.)
> >
> > return 0;
> > }
> > @@ -200,6 +203,7 @@ static int __init ramoops_probe(struct platform_device *pdev)
> > struct ramoops_platform_data *pdata = pdev->dev.platform_data;
> > struct ramoops_context *cxt = &oops_cxt;
> > int err = -EINVAL;
> > + int i;
> >
> > /* Only a single ramoops area allowed at a time, so fail extra
> > * probes.
> > @@ -237,32 +241,37 @@ static int __init ramoops_probe(struct platform_device *pdev)
> > cxt->record_size = pdata->record_size;
> > cxt->dump_oops = pdata->dump_oops;
> >
> > + cxt->przs = kzalloc(sizeof(*cxt->przs) * cxt->max_count, GFP_KERNEL);
> > + if (!cxt->przs) {
> > + pr_err("failed to initialize a prz array\n");
> > + goto fail_przs;
>
> This should be fail_out.
Thanks, will fix all of these error handling negligences.
> > + }
> > +
> > + for (i = 0; i < cxt->max_count; i++) {
> > + size_t sz = cxt->record_size;
> > + phys_addr_t start = cxt->phys_addr + sz * i;
> > +
> > + cxt->przs[i] = persistent_ram_new(start, sz, 0);
>
> persistent_ram_new() is marked as __init, so this is unsafe to call if
> built as a module. I think persistent_ram_new() will need to lose the
> __init marking, or I'm misunderstanding something.
Um. ramoops' probe routine is also __init. persistent_ram_new is a
part of ramoops module, so their __init functions will be discarded
at the same time.
ram_console can't be a module, so it is also fine.
So I think it's all fine.
> > + if (IS_ERR(cxt->przs[i])) {
> > + err = PTR_ERR(cxt->przs[i]);
> > + pr_err("failed to initialize a prz\n");
>
> Since neither persistent_ram_new() nor persistent_ram_buffer_map()
> report the location of the failure, I'd like to keep the error report
> (removed below "pr_err("request mem region (0x%lx@0x%llx)
> failed\n",...") for failures, so there is something actionable in
> dmesg when the platform data is mismatched for the hardware.
Sure thing, will do. I'll also start using dev_err() for new
code, that way it's more clearer which module reported the error.
[...]
> > cxt->pstore.data = cxt;
> > - cxt->pstore.bufsize = cxt->record_size;
> > - cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
> > spin_lock_init(&cxt->pstore.buf_lock);
> > + cxt->pstore.bufsize = cxt->przs[0]->buffer_size;
> > + cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
>
> I don't see a reason to re-order these (nothing can use buf yet
> because we haven't registered it with pstore yet).
Yeah, this is a left over. Thank for catching.
[...]
> > +fail_przs:
> > + for (i = 0; cxt->przs[i]; i++)
> > + persistent_ram_free(cxt->przs[i]);
>
> This can lead to a BUG, since persistent_ram_free() doesn't handle
> NULL arguments.
The for loop has 'cxt->przs[i]' condition. :-)
Thanks for the review!
--
Anton Vorontsov
Email: [email protected]
Hi Shuah,
On Tue, May 15, 2012 at 09:12:59AM -0600, Shuah Khan wrote:
> On Fri, 2012-05-11 at 17:18 -0700, Anton Vorontsov wrote:
> > Since ramoops was converted to pstore, it has nothing to do with character
> > devices nowadays. Instead, today it is just a RAM backend for pstore.
> >
> > The patch just moves things around. There are a few changes were needed
> > because of the move:
> >
> > 1. Kconfig and Makefiles fixups, of course.
> >
> > 2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
> > is needed to keep user experience the same as with ramoops driver
> > (i.e. so that ramoops.foo kernel command line arguments would still
> > work).
>
> Anton,
>
> Could you please enhance Kconfig as well as ram.c with information with
> the new functionality it supports.
Sure, will do.
> Also ram.c in my opinion doesn't
> really reflect the feature it currently supports and its future plans.
> ramoops doesn't either. ramdesg or ramkmsg probably are better suited.
No, I actually think we shouldn't mention neither dmesg nor kmsg in
the name of the module. We might support MCE messages, tracing
messages and so on, and this all will be handled by ram.c.
So, ram.c is a generic backend for pstore.
> Also leaving the ABI that ramoops specific might lead confusion in the
> long run. It might make sense to update the ABI to reflect its new
> features, if it doesn't impact existing ramoops users.
We can do this, I can prepare a separate patch to change the ABI, but
so far I tend to not break any ABIs. We can always do it later -- it is
easy. :-D
> Would you be interested in adding a doc file for usage describing how
> users can configure the driver - the details I would like to see are how
> to pick a ram address especially when mem_address and mem_size are
> passed in as module parameters.
We actually have Documentation/ramoops.txt already, but I'll add
a documentation for the new ecc option.
Thanks!
--
Anton Vorontsov
Email: [email protected]
On Tue, May 15, 2012 at 11:14 PM, Anton Vorontsov
<[email protected]> wrote:
> Hello Kees,
>
> On Mon, May 14, 2012 at 03:21:17PM -0700, Kees Cook wrote:
> [...]
>> > - ? ? ? buf = cxt->virt_addr + (id * cxt->record_size);
>> > - ? ? ? memset(buf, '\0', cxt->record_size);
>> > + ? ? ? persistent_ram_free_old(cxt->przs[id]);
>>
>> Hm, I don't think persistent_ram_free_old() is what's wanted here.
>> That appears to entirely release the region? I want to make sure the
>> memory is cleared first. And will this area come back on a write, or
>> does it stay released?
>
> It just releases ECC-restored memory region (a copy). The original
> (persistent) region is still fully reusable after that call.
Ah-ha, okay. So this still needs to clear the memory in the "real"
copy then. Thanks for the clarification.
>> > + ? ? ? }
>> > +
>> > + ? ? ? for (i = 0; i < cxt->max_count; i++) {
>> > + ? ? ? ? ? ? ? size_t sz = cxt->record_size;
>> > + ? ? ? ? ? ? ? phys_addr_t start = cxt->phys_addr + sz * i;
>> > +
>> > + ? ? ? ? ? ? ? cxt->przs[i] = persistent_ram_new(start, sz, 0);
>>
>> persistent_ram_new() is marked as __init, so this is unsafe to call if
>> built as a module. I think persistent_ram_new() will need to lose the
>> __init marking, or I'm misunderstanding something.
>
> Um. ramoops' probe routine is also __init. persistent_ram_new is a
> part of ramoops module, so their __init functions will be discarded
> at the same time.
>
> ram_console can't be a module, so it is also fine.
>
> So I think it's all fine.
This is what I get for staring at patches instead of applying them. :)
Yeah, if it's all built together, it's no problem. It looked to me
like they were in different modules.
>> > +fail_przs:
>> > + ? ? ? for (i = 0; cxt->przs[i]; i++)
>> > + ? ? ? ? ? ? ? persistent_ram_free(cxt->przs[i]);
>>
>> This can lead to a BUG, since persistent_ram_free() doesn't handle
>> NULL arguments.
>
> The for loop has 'cxt->przs[i]' condition. :-)
Okay, fair enough. :)
> Thanks for the review!
Sure thing! Thanks for doing this work; I'm excited to have access in
ramoops to the new interfaces. :)
-Kees
--
Kees Cook
Chrome OS Security
On Wed, 2012-05-16 at 00:30 -0700, Anton Vorontsov wrote:
> Hi Shuah,
>
> On Tue, May 15, 2012 at 09:12:59AM -0600, Shuah Khan wrote:
> > On Fri, 2012-05-11 at 17:18 -0700, Anton Vorontsov wrote:
> > > Since ramoops was converted to pstore, it has nothing to do with character
> > > devices nowadays. Instead, today it is just a RAM backend for pstore.
> > >
> > > The patch just moves things around. There are a few changes were needed
> > > because of the move:
> > >
> > > 1. Kconfig and Makefiles fixups, of course.
> > >
> > > 2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
> > > is needed to keep user experience the same as with ramoops driver
> > > (i.e. so that ramoops.foo kernel command line arguments would still
> > > work).
> >
> > Anton,
> >
> > Could you please enhance Kconfig as well as ram.c with information with
> > the new functionality it supports.
>
> Sure, will do.
>
> > Also ram.c in my opinion doesn't
> > really reflect the feature it currently supports and its future plans.
> > ramoops doesn't either. ramdesg or ramkmsg probably are better suited.
>
> No, I actually think we shouldn't mention neither dmesg nor kmsg in
> the name of the module. We might support MCE messages, tracing
> messages and so on, and this all will be handled by ram.c.
Good point.
>
> So, ram.c is a generic backend for pstore.
>
> > Also leaving the ABI that ramoops specific might lead confusion in the
> > long run. It might make sense to update the ABI to reflect its new
> > features, if it doesn't impact existing ramoops users.
>
> We can do this, I can prepare a separate patch to change the ABI, but
> so far I tend to not break any ABIs. We can always do it later -- it is
> easy. :-D
Yes it can be done later.
>
> > Would you be interested in adding a doc file for usage describing how
> > users can configure the driver - the details I would like to see are how
> > to pick a ram address especially when mem_address and mem_size are
> > passed in as module parameters.
>
> We actually have Documentation/ramoops.txt already, but I'll add
> a documentation for the new ecc option.
>
> Thanks!
Thanks for doing this. One thing that would be helpful for users is some
kind of guidance/tips on how to pick ram range for module parameter
passing, which is missing from the current ramoops.txt
Thanks,
-- Shuah
On Fri, May 11, 2012 at 5:17 PM, Anton Vorontsov
<[email protected]> wrote:
> This includes devices' memory (e.g. framebuffers or memory mapped
> EEPROMs on a local bus), as well as the normal RAM that we don't use
> for the main memory.
>
> For the normal (but unused) ram we could use kmaps, but this assumes
> highmem support, so we don't bother and just use the memory via
> ioremap.
>
> As a side effect, the following hack is possible: when used together
> with pstore_ram (new ramoops) module, we can limit the normal RAM region
> with mem= and then point ramoops to use the rest of the memory, e.g.
>
> ? ? ? ?mem=128M ramoops.mem_address=0x8000000
>
> Sure, we could just reserve the region with memblock_reserve() early in
> the arch/ code, and then register a pstore_ram platform device pointing
> to the reserved region. It's still a viable option if platform wants
> to do so.
>
> Also, we might want to use IO accessors in case of a real device,
> but for now we don't bother (the old ramoops wasn't using it either, so
> at least we don't make things worse).
This is long merged, but I remembered why I moved away from using
ioremap. The current code uses atomics to track the ringbuffer
positions, which results in ldrex and strex instructions on ARM.
ldrex and strex on memory that is mapped as Device memory (which is
what ioremap maps as) is implementation defined, and is unpredictable
at the architecture level.
On Wed, Jun 06, 2012 at 02:10:34PM -0700, Colin Cross wrote:
> On Fri, May 11, 2012 at 5:17 PM, Anton Vorontsov
> <[email protected]> wrote:
> > This includes devices' memory (e.g. framebuffers or memory mapped
> > EEPROMs on a local bus), as well as the normal RAM that we don't use
> > for the main memory.
> >
> > For the normal (but unused) ram we could use kmaps, but this assumes
> > highmem support, so we don't bother and just use the memory via
> > ioremap.
> >
> > As a side effect, the following hack is possible: when used together
> > with pstore_ram (new ramoops) module, we can limit the normal RAM region
> > with mem= and then point ramoops to use the rest of the memory, e.g.
> >
> > mem=128M ramoops.mem_address=0x8000000
> >
> > Sure, we could just reserve the region with memblock_reserve() early in
> > the arch/ code, and then register a pstore_ram platform device pointing
> > to the reserved region. It's still a viable option if platform wants
> > to do so.
> >
> > Also, we might want to use IO accessors in case of a real device,
> > but for now we don't bother (the old ramoops wasn't using it either, so
> > at least we don't make things worse).
>
> This is long merged, but I remembered why I moved away from using
> ioremap. The current code uses atomics to track the ringbuffer
> positions, which results in ldrex and strex instructions on ARM.
> ldrex and strex on memory that is mapped as Device memory (which is
> what ioremap maps as) is implementation defined, and is unpredictable
> at the architecture level.
Makes sense, thanks for sharing! Fortunately, we still map things
w/ vmap if pfn appears to be valid. :-)
Thanks,
--
Anton Vorontsov
Email: [email protected]