2013-04-24 06:20:13

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 0/8] powerpc/pseries: Nvram-to-pstore

Currently the kernel provides the contents of p-series NVRAM only as a
simple stream of bytes via /dev/nvram, which must be interpreted in user
space by the nvram command in the powerpc-utils package. This patch set
exploits the pstore subsystem to expose each partition in NVRAM as a
separate file in /dev/pstore. For instance Oops messages will stored in a
file named [dmesg-nvram-2].

Changes from v1:
- Reduce #ifdefs by and remove forward declarations of pstore callbacks
- Handle return value of nvram_write_os_partition
- Remove empty pstore callbacks and register pstore only when pstore
is configured

---

Aruna Balakrishnaiah (8):
powerpc/pseries: Remove syslog prefix in uncompressed oops text
powerpc/pseries: Add version and timestamp to oops header
powerpc/pseries: Introduce generic read function to read nvram-partitions
powerpc/pseries: Read/Write oops nvram partition via pstore
powerpc/pseries: Read rtas partition via pstore
powerpc/pseries: Distinguish between a os-partition and non-os partition
powerpc/pseries: Read of-config partition via pstore
powerpc/pseries: Read common partition via pstore


arch/powerpc/platforms/pseries/nvram.c | 353 +++++++++++++++++++++++++++-----
fs/pstore/inode.c | 9 +
include/linux/pstore.h | 4
3 files changed, 313 insertions(+), 53 deletions(-)

--


2013-04-24 06:20:14

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 1/8] powerpc/pseries: Remove syslog prefix in uncompressed oops text

