[PATCH 1/2] powerpc/powernv: new function to access OPAL msglog

[PATCH 1/2] powerpc/powernv: new function to access OPAL msglog

[Andrew Donnellan]

Currently, the OPAL msglog/console buffer is exposed as a sysfs file, with
the sysfs read handler responsible for retrieving the log from the OPAL
buffer. We'd like to be able to use it in xmon as well.

Refactor the OPAL msglog code to create a new function, opal_msglog_copy(),
that copies to an arbitrary buffer.

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 arch/powerpc/include/asm/opal.h              |  2 ++
 arch/powerpc/platforms/powernv/opal-msglog.c | 15 +++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 07a99e6..3f6cb85 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -273,6 +273,8 @@ void opal_free_sg_list(struct opal_sg_list *sg);
 
 extern int opal_error_code(int rc);
 
+ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
index 44ed78a..a06191c 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -31,11 +31,11 @@ struct memcons {
 	__be32 in_cons;
 };
 
-static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
-				struct bin_attribute *bin_attr, char *to,
-				loff_t pos, size_t count)
+static struct bin_attribute opal_msglog_attr;
+
+ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
 {
-	struct memcons *mc = bin_attr->private;
+	struct memcons *mc = opal_msglog_attr.private;
 	const char *conbuf;
 	ssize_t ret;
 	size_t first_read = 0;
@@ -91,6 +91,13 @@ out:
 	return ret;
 }
 
+static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *to,
+				loff_t pos, size_t count)
+{
+	return opal_msglog_copy(to, pos, count);
+}
+
 static struct bin_attribute opal_msglog_attr = {
 	.attr = {.name = "msglog", .mode = 0444},
 	.read = opal_msglog_read
-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

[PATCH 2/2] powerpc/xmon: add command to dump OPAL msglog

[Andrew Donnellan]

Add the 'do' command to dump the OPAL msglog in xmon.

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 arch/powerpc/xmon/xmon.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 07a8508..48b75ae 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -47,6 +47,11 @@
 #include <asm/debug.h>
 #include <asm/hw_breakpoint.h>
 
+#ifdef CONFIG_PPC_POWERNV
+#include <asm/opal.h>
+#include <asm/firmware.h>
+#endif
+
 #ifdef CONFIG_PPC64
 #include <asm/hvcall.h>
 #include <asm/paca.h>
@@ -119,6 +124,11 @@ static void dump(void);
 static void prdump(unsigned long, long);
 static int ppc_inst_dump(unsigned long, long, int);
 static void dump_log_buf(void);
+
+#ifdef CONFIG_PPC_POWERNV
+static void dump_opal_msglog(void);
+#endif
+
 static void backtrace(struct pt_regs *);
 static void excprint(struct pt_regs *);
 static void prregs(struct pt_regs *);
@@ -202,6 +212,10 @@ Commands:\n\
   df	dump float values\n\
   dd	dump double values\n\
   dl    dump the kernel log buffer\n"
+#ifdef CONFIG_PPC_POWERNV
+  "\
+  do    dump the OPAL message log\n"
+#endif
 #ifdef CONFIG_PPC64
   "\
   dp[#]	dump paca for current cpu, or cpu #\n\
@@ -2253,6 +2267,12 @@ dump(void)
 		last_cmd = "di\n";
 	} else if (c == 'l') {
 		dump_log_buf();
+	} else if (c == 'o') {
+#ifdef CONFIG_PPC_POWERNV
+		dump_opal_msglog();
+#else
+		printf("Machine is not running OPAL firmware.\n");
+#endif
 	} else if (c == 'r') {
 		scanhex(&ndump);
 		if (ndump == 0)
@@ -2395,6 +2415,46 @@ dump_log_buf(void)
 	catch_memory_errors = 0;
 }
 
+#ifdef CONFIG_PPC_POWERNV
+void
+dump_opal_msglog(void)
+{
+	unsigned char buf[128];
+	ssize_t res;
+	loff_t pos = 0;
+
+	if (!firmware_has_feature(FW_FEATURE_OPAL)) {
+		printf("Machine is not running OPAL firmware.\n");
+		return;
+	}
+
+	if (setjmp(bus_error_jmp) != 0) {
+		printf("Error dumping OPAL msglog!\n");
+		return;
+	}
+
+	catch_memory_errors = 1;
+	sync();
+
+	xmon_start_pagination();
+	while ((res = opal_msglog_copy(buf, pos, sizeof(buf) - 1))) {
+		if (res < 0) {
+			printf("Error dumping OPAL msglog! Error: %zd\n", res);
+			break;
+		}
+		buf[res] = '\0';
+		printf("%s", buf);
+		pos += res;
+	}
+	xmon_end_pagination();
+
+	sync();
+	/* wait a little while to see if we get a machine check */
+	__delay(200);
+	catch_memory_errors = 0;
+}
+#endif
+
 /*
  * Memory operations - move, set, print differences
  */
