[PATCH 0/9] Fixes and Enablement of ibm,drc-info property

There was a previous effort to add support for the PAPR
architected ibm,drc-info property. This property provides a more
memory compact representation of a paritions Dynamic Reconfig
Connectors (DRC). These can otherwise be thought of as currently
partitioned, or available but yet to be partitioned system resources
such as cpus, memory, and physical/logical IOA devices.

The initial implementation proved buggy and was fully turned of by
disabling the bit in the appropriate CAS support vector. We now have
PowerVM firmware in the field that supports this new property, and 
further to suppport partitions with 24TB+ of possible memory this
property is required to perform platform migration.

This serious fixs the short comings of the previous submission
in the areas of general implementation, cpu hotplug, and IOA hotplug.

Tyrel Datwyler (9):
  powerpc/pseries: Fix bad drc_index_start value parsing of drc-info
    entry
  powerpc/pseries: Fix drc-info mappings of logical cpus to drc-index
  powerpc/pseries: Add cpu DLPAR support for drc-info property
  PCI: rpaphp: Fix up pointer to first drc-info entry
  PCI: rpaphp: Don't rely on firmware feature to imply drc-info support
  PCI: rpaphp: Add drc-info support for hotplug slot registration
  PCI: rpaphp: annotate and correctly byte swap DRC properties
  PCI: rpaphp: Correctly match ibm,my-drc-index to drc-name when using
    drc-info
  powerpc/pseries: Enable support for ibm,drc-info property

 arch/powerpc/kernel/prom_init.c                 |   2 +-
 arch/powerpc/platforms/pseries/hotplug-cpu.c    | 101 ++++++++++++++++---
 arch/powerpc/platforms/pseries/of_helpers.c     |   8 +-
 arch/powerpc/platforms/pseries/pseries_energy.c |  23 ++---
 drivers/pci/hotplug/rpaphp_core.c               | 124 +++++++++++++++++-------
 5 files changed, 187 insertions(+), 71 deletions(-)

-- 
2.7.4

In the event that the partition is migrated to a platform with older
firmware that doesn't support the ibm,drc-info property the device
tree is modified to remove the ibm,drc-info property and replace it
with the older style ibm,drc-* properties for types, names, indexes,
and power-domains. One of the requirements of the drc-info firmware
feature is the the client is able to handle both the new property,
and old properties. Therefore we can't rely on the firmware feature
alone to dictate which property is currently present in the device
tree.

Fix this short coming by checking explicitly for the ibm,drc-info
property, and falling back to the older ibm,drc-* properties if it
doesn't exist.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index e350264..e18e9a0 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c

@@ -275,7 +275,7 @@ int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
 		return -EINVAL;
 	}
 
-	if (firmware_has_feature(FW_FEATURE_DRC_INFO))
+	if (of_find_property(dn->parent, "ibm,drc-info", NULL))
 		return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
 						*my_index);
 	else

-- 
2.7.4

From: Tyrel Datwyler <redacted>

Older firmwares provided information about Dynamic Reconfig
Connectors (DRC) through several device tree properties, namely
ibm,drc-types, ibm,drc-indexes, ibm,drc-names, and
ibm,drc-power-domains. New firmwares have the ability to present this
same information in a much condensed format through a device tree
property called ibm,drc-info.

The existing cpu DLPAR hotplug code only understands the older DRC
property format when validating the drc-index of a cpu during a
hotplug add. This updates those code paths to use the ibm,drc-info
property, when present, instead for validation.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 101 ++++++++++++++++++++++-----
 1 file changed, 85 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index bbda646..9ba006c 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c

@@ -407,17 +407,58 @@ static bool dlpar_cpu_exists(struct device_node *parent, u32 drc_index)
 	return found;
 }
 