Removal of syslog prefix in the uncompressed oops text will
help in capturing more oops data.

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 8733a86..e54a8b7 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -619,7 +619,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
}
if (rc != 0) {
kmsg_dump_rewind(dumper);
- kmsg_dump_get_buffer(dumper, true,
+ kmsg_dump_get_buffer(dumper, false,
oops_data, oops_data_sz, &text_len);
err_type = ERR_TYPE_KERNEL_PANIC;
*oops_len = (u16) text_len;

2013-04-24 06:20:53

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 4/8] powerpc/pseries: Read/Write oops nvram partition via pstore

IBM's p series machines provide persistent storage for LPARs through NVRAM.
NVRAM's lnx,oops-log partition is used to log oops messages.
Currently the kernel provides the contents of p-series NVRAM only as a
simple stream of bytes via /dev/nvram, which must be interpreted in user
space by the nvram command in the powerpc-utils package.

This patch set exploits the pstore subsystem to expose oops partition in
NVRAM as a separate file in /dev/pstore. For instance, Oops messages will be
stored in a file named [dmesg-nvram-2]. In case pstore registration fails it
will fall back to kmsg_dump mechanism.

This patch will read/write the oops messages from/to this partition via pstore.

Signed-off-by: Jim Keniston <[email protected]>
Signed-off-by: Aruna Balakrishnaiah <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 172 +++++++++++++++++++++++++++++---
1 file changed, 157 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 088f023..9edec8e 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -18,6 +18,7 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/kmsg_dump.h>
+#include <linux/pstore.h>
#include <linux/ctype.h>
#include <linux/zlib.h>
#include <asm/uaccess.h>
@@ -127,6 +128,14 @@ static size_t oops_data_sz;
#define MEM_LEVEL 4
static struct z_stream_s stream;

+#ifdef CONFIG_PSTORE
+static enum pstore_type_id nvram_type_ids[] = {
+ PSTORE_TYPE_DMESG,
+ -1
+};
+static int read_type;
+#endif
+
static ssize_t pSeries_nvram_read(char *buf, size_t count, loff_t *index)
{
unsigned int i;
@@ -430,6 +439,149 @@ static int __init pseries_nvram_init_os_partition(struct nvram_os_partition
return 0;
}

+/*
+ * Are we using the ibm,rtas-log for oops/panic reports? And if so,
+ * would logging this oops/panic overwrite an RTAS event that rtas_errd
+ * hasn't had a chance to read and process? Return 1 if so, else 0.
+ *
+ * We assume that if rtas_errd hasn't read the RTAS event in
+ * NVRAM_RTAS_READ_TIMEOUT seconds, it's probably not going to.
+ */
+static int clobbering_unread_rtas_event(void)
+{
+ return (oops_log_partition.index == rtas_log_partition.index
+ && last_unread_rtas_event
+ && get_seconds() - last_unread_rtas_event <=
+ NVRAM_RTAS_READ_TIMEOUT);
+}
+
+#ifdef CONFIG_PSTORE
+static int nvram_pstore_open(struct pstore_info *psi)
+{
+ /* Reset the iterator to start reading partitions again */
+ read_type = -1;
+ return 0;
+}
+
+/**
+ * nvram_pstore_write - pstore write callback for nvram
+ * @type: Type of message logged
+ * @reason: reason behind dump (oops/panic)
+ * @id: identifier to indicate the write performed
+ * @part: pstore writes data to registered buffer in parts,
+ * part number will indicate the same.
+ * @count: Indicates oops count
+ * @size: number of bytes written to the registered buffer
+ * @psi: registered pstore_info structure
+ *
+ * Called by pstore_dump() when an oops or panic report is logged in the
+ * printk buffer.
+ * Returns 0 on successful write.
+ */
+static int nvram_pstore_write(enum pstore_type_id type,
+ enum kmsg_dump_reason reason,
+ u64 *id, unsigned int part, int count,
+ size_t size, struct pstore_info *psi)
+{
+ int rc;
+ struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
+
+ /* part 1 has the recent messages from printk buffer */
+ if (part > 1 || type != PSTORE_TYPE_DMESG ||
+ clobbering_unread_rtas_event())
+ return -1;
+
+ oops_hdr->version = OOPS_HDR_VERSION;
+ oops_hdr->report_length = (u16) size;
+ oops_hdr->timestamp = get_seconds();
+ rc = nvram_write_os_partition(&oops_log_partition, oops_buf,
+ (int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
+ count);
+
+ if (rc != 0)
+ return rc;
+
+ *id = part;
+ return 0;
+}
+
+/*
+ * Reads the oops/panic report.
+ * Returns the length of the data we read from each partition.
+ * Returns 0 if we've been called before.
+ */
+static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
+ int *count, struct timespec *time, char **buf,
+ struct pstore_info *psi)
+{
+ struct oops_log_info *oops_hdr;
+ unsigned int err_type, id_no;
+ struct nvram_os_partition *part = NULL;
+ char *buff = NULL;
+
+ read_type++;
+
+ switch (nvram_type_ids[read_type]) {
+ case PSTORE_TYPE_DMESG:
+ part = &oops_log_partition;
+ *type = PSTORE_TYPE_DMESG;
+ break;
+ default:
+ return 0;
+ }
+
+ buff = kmalloc(part->size, GFP_KERNEL);
+
+ if (!buff)
+ return -ENOMEM;
+
+ if (nvram_read_partition(part, buff, part->size, &err_type, &id_no)) {
+ kfree(buff);
+ return 0;
+ }
+
+ *count = 0;
+ *id = id_no;
+ oops_hdr = (struct oops_log_info *)buff;
+ *buf = buff + sizeof(*oops_hdr);
+ time->tv_sec = oops_hdr->timestamp;
+ time->tv_nsec = 0;
+ return oops_hdr->report_length;
+}
+
+static struct pstore_info nvram_pstore_info = {
+ .owner = THIS_MODULE,
+ .name = "nvram",
+ .open = nvram_pstore_open,
+ .read = nvram_pstore_read,
+ .write = nvram_pstore_write,
+};
+
+static int nvram_pstore_init(void)
+{
+ int rc = 0;
+
+ nvram_pstore_info.buf = oops_data;
+ nvram_pstore_info.bufsize = oops_data_sz;
+
+ rc = pstore_register(&nvram_pstore_info);
+ if (rc != 0)
+ pr_err("nvram: pstore_register() failed, defaults to "
+ "kmsg_dump; returned %d\n", rc);
+ else
+ /*TODO: Support compression when pstore is configured */
+ pr_info("nvram: Compression of oops text supported only when "
+ "pstore is not configured");
+
+ return rc;
+}
+#else
+static int nvram_pstore_init(void)
+{
+ return -1;
+}
+#endif
+
static void __init nvram_init_oops_partition(int rtas_partition_exists)
{
int rc;
@@ -453,6 +605,11 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
oops_data = oops_buf + sizeof(struct oops_log_info);
oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);

+ rc = nvram_pstore_init();
+
+ if (!rc)
+ return;
+
/*
* Figure compression (preceded by elimination of each line's <n>
* severity prefix) will reduce the oops/panic report to at most
@@ -525,21 +682,6 @@ int __init pSeries_nvram_init(void)
return 0;
}

-/*
- * Are we using the ibm,rtas-log for oops/panic reports? And if so,
- * would logging this oops/panic overwrite an RTAS event that rtas_errd
- * hasn't had a chance to read and process? Return 1 if so, else 0.
- *
- * We assume that if rtas_errd hasn't read the RTAS event in
- * NVRAM_RTAS_READ_TIMEOUT seconds, it's probably not going to.
- */
-static int clobbering_unread_rtas_event(void)
-{
- return (oops_log_partition.index == rtas_log_partition.index
- && last_unread_rtas_event
- && get_seconds() - last_unread_rtas_event <=
- NVRAM_RTAS_READ_TIMEOUT);
-}

/* Derived from logfs_compress() */
static int nvram_compress(const void *in, void *out, size_t inlen,

2013-04-24 06:21:04

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 5/8] powerpc/pseries: Read rtas partition via pstore

This patch set exploits the pstore subsystem to read details of rtas partition
in NVRAM to a separate file in /dev/pstore. For instance, rtas details will be
stored in a file named [rtas-nvram-4].

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 33 +++++++++++++++++++++++++-------
fs/pstore/inode.c | 3 +++
include/linux/pstore.h | 2 ++
3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 9edec8e..8a7eefb 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -131,9 +131,11 @@ static struct z_stream_s stream;
#ifdef CONFIG_PSTORE
static enum pstore_type_id nvram_type_ids[] = {
PSTORE_TYPE_DMESG,
+ PSTORE_TYPE_RTAS,
-1
};
static int read_type;
+static unsigned long last_rtas_event;
#endif

static ssize_t pSeries_nvram_read(char *buf, size_t count, loff_t *index)
@@ -297,8 +299,13 @@ int nvram_write_error_log(char * buff, int length,
{
int rc = nvram_write_os_partition(&rtas_log_partition, buff, length,
err_type, error_log_cnt);
- if (!rc)
+ if (!rc) {
last_unread_rtas_event = get_seconds();
+#ifdef CONFIG_PSTORE
+ last_rtas_event = get_seconds();
+#endif
+ }
+
return rc;
}

@@ -506,7 +513,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
}

/*
- * Reads the oops/panic report.
+ * Reads the oops/panic report and ibm,rtas-log partition.
* Returns the length of the data we read from each partition.
* Returns 0 if we've been called before.
*/
@@ -526,6 +533,12 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
part = &oops_log_partition;
*type = PSTORE_TYPE_DMESG;
break;
+ case PSTORE_TYPE_RTAS:
+ part = &rtas_log_partition;
+ *type = PSTORE_TYPE_RTAS;
+ time->tv_sec = last_rtas_event;
+ time->tv_nsec = 0;
+ break;
default:
return 0;
}
@@ -542,11 +555,17 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,

*count = 0;
*id = id_no;
- oops_hdr = (struct oops_log_info *)buff;
- *buf = buff + sizeof(*oops_hdr);
- time->tv_sec = oops_hdr->timestamp;
- time->tv_nsec = 0;
- return oops_hdr->report_length;
+
+ if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
+ oops_hdr = (struct oops_log_info *)buff;
+ *buf = buff + sizeof(*oops_hdr);
+ time->tv_sec = oops_hdr->timestamp;
+ time->tv_nsec = 0;
+ return oops_hdr->report_length;
+ }
+
+ *buf = buff;
+ return part->size;
}

