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 parition's 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 off 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 support partitions with 24TB+ of possible memory this property is required to perform platform migration. This series fixs the short comings of the previous submission in the areas of general implementation, cpu hotplug, and IOA hotplug. v2 changelog: Cover Letter: fixed up spelling errors (mpe, tfalcon) Patch 3: added comment regarding indexing of drc values (tfalcon) split drc-index and drc-info logic into multiple functions for collecting cpu drc's for dlpar (mpe) Patch 7: fix up a couple more sparse warnings (mpe) 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 | 127 +++++++++++++++++++++--- arch/powerpc/platforms/pseries/of_helpers.c | 8 +- arch/powerpc/platforms/pseries/pseries_energy.c | 23 ++--- drivers/pci/hotplug/rpaphp_core.c | 127 +++++++++++++++++------- 5 files changed, 216 insertions(+), 71 deletions(-) -- 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
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. PAPR even goes as far as dictating that if we advertise support for drc-info that we are capable of supporting either property type at runtime. 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
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 | 127 +++++++++++++++++++++++---- 1 file changed, 112 insertions(+), 15 deletions(-) diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c index bbda646..967c5e1 100644 --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c @@ -407,17 +407,67 @@ 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; + u32 index; + 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; + + index = drc.drc_index_start; + for (j = 0; j < drc.num_sequential_elems; j++) { + if (drc_index == index) + return true; + + index += drc.sequential_inc; + } + } + + 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); + + /* 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 (!found) { u32 drc; rc = of_property_read_u32_index(parent, "ibm,drc-indexes", index++, &drc); + if (rc) break; @@ -717,19 +767,52 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove) return rc; } -static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add) +static int find_drc_info_cpus_to_add(struct device_node *cpus, + struct property *info, + u32 *cpu_drcs, u32 cpus_to_add) { - struct device_node *parent; + struct of_drc_info drc; + const __be32 *value; + u32 count, drc_index; int cpus_found = 0; - int index, rc; + int i, j; - parent = of_find_node_by_path("/cpus"); - if (!parent) { - pr_warn("Could not find CPU root node in device tree\n"); - kfree(cpu_drcs); + if (!info) return -1; + + value = of_prop_next_u32(info, NULL, &count); + if (value) + value++; + + for (i = 0; i < count; i++) { + of_read_drc_info_cell(&info, &value, &drc); + if (strncmp(drc.drc_type, "CPU", 3)) + break; + + drc_index = drc.drc_index_start; + for (j = 0; j < drc.num_sequential_elems; j++) { + if (dlpar_cpu_exists(cpus, drc_index)) + continue; + + cpu_drcs[cpus_found++] = drc_index; + + if (cpus_found == cpus_to_add) + return cpus_found; + + drc_index += drc.sequential_inc; + } } + return cpus_found; +} + +static int find_drc_index_cpus_to_add(struct device_node *cpus, + u32 *cpu_drcs, u32 cpus_to_add) +{ + int cpus_found = 0; + int index, rc; + u32 drc_index; + /* 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 @@ -737,25 +820,25 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add) */ index = 1; while (cpus_found < cpus_to_add) { - u32 drc; + rc = of_property_read_u32_index(cpus, "ibm,drc-indexes", + index++, &drc_index); - rc = of_property_read_u32_index(parent, "ibm,drc-indexes", - index++, &drc); if (rc) break; - if (dlpar_cpu_exists(parent, drc)) + if (dlpar_cpu_exists(cpus, drc_index)) continue; - cpu_drcs[cpus_found++] = drc; + cpu_drcs[cpus_found++] = drc_index; } - of_node_put(parent); return cpus_found; } static int dlpar_cpu_add_by_count(u32 cpus_to_add) { + struct device_node *parent; + struct property *info; u32 *cpu_drcs; int cpus_added = 0; int cpus_found; @@ -767,7 +850,21 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add) if (!cpu_drcs) return -EINVAL; - cpus_found = find_dlpar_cpus_to_add(cpu_drcs, cpus_to_add); + parent = of_find_node_by_path("/cpus"); + if (!parent) { + pr_warn("Could not find CPU root node in device tree\n"); + kfree(cpu_drcs); + return -1; + } + + info = of_find_property(parent, "ibm,drc-info", NULL); + if (info) + cpus_found = find_drc_info_cpus_to_add(parent, info, cpu_drcs, cpus_to_add); + else + cpus_found = find_drc_index_cpus_to_add(parent, cpu_drcs, cpus_to_add); + + of_node_put(parent); + if (cpus_found < cpus_to_add) { pr_warn("Failed to find enough CPUs (%d of %d) to add\n", cpus_found, cpus_to_add); -- 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
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 that the client is able to handle both the new property, and old style properties at runtime. 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
Split physical PCI slot registration scanning into separate 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 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 | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c index 75d5771..129534c 100644 --- a/drivers/pci/hotplug/rpaphp_core.c +++ b/drivers/pci/hotplug/rpaphp_core.c @@ -154,11 +154,11 @@ 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; + const __be32 *indexes, *names, *types, *domains; indexes = of_get_property(dn, "ibm,drc-indexes", NULL); names = of_get_property(dn, "ibm,drc-names", NULL); @@ -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,11 @@ 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 +375,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
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 | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c index 75d5771..129534c 100644 --- a/drivers/pci/hotplug/rpaphp_core.c +++ b/drivers/pci/hotplug/rpaphp_core.c @@ -154,11 +154,11 @@ 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; + const __be32 *indexes, *names, *types, *domains; indexes = of_get_property(dn, "ibm,drc-indexes", NULL); names = of_get_property(dn, "ibm,drc-names", NULL); @@ -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,11 @@ 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 +375,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
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 concatenated with the starting drc-name-suffix value and the sequential index obtained by 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 129534c..951f7f2 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
Advertise client support for the PAPR architected ibm,drc-info device tree property during CAS handshake. Fixes: c7a3275e0f9e ("powerpc/pseries: Revert support for ibm,drc-info devtree property") 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 100f1b5..eba9d4e 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
On Sun, Nov 10, 2019 at 11:21:27PM -0600, Tyrel Datwyler wrote: > 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 parition's 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 off 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 support partitions with 24TB+ of possible memory this > property is required to perform platform migration. > > This series fixs the short comings of the previous submission > in the areas of general implementation, cpu hotplug, and IOA hotplug. > > v2 changelog: > Cover Letter: fixed up spelling errors (mpe, tfalcon) > Patch 3: added comment regarding indexing of drc values (tfalcon) > split drc-index and drc-info logic into multiple > functions for collecting cpu drc's for dlpar (mpe) > Patch 7: fix up a couple more sparse warnings (mpe) > > 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 | 127 +++++++++++++++++++++--- > arch/powerpc/platforms/pseries/of_helpers.c | 8 +- > arch/powerpc/platforms/pseries/pseries_energy.c | 23 ++--- > drivers/pci/hotplug/rpaphp_core.c | 127 +++++++++++++++++------- For the drivers/pci/* parts: Acked-by: Bjorn Helgaas <bhelgaas@google.com> I assume they will be merged along with the rest of the series via powerpc. > 5 files changed, 216 insertions(+), 71 deletions(-) > > -- > 2.7.4 >
Bjorn Helgaas <helgaas@kernel.org> writes: > On Sun, Nov 10, 2019 at 11:21:27PM -0600, Tyrel Datwyler wrote: >> 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 parition's 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 off 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 support partitions with 24TB+ of possible memory this >> property is required to perform platform migration. >> >> This series fixs the short comings of the previous submission >> in the areas of general implementation, cpu hotplug, and IOA hotplug. >> >> v2 changelog: >> Cover Letter: fixed up spelling errors (mpe, tfalcon) >> Patch 3: added comment regarding indexing of drc values (tfalcon) >> split drc-index and drc-info logic into multiple >> functions for collecting cpu drc's for dlpar (mpe) >> Patch 7: fix up a couple more sparse warnings (mpe) >> >> 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 | 127 +++++++++++++++++++++--- >> arch/powerpc/platforms/pseries/of_helpers.c | 8 +- >> arch/powerpc/platforms/pseries/pseries_energy.c | 23 ++--- >> drivers/pci/hotplug/rpaphp_core.c | 127 +++++++++++++++++------- > > For the drivers/pci/* parts: > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> I added it to the PCI touching patches. > I assume they will be merged along with the rest of the series via > powerpc. Yep. Thanks for the review. cheers
On Mon, 2019-11-11 at 05:21:28 UTC, Tyrel Datwyler wrote: > 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> Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/57409d4fb12c185b2c0689e0496878c8f6bb5b58 cheers