+static bool drc_info_valid_index(struct device_node *parent, u32 drc_index)
+{
+	struct property *info;
+	struct of_drc_info drc;
+	const __be32 *value;
+	int count, i, j;
+
+	info = of_find_property(parent, "ibm,drc-info", NULL);
+	if (!info)
+		return false;
+
+	value = of_prop_next_u32(info, NULL, &count);
+
+	/* First value of ibm,drc-info is number of drc-info records */
+	if (value)
+		value++;
+	else
+		return false;
+
+	for (i = 0; i < count; i++) {
+		if (of_read_drc_info_cell(&info, &value, &drc))
+			return false;
+
+		if (strncmp(drc.drc_type, "CPU", 3))
+			break;
+
+		if (drc_index > drc.last_drc_index)
+			continue;
+
+		for (j = 0; j < drc.num_sequential_elems; j++)
+			if (drc_index == (drc.drc_index_start + (drc.sequential_inc * j)))
+					return true;
+	}
+
+	return false;
+}
+
 static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
 {
 	bool found = false;
 	int rc, index;
 
-	index = 0;
+	if (of_find_property(parent, "ibm,drc-info", NULL))
+		return drc_info_valid_index(parent, drc_index);
+
+	index = 1;
 	while (!found) {
 		u32 drc;
 
 		rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
 						index++, &drc);
+
 		if (rc)
 			break;
 

@@ -720,8 +761,11 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
 static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
 {
 	struct device_node *parent;
+	struct property *info;
 	int cpus_found = 0;
 	int index, rc;
+	int i, j;
+	u32 drc_index;
 
 	parent = of_find_node_by_path("/cpus");
 	if (!parent) {

@@ -730,24 +774,49 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
 		return -1;
 	}
 
-	/* Search the ibm,drc-indexes array for possible CPU drcs to
-	 * add. Note that the format of the ibm,drc-indexes array is
-	 * the number of entries in the array followed by the array
-	 * of drc values so we start looking at index = 1.
-	 */
-	index = 1;
-	while (cpus_found < cpus_to_add) {
-		u32 drc;
+	info = of_find_property(parent, "ibm,drc-info", NULL);
+	if (info) {
+		struct of_drc_info drc;
+		const __be32 *value;
+		int count;
 
-		rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
-						index++, &drc);
-		if (rc)
-			break;
+		value = of_prop_next_u32(info, NULL, &count);
+		if (value)
+			value++;
 
-		if (dlpar_cpu_exists(parent, drc))
-			continue;
+		for (i = 0; i < count; i++) {
+			of_read_drc_info_cell(&info, &value, &drc);
+			if (strncmp(drc.drc_type, "CPU", 3))
+				break;
+
+			for (j = 0; j < drc.num_sequential_elems && cpus_found < cpus_to_add; j++) {
+				drc_index = drc.drc_index_start + (drc.sequential_inc * j);
+
+				if (dlpar_cpu_exists(parent, drc_index))
+					continue;
+
+				cpu_drcs[cpus_found++] = drc_index;
+			}
+		}
+	} else {
+		/* Search the ibm,drc-indexes array for possible CPU drcs to
+		 * add. Note that the format of the ibm,drc-indexes array is
+		 * the number of entries in the array followed by the array
+		 * of drc values so we start looking at index = 1.
+		 */
+		index = 1;
+		while (cpus_found < cpus_to_add) {
+			rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
+							index++, &drc_index);
+
+			if (rc)
+				break;
 
-		cpu_drcs[cpus_found++] = drc;
+			if (dlpar_cpu_exists(parent, drc_index))
+				continue;
+
+			cpu_drcs[cpus_found++] = drc_index;
+		}
 	}
 
 	of_node_put(parent);

-- 
2.7.4

The ibm,drc-info property is an array property that contains drc-info
entries such that each entry is made up of 2 string encoded elements
followed by 5 int encoded elements. The of_read_drc_info_cell()
helper contains comments that correctly name the expected elements
and their encoding. However, the usage of of_prop_next_string() and
of_prop_next_u32() introduced a subtle skippage of the first u32.
This is a result of of_prop_next_string() returning a pointer to the
next property value which is not a string, but actually a (__be32 *).
As, a result the following call to of_prop_next_u32() passes over the
current int encoded value and actually stores the next one wrongly.

Simply endian swap the current value in place after reading the first
two string values. The remaining int encoded values can then be read
correctly using of_prop_next_u32().

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/of_helpers.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
index 6df192f..66dfd82 100644
--- a/arch/powerpc/platforms/pseries/of_helpers.c
+++ b/arch/powerpc/platforms/pseries/of_helpers.c

@@ -45,14 +45,14 @@ struct device_node *pseries_of_derive_parent(const char *path)
 int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
 			struct of_drc_info *data)
 {
-	const char *p;
+	const char *p = (char *)(*curval);
 	const __be32 *p2;
 
 	if (!data)
 		return -EINVAL;
 
 	/* Get drc-type:encode-string */
-	p = data->drc_type = (char*) (*curval);
+	data->drc_type = (char *)p;
 	p = of_prop_next_string(*prop, p);
 	if (!p)
 		return -EINVAL;

@@ -65,9 +65,7 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
 
 	/* 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;
+	data->drc_index_start = be32_to_cpu(*p2);
 
 	/* Get drc-name-suffix-start:encode-int */
 	p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);