static struct pstore_info nvram_pstore_info = {
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index e4bcb2c..ec24f9c 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -324,6 +324,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
case PSTORE_TYPE_MCE:
sprintf(name, "mce-%s-%lld", psname, id);
break;
+ case PSTORE_TYPE_PPC_RTAS:
+ sprintf(name, "rtas-%s-%lld", psname, id);
+ break;
case PSTORE_TYPE_UNKNOWN:
sprintf(name, "unknown-%s-%lld", psname, id);
break;
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 75d0176..d7a8fe9 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -35,6 +35,8 @@ enum pstore_type_id {
PSTORE_TYPE_MCE = 1,
PSTORE_TYPE_CONSOLE = 2,
PSTORE_TYPE_FTRACE = 3,
+ /* PPC64 partition types */
+ PSTORE_TYPE_PPC_RTAS = 4,
PSTORE_TYPE_UNKNOWN = 255
};

2013-04-24 06:21:17

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 6/8] powerpc/pseries: Distinguish between a os-partition and non-os partition

Introduce os_partition member in nvram_os_partition structure to identify
if the partition is an os partition or not. This will be useful to handle
non-os partitions of-config and common.

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 8a7eefb..b118382 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -53,20 +53,23 @@ struct nvram_os_partition {
int min_size; /* minimum acceptable size (0 means req_size) */
long size; /* size of data portion (excluding err_log_info) */
long index; /* offset of data portion of partition */
+ bool os_partition; /* partition initialized by OS, not FW */
};

static struct nvram_os_partition rtas_log_partition = {
.name = "ibm,rtas-log",
.req_size = 2079,
.min_size = 1055,
- .index = -1
+ .index = -1,
+ .os_partition = true
};

static struct nvram_os_partition oops_log_partition = {
.name = "lnx,oops-log",
.req_size = 4000,
.min_size = 2000,
- .index = -1
+ .index = -1,
+ .os_partition = true
};

static const char *pseries_nvram_os_partitions[] = {

2013-04-24 06:21:37

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 7/8] powerpc/pseries: Read of-config partition via pstore

This patch set exploits the pstore subsystem to read details of
of-config partition in NVRAM to a separate file in /dev/pstore.
For instance, of-config partition details will be stored in a
file named [of-nvram-5].

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 55 +++++++++++++++++++++++++++-----
fs/pstore/inode.c | 3 ++
include/linux/pstore.h | 1 +
3 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index b118382..de448af 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -132,9 +132,16 @@ static size_t oops_data_sz;
static struct z_stream_s stream;

#ifdef CONFIG_PSTORE
+static struct nvram_os_partition of_config_partition = {
+ .name = "of-config",
+ .index = -1,
+ .os_partition = false
+};
+
static enum pstore_type_id nvram_type_ids[] = {
PSTORE_TYPE_DMESG,
PSTORE_TYPE_RTAS,
+ PSTORE_TYPE_OF,
-1
};
static int read_type;
@@ -332,10 +339,15 @@ int nvram_read_partition(struct nvram_os_partition *part, char *buff,

tmp_index = part->index;

- rc = ppc_md.nvram_read((char *)&info, sizeof(struct err_log_info), &tmp_index);
- if (rc <= 0) {
- pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__, rc);
- return rc;
+ if (part->os_partition) {
+ rc = ppc_md.nvram_read((char *)&info,
+ sizeof(struct err_log_info),
+ &tmp_index);
+ if (rc <= 0) {
+ pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__,
+ rc);
+ return rc;
+ }
}

rc = ppc_md.nvram_read(buff, length, &tmp_index);
@@ -344,8 +356,10 @@ int nvram_read_partition(struct nvram_os_partition *part, char *buff,
return rc;
}

- *error_log_cnt = info.seq_num;
- *err_type = info.error_type;
+ if (part->os_partition) {
+ *error_log_cnt = info.seq_num;
+ *err_type = info.error_type;
+ }

return 0;
}
@@ -516,7 +530,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
}

/*
- * Reads the oops/panic report and ibm,rtas-log partition.
+ * Reads the oops/panic report, rtas and of-config partition.
* Returns the length of the data we read from each partition.
* Returns 0 if we've been called before.
*/
@@ -525,9 +539,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
struct pstore_info *psi)
{
struct oops_log_info *oops_hdr;
- unsigned int err_type, id_no;
+ unsigned int err_type, id_no, size = 0;
struct nvram_os_partition *part = NULL;
char *buff = NULL;
+ int sig = 0;
+ loff_t p;

read_type++;

@@ -542,10 +558,29 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
time->tv_sec = last_rtas_event;
time->tv_nsec = 0;
break;
+ case PSTORE_TYPE_OF:
+ sig = NVRAM_SIG_OF;
+ part = &of_config_partition;
+ *type = PSTORE_TYPE_OF;
+ *id = PSTORE_TYPE_OF;
+ time->tv_sec = 0;
+ time->tv_nsec = 0;
+ break;
default:
return 0;
}

