LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] powerpc/powernv: Rework exports to support subnodes
@ 2019-10-04  8:43 Oliver O'Halloran
  2019-10-04  8:43 ` [PATCH 2/2] powerpc/powernv: Use common code for the symbol_map export Oliver O'Halloran
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver O'Halloran @ 2019-10-04  8:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

Originally we only had a handful of exported memory ranges, but we'd to
export the per-core trace buffers. This results in a lot of files in the
exports directory which is a but unfortunate. We can clean things up a bit
by turning subnodes into subdirectories of the exports directory.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/opal.c | 114 +++++++++++++++++++++-------------
 1 file changed, 72 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 38e9027..0373da5 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -752,6 +752,75 @@ static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
 				       bin_attr->size);
 }
 
+static int opal_add_one_export(struct kobject *parent, const char *export_name,
+			       struct device_node *np, const char *prop_name)
+{
+	struct bin_attribute *attr = NULL;
+	const char *name = NULL;
+	u64 vals[2];
+	int rc;
+
+	rc = of_property_read_u64_array(np, prop_name, &vals[0], 2);
+	if (rc)
+		goto out;
+
+	attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+	name = kstrdup(export_name, GFP_KERNEL);
+	if (!name) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	sysfs_bin_attr_init(attr);
+	attr->attr.name = name;
+	attr->attr.mode = 0400;
+	attr->read = export_attr_read;
+	attr->private = __va(vals[0]);
+	attr->size = vals[1];
+
+	rc = sysfs_create_bin_file(parent, attr);
+out:
+	if (rc) {
+		kfree(name);
+		kfree(attr);
+	}
+
+	return rc;
+}
+
+static void opal_add_exported_attrs(struct device_node *np,
+				    struct kobject *kobj)
+{
+	struct device_node *child;
+	struct property *prop;
+
+	for_each_property_of_node(np, prop) {
+		int rc;
+
+		if (!strcmp(prop->name, "name") ||
+		    !strcmp(prop->name, "phandle"))
+			continue;
+
+		rc = opal_add_one_export(kobj, prop->name, np, prop->name);
+		if (rc) {
+			pr_warn("Unable to add export %pOF/%s, rc = %d!\n",
+				np, prop->name, rc);
+		}
+	}
+
+	for_each_child_of_node(np, child) {
+		struct kobject *child_kobj;
+
+		child_kobj = kobject_create_and_add(child->name, kobj);
+		if (!child_kobj) {
+			pr_err("Unable to create export dir for %pOF\n", child);
+			continue;
+		}
+
+		opal_add_exported_attrs(child, child_kobj);
+	}
+}
+
 /*
  * opal_export_attrs: creates a sysfs node for each property listed in
  * the device-tree under /ibm,opal/firmware/exports/
@@ -761,12 +830,8 @@ static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
  */
 static void opal_export_attrs(void)
 {
-	struct bin_attribute *attr;
 	struct device_node *np;
-	struct property *prop;
 	struct kobject *kobj;
-	u64 vals[2];
-	int rc;
 
 	np = of_find_node_by_path("/ibm,opal/firmware/exports");
 	if (!np)
@@ -779,41 +844,7 @@ static void opal_export_attrs(void)
 		return;
 	}
 
-	for_each_property_of_node(np, prop) {
-		if (!strcmp(prop->name, "name") || !strcmp(prop->name, "phandle"))
-			continue;
-
-		if (of_property_read_u64_array(np, prop->name, &vals[0], 2))
-			continue;
-
-		attr = kzalloc(sizeof(*attr), GFP_KERNEL);
-
-		if (attr == NULL) {
-			pr_warn("Failed kmalloc for bin_attribute!");
-			continue;
-		}
-
-		sysfs_bin_attr_init(attr);
-		attr->attr.name = kstrdup(prop->name, GFP_KERNEL);
-		attr->attr.mode = 0400;
-		attr->read = export_attr_read;
-		attr->private = __va(vals[0]);
-		attr->size = vals[1];
-
-		if (attr->attr.name == NULL) {
-			pr_warn("Failed kstrdup for bin_attribute attr.name");
-			kfree(attr);
-			continue;
-		}
-
-		rc = sysfs_create_bin_file(kobj, attr);
-		if (rc) {
-			pr_warn("Error %d creating OPAL sysfs exports/%s file\n",
-				 rc, prop->name);
-			kfree(attr->attr.name);
-			kfree(attr);
-		}
-	}
+	opal_add_exported_attrs(np, kobj);
 
 	of_node_put(np);
 }
@@ -974,11 +1005,10 @@ static int __init opal_init(void)
 		opal_sys_param_init();
 		/* Setup message log sysfs interface. */
 		opal_msglog_sysfs_init();
+		/* Add all export properties*/
+		opal_export_attrs();
 	}
 
-	/* Export all properties */
-	opal_export_attrs();
-
 	/* Initialize platform devices: IPMI backend, PRD & flash interface */
 	opal_pdev_init("ibm,opal-ipmi");
 	opal_pdev_init("ibm,opal-flash");
-- 
2.9.5


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] powerpc/powernv: Use common code for the symbol_map export
  2019-10-04  8:43 [PATCH 1/2] powerpc/powernv: Rework exports to support subnodes Oliver O'Halloran
@ 2019-10-04  8:43 ` Oliver O'Halloran
  2019-10-04  9:23   ` kbuild test robot
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver O'Halloran @ 2019-10-04  8:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

Long before we had a generic way for firmware to export memory ranges of
interest we added a special case for the skiboot symbol map. The code is
pretty much identical to the generic export so re-use the code.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/platforms/powernv/opal.c | 47 +++++++----------------------------
 1 file changed, 9 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 0373da5..1ef26e2 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -708,42 +708,6 @@ static int opal_sysfs_init(void)
 	return 0;
 }
 
-static ssize_t symbol_map_read(struct file *fp, struct kobject *kobj,
-			       struct bin_attribute *bin_attr,
-			       char *buf, loff_t off, size_t count)
-{
-	return memory_read_from_buffer(buf, count, &off, bin_attr->private,
-				       bin_attr->size);
-}
-
-static struct bin_attribute symbol_map_attr = {
-	.attr = {.name = "symbol_map", .mode = 0400},
-	.read = symbol_map_read
-};
-
-static void opal_export_symmap(void)
-{
-	const __be64 *syms;
-	unsigned int size;
-	struct device_node *fw;
-	int rc;
-
-	fw = of_find_node_by_path("/ibm,opal/firmware");
-	if (!fw)
-		return;
-	syms = of_get_property(fw, "symbol-map", &size);
-	if (!syms || size != 2 * sizeof(__be64))
-		return;
-
-	/* Setup attributes */
-	symbol_map_attr.private = __va(be64_to_cpu(syms[0]));
-	symbol_map_attr.size = be64_to_cpu(syms[1]);
-
-	rc = sysfs_create_bin_file(opal_kobj, &symbol_map_attr);
-	if (rc)
-		pr_warn("Error %d creating OPAL symbols file\n", rc);
-}
-
 static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
 				struct bin_attribute *bin_attr, char *buf,
 				loff_t off, size_t count)
@@ -846,6 +810,15 @@ static void opal_export_attrs(void)
 
 	opal_add_exported_attrs(np, kobj);
 
+	/*
+	 * NB: symbol_map existed before the generic export interface so it
+	 * lives under the top level opal_kobj.
+	 */
+	rc = opal_add_one_export(opal_kobj, "symbol_map",
+				 np->parent, "symbol-map");
+	if (rc)
+		pr_warn("Error %d creating OPAL symbols file\n", rc);
+
 	of_node_put(np);
 }
 
@@ -991,8 +964,6 @@ static int __init opal_init(void)
 	/* Create "opal" kobject under /sys/firmware */
 	rc = opal_sysfs_init();
 	if (rc == 0) {
-		/* Export symbol map to userspace */
-		opal_export_symmap();
 		/* Setup dump region interface */
 		opal_dump_region_init();
 		/* Setup error log interface */
-- 
2.9.5


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] powerpc/powernv: Use common code for the symbol_map export
  2019-10-04  8:43 ` [PATCH 2/2] powerpc/powernv: Use common code for the symbol_map export Oliver O'Halloran