-- 
2.7.4

Split physical PCI slot registration scanning into seperate routines
that support the old ibm,drc-* properties and one that supports the
new compressed ibm,drc-info property.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c | 89 ++++++++++++++++++++++++++++++---------
 1 file changed, 69 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index e18e9a0..75d5771 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c

@@ -328,23 +328,48 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
 	return 1;
 }
 
-/**
- * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
- * @dn: device node of slot
- *
- * This subroutine will register a hotpluggable slot with the
- * PCI hotplug infrastructure. This routine is typically called
- * during boot time, if the hotplug slots are present at boot time,
- * or is called later, by the dlpar add code, if the slot is
- * being dynamically added during runtime.
- *
- * If the device node points at an embedded (built-in) slot, this
- * routine will just return without doing anything, since embedded
- * slots cannot be hotplugged.
- *
- * To remove a slot, it suffices to call rpaphp_deregister_slot().
- */
-int rpaphp_add_slot(struct device_node *dn)
+static int rpaphp_drc_info_add_slot(struct device_node *dn)
+{
+	struct slot *slot;
+	struct property *info;
+	struct of_drc_info drc;
+	char drc_name[MAX_DRC_NAME_LEN];
+	const __be32 *cur;
+	u32 count;
+	int retval = 0;
+
+	info = of_find_property(dn, "ibm,drc-info", NULL);
+	if (!info)
+		return 0;
+
+	cur = of_prop_next_u32(info, NULL, &count);
+	if (cur)
+		cur++;
+	else
+		return 0;
+
+	of_read_drc_info_cell(&info, &cur, &drc);
+	if (!is_php_type(drc.drc_type))
+		return 0;
+
+	sprintf(drc_name, "%s%d", drc.drc_name_prefix, drc.drc_name_suffix_start);
+
+	slot = alloc_slot_struct(dn, drc.drc_index_start, drc_name, drc.drc_power_domain);
+	if (!slot)
+		return -ENOMEM;
+
+	slot->type = simple_strtoul(drc.drc_type, NULL, 10);
+	retval = rpaphp_enable_slot(slot);
+	if (!retval)
+		retval = rpaphp_register_slot(slot);
+
+	if (retval)
+		dealloc_slot_struct(slot);
+
+	return retval;
+}
+
+static int rpaphp_drc_add_slot(struct device_node *dn)
 {
 	struct slot *slot;
 	int retval = 0;

@@ -352,9 +377,6 @@ int rpaphp_add_slot(struct device_node *dn)
 	const int *indexes, *names, *types, *power_domains;
 	char *name, *type;
 
-	if (!dn->name || strcmp(dn->name, "pci"))
-		return 0;
-
 	/* If this is not a hotplug slot, return without doing anything. */
 	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
 		return 0;

@@ -393,6 +415,33 @@ int rpaphp_add_slot(struct device_node *dn)
 	/* XXX FIXME: reports a failure only if last entry in loop failed */
 	return retval;
 }
+
+/**
+ * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
+ * @dn: device node of slot
+ *
+ * This subroutine will register a hotpluggable slot with the
+ * PCI hotplug infrastructure. This routine is typically called
+ * during boot time, if the hotplug slots are present at boot time,
+ * or is called later, by the dlpar add code, if the slot is
+ * being dynamically added during runtime.
+ *
+ * If the device node points at an embedded (built-in) slot, this
+ * routine will just return without doing anything, since embedded
+ * slots cannot be hotplugged.
+ *
+ * To remove a slot, it suffices to call rpaphp_deregister_slot().
+ */
+int rpaphp_add_slot(struct device_node *dn)
+{
+	if (!dn->name || strcmp(dn->name, "pci"))
+		return 0;
+
+	if (of_find_property(dn, "ibm,drc-info", NULL))
+		return rpaphp_drc_info_add_slot(dn);
+	else
+		return rpaphp_drc_add_slot(dn);
+}
 EXPORT_SYMBOL_GPL(rpaphp_add_slot);
 
 static void __exit cleanup_slots(void)