+ if (!part->os_partition) {
+ p = nvram_find_partition(part->name, sig, &size);
+ if (p <= 0) {
+ pr_err("nvram: Failed to find partition %s, "
+ "err %d\n", part->name, (int)p);
+ return 0;
+ }
+ part->index = p;
+ part->size = size;
+ }
+
buff = kmalloc(part->size, GFP_KERNEL);

if (!buff)
@@ -557,7 +592,9 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
}

*count = 0;
- *id = id_no;
+
+ if (part->os_partition)
+ *id = id_no;

if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
oops_hdr = (struct oops_log_info *)buff;
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index ec24f9c..8d4fb65 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -327,6 +327,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
case PSTORE_TYPE_PPC_RTAS:
sprintf(name, "rtas-%s-%lld", psname, id);
break;
+ case PSTORE_TYPE_PPC_OF:
+ sprintf(name, "of-%s-%lld", psname, id);
+ break;
case PSTORE_TYPE_UNKNOWN:
sprintf(name, "unknown-%s-%lld", psname, id);
break;
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index d7a8fe9..615dc18 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -37,6 +37,7 @@ enum pstore_type_id {
PSTORE_TYPE_FTRACE = 3,
/* PPC64 partition types */
PSTORE_TYPE_PPC_RTAS = 4,
+ PSTORE_TYPE_PPC_OF = 5,
PSTORE_TYPE_UNKNOWN = 255
};

2013-04-24 06:20:50

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 3/8] powerpc/pseries: Introduce generic read function to read nvram-partitions

Introduce generic read function to read nvram partitions other than rtas.
nvram_read_error_log will be retained which is used to read rtas partition
from rtasd. nvram_read_partition is the generic read function to read from
any nvram partition.

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index 742735a..088f023 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -293,34 +293,35 @@ int nvram_write_error_log(char * buff, int length,
return rc;
}

-/* nvram_read_error_log
+/* nvram_read_partition
*
- * Reads nvram for error log for at most 'length'
+ * Reads nvram partition for at most 'length'
*/
-int nvram_read_error_log(char * buff, int length,
- unsigned int * err_type, unsigned int * error_log_cnt)
+int nvram_read_partition(struct nvram_os_partition *part, char *buff,
+ int length, unsigned int *err_type,
+ unsigned int *error_log_cnt)
{
int rc;
loff_t tmp_index;
struct err_log_info info;

- if (rtas_log_partition.index == -1)
+ if (part->index == -1)
return -1;

- if (length > rtas_log_partition.size)
- length = rtas_log_partition.size;
+ if (length > part->size)
+ length = part->size;

- tmp_index = rtas_log_partition.index;
+ tmp_index = part->index;

rc = ppc_md.nvram_read((char *)&info, sizeof(struct err_log_info), &tmp_index);
if (rc <= 0) {
- printk(KERN_ERR "nvram_read_error_log: Failed nvram_read (%d)\n", rc);
+ pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__, rc);
return rc;
}

rc = ppc_md.nvram_read(buff, length, &tmp_index);
if (rc <= 0) {
- printk(KERN_ERR "nvram_read_error_log: Failed nvram_read (%d)\n", rc);
+ pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__, rc);
return rc;
}

@@ -330,6 +331,17 @@ int nvram_read_error_log(char * buff, int length,
return 0;
}

+/* nvram_read_error_log
+ *
+ * Reads nvram for error log for at most 'length'
+ */
+int nvram_read_error_log(char *buff, int length,
+ unsigned int *err_type, unsigned int *error_log_cnt)
+{
+ return nvram_read_partition(&rtas_log_partition, buff, length,
+ err_type, error_log_cnt);
+}
+
/* This doesn't actually zero anything, but it sets the event_logged
* word to tell that this event is safely in syslog.
*/

2013-04-24 06:21:59

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 8/8] powerpc/pseries: Read common partition via pstore

This patch exploits pstore subsystem to read details of common partition
in NVRAM to a separate file in /dev/pstore. For instance, common partition
details will be stored in a file named [common-nvram-6].

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 17 ++++++++++++++++-
fs/pstore/inode.c | 3 +++
include/linux/pstore.h | 1 +
3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index de448af..8417816 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -138,10 +138,17 @@ static struct nvram_os_partition of_config_partition = {
.os_partition = false
};

+static struct nvram_os_partition common_partition = {
+ .name = "common",
+ .index = -1,
+ .os_partition = false
+};
+
static enum pstore_type_id nvram_type_ids[] = {
PSTORE_TYPE_DMESG,
PSTORE_TYPE_RTAS,
PSTORE_TYPE_OF,
+ PSTORE_TYPE_COMMON,
-1
};
static int read_type;
@@ -530,7 +537,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
}