@ 2019-10-04  9:23   ` kbuild test robot
  2019-10-04  9:44     ` Oliver O'Halloran
  0 siblings, 1 reply; 4+ messages in thread
From: kbuild test robot @ 2019-10-04  9:23 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: Oliver O'Halloran, linuxppc-dev, kbuild-all

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

Hi Oliver,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[cannot apply to v5.4-rc1 next-20191004]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-powernv-Rework-exports-to-support-subnodes/20191004-165257
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (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 >>):

   arch/powerpc/platforms/powernv/opal.c: In function 'opal_export_attrs':
>> arch/powerpc/platforms/powernv/opal.c:817:2: error: 'rc' undeclared (first use in this function); did you mean 'rq'?
     rc = opal_add_one_export(opal_kobj, "symbol_map",
     ^~
     rq
   arch/powerpc/platforms/powernv/opal.c:817:2: note: each undeclared identifier is reported only once for each function it appears in

vim +817 arch/powerpc/platforms/powernv/opal.c

   787	
   788	/*
   789	 * opal_export_attrs: creates a sysfs node for each property listed in
   790	 * the device-tree under /ibm,opal/firmware/exports/
   791	 * All new sysfs nodes are created under /opal/exports/.
   792	 * This allows for reserved memory regions (e.g. HDAT) to be read.
   793	 * The new sysfs nodes are only readable by root.
   794	 */
   795	static void opal_export_attrs(void)
   796	{
   797		struct device_node *np;
   798		struct kobject *kobj;
   799	
   800		np = of_find_node_by_path("/ibm,opal/firmware/exports");
   801		if (!np)
   802			return;
   803	
   804		/* Create new 'exports' directory - /sys/firmware/opal/exports */
   805		kobj = kobject_create_and_add("exports", opal_kobj);
   806		if (!kobj) {
   807			pr_warn("kobject_create_and_add() of exports failed\n");
   808			return;
   809		}
   810	
   811		opal_add_exported_attrs(np, kobj);
   812	
   813		/*
   814		 * NB: symbol_map existed before the generic export interface so it
   815		 * lives under the top level opal_kobj.
   816		 */
 > 817		rc = opal_add_one_export(opal_kobj, "symbol_map",
   818					 np->parent, "symbol-map");
   819		if (rc)
   820			pr_warn("Error %d creating OPAL symbols file\n", rc);
   821	
   822		of_node_put(np);
   823	}
   824	

---
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: 62478 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] powerpc/powernv: Use common code for the symbol_map export
  2019-10-04  9:23   ` kbuild test robot
@ 2019-10-04  9:44     ` Oliver O'Halloran
  0 siblings, 0 replies; 4+ messages in thread