-- 
2.7.4

The first entry of the ibm,drc-info property is an int encoded count
of the number of drc-info entries that follow. The "value" pointer
returned by of_prop_next_u32() is still pointing at the this value
when we call of_read_drc_info_cell(), but the helper function
expects that value to be pointing at the first element of an entry.

Fix up by incrementing the "value" pointer to point at the first
element of the first drc-info entry prior.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index 18627bb..e350264 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c

@@ -239,6 +239,8 @@ static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
 	value = of_prop_next_u32(info, NULL, &entries);
 	if (!value)
 		return -EINVAL;
+	else
+		value++;
 
 	for (j = 0; j < entries; j++) {
 		of_read_drc_info_cell(&info, &value, &drc);

-- 
2.7.4

The device tree is in big endian format and any properties directly
retrieved using OF helpers that don't explicitly byte swap should
be annotated. In particular there are several places where we grab
the opaque property value for the old ibm,drc-* properties and the
ibm,my-drc-index property.

Fix this for better static checking by annotating values we know to
explicitly big endian, and byte swap where appropriate.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index 75d5771..eabc0c51 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c

@@ -154,9 +154,9 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
 	return speed;
 }
 
-static int get_children_props(struct device_node *dn, const int **drc_indexes,
-		const int **drc_names, const int **drc_types,
-		const int **drc_power_domains)
+static int get_children_props(struct device_node *dn, const __be32 **drc_indexes,
+		const __be32 **drc_names, const __be32 **drc_types,
+		const __be32 **drc_power_domains)
 {
 	const int *indexes, *names, *types, *domains;
 

@@ -194,8 +194,8 @@ 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 __be32 *indexes, *names;
+	const __be32 *types, *domains;
 	int i, rc;
 
 	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);

@@ -208,7 +208,7 @@ static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
 
 	/* 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 (be32_to_cpu(indexes[i + 1]) == my_index)
 			break;
 
 		name_tmp += (strlen(name_tmp) + 1);

@@ -267,7 +267,7 @@ static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
 int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
 			char *drc_type)
 {
-	const unsigned int *my_index;
+	const __be32 *my_index;
 
 	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
 	if (!my_index) {

@@ -277,10 +277,10 @@ int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
 
 	if (of_find_property(dn->parent, "ibm,drc-info", NULL))
 		return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
-						*my_index);
+						be32_to_cpu(*my_index));
 	else
 		return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
-						*my_index);
+						be32_to_cpu(*my_index));
 }
 EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
 

@@ -311,10 +311,10 @@ static int is_php_type(char *drc_type)
  * for built-in pci slots (even when the built-in slots are
  * dlparable.)
  */
-static int is_php_dn(struct device_node *dn, const int **indexes,
-		const int **names, const int **types, const int **power_domains)
+static int is_php_dn(struct device_node *dn, const __be32 **indexes,
+		const __be32 **names, const __be32 **types, const __be32 **power_domains)
 {
-	const int *drc_types;
+	const __be32 *drc_types;
 	int rc;
 
 	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);

@@ -374,7 +374,7 @@ static int rpaphp_drc_add_slot(struct device_node *dn)
 	struct slot *slot;
 	int retval = 0;
 	int i;
-	const int *indexes, *names, *types, *power_domains;
+	const __be32 *indexes, *names, *types, *power_domains;
 	char *name, *type;
 
 	/* If this is not a hotplug slot, return without doing anything. */

-- 
2.7.4

Advertise client support for the PAPR architected ibm,drc-info device
tree property during CAS handshake.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 arch/powerpc/kernel/prom_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index a4e7762..2ca9966 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c

@@ -1053,7 +1053,7 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
 		.reserved2 = 0,
 		.reserved3 = 0,
 		.subprocessors = 1,
-		.byte22 = OV5_FEAT(OV5_DRMEM_V2),
+		.byte22 = OV5_FEAT(OV5_DRMEM_V2) | OV5_FEAT(OV5_DRC_INFO),
 		.intarch = 0,
 		.mmu = 0,
 		.hash_ext = 0,

-- 
2.7.4

The newer ibm,drc-info property is a condensed description of the old
ibm,drc-* properties (ie. names, types, indexes, and power-domains).
When matching a drc-index to a drc-name we need to verify that the
index is within the start and last drc-index range and map it to a
drc-name using the drc-name-prefix and logical index.

Fix the mapping by checking that the index is within the range of the
current drc-info entry, and build the name from the drc-name-prefix
and by adding the starting drc-name-suffix value with the sequential
index of subtracting ibm,my-drc-index from this entries
drc-start-index.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
index eabc0c51..5327606 100644
--- a/drivers/pci/hotplug/rpaphp_core.c
+++ b/drivers/pci/hotplug/rpaphp_core.c

@@ -248,9 +248,10 @@ static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
 		/* Should now know end of current entry */
 
 		/* Found it */
-		if (my_index <= drc.last_drc_index) {
+		if (my_index >= drc.drc_index_start && my_index <= drc.last_drc_index) {
+			int index = my_index - drc.drc_index_start;
 			sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
-				my_index);
+				drc.drc_name_suffix_start + index);
 			break;
 		}
 	}

-- 
2.7.4

There are a couple subtle errors in the mapping between cpu-ids and a
cpus associated drc-index when using the new ibm,drc-info property.

The first is that while drc-info may have been a supported firmware
feature at boot it is possible we have migrated to a CEC with older
firmware that doesn't support the ibm,drc-info property. In that case
the device tree would have been updated after migration to remove the
ibm,drc-info property and replace it with the older style ibm,drc-*
properties for types, indexes, names, and power-domains.

The second is that the first value of the ibm,drc-info property is
the int encoded count of drc-info entries. As such "value" returned
by of_prop_next_u32() is pointing at that count, and not the first
element of the first drc-info entry as is expected by the
of_read_drc_info_cell() helper.

Fix the first by ignoring DRC-INFO firmware feature and instead
testing directly for ibm,drc-info, and then falling back to the
old style ibm,drc-indexes in the case it doesn't exit.

Fix the second by incrementing value to the next element prior to
parsing drc-info entries.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/pseries_energy.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index a96874f..09e98d3 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c

@@ -36,6 +36,7 @@ static int sysfs_entries;
 static u32 cpu_to_drc_index(int cpu)
 {
 	struct device_node *dn = NULL;
+	struct property *info;
 	int thread_index;
 	int rc = 1;
 	u32 ret = 0;

@@ -47,20 +48,18 @@ static u32 cpu_to_drc_index(int cpu)
 	/* Convert logical cpu number to core number */
 	thread_index = cpu_core_index_of_thread(cpu);
 
-	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
-		struct property *info = NULL;
+	info = of_find_property(dn, "ibm,drc-info", NULL);
+	if (info) {
 		struct of_drc_info drc;
 		int j;
 		u32 num_set_entries;
 		const __be32 *value;
 
-		info = of_find_property(dn, "ibm,drc-info", NULL);
-		if (info == NULL)
-			goto err_of_node_put;
-
 		value = of_prop_next_u32(info, NULL, &num_set_entries);
 		if (!value)
 			goto err_of_node_put;
+		else
+			value++;
 
 		for (j = 0; j < num_set_entries; j++) {
 

@@ -110,6 +109,7 @@ static u32 cpu_to_drc_index(int cpu)
 static int drc_index_to_cpu(u32 drc_index)
 {
 	struct device_node *dn = NULL;
+	struct property *info;
 	const int *indexes;
 	int thread_index = 0, cpu = 0;
 	int rc = 1;

@@ -117,21 +117,18 @@ static int drc_index_to_cpu(u32 drc_index)
 	dn = of_find_node_by_path("/cpus");
 	if (dn == NULL)
 		goto err;
-
-	if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
-		struct property *info = NULL;
+	info = of_find_property(dn, "ibm,drc-info", NULL);
+	if (info) {
 		struct of_drc_info drc;
 		int j;
 		u32 num_set_entries;
 		const __be32 *value;
 
-		info = of_find_property(dn, "ibm,drc-info", NULL);
-		if (info == NULL)
-			goto err_of_node_put;
-
 		value = of_prop_next_u32(info, NULL, &num_set_entries);
 		if (!value)
 			goto err_of_node_put;
+		else
+			value++;
 
 		for (j = 0; j < num_set_entries; j++) {
 

-- 
2.7.4

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

Hi, this change was confusing to me until I continued reading the patch 
and saw the comment below regarding the first element of the 
ibm,drc-info property.  Would it be good to have a similar comment here too?

  	while (!found) {
  		u32 drc;

  		rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
  						index++, &drc);
+

Another nitpick but this could be cleaned up.

Thanks,

Tom

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

Hi, just pointing out a few typos...

There was a previous effort to add support for the PAPR
architected ibm,drc-info property. This property provides a more
memory compact representation of a paritions Dynamic Reconfig

s/paritions/partition's

Connectors (DRC). These can otherwise be thought of as currently
partitioned, or available but yet to be partitioned system resources
such as cpus, memory, and physical/logical IOA devices.

The initial implementation proved buggy and was fully turned of by

s/turned of/turned off

disabling the bit in the appropriate CAS support vector. We now have
PowerVM firmware in the field that supports this new property, and
further to suppport partitions with 24TB+ of possible memory this

s/suppport/support

property is required to perform platform migration.

This serious fixs the short comings of the previous submission

Either "seriously fixes the shortcomings", or "fixes the serious 
shortcomings?"

Thanks,

Tom

in the areas of general implementation, cpu hotplug, and IOA hotplug.

Tyrel Datwyler (9):
   powerpc/pseries: Fix bad drc_index_start value parsing of drc-info
     entry
   powerpc/pseries: Fix drc-info mappings of logical cpus to drc-index
   powerpc/pseries: Add cpu DLPAR support for drc-info property
   PCI: rpaphp: Fix up pointer to first drc-info entry
   PCI: rpaphp: Don't rely on firmware feature to imply drc-info support
   PCI: rpaphp: Add drc-info support for hotplug slot registration
   PCI: rpaphp: annotate and correctly byte swap DRC properties
   PCI: rpaphp: Correctly match ibm,my-drc-index to drc-name when using
     drc-info
   powerpc/pseries: Enable support for ibm,drc-info property

  arch/powerpc/kernel/prom_init.c                 |   2 +-
  arch/powerpc/platforms/pseries/hotplug-cpu.c    | 101 ++++++++++++++++---
  arch/powerpc/platforms/pseries/of_helpers.c     |   8 +-
  arch/powerpc/platforms/pseries/pseries_energy.c |  23 ++---
  drivers/pci/hotplug/rpaphp_core.c               | 124 +++++++++++++++++-------
  5 files changed, 187 insertions(+), 71 deletions(-)

On 11/5/19 9:03 AM, Thomas Falcon wrote:

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

Hi, just pointing out a few typos...

Damn, I thought I squashed them all the second time around.

quoted

There was a previous effort to add support for the PAPR
architected ibm,drc-info property. This property provides a more
memory compact representation of a paritions Dynamic Reconfig

s/paritions/partition's

quoted

Connectors (DRC). These can otherwise be thought of as currently
partitioned, or available but yet to be partitioned system resources
such as cpus, memory, and physical/logical IOA devices.

The initial implementation proved buggy and was fully turned of by

s/turned of/turned off

quoted

disabling the bit in the appropriate CAS support vector. We now have
PowerVM firmware in the field that supports this new property, and
further to suppport partitions with 24TB+ of possible memory this

s/suppport/support

quoted

property is required to perform platform migration.

This serious fixs the short comings of the previous submission

Either "seriously fixes the shortcomings", or "fixes the serious shortcomings?"

Should be "series" as in this "patch series".

-Tyrel

Thanks,

Tom

quoted

in the areas of general implementation, cpu hotplug, and IOA hotplug.

Tyrel Datwyler (9):
   powerpc/pseries: Fix bad drc_index_start value parsing of drc-info
     entry
   powerpc/pseries: Fix drc-info mappings of logical cpus to drc-index
   powerpc/pseries: Add cpu DLPAR support for drc-info property
   PCI: rpaphp: Fix up pointer to first drc-info entry
   PCI: rpaphp: Don't rely on firmware feature to imply drc-info support
   PCI: rpaphp: Add drc-info support for hotplug slot registration
   PCI: rpaphp: annotate and correctly byte swap DRC properties
   PCI: rpaphp: Correctly match ibm,my-drc-index to drc-name when using
     drc-info
   powerpc/pseries: Enable support for ibm,drc-info property

  arch/powerpc/kernel/prom_init.c                 |   2 +-
  arch/powerpc/platforms/pseries/hotplug-cpu.c    | 101 ++++++++++++++++---
  arch/powerpc/platforms/pseries/of_helpers.c     |   8 +-
  arch/powerpc/platforms/pseries/pseries_energy.c |  23 ++---
  drivers/pci/hotplug/rpaphp_core.c               | 124 +++++++++++++++++-------
  5 files changed, 187 insertions(+), 71 deletions(-)

On 11/5/19 8:55 AM, Thomas Falcon wrote:

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

quoted

From: Tyrel Datwyler <redacted>

Older firmwares provided information about Dynamic Reconfig
Connectors (DRC) through several device tree properties, namely
ibm,drc-types, ibm,drc-indexes, ibm,drc-names, and
ibm,drc-power-domains. New firmwares have the ability to present this
same information in a much condensed format through a device tree
property called ibm,drc-info.

The existing cpu DLPAR hotplug code only understands the older DRC
property format when validating the drc-index of a cpu during a
hotplug add. This updates those code paths to use the ibm,drc-info
property, when present, instead for validation.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
  arch/powerpc/platforms/pseries/hotplug-cpu.c | 101 ++++++++++++++++++++++-----
  1 file changed, 85 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c

b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index bbda646..9ba006c 100644

--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c

@@ -407,17 +407,58 @@ static bool dlpar_cpu_exists(struct device_node *parent,

u32 drc_index)
      return found;
  }

+static bool drc_info_valid_index(struct device_node *parent, u32 drc_index)
+{
+    struct property *info;
+    struct of_drc_info drc;
+    const __be32 *value;
+    int count, i, j;
+
+    info = of_find_property(parent, "ibm,drc-info", NULL);
+    if (!info)
+        return false;
+
+    value = of_prop_next_u32(info, NULL, &count);
+
+    /* First value of ibm,drc-info is number of drc-info records */
+    if (value)
+        value++;
+    else
+        return false;
+
+    for (i = 0; i < count; i++) {
+        if (of_read_drc_info_cell(&info, &value, &drc))
+            return false;
+
+        if (strncmp(drc.drc_type, "CPU", 3))
+            break;
+
+        if (drc_index > drc.last_drc_index)
+            continue;
+
+        for (j = 0; j < drc.num_sequential_elems; j++)
+            if (drc_index == (drc.drc_index_start + (drc.sequential_inc * j)))
+                    return true;
+    }
+
+    return false;
+}
+
  static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
  {
      bool found = false;
      int rc, index;

-    index = 0;
+    if (of_find_property(parent, "ibm,drc-info", NULL))
+        return drc_info_valid_index(parent, drc_index);
+
+    index = 1;

Hi, this change was confusing to me until I continued reading the patch and saw
the comment below regarding the first element of the ibm,drc-info property. 
Would it be good to have a similar comment here too?

Yeah, clearly wouldn't hurt. Probably should split it out into a separate fix
prior to this patch.

quoted

      while (!found) {
          u32 drc;

          rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
                          index++, &drc);
+

Another nitpick but this could be cleaned up.

Yep, noticed the newline addition after I'd already sent it out.

-Tyrel

Thanks,

Tom

quoted

          if (rc)
              break;

@@ -720,8 +761,11 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)

  static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
  {
      struct device_node *parent;
+    struct property *info;
      int cpus_found = 0;
      int index, rc;
+    int i, j;
+    u32 drc_index;

      parent = of_find_node_by_path("/cpus");
      if (!parent) {

@@ -730,24 +774,49 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32

cpus_to_add)
          return -1;
      }

-    /* Search the ibm,drc-indexes array for possible CPU drcs to
-     * add. Note that the format of the ibm,drc-indexes array is
-     * the number of entries in the array followed by the array
-     * of drc values so we start looking at index = 1.
-     */
-    index = 1;
-    while (cpus_found < cpus_to_add) {
-        u32 drc;
+    info = of_find_property(parent, "ibm,drc-info", NULL);
+    if (info) {
+        struct of_drc_info drc;
+        const __be32 *value;
+        int count;

-        rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
-                        index++, &drc);
-        if (rc)
-            break;
+        value = of_prop_next_u32(info, NULL, &count);
+        if (value)
+            value++;

-        if (dlpar_cpu_exists(parent, drc))
-            continue;
+        for (i = 0; i < count; i++) {
+            of_read_drc_info_cell(&info, &value, &drc);
+            if (strncmp(drc.drc_type, "CPU", 3))
+                break;
+
+            for (j = 0; j < drc.num_sequential_elems && cpus_found <
cpus_to_add; j++) {
+                drc_index = drc.drc_index_start + (drc.sequential_inc * j);
+
+                if (dlpar_cpu_exists(parent, drc_index))
+                    continue;
+
+                cpu_drcs[cpus_found++] = drc_index;
+            }
+        }
+    } else {
+        /* Search the ibm,drc-indexes array for possible CPU drcs to
+         * add. Note that the format of the ibm,drc-indexes array is
+         * the number of entries in the array followed by the array
+         * of drc values so we start looking at index = 1.
+         */
+        index = 1;
+        while (cpus_found < cpus_to_add) {
+            rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
+                            index++, &drc_index);
+
+            if (rc)
+                break;

-        cpu_drcs[cpus_found++] = drc;
+            if (dlpar_cpu_exists(parent, drc_index))
+                continue;
+
+            cpu_drcs[cpus_found++] = drc_index;
+        }
      }

      of_node_put(parent);

Tyrel Datwyler [off-list ref] writes:

On 11/5/19 9:03 AM, Thomas Falcon wrote:

quoted

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

..

quoted

quoted

This serious fixs the short comings of the previous submission

Either "seriously fixes the shortcomings", or "fixes the serious shortcomings?"

Should be "series" as in this "patch series".

This serious series seriously fixes the series of serious shortcomings?

:P

cheers

Tyrel Datwyler [off-list ref] writes:

This line's nearly 100 columns, which suggests that this logic has
gotten too convoluted to be a single function.

So I think you should split one or both arms of the if out into separate
functions.

You're basically doing nothing after the if, so possibly you can just
return the result of the split out functions directly.

cheers

+				drc_index = drc.drc_index_start + (drc.sequential_inc * j);
+
+				if (dlpar_cpu_exists(parent, drc_index))
+					continue;
+
+				cpu_drcs[cpus_found++] = drc_index;
+			}
+		}
+	} else {
+		/* Search the ibm,drc-indexes array for possible CPU drcs to
+		 * add. Note that the format of the ibm,drc-indexes array is
+		 * the number of entries in the array followed by the array
+		 * of drc values so we start looking at index = 1.
+		 */
+		index = 1;
+		while (cpus_found < cpus_to_add) {
+			rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
+							index++, &drc_index);
+
+			if (rc)
+				break;
 
-		cpu_drcs[cpus_found++] = drc;
+			if (dlpar_cpu_exists(parent, drc_index))
+				continue;
+
+			cpu_drcs[cpus_found++] = drc_index;
+		}
 	}
 
 	of_node_put(parent);