/*
- * Reads the oops/panic report, rtas and of-config partition.
+ * Reads the oops/panic report, rtas, of-config and common partition.
* Returns the length of the data we read from each partition.
* Returns 0 if we've been called before.
*/
@@ -566,6 +573,14 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
time->tv_sec = 0;
time->tv_nsec = 0;
break;
+ case PSTORE_TYPE_COMMON:
+ sig = NVRAM_SIG_SYS;
+ part = &common_partition;
+ *type = PSTORE_TYPE_COMMON;
+ *id = PSTORE_TYPE_COMMON;
+ time->tv_sec = 0;
+ time->tv_nsec = 0;
+ break;
default:
return 0;
}
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 8d4fb65..88cc050 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -330,6 +330,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
case PSTORE_TYPE_PPC_OF:
sprintf(name, "of-%s-%lld", psname, id);
break;
+ case PSTORE_TYPE_PPC_COMMON:
+ sprintf(name, "common-%s-%lld", psname, id);
+ break;
case PSTORE_TYPE_UNKNOWN:
sprintf(name, "unknown-%s-%lld", psname, id);
break;
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 615dc18..656699f 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -38,6 +38,7 @@ enum pstore_type_id {
/* PPC64 partition types */
PSTORE_TYPE_PPC_RTAS = 4,
PSTORE_TYPE_PPC_OF = 5,
+ PSTORE_TYPE_PPC_COMMON = 6,
PSTORE_TYPE_UNKNOWN = 255
};

2013-04-24 06:20:46

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: [PATCH v2 2/8] powerpc/pseries: Add version and timestamp to oops header

Introduce version and timestamp information in the oops header.
oops_log_info (oops header) holds version (to distinguish between old
and new format oops header), length of the oops text
(compressed or uncompressed) and timestamp.

The version field will sit in the same place as the length in old
headers. version is assigned 5000 (greater than oops partition size)
so that existing tools will refuse to dump new style partitions as
the length is too large. The updated tools will work with both
old and new format headers.

Signed-off-by: Aruna Balakrishnaiah <[email protected]>
Reviewed-by: Jim Keniston <[email protected]>
---
arch/powerpc/platforms/pseries/nvram.c | 57 +++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
index e54a8b7..742735a 100644
--- a/arch/powerpc/platforms/pseries/nvram.c
+++ b/arch/powerpc/platforms/pseries/nvram.c
@@ -29,6 +29,13 @@
/* Max bytes to read/write in one go */
#define NVRW_CNT 0x20

+/*
+ * Set oops header version to distingush between old and new format header.
+ * lnx,oops-log partition max size is 4000, header version > 4000 will
+ * help in identifying new header.
+ */
+#define OOPS_HDR_VERSION 5000
+
static unsigned int nvram_size;
static int nvram_fetch, nvram_store;
static char nvram_buf[NVRW_CNT]; /* assume this is in the first 4GB */
@@ -67,6 +74,12 @@ static const char *pseries_nvram_os_partitions[] = {
NULL
};

+struct oops_log_info {
+ u16 version;
+ u16 report_length;
+ u64 timestamp;
+} __attribute__((packed));
+
static void oops_to_nvram(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason);

@@ -83,28 +96,28 @@ static unsigned long last_unread_rtas_event; /* timestamp */

* big_oops_buf[] holds the uncompressed text we're capturing.
*
- * oops_buf[] holds the compressed text, preceded by a prefix.
- * The prefix is just a u16 holding the length of the compressed* text.
- * (*Or uncompressed, if compression fails.) oops_buf[] gets written
- * to NVRAM.
+ * oops_buf[] holds the compressed text, preceded by a oops header.
+ * oops header has u16 holding the version of oops header (to differentiate
+ * between old and new format header) followed by u16 holding the length of
+ * the compressed* text (*Or uncompressed, if compression fails.) and u64
+ * holding the timestamp. oops_buf[] gets written to NVRAM.
*
- * oops_len points to the prefix. oops_data points to the compressed text.
+ * oops_log_info points to the header. oops_data points to the compressed text.
*
* +- oops_buf
- * | +- oops_data
- * v v
- * +------------+-----------------------------------------------+
- * | length | text |
- * | (2 bytes) | (oops_data_sz bytes) |
- * +------------+-----------------------------------------------+
+ * | +- oops_data
+ * v v
+ * +-----------+-----------+-----------+------------------------+
+ * | version | length | timestamp | text |
+ * | (2 bytes) | (2 bytes) | (8 bytes) | (oops_data_sz bytes) |
+ * +-----------+-----------+-----------+------------------------+
* ^
- * +- oops_len
+ * +- oops_log_info
*
* We preallocate these buffers during init to avoid kmalloc during oops/panic.
*/
static size_t big_oops_buf_sz;
static char *big_oops_buf, *oops_buf;
-static u16 *oops_len;
static char *oops_data;
static size_t oops_data_sz;

@@ -425,9 +438,8 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
oops_log_partition.name);
return;
}
- oops_len = (u16*) oops_buf;
- oops_data = oops_buf + sizeof(u16);
- oops_data_sz = oops_log_partition.size - sizeof(u16);
+ oops_data = oops_buf + sizeof(struct oops_log_info);
+ oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);

/*
* Figure compression (preceded by elimination of each line's <n>
@@ -555,6 +567,7 @@ error:
/* Compress the text from big_oops_buf into oops_buf. */
static int zip_oops(size_t text_len)
{
+ struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
int zipped_len = nvram_compress(big_oops_buf, oops_data, text_len,
oops_data_sz);
if (zipped_len < 0) {
@@ -562,7 +575,9 @@ static int zip_oops(size_t text_len)
pr_err("nvram: logging uncompressed oops/panic report\n");
return -1;
}
- *oops_len = (u16) zipped_len;
+ oops_hdr->version = OOPS_HDR_VERSION;
+ oops_hdr->report_length = (u16) zipped_len;
+ oops_hdr->timestamp = get_seconds();
return 0;
}