From: Oliver O'Halloran @ 2019-10-04  9:44 UTC (permalink / raw)
  To: kbuild test robot; +Cc: linuxppc-dev, kbuild-all

On Fri, Oct 4, 2019 at 7:24 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Oliver,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on powerpc/next]
> [cannot apply to v5.4-rc1 next-20191004]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-powernv-Rework-exports-to-support-subnodes/20191004-165257
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc-allyesconfig (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 >>):
>
>    arch/powerpc/platforms/powernv/opal.c: In function 'opal_export_attrs':
> >> arch/powerpc/platforms/powernv/opal.c:817:2: error: 'rc' undeclared (first use in this function); did you mean 'rq'?
>      rc = opal_add_one_export(opal_kobj, "symbol_map",
>      ^~
>      rq
>    arch/powerpc/platforms/powernv/opal.c:817:2: note: each undeclared identifier is reported only once for each function it appears in
>

I really need to start checking stuff still compiles after a rebase

*sigh*

>    787
>    788  /*
>    789   * opal_export_attrs: creates a sysfs node for each property listed in
>    790   * the device-tree under /ibm,opal/firmware/exports/
>    791   * All new sysfs nodes are created under /opal/exports/.
>    792   * This allows for reserved memory regions (e.g. HDAT) to be read.
>    793   * The new sysfs nodes are only readable by root.
>    794   */
>    795  static void opal_export_attrs(void)
>    796  {
>    797          struct device_node *np;
>    798          struct kobject *kobj;
>    799
>    800          np = of_find_node_by_path("/ibm,opal/firmware/exports");
>    801          if (!np)
>    802                  return;
>    803
>    804          /* Create new 'exports' directory - /sys/firmware/opal/exports */
>    805          kobj = kobject_create_and_add("exports", opal_kobj);
>    806          if (!kobj) {
>    807                  pr_warn("kobject_create_and_add() of exports failed\n");
>    808                  return;
>    809          }
>    810
>    811          opal_add_exported_attrs(np, kobj);
>    812
>    813          /*
>    814           * NB: symbol_map existed before the generic export interface so it
>    815           * lives under the top level opal_kobj.
>    816           */
>  > 817          rc = opal_add_one_export(opal_kobj, "symbol_map",
>    818                                   np->parent, "symbol-map");
>    819          if (rc)
>    820                  pr_warn("Error %d creating OPAL symbols file\n", rc);
>    821
>    822          of_node_put(np);
>    823  }
>    824
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04  8:43 [PATCH 1/2] powerpc/powernv: Rework exports to support subnodes Oliver O'Halloran
2019-10-04  8:43 ` [PATCH 2/2] powerpc/powernv: Use common code for the symbol_map export Oliver O'Halloran
2019-10-04  9:23   ` kbuild test robot
2019-10-04  9:44     ` Oliver O'Halloran

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox