RESEND [PATCH V3 0/4] powerpc/devtree: Add support for 'ibm, drc-info' property

RESEND [PATCH V3 0/4] powerpc/devtree: Add support for 'ibm, drc-info' property

[Michael Bringmann]

Several properties in the DRC device tree format are replaced by
more compact representations to allow, for example, for the encoding
of vast amounts of memory, and or reduced duplication of information
in related data structures.

"ibm,drc-info": This property, when present, replaces the following
four properties: "ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
and "ibm,drc-power-domains".  This property is defined for all
dynamically reconfigurable platform nodes.  The "ibm,drc-info" elements
are intended to provide a more compact representation, and reduce some
search overhead.

"ibm,architecture.vec": Bidirectional communication mechanism between
the host system and the front end processor indicating what features
the host system supports and what features the front end processor will
actually provide.  In this case, we are indicating that the host system
can support the new device tree structure "ibm,drc-info".

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>

Michael Bringmann (4):
  powerpc/firmware: Add definitions for new drc-info firmware feature.
  pseries/drc-info: Search new DRC properties for CPU indexes
  hotplug/drc-info: Add code to search new devtree property
  powerpc: Enable support for new DRC devtree property
---
Changes in V3:
  -- Achieved some code compression by passing data by structure
  -- Now passing more values by structure reducing use of local
     declarations / initialization.
  -- Improve some code spacing for better clarity.

RESEND [PATCH V3 1/4] powerpc/firmware: Add definitions for new drc-info firmware feature

[Michael Bringmann]

Firmware Features: Define new bit flag representing the presence of
new device tree property "ibm,drc-info".  The flag is used to tell
the front end processor whether the Linux kernel supports the new
property, and by the front end processor to tell the Linux kernel
that the new property is present in the device tree.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/firmware.h       |    3 ++-
 arch/powerpc/include/asm/prom.h           |    1 +
 arch/powerpc/platforms/pseries/firmware.c |    1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 8645897..329d537 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -51,6 +51,7 @@
 #define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
 #define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
 #define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
+#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000400000000)
 
 #ifndef __ASSEMBLY__
 
