[PATCH] powerpc/powernv: add hdat attribute to sysfs

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3454d

4 messages, 3 authors, 2017-02-23 · open the first message on its own page

[PATCH] powerpc/powernv: add hdat attribute to sysfs

From: Matt Brown <hidden>
Date: 2017-02-23 02:30:06

From: Matt Brown <redacted>

The HDAT data area is consumed by skiboot and turned into a device-tree.
In some cases we would like to look directly at the HDAT, so this patch
adds a sysfs node to allow it to be viewed.  This is not possible through
/dev/mem as it is reserved memory which is stopped by the /dev/mem filter.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/include/asm/opal.h              |  1 +
 arch/powerpc/platforms/powernv/opal-msglog.c | 49 ++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c        |  2 ++
 3 files changed, 52 insertions(+)
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 5c7db0f..b26944e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -277,6 +277,7 @@ extern int opal_async_comp_init(void);
 extern int opal_sensor_init(void);
 extern int opal_hmi_handler_init(void);
 extern int opal_event_init(void);
+extern void opal_hdat_sysfs_init(void);
 
 extern int opal_machine_check(struct pt_regs *regs);
 extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
index 39d6ff9..a637055 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -31,7 +31,13 @@ struct memcons {
 	__be32 in_cons;
 };
 
+struct hdatInfo {
+	char *base;
+	u64 size;
+};
+
 static struct memcons *opal_memcons = NULL;
+static struct hdatInfo hdat_inf;
 
 ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
 {
@@ -136,3 +142,46 @@ void __init opal_msglog_sysfs_init(void)
 	if (sysfs_create_bin_file(opal_kobj, &opal_msglog_attr) != 0)
 		pr_warn("OPAL: sysfs file creation failed\n");
 }
+
+
+
+/* Read function for HDAT attribute in sysfs */
+static ssize_t hdat_read(struct file *file, struct kobject *kobj,
+			 struct bin_attribute *bin_attr, char *to,
+			 loff_t pos, size_t count)
+{
+	if (!hdat_inf.base)
+		return -ENODEV;
+
+	return memory_read_from_buffer(to, count, &pos, hdat_inf.base,
+					hdat_inf.size);
+}
+
+
+/* HDAT attribute for sysfs */
+static struct bin_attribute hdat_attr = {
+	.attr = {.name = "hdat", .mode = 0444},
+	.read = hdat_read
+};
+
+void __init opal_hdat_sysfs_init(void)
+{
+	u64 hdatAddr[2];
+
+	/* Check for the hdat-map prop in device-tree */
+	if (of_property_read_u64_array(opal_node, "hdat-map", hdatAddr, 2)) {
+		pr_debug("OPAL: Property hdat-map not found.\n");
+		return;
+	}
+
+	/* Print out hdat-map values. [0]: base, [1]: size */
+	pr_debug("HDAT Base address: %#llx\n", hdatAddr[0]);
+	pr_debug("HDAT Size: %#llx\n", hdatAddr[1]);
+
+	hdat_inf.base = phys_to_virt(hdatAddr[0]);
+	hdat_inf.size = hdatAddr[1];
+
+	if (sysfs_create_bin_file(opal_kobj, &hdat_attr) != 0)
+		pr_debug("OPAL: sysfs file creation for HDAT failed");
+
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2822935..cae3745 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -740,6 +740,8 @@ static int __init opal_init(void)
 		opal_sys_param_init();
 		/* Setup message log sysfs interface. */
 		opal_msglog_sysfs_init();
+		/* Create hdat object under sys/firmware/opal */
+		opal_hdat_sysfs_init();
 	}
 
 	/* Initialize platform devices: IPMI backend, PRD & flash interface */
-- 
2.9.3

Re: [PATCH] powerpc/powernv: add hdat attribute to sysfs

From: "Oliver O'Halloran" <oohall@gmail.com>
Date: 2017-02-23 03:47:40

On Thu, Feb 23, 2017 at 1:29 PM, Matt Brown [off-list ref] wrote:
quoted hunk
From: Matt Brown <redacted>

The HDAT data area is consumed by skiboot and turned into a device-tree.
In some cases we would like to look directly at the HDAT, so this patch
adds a sysfs node to allow it to be viewed.  This is not possible through
/dev/mem as it is reserved memory which is stopped by the /dev/mem filter.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/include/asm/opal.h              |  1 +
 arch/powerpc/platforms/powernv/opal-msglog.c | 49 ++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c        |  2 ++
 3 files changed, 52 insertions(+)
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 5c7db0f..b26944e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -277,6 +277,7 @@ extern int opal_async_comp_init(void);
 extern int opal_sensor_init(void);
 extern int opal_hmi_handler_init(void);
 extern int opal_event_init(void);
+extern void opal_hdat_sysfs_init(void);

 extern int opal_machine_check(struct pt_regs *regs);
 extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
index 39d6ff9..a637055 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
@@ -31,7 +31,13 @@ struct memcons {
        __be32 in_cons;
 };

+struct hdatInfo {
+       char *base;
+       u64 size;
+};
+
 static struct memcons *opal_memcons = NULL;
+static struct hdatInfo hdat_inf;
I have a few 'o's to spare if you need one.
quoted hunk
 ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
 {
@@ -136,3 +142,46 @@ void __init opal_msglog_sysfs_init(void)
        if (sysfs_create_bin_file(opal_kobj, &opal_msglog_attr) != 0)
                pr_warn("OPAL: sysfs file creation failed\n");
 }
+
+
+
+/* Read function for HDAT attribute in sysfs */
Bonus whitespace!
+static ssize_t hdat_read(struct file *file, struct kobject *kobj,
+                        struct bin_attribute *bin_attr, char *to,
+                        loff_t pos, size_t count)
+{
+       if (!hdat_inf.base)
+               return -ENODEV;
+
+       return memory_read_from_buffer(to, count, &pos, hdat_inf.base,
+                                       hdat_inf.size);
+}
Hmm... There's been some ideas floating around about removing Skiboot
from the linear mapping and that would break this. However, that is
something we should probably shouldn't worry about until it happens.
quoted hunk
+
+
+/* HDAT attribute for sysfs */
+static struct bin_attribute hdat_attr = {
+       .attr = {.name = "hdat", .mode = 0444},
+       .read = hdat_read
+};
+
+void __init opal_hdat_sysfs_init(void)
+{
+       u64 hdatAddr[2];
+
+       /* Check for the hdat-map prop in device-tree */
+       if (of_property_read_u64_array(opal_node, "hdat-map", hdatAddr, 2)) {
+               pr_debug("OPAL: Property hdat-map not found.\n");
+               return;
+       }
+
+       /* Print out hdat-map values. [0]: base, [1]: size */
+       pr_debug("HDAT Base address: %#llx\n", hdatAddr[0]);
+       pr_debug("HDAT Size: %#llx\n", hdatAddr[1]);
+
+       hdat_inf.base = phys_to_virt(hdatAddr[0]);
+       hdat_inf.size = hdatAddr[1];
+
+       if (sysfs_create_bin_file(opal_kobj, &hdat_attr) != 0)
+               pr_debug("OPAL: sysfs file creation for HDAT failed");
+
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2822935..cae3745 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -740,6 +740,8 @@ static int __init opal_init(void)
                opal_sys_param_init();
                /* Setup message log sysfs interface. */
                opal_msglog_sysfs_init();
+               /* Create hdat object under sys/firmware/opal */
+               opal_hdat_sysfs_init();
        }

        /* Initialize platform devices: IPMI backend, PRD & flash interface */
--
2.9.3
Quibbling aside, look ok.

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

Re: [PATCH] powerpc/powernv: add hdat attribute to sysfs

From: Andrew Donnellan <hidden>
Date: 2017-02-23 07:43:56

On 23/02/17 13:29, Matt Brown wrote:
From: Matt Brown <redacted>
Did you intend for this to be different from the email you're using to 
send + sign-off?
The HDAT data area is consumed by skiboot and turned into a device-tree.
In some cases we would like to look directly at the HDAT, so this patch
adds a sysfs node to allow it to be viewed.  This is not possible through
/dev/mem as it is reserved memory which is stopped by the /dev/mem filter.

Signed-off-by: Matt Brown <redacted>
Oliver obviously knows more about how HDAT actually works, I won't 
repeat his comments.

