Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface
* Claudio Carvalho [2019-08-24 23:19:19]: > > On 8/23/19 9:48 AM, Michael Ellerman wrote: > > Hi Claudio, > > Hi Michael, > > > > > Claudio Carvalho 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 > >> CC: Oliver O'Halloran > >> Signed-off-by: Claudio Carvalho > >> --- > >> 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
Claudio Carvalho writes: > On 8/23/19 9:48 AM, Michael Ellerman wrote: >> Claudio Carvalho 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 >>> CC: Oliver O'Halloran >>> Signed-off-by: Claudio Carvalho >>> --- >>> 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
On 8/23/19 9:48 AM, Michael Ellerman wrote: > Hi Claudio, Hi Michael, > > Claudio Carvalho 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 >> CC: Oliver O'Halloran >> Signed-off-by: Claudio Carvalho >> --- >> 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
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
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
9 #include
10 #include
11 #include
12 #include
13 #include
14 #include
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_WRAP0x8000u
26 #define MEMCONS_OUT_POS_MASK0x00ffu
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, ,
62 conbuf + out_pos,
Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface
Hi Claudio, Claudio Carvalho 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 > CC: Oliver O'Halloran > Signed-off-by: Claudio Carvalho > --- > 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
[PATCH v2] powerpc/powernv: Add ultravisor message log interface
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
CC: Oliver O'Halloran
Signed-off-by: Claudio Carvalho
---
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
#include
#include
+#include
/* 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, ,
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", )) {
- pr_warn("OPAL: Property ibm,opal-memcons not found, no message
log\n");
+ if (of_property_read_u64(opal_node, dt_prop_name, _addr)) {
+ pr_warn("OPAL: Property '%s' not found,
