* [PATCH] powerpc/powernv: add hdat attribute to sysfs
@ 2017-02-23 2:29 Matt Brown
2017-02-23 3:47 ` Oliver O'Halloran
2017-02-23 7:42 ` Andrew Donnellan
0 siblings, 2 replies; 4+ messages in thread
From: Matt Brown @ 2017-02-23 2:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Matt Brown, Matt Brown
From: Matt Brown <brownmatt1997@gmail.com>
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 <matthew.brown.dev@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/powernv: add hdat attribute to sysfs
2017-02-23 2:29 [PATCH] powerpc/powernv: add hdat attribute to sysfs Matt Brown
@ 2017-02-23 3:47 ` Oliver O'Halloran
2017-02-23 7:42 ` Andrew Donnellan
1 sibling, 0 replies; 4+ messages in thread
From: Oliver O'Halloran @ 2017-02-23 3:47 UTC (permalink / raw)
To: Matt Brown; +Cc: linuxppc-dev, Stewart Smith
On Thu, Feb 23, 2017 at 1:29 PM, Matt Brown <matthew.brown.dev@gmail.com> wrote:
> From: Matt Brown <brownmatt1997@gmail.com>
>
> 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 <matthew.brown.dev@gmail.com>
> ---
> 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.
>
> 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.
> +
> +
> +/* 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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/powernv: add hdat attribute to sysfs
2017-02-23 2:29 [PATCH] powerpc/powernv: add hdat attribute to sysfs Matt Brown
2017-02-23 3:47 ` Oliver O'Halloran
@ 2017-02-23 7:42 ` Andrew Donnellan
2017-02-23 7:44 ` Andrew Donnellan
1 sibling, 1 reply; 4+ messages in thread
From: Andrew Donnellan @ 2017-02-23 7:42 UTC (permalink / raw)
To: Matt Brown, linuxppc-dev; +Cc: Matt Brown
On 23/02/17 13:29, Matt Brown wrote:
> From: Matt Brown <brownmatt1997@gmail.com>
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 <matthew.brown.dev@gmail.com>
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!
> ---
> 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?
> @@ -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)
> + 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?
> +
> + 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/powernv: add hdat attribute to sysfs
2017-02-23 7:42 ` Andrew Donnellan
@ 2017-02-23 7:44 ` Andrew Donnellan
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Donnellan @ 2017-02-23 7:44 UTC (permalink / raw)
To: Matt Brown, linuxppc-dev; +Cc: Matt Brown
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-23 7:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-23 2:29 [PATCH] powerpc/powernv: add hdat attribute to sysfs Matt Brown
2017-02-23 3:47 ` Oliver O'Halloran
2017-02-23 7:42 ` Andrew Donnellan
2017-02-23 7:44 ` Andrew Donnellan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).