@@ -67,7 +68,7 @@ enum {
 		FW_FEATURE_CMO | FW_FEATURE_VPHN | FW_FEATURE_XCMO |
 		FW_FEATURE_SET_MODE | FW_FEATURE_BEST_ENERGY |
 		FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
-		FW_FEATURE_HPT_RESIZE,
+		FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRC_INFO,
 	FW_FEATURE_PSERIES_ALWAYS = 0,
 	FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL,
 	FW_FEATURE_POWERNV_ALWAYS = 0,
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 825bd59..3243455 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -175,6 +175,7 @@ struct of_drconf_cell {
 #define OV5_HASH_GTSE		0x1940	/* Guest Translation Shoot Down Avail */
 /* Radix Table Extensions */
 #define OV5_RADIX_GTSE		0x1A40	/* Guest Translation Shoot Down Avail */
+#define OV5_DRC_INFO		0x1640	/* Redef Prop Structures: drc-info   */
 
 /* Option Vector 6: IBM PAPR hints */
 #define OV6_LINUX		0x02	/* Linux is our OS */
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index 63cc82a..757d757 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -114,6 +114,7 @@ struct vec5_fw_feature {
 vec5_fw_features_table[] = {
 	{FW_FEATURE_TYPE1_AFFINITY,	OV5_TYPE1_AFFINITY},
 	{FW_FEATURE_PRRN,		OV5_PRRN},
+	{FW_FEATURE_DRC_INFO,		OV5_DRC_INFO},
 };
 
 static void __init fw_vec5_feature_init(const char *vec5, unsigned long len)

Re: RESEND [PATCH V3 1/4] powerpc/firmware: Add definitions for new drc-info firmware feature

[Nathan Fontenot]

On 11/15/2017 12:09 PM, Michael Bringmann wrote:
> Firmware Features: Define new bit flag representing the presence of
> new device tree property "ibm,drc-info".  The flag is used to tell
> the front end processor whether the Linux kernel supports the new
> property, and by the front end processor to tell the Linux kernel
> that the new property is present in the device tree.

This patch seems to be adding a bit for the drc-info feature so that
we can use the firmware_has_feature() interface to determine if the
device tree has the new ibm,drc-info properties.

I'm not sure what front-end processor you're referring to? Is this
in reference to the architecture vector that is exchanged with firmware?

-Nathan

> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/firmware.h       |    3 ++-
>  arch/powerpc/include/asm/prom.h           |    1 +
>  arch/powerpc/platforms/pseries/firmware.c |    1 +
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> index 8645897..329d537 100644
> --- a/arch/powerpc/include/asm/firmware.h
> +++ b/arch/powerpc/include/asm/firmware.h
> @@ -51,6 +51,7 @@
>  #define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
>  #define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
>  #define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
> +#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000400000000)
> 
>  #ifndef __ASSEMBLY__
> 
> @@ -67,7 +68,7 @@ enum {
>  		FW_FEATURE_CMO | FW_FEATURE_VPHN | FW_FEATURE_XCMO |
>  		FW_FEATURE_SET_MODE | FW_FEATURE_BEST_ENERGY |
>  		FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
> -		FW_FEATURE_HPT_RESIZE,
> +		FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRC_INFO,
>  	FW_FEATURE_PSERIES_ALWAYS = 0,
>  	FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL,
>  	FW_FEATURE_POWERNV_ALWAYS = 0,
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 825bd59..3243455 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -175,6 +175,7 @@ struct of_drconf_cell {
>  #define OV5_HASH_GTSE		0x1940	/* Guest Translation Shoot Down Avail */
>  /* Radix Table Extensions */
>  #define OV5_RADIX_GTSE		0x1A40	/* Guest Translation Shoot Down Avail */
> +#define OV5_DRC_INFO		0x1640	/* Redef Prop Structures: drc-info   */
> 
>  /* Option Vector 6: IBM PAPR hints */
>  #define OV6_LINUX		0x02	/* Linux is our OS */
> diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
> index 63cc82a..757d757 100644
> --- a/arch/powerpc/platforms/pseries/firmware.c
> +++ b/arch/powerpc/platforms/pseries/firmware.c
> @@ -114,6 +114,7 @@ struct vec5_fw_feature {
>  vec5_fw_features_table[] = {
>  	{FW_FEATURE_TYPE1_AFFINITY,	OV5_TYPE1_AFFINITY},
>  	{FW_FEATURE_PRRN,		OV5_PRRN},
> +	{FW_FEATURE_DRC_INFO,		OV5_DRC_INFO},
>  };
> 
>  static void __init fw_vec5_feature_init(const char *vec5, unsigned long len)
> 

Re: RESEND [PATCH V3 1/4] powerpc/firmware: Add definitions for new drc-info firmware feature

[Michael Bringmann]

On 11/16/2017 11:06 AM, Nathan Fontenot wrote:
> On 11/15/2017 12:09 PM, Michael Bringmann wrote:
>> Firmware Features: Define new bit flag representing the presence of
>> new device tree property "ibm,drc-info".  The flag is used to tell
>> the front end processor whether the Linux kernel supports the new
>> property, and by the front end processor to tell the Linux kernel
>> that the new property is present in the device tree.
> 
> This patch seems to be adding a bit for the drc-info feature so that
> we can use the firmware_has_feature() interface to determine if the
> device tree has the new ibm,drc-info properties.
> 
> I'm not sure what front-end processor you're referring to? Is this
> in reference to the architecture vector that is exchanged with firmware?

I was trying to be generic instead of writing pHyp, BMC, or other.
We can change the comment if it is misleading.

> 
> -Nathan
> 
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/firmware.h       |    3 ++-
>>  arch/powerpc/include/asm/prom.h           |    1 +
>>  arch/powerpc/platforms/pseries/firmware.c |    1 +
>>  3 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
>> index 8645897..329d537 100644
>> --- a/arch/powerpc/include/asm/firmware.h
>> +++ b/arch/powerpc/include/asm/firmware.h
>> @@ -51,6 +51,7 @@
>>  #define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
>>  #define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
>>  #define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
>> +#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000400000000)
>>
>>  #ifndef __ASSEMBLY__
>>
>> @@ -67,7 +68,7 @@ enum {
>>  		FW_FEATURE_CMO | FW_FEATURE_VPHN | FW_FEATURE_XCMO |
>>  		FW_FEATURE_SET_MODE | FW_FEATURE_BEST_ENERGY |
>>  		FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
>> -		FW_FEATURE_HPT_RESIZE,
>> +		FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRC_INFO,
>>  	FW_FEATURE_PSERIES_ALWAYS = 0,
>>  	FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL,
>>  	FW_FEATURE_POWERNV_ALWAYS = 0,
>> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> index 825bd59..3243455 100644
>> --- a/arch/powerpc/include/asm/prom.h
>> +++ b/arch/powerpc/include/asm/prom.h
>> @@ -175,6 +175,7 @@ struct of_drconf_cell {
>>  #define OV5_HASH_GTSE		0x1940	/* Guest Translation Shoot Down Avail */
>>  /* Radix Table Extensions */
>>  #define OV5_RADIX_GTSE		0x1A40	/* Guest Translation Shoot Down Avail */
>> +#define OV5_DRC_INFO		0x1640	/* Redef Prop Structures: drc-info   */
>>
>>  /* Option Vector 6: IBM PAPR hints */
>>  #define OV6_LINUX		0x02	/* Linux is our OS */
>> diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
>> index 63cc82a..757d757 100644
>> --- a/arch/powerpc/platforms/pseries/firmware.c
>> +++ b/arch/powerpc/platforms/pseries/firmware.c
>> @@ -114,6 +114,7 @@ struct vec5_fw_feature {
>>  vec5_fw_features_table[] = {
>>  	{FW_FEATURE_TYPE1_AFFINITY,	OV5_TYPE1_AFFINITY},
>>  	{FW_FEATURE_PRRN,		OV5_PRRN},
>> +	{FW_FEATURE_DRC_INFO,		OV5_DRC_INFO},
>>  };
>>
>>  static void __init fw_vec5_feature_init(const char *vec5, unsigned long len)
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com

RESEND [PATCH V3 2/4] pseries/drc-info: Search DRC properties for CPU indexes

[Michael Bringmann]

pseries/drc-info: Provide parallel routines to convert between
drc_index and CPU numbers at runtime, using the older device-tree
properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
and "ibm,drc-power-domains"), or the new property "ibm,drc-info".

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in V3:
  -- Some code compression and use of data structures for value passing.
---
 arch/powerpc/include/asm/prom.h                 |   15 ++
 arch/powerpc/platforms/pseries/of_helpers.c     |   60 ++++++++++
 arch/powerpc/platforms/pseries/pseries_energy.c |  139 ++++++++++++++++++-----
 3 files changed, 186 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 3243455..007430a 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -96,6 +96,21 @@ struct of_drconf_cell {
 #define DRCONF_MEM_AI_INVALID	0x00000040
 #define DRCONF_MEM_RESERVED	0x00000080
 
+struct of_drc_info {
+	char *drc_type;
+	char *drc_name_prefix;
+	u32 drc_index_start;
+	u32 drc_name_suffix_start;
+	u32 num_sequential_elems;
+	u32 sequential_inc;
+	u32 drc_power_domain;
+	u32 last_drc_index;
+};
+
+extern int of_one_drc_info(struct property **prop, void **curval,
+			struct of_drc_info *data);
+
+
 /*
  * There are two methods for telling firmware what our capabilities are.
  * Newer machines have an "ibm,client-architecture-support" method on the
diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 2798933..62dc8e9 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c
@@ -2,6 +2,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <asm/prom.h>
 
 #include "of_helpers.h"
 
@@ -36,3 +37,62 @@ struct device_node *pseries_of_derive_parent(const char *path)
 		kfree(parent_path);
 	return parent ? parent : ERR_PTR(-EINVAL);
 }
+
+
+/* Helper Routines to convert between drc_index to cpu numbers */
+
+int of_one_drc_info(struct property **prop, void **curval,
+			struct of_drc_info *data)
+{
+	const char *p;
+	const __be32 *p2;
+
+	if (!data)
+		return -EINVAL;
+
+	/* Get drc-type:encode-string */
+	p = data->drc_type = (*curval);
+	p = of_prop_next_string(*prop, p);
+	if (!p)
+		return -EINVAL;
+
+	/* Get drc-name-prefix:encode-string */
+	data->drc_name_prefix = (char *)p;
+	p = of_prop_next_string(*prop, p);
+	if (!p)
+		return -EINVAL;
+
+	/* Get drc-index-start:encode-int */
+	p2 = (const __be32 *)p;
+	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get/skip drc-name-suffix-start:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get number-sequential-elements:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get sequential-increment:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
+	if (!p2)
+		return -EINVAL;
+
+	/* Get/skip drc-power-domain:encode-int */
+	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
+	if (!p2)
+		return -EINVAL;
+
+	/* Should now know end of current entry */
+	(*curval) = (void *)p2;
+	data->last_drc_index = data->drc_index_start +
+		((data->num_sequential_elems-1)*data->sequential_inc);
+
+	return 0;
+}
+EXPORT_SYMBOL(of_one_drc_info);
diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index 35c891a..7160855 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c
@@ -22,6 +22,7 @@
 #include <asm/page.h>
 #include <asm/hvcall.h>
 #include <asm/firmware.h>
+#include <asm/prom.h>
 
 
 #define MODULE_VERS "1.0"
@@ -38,26 +39,65 @@
 static u32 cpu_to_drc_index(int cpu)
 {
 	struct device_node *dn = NULL;
-	const int *indexes;
-	int i;
+	int thread_index;
 	int rc = 1;
 	u32 ret = 0;
 
 	dn = of_find_node_by_path("/cpus");
 	if (dn == NULL)
 		goto err;
-	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
-	if (indexes == NULL)
-		goto err_of_node_put;
+
 	/* Convert logical cpu number to core number */
-	i = cpu_core_index_of_thread(cpu);
-	/*
-	 * The first element indexes[0] is the number of drc_indexes
-	 * returned in the list.  Hence i+1 will get the drc_index
-	 * corresponding to core number i.
-	 */
-	WARN_ON(i > indexes[0]);
-	ret = indexes[i + 1];
+	thread_index = cpu_core_index_of_thread(cpu);
+
+	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
+		struct property *info = NULL;
+		struct of_drc_info drc;
+		int j;
+		u32 num_set_entries;
+		void *value;
+
+		info = of_find_property(dn, "ibm,drc-info", NULL);
+		if (info == NULL)
+			goto err_of_node_put;
+
+		value = info->value;
+		value = (void *)of_prop_next_u32(info, value, &num_set_entries);
+		if (!value)
+			goto err_of_node_put;
+
+		for (j = 0; j < num_set_entries; j++) {
+
+			of_one_drc_info(&info, &value, &drc);
+			if (strncmp(drc.drc_type, "CPU", 3))
+				goto err;
+
+			if (thread_index < drc.last_drc_index)
+				break;
+
+			WARN_ON(((thread_index-drc.drc_index_start)%
+					drc.sequential_inc) != 0);
+		}
+		WARN_ON((drc.num_sequential_elems == 0) ||
+			(drc.sequential_inc == 0));
+
+		ret = drc.drc_index_start + (thread_index*drc.sequential_inc);
+	} else {
+		const __be32 *indexes;
+
+		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
+		if (indexes == NULL)
+			goto err_of_node_put;
+
+		/*
+		 * The first element indexes[0] is the number of drc_indexes
+		 * returned in the list.  Hence thread_index+1 will get the
+		 * drc_index corresponding to core number thread_index.
+		 */
+		WARN_ON(thread_index > indexes[0]);
+		ret = indexes[thread_index + 1];
+	}
+
 	rc = 0;
 
 err_of_node_put:
@@ -72,34 +112,77 @@ static int drc_index_to_cpu(u32 drc_index)
 {
 	struct device_node *dn = NULL;
 	const int *indexes;
-	int i, cpu = 0;
+	int thread_index = 0, cpu = 0;
 	int rc = 1;
 
 	dn = of_find_node_by_path("/cpus");
 	if (dn == NULL)
 		goto err;
-	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
-	if (indexes == NULL)
-		goto err_of_node_put;
-	/*
-	 * First element in the array is the number of drc_indexes
-	 * returned.  Search through the list to find the matching
-	 * drc_index and get the core number
-	 */
-	for (i = 0; i < indexes[0]; i++) {
-		if (indexes[i + 1] == drc_index)
+
+	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
+		struct property *info = NULL;
+		struct of_drc_info drc;
+		int j;
+		u32 num_set_entries;
+		void *value;
+
+		info = of_find_property(dn, "ibm,drc-info", NULL);
+		if (info == NULL)
+			goto err_of_node_put;
+
+		value = info->value;
+		value = (void *)of_prop_next_u32(info, value, &num_set_entries);
+		if (!value)
+			goto err_of_node_put;
+
+		for (j = 0; j < num_set_entries; j++) {
+
+			of_one_drc_info(&info, &value, &drc);
+			if (strncmp(drc.drc_type, "CPU", 3))
+				goto err;
+
+			WARN_ON(drc_index < drc.drc_index_start);
+			WARN_ON(((drc_index-drc.drc_index_start)%
+					drc.sequential_inc) != 0);
+
+			if (drc_index > drc.last_drc_index) {
+				cpu += drc.num_sequential_elems;
+				continue;
+			} else {
+				cpu += ((drc_index-drc.drc_index_start)/
+					drc.sequential_inc);
+			}
+
+			thread_index = cpu_first_thread_of_core(cpu);
+			rc = 0;
 			break;
+		}
+	} else {
+		unsigned long int i;
+
+		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
+		if (indexes == NULL)
+			goto err_of_node_put;
+		/*
+		 * First element in the array is the number of drc_indexes
+		 * returned.  Search through the list to find the matching
+		 * drc_index and get the core number
+		 */
+		for (i = 0; i < indexes[0]; i++) {
+			if (indexes[i + 1] == drc_index)
+				break;
+		}
+		/* Convert core number to logical cpu number */
+		thread_index = cpu_first_thread_of_core(i);
+		rc = 0;
 	}
-	/* Convert core number to logical cpu number */
-	cpu = cpu_first_thread_of_core(i);
-	rc = 0;
 
 err_of_node_put:
 	of_node_put(dn);
 err:
 	if (rc)
 		printk(KERN_WARNING "drc_index_to_cpu(%d) failed", drc_index);
-	return cpu;
+	return thread_index;
 }
 
 /*

Re: RESEND [PATCH V3 2/4] pseries/drc-info: Search DRC properties for CPU indexes

[Nathan Fontenot]

On 11/15/2017 12:09 PM, Michael Bringmann wrote:
> pseries/drc-info: Provide parallel routines to convert between
> drc_index and CPU numbers at runtime, using the older device-tree
> properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
> and "ibm,drc-power-domains"), or the new property "ibm,drc-info".
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in V3:
>   -- Some code compression and use of data structures for value passing.
> ---
>  arch/powerpc/include/asm/prom.h                 |   15 ++
>  arch/powerpc/platforms/pseries/of_helpers.c     |   60 ++++++++++
>  arch/powerpc/platforms/pseries/pseries_energy.c |  139 ++++++++++++++++++-----
>  3 files changed, 186 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 3243455..007430a 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -96,6 +96,21 @@ struct of_drconf_cell {
>  #define DRCONF_MEM_AI_INVALID	0x00000040
>  #define DRCONF_MEM_RESERVED	0x00000080
> 
> +struct of_drc_info {
> +	char *drc_type;
> +	char *drc_name_prefix;
> +	u32 drc_index_start;
> +	u32 drc_name_suffix_start;
> +	u32 num_sequential_elems;
> +	u32 sequential_inc;
> +	u32 drc_power_domain;
> +	u32 last_drc_index;
> +};
> +
> +extern int of_one_drc_info(struct property **prop, void **curval,
> +			struct of_drc_info *data);

I'm not sure if prom.h is where this really belongs but I also do
not see an existing header file that it really makes sense to put it in.
 
> +
> +
>  /*
>   * There are two methods for telling firmware what our capabilities are.
>   * Newer machines have an "ibm,client-architecture-support" method on the
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 2798933..62dc8e9 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -2,6 +2,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/of.h>
> +#include <asm/prom.h>
> 
>  #include "of_helpers.h"
> 
> @@ -36,3 +37,62 @@ struct device_node *pseries_of_derive_parent(const char *path)
>  		kfree(parent_path);
>  	return parent ? parent : ERR_PTR(-EINVAL);
>  }
> +
> +
> +/* Helper Routines to convert between drc_index to cpu numbers */
> +
> +int of_one_drc_info(struct property **prop, void **curval,
> +			struct of_drc_info *data)

Small nit, this should probably be of_read_drc_info_cell.
 
> +{
> +	const char *p;
> +	const __be32 *p2;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	/* Get drc-type:encode-string */
> +	p = data->drc_type = (*curval);
> +	p = of_prop_next_string(*prop, p);
> +	if (!p)
> +		return -EINVAL;
> +
> +	/* Get drc-name-prefix:encode-string */
> +	data->drc_name_prefix = (char *)p;
> +	p = of_prop_next_string(*prop, p);
> +	if (!p)
> +		return -EINVAL;
> +
> +	/* Get drc-index-start:encode-int */
> +	p2 = (const __be32 *)p;
> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get/skip drc-name-suffix-start:encode-int */

You're getting the suffix, should probably drop 'skip' in the comment.

> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get number-sequential-elements:encode-int */
> +	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get sequential-increment:encode-int */
> +	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Get/skip drc-power-domain:encode-int */

Same here.

> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
> +	if (!p2)
> +		return -EINVAL;
> +
> +	/* Should now know end of current entry */
> +	(*curval) = (void *)p2;
> +	data->last_drc_index = data->drc_index_start +
> +		((data->num_sequential_elems-1)*data->sequential_inc);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(of_one_drc_info);
> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> index 35c891a..7160855 100644
> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
> @@ -22,6 +22,7 @@
>  #include <asm/page.h>
>  #include <asm/hvcall.h>
>  #include <asm/firmware.h>
> +#include <asm/prom.h>
> 
> 
>  #define MODULE_VERS "1.0"
> @@ -38,26 +39,65 @@
>  static u32 cpu_to_drc_index(int cpu)
>  {
>  	struct device_node *dn = NULL;
> -	const int *indexes;
> -	int i;
> +	int thread_index;
>  	int rc = 1;
>  	u32 ret = 0;
> 
>  	dn = of_find_node_by_path("/cpus");
>  	if (dn == NULL)
>  		goto err;
> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> -	if (indexes == NULL)
> -		goto err_of_node_put;
> +
>  	/* Convert logical cpu number to core number */
> -	i = cpu_core_index_of_thread(cpu);
> -	/*
> -	 * The first element indexes[0] is the number of drc_indexes
> -	 * returned in the list.  Hence i+1 will get the drc_index
> -	 * corresponding to core number i.
> -	 */
> -	WARN_ON(i > indexes[0]);
> -	ret = indexes[i + 1];
> +	thread_index = cpu_core_index_of_thread(cpu);
> +
> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
> +		struct property *info = NULL;

No need to set this to NULL here.

> +		struct of_drc_info drc;
> +		int j;
> +		u32 num_set_entries;
> +		void *value;

For device tree values, which are BE by design, this would be better as
		const __be32 *value;

> +
> +		info = of_find_property(dn, "ibm,drc-info", NULL);
> +		if (info == NULL)
> +			goto err_of_node_put;
> +
> +		value = info->value;
> +		value = (void *)of_prop_next_u32(info, value, &num_set_entries);
> +		if (!value)
> +			goto err_of_node_put;
> +
> +		for (j = 0; j < num_set_entries; j++) {
> +
> +			of_one_drc_info(&info, &value, &drc);
> +			if (strncmp(drc.drc_type, "CPU", 3))
> +				goto err;
> +
> +			if (thread_index < drc.last_drc_index)
> +				break;
> +
> +			WARN_ON(((thread_index-drc.drc_index_start)%

Spaces around the '-' sign please.

> +					drc.sequential_inc) != 0);
> +		}
> +		WARN_ON((drc.num_sequential_elems == 0) ||
> +			(drc.sequential_inc == 0));

This warning seems like it would fit better in the routine that reads the
drc-info property values.

> +
> +		ret = drc.drc_index_start + (thread_index*drc.sequential_inc);

Spaces... '*'

> +	} else {
> +		const __be32 *indexes;
> +
> +		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> +		if (indexes == NULL)
> +			goto err_of_node_put;
> +
> +		/*
> +		 * The first element indexes[0] is the number of drc_indexes
> +		 * returned in the list.  Hence thread_index+1 will get the
> +		 * drc_index corresponding to core number thread_index.
> +		 */
> +		WARN_ON(thread_index > indexes[0]);
> +		ret = indexes[thread_index + 1];
> +	}
> +
>  	rc = 0;
> 
>  err_of_node_put:
> @@ -72,34 +112,77 @@ static int drc_index_to_cpu(u32 drc_index)
>  {
>  	struct device_node *dn = NULL;
>  	const int *indexes;
> -	int i, cpu = 0;
> +	int thread_index = 0, cpu = 0;
>  	int rc = 1;
> 
>  	dn = of_find_node_by_path("/cpus");
>  	if (dn == NULL)
>  		goto err;
> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> -	if (indexes == NULL)
> -		goto err_of_node_put;
> -	/*
> -	 * First element in the array is the number of drc_indexes
> -	 * returned.  Search through the list to find the matching
> -	 * drc_index and get the core number
> -	 */
> -	for (i = 0; i < indexes[0]; i++) {
> -		if (indexes[i + 1] == drc_index)
> +
> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
> +		struct property *info = NULL;
> +		struct of_drc_info drc;
> +		int j;
> +		u32 num_set_entries;
> +		void *value;
> +
> +		info = of_find_property(dn, "ibm,drc-info", NULL);
> +		if (info == NULL)
> +			goto err_of_node_put;
> +
> +		value = info->value;
> +		value = (void *)of_prop_next_u32(info, value, &num_set_entries);
> +		if (!value)
> +			goto err_of_node_put;
> +
> +		for (j = 0; j < num_set_entries; j++) {
> +
> +			of_one_drc_info(&info, &value, &drc);
> +			if (strncmp(drc.drc_type, "CPU", 3))
> +				goto err;
> +
> +			WARN_ON(drc_index < drc.drc_index_start);
> +			WARN_ON(((drc_index-drc.drc_index_start)%
> +					drc.sequential_inc) != 0);
> +
> +			if (drc_index > drc.last_drc_index) {
> +				cpu += drc.num_sequential_elems;
> +				continue;
> +			} else {

Since you do a continue in the if() part above you shouldn't need to
put this in an else block.

-Nathan

> +				cpu += ((drc_index-drc.drc_index_start)/
> +					drc.sequential_inc);
> +			}
> +
> +			thread_index = cpu_first_thread_of_core(cpu);
> +			rc = 0;
>  			break;
> +		}
> +	} else {
> +		unsigned long int i;
> +
> +		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
> +		if (indexes == NULL)
> +			goto err_of_node_put;
> +		/*
> +		 * First element in the array is the number of drc_indexes
> +		 * returned.  Search through the list to find the matching
> +		 * drc_index and get the core number
> +		 */
> +		for (i = 0; i < indexes[0]; i++) {
> +			if (indexes[i + 1] == drc_index)
> +				break;
> +		}
> +		/* Convert core number to logical cpu number */
> +		thread_index = cpu_first_thread_of_core(i);
> +		rc = 0;
>  	}
> -	/* Convert core number to logical cpu number */
> -	cpu = cpu_first_thread_of_core(i);
> -	rc = 0;
> 
>  err_of_node_put:
>  	of_node_put(dn);
>  err:
>  	if (rc)
>  		printk(KERN_WARNING "drc_index_to_cpu(%d) failed", drc_index);
> -	return cpu;
> +	return thread_index;
>  }
> 
>  /*
> 

Re: RESEND [PATCH V3 2/4] pseries/drc-info: Search DRC properties for CPU indexes

[Michael Bringmann]

See below.

On 11/16/2017 11:34 AM, Nathan Fontenot wrote:
> On 11/15/2017 12:09 PM, Michael Bringmann wrote:
>> pseries/drc-info: Provide parallel routines to convert between
>> drc_index and CPU numbers at runtime, using the older device-tree
>> properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
>> and "ibm,drc-power-domains"), or the new property "ibm,drc-info".
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in V3:
>>   -- Some code compression and use of data structures for value passing.
>> ---
>>  arch/powerpc/include/asm/prom.h                 |   15 ++
>>  arch/powerpc/platforms/pseries/of_helpers.c     |   60 ++++++++++
>>  arch/powerpc/platforms/pseries/pseries_energy.c |  139 ++++++++++++++++++-----
>>  3 files changed, 186 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> index 3243455..007430a 100644
>> --- a/arch/powerpc/include/asm/prom.h
>> +++ b/arch/powerpc/include/asm/prom.h
>> @@ -96,6 +96,21 @@ struct of_drconf_cell {
>>  #define DRCONF_MEM_AI_INVALID	0x00000040
>>  #define DRCONF_MEM_RESERVED	0x00000080
>>
>> +struct of_drc_info {
>> +	char *drc_type;
>> +	char *drc_name_prefix;
>> +	u32 drc_index_start;
>> +	u32 drc_name_suffix_start;
>> +	u32 num_sequential_elems;
>> +	u32 sequential_inc;
>> +	u32 drc_power_domain;
>> +	u32 last_drc_index;
>> +};
>> +
>> +extern int of_one_drc_info(struct property **prop, void **curval,
>> +			struct of_drc_info *data);
> 
> I'm not sure if prom.h is where this really belongs but I also do
> not see an existing header file that it really makes sense to put it in.

If you think of a better place, please let me know.
> 
>> +
>> +
>>  /*
>>   * There are two methods for telling firmware what our capabilities are.
>>   * Newer machines have an "ibm,client-architecture-support" method on the
>> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
>> index 2798933..62dc8e9 100644
>> --- a/arch/powerpc/platforms/pseries/of_helpers.c
>> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
>> @@ -2,6 +2,7 @@
>>  #include <linux/err.h>
>>  #include <linux/slab.h>
>>  #include <linux/of.h>
>> +#include <asm/prom.h>
>>
>>  #include "of_helpers.h"
>>
>> @@ -36,3 +37,62 @@ struct device_node *pseries_of_derive_parent(const char *path)
>>  		kfree(parent_path);
>>  	return parent ? parent : ERR_PTR(-EINVAL);
>>  }
>> +
>> +
>> +/* Helper Routines to convert between drc_index to cpu numbers */
>> +
>> +int of_one_drc_info(struct property **prop, void **curval,
>> +			struct of_drc_info *data)
> 
> Small nit, this should probably be of_read_drc_info_cell.

Okay.  Will change.

> 
>> +{
>> +	const char *p;
>> +	const __be32 *p2;
>> +
>> +	if (!data)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-type:encode-string */
>> +	p = data->drc_type = (*curval);
>> +	p = of_prop_next_string(*prop, p);
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-name-prefix:encode-string */
>> +	data->drc_name_prefix = (char *)p;
>> +	p = of_prop_next_string(*prop, p);
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	/* Get drc-index-start:encode-int */
>> +	p2 = (const __be32 *)p;
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_index_start);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get/skip drc-name-suffix-start:encode-int */
> 
> You're getting the suffix, should probably drop 'skip' in the comment.

Okay.

> 
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get number-sequential-elements:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get sequential-increment:encode-int */
>> +	p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Get/skip drc-power-domain:encode-int */
> 
> Same here.

Okay.

> 
>> +	p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
>> +	if (!p2)
>> +		return -EINVAL;
>> +
>> +	/* Should now know end of current entry */
>> +	(*curval) = (void *)p2;
>> +	data->last_drc_index = data->drc_index_start +
>> +		((data->num_sequential_elems-1)*data->sequential_inc);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(of_one_drc_info);
>> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
>> index 35c891a..7160855 100644
>> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
>> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
>> @@ -22,6 +22,7 @@
>>  #include <asm/page.h>
>>  #include <asm/hvcall.h>
>>  #include <asm/firmware.h>
>> +#include <asm/prom.h>
>>
>>
>>  #define MODULE_VERS "1.0"
>> @@ -38,26 +39,65 @@
>>  static u32 cpu_to_drc_index(int cpu)
>>  {
>>  	struct device_node *dn = NULL;
>> -	const int *indexes;
>> -	int i;
>> +	int thread_index;
>>  	int rc = 1;
>>  	u32 ret = 0;
>>
>>  	dn = of_find_node_by_path("/cpus");
>>  	if (dn == NULL)
>>  		goto err;
>> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> -	if (indexes == NULL)
>> -		goto err_of_node_put;
>> +
>>  	/* Convert logical cpu number to core number */
>> -	i = cpu_core_index_of_thread(cpu);
>> -	/*
>> -	 * The first element indexes[0] is the number of drc_indexes
>> -	 * returned in the list.  Hence i+1 will get the drc_index
>> -	 * corresponding to core number i.
>> -	 */
>> -	WARN_ON(i > indexes[0]);
>> -	ret = indexes[i + 1];
>> +	thread_index = cpu_core_index_of_thread(cpu);
>> +
>> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
>> +		struct property *info = NULL;
> 
> No need to set this to NULL here.

Okay.

> 
>> +		struct of_drc_info drc;
>> +		int j;
>> +		u32 num_set_entries;
>> +		void *value;
> 
> For device tree values, which are BE by design, this would be better as
> 		const __be32 *value;

Okay.

> 
>> +
>> +		info = of_find_property(dn, "ibm,drc-info", NULL);
>> +		if (info == NULL)
>> +			goto err_of_node_put;
>> +
>> +		value = info->value;
>> +		value = (void *)of_prop_next_u32(info, value, &num_set_entries);
>> +		if (!value)
>> +			goto err_of_node_put;
>> +
>> +		for (j = 0; j < num_set_entries; j++) {
>> +
>> +			of_one_drc_info(&info, &value, &drc);
>> +			if (strncmp(drc.drc_type, "CPU", 3))
>> +				goto err;
>> +
>> +			if (thread_index < drc.last_drc_index)
>> +				break;
>> +
>> +			WARN_ON(((thread_index-drc.drc_index_start)%
> 
> Spaces around the '-' sign please.

Okay.  Missed that one.

> 
>> +					drc.sequential_inc) != 0);
>> +		}
>> +		WARN_ON((drc.num_sequential_elems == 0) ||
>> +			(drc.sequential_inc == 0));
> 
> This warning seems like it would fit better in the routine that reads the
> drc-info property values.

Wanted to have minimal impact on original calling code.  Will remove this one.

> 
>> +
>> +		ret = drc.drc_index_start + (thread_index*drc.sequential_inc);
> 
> Spaces... '*'

Okay.

> 
>> +	} else {
>> +		const __be32 *indexes;
>> +
>> +		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> +		if (indexes == NULL)
>> +			goto err_of_node_put;
>> +
>> +		/*
>> +		 * The first element indexes[0] is the number of drc_indexes
>> +		 * returned in the list.  Hence thread_index+1 will get the
>> +		 * drc_index corresponding to core number thread_index.
>> +		 */
>> +		WARN_ON(thread_index > indexes[0]);
>> +		ret = indexes[thread_index + 1];
>> +	}
>> +
>>  	rc = 0;
>>
>>  err_of_node_put:
>> @@ -72,34 +112,77 @@ static int drc_index_to_cpu(u32 drc_index)
>>  {
>>  	struct device_node *dn = NULL;
>>  	const int *indexes;
>> -	int i, cpu = 0;
>> +	int thread_index = 0, cpu = 0;
>>  	int rc = 1;
>>
>>  	dn = of_find_node_by_path("/cpus");
>>  	if (dn == NULL)
>>  		goto err;
>> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> -	if (indexes == NULL)
>> -		goto err_of_node_put;
>> -	/*
>> -	 * First element in the array is the number of drc_indexes
>> -	 * returned.  Search through the list to find the matching
>> -	 * drc_index and get the core number
>> -	 */
>> -	for (i = 0; i < indexes[0]; i++) {
>> -		if (indexes[i + 1] == drc_index)
>> +
>> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
>> +		struct property *info = NULL;
>> +		struct of_drc_info drc;
>> +		int j;
>> +		u32 num_set_entries;
>> +		void *value;
>> +
>> +		info = of_find_property(dn, "ibm,drc-info", NULL);
>> +		if (info == NULL)
>> +			goto err_of_node_put;
>> +
>> +		value = info->value;
>> +		value = (void *)of_prop_next_u32(info, value, &num_set_entries);
>> +		if (!value)
>> +			goto err_of_node_put;
>> +
>> +		for (j = 0; j < num_set_entries; j++) {
>> +
>> +			of_one_drc_info(&info, &value, &drc);
>> +			if (strncmp(drc.drc_type, "CPU", 3))
>> +				goto err;
>> +
>> +			WARN_ON(drc_index < drc.drc_index_start);
>> +			WARN_ON(((drc_index-drc.drc_index_start)%
>> +					drc.sequential_inc) != 0);
>> +
>> +			if (drc_index > drc.last_drc_index) {
>> +				cpu += drc.num_sequential_elems;
>> +				continue;
>> +			} else {
> 
> Since you do a continue in the if() part above you shouldn't need to
> put this in an else block.

Okay.

> 
> -Nathan
> 
>> +				cpu += ((drc_index-drc.drc_index_start)/
>> +					drc.sequential_inc);
>> +			}
>> +
>> +			thread_index = cpu_first_thread_of_core(cpu);
>> +			rc = 0;
>>  			break;
>> +		}
>> +	} else {
>> +		unsigned long int i;
>> +
>> +		indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> +		if (indexes == NULL)
>> +			goto err_of_node_put;
>> +		/*
>> +		 * First element in the array is the number of drc_indexes
>> +		 * returned.  Search through the list to find the matching
>> +		 * drc_index and get the core number
>> +		 */
>> +		for (i = 0; i < indexes[0]; i++) {
>> +			if (indexes[i + 1] == drc_index)
>> +				break;
>> +		}
>> +		/* Convert core number to logical cpu number */
>> +		thread_index = cpu_first_thread_of_core(i);
>> +		rc = 0;
>>  	}
>> -	/* Convert core number to logical cpu number */
>> -	cpu = cpu_first_thread_of_core(i);
>> -	rc = 0;
>>
>>  err_of_node_put:
>>  	of_node_put(dn);
>>  err:
>>  	if (rc)
>>  		printk(KERN_WARNING "drc_index_to_cpu(%d) failed", drc_index);
>> -	return cpu;
>> +	return thread_index;
>>  }
>>
>>  /*
>>
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com

RESEND [PATCH V3 3/4] hotplug/drc-info: Add code to search ibm,drc-info property

[Michael Bringmann]

rpadlpar_core.c: Provide parallel routines to search the older device-
tree properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
and "ibm,drc-power-domains"), or the new property "ibm,drc-info".

The interface to examine the DRC information is changed from a "get"
function that returns values for local verification elsewhere, to a
"check" function that validates the 'name' and/or 'type' of a device
node.  This update hides the format of the underlying device-tree
properties, and concentrates the value checks into a single function
without requiring the user to verify whether a search was successful.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in V3:
  -- Now passing more values by structure reducing use of local
     declarations / initialization.
  -- Improve some code spacing for better clarity.
---
 drivers/pci/hotplug/rpadlpar_core.c |   13 ++--
 drivers/pci/hotplug/rpaphp.h        |    4 +
 drivers/pci/hotplug/rpaphp_core.c   |  110 +++++++++++++++++++++++++++--------
 3 files changed, 92 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index a3449d7..fc01d7d 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -27,6 +27,7 @@
 #include <linux/mutex.h>
 #include <asm/rtas.h>
 #include <asm/vio.h>
+#include <linux/firmware.h>
 
 #include "../pci.h"
 #include "rpaphp.h"
@@ -44,15 +45,14 @@ static struct device_node *find_vio_slot_node(char *drc_name)
 {
 	struct device_node *parent = of_find_node_by_name(NULL, "vdevice");
 	struct device_node *dn = NULL;
-	char *name;
 	int rc;
 
 	if (!parent)
 		return NULL;
 
 	while ((dn = of_get_next_child(parent, dn))) {
-		rc = rpaphp_get_drc_props(dn, NULL, &name, NULL, NULL);
-		if ((rc == 0) && (!strcmp(drc_name, name)))
+		rc = rpaphp_check_drc_props(dn, drc_name, NULL);
+		if (rc == 0)
 			break;
 	}
 
@@ -64,15 +64,12 @@ static struct device_node *find_php_slot_pci_node(char *drc_name,
 						  char *drc_type)
 {
 	struct device_node *np = NULL;
-	char *name;
-	char *type;
 	int rc;
 
 	while ((np = of_find_node_by_name(np, "pci"))) {
-		rc = rpaphp_get_drc_props(np, NULL, &name, &type, NULL);
+		rc = rpaphp_check_drc_props(np, drc_name, drc_type);
 		if (rc == 0)
-			if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
-				break;
+			break;
 	}
 
 	return np;
diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
index 7db024e..8db5f2e 100644
--- a/drivers/pci/hotplug/rpaphp.h
+++ b/drivers/pci/hotplug/rpaphp.h
@@ -91,8 +91,8 @@ struct slot {
 
 /* rpaphp_core.c */
 int rpaphp_add_slot(struct device_node *dn);
-int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
-		char **drc_name, char **drc_type, int *drc_power_domain);
+int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
+		char *drc_type);
 
 /* rpaphp_slot.c */
 void dealloc_slot_struct(struct slot *slot);
diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index 1e29aba..6606175 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c
@@ -30,6 +30,7 @@
 #include <linux/smp.h>
 #include <linux/init.h>
 #include <linux/vmalloc.h>
+#include <asm/firmware.h>
 #include <asm/eeh.h>       /* for eeh_add_device() */
 #include <asm/rtas.h>		/* rtas_call */
 #include <asm/pci-bridge.h>	/* for pci_controller */
@@ -196,25 +197,21 @@ static int get_children_props(struct device_node *dn, const int **drc_indexes,
 	return 0;
 }
 
-/* To get the DRC props describing the current node, first obtain it's
- * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
- * the my-drc-index for correlation, and obtain the requested properties.
+
+/* Verify the existence of 'drc_name' and/or 'drc_type' within the
+ * current node.  First obtain it's my-drc-index property.  Next,
+ * obtain the DRC info from it's parent.  Use the my-drc-index for
+ * correlation, and obtain/validate the requested properties.
  */
-int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
-		char **drc_name, char **drc_type, int *drc_power_domain)
+
+static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
+				char *drc_type, unsigned int my_index)
 {
+	char *name_tmp, *type_tmp;
 	const int *indexes, *names;
 	const int *types, *domains;
-	const unsigned int *my_index;
-	char *name_tmp, *type_tmp;
 	int i, rc;
 
-	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
-	if (!my_index) {
-		/* Node isn't DLPAR/hotplug capable */
-		return -EINVAL;
-	}
-
 	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
 	if (rc < 0) {
 		return -EINVAL;
@@ -225,24 +222,87 @@ int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
 
 	/* Iterate through parent properties, looking for my-drc-index */
 	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
-		if ((unsigned int) indexes[i + 1] == *my_index) {
-			if (drc_name)
-				*drc_name = name_tmp;
-			if (drc_type)
-				*drc_type = type_tmp;
-			if (drc_index)
-				*drc_index = be32_to_cpu(*my_index);
-			if (drc_power_domain)
-				*drc_power_domain = be32_to_cpu(domains[i+1]);
-			return 0;
-		}
+		if ((unsigned int) indexes[i + 1] == my_index)
+			break;
+
 		name_tmp += (strlen(name_tmp) + 1);
 		type_tmp += (strlen(type_tmp) + 1);
 	}
 
+	if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
+	    ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
+		return 0;
+
+	return -EINVAL;
+}
+
+static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
+				char *drc_type, unsigned int my_index)
+{
+	struct property *info;
+	unsigned int entries;
+	struct of_drc_info drc;
+	void *value;
+	int j;
+
+	info = of_find_property(dn->parent, "ibm,drc-info", NULL);
+	if (info == NULL)
+		return -EINVAL;
+
+	value = info->value;
+	value = (void *)of_prop_next_u32(info, value, &entries);
+	if (!value)
+		return -EINVAL;
+
+	for (j = 0; j < entries; j++) {
+		of_one_drc_info(&info, &value, &drc);
+
+		/* Should now know end of current entry */
+
+		WARN_ON((my_index < drc.drc_index_start) ||
+			(((my_index-drc.drc_index_start)%
+				drc.sequential_inc) != 0));
+
+		if (my_index > drc.last_drc_index)
+			continue;
+
+		break;
+	}
+	/* Found it */
+
+	if (((drc_name == NULL) ||
+	     (drc_name && !strncmp(drc_name,
+				drc.drc_name_prefix,
+				strlen(drc.drc_name_prefix)))) &&
+	    ((drc_type == NULL) ||
+	     (drc_type && !strncmp(drc_type,
+				drc.drc_type,
+				strlen(drc.drc_type)))))
+		return 0;
+
 	return -EINVAL;
 }
-EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
+
+int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
+			char *drc_type)
+{
+	const unsigned int *my_index;
+
+	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
+	if (!my_index) {
+		/* Node isn't DLPAR/hotplug capable */
+		return -EINVAL;
+	}
+
+	if (firmware_has_feature(FW_FEATURE_DRC_INFO))
+		return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
+						*my_index);
+	else
+		return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
+						*my_index);
+}
+EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
+
 
 static int is_php_type(char *drc_type)
 {

Re: RESEND [PATCH V3 3/4] hotplug/drc-info: Add code to search ibm,drc-info property

[Nathan Fontenot]

On 11/15/2017 12:09 PM, Michael Bringmann wrote:
> rpadlpar_core.c: Provide parallel routines to search the older device-
> tree properties ("ibm,drc-indexes", "ibm,drc-names", "ibm,drc-types"
> and "ibm,drc-power-domains"), or the new property "ibm,drc-info".
> 
> The interface to examine the DRC information is changed from a "get"
> function that returns values for local verification elsewhere, to a
> "check" function that validates the 'name' and/or 'type' of a device
> node.  This update hides the format of the underlying device-tree
> properties, and concentrates the value checks into a single function
> without requiring the user to verify whether a search was successful.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in V3:
>   -- Now passing more values by structure reducing use of local
>      declarations / initialization.
>   -- Improve some code spacing for better clarity.
> ---
>  drivers/pci/hotplug/rpadlpar_core.c |   13 ++--
>  drivers/pci/hotplug/rpaphp.h        |    4 +
>  drivers/pci/hotplug/rpaphp_core.c   |  110 +++++++++++++++++++++++++++--------
>  3 files changed, 92 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index a3449d7..fc01d7d 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -27,6 +27,7 @@
>  #include <linux/mutex.h>
>  #include <asm/rtas.h>
>  #include <asm/vio.h>
> +#include <linux/firmware.h>
> 
>  #include "../pci.h"
>  #include "rpaphp.h"
> @@ -44,15 +45,14 @@ static struct device_node *find_vio_slot_node(char *drc_name)
>  {
>  	struct device_node *parent = of_find_node_by_name(NULL, "vdevice");
>  	struct device_node *dn = NULL;
> -	char *name;
>  	int rc;
> 
>  	if (!parent)
>  		return NULL;
> 
>  	while ((dn = of_get_next_child(parent, dn))) {
> -		rc = rpaphp_get_drc_props(dn, NULL, &name, NULL, NULL);
> -		if ((rc == 0) && (!strcmp(drc_name, name)))
> +		rc = rpaphp_check_drc_props(dn, drc_name, NULL);
> +		if (rc == 0)
>  			break;
>  	}
> 
> @@ -64,15 +64,12 @@ static struct device_node *find_php_slot_pci_node(char *drc_name,
>  						  char *drc_type)
>  {
>  	struct device_node *np = NULL;
> -	char *name;
> -	char *type;
>  	int rc;
> 
>  	while ((np = of_find_node_by_name(np, "pci"))) {
> -		rc = rpaphp_get_drc_props(np, NULL, &name, &type, NULL);
> +		rc = rpaphp_check_drc_props(np, drc_name, drc_type);
>  		if (rc == 0)
> -			if (!strcmp(drc_name, name) && !strcmp(drc_type, type))
> -				break;
> +			break;
>  	}
> 
>  	return np;
> diff --git a/drivers/pci/hotplug/rpaphp.h b/drivers/pci/hotplug/rpaphp.h
> index 7db024e..8db5f2e 100644
> --- a/drivers/pci/hotplug/rpaphp.h
> +++ b/drivers/pci/hotplug/rpaphp.h
> @@ -91,8 +91,8 @@ struct slot {
> 
>  /* rpaphp_core.c */
>  int rpaphp_add_slot(struct device_node *dn);
> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> -		char **drc_name, char **drc_type, int *drc_power_domain);
> +int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
> +		char *drc_type);
> 
>  /* rpaphp_slot.c */
>  void dealloc_slot_struct(struct slot *slot);
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index 1e29aba..6606175 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -30,6 +30,7 @@
>  #include <linux/smp.h>
>  #include <linux/init.h>
>  #include <linux/vmalloc.h>
> +#include <asm/firmware.h>
>  #include <asm/eeh.h>       /* for eeh_add_device() */
>  #include <asm/rtas.h>		/* rtas_call */
>  #include <asm/pci-bridge.h>	/* for pci_controller */
> @@ -196,25 +197,21 @@ static int get_children_props(struct device_node *dn, const int **drc_indexes,
>  	return 0;
>  }
> 
> -/* To get the DRC props describing the current node, first obtain it's
> - * my-drc-index property.  Next obtain the DRC list from it's parent.  Use
> - * the my-drc-index for correlation, and obtain the requested properties.
> +
> +/* Verify the existence of 'drc_name' and/or 'drc_type' within the
> + * current node.  First obtain it's my-drc-index property.  Next,
> + * obtain the DRC info from it's parent.  Use the my-drc-index for
> + * correlation, and obtain/validate the requested properties.
>   */
> -int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> -		char **drc_name, char **drc_type, int *drc_power_domain)
> +
> +static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
> +				char *drc_type, unsigned int my_index)
>  {
> +	char *name_tmp, *type_tmp;
>  	const int *indexes, *names;
>  	const int *types, *domains;
> -	const unsigned int *my_index;
> -	char *name_tmp, *type_tmp;
>  	int i, rc;
> 
> -	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> -	if (!my_index) {
> -		/* Node isn't DLPAR/hotplug capable */
> -		return -EINVAL;
> -	}
> -
>  	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>  	if (rc < 0) {
>  		return -EINVAL;
> @@ -225,24 +222,87 @@ int rpaphp_get_drc_props(struct device_node *dn, int *drc_index,
> 
>  	/* Iterate through parent properties, looking for my-drc-index */
>  	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> -		if ((unsigned int) indexes[i + 1] == *my_index) {
> -			if (drc_name)
> -				*drc_name = name_tmp;
> -			if (drc_type)
> -				*drc_type = type_tmp;
> -			if (drc_index)
> -				*drc_index = be32_to_cpu(*my_index);
> -			if (drc_power_domain)
> -				*drc_power_domain = be32_to_cpu(domains[i+1]);
> -			return 0;
> -		}
> +		if ((unsigned int) indexes[i + 1] == my_index)
> +			break;
> +
>  		name_tmp += (strlen(name_tmp) + 1);
>  		type_tmp += (strlen(type_tmp) + 1);
>  	}
> 
> +	if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
> +	    ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
> +		return 0;
> +
> +	return -EINVAL;
> +}
> +
> +static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
> +				char *drc_type, unsigned int my_index)
> +{
> +	struct property *info;
> +	unsigned int entries;
> +	struct of_drc_info drc;
> +	void *value;

This should be __be32 *

> +	int j;
> +
> +	info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> +	if (info == NULL)
> +		return -EINVAL;
> +
> +	value = info->value;
> +	value = (void *)of_prop_next_u32(info, value, &entries);
> +	if (!value)
> +		return -EINVAL;
> +
> +	for (j = 0; j < entries; j++) {
> +		of_one_drc_info(&info, &value, &drc);
> +
> +		/* Should now know end of current entry */
> +
> +		WARN_ON((my_index < drc.drc_index_start) ||
> +			(((my_index-drc.drc_index_start)%
> +				drc.sequential_inc) != 0));
> +
> +		if (my_index > drc.last_drc_index)
> +			continue;
> +
> +		break;
> +	}
> +	/* Found it */
> +
> +	if (((drc_name == NULL) ||
> +	     (drc_name && !strncmp(drc_name,
> +				drc.drc_name_prefix,
> +				strlen(drc.drc_name_prefix)))) &&

Shouldn't we be doing a string compare on the entire name, not just the prefix?

If I remember correctly the prefix is the same for every cpu.

-Nathan

> +	    ((drc_type == NULL) ||
> +	     (drc_type && !strncmp(drc_type,
> +				drc.drc_type,
> +				strlen(drc.drc_type)))))
> +		return 0;
> +
>  	return -EINVAL;
>  }
> -EXPORT_SYMBOL_GPL(rpaphp_get_drc_props);
> +
> +int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
> +			char *drc_type)
> +{
> +	const unsigned int *my_index;
> +
> +	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
> +	if (!my_index) {
> +		/* Node isn't DLPAR/hotplug capable */
> +		return -EINVAL;
> +	}
> +
> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO))
> +		return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
> +						*my_index);
> +	else
> +		return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
> +						*my_index);
> +}
> +EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
> +
> 
>  static int is_php_type(char *drc_type)
>  {
> 

Re: RESEND [PATCH V3 3/4] hotplug/drc-info: Add code to search ibm,drc-info property

[Michael Bringmann]

>> +
>> +static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
>> +				char *drc_type, unsigned int my_index)
>> +{
>> +	struct property *info;
>> +	unsigned int entries;
>> +	struct of_drc_info drc;
>> +	void *value;
> 
> This should be __be32 *

Okay.

> 
>> +	int j;
>> +
>> +	info = of_find_property(dn->parent, "ibm,drc-info", NULL);
>> +	if (info == NULL)
>> +		return -EINVAL;
>> +
>> +	value = info->value;
>> +	value = (void *)of_prop_next_u32(info, value, &entries);
>> +	if (!value)
>> +		return -EINVAL;
>> +
>> +	for (j = 0; j < entries; j++) {
>> +		of_one_drc_info(&info, &value, &drc);
>> +
>> +		/* Should now know end of current entry */
>> +
>> +		WARN_ON((my_index < drc.drc_index_start) ||
>> +			(((my_index-drc.drc_index_start)%
>> +				drc.sequential_inc) != 0));
>> +
>> +		if (my_index > drc.last_drc_index)
>> +			continue;
>> +
>> +		break;
>> +	}
>> +	/* Found it */
>> +
>> +	if (((drc_name == NULL) ||
>> +	     (drc_name && !strncmp(drc_name,
>> +				drc.drc_name_prefix,
>> +				strlen(drc.drc_name_prefix)))) &&
> 
> Shouldn't we be doing a string compare on the entire name, not just the prefix?
> 
> If I remember correctly the prefix is the same for every cpu.

The prefix may be a value like "CPU", "MEM", "PHB", or other.
I modeled the comparisons using 'drc_name_prefix' after the comparison
of the 'name_tmp' found in the array 'ibm,drc-names' and 'type_tmp'
found in the array 'ibm,drc-types'.  This is modeled in the new
function 'rpaphp_check_drc_props_v1' which was lifted from the original
function rpaphp_get_drc_props().

> 
> -Nathan
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com

RESEND [PATCH V3 4/4] powerpc: Enable support for ibm,drc-info devtree property

[Michael Bringmann]

prom_init.c: Enable support for new DRC device tree property
"ibm,drc-info" in initial handshake between the Linux kernel and
the front end processor.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/prom_init.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 02190e9..f962908 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -873,6 +873,7 @@ struct ibm_arch_vec __cacheline_aligned ibm_architecture_vec = {
 		.mmu = 0,
 		.hash_ext = 0,
 		.radix_ext = 0,
+		.byte22 = OV5_FEAT(OV5_DRC_INFO),
 	},
 
 	/* option vector 6: IBM PAPR hints */