| Message ID | 20190823060654.28842-1-cclaudio@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] powerpc/powernv: Add ultravisor message log interface | expand |
Hi Claudio, Claudio Carvalho <cclaudio@linux.ibm.com> writes: > Ultravisor (UV) provides an in-memory console which follows the OPAL > in-memory console structure. > > This patch extends the OPAL msglog code to also initialize the UV memory > console and provide a sysfs interface (uv_msglog) for userspace to view > the UV message log. > > CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > CC: Oliver O'Halloran <oohall@gmail.com> > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > --- > This patch depends on the "kvmppc: Paravirtualize KVM to support > ultravisor" patchset submitted by Claudio Carvalho. > --- > arch/powerpc/platforms/powernv/opal-msglog.c | 99 ++++++++++++++------ > 1 file changed, 72 insertions(+), 27 deletions(-) I think the code changes look mostly OK here. But I'm not sure about the end result in sysfs. If I'm reading it right this will create: /sys/firmware/opal/uv_msglog Which I think is a little weird, because the UV is not OPAL. So I guess I wonder if the file should be created elsewhere to avoid any confusion and keep things nicely separated. Possibly /sys/firmware/ultravisor/msglog ? cheers
On 8/23/19 9:48 AM, Michael Ellerman wrote: > Hi Claudio, Hi Michael, > > Claudio Carvalho <cclaudio@linux.ibm.com> writes: >> Ultravisor (UV) provides an in-memory console which follows the OPAL >> in-memory console structure. >> >> This patch extends the OPAL msglog code to also initialize the UV memory >> console and provide a sysfs interface (uv_msglog) for userspace to view >> the UV message log. >> >> CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> >> CC: Oliver O'Halloran <oohall@gmail.com> >> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> >> --- >> This patch depends on the "kvmppc: Paravirtualize KVM to support >> ultravisor" patchset submitted by Claudio Carvalho. >> --- >> arch/powerpc/platforms/powernv/opal-msglog.c | 99 ++++++++++++++------ >> 1 file changed, 72 insertions(+), 27 deletions(-) > I think the code changes look mostly OK here. > > But I'm not sure about the end result in sysfs. > > If I'm reading it right this will create: > > /sys/firmware/opal/uv_msglog > > Which I think is a little weird, because the UV is not OPAL. > > So I guess I wonder if the file should be created elsewhere to avoid any > confusion and keep things nicely separated. > > Possibly /sys/firmware/ultravisor/msglog ? Yes, makes sense. I will do that. Currently, the ultravisor memory console DT property is in /ibm,opal/ibm,opal-uv-memcons. I think we should move it to /ibm,ultravisor/ibm,uv-firmware/ibm,uv-memcons. What do you think? Thanks, Claudio > > cheers
Claudio Carvalho <cclaudio@linux.ibm.com> writes: > On 8/23/19 9:48 AM, Michael Ellerman wrote: >> Claudio Carvalho <cclaudio@linux.ibm.com> writes: >>> Ultravisor (UV) provides an in-memory console which follows the OPAL >>> in-memory console structure. >>> >>> This patch extends the OPAL msglog code to also initialize the UV memory >>> console and provide a sysfs interface (uv_msglog) for userspace to view >>> the UV message log. >>> >>> CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> >>> CC: Oliver O'Halloran <oohall@gmail.com> >>> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> >>> --- >>> This patch depends on the "kvmppc: Paravirtualize KVM to support >>> ultravisor" patchset submitted by Claudio Carvalho. >>> --- >>> arch/powerpc/platforms/powernv/opal-msglog.c | 99 ++++++++++++++------ >>> 1 file changed, 72 insertions(+), 27 deletions(-) >> I think the code changes look mostly OK here. >> >> But I'm not sure about the end result in sysfs. >> >> If I'm reading it right this will create: >> >> /sys/firmware/opal/uv_msglog >> >> Which I think is a little weird, because the UV is not OPAL. >> >> So I guess I wonder if the file should be created elsewhere to avoid any >> confusion and keep things nicely separated. >> >> Possibly /sys/firmware/ultravisor/msglog ? > > > Yes, makes sense. I will do that. Thanks. > Currently, the ultravisor memory console DT property is in > /ibm,opal/ibm,opal-uv-memcons. I think we should move it to > /ibm,ultravisor/ibm,uv-firmware/ibm,uv-memcons. What do you think? Yes that looks better. As an aside, you don't really need to namespace every node and property under ibm,ultravisor, the top-level ibm,ultravisor is already a namespace of sorts. ie. it could just be: /ibm,ultravisor/firmware/memcons But if it's too late to change those paths it doesn't really matter. cheers
* Claudio Carvalho <cclaudio@linux.ibm.com> [2019-08-24 23:19:19]: > > On 8/23/19 9:48 AM, Michael Ellerman wrote: > > Hi Claudio, > > Hi Michael, > > > > > Claudio Carvalho <cclaudio@linux.ibm.com> writes: > >> Ultravisor (UV) provides an in-memory console which follows the OPAL > >> in-memory console structure. > >> > >> This patch extends the OPAL msglog code to also initialize the UV memory > >> console and provide a sysfs interface (uv_msglog) for userspace to view > >> the UV message log. > >> > >> CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > >> CC: Oliver O'Halloran <oohall@gmail.com> > >> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > >> --- > >> This patch depends on the "kvmppc: Paravirtualize KVM to support > >> ultravisor" patchset submitted by Claudio Carvalho. > >> --- > >> arch/powerpc/platforms/powernv/opal-msglog.c | 99 ++++++++++++++------ > >> 1 file changed, 72 insertions(+), 27 deletions(-) > > I think the code changes look mostly OK here. > > > > But I'm not sure about the end result in sysfs. > > > > If I'm reading it right this will create: > > > > /sys/firmware/opal/uv_msglog > > > > Which I think is a little weird, because the UV is not OPAL. > > > > So I guess I wonder if the file should be created elsewhere to avoid any > > confusion and keep things nicely separated. > > > > Possibly /sys/firmware/ultravisor/msglog ? > > > Yes, makes sense. I will do that. +1 Letting the UV have its own /sys/firmware/ultravisor/xxx is a good idea. We may have a need to export more runtime data from UV for debug/profile purposes and this sysfs directory will come handy. --Vaidy
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c index dc51d03c6370..da73908fdabe 100644 --- a/arch/powerpc/platforms/powernv/opal-msglog.c +++ b/arch/powerpc/platforms/powernv/opal-msglog.c @@ -11,6 +11,7 @@ #include <linux/of.h> #include <linux/types.h> #include <asm/barrier.h> +#include <asm/firmware.h> /* OPAL in-memory console. Defined in OPAL source at core/console.c */ struct memcons { @@ -28,24 +29,26 @@ struct memcons { }; static struct memcons *opal_memcons = NULL; +static struct memcons *opal_uv_memcons; -ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count) +static ssize_t msglog_copy(struct memcons *memcons, const char *bin_attr_name, + char *to, loff_t pos, size_t count) { const char *conbuf; ssize_t ret; size_t first_read = 0; uint32_t out_pos, avail; - if (!opal_memcons) + if (!memcons) return -ENODEV; - out_pos = be32_to_cpu(READ_ONCE(opal_memcons->out_pos)); + out_pos = be32_to_cpu(READ_ONCE(memcons->out_pos)); /* Now we've read out_pos, put a barrier in before reading the new * data it points to in conbuf. */ smp_rmb(); - conbuf = phys_to_virt(be64_to_cpu(opal_memcons->obuf_phys)); + conbuf = phys_to_virt(be64_to_cpu(memcons->obuf_phys)); /* When the buffer has wrapped, read from the out_pos marker to the end * of the buffer, and then read the remaining data as in the un-wrapped @@ -53,7 +56,7 @@ ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count) if (out_pos & MEMCONS_OUT_POS_WRAP) { out_pos &= MEMCONS_OUT_POS_MASK; - avail = be32_to_cpu(opal_memcons->obuf_size) - out_pos; + avail = be32_to_cpu(memcons->obuf_size) - out_pos; ret = memory_read_from_buffer(to, count, &pos, conbuf + out_pos, avail); @@ -71,8 +74,8 @@ ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count) } /* Sanity check. The firmware should not do this to us. */ - if (out_pos > be32_to_cpu(opal_memcons->obuf_size)) { - pr_err("OPAL: memory console corruption. Aborting read.\n"); + if (out_pos > be32_to_cpu(memcons->obuf_size)) { + pr_err("OPAL: %s corruption. Aborting read.\n", bin_attr_name); return -EINVAL; } @@ -86,53 +89,95 @@ ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count) return ret; } +#define BIN_ATTR_NAME_OPAL "msglog" +#define BIN_ATTR_NAME_UV "uv_msglog" + +ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count) +{ + return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos, + count); +} + 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); + return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos, + count); +} + +static ssize_t opal_uv_msglog_read(struct file *file, struct kobject *kobj, + struct bin_attribute *bin_attr, char *to, + loff_t pos, size_t count) +{ + return msglog_copy(opal_uv_memcons, BIN_ATTR_NAME_UV, to, pos, + count); } static struct bin_attribute opal_msglog_attr = { - .attr = {.name = "msglog", .mode = 0400}, + .attr = {.name = BIN_ATTR_NAME_OPAL, .mode = 0400}, .read = opal_msglog_read }; -void __init opal_msglog_init(void) +static struct bin_attribute opal_uv_msglog_attr = { + .attr = {.name = BIN_ATTR_NAME_UV, .mode = 0400}, + .read = opal_uv_msglog_read +}; + +static void __init msglog_init(struct memcons **memcons, + struct bin_attribute *bin_attr, + const char *dt_prop_name) { - u64 mcaddr; - struct memcons *mc; + u64 memcons_addr; - if (of_property_read_u64(opal_node, "ibm,opal-memcons", &mcaddr)) { - pr_warn("OPAL: Property ibm,opal-memcons not found, no message log\n"); + if (of_property_read_u64(opal_node, dt_prop_name, &memcons_addr)) { + pr_warn("OPAL: Property '%s' not found, no message log\n", + dt_prop_name); return; } - mc = phys_to_virt(mcaddr); - if (!mc) { - pr_warn("OPAL: memory console address is invalid\n"); + *memcons = phys_to_virt(memcons_addr); + if (!(*memcons)) { + pr_warn("OPAL: '%s' address is invalid\n", dt_prop_name); return; } - if (be64_to_cpu(mc->magic) != MEMCONS_MAGIC) { - pr_warn("OPAL: memory console version is invalid\n"); + if (be64_to_cpu((*memcons)->magic) != MEMCONS_MAGIC) { + pr_warn("OPAL: '%s' version is invalid\n", dt_prop_name); + *memcons = NULL; return; } /* Report maximum size */ - opal_msglog_attr.size = be32_to_cpu(mc->ibuf_size) + - be32_to_cpu(mc->obuf_size); + bin_attr->size = be32_to_cpu((*memcons)->ibuf_size) + + be32_to_cpu((*memcons)->obuf_size); +} - opal_memcons = mc; +void __init opal_msglog_init(void) +{ + msglog_init(&opal_memcons, &opal_msglog_attr, "ibm,opal-memcons"); + if (firmware_has_feature(FW_FEATURE_ULTRAVISOR)) + msglog_init(&opal_uv_memcons, &opal_uv_msglog_attr, + "ibm,opal-uv-memcons"); } -void __init opal_msglog_sysfs_init(void) +static void __init msglog_sysfs_create(struct memcons *memcons, + struct bin_attribute *bin_attr) { - if (!opal_memcons) { - pr_warn("OPAL: message log initialisation failed, not creating sysfs entry\n"); + if (!memcons) { + pr_warn("OPAL: %s initialization failed, not creating sysfs entry\n", + bin_attr->attr.name); return; } - if (sysfs_create_bin_file(opal_kobj, &opal_msglog_attr) != 0) - pr_warn("OPAL: sysfs file creation failed\n"); + if (sysfs_create_bin_file(opal_kobj, bin_attr) != 0) + pr_warn("OPAL: sysfs %s creation failed\n", + bin_attr->attr.name); +} + +void __init opal_msglog_sysfs_init(void) +{ + msglog_sysfs_create(opal_memcons, &opal_msglog_attr); + if (firmware_has_feature(FW_FEATURE_ULTRAVISOR)) + msglog_sysfs_create(opal_uv_memcons, &opal_uv_msglog_attr); }
Ultravisor (UV) provides an in-memory console which follows the OPAL in-memory console structure. This patch extends the OPAL msglog code to also initialize the UV memory console and provide a sysfs interface (uv_msglog) for userspace to view the UV message log. CC: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> CC: Oliver O'Halloran <oohall@gmail.com> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> --- This patch depends on the "kvmppc: Paravirtualize KVM to support ultravisor" patchset submitted by Claudio Carvalho. --- arch/powerpc/platforms/powernv/opal-msglog.c | 99 ++++++++++++++------ 1 file changed, 72 insertions(+), 27 deletions(-)