@@ -576,6 +591,7 @@ static int zip_oops(size_t text_len)
static void oops_to_nvram(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason)
{
+ struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
static unsigned int oops_count = 0;
static bool panicking = false;
static DEFINE_SPINLOCK(lock);
@@ -622,11 +638,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
kmsg_dump_get_buffer(dumper, false,
oops_data, oops_data_sz, &text_len);
err_type = ERR_TYPE_KERNEL_PANIC;
- *oops_len = (u16) text_len;
+ oops_hdr->version = OOPS_HDR_VERSION;
+ oops_hdr->report_length = (u16) text_len;
+ oops_hdr->timestamp = get_seconds();
}

(void) nvram_write_os_partition(&oops_log_partition, oops_buf,
- (int) (sizeof(*oops_len) + *oops_len), err_type, ++oops_count);
+ (int) (sizeof(*oops_hdr) + oops_hdr->report_length), err_type,
+ ++oops_count);

spin_unlock_irqrestore(&lock, flags);
}

2013-04-24 20:43:56

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] powerpc/pseries: Read of-config partition via pstore

On Tue, Apr 23, 2013 at 11:20 PM, Aruna Balakrishnaiah
<[email protected]> wrote:
> This patch set exploits the pstore subsystem to read details of
> of-config partition in NVRAM to a separate file in /dev/pstore.
> For instance, of-config partition details will be stored in a
> file named [of-nvram-5].
>
> Signed-off-by: Aruna Balakrishnaiah <[email protected]>
> Reviewed-by: Jim Keniston <[email protected]>
> ---
> arch/powerpc/platforms/pseries/nvram.c | 55 +++++++++++++++++++++++++++-----
> fs/pstore/inode.c | 3 ++
> include/linux/pstore.h | 1 +
> 3 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index b118382..de448af 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -132,9 +132,16 @@ static size_t oops_data_sz;
> static struct z_stream_s stream;
>
> #ifdef CONFIG_PSTORE
> +static struct nvram_os_partition of_config_partition = {
> + .name = "of-config",
> + .index = -1,
> + .os_partition = false
> +};
> +
> static enum pstore_type_id nvram_type_ids[] = {
> PSTORE_TYPE_DMESG,
> PSTORE_TYPE_RTAS,
> + PSTORE_TYPE_OF,
> -1
> };
> static int read_type;
> @@ -332,10 +339,15 @@ int nvram_read_partition(struct nvram_os_partition *part, char *buff,
>
> tmp_index = part->index;
>
> - rc = ppc_md.nvram_read((char *)&info, sizeof(struct err_log_info), &tmp_index);
> - if (rc <= 0) {
> - pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__, rc);
> - return rc;
> + if (part->os_partition) {
> + rc = ppc_md.nvram_read((char *)&info,
> + sizeof(struct err_log_info),
> + &tmp_index);
> + if (rc <= 0) {
> + pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__,
> + rc);
> + return rc;
> + }
> }
>
> rc = ppc_md.nvram_read(buff, length, &tmp_index);
> @@ -344,8 +356,10 @@ int nvram_read_partition(struct nvram_os_partition *part, char *buff,
> return rc;
> }
>
> - *error_log_cnt = info.seq_num;
> - *err_type = info.error_type;
> + if (part->os_partition) {
> + *error_log_cnt = info.seq_num;
> + *err_type = info.error_type;
> + }
>
> return 0;
> }
> @@ -516,7 +530,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
> }
>
> /*
> - * Reads the oops/panic report and ibm,rtas-log partition.
> + * Reads the oops/panic report, rtas and of-config partition.
> * Returns the length of the data we read from each partition.
> * Returns 0 if we've been called before.
> */
> @@ -525,9 +539,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> struct pstore_info *psi)
> {
> struct oops_log_info *oops_hdr;
> - unsigned int err_type, id_no;
> + unsigned int err_type, id_no, size = 0;
> struct nvram_os_partition *part = NULL;
> char *buff = NULL;
> + int sig = 0;
> + loff_t p;
>
> read_type++;
>
> @@ -542,10 +558,29 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> time->tv_sec = last_rtas_event;
> time->tv_nsec = 0;
> break;
> + case PSTORE_TYPE_OF:
> + sig = NVRAM_SIG_OF;
> + part = &of_config_partition;
> + *type = PSTORE_TYPE_OF;
> + *id = PSTORE_TYPE_OF;
> + time->tv_sec = 0;
> + time->tv_nsec = 0;
> + break;
> default:
> return 0;
> }
>
> + if (!part->os_partition) {
> + p = nvram_find_partition(part->name, sig, &size);
> + if (p <= 0) {
> + pr_err("nvram: Failed to find partition %s, "
> + "err %d\n", part->name, (int)p);
> + return 0;
> + }
> + part->index = p;
> + part->size = size;
> + }
> +
> buff = kmalloc(part->size, GFP_KERNEL);
>
> if (!buff)
> @@ -557,7 +592,9 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> }
>
> *count = 0;
> - *id = id_no;
> +
> + if (part->os_partition)
> + *id = id_no;
>
> if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
> oops_hdr = (struct oops_log_info *)buff;
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index ec24f9c..8d4fb65 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -327,6 +327,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
> case PSTORE_TYPE_PPC_RTAS:
> sprintf(name, "rtas-%s-%lld", psname, id);
> break;
> + case PSTORE_TYPE_PPC_OF:
> + sprintf(name, "of-%s-%lld", psname, id);
> + break;
> case PSTORE_TYPE_UNKNOWN:
> sprintf(name, "unknown-%s-%lld", psname, id);
> break;
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index d7a8fe9..615dc18 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -37,6 +37,7 @@ enum pstore_type_id {
> PSTORE_TYPE_FTRACE = 3,
> /* PPC64 partition types */
> PSTORE_TYPE_PPC_RTAS = 4,
> + PSTORE_TYPE_PPC_OF = 5,
> PSTORE_TYPE_UNKNOWN = 255
> };
>
>