I haven't checked the actual important bit of how HDAT is exposed in 
device tree but other than that things look fairly good. Boring 
stylistic comments below!
quoted hunk
---
 arch/powerpc/include/asm/opal.h              |  1 +
 arch/powerpc/platforms/powernv/opal-msglog.c | 49 ++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c        |  2 ++
 3 files changed, 52 insertions(+)
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 5c7db0f..b26944e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -277,6 +277,7 @@ extern int opal_async_comp_init(void);
 extern int opal_sensor_init(void);
 extern int opal_hmi_handler_init(void);
 extern int opal_event_init(void);
+extern void opal_hdat_sysfs_init(void);

 extern int opal_machine_check(struct pt_regs *regs);
 extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/opal-msglog.c b/arch/powerpc/platforms/powernv/opal-msglog.c
index 39d6ff9..a637055 100644
--- a/arch/powerpc/platforms/powernv/opal-msglog.c
+++ b/arch/powerpc/platforms/powernv/opal-msglog.c
I think I know why you've put it in opal-msglog.c - the existing code in 
there does similar things in exposing a chunk of skiboot stuff via a 
sysfs file - but it's not immediately obvious given that HDAT and msglog 
are quite different things. Perhaps add a new file, opal-hdat.c?
quoted hunk
@@ -31,7 +31,13 @@ struct memcons {
 	__be32 in_cons;
 };

+struct hdatInfo {
Kernel coding style tends to avoid using camelCase naming, instead 
preferring all-lowercase separated by underscores. (same applies to a 
couple of other places)
quoted hunk
+	char *base;
+	u64 size;
+};
+
 static struct memcons *opal_memcons = NULL;
+static struct hdatInfo hdat_inf;

 ssize_t opal_msglog_copy(char *to, loff_t pos, size_t count)
 {
@@ -136,3 +142,46 @@ void __init opal_msglog_sysfs_init(void)
 	if (sysfs_create_bin_file(opal_kobj, &opal_msglog_attr) != 0)
 		pr_warn("OPAL: sysfs file creation failed\n");
 }
+
+
+
+/* Read function for HDAT attribute in sysfs */
+static ssize_t hdat_read(struct file *file, struct kobject *kobj,
+			 struct bin_attribute *bin_attr, char *to,
+			 loff_t pos, size_t count)
+{
+	if (!hdat_inf.base)
+		return -ENODEV;
+
+	return memory_read_from_buffer(to, count, &pos, hdat_inf.base,
+					hdat_inf.size);
+}
+
+
+/* HDAT attribute for sysfs */
+static struct bin_attribute hdat_attr = {
+	.attr = {.name = "hdat", .mode = 0444},
+	.read = hdat_read
+};
+
+void __init opal_hdat_sysfs_init(void)
+{
+	u64 hdatAddr[2];
+
+	/* Check for the hdat-map prop in device-tree */
+	if (of_property_read_u64_array(opal_node, "hdat-map", hdatAddr, 2)) {
+		pr_debug("OPAL: Property hdat-map not found.\n");
+		return;
+	}
+
+	/* Print out hdat-map values. [0]: base, [1]: size */
+	pr_debug("HDAT Base address: %#llx\n", hdatAddr[0]);
+	pr_debug("HDAT Size: %#llx\n", hdatAddr[1]);
Prefix these with "OPAL: " like the other messages?
quoted hunk
+
+	hdat_inf.base = phys_to_virt(hdatAddr[0]);
+	hdat_inf.size = hdatAddr[1];
+
+	if (sysfs_create_bin_file(opal_kobj, &hdat_attr) != 0)
+		pr_debug("OPAL: sysfs file creation for HDAT failed");
+
+}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2822935..cae3745 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -740,6 +740,8 @@ static int __init opal_init(void)
 		opal_sys_param_init();
 		/* Setup message log sysfs interface. */
 		opal_msglog_sysfs_init();
+		/* Create hdat object under sys/firmware/opal */
+		opal_hdat_sysfs_init();
 	}

 	/* Initialize platform devices: IPMI backend, PRD & flash interface */
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] powerpc/powernv: add hdat attribute to sysfs

From: Andrew Donnellan <hidden>
Date: 2017-02-23 07:45:42

On 23/02/17 18:42, Andrew Donnellan wrote:
I haven't checked the actual important bit of how HDAT is exposed in
device tree but other than that things look fairly good. Boring
stylistic comments below!
And now that I have - it's probably worth noting that this patch depends 
on a corresponding skiboot patch at 
http://patchwork.ozlabs.org/patch/731385/

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help