Re: [PATCH 1/2] powerpc/powernv: Add OPAL message log interface
On Mon, 2014-03-31 at 15:08 +1100, Michael Neuling wrote:
+/* OPAL in-memory console. Defined in OPAL source at core/console.c */
+struct memcons {
+ __be64 magic;
+#define MEMCONS_MAGIC 0x6630696567726173L
0x6630696567726173 == f0iegras ... Ben!!! :-P
Yummy ! :-)
+ __be64 obuf_phys;
+ __be64 ibuf_phys;
+ __be32 obuf_size;
+ __be32 ibuf_size;
+ __be32 out_pos;
+#define MEMCONS_OUT_POS_WRAP 0x8000u
+#define MEMCONS_OUT_POS_MASK 0x00ffu
+ __be32 in_prod;
+ __be32 in_cons;
+};
+
+static ssize_t opal_messages_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *to, loff_t pos, size_t count)
+{
+ struct memcons *mc = bin_attr-private;
+ const char *conbuf;
+ bool wrapped;
+ size_t num_read;
+ int out_pos;
+
+ if (!mc)
+ return -ENODEV;
+
+ conbuf = phys_to_virt(be64_to_cpu(mc-obuf_phys));
+ wrapped = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_WRAP;
+ out_pos = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_MASK;
+
Are there ordering issues we need to think about here with reading
these? Can the messages be written on another CPU at the same time as
these are being read?
Good point. out_pos should probably be read only once into a local
variable using the ACCESS_ONCE macro, and then only be broken up.
What happens if in between reading wrapped and out_pos the buffer wraps?
You'd end up getting only a few bytes of console? Maybe you need to
read wrapped before and after out_pos to make should it's not wrapped in
between.
Unlikely but yes, it can happen.
+ if (!wrapped) {
Why the negative case first? Just make it:
if (wrapped) {
wrapped case
} else {
not wrapped case
}
Also, no curlies needed for single statement.
+ num_read = memory_read_from_buffer(to, count, pos, conbuf,
+ out_pos);
This is probably not necessary, but do we need to sanity check out_pos
obuf_size? I guess we don't generally sanity check numbers from OPAL as
it can screw us in many other ways anyway.
On the other hand it doesn't cost much and if the FW goes bonkers it
will give us a better handle to debug from.
+ } else {
+ num_read = memory_read_from_buffer(to, count, pos,
+ conbuf + out_pos,
+ be32_to_cpu(mc-obuf_size) - out_pos);
+
+ if (num_read 0)
+ goto out;
+
+ num_read += memory_read_from_buffer(to + num_read,
+ count - num_read, pos, conbuf,
out_pos);
What if this second read returns an error? num_read += -ERRNO? I think
you need to check this return independently.
Cheers,
Ben.
Mikey
+ }
+out:
+ return num_read;
+}
+
+static struct bin_attribute messages_attr = {
+ .attr = {.name = messages, .mode = 0444},
+ .read = opal_messages_read
+};
+
+void __init opal_messages_init(void)
+{
+ u64 mcaddr;
+ struct memcons *mc;
+
+ if (of_property_read_u64(opal_node, ibm,opal-memcons, mcaddr)) {
+ pr_warn(OPAL: Property ibm,opal-memcons not found, no message
log\n);
+ return;
+ }
+
+ mc = phys_to_virt(mcaddr);
+ if (!mc) {
+ pr_warn(OPAL: memory console address is invalid\n);
+ return;
+ }
+
+ if (be64_to_cpu(mc-magic) != MEMCONS_MAGIC) {
+ pr_warn(OPAL: memory console version is invalid\n);
+ return;
+ }
+
+ messages_attr.private = mc;
+
+ if (sysfs_create_bin_file(opal_kobj, messages_attr) != 0)
+ pr_warn(OPAL: sysfs file creation failed\n);
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c
b/arch/powerpc/platforms/powernv/opal.c
index e92f2f6..2bc032a 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -46,7 +46,7 @@ struct mcheck_recoverable_range {
static struct mcheck_recoverable_range *mc_recoverable_range;
static int mc_recoverable_range_len;
-static struct device_node *opal_node;
+struct device_node *opal_node;
static DEFINE_SPINLOCK(opal_write_lock);
extern u64 opal_mc_secondary_handler[];
static unsigned int *opal_irqs;
@@ -574,6 +574,8 @@ static int __init opal_init(void)
opal_platform_dump_init();
/* Setup system parameters interface */
opal_sys_param_init();
+ /* Setup message log interface. */
+ opal_messages_init();
}
return 0;
--
1.9.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] powerpc/powernv: Add OPAL message log interface
On Mon, Mar 31, 2014 at 10:29 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: + conbuf = phys_to_virt(be64_to_cpu(mc-obuf_phys)); + wrapped = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_WRAP; + out_pos = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_MASK; + Are there ordering issues we need to think about here with reading these? Can the messages be written on another CPU at the same time as these are being read? Good point. out_pos should probably be read only once into a local variable using the ACCESS_ONCE macro, and then only be broken up. I've got a V2 that fixes this and the other issues Mikey pointed out. I found that the log would get corrupted from Linux's point of view once full. Dumping the memory suggests that the contents of the circular buffer is fine, and it's just our pointer (out_pos) that is incorrect. It's not clear why this is happening; I'll do some more testing before sending out the updated patch. Cheers, Joel ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] powerpc/powernv: Add OPAL message log interface
Joel Stanley j...@jms.id.au writes:
OPAL provides an in-memory circular buffer containing a message log
populated with various runtime messages produced by the firmware.
Provide a sysfs interface /sys/firmware/opal/messages for userspace to
view the messages.
Acked-by: Stewart Smith stew...@linux.vnet.ibm.com
Signed-off-by: Joel Stanley j...@jms.id.au
---
arch/powerpc/include/asm/opal.h| 4 ++
arch/powerpc/platforms/powernv/Makefile| 1 +
arch/powerpc/platforms/powernv/opal-messages.c | 97
++
arch/powerpc/platforms/powernv/opal.c | 4 +-
4 files changed, 105 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/opal-messages.c
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index ffafab0..6aa757e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -729,6 +729,9 @@ typedef struct oppanel_line {
/* /sys/firmware/opal */
extern struct kobject *opal_kobj;
+/* /ibm,opal */
+extern struct device_node *opal_node;
+
/* API functions */
int64_t opal_console_write(int64_t term_number, __be64 *length,
const uint8_t *buffer);
@@ -918,6 +921,7 @@ extern void opal_flash_init(void);
extern int opal_elog_init(void);
extern void opal_platform_dump_init(void);
extern void opal_sys_param_init(void);
+extern void opal_messages_init(void);
extern int opal_machine_check(struct pt_regs *regs);
extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/Makefile
b/arch/powerpc/platforms/powernv/Makefile
index f324ea0..e2ba418 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,6 +1,7 @@
obj-y+= setup.o opal-takeover.o opal-wrappers.o
opal.o opal-async.o
obj-y+= opal-rtc.o opal-nvram.o opal-lpc.o
opal-flash.o
obj-y+= rng.o opal-elog.o opal-dump.o
opal-sysparam.o opal-sensor.o
+obj-y+= opal-messages.o
obj-$(CONFIG_SMP)+= smp.o
obj-$(CONFIG_PCI)+= pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-messages.c
b/arch/powerpc/platforms/powernv/opal-messages.c
new file mode 100644
index 000..3a863e8
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-messages.c
@@ -0,0 +1,97 @@
+/*
+ * PowerNV OPAL in-memory console interface
+ *
+ * Copyright 2014 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include asm/io.h
+#include asm/opal.h
+#include linux/debugfs.h
+#include linux/of.h
+#include linux/types.h
+
+/* OPAL in-memory console. Defined in OPAL source at core/console.c */
+struct memcons {
+ __be64 magic;
+#define MEMCONS_MAGIC0x6630696567726173L
+ __be64 obuf_phys;
+ __be64 ibuf_phys;
+ __be32 obuf_size;
+ __be32 ibuf_size;
+ __be32 out_pos;
+#define MEMCONS_OUT_POS_WRAP 0x8000u
+#define MEMCONS_OUT_POS_MASK 0x00ffu
+ __be32 in_prod;
+ __be32 in_cons;
+};
+
+static ssize_t opal_messages_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *to, loff_t pos, size_t count)
+{
+ struct memcons *mc = bin_attr-private;
+ const char *conbuf;
+ bool wrapped;
+ size_t num_read;
+ int out_pos;
+
+ if (!mc)
+ return -ENODEV;
+
+ conbuf = phys_to_virt(be64_to_cpu(mc-obuf_phys));
+ wrapped = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_WRAP;
+ out_pos = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_MASK;
+
+ if (!wrapped) {
+ num_read = memory_read_from_buffer(to, count, pos, conbuf,
+ out_pos);
+ } else {
+ num_read = memory_read_from_buffer(to, count, pos,
+ conbuf + out_pos,
+ be32_to_cpu(mc-obuf_size) - out_pos);
+
+ if (num_read 0)
+ goto out;
+
+ num_read += memory_read_from_buffer(to + num_read,
+ count - num_read, pos, conbuf, out_pos);
+ }
+out:
+ return num_read;
+}
+
+static struct bin_attribute messages_attr = {
+ .attr = {.name = messages, .mode = 0444},
+ .read = opal_messages_read
+};
+
+void __init opal_messages_init(void)
+{
+ u64 mcaddr;
+ struct memcons *mc;
+
+ if (of_property_read_u64(opal_node, ibm,opal-memcons, mcaddr)) {
+ pr_warn(OPAL: Property ibm,opal-memcons not found, no message
log\n);
+ return;
+ }
+
+ mc = phys_to_virt(mcaddr);
+ if (!mc) {
+
Re: [PATCH 1/2] powerpc/powernv: Add OPAL message log interface
Joel Stanley j...@jms.id.au wrote:
OPAL provides an in-memory circular buffer containing a message log
populated with various runtime messages produced by the firmware.
Provide a sysfs interface /sys/firmware/opal/messages for userspace to
view the messages.
Signed-off-by: Joel Stanley j...@jms.id.au
---
arch/powerpc/include/asm/opal.h| 4 ++
arch/powerpc/platforms/powernv/Makefile| 1 +
arch/powerpc/platforms/powernv/opal-messages.c | 97
++
arch/powerpc/platforms/powernv/opal.c | 4 +-
4 files changed, 105 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/opal-messages.c
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index ffafab0..6aa757e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -729,6 +729,9 @@ typedef struct oppanel_line {
/* /sys/firmware/opal */
extern struct kobject *opal_kobj;
+/* /ibm,opal */
+extern struct device_node *opal_node;
+
/* API functions */
int64_t opal_console_write(int64_t term_number, __be64 *length,
const uint8_t *buffer);
@@ -918,6 +921,7 @@ extern void opal_flash_init(void);
extern int opal_elog_init(void);
extern void opal_platform_dump_init(void);
extern void opal_sys_param_init(void);
+extern void opal_messages_init(void);
extern int opal_machine_check(struct pt_regs *regs);
extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/Makefile
b/arch/powerpc/platforms/powernv/Makefile
index f324ea0..e2ba418 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,6 +1,7 @@
obj-y+= setup.o opal-takeover.o opal-wrappers.o
opal.o opal-async.o
obj-y+= opal-rtc.o opal-nvram.o opal-lpc.o
opal-flash.o
obj-y+= rng.o opal-elog.o opal-dump.o
opal-sysparam.o opal-sensor.o
+obj-y+= opal-messages.o
obj-$(CONFIG_SMP)+= smp.o
obj-$(CONFIG_PCI)+= pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-messages.c
b/arch/powerpc/platforms/powernv/opal-messages.c
new file mode 100644
index 000..3a863e8
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-messages.c
@@ -0,0 +1,97 @@
+/*
+ * PowerNV OPAL in-memory console interface
+ *
+ * Copyright 2014 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include asm/io.h
+#include asm/opal.h
+#include linux/debugfs.h
+#include linux/of.h
+#include linux/types.h
+
+/* OPAL in-memory console. Defined in OPAL source at core/console.c */
+struct memcons {
+ __be64 magic;
+#define MEMCONS_MAGIC0x6630696567726173L
0x6630696567726173 == f0iegras ... Ben!!! :-P
+ __be64 obuf_phys;
+ __be64 ibuf_phys;
+ __be32 obuf_size;
+ __be32 ibuf_size;
+ __be32 out_pos;
+#define MEMCONS_OUT_POS_WRAP 0x8000u
+#define MEMCONS_OUT_POS_MASK 0x00ffu
+ __be32 in_prod;
+ __be32 in_cons;
+};
+
+static ssize_t opal_messages_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *to, loff_t pos, size_t count)
+{
+ struct memcons *mc = bin_attr-private;
+ const char *conbuf;
+ bool wrapped;
+ size_t num_read;
+ int out_pos;
+
+ if (!mc)
+ return -ENODEV;
+
+ conbuf = phys_to_virt(be64_to_cpu(mc-obuf_phys));
+ wrapped = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_WRAP;
+ out_pos = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_MASK;
+
Are there ordering issues we need to think about here with reading
these? Can the messages be written on another CPU at the same time as
these are being read?
What happens if in between reading wrapped and out_pos the buffer wraps?
You'd end up getting only a few bytes of console? Maybe you need to
read wrapped before and after out_pos to make should it's not wrapped in
between.
+ if (!wrapped) {
Why the negative case first? Just make it:
if (wrapped) {
wrapped case
} else {
not wrapped case
}
Also, no curlies needed for single statement.
+ num_read = memory_read_from_buffer(to, count, pos, conbuf,
+ out_pos);
This is probably not necessary, but do we need to sanity check out_pos
obuf_size? I guess we don't generally sanity check numbers from OPAL as
it can screw us in many other ways anyway.
+ } else {
+ num_read = memory_read_from_buffer(to, count, pos,
+ conbuf + out_pos,
+
Re: [PATCH 1/2] powerpc/powernv: Add OPAL message log interface
Michael Neuling mi...@neuling.org wrote:
Joel Stanley j...@jms.id.au wrote:
OPAL provides an in-memory circular buffer containing a message log
populated with various runtime messages produced by the firmware.
Provide a sysfs interface /sys/firmware/opal/messages for userspace to
view the messages.
Signed-off-by: Joel Stanley j...@jms.id.au
---
arch/powerpc/include/asm/opal.h| 4 ++
arch/powerpc/platforms/powernv/Makefile| 1 +
arch/powerpc/platforms/powernv/opal-messages.c | 97
++
arch/powerpc/platforms/powernv/opal.c | 4 +-
4 files changed, 105 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/opal-messages.c
diff --git a/arch/powerpc/include/asm/opal.h
b/arch/powerpc/include/asm/opal.h
index ffafab0..6aa757e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -729,6 +729,9 @@ typedef struct oppanel_line {
/* /sys/firmware/opal */
extern struct kobject *opal_kobj;
+/* /ibm,opal */
+extern struct device_node *opal_node;
+
/* API functions */
int64_t opal_console_write(int64_t term_number, __be64 *length,
const uint8_t *buffer);
@@ -918,6 +921,7 @@ extern void opal_flash_init(void);
extern int opal_elog_init(void);
extern void opal_platform_dump_init(void);
extern void opal_sys_param_init(void);
+extern void opal_messages_init(void);
extern int opal_machine_check(struct pt_regs *regs);
extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/Makefile
b/arch/powerpc/platforms/powernv/Makefile
index f324ea0..e2ba418 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,6 +1,7 @@
obj-y += setup.o opal-takeover.o opal-wrappers.o
opal.o opal-async.o
obj-y += opal-rtc.o opal-nvram.o opal-lpc.o
opal-flash.o
obj-y += rng.o opal-elog.o opal-dump.o
opal-sysparam.o opal-sensor.o
+obj-y += opal-messages.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-messages.c
b/arch/powerpc/platforms/powernv/opal-messages.c
new file mode 100644
index 000..3a863e8
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-messages.c
@@ -0,0 +1,97 @@
+/*
+ * PowerNV OPAL in-memory console interface
+ *
+ * Copyright 2014 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include asm/io.h
+#include asm/opal.h
+#include linux/debugfs.h
+#include linux/of.h
+#include linux/types.h
+
+/* OPAL in-memory console. Defined in OPAL source at core/console.c */
+struct memcons {
+ __be64 magic;
+#define MEMCONS_MAGIC 0x6630696567726173L
0x6630696567726173 == f0iegras ... Ben!!! :-P
+ __be64 obuf_phys;
+ __be64 ibuf_phys;
+ __be32 obuf_size;
+ __be32 ibuf_size;
+ __be32 out_pos;
+#define MEMCONS_OUT_POS_WRAP 0x8000u
+#define MEMCONS_OUT_POS_MASK 0x00ffu
+ __be32 in_prod;
+ __be32 in_cons;
+};
+
+static ssize_t opal_messages_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *to, loff_t pos, size_t count)
+{
+ struct memcons *mc = bin_attr-private;
+ const char *conbuf;
+ bool wrapped;
+ size_t num_read;
+ int out_pos;
+
+ if (!mc)
+ return -ENODEV;
+
+ conbuf = phys_to_virt(be64_to_cpu(mc-obuf_phys));
+ wrapped = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_WRAP;
+ out_pos = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_MASK;
+
Are there ordering issues we need to think about here with reading
these? Can the messages be written on another CPU at the same time as
these are being read?
What happens if in between reading wrapped and out_pos the buffer wraps?
You'd end up getting only a few bytes of console? Maybe you need to
read wrapped before and after out_pos to make should it's not wrapped in
between.
wrapped = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_WRAP;
out_pos = be32_to_cpu(mc-out_pos) MEMCONS_OUT_POS_MASK;
OK, I just realised this is reading from the same location. So yeah,
don't do that. Read it once and calculate wrapped and out_pos from that
single read.
+ if (!wrapped) {
Why the negative case first? Just make it:
if (wrapped) {
wrapped case
} else {
not wrapped case
}
Also, no curlies needed for single statement.
+ num_read = memory_read_from_buffer(to, count, pos, conbuf,