Should this be named just "PSTORE_TYPE_OF" instead of "...PPC_OF"?

-Kees

--
Kees Cook
Chrome OS Security

2013-04-24 20:45:10

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] powerpc/pseries: Nvram-to-pstore

On Tue, Apr 23, 2013 at 11:19 PM, Aruna Balakrishnaiah
<[email protected]> wrote:
> Currently the kernel provides the contents of p-series NVRAM only as a
> simple stream of bytes via /dev/nvram, which must be interpreted in user
> space by the nvram command in the powerpc-utils package. This patch set
> exploits the pstore subsystem to expose each partition in NVRAM as a
> separate file in /dev/pstore. For instance Oops messages will stored in a
> file named [dmesg-nvram-2].
>
> Changes from v1:
> - Reduce #ifdefs by and remove forward declarations of pstore callbacks
> - Handle return value of nvram_write_os_partition
> - Remove empty pstore callbacks and register pstore only when pstore
> is configured
>
> ---
>
> Aruna Balakrishnaiah (8):
> powerpc/pseries: Remove syslog prefix in uncompressed oops text
> powerpc/pseries: Add version and timestamp to oops header
> powerpc/pseries: Introduce generic read function to read nvram-partitions
> powerpc/pseries: Read/Write oops nvram partition via pstore
> powerpc/pseries: Read rtas partition via pstore
> powerpc/pseries: Distinguish between a os-partition and non-os partition
> powerpc/pseries: Read of-config partition via pstore
> powerpc/pseries: Read common partition via pstore
>
>
> arch/powerpc/platforms/pseries/nvram.c | 353 +++++++++++++++++++++++++++-----
> fs/pstore/inode.c | 9 +
> include/linux/pstore.h | 4
> 3 files changed, 313 insertions(+), 53 deletions(-)

This series looks good! Other than the naming conventions (are these
new pstore types really PPC-only?) I think it's a fine addition.

Thanks!

-Kees

--
Kees Cook
Chrome OS Security

2013-04-25 05:10:30

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] powerpc/pseries: Read of-config partition via pstore

On Thursday 25 April 2013 02:13 AM, Kees Cook wrote:

Hi Kees,

