[PATCH v2] powerpc/powernv: Add ultravisor message log interface

[PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Claudio Carvalho]

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(-)

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);
 }
-- 
2.20.1

[PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Claudio Carvalho]

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(-)

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);
 }
-- 
2.20.1

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Michael Ellerman]

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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Michael Ellerman]

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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8045 bytes --]

Hi Claudio,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc5 next-20190823]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Claudio-Carvalho/powerpc-powernv-Add-ultravisor-message-log-interface/20190823-214650
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/lppaca.h:48:0,
                    from arch/powerpc/include/asm/paca.h:17,
                    from arch/powerpc/include/asm/current.h:13,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:12,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:16,
                    from arch/powerpc/include/asm/io.h:27,
                    from arch/powerpc/platforms/powernv/opal-msglog.c:8:
   arch/powerpc/platforms/powernv/opal-msglog.c: In function 'opal_msglog_init':
>> arch/powerpc/platforms/powernv/opal-msglog.c:159:27: error: 'FW_FEATURE_ULTRAVISOR' undeclared (first use in this function); did you mean 'FW_FEATURE_ALWAYS'?
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~
   arch/powerpc/platforms/powernv/opal-msglog.c:159:27: note: each undeclared identifier is reported only once for each function it appears in
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~
   arch/powerpc/platforms/powernv/opal-msglog.c: In function 'opal_msglog_sysfs_init':
   arch/powerpc/platforms/powernv/opal-msglog.c:181:27: error: 'FW_FEATURE_ULTRAVISOR' undeclared (first use in this function); did you mean 'FW_FEATURE_ALWAYS'?
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~

vim +159 arch/powerpc/platforms/powernv/opal-msglog.c

   > 8	#include <asm/io.h>
     9	#include <asm/opal.h>
    10	#include <linux/debugfs.h>
    11	#include <linux/of.h>
    12	#include <linux/types.h>
    13	#include <asm/barrier.h>
    14	#include <asm/firmware.h>
    15	
    16	/* OPAL in-memory console. Defined in OPAL source at core/console.c */
    17	struct memcons {
    18		__be64 magic;
    19	#define MEMCONS_MAGIC	0x6630696567726173L
    20		__be64 obuf_phys;
    21		__be64 ibuf_phys;
    22		__be32 obuf_size;
    23		__be32 ibuf_size;
    24		__be32 out_pos;
    25	#define MEMCONS_OUT_POS_WRAP	0x80000000u
    26	#define MEMCONS_OUT_POS_MASK	0x00ffffffu
    27		__be32 in_prod;
    28		__be32 in_cons;
    29	};
    30	
    31	static struct memcons *opal_memcons = NULL;
    32	static struct memcons *opal_uv_memcons;
    33	
    34	static ssize_t msglog_copy(struct memcons *memcons, const char *bin_attr_name,
    35				   char *to, loff_t pos, size_t count)
    36	{
    37		const char *conbuf;
    38		ssize_t ret;
    39		size_t first_read = 0;
    40		uint32_t out_pos, avail;
    41	
    42		if (!memcons)
    43			return -ENODEV;
    44	
    45		out_pos = be32_to_cpu(READ_ONCE(memcons->out_pos));
    46	
    47		/* Now we've read out_pos, put a barrier in before reading the new
    48		 * data it points to in conbuf. */
    49		smp_rmb();
    50	
    51		conbuf = phys_to_virt(be64_to_cpu(memcons->obuf_phys));
    52	
    53		/* When the buffer has wrapped, read from the out_pos marker to the end
    54		 * of the buffer, and then read the remaining data as in the un-wrapped
    55		 * case. */
    56		if (out_pos & MEMCONS_OUT_POS_WRAP) {
    57	
    58			out_pos &= MEMCONS_OUT_POS_MASK;
    59			avail = be32_to_cpu(memcons->obuf_size) - out_pos;
    60	
    61			ret = memory_read_from_buffer(to, count, &pos,
    62					conbuf + out_pos, avail);
    63	
    64			if (ret < 0)
    65				goto out;
    66	
    67			first_read = ret;
    68			to += first_read;
    69			count -= first_read;
    70			pos -= avail;
    71	
    72			if (count <= 0)
    73				goto out;
    74		}
    75	
    76		/* Sanity check. The firmware should not do this to us. */
    77		if (out_pos > be32_to_cpu(memcons->obuf_size)) {
    78			pr_err("OPAL: %s corruption. Aborting read.\n", bin_attr_name);
    79			return -EINVAL;
    80		}
    81	
    82		ret = memory_read_from_buffer(to, count, &pos, conbuf, out_pos);
    83	
    84		if (ret < 0)
    85			goto out;
    86	
    87		ret += first_read;
    88	out:
    89		return ret;
    90	}
    91	
    92	#define BIN_ATTR_NAME_OPAL	"msglog"
    93	#define BIN_ATTR_NAME_UV	"uv_msglog"
    94	
    95	ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
    96	{
    97		return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos,
    98				   count);
    99	}
   100	
   101	static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
   102					struct bin_attribute *bin_attr, char *to,
   103					loff_t pos, size_t count)
   104	{
   105		return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos,
   106				   count);
   107	}
   108	
   109	static ssize_t opal_uv_msglog_read(struct file *file, struct kobject *kobj,
   110					   struct bin_attribute *bin_attr, char *to,
   111					   loff_t pos, size_t count)
   112	{
   113		return msglog_copy(opal_uv_memcons, BIN_ATTR_NAME_UV, to, pos,
   114				   count);
   115	}
   116	
   117	static struct bin_attribute opal_msglog_attr = {
   118		.attr = {.name = BIN_ATTR_NAME_OPAL, .mode = 0400},
   119		.read = opal_msglog_read
   120	};
   121	
   122	static struct bin_attribute opal_uv_msglog_attr = {
   123		.attr = {.name = BIN_ATTR_NAME_UV, .mode = 0400},
   124		.read = opal_uv_msglog_read
   125	};
   126	
   127	static void __init msglog_init(struct memcons **memcons,
   128				       struct bin_attribute *bin_attr,
   129				       const char *dt_prop_name)
   130	{
   131		u64 memcons_addr;
   132	
   133		if (of_property_read_u64(opal_node, dt_prop_name, &memcons_addr)) {
   134			pr_warn("OPAL: Property '%s' not found, no message log\n",
   135				dt_prop_name);
   136			return;
   137		}
   138	
   139		*memcons = phys_to_virt(memcons_addr);
   140		if (!(*memcons)) {
   141			pr_warn("OPAL: '%s' address is invalid\n", dt_prop_name);
   142			return;
   143		}
   144	
   145		if (be64_to_cpu((*memcons)->magic) != MEMCONS_MAGIC) {
   146			pr_warn("OPAL: '%s' version is invalid\n", dt_prop_name);
   147			*memcons = NULL;
   148			return;
   149		}
   150	
   151		/* Report maximum size */
   152		bin_attr->size = be32_to_cpu((*memcons)->ibuf_size) +
   153				 be32_to_cpu((*memcons)->obuf_size);
   154	}
   155	
   156	void __init opal_msglog_init(void)
   157	{
   158		msglog_init(&opal_memcons, &opal_msglog_attr, "ibm,opal-memcons");
 > 159		if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
   160			msglog_init(&opal_uv_memcons, &opal_uv_msglog_attr,
   161				    "ibm,opal-uv-memcons");
   162	}
   163	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25334 bytes --]

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8045 bytes --]

