| Message ID | 1381218400.645.204.camel@pasglop (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote: > With OPALv3, the firmware can provide the address of it's internal console > to Linux, which we can then display using debugfs. This is handy for > diagnostics and debugging. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c > index 2911abe..10d7894 100644 > --- a/arch/powerpc/platforms/powernv/opal.c > +++ b/arch/powerpc/platforms/powernv/opal.c > @@ -17,6 +17,8 @@ > #include <linux/interrupt.h> > #include <linux/notifier.h> > #include <linux/slab.h> > +#include <linux/debugfs.h> > +#include <linux/uaccess.h> > #include <asm/opal.h> > #include <asm/firmware.h> > > @@ -27,6 +29,21 @@ struct opal { > u64 entry; > } opal; > > +/* OPAL in-memory console */ It might be nice to point out that the format of the struct is defined by OPAL and must be in sync with what OPAL is using. > +struct memcons { > + uint64_t magic; u64 ? > +#define MEMCONS_MAGIC 0x6630696567726173 > + uint64_t obuf_phys; > + uint64_t ibuf_phys; > + uint32_t obuf_size; > + uint32_t ibuf_size; > + uint32_t out_pos; > +#define MEMCONS_OUT_POS_WRAP 0x80000000u > +#define MEMCONS_OUT_POS_MASK 0x00ffffffu Where does this come from? > + uint32_t in_prod; > + uint32_t in_cons; > +}; Should it be packed? > @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data) > return IRQ_HANDLED; > } > > +#ifdef CONFIG_DEBUG_FS > +static ssize_t opal_memcons_read(struct file *file, char __user *to, > + size_t count, loff_t *ppos) > +{ > + struct memcons *mc = file->private_data; > + size_t available, ret, chunk0, chunk1, lcount; > + const char *start, *conbuf = __va(mc->obuf_phys); > + loff_t opos, pos; > + > + /* > + * Find out how much is in the buffer. If it has wrapped > + * the whole buffer, else just the beginning. It has wrapped > + * if the next character is not \0 > + */ > + if (mc->out_pos & MEMCONS_OUT_POS_WRAP) { > + available = mc->obuf_size; > + chunk1 = mc->out_pos & MEMCONS_OUT_POS_MASK; > + start = conbuf + chunk1; > + chunk0 = mc->obuf_size - chunk1; > + } else { > + available = mc->out_pos; > + start = conbuf; > + chunk0 = available; > + chunk1 = 0; > + } Surely simple_read_from_buffer() could make some of this simpler? cheers
On Wed, 2013-10-09 at 14:23 +1100, Michael Ellerman wrote: > On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote: > > With OPALv3, the firmware can provide the address of it's internal console > > to Linux, which we can then display using debugfs. This is handy for > > diagnostics and debugging. > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > > > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c > > index 2911abe..10d7894 100644 > > --- a/arch/powerpc/platforms/powernv/opal.c > > +++ b/arch/powerpc/platforms/powernv/opal.c > > @@ -17,6 +17,8 @@ > > #include <linux/interrupt.h> > > #include <linux/notifier.h> > > #include <linux/slab.h> > > +#include <linux/debugfs.h> > > +#include <linux/uaccess.h> > > #include <asm/opal.h> > > #include <asm/firmware.h> > > > > @@ -27,6 +29,21 @@ struct opal { > > u64 entry; > > } opal; > > > > +/* OPAL in-memory console */ > > It might be nice to point out that the format of the struct is defined > by OPAL and must be in sync with what OPAL is using. Yes, we could move the structure definition to opal.h... > > +struct memcons { > > + uint64_t magic; > > u64 ? Who cares ? Especially if it goes into opal.h it should stick to the types used in that file. > > +#define MEMCONS_MAGIC 0x6630696567726173 > > + uint64_t obuf_phys; > > + uint64_t ibuf_phys; > > + uint32_t obuf_size; > > + uint32_t ibuf_size; > > + uint32_t out_pos; > > +#define MEMCONS_OUT_POS_WRAP 0x80000000u > > +#define MEMCONS_OUT_POS_MASK 0x00ffffffu > > Where does this come from? My a** :-) I made it up as I wrote the OPAL side one, why ? > > + uint32_t in_prod; > > + uint32_t in_cons; > > +}; > > Should it be packed? Nope, no need. It's all nice and naturally aligned. > > @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data) > > return IRQ_HANDLED; > > } > > > > +#ifdef CONFIG_DEBUG_FS > > +static ssize_t opal_memcons_read(struct file *file, char __user *to, > > + size_t count, loff_t *ppos) > > +{ > > + struct memcons *mc = file->private_data; > > + size_t available, ret, chunk0, chunk1, lcount; > > + const char *start, *conbuf = __va(mc->obuf_phys); > > + loff_t opos, pos; > > + > > + /* > > + * Find out how much is in the buffer. If it has wrapped > > + * the whole buffer, else just the beginning. It has wrapped > > + * if the next character is not \0 > > + */ > > + if (mc->out_pos & MEMCONS_OUT_POS_WRAP) { > > + available = mc->obuf_size; > > + chunk1 = mc->out_pos & MEMCONS_OUT_POS_MASK; > > + start = conbuf + chunk1; > > + chunk0 = mc->obuf_size - chunk1; > > + } else { > > + available = mc->out_pos; > > + start = conbuf; > > + chunk0 = available; > > + chunk1 = 0; > > + } > > Surely simple_read_from_buffer() could make some of this simpler? If you can find a way to make it deal with a ring buffer... Cheers, Ben.
On Wed, Oct 09, 2013 at 03:23:21PM +1100, Benjamin Herrenschmidt wrote: > On Wed, 2013-10-09 at 14:23 +1100, Michael Ellerman wrote: > > On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote: > > > With OPALv3, the firmware can provide the address of it's internal console > > > to Linux, which we can then display using debugfs. This is handy for > > > diagnostics and debugging. > > > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > --- > > > > > > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c > > > index 2911abe..10d7894 100644 > > > --- a/arch/powerpc/platforms/powernv/opal.c > > > +++ b/arch/powerpc/platforms/powernv/opal.c > > > @@ -17,6 +17,8 @@ > > > #include <linux/interrupt.h> > > > #include <linux/notifier.h> > > > #include <linux/slab.h> > > > +#include <linux/debugfs.h> > > > +#include <linux/uaccess.h> > > > #include <asm/opal.h> > > > #include <asm/firmware.h> > > > > > > @@ -27,6 +29,21 @@ struct opal { > > > u64 entry; > > > } opal; > > > > > > +/* OPAL in-memory console */ > > > > It might be nice to point out that the format of the struct is defined > > by OPAL and must be in sync with what OPAL is using. > > Yes, we could move the structure definition to opal.h... > > > > +struct memcons { > > > + uint64_t magic; > > > > u64 ? > > Who cares ? That's what we use in the kernel. But no I don't care. > > > +#define MEMCONS_MAGIC 0x6630696567726173 > > > + uint64_t obuf_phys; > > > + uint64_t ibuf_phys; > > > + uint32_t obuf_size; > > > + uint32_t ibuf_size; > > > + uint32_t out_pos; > > > +#define MEMCONS_OUT_POS_WRAP 0x80000000u > > > +#define MEMCONS_OUT_POS_MASK 0x00ffffffu > > > > Where does this come from? > > My a** :-) I made it up as I wrote the OPAL side one, why ? ... > > > + uint32_t in_prod; > > > + uint32_t in_cons; > > > +}; > > > > Should it be packed? > > Nope, no need. It's all nice and naturally aligned. Sure, that's obvious. > > > @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data) > > > return IRQ_HANDLED; > > > } > > > > > > +#ifdef CONFIG_DEBUG_FS > > > +static ssize_t opal_memcons_read(struct file *file, char __user *to, > > > + size_t count, loff_t *ppos) > > > +{ > > > + struct memcons *mc = file->private_data; > > > + size_t available, ret, chunk0, chunk1, lcount; > > > + const char *start, *conbuf = __va(mc->obuf_phys); > > > + loff_t opos, pos; > > > + > > > + /* > > > + * Find out how much is in the buffer. If it has wrapped > > > + * the whole buffer, else just the beginning. It has wrapped > > > + * if the next character is not \0 > > > + */ > > > + if (mc->out_pos & MEMCONS_OUT_POS_WRAP) { > > > + available = mc->obuf_size; > > > + chunk1 = mc->out_pos & MEMCONS_OUT_POS_MASK; > > > + start = conbuf + chunk1; > > > + chunk0 = mc->obuf_size - chunk1; > > > + } else { > > > + available = mc->out_pos; > > > + start = conbuf; > > > + chunk0 = available; > > > + chunk1 = 0; > > > + } > > > > Surely simple_read_from_buffer() could make some of this simpler? > > If you can find a way to make it deal with a ring buffer... Call it twice. And you wonder why no one reviews your patches? cheers
On Wed, 2013-10-09 at 17:06 +1100, Michael Ellerman wrote: > Call it twice. > > And you wonder why no one reviews your patches? Not that easy :-) I had a look at using it and unless I did something stupid, it wasn't actually that trivial to figure out what arg to pass it for calling it twice, ie, it didn't simplify the code significantly ... I had another problem too but I don't remember what it was. But it could be that I had a very dumb moment... Cheers, Ben.
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 2911abe..10d7894 100644 --- a/arch/powerpc/platforms/powernv/opal.c +++ b/arch/powerpc/platforms/powernv/opal.c @@ -17,6 +17,8 @@ #include <linux/interrupt.h> #include <linux/notifier.h> #include <linux/slab.h> +#include <linux/debugfs.h> +#include <linux/uaccess.h> #include <asm/opal.h> #include <asm/firmware.h> @@ -27,6 +29,21 @@ struct opal { u64 entry; } opal; +/* OPAL in-memory console */ +struct memcons { + uint64_t magic; +#define MEMCONS_MAGIC 0x6630696567726173 + uint64_t obuf_phys; + uint64_t ibuf_phys; + uint32_t obuf_size; + uint32_t ibuf_size; + uint32_t out_pos; +#define MEMCONS_OUT_POS_WRAP 0x80000000u +#define MEMCONS_OUT_POS_MASK 0x00ffffffu + uint32_t in_prod; + uint32_t in_cons; +}; + static struct device_node *opal_node; static DEFINE_SPINLOCK(opal_write_lock); extern u64 opal_mc_secondary_handler[]; @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data) return IRQ_HANDLED; } +#ifdef CONFIG_DEBUG_FS +static ssize_t opal_memcons_read(struct file *file, char __user *to, + size_t count, loff_t *ppos) +{ + struct memcons *mc = file->private_data; + size_t available, ret, chunk0, chunk1, lcount; + const char *start, *conbuf = __va(mc->obuf_phys); + loff_t opos, pos; + + /* + * Find out how much is in the buffer. If it has wrapped + * the whole buffer, else just the beginning. It has wrapped + * if the next character is not \0 + */ + if (mc->out_pos & MEMCONS_OUT_POS_WRAP) { + available = mc->obuf_size; + chunk1 = mc->out_pos & MEMCONS_OUT_POS_MASK; + start = conbuf + chunk1; + chunk0 = mc->obuf_size - chunk1; + } else { + available = mc->out_pos; + start = conbuf; + chunk0 = available; + chunk1 = 0; + } + + opos = pos = *ppos; + + /* Sanity check arguments */ + if (pos < 0) + return -EINVAL; + if (pos >= available || !count) + return 0; + if (count > available - pos) + count = available - pos; + + /* Handle first chunk */ + if (pos < chunk0) { + lcount = min(count, chunk0 - (size_t)pos); + ret = copy_to_user(to, start + pos, lcount); + if (ret == lcount) + return -EFAULT; + lcount -= ret; + count -= lcount; + *ppos += lcount; + to += lcount; + pos = 0; + } else { + *ppos += chunk0; + pos -= chunk0; + ret = 0; + } + + /* Handle second chunk */ + if (count && chunk1 && ret == 0) { + lcount = min(count, chunk1 - (size_t)pos); + ret = copy_to_user(to, conbuf + pos, lcount); + if (ret == lcount) + return -EFAULT; + lcount -= ret; + *ppos += lcount; + } + return *ppos - opos; + +} + +static const struct file_operations opal_fops_memcons = { + .read = opal_memcons_read, + .open = simple_open, + .llseek = default_llseek, +}; + +static void opal_init_debugfs(void) +{ + u64 mcaddr; + + if (of_property_read_u64(opal_node, "ibm,opal-memcons", &mcaddr) == 0) + debugfs_create_file("opal-log", 0400, powerpc_debugfs_root, + __va(mcaddr), &opal_fops_memcons); +} +#else +static void opal_init_debugfs(void) { } +#endif /* CONFIG_DEBUG_FS */ + static int __init opal_init(void) { struct device_node *np, *consoles; @@ -414,6 +515,9 @@ static int __init opal_init(void) " (0x%x)\n", rc, irq, hwirq); opal_irqs[i] = irq; } + + opal_init_debugfs(); + return 0; } subsys_initcall(opal_init);
With OPALv3, the firmware can provide the address of it's internal console to Linux, which we can then display using debugfs. This is handy for diagnostics and debugging. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> ---