> On Tue, Apr 23, 2013 at 11:20 PM, Aruna Balakrishnaiah
> <[email protected]> wrote:
>> This patch set exploits the pstore subsystem to read details of
>> of-config partition in NVRAM to a separate file in /dev/pstore.
>> For instance, of-config partition details will be stored in a
>> file named [of-nvram-5].
>>
>> Signed-off-by: Aruna Balakrishnaiah <[email protected]>
>> Reviewed-by: Jim Keniston <[email protected]>
>> ---
>> arch/powerpc/platforms/pseries/nvram.c | 55 +++++++++++++++++++++++++++-----
>> fs/pstore/inode.c | 3 ++
>> include/linux/pstore.h | 1 +
>> 3 files changed, 50 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
>> index b118382..de448af 100644
>> --- a/arch/powerpc/platforms/pseries/nvram.c
>> +++ b/arch/powerpc/platforms/pseries/nvram.c
>> @@ -132,9 +132,16 @@ static size_t oops_data_sz;
>> static struct z_stream_s stream;
>>
>> #ifdef CONFIG_PSTORE
>> +static struct nvram_os_partition of_config_partition = {
>> + .name = "of-config",
>> + .index = -1,
>> + .os_partition = false
>> +};
>> +
>> static enum pstore_type_id nvram_type_ids[] = {
>> PSTORE_TYPE_DMESG,
>> PSTORE_TYPE_RTAS,
>> + PSTORE_TYPE_OF,
>> -1
>> };
>> static int read_type;
>> @@ -332,10 +339,15 @@ int nvram_read_partition(struct nvram_os_partition *part, char *buff,
>>
>> tmp_index = part->index;
>>
>> - rc = ppc_md.nvram_read((char *)&info, sizeof(struct err_log_info), &tmp_index);
>> - if (rc <= 0) {
>> - pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__, rc);
>> - return rc;
>> + if (part->os_partition) {
>> + rc = ppc_md.nvram_read((char *)&info,
>> + sizeof(struct err_log_info),
>> + &tmp_index);
>> + if (rc <= 0) {
>> + pr_err("%s: Failed nvram_read (%d)\n", __FUNCTION__,
>> + rc);
>> + return rc;
>> + }
>> }
>>
>> rc = ppc_md.nvram_read(buff, length, &tmp_index);
>> @@ -344,8 +356,10 @@ int nvram_read_partition(struct nvram_os_partition *part, char *buff,
>> return rc;
>> }
>>
>> - *error_log_cnt = info.seq_num;
>> - *err_type = info.error_type;
>> + if (part->os_partition) {
>> + *error_log_cnt = info.seq_num;
>> + *err_type = info.error_type;
>> + }
>>
>> return 0;
>> }
>> @@ -516,7 +530,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
>> }
>>
>> /*
>> - * Reads the oops/panic report and ibm,rtas-log partition.
>> + * Reads the oops/panic report, rtas and of-config partition.
>> * Returns the length of the data we read from each partition.
>> * Returns 0 if we've been called before.
>> */
>> @@ -525,9 +539,11 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>> struct pstore_info *psi)
>> {
>> struct oops_log_info *oops_hdr;
>> - unsigned int err_type, id_no;
>> + unsigned int err_type, id_no, size = 0;
>> struct nvram_os_partition *part = NULL;
>> char *buff = NULL;
>> + int sig = 0;
>> + loff_t p;
>>
>> read_type++;
>>
>> @@ -542,10 +558,29 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>> time->tv_sec = last_rtas_event;
>> time->tv_nsec = 0;
>> break;
>> + case PSTORE_TYPE_OF:
>> + sig = NVRAM_SIG_OF;
>> + part = &of_config_partition;
>> + *type = PSTORE_TYPE_OF;
>> + *id = PSTORE_TYPE_OF;
>> + time->tv_sec = 0;
>> + time->tv_nsec = 0;
>> + break;
>> default:
>> return 0;
>> }
>>
>> + if (!part->os_partition) {
>> + p = nvram_find_partition(part->name, sig, &size);
>> + if (p <= 0) {
>> + pr_err("nvram: Failed to find partition %s, "
>> + "err %d\n", part->name, (int)p);
>> + return 0;
>> + }
>> + part->index = p;
>> + part->size = size;
>> + }
>> +
>> buff = kmalloc(part->size, GFP_KERNEL);
>>
>> if (!buff)
>> @@ -557,7 +592,9 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>> }
>>
>> *count = 0;
>> - *id = id_no;
>> +
>> + if (part->os_partition)
>> + *id = id_no;
>>
>> if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
>> oops_hdr = (struct oops_log_info *)buff;
>> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
>> index ec24f9c..8d4fb65 100644
>> --- a/fs/pstore/inode.c
>> +++ b/fs/pstore/inode.c
>> @@ -327,6 +327,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
>> case PSTORE_TYPE_PPC_RTAS:
>> sprintf(name, "rtas-%s-%lld", psname, id);
>> break;
>> + case PSTORE_TYPE_PPC_OF:
>> + sprintf(name, "of-%s-%lld", psname, id);
>> + break;
>> case PSTORE_TYPE_UNKNOWN:
>> sprintf(name, "unknown-%s-%lld", psname, id);
>> break;
>> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
>> index d7a8fe9..615dc18 100644
>> --- a/include/linux/pstore.h
>> +++ b/include/linux/pstore.h
>> @@ -37,6 +37,7 @@ enum pstore_type_id {
>> PSTORE_TYPE_FTRACE = 3,
>> /* PPC64 partition types */
>> PSTORE_TYPE_PPC_RTAS = 4,
>> + PSTORE_TYPE_PPC_OF = 5,
>> PSTORE_TYPE_UNKNOWN = 255
>> };
>>
>>
> Should this be named just "PSTORE_TYPE_OF" instead of "...PPC_OF"?

I renamed it from PSTORE_TYPE_OF to PSTORE_TYPE_PPC_OF as per Michael's
suggestion. But I have made a mistake of not changing the new name in nvram.c
file.

Will wait for other review comments and repost the patch.

>
> -Kees
>
> --
> Kees Cook
> Chrome OS Security
> _______________________________________________
> Linuxppc-dev mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

2013-04-25 05:53:31

by Aruna Balakrishnaiah

[permalink] [raw]
Subject: Re: [PATCH v2 0/8] powerpc/pseries: Nvram-to-pstore

Hi Kees,

On Thursday 25 April 2013 02:15 AM, Kees Cook wrote:
> On Tue, Apr 23, 2013 at 11:19 PM, Aruna Balakrishnaiah
> <[email protected]> wrote:
>> Currently the kernel provides the contents of p-series NVRAM only as a
>> simple stream of bytes via /dev/nvram, which must be interpreted in user
>> space by the nvram command in the powerpc-utils package. This patch set
>> exploits the pstore subsystem to expose each partition in NVRAM as a
>> separate file in /dev/pstore. For instance Oops messages will stored in a
>> file named [dmesg-nvram-2].
>>
>> Changes from v1:
>> - Reduce #ifdefs by and remove forward declarations of pstore callbacks
>> - Handle return value of nvram_write_os_partition
>> - Remove empty pstore callbacks and register pstore only when pstore
>> is configured
>>
>> ---
>>
>> Aruna Balakrishnaiah (8):
>> powerpc/pseries: Remove syslog prefix in uncompressed oops text
>> powerpc/pseries: Add version and timestamp to oops header
>> powerpc/pseries: Introduce generic read function to read nvram-partitions
>> powerpc/pseries: Read/Write oops nvram partition via pstore
>> powerpc/pseries: Read rtas partition via pstore
>> powerpc/pseries: Distinguish between a os-partition and non-os partition
>> powerpc/pseries: Read of-config partition via pstore
>> powerpc/pseries: Read common partition via pstore
>>
>>
>> arch/powerpc/platforms/pseries/nvram.c | 353 +++++++++++++++++++++++++++-----
>> fs/pstore/inode.c | 9 +
>> include/linux/pstore.h | 4
>> 3 files changed, 313 insertions(+), 53 deletions(-)
> This series looks good! Other than the naming conventions (are these
> new pstore types really PPC-only?) I think it's a fine addition.
>
> Thanks!

The new pstore types are PPC specific. Hence it would be better to have the
(_PPC) in the type ids so that other does not end up using these ids.

> -Kees
>
> --
> Kees Cook
> Chrome OS Security
>