Hi Claudio,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc5 next-20190823]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Claudio-Carvalho/powerpc-powernv-Add-ultravisor-message-log-interface/20190823-214650
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/lppaca.h:48:0,
                    from arch/powerpc/include/asm/paca.h:17,
                    from arch/powerpc/include/asm/current.h:13,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:12,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/device.h:16,
                    from arch/powerpc/include/asm/io.h:27,
                    from arch/powerpc/platforms/powernv/opal-msglog.c:8:
   arch/powerpc/platforms/powernv/opal-msglog.c: In function 'opal_msglog_init':
>> arch/powerpc/platforms/powernv/opal-msglog.c:159:27: error: 'FW_FEATURE_ULTRAVISOR' undeclared (first use in this function); did you mean 'FW_FEATURE_ALWAYS'?
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~
   arch/powerpc/platforms/powernv/opal-msglog.c:159:27: note: each undeclared identifier is reported only once for each function it appears in
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~
   arch/powerpc/platforms/powernv/opal-msglog.c: In function 'opal_msglog_sysfs_init':
   arch/powerpc/platforms/powernv/opal-msglog.c:181:27: error: 'FW_FEATURE_ULTRAVISOR' undeclared (first use in this function); did you mean 'FW_FEATURE_ALWAYS'?
     if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
                              ^
   arch/powerpc/include/asm/firmware.h:120:25: note: in definition of macro 'firmware_has_feature'
     ((FW_FEATURE_ALWAYS & (feature)) ||    \
                            ^~~~~~~

vim +159 arch/powerpc/platforms/powernv/opal-msglog.c

   > 8	#include <asm/io.h>
     9	#include <asm/opal.h>
    10	#include <linux/debugfs.h>
    11	#include <linux/of.h>
    12	#include <linux/types.h>
    13	#include <asm/barrier.h>
    14	#include <asm/firmware.h>
    15	
    16	/* OPAL in-memory console. Defined in OPAL source at core/console.c */
    17	struct memcons {
    18		__be64 magic;
    19	#define MEMCONS_MAGIC	0x6630696567726173L
    20		__be64 obuf_phys;
    21		__be64 ibuf_phys;
    22		__be32 obuf_size;
    23		__be32 ibuf_size;
    24		__be32 out_pos;
    25	#define MEMCONS_OUT_POS_WRAP	0x80000000u
    26	#define MEMCONS_OUT_POS_MASK	0x00ffffffu
    27		__be32 in_prod;
    28		__be32 in_cons;
    29	};
    30	
    31	static struct memcons *opal_memcons = NULL;
    32	static struct memcons *opal_uv_memcons;
    33	
    34	static ssize_t msglog_copy(struct memcons *memcons, const char *bin_attr_name,
    35				   char *to, loff_t pos, size_t count)
    36	{
    37		const char *conbuf;
    38		ssize_t ret;
    39		size_t first_read = 0;
    40		uint32_t out_pos, avail;
    41	
    42		if (!memcons)
    43			return -ENODEV;
    44	
    45		out_pos = be32_to_cpu(READ_ONCE(memcons->out_pos));
    46	
    47		/* Now we've read out_pos, put a barrier in before reading the new
    48		 * data it points to in conbuf. */
    49		smp_rmb();
    50	
    51		conbuf = phys_to_virt(be64_to_cpu(memcons->obuf_phys));
    52	
    53		/* When the buffer has wrapped, read from the out_pos marker to the end
    54		 * of the buffer, and then read the remaining data as in the un-wrapped
    55		 * case. */
    56		if (out_pos & MEMCONS_OUT_POS_WRAP) {
    57	
    58			out_pos &= MEMCONS_OUT_POS_MASK;
    59			avail = be32_to_cpu(memcons->obuf_size) - out_pos;
    60	
    61			ret = memory_read_from_buffer(to, count, &pos,
    62					conbuf + out_pos, avail);
    63	
    64			if (ret < 0)
    65				goto out;
    66	
    67			first_read = ret;
    68			to += first_read;
    69			count -= first_read;
    70			pos -= avail;
    71	
    72			if (count <= 0)
    73				goto out;
    74		}
    75	
    76		/* Sanity check. The firmware should not do this to us. */
    77		if (out_pos > be32_to_cpu(memcons->obuf_size)) {
    78			pr_err("OPAL: %s corruption. Aborting read.\n", bin_attr_name);
    79			return -EINVAL;
    80		}
    81	
    82		ret = memory_read_from_buffer(to, count, &pos, conbuf, out_pos);
    83	
    84		if (ret < 0)
    85			goto out;
    86	
    87		ret += first_read;
    88	out:
    89		return ret;
    90	}
    91	
    92	#define BIN_ATTR_NAME_OPAL	"msglog"
    93	#define BIN_ATTR_NAME_UV	"uv_msglog"
    94	
    95	ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
    96	{
    97		return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos,
    98				   count);
    99	}
   100	
   101	static ssize_t opal_msglog_read(struct file *file, struct kobject *kobj,
   102					struct bin_attribute *bin_attr, char *to,
   103					loff_t pos, size_t count)
   104	{
   105		return msglog_copy(opal_memcons, BIN_ATTR_NAME_OPAL, to, pos,
   106				   count);
   107	}
   108	
   109	static ssize_t opal_uv_msglog_read(struct file *file, struct kobject *kobj,
   110					   struct bin_attribute *bin_attr, char *to,
   111					   loff_t pos, size_t count)
   112	{
   113		return msglog_copy(opal_uv_memcons, BIN_ATTR_NAME_UV, to, pos,
   114				   count);
   115	}
   116	
   117	static struct bin_attribute opal_msglog_attr = {
   118		.attr = {.name = BIN_ATTR_NAME_OPAL, .mode = 0400},
   119		.read = opal_msglog_read
   120	};
   121	
   122	static struct bin_attribute opal_uv_msglog_attr = {
   123		.attr = {.name = BIN_ATTR_NAME_UV, .mode = 0400},
   124		.read = opal_uv_msglog_read
   125	};
   126	
   127	static void __init msglog_init(struct memcons **memcons,
   128				       struct bin_attribute *bin_attr,
   129				       const char *dt_prop_name)
   130	{
   131		u64 memcons_addr;
   132	
   133		if (of_property_read_u64(opal_node, dt_prop_name, &memcons_addr)) {
   134			pr_warn("OPAL: Property '%s' not found, no message log\n",
   135				dt_prop_name);
   136			return;
   137		}
   138	
   139		*memcons = phys_to_virt(memcons_addr);
   140		if (!(*memcons)) {
   141			pr_warn("OPAL: '%s' address is invalid\n", dt_prop_name);
   142			return;
   143		}
   144	
   145		if (be64_to_cpu((*memcons)->magic) != MEMCONS_MAGIC) {
   146			pr_warn("OPAL: '%s' version is invalid\n", dt_prop_name);
   147			*memcons = NULL;
   148			return;
   149		}
   150	
   151		/* Report maximum size */
   152		bin_attr->size = be32_to_cpu((*memcons)->ibuf_size) +
   153				 be32_to_cpu((*memcons)->obuf_size);
   154	}
   155	
   156	void __init opal_msglog_init(void)
   157	{
   158		msglog_init(&opal_memcons, &opal_msglog_attr, "ibm,opal-memcons");
 > 159		if (firmware_has_feature(FW_FEATURE_ULTRAVISOR))
   160			msglog_init(&opal_uv_memcons, &opal_uv_msglog_attr,
   161				    "ibm,opal-uv-memcons");
   162	}
   163	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25334 bytes --]

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Claudio Carvalho]

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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Claudio Carvalho]

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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Michael Ellerman]

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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Michael Ellerman]

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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Vaidyanathan Srinivasan]

* 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

Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface

[Vaidyanathan Srinivasan]

* 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