-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [1/2] powerpc/powernv: new function to access OPAL msglog

[Michael Ellerman]

On Wed, 2016-20-01 at 06:50:17 UTC, Andrew Donnellan wrote:
> Currently, the OPAL msglog/console buffer is exposed as a sysfs file, with
> the sysfs read handler responsible for retrieving the log from the OPAL
> buffer. We'd like to be able to use it in xmon as well.
> 
> Refactor the OPAL msglog code to create a new function, opal_msglog_copy(),
> that copies to an arbitrary buffer.

This is a good idea, but the implementation is not quite right, comments below.

> diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
> index 44ed78a..a06191c 100644
> --- a/arch/powerpc/platforms/powernv/opal-msglog.c
> +++ b/arch/powerpc/platforms/powernv/opal-msglog.c
> @@ -31,11 +31,11 @@ struct memcons {
>  	__be32 in_cons;
>  };
>  
> -static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
> -				struct bin_attribute *bin_attr, char *to,
> -				loff_t pos, size_t count)
> +static struct bin_attribute opal_msglog_attr;
> +
> +ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
>  {
> -	struct memcons *mc = bin_attr->private;
> +	struct memcons *mc = opal_msglog_attr.private;

Pulling the memcons out of the bin_attr here is not all that nice. This routine
should really stand on its own without reference to the bin_attr. In theory I
might want to disable building sysfs but still have this routine available.

It's also a bit fishy if it's called before the bin_attr is initialised or when
the memcons initialisation fails. In both cases it should be OK, because the
structs in question are static and so the private pointer will be NULL, but
that's a bit fragile.

I think the solution is simply to create a:

  static struct memcons *opal_memcons;

And use that in opal_msglog_copy() and so on.

cheers

Re: [1/2] powerpc/powernv: new function to access OPAL msglog

[Andrew Donnellan]

On 08/02/16 22:31, Michael Ellerman wrote:
> Pulling the memcons out of the bin_attr here is not all that nice. This routine
> should really stand on its own without reference to the bin_attr. In theory I
> might want to disable building sysfs but still have this routine available.

Yeah it's a bit ugly, though does disabling sysfs actually break it? I 
can separate it out anyway - there's no reason for the memcons to be 
tied to the sysfs entry.

> It's also a bit fishy if it's called before the bin_attr is initialised or when
> the memcons initialisation fails. In both cases it should be OK, because the
> structs in question are static and so the private pointer will be NULL, but
> that's a bit fragile.
>
> I think the solution is simply to create a:
>
>    static struct memcons *opal_memcons;
>
> And use that in opal_msglog_copy() and so on.

Will respin.

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [1/2] powerpc/powernv: new function to access OPAL msglog

[Michael Ellerman]

On Tue, 2016-02-09 at 16:29 +1100, Andrew Donnellan wrote:
> On 08/02/16 22:31, Michael Ellerman wrote:
> > Pulling the memcons out of the bin_attr here is not all that nice. This routine
> > should really stand on its own without reference to the bin_attr. In theory I
> > might want to disable building sysfs but still have this routine available.
>
> Yeah it's a bit ugly, though does disabling sysfs actually break it?

Probably not, it looks like bin_attribute is still defined even when sysfs is
disabled. And the build would break in other places too.

> I can separate it out anyway - there's no reason for the memcons to be
> tied to the sysfs entry.

Yeah that was more my point.

> > It's also a bit fishy if it's called before the bin_attr is initialised or when
> > the memcons initialisation fails. In both cases it should be OK, because the
> > structs in question are static and so the private pointer will be NULL, but
> > that's a bit fragile.
> >
> > I think the solution is simply to create a:
> >
> >    static struct memcons *opal_memcons;
> >
> > And use that in opal_msglog_copy() and so on.
>
> Will respin.

Thanks.

cheers