Tyrel Datwyler [off-list ref] writes:

Advertise client support for the PAPR architected ibm,drc-info device
tree property during CAS handshake.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>

Can you mark this as:

  Fixes: c7a3275e0f9e ("powerpc/pseries: Revert support for ibm,drc-info devtree property")


I'm not sure we're going to backport all those fixes into stable
kernels, but at least then we have the link between this commit
c7a3275e0f9e recorded.

cheers

Tyrel Datwyler [off-list ref] writes:

The device tree is in big endian format and any properties directly
retrieved using OF helpers that don't explicitly byte swap should
be annotated. In particular there are several places where we grab
the opaque property value for the old ibm,drc-* properties and the
ibm,my-drc-index property.

Fix this for better static checking by annotating values we know to
explicitly big endian, and byte swap where appropriate.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/pci/hotplug/rpaphp_core.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

This is allegedly still popping some sparse warnings:

  +drivers/pci/hotplug/rpaphp_core.c:XX:28: warning: incorrect type in assignment (different base types) expected restricted __be32 const [usertype] * got int const *[assigned] names
  +drivers/pci/hotplug/rpaphp_core.c:XX:28: warning: incorrect type in assignment (different base types) expected restricted __be32 const [usertype] * got int const *[assigned] types
  +drivers/pci/hotplug/rpaphp_core.c:XX:30: warning: incorrect type in assignment (different base types) expected restricted __be32 const [usertype] * got int const *[assigned] indexes
  +drivers/pci/hotplug/rpaphp_core.c:XX:36: warning: incorrect type in assignment (different base types) expected restricted __be32 const [usertype] * got int const *[assigned] domains


I say allegedly because that output's from a script that tries to diff
sparse warnings before and after the build and it's not always 100% reliable.

cheers