* Re: [RFC v5 6/6] migration/memory: Update memory for assoc changes
From: Michael Bringmann @ 2018-05-22 23:54 UTC (permalink / raw)
To: Thomas Falcon, linuxppc-dev; +Cc: Nathan Fontenot, Tyrel Datwyler, John Allen
In-Reply-To: <a67040bb-82e7-8bc2-06b6-8b8615aa3bf1@linux.vnet.ibm.com>
This patch was intended to apply the necessary changes for the
'ibm,dynamic-memory[-v2]' properties. Before the advent of the
LMB representation, that code took up a lot more space. At this
point, it has shrunk to only one line of unique change. I was
hoping to include it here rather than create another patch.
But that can be done.
Michael
On 05/22/2018 04:11 PM, Thomas Falcon wrote:
> On 05/21/2018 12:52 PM, Michael Bringmann wrote:
>> migration/memory: This patch adds more recognition for changes to
>> the associativity of memory blocks described by the device-tree
>> properties and updates local and general kernel data structures to
>> reflect those changes. These differences may include:
>>
>> * Evaluating 'ibm,dynamic-memory' properties when processing the
>> topology of LPARS in Post Migration events. Previous efforts
>> only recognized whether a memory block's assignment had changed
>> in the property. Changes here include checking the aa_index
>> values for each drc_index of the old/new LMBs and to 'readd'
>> any block for which the setting has changed.
>>
>> * In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
>> property may change. In the event that a row of the array differs,
>> locate all assigned memory blocks with that 'aa_index' and 're-add'
>> them to the system memory block data structures. In the process of
>> the 're-add', the system routines will update the corresponding entry
>> for the memory in the LMB structures and any other relevant kernel
>> data structures.
>>
>> * Extend the previous work for the 'ibm,associativity-lookup-array'
>> and 'ibm,dynamic-memory' properties to support the property
>> 'ibm,dynamic-memory-v2' by means of the DRMEM LMB interpretation
>> code.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in RFC:
>> -- Simplify code to update memory nodes during mobility checks.
>> -- Reuse code from DRMEM changes to scan for LMBs when updating
>> aa_index
>> -- Combine common code for properties 'ibm,dynamic-memory' and
>> 'ibm,dynamic-memory-v2' after integrating DRMEM features.
>> -- Rearrange patches to co-locate memory property-related changes.
>> -- Use new paired list iterator for the drmem info arrays.
>> -- Use direct calls to add/remove memory from the update drconf
>> function as those operations are only intended for user DLPAR
>> ops, and should not occur during Migration reconfig notifier
>> changes.
>> -- Correct processing bug in processing of ibm,associativity-lookup-arrays
>> -- Rebase to 4.17-rc5 kernel
>> -- Apply minor code cleanups
>> ---
>> arch/powerpc/platforms/pseries/hotplug-memory.c | 153 ++++++++++++++++++-----
>> 1 file changed, 121 insertions(+), 32 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index c1578f5..ac329aa 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -994,13 +994,11 @@ static int pseries_add_mem_node(struct device_node *np)
>> return (ret < 0) ? -EINVAL : 0;
>> }
>>
>> -static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
>> {
>> - struct of_drconf_cell_v1 *new_drmem, *old_drmem;
>> + struct drmem_lmb *old_lmb, *new_lmb;
>> unsigned long memblock_size;
>> - u32 entries;
>> - __be32 *p;
>> - int i, rc = -EINVAL;
>> + int rc = 0;
>>
>> if (rtas_hp_event)
>> return 0;
>> @@ -1009,42 +1007,124 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>> if (!memblock_size)
>> return -EINVAL;
>>
>> - p = (__be32 *) pr->old_prop->value;
>> - if (!p)
>> - return -EINVAL;
>> + /* Arrays should have the same size and DRC indexes */
>> + for_each_pair_drmem_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
>>
>> - /* The first int of the property is the number of lmb's described
>> - * by the property. This is followed by an array of of_drconf_cell
>> - * entries. Get the number of entries and skip to the array of
>> - * of_drconf_cell's.
>> - */
>> - entries = be32_to_cpu(*p++);
>> - old_drmem = (struct of_drconf_cell_v1 *)p;
>> -
>> - p = (__be32 *)pr->prop->value;
>> - p++;
>> - new_drmem = (struct of_drconf_cell_v1 *)p;
>> + if (new_lmb->drc_index != old_lmb->drc_index)
>> + continue;
>>
>> - for (i = 0; i < entries; i++) {
>> - if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
>> - (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
>> + if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
>> + (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
>> rc = pseries_remove_memblock(
>> - be64_to_cpu(old_drmem[i].base_addr),
>> - memblock_size);
>> + old_lmb->base_addr, memblock_size);
>> break;
>> - } else if ((!(be32_to_cpu(old_drmem[i].flags) &
>> - DRCONF_MEM_ASSIGNED)) &&
>> - (be32_to_cpu(new_drmem[i].flags) &
>> - DRCONF_MEM_ASSIGNED)) {
>> - rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
>> - memblock_size);
>> + } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
>> + (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> + rc = memblock_add(old_lmb->base_addr,
>> + memblock_size);
>> rc = (rc < 0) ? -EINVAL : 0;
>> break;
>> + } else if ((old_lmb->aa_index != new_lmb->aa_index) &&
>> + (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> + dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_MEM,
>> + PSERIES_HP_ELOG_ACTION_READD,
>> + new_lmb->drc_index);
>> }
>> }
>> return rc;
>> }
>>
>> +static void pseries_update_ala_memory_aai(int aa_index)
>> +{
>> + struct drmem_lmb *lmb;
>> +
>> + /* Readd all LMBs which were previously using the
>> + * specified aa_index value.
>> + */
>> + for_each_drmem_lmb(lmb) {
>> + if ((lmb->aa_index == aa_index) &&
>> + (lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> + dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_MEM,
>> + PSERIES_HP_ELOG_ACTION_READD,
>> + lmb->drc_index);
>> + }
>> + }
>> +}
>> +
>> +struct assoc_arrays {
>> + u32 n_arrays;
>> + u32 array_sz;
>> + const __be32 *arrays;
>> +};
>> +
>> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>> +{
>> + struct assoc_arrays new_ala, old_ala;
>> + __be32 *p;
>> + int i, lim;
>> +
>> + if (rtas_hp_event)
>> + return 0;
>> +
>> + /*
>> + * The layout of the ibm,associativity-lookup-arrays
>> + * property is a number N indicating the number of
>> + * associativity arrays, followed by a number M
>> + * indicating the size of each associativity array,
>> + * followed by a list of N associativity arrays.
>> + */
>> +
>> + p = (__be32 *) pr->old_prop->value;
>> + if (!p)
>> + return -EINVAL;
>> + old_ala.n_arrays = of_read_number(p++, 1);
>> + old_ala.array_sz = of_read_number(p++, 1);
>> + old_ala.arrays = p;
>> +
>> + p = (__be32 *) pr->prop->value;
>> + if (!p)
>> + return -EINVAL;
>> + new_ala.n_arrays = of_read_number(p++, 1);
>> + new_ala.array_sz = of_read_number(p++, 1);
>> + new_ala.arrays = p;
>> +
>
> I don't know how often associativity lookup arrays needs to be parsed, but maybe it would be helpful to create a helper function to parse those here.
>
>> + lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
>> + new_ala.n_arrays;
>> +
>> + if (old_ala.array_sz == new_ala.array_sz) {
>> +
>> + /* Reset any entries where the old and new rows
>> + * the array have changed.
>> + */
>> + for (i = 0; i < lim; i++) {
>> + int index = (i * new_ala.array_sz);
>> +
>> + if (!memcmp(&old_ala.arrays[index],
>> + &new_ala.arrays[index],
>> + new_ala.array_sz))
>> + continue;
>> +
>> + pseries_update_ala_memory_aai(i);
>> + }
>> +
>> + /* Reset any entries representing the extra rows.
>> + * There shouldn't be any, but just in case ...
>> + */
>> + for (i = lim; i < new_ala.n_arrays; i++)
>> + pseries_update_ala_memory_aai(i);
>> +
>> + } else {
>> + /* Update all entries representing these rows;
>> + * as all rows have different sizes, none can
>> + * have equivalent values.
>> + */
>> + for (i = 0; i < lim; i++)
>> + pseries_update_ala_memory_aai(i);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int pseries_memory_notifier(struct notifier_block *nb,
>> unsigned long action, void *data)
>> {
>> @@ -1059,8 +1139,17 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>> err = pseries_remove_mem_node(rd->dn);
>> break;
>> case OF_RECONFIG_UPDATE_PROPERTY:
>> - if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
>> - err = pseries_update_drconf_memory(rd);
>> + if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
>> + !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
>> + struct drmem_lmb_info *dinfo =
>> + drmem_lmbs_init(rd->prop);
>> + if (!dinfo)
>> + return -EINVAL;
>> + err = pseries_update_drconf_memory(dinfo);
>> + drmem_lmbs_free(dinfo);
>
> Is this block above related to the other associativity changes? It seems to be an update for dynamic-memory-v2, so should probably be in a separate patch.
>
> Thanks,
> Tom
>
>> + } else if (!strcmp(rd->prop->name,
>> + "ibm,associativity-lookup-arrays"))
>> + err = pseries_update_ala_memory(rd);
>> break;
>> }
>> return notifier_from_errno(err);
>
>
>
--
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
^ permalink raw reply
* Re: [RFC v5 3/6] migration/dlpar: Add device readd queuing function
From: Michael Bringmann @ 2018-05-22 23:49 UTC (permalink / raw)
To: Thomas Falcon, linuxppc-dev; +Cc: Nathan Fontenot, Tyrel Datwyler, John Allen
In-Reply-To: <0445c87f-1ab3-7b03-6956-56d4c1beab80@linux.vnet.ibm.com>
On 05/22/2018 03:24 PM, Thomas Falcon wrote:
> On 05/21/2018 12:52 PM, Michael Bringmann wrote:
>> migration/dlpar: This patch adds function dlpar_readd_action()
>> which will queue a worker function to 'readd' a device in the
>> system. Such devices must be identified by a 'resource' type
>> and a drc_index to be readded.
>
> The function in the commit message and the patch have different names. The patch seems to queue a generic action instead of a readd. The commit message needs to be updated to describe this new function.
>
> Tom
Fixed.
Michael
>
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/dlpar.c | 14 ++++++++++++++
>> arch/powerpc/platforms/pseries/pseries.h | 1 +
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
>> index a0b20c0..a14684e 100644
>> --- a/arch/powerpc/platforms/pseries/dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/dlpar.c
>> @@ -407,6 +407,20 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>> }
>> }
>>
>> +int dlpar_queue_action(int resource, int action, u32 drc_index)
>> +{
>> + struct pseries_hp_errorlog hp_elog;
>> +
>> + hp_elog.resource = resource;
>> + hp_elog.action = action;
>> + hp_elog.id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
>> + hp_elog._drc_u.drc_index = drc_index;
>> +
>> + queue_hotplug_event(&hp_elog, NULL, NULL);
>> +
>> + return 0;
>> +}
>> +
>> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
>> {
>> char *arg;
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>> index 60db2ee..cb2beb1 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -61,6 +61,7 @@ extern struct device_node *dlpar_configure_connector(__be32,
>>
>> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>> struct completion *hotplug_done, int *rc);
>> +extern int dlpar_queue_action(int resource, int action, u32 drc_index);
>> #ifdef CONFIG_MEMORY_HOTPLUG
>> int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
>> #else
>
>
>
--
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
^ permalink raw reply
* Re: [RFC v5 2/6] powerpc/cpu: Conditionally acquire/release DRC index
From: Michael Bringmann @ 2018-05-22 23:46 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <f1b2b32c-d8bb-4e25-6272-1fc41ffe1502@linux.vnet.ibm.com>
Okay. Moving validity check mentioned at bottom of patch to 4/6,
Provide CPU readd operation.
On 05/22/2018 03:17 PM, Nathan Fontenot wrote:
> On 05/21/2018 12:52 PM, Michael Bringmann wrote:
>> powerpc/cpu: Modify dlpar_cpu_add and dlpar_cpu_remove to allow the
>> skipping of DRC index acquire or release operations during the CPU
>> add or remove operations. This is intended to support subsequent
>> changes to provide a 'CPU readd' operation.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/hotplug-cpu.c | 71 +++++++++++++++-----------
>> 1 file changed, 42 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> index a408217..ec78cc6 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -474,7 +474,7 @@ static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
>> &cdata);
>> }
>>
>> -static ssize_t dlpar_cpu_add(u32 drc_index)
>> +static ssize_t dlpar_cpu_add(u32 drc_index, bool acquire_drc)
>> {
>> struct device_node *dn, *parent;
>> int rc, saved_rc;
>> @@ -499,19 +499,22 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>> return -EINVAL;
>> }
>>
>> - rc = dlpar_acquire_drc(drc_index);
>> - if (rc) {
>> - pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
>> - rc, drc_index);
>> - of_node_put(parent);
>> - return -EINVAL;
>> + if (acquire_drc) {
>> + rc = dlpar_acquire_drc(drc_index);
>> + if (rc) {
>> + pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
>> + rc, drc_index);
>> + of_node_put(parent);
>> + return -EINVAL;
>> + }
>> }
>>
>> dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
>> if (!dn) {
>> pr_warn("Failed call to configure-connector, drc index: %x\n",
>> drc_index);
>> - dlpar_release_drc(drc_index);
>> + if (acquire_drc)
>> + dlpar_release_drc(drc_index);
>> of_node_put(parent);
>> return -EINVAL;
>> }
>> @@ -526,8 +529,9 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>> pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
>> dn->name, rc, drc_index);
>>
>> - rc = dlpar_release_drc(drc_index);
>> - if (!rc)
>> + if (acquire_drc)
>> + rc = dlpar_release_drc(drc_index);
>> + if (!rc || acquire_drc)
>> dlpar_free_cc_nodes(dn);
>>
>> return saved_rc;
>> @@ -540,7 +544,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>> dn->name, rc, drc_index);
>>
>> rc = dlpar_detach_node(dn);
>> - if (!rc)
>> + if (!rc && acquire_drc)
>> dlpar_release_drc(drc_index);
>>
>> return saved_rc;
>> @@ -608,7 +612,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
>>
>> }
>>
>> -static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
>> +static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index,
>> + bool release_drc)
>> {
>> int rc;
>>
>> @@ -621,12 +626,14 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
>> return -EINVAL;
>> }
>>
>> - rc = dlpar_release_drc(drc_index);
>> - if (rc) {
>> - pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
>> - drc_index, dn->name, rc);
>> - dlpar_online_cpu(dn);
>> - return rc;
>> + if (release_drc) {
>> + rc = dlpar_release_drc(drc_index);
>> + if (rc) {
>> + pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
>> + drc_index, dn->name, rc);
>> + dlpar_online_cpu(dn);
>> + return rc;
>> + }
>> }
>>
>> rc = dlpar_detach_node(dn);
>> @@ -635,7 +642,10 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
>>
>> pr_warn("Failed to detach CPU %s, rc: %d", dn->name, rc);
>>
>> - rc = dlpar_acquire_drc(drc_index);
>> + if (release_drc)
>> + rc = dlpar_acquire_drc(drc_index);
>> + else
>> + rc = 0;
>> if (!rc)
>> dlpar_online_cpu(dn);
>>
>> @@ -664,7 +674,7 @@ static struct device_node *cpu_drc_index_to_dn(u32 drc_index)
>> return dn;
>> }
>>
>> -static int dlpar_cpu_remove_by_index(u32 drc_index)
>> +static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
>> {
>> struct device_node *dn;
>> int rc;
>> @@ -676,7 +686,7 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>> return -ENODEV;
>> }
>>
>> - rc = dlpar_cpu_remove(dn, drc_index);
>> + rc = dlpar_cpu_remove(dn, drc_index, release_drc);
>> of_node_put(dn);
>> return rc;
>> }
>> @@ -741,7 +751,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
>> }
>>
>> for (i = 0; i < cpus_to_remove; i++) {
>> - rc = dlpar_cpu_remove_by_index(cpu_drcs[i]);
>> + rc = dlpar_cpu_remove_by_index(cpu_drcs[i], true);
>> if (rc)
>> break;
>>
>> @@ -752,7 +762,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
>> pr_warn("CPU hot-remove failed, adding back removed CPUs\n");
>>
>> for (i = 0; i < cpus_removed; i++)
>> - dlpar_cpu_add(cpu_drcs[i]);
>> + dlpar_cpu_add(cpu_drcs[i], true);
>>
>> rc = -EINVAL;
>> } else {
>> @@ -843,7 +853,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
>> }
>>
>> for (i = 0; i < cpus_to_add; i++) {
>> - rc = dlpar_cpu_add(cpu_drcs[i]);
>> + rc = dlpar_cpu_add(cpu_drcs[i], true);
>> if (rc)
>> break;
>>
>> @@ -854,7 +864,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
>> pr_warn("CPU hot-add failed, removing any added CPUs\n");
>>
>> for (i = 0; i < cpus_added; i++)
>> - dlpar_cpu_remove_by_index(cpu_drcs[i]);
>> + dlpar_cpu_remove_by_index(cpu_drcs[i], true);
>>
>> rc = -EINVAL;
>> } else {
>> @@ -880,7 +890,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
>> if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
>> rc = dlpar_cpu_remove_by_count(count);
>> else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
>> - rc = dlpar_cpu_remove_by_index(drc_index);
>> + rc = dlpar_cpu_remove_by_index(drc_index, true);
>> else
>> rc = -EINVAL;
>> break;
>> @@ -888,7 +898,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
>> if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
>> rc = dlpar_cpu_add_by_count(count);
>> else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
>> - rc = dlpar_cpu_add(drc_index);
>> + rc = dlpar_cpu_add(drc_index, true);
>> else
>> rc = -EINVAL;
>> break;
>> @@ -913,7 +923,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
>> if (rc)
>> return -EINVAL;
>>
>> - rc = dlpar_cpu_add(drc_index);
>> + rc = dlpar_cpu_add(drc_index, true);
>>
>> return rc ? rc : count;
>> }
>> @@ -934,7 +944,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>> return -EINVAL;
>> }
>>
>> - rc = dlpar_cpu_remove(dn, drc_index);
>> + rc = dlpar_cpu_remove(dn, drc_index, true);
>> of_node_put(dn);
>>
>> return rc ? rc : count;
>> @@ -948,6 +958,9 @@ static int pseries_smp_notifier(struct notifier_block *nb,
>> struct of_reconfig_data *rd = data;
>> int err = 0;
>>
>> + if (strcmp(rd->dn->type, "cpu"))
>> + return notifier_from_errno(err);> +
>
> This last change doesn't seem to fit in this patch, should this be a part of a different patch?
>
> -Nathan
>
>> switch (action) {
>> case OF_RECONFIG_ATTACH_NODE:
>> err = pseries_add_processor(rd->dn);
>>
>
>
--
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
^ permalink raw reply
* [RFC v5 6/6] migration/memory: Update memory for assoc changes
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
In-Reply-To: <4b1138ff-4ca0-bdce-26cc-b42191258017@linux.vnet.ibm.com>
migration/memory: This patch adds more recognition for changes to
the associativity of memory blocks described by the device-tree
properties and updates local and general kernel data structures to
reflect those changes. These differences may include:
* Evaluating 'ibm,dynamic-memory' properties when processing the
topology of LPARS in Post Migration events. Previous efforts
only recognized whether a memory block's assignment had changed
in the property. Changes here include checking the aa_index
values for each drc_index of the old/new LMBs and to 'readd'
any block for which the setting has changed.
* In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
property may change. In the event that a row of the array differs,
locate all assigned memory blocks with that 'aa_index' and 're-add'
them to the system memory block data structures. In the process of
the 're-add', the system routines will update the corresponding entry
for the memory in the LMB structures and any other relevant kernel
data structures.
* Extend the previous work for the 'ibm,associativity-lookup-array'
and 'ibm,dynamic-memory' properties to support the property
'ibm,dynamic-memory-v2' by means of the DRMEM LMB interpretation
code.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in RFC:
-- Simplify code to update memory nodes during mobility checks.
-- Reuse code from DRMEM changes to scan for LMBs when updating
aa_index
-- Combine common code for properties 'ibm,dynamic-memory' and
'ibm,dynamic-memory-v2' after integrating DRMEM features.
-- Rearrange patches to co-locate memory property-related changes.
-- Use new paired list iterator for the drmem info arrays.
-- Use direct calls to add/remove memory from the update drconf
function as those operations are only intended for user DLPAR
ops, and should not occur during Migration reconfig notifier
changes.
-- Correct processing bug in processing of ibm,associativity-lookup-arrays
-- Rebase to 4.17-rc5 kernel
-- Apply minor code cleanups
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 153 ++++++++++++++++++-----
1 file changed, 121 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c1578f5..ac329aa 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -994,13 +994,11 @@ static int pseries_add_mem_node(struct device_node *np)
return (ret < 0) ? -EINVAL : 0;
}
-static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
+static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
{
- struct of_drconf_cell_v1 *new_drmem, *old_drmem;
+ struct drmem_lmb *old_lmb, *new_lmb;
unsigned long memblock_size;
- u32 entries;
- __be32 *p;
- int i, rc = -EINVAL;
+ int rc = 0;
if (rtas_hp_event)
return 0;
@@ -1009,42 +1007,124 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
if (!memblock_size)
return -EINVAL;
- p = (__be32 *) pr->old_prop->value;
- if (!p)
- return -EINVAL;
+ /* Arrays should have the same size and DRC indexes */
+ for_each_pair_drmem_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
- /* The first int of the property is the number of lmb's described
- * by the property. This is followed by an array of of_drconf_cell
- * entries. Get the number of entries and skip to the array of
- * of_drconf_cell's.
- */
- entries = be32_to_cpu(*p++);
- old_drmem = (struct of_drconf_cell_v1 *)p;
-
- p = (__be32 *)pr->prop->value;
- p++;
- new_drmem = (struct of_drconf_cell_v1 *)p;
+ if (new_lmb->drc_index != old_lmb->drc_index)
+ continue;
- for (i = 0; i < entries; i++) {
- if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
- (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
+ if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
+ (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
rc = pseries_remove_memblock(
- be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
+ old_lmb->base_addr, memblock_size);
break;
- } else if ((!(be32_to_cpu(old_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) &&
- (be32_to_cpu(new_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) {
- rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
+ } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ rc = memblock_add(old_lmb->base_addr,
+ memblock_size);
rc = (rc < 0) ? -EINVAL : 0;
break;
+ } else if ((old_lmb->aa_index != new_lmb->aa_index) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_MEM,
+ PSERIES_HP_ELOG_ACTION_READD,
+ new_lmb->drc_index);
}
}
return rc;
}
+static void pseries_update_ala_memory_aai(int aa_index)
+{
+ struct drmem_lmb *lmb;
+
+ /* Readd all LMBs which were previously using the
+ * specified aa_index value.
+ */
+ for_each_drmem_lmb(lmb) {
+ if ((lmb->aa_index == aa_index) &&
+ (lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_MEM,
+ PSERIES_HP_ELOG_ACTION_READD,
+ lmb->drc_index);
+ }
+ }
+}
+
+struct assoc_arrays {
+ u32 n_arrays;
+ u32 array_sz;
+ const __be32 *arrays;
+};
+
+static int pseries_update_ala_memory(struct of_reconfig_data *pr)
+{
+ struct assoc_arrays new_ala, old_ala;
+ __be32 *p;
+ int i, lim;
+
+ if (rtas_hp_event)
+ return 0;
+
+ /*
+ * The layout of the ibm,associativity-lookup-arrays
+ * property is a number N indicating the number of
+ * associativity arrays, followed by a number M
+ * indicating the size of each associativity array,
+ * followed by a list of N associativity arrays.
+ */
+
+ p = (__be32 *) pr->old_prop->value;
+ if (!p)
+ return -EINVAL;
+ old_ala.n_arrays = of_read_number(p++, 1);
+ old_ala.array_sz = of_read_number(p++, 1);
+ old_ala.arrays = p;
+
+ p = (__be32 *) pr->prop->value;
+ if (!p)
+ return -EINVAL;
+ new_ala.n_arrays = of_read_number(p++, 1);
+ new_ala.array_sz = of_read_number(p++, 1);
+ new_ala.arrays = p;
+
+ lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
+ new_ala.n_arrays;
+
+ if (old_ala.array_sz == new_ala.array_sz) {
+
+ /* Reset any entries where the old and new rows
+ * the array have changed.
+ */
+ for (i = 0; i < lim; i++) {
+ int index = (i * new_ala.array_sz);
+
+ if (!memcmp(&old_ala.arrays[index],
+ &new_ala.arrays[index],
+ new_ala.array_sz))
+ continue;
+
+ pseries_update_ala_memory_aai(i);
+ }
+
+ /* Reset any entries representing the extra rows.
+ * There shouldn't be any, but just in case ...
+ */
+ for (i = lim; i < new_ala.n_arrays; i++)
+ pseries_update_ala_memory_aai(i);
+
+ } else {
+ /* Update all entries representing these rows;
+ * as all rows have different sizes, none can
+ * have equivalent values.
+ */
+ for (i = 0; i < lim; i++)
+ pseries_update_ala_memory_aai(i);
+ }
+
+ return 0;
+}
+
static int pseries_memory_notifier(struct notifier_block *nb,
unsigned long action, void *data)
{
@@ -1059,8 +1139,17 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
- err = pseries_update_drconf_memory(rd);
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
+ !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
+ struct drmem_lmb_info *dinfo =
+ drmem_lmbs_init(rd->prop);
+ if (!dinfo)
+ return -EINVAL;
+ err = pseries_update_drconf_memory(dinfo);
+ drmem_lmbs_free(dinfo);
+ } else if (!strcmp(rd->prop->name,
+ "ibm,associativity-lookup-arrays"))
+ err = pseries_update_ala_memory(rd);
break;
}
return notifier_from_errno(err);
^ permalink raw reply related
* [RFC v5 5/6] powerpc/mobility: Add lock/unlock device hotplug
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
In-Reply-To: <4b1138ff-4ca0-bdce-26cc-b42191258017@linux.vnet.ibm.com>
powerpc/mobility: Add device lock/unlock to PowerPC 'mobility' operation
to delay the operation of CPU DLPAR work queue operations by the 'readd'
activity until after any changes to the corresponding device-tree
properties have been written.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 8a8033a..6d98f84 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -283,6 +283,8 @@ int pseries_devicetree_update(s32 scope)
if (!rtas_buf)
return -ENOMEM;
+ lock_device_hotplug();
+
do {
rc = mobility_rtas_call(update_nodes_token, rtas_buf, scope);
if (rc && rc != 1)
@@ -321,6 +323,7 @@ int pseries_devicetree_update(s32 scope)
} while (rc == 1);
kfree(rtas_buf);
+ unlock_device_hotplug();
return rc;
}
^ permalink raw reply related
* [RFC v5 4/6] powerpc/dlpar: Provide CPU readd operation
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
In-Reply-To: <4b1138ff-4ca0-bdce-26cc-b42191258017@linux.vnet.ibm.com>
powerpc/dlpar: Provide hotplug CPU 'readd by index' operation to
support LPAR Post Migration state updates. When such changes are
invoked by the PowerPC 'mobility' code, they will be queued up so
that modifications to CPU properties will take place after the new
property value is written to the device-tree.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 29 ++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index ec78cc6..ac08d85 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -691,6 +691,26 @@ static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
return rc;
}
+static int dlpar_cpu_readd_by_index(u32 drc_index)
+{
+ int rc = 0;
+
+ pr_info("Attempting to re-add CPU, drc index %x\n", drc_index);
+
+ rc = dlpar_cpu_remove_by_index(drc_index, false);
+ if (!rc)
+ rc = dlpar_cpu_add(drc_index, false);
+
+ if (rc)
+ pr_info("Failed to update cpu at drc_index %lx\n",
+ (unsigned long int)drc_index);
+ else
+ pr_info("CPU at drc_index %lx was updated\n",
+ (unsigned long int)drc_index);
+
+ return rc;
+}
+
static int find_dlpar_cpus_to_remove(u32 *cpu_drcs, int cpus_to_remove)
{
struct device_node *dn;
@@ -902,6 +922,9 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
else
rc = -EINVAL;
break;
+ case PSERIES_HP_ELOG_ACTION_READD:
+ rc = dlpar_cpu_readd_by_index(drc_index);
+ break;
default:
pr_err("Invalid action (%d) specified\n", hp_elog->action);
rc = -EINVAL;
@@ -968,6 +991,12 @@ static int pseries_smp_notifier(struct notifier_block *nb,
case OF_RECONFIG_DETACH_NODE:
pseries_remove_processor(rd->dn);
break;
+ case OF_RECONFIG_UPDATE_PROPERTY:
+ if (!strcmp(rd->prop->name, "ibm,associativity"))
+ dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_CPU,
+ PSERIES_HP_ELOG_ACTION_READD,
+ be32_to_cpu(rd->dn->phandle));
+ break;
}
return notifier_from_errno(err);
}
^ permalink raw reply related
* [RFC v5 3/6] migration/dlpar: Add device readd queuing function
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
In-Reply-To: <4b1138ff-4ca0-bdce-26cc-b42191258017@linux.vnet.ibm.com>
migration/dlpar: This patch adds function dlpar_readd_action()
which will queue a worker function to 'readd' a device in the
system. Such devices must be identified by a 'resource' type
and a drc_index to be readded.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 14 ++++++++++++++
arch/powerpc/platforms/pseries/pseries.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a0b20c0..a14684e 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -407,6 +407,20 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
}
}
+int dlpar_queue_action(int resource, int action, u32 drc_index)
+{
+ struct pseries_hp_errorlog hp_elog;
+
+ hp_elog.resource = resource;
+ hp_elog.action = action;
+ hp_elog.id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
+ hp_elog._drc_u.drc_index = drc_index;
+
+ queue_hotplug_event(&hp_elog, NULL, NULL);
+
+ return 0;
+}
+
static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
{
char *arg;
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 60db2ee..cb2beb1 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -61,6 +61,7 @@ extern struct device_node *dlpar_configure_connector(__be32,
void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
struct completion *hotplug_done, int *rc);
+extern int dlpar_queue_action(int resource, int action, u32 drc_index);
#ifdef CONFIG_MEMORY_HOTPLUG
int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
#else
^ permalink raw reply related
* [RFC v5 2/6] powerpc/cpu: Conditionally acquire/release DRC index
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
In-Reply-To: <4b1138ff-4ca0-bdce-26cc-b42191258017@linux.vnet.ibm.com>
powerpc/cpu: Modify dlpar_cpu_add and dlpar_cpu_remove to allow the
skipping of DRC index acquire or release operations during the CPU
add or remove operations. This is intended to support subsequent
changes to provide a 'CPU readd' operation.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 71 +++++++++++++++-----------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index a408217..ec78cc6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -474,7 +474,7 @@ static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
&cdata);
}
-static ssize_t dlpar_cpu_add(u32 drc_index)
+static ssize_t dlpar_cpu_add(u32 drc_index, bool acquire_drc)
{
struct device_node *dn, *parent;
int rc, saved_rc;
@@ -499,19 +499,22 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
return -EINVAL;
}
- rc = dlpar_acquire_drc(drc_index);
- if (rc) {
- pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
- rc, drc_index);
- of_node_put(parent);
- return -EINVAL;
+ if (acquire_drc) {
+ rc = dlpar_acquire_drc(drc_index);
+ if (rc) {
+ pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
+ rc, drc_index);
+ of_node_put(parent);
+ return -EINVAL;
+ }
}
dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
if (!dn) {
pr_warn("Failed call to configure-connector, drc index: %x\n",
drc_index);
- dlpar_release_drc(drc_index);
+ if (acquire_drc)
+ dlpar_release_drc(drc_index);
of_node_put(parent);
return -EINVAL;
}
@@ -526,8 +529,9 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
dn->name, rc, drc_index);
- rc = dlpar_release_drc(drc_index);
- if (!rc)
+ if (acquire_drc)
+ rc = dlpar_release_drc(drc_index);
+ if (!rc || acquire_drc)
dlpar_free_cc_nodes(dn);
return saved_rc;
@@ -540,7 +544,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
dn->name, rc, drc_index);
rc = dlpar_detach_node(dn);
- if (!rc)
+ if (!rc && acquire_drc)
dlpar_release_drc(drc_index);
return saved_rc;
@@ -608,7 +612,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
}
-static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
+static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index,
+ bool release_drc)
{
int rc;
@@ -621,12 +626,14 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
return -EINVAL;
}
- rc = dlpar_release_drc(drc_index);
- if (rc) {
- pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
- drc_index, dn->name, rc);
- dlpar_online_cpu(dn);
- return rc;
+ if (release_drc) {
+ rc = dlpar_release_drc(drc_index);
+ if (rc) {
+ pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
+ drc_index, dn->name, rc);
+ dlpar_online_cpu(dn);
+ return rc;
+ }
}
rc = dlpar_detach_node(dn);
@@ -635,7 +642,10 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
pr_warn("Failed to detach CPU %s, rc: %d", dn->name, rc);
- rc = dlpar_acquire_drc(drc_index);
+ if (release_drc)
+ rc = dlpar_acquire_drc(drc_index);
+ else
+ rc = 0;
if (!rc)
dlpar_online_cpu(dn);
@@ -664,7 +674,7 @@ static struct device_node *cpu_drc_index_to_dn(u32 drc_index)
return dn;
}
-static int dlpar_cpu_remove_by_index(u32 drc_index)
+static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
{
struct device_node *dn;
int rc;
@@ -676,7 +686,7 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
return -ENODEV;
}
- rc = dlpar_cpu_remove(dn, drc_index);
+ rc = dlpar_cpu_remove(dn, drc_index, release_drc);
of_node_put(dn);
return rc;
}
@@ -741,7 +751,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
}
for (i = 0; i < cpus_to_remove; i++) {
- rc = dlpar_cpu_remove_by_index(cpu_drcs[i]);
+ rc = dlpar_cpu_remove_by_index(cpu_drcs[i], true);
if (rc)
break;
@@ -752,7 +762,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
pr_warn("CPU hot-remove failed, adding back removed CPUs\n");
for (i = 0; i < cpus_removed; i++)
- dlpar_cpu_add(cpu_drcs[i]);
+ dlpar_cpu_add(cpu_drcs[i], true);
rc = -EINVAL;
} else {
@@ -843,7 +853,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
}
for (i = 0; i < cpus_to_add; i++) {
- rc = dlpar_cpu_add(cpu_drcs[i]);
+ rc = dlpar_cpu_add(cpu_drcs[i], true);
if (rc)
break;
@@ -854,7 +864,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
pr_warn("CPU hot-add failed, removing any added CPUs\n");
for (i = 0; i < cpus_added; i++)
- dlpar_cpu_remove_by_index(cpu_drcs[i]);
+ dlpar_cpu_remove_by_index(cpu_drcs[i], true);
rc = -EINVAL;
} else {
@@ -880,7 +890,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
rc = dlpar_cpu_remove_by_count(count);
else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
- rc = dlpar_cpu_remove_by_index(drc_index);
+ rc = dlpar_cpu_remove_by_index(drc_index, true);
else
rc = -EINVAL;
break;
@@ -888,7 +898,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
rc = dlpar_cpu_add_by_count(count);
else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
- rc = dlpar_cpu_add(drc_index);
+ rc = dlpar_cpu_add(drc_index, true);
else
rc = -EINVAL;
break;
@@ -913,7 +923,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
if (rc)
return -EINVAL;
- rc = dlpar_cpu_add(drc_index);
+ rc = dlpar_cpu_add(drc_index, true);
return rc ? rc : count;
}
@@ -934,7 +944,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
return -EINVAL;
}
- rc = dlpar_cpu_remove(dn, drc_index);
+ rc = dlpar_cpu_remove(dn, drc_index, true);
of_node_put(dn);
return rc ? rc : count;
@@ -948,6 +958,9 @@ static int pseries_smp_notifier(struct notifier_block *nb,
struct of_reconfig_data *rd = data;
int err = 0;
+ if (strcmp(rd->dn->type, "cpu"))
+ return notifier_from_errno(err);
+
switch (action) {
case OF_RECONFIG_ATTACH_NODE:
err = pseries_add_processor(rd->dn);
^ permalink raw reply related
* [RFC v5 1/6] powerpc/drmem: Export 'dynamic-memory' loader
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
In-Reply-To: <4b1138ff-4ca0-bdce-26cc-b42191258017@linux.vnet.ibm.com>
powerpc/drmem: Export many of the functions of DRMEM to parse
"ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
operations and for Post Migration events.
Also modify the DRMEM initialization code to allow it to,
* Be called after system initialization
* Provide a separate user copy of the LMB array that is produces
* Free the user copy upon request
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in RFC:
-- Separate DRMEM changes into a standalone patch
-- Do not export excess functions. Make exported names more explicit.
-- Add new iterator to work through a pair of drmem_info arrays.
-- Modify DRMEM code to replace usages of dt_root_addr_cells, and
dt_mem_next_cell, as these are only available at first boot.
-- Rebase to 4.17-rc5 kernel
-- Apply several code and patch cleanups.
---
arch/powerpc/include/asm/drmem.h | 10 +++++
arch/powerpc/mm/drmem.c | 73 ++++++++++++++++++++++++++++----------
2 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index ce242b9..e82d254 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -35,6 +35,13 @@ struct drmem_lmb_info {
&drmem_info->lmbs[0], \
&drmem_info->lmbs[drmem_info->n_lmbs - 1])
+#define for_each_pair_drmem_lmb(dinfo1, lmb1, dinfo2, lmb2) \
+ for ((lmb1) = (&dinfo1->lmbs[0]), \
+ (lmb2) = (&dinfo2->lmbs[0]); \
+ ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
+ ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
+ (lmb1)++, (lmb2)++)
+
/*
* The of_drconf_cell_v1 struct defines the layout of the LMB data
* specified in the ibm,dynamic-memory device tree property.
@@ -94,6 +101,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
int drmem_update_dt(void);
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
+
#ifdef CONFIG_PPC_PSERIES
void __init walk_drmem_lmbs_early(unsigned long node,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 3f18036..2bd6a70 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -20,6 +20,7 @@
static struct drmem_lmb_info __drmem_info;
struct drmem_lmb_info *drmem_info = &__drmem_info;
+static int n_root_addr_cells;
u64 drmem_lmb_memory_max(void)
{
@@ -193,12 +194,13 @@ int drmem_update_dt(void)
return rc;
}
-static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
+static void read_drconf_v1_cell(struct drmem_lmb *lmb,
const __be32 **prop)
{
const __be32 *p = *prop;
- lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ lmb->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
lmb->drc_index = of_read_number(p++, 1);
p++; /* skip reserved field */
@@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
*prop = p;
}
-static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct drmem_lmb lmb;
@@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
}
}
-static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
const __be32 **prop)
{
const __be32 *p = *prop;
dr_cell->seq_lmbs = of_read_number(p++, 1);
- dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
dr_cell->drc_index = of_read_number(p++, 1);
dr_cell->aa_index = of_read_number(p++, 1);
dr_cell->flags = of_read_number(p++, 1);
@@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
*prop = p;
}
-static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct of_drconf_cell_v2 dr_cell;
@@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
const __be32 *prop, *usm;
int len;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
if (!prop || len < dt_root_size_cells * sizeof(__be32))
return;
@@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
}
}
-static void __init init_drmem_v1_lmbs(const __be32 *prop)
+static void init_drmem_v1_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
- drmem_info->n_lmbs = of_read_number(prop++, 1);
- if (drmem_info->n_lmbs == 0)
+ dinfo->n_lmbs = of_read_number(prop++, 1);
+ if (dinfo->n_lmbs == 0)
return;
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
for_each_drmem_lmb(lmb)
read_drconf_v1_cell(lmb, &prop);
}
-static void __init init_drmem_v2_lmbs(const __be32 *prop)
+static void init_drmem_v2_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
struct of_drconf_cell_v2 dr_cell;
@@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
p = prop;
for (i = 0; i < lmb_sets; i++) {
read_drconf_v2_cell(&dr_cell, &p);
- drmem_info->n_lmbs += dr_cell.seq_lmbs;
+ dinfo->n_lmbs += dr_cell.seq_lmbs;
}
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
/* second pass, read in the LMB information */
@@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
read_drconf_v2_cell(&dr_cell, &p);
for (j = 0; j < dr_cell.seq_lmbs; j++) {
- lmb = &drmem_info->lmbs[lmb_index++];
+ lmb = &dinfo->lmbs[lmb_index++];
lmb->base_addr = dr_cell.base_addr;
- dr_cell.base_addr += drmem_info->lmb_size;
+ dr_cell.base_addr += dinfo->lmb_size;
lmb->drc_index = dr_cell.drc_index;
dr_cell.drc_index++;
@@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
}
}
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
+{
+ if (dinfo) {
+ kfree(dinfo->lmbs);
+ kfree(dinfo);
+ }
+}
+
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
+{
+ struct drmem_lmb_info *dinfo;
+
+ dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
+ if (!dinfo)
+ return NULL;
+
+ if (!strcmp("ibm,dynamic-memory", prop->name))
+ init_drmem_v1_lmbs(prop->value, dinfo);
+ else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
+ init_drmem_v2_lmbs(prop->value, dinfo);
+
+ return dinfo;
+}
+
static int __init drmem_init(void)
{
struct device_node *dn;
const __be32 *prop;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (!dn) {
pr_info("No dynamic reconfiguration memory found\n");
@@ -434,11 +469,11 @@ static int __init drmem_init(void)
prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
if (prop) {
- init_drmem_v1_lmbs(prop);
+ init_drmem_v1_lmbs(prop, drmem_info);
} else {
prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
if (prop)
- init_drmem_v2_lmbs(prop);
+ init_drmem_v2_lmbs(prop, drmem_info);
}
of_node_put(dn);
^ permalink raw reply related
* [RFC v6 0/6] powerpc/hotplug: Fix affinity assoc for LPAR migration
From: Michael Bringmann @ 2018-05-22 23:36 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Bringmann, Nathan Fontenot, John Allen, Tyrel Datwyler,
Thomas Falcon
The migration of LPARs across Power systems affects many attributes
including that of the associativity of memory blocks and CPUs. The
patches in this set execute when a system is coming up fresh upon a
migration target. They are intended to,
* Recognize changes to the associativity of memory and CPUs recorded
in internal data structures when compared to the latest copies in
the device tree (e.g. ibm,dynamic-memory, ibm,dynamic-memory-v2,
cpus),
* Recognize changes to the associativity mapping (e.g. ibm,
associativity-lookup-arrays), locate all assigned memory blocks
corresponding to each changed row, and readd all such blocks.
* Generate calls to other code layers to reset the data structures
related to associativity of the CPUs and memory.
* Re-register the 'changed' entities into the target system.
Re-registration of CPUs and memory blocks mostly entails acting as
if they have been newly hot-added into the target system.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
Michael Bringmann (3):
powerpc migration/drmem: Modify DRMEM code to export more features
powerpc migration/cpu: Associativity & cpu changes
powerpc migration/memory: Associativity & memory updates
---
Changes in RFC:
-- Restructure and rearrange content of patches to co-locate
similar or related modifications
-- Rename pseries_update_drconf_cpu to pseries_update_cpu
-- Simplify code to update CPU nodes during mobility checks.
Remove functions to generate extra HP_ELOG messages in favor
of direct function calls to dlpar_cpu_readd_by_index, or
dlpar_memory_readd_by_index.
-- Revise code order in dlpar_cpu_readd_by_index() to present
more appropriate error codes from underlying layers of the
implementation.
-- Add hotplug device lock around all property updates
-- Schedule all CPU and memory changes due to device-tree updates /
LPAR mobility as workqueue operations
-- Export DRMEM accessor functions to parse 'ibm,dynamic-memory-v2'
-- Export DRMEM functions to provide user copies of LMB array
-- Compress code using DRMEM accessor functions.
-- Split topology timer crash fix into new patch.
-- Modify DRMEM code to replace usages of dt_root_addr_cells, and
dt_mem_next_cell, as these are only available at first boot.
-- Correct a bug in DRC index selection for queued operation.
-- Rebase to 4.17-rc5 kernel
-- Minor code cleanups
-- Correct drc_index for worker fn invocation
^ permalink raw reply
* Re: [RFC v4 4/4] hotplug/drcinfo: Code cleanup for devices
From: Nathan Fontenot @ 2018-05-22 21:39 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <3912e8ff-0361-bc42-b8bd-ed1c4d24e218@linux.vnet.ibm.com>
On 05/22/2018 11:37 AM, Michael Bringmann wrote:
> This patch extends the use of a common parse function for the
> ibm,drc-info property that can be modified by a callback function
> to the hotplug device processing. Candidate code is replaced by
> a call to the parser including a pointer to a local context-specific
> functions, and local data.
>
> In addition, several more opportunities to compress and reuse
> common code between the old and new property parsers were applied.
>
> Finally, a bug with the registration of slots was observed on some
> systems, and the code was rewritten to prevent its reoccurrence.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info firmwar
> e feature" -- end of patch series applied to powerpc next)
> ---
> Changes in V4:
> -- Update code to account for latest kernel checkins.
> -- Fix bug searching for virtual device slots.
> -- Rebased to 4.17-rc5 kernel
> -- Patch cleanup
> ---
> drivers/pci/hotplug/rpaphp_core.c | 181 ++++++++++++++++++++++++++-----------
> 1 file changed, 126 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index 435c1a0..dc4ec68 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -222,47 +222,51 @@ static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
> return -EINVAL;
> }
>
> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
> - char *drc_type, unsigned int my_index)
> +struct check_drc_props_v2_struct {
> + char *drc_name;
> + char *drc_type;
> + unsigned int my_index;
> +};
> +
> +static int check_drc_props_v2_cb(struct of_drc_info *drc, void *idata,
> + void *not_used, int *ret_code)
> {
> - struct property *info;
> - unsigned int entries;
> - struct of_drc_info drc;
> - const __be32 *value;
> + struct check_drc_props_v2_struct *cdata = idata;
> char cell_drc_name[MAX_DRC_NAME_LEN];
> - int j, fndit;
>
> - info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> - if (info == NULL)
> - return -EINVAL;
> + (*ret_code) = -EINVAL;
>
> - value = info->value;
> - entries = of_read_number(value++, 1);
> -
> - for (j = 0; j < entries; j++) {
> - of_read_drc_info_cell(&info, &value, &drc);
> -
> - /* Should now know end of current entry */
> -
> - if (my_index > drc.last_drc_index)
> - continue;
> + if (cdata->my_index > drc->last_drc_index)
> + return 0;
>
> - fndit = 1;
> - break;
> + /* Found drc_index. Now match the rest. */
> + sprintf(cell_drc_name, "%s%d", drc->drc_name_prefix,
> + cdata->my_index - drc->drc_index_start +
> + drc->drc_name_suffix_start);
> +
> + if (((cdata->drc_name == NULL) ||
> + (cdata->drc_name && !strcmp(cdata->drc_name, cell_drc_name))) &&
> + ((cdata->drc_type == NULL) ||
> + (cdata->drc_type && !strcmp(cdata->drc_type, drc->drc_type)))) {
> + (*ret_code) = 0;
> + return 1;
> }
> - /* Found it */
>
> - if (fndit)
> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> - my_index);
> + return 0;
> +}
>
> - if (((drc_name == NULL) ||
> - (drc_name && !strcmp(drc_name, cell_drc_name))) &&
> - ((drc_type == NULL) ||
> - (drc_type && !strcmp(drc_type, drc.drc_type))))
> - return 0;
> +static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
> + char *drc_type, unsigned int my_index)
> +{
> + struct device_node *root = dn;
> + struct check_drc_props_v2_struct cdata = {
> + drc_name, drc_type, be32_to_cpu(my_index) };
>
> - return -EINVAL;
> + if (!drc_type || (drc_type && strcmp(drc_type, "SLOT")))
> + root = dn->parent;
> +
> + return drc_info_parser(root, check_drc_props_v2_cb,
> + drc_type, &cdata);
> }
>
> int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
> @@ -285,7 +289,6 @@ int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
> }
> EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>
> -
> static int is_php_type(char *drc_type)
> {
> unsigned long value;
> @@ -345,17 +348,41 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
> *
> * To remove a slot, it suffices to call rpaphp_deregister_slot().
> */
> -int rpaphp_add_slot(struct device_node *dn)
> +
> +static int rpaphp_add_slot_common(struct device_node *dn,
> + u32 drc_index, char *drc_name, char *drc_type,
> + u32 drc_power_domain)
> {
> struct slot *slot;
> int retval = 0;
> - int i;
> +
> + slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
> + if (!slot)
> + return -ENOMEM;
> +
> + slot->type = simple_strtoul(drc_type, NULL, 10);
> + if (retval)
> + return -EINVAL;
> +
> + dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> + drc_index, drc_name, drc_type);
> +
> + retval = rpaphp_enable_slot(slot);
> + if (!retval)
> + retval = rpaphp_register_slot(slot);
> +
> + if (retval)
> + dealloc_slot_struct(slot);
> +
> + return retval;
> +}
> +
> +static int rpaphp_add_slot_v1(struct device_node *dn)
> +{
> + int i, retval = 0;
> 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;
> @@ -366,25 +393,12 @@ int rpaphp_add_slot(struct device_node *dn)
> name = (char *) &names[1];
> type = (char *) &types[1];
> for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
> - int index;
> -
> - index = be32_to_cpu(indexes[i + 1]);
> - slot = alloc_slot_struct(dn, index, name,
> - be32_to_cpu(power_domains[i + 1]));
> - if (!slot)
> - return -ENOMEM;
> -
> - slot->type = simple_strtoul(type, NULL, 10);
>
> - dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
> - index, name, type);
> -
> - retval = rpaphp_enable_slot(slot);
> + retval = rpaphp_add_slot_common(dn,
> + be32_to_cpu(indexes[i + 1]), name, type,
> + be32_to_cpu(power_domains[i + 1]));
> if (!retval)
> - retval = rpaphp_register_slot(slot);
> -
> - if (retval)
> - dealloc_slot_struct(slot);
> + return retval;
>
> name += strlen(name) + 1;
> type += strlen(type) + 1;
> @@ -394,6 +408,63 @@ int rpaphp_add_slot(struct device_node *dn)
> /* XXX FIXME: reports a failure only if last entry in loop failed */
> return retval;
> }
> +
> +struct rpaphp_add_slot_v2_struct {
> + struct device_node *dn;
> +};
> +
> +static int rpaphp_add_slot_v2_cb(struct of_drc_info *drc, void *idata,
> + void *not_used, int *ret_code)
> +{
> + struct rpaphp_add_slot_v2_struct *cdata = idata;
> + u32 drc_index;
> + char drc_name[MAX_DRC_NAME_LEN];
> + int i, retval;
> +
> + (*ret_code) = -EINVAL;
> +
> + if (!is_php_type((char *) drc->drc_type)) {
> + (*ret_code) = 0;
> + return 1;
> + }
> +
> + for (i = 0, drc_index = drc->drc_index_start;
> + i < drc->num_sequential_elems; i++, drc_index++) {
> +
> + sprintf(drc_name, "%s%d", drc->drc_name_prefix,
> + drc_index - drc->drc_index_start +
> + drc->drc_name_suffix_start);
> +
> + retval = rpaphp_add_slot_common(cdata->dn,
> + drc_index, drc_name, drc->drc_type,
> + drc->drc_power_domain);
> + if (!retval) {
> + (*ret_code) = retval;
> + return 1;
> + }
> + }
I may be reading this loop wrong, but it appears that it will add all drc indexes
for all drc_info entries to the specified device node, minus drc-types that don't match.
-Nathan
> +
> + (*ret_code) = retval;
> + return 0;
> +}
> +
> +static int rpaphp_add_slot_v2(struct device_node *dn)
> +{
> + struct rpaphp_add_slot_v2_struct cdata = { dn };
> +
> + return drc_info_parser(dn, rpaphp_add_slot_v2_cb, NULL, &cdata);
> +}
> +
> +int rpaphp_add_slot(struct device_node *dn)
> +{
> + if (!dn->name || strcmp(dn->name, "pci"))
> + return 0;
> +
> + if (firmware_has_feature(FW_FEATURE_DRC_INFO))
> + return rpaphp_add_slot_v2(dn);
> + else
> + return rpaphp_add_slot_v1(dn);
> +}
> EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>
> static void __exit cleanup_slots(void)
>
^ permalink raw reply
* Re: [RFC v4 3/4] hotplug/drcinfo: Fix hot-add CPU issues
From: Nathan Fontenot @ 2018-05-22 21:31 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <6f2c522c-8c57-9416-3860-10a5270da059@linux.vnet.ibm.com>
On 05/22/2018 11:37 AM, Michael Bringmann wrote:
> This patch applies a common parse function for the ibm,drc-info
> property that can be modified by a callback function to the
> hot-add CPU code. Candidate code is replaced by a call to the
> parser including a pointer to a local context-specific functions,
> and local data.
>
> In addition, a bug in the release of the previous patch set may
> break things in some of the CPU DLPAR operations. For instance,
> when attempting to hot-add a new CPU or set of CPUs, the original
> patch failed to always properly calculate the available resources,
> and aborted the operation.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info firmwar
> e feature" -- end of patch series applied to powerpc next)
> ---
> Changes in V4:
> -- Update code to account for latest kernel checkins.
> -- Rebased to 4.17-rc5 kernel
> -- Compress some more code
> ---
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 118 +++++++++++++++++------
> arch/powerpc/platforms/pseries/pseries_energy.c | 107 +++++++++++----------
> 2 files changed, 141 insertions(+), 84 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 6ef77ca..ceacad9 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -411,27 +411,63 @@ static bool dlpar_cpu_exists(struct device_node *parent, u32 drc_index)
> return found;
> }
>
> -static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
> +static bool check_cpu_drc_index(struct device_node *parent,
> + int (*cb)(struct of_drc_info *drc,
> + void *data, void *not_used,
> + int *ret_code),
> + void *cdata)
> {
> bool found = false;
> - int rc, index;
>
> - index = 0;
> - while (!found) {
> - u32 drc;
> + if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
> + if (drc_info_parser(parent, cb, "CPU", cdata))
> + found = true;
> + } else {
> + int index = 0;
>
> - rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
> - index++, &drc);
> - if (rc)
> - break;
> + while (!found) {
> + u32 drc;
>
> - if (drc == drc_index)
> - found = true;
> + if (of_property_read_u32_index(parent,
> + "ibm,drc-indexes",
> + index++, &drc))
> + break;
> + if (cb(NULL, cdata, &drc, NULL))
> + found = true;
> + }
> }
>
> return found;
> }
>
> +struct valid_drc_index_struct {
> + u32 targ_drc_index;
> +};
Can you help me understand the need to encapsulate the drc_index as a struct.
> +
> +static int valid_drc_index_cb(struct of_drc_info *drc, void *idata,
> + void *drc_index, int *ret_code)
> +{
> + struct valid_drc_index_struct *cdata = idata;
> +
> + if (drc) {
> + if (!((drc->drc_index_start <= cdata->targ_drc_index) &&
> + (cdata->targ_drc_index <= drc->last_drc_index)))
> + return 0;
> + } else {
> + if (*((u32 *)drc_index) != cdata->targ_drc_index)
> + return 0;
> + }
> + (*ret_code) = 1;
> + return 1;
> +}
> +
> +static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
> +{
> + struct valid_drc_index_struct cdata = { drc_index };
> +
> + return check_cpu_drc_index(parent, valid_drc_index_cb, &cdata);
> +}
> +
> static ssize_t dlpar_cpu_add(u32 drc_index)
> {
> struct device_node *dn, *parent;
> @@ -721,11 +757,43 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
> return rc;
> }
>
> +struct cpus_to_add_struct {
> + struct device_node *parent;
> + u32 *cpu_drcs;
> + u32 cpus_to_add;
> + u32 cpus_found;
> +};
> +
> +static int cpus_to_add_cb(struct of_drc_info *drc, void *idata,
> + void *drc_index, int *ret_code)
> +{
> + struct cpus_to_add_struct *cdata = idata;
> +
> + if (drc) {
> + int k;
> +
> + for (k = 0; (k < drc->num_sequential_elems) &&
> + (cdata->cpus_found < cdata->cpus_to_add); k++) {
> + u32 idrc = drc->drc_index_start +
> + (k * drc->sequential_inc);
> +
> + if (dlpar_cpu_exists(cdata->parent, idrc))
> + continue;
> + cdata->cpu_drcs[cdata->cpus_found++] = idrc;
> + }
> + } else {
> + if (!dlpar_cpu_exists(cdata->parent, *((u32 *)drc_index)))
> + cdata->cpu_drcs[cdata->cpus_found++] =
> + *((u32 *)drc_index);
> + }
> + return 0;
> +}
> +
> static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
> {
> struct device_node *parent;
> - int cpus_found = 0;
> - int index, rc;
> + struct cpus_to_add_struct cdata = {
> + NULL, cpu_drcs, cpus_to_add, 0 };
>
> parent = of_find_node_by_path("/cpus");
> if (!parent) {
> @@ -734,28 +802,14 @@ 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.
> + /* Search the appropriate property for possible CPU drcs to
> + * add.
> */
> - index = 1;
> - while (cpus_found < cpus_to_add) {
> - u32 drc;
> -
> - rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
> - index++, &drc);
> - if (rc)
> - break;
> -
> - if (dlpar_cpu_exists(parent, drc))
> - continue;
> -
> - cpu_drcs[cpus_found++] = drc;
> - }
> + cdata.parent = parent;
> + check_cpu_drc_index(parent, cpus_to_add_cb, &cdata);
>
> of_node_put(parent);
> - return cpus_found;
> + return cdata.cpus_found;
> }
>
> static int dlpar_cpu_add_by_count(u32 cpus_to_add)
> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> index 5261975..d8d7750 100644
> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
> @@ -36,6 +36,26 @@
>
> /* Helper Routines to convert between drc_index to cpu numbers */
>
> +struct cpu_to_drc_index_struct {
> + u32 thread_index;
> + u32 ret;
> +};
> +
> +static int cpu_to_drc_index_cb(struct of_drc_info *drc, void *idata,
> + void *not_used, int *ret_code)
> +{
> + struct cpu_to_drc_index_struct *cdata = idata;
> + int ret = 0;
> +
> + if (cdata->thread_index < drc->last_drc_index) {
Is this correct? You're comparing a thread index to a drc index.
> + cdata->ret = drc->drc_index_start +
> + (cdata->thread_index * drc->sequential_inc);
> + ret = 1;
> + }
> + (*ret_code) = ret;
> + return ret;
> +}
> +
> static u32 cpu_to_drc_index(int cpu)
> {
> struct device_node *dn = NULL;
> @@ -51,30 +71,14 @@ static u32 cpu_to_drc_index(int cpu)
> 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;
> - const __be32 *value;
> -
> - info = of_find_property(dn, "ibm,drc-info", NULL);
> - if (info == NULL)
> - goto err_of_node_put;
> + struct cpu_to_drc_index_struct cdata = {
> + thread_index, 0 };
>
> - value = info->value;
> - num_set_entries = of_read_number(value++, 1);
> -
> - for (j = 0; j < num_set_entries; j++) {
> -
> - of_read_drc_info_cell(&info, &value, &drc);
> - if (strncmp(drc.drc_type, "CPU", 3))
> - goto err;
> -
> - if (thread_index < drc.last_drc_index)
> - break;
> - }
> -
> - ret = drc.drc_index_start + (thread_index * drc.sequential_inc);
> + rc = drc_info_parser(dn, &cpu_to_drc_index_cb,
> + "CPU", &cdata);
> + if (rc < 0)
> + goto err_of_node_put;
> + ret = cdata.ret;
> } else {
> const __be32 *indexes;
>
> @@ -100,11 +104,33 @@ static u32 cpu_to_drc_index(int cpu)
> return ret;
> }
>
> +struct drc_index_to_cpu_struct {
> + u32 drc_index;
> + u32 thread_index;
> + u32 cpu;
> +};
> +
> +static int drc_index_to_cpu_cb(struct of_drc_info *drc,
> + void *idata, void *not_used, int *ret_code)
> +{
> + struct drc_index_to_cpu_struct *cdata = idata;
> +
> + if (cdata->drc_index > drc->last_drc_index) {
> + cdata->cpu += drc->num_sequential_elems;
> + } else {
> + cdata->cpu += ((cdata->drc_index - drc->drc_index_start) /
> + drc->sequential_inc);
> + cdata->thread_index = cpu_first_thread_of_core(cdata->cpu);
Should this return 1 here to avoid continuing to walk the drc_info entries?
-Nathan
> + }
> + (*ret_code) = 0;
> + return 0;
> +}
> +
> static int drc_index_to_cpu(u32 drc_index)
> {
> struct device_node *dn = NULL;
> const int *indexes;
> - int thread_index = 0, cpu = 0;
> + int thread_index = 0;
> int rc = 1;
>
> dn = of_find_node_by_path("/cpus");
> @@ -112,36 +138,13 @@ static int drc_index_to_cpu(u32 drc_index)
> goto err;
>
> if (firmware_has_feature(FW_FEATURE_DRC_INFO)) {
> - struct property *info = NULL;
> - 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 = info->value;
> - num_set_entries = of_read_number(value++, 1);
> -
> - for (j = 0; j < num_set_entries; j++) {
> + struct drc_index_to_cpu_struct cdata = {
> + drc_index, 0, 0 };
>
> - of_read_drc_info_cell(&info, &value, &drc);
> - if (strncmp(drc.drc_type, "CPU", 3))
> - goto err;
> + rc = drc_info_parser(dn, &drc_index_to_cpu_cb,
> + "CPU", &cdata);
> + thread_index = cdata.thread_index;
>
> - if (drc_index > drc.last_drc_index) {
> - cpu += drc.num_sequential_elems;
> - continue;
> - }
> - 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;
>
^ permalink raw reply
* Re: [RFC v4 2/4] hotplug/drcinfo: Provide parser with callback
From: Nathan Fontenot @ 2018-05-22 21:23 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <2e2261b4-c0b3-3b6d-d649-541bc8802546@linux.vnet.ibm.com>
On 05/22/2018 11:37 AM, Michael Bringmann wrote:
> This patch provides a common parse function for the ibm,drc-info
> property that can be modified by a callback function. The caller
> provides a pointer to the function and a pointer to their unique
> data, and the parser provides the current lmb set from the struct.
> The callback function may return codes indicating that the parsing
> is complete, or should continue, along with an error code that may
> be returned to the caller.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info firmwar
> e feature" -- end of patch series applied to powerpc next)
> ---
> Changes in V4:
> -- Update code to account for latest kernel checkins.
> -- Rebased to 4.17-rc5 kernel
> -- Some patch cleanup including file combination
> ---
> arch/powerpc/include/asm/prom.h | 7 +++++
> arch/powerpc/platforms/pseries/of_helpers.c | 37 +++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index b04c5ce..2e947b3 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -94,6 +94,13 @@ struct of_drc_info {
> extern int of_read_drc_info_cell(struct property **prop,
> const __be32 **curval, struct of_drc_info *data);
>
> +extern int drc_info_parser(struct device_node *dn,
> + int (*usercb)(struct of_drc_info *drc,
> + void *data,
> + void *optional_data,
> + int *ret_code),
> + char *opt_drc_type,
> + void *data);
After looking at the patch 3 in this series, I think a couple of comments and
a small change may help. It was not clear at first what the call back function
was supposed to return. After reading users of this routine it appears that the
callback function is returning a bool value indicating whether or not the parser
should continue. I documenting this and having the callback routine return a bool
may make this clearer.
Also, I see other places in the kernel name these types of routines as walk_*,
perhaps a slight name change to walk_drc_info_entries() may also make it clearer
what the code is doing.
-Nathan
>
> /*
> * There are two methods for telling firmware what our capabilities are.
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 11b2ef1..a588ee6 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -6,6 +6,9 @@
> #include <asm/prom.h>
>
> #include "of_helpers.h"
> +#include "pseries.h"
> +
> +#define MAX_DRC_NAME_LEN 64
>
> /**
> * pseries_of_derive_parent - basically like dirname(1)
> @@ -87,3 +90,37 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
> return 0;
> }
> EXPORT_SYMBOL(of_read_drc_info_cell);
> +
> +int drc_info_parser(struct device_node *dn,
> + int (*usercb)(struct of_drc_info *drc,
> + void *data,
> + void *optional_data,
> + int *ret_code),
> + char *opt_drc_type,
> + void *data)
> +{
> + struct property *info;
> + unsigned int entries;
> + struct of_drc_info drc;
> + const __be32 *value;
> + int j, done = 0, ret_code = -EINVAL;
> +
> + info = of_find_property(dn, "ibm,drc-info", NULL);
> + if (info == NULL)
> + return -EINVAL;
> +
> + value = info->value;
> + entries = of_read_number(value++, 1);
> +
> + for (j = 0, done = 0; (j < entries) && (!done); j++) {
> + of_read_drc_info_cell(&info, &value, &drc);
> +
> + if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
> + continue;
> +
> + done = usercb(&drc, data, NULL, &ret_code);
> + }
> +
> + return ret_code;
> +}
> +EXPORT_SYMBOL(drc_info_parser);
>
^ permalink raw reply
* Re: [RFC PATCH] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book E
From: Scott Wood @ 2018-05-22 20:29 UTC (permalink / raw)
To: Diana Craciun, linuxppc-dev; +Cc: mpe, leoyang.li
In-Reply-To: <1526973031-9543-1-git-send-email-diana.craciun@nxp.com>
On Tue, 2018-05-22 at 10:10 +0300, Diana Craciun wrote:
> Implement the barrier_nospec as a isync;sync instruction sequence.
> The implementation uses the infrastructure built for BOOK3S 64
> with the difference that for NXP platforms there is no firmware involved
> and the need for a speculation barrier is read from the device tree.
> I have used the same name for the property:
> fsl,needs-spec-barrier-for-bounds-check
Using the device tree this way means that anyone without an updated device
tree won't get the protection. I also don't see any device tree updates --
which chips are affected? Wouldn't it be more robust to just have the kernel
check the CPU type, especially given that it already does so for a lot of
other purposes?
> +#ifdef CONFIG_PPC_BOOK3S_64
> void setup_barrier_nospec(void)
> {
> bool enable;
> @@ -44,6 +46,12 @@ void setup_barrier_nospec(void)
>
> enable_barrier_nospec(enable);
> }
> +#elif CONFIG_PPC_FSL_BOOK3E
> +void setup_barrier_nospec(void)
> +{
> + enable_barrier_nospec(true);
> +}
> +#endif
The call site is in FSL_BOOK3E-specific code so why not call
enable_barrier_nospec() directly from there?
>
> +#ifdef CONFIG_PPC_BOOK3S_64
> ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute
> *attr, char *buf)
> {
> bool thread_priv;
> @@ -155,3 +164,4 @@ ssize_t cpu_show_spectre_v2(struct device *dev, struct
> device_attribute *attr, c
>
> return s.len;
> }
> +#endif
No equivalent of this for FSL?
> +#ifdef CONFIG_PPC_FSL_BOOK3E
> +void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void
> *fixup_end)
> +{
> + unsigned int instr[2], *dest;
> + long *start, *end;
> + int i;
> +
> + start = fixup_start;
> + end = fixup_end;
> +
> + instr[0] = PPC_INST_NOP; /* nop */
> + instr[1] = PPC_INST_NOP; /* nop */
> +
> + if (enable) {
> + pr_info("barrier_nospec: using isync; sync as a speculation
> barrier\n");
> + instr[0] = PPC_INST_ISYNC;
> + instr[1] = PPC_INST_SYNC;
> + }
> +
> + for (i = 0; start < end; start++, i++) {
> + dest = (void *)start + *start;
> + pr_devel("patching dest %lx\n", (unsigned long)dest);
> +
> + patch_instruction(dest, instr[0]);
> + patch_instruction(dest + 1, instr[1]);
> +
> + }
> +
> + pr_debug("barrier-nospec: patched %d locations\n", i);
Why patch nops in if not enabled? Aren't those locations already nops? For
that matter, how can this function even be called on FSL_BOOK3E with enable !=
true?
> diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c
> b/arch/powerpc/platforms/85xx/corenet_generic.c
> index ac191a7..11bce3d 100644
> --- a/arch/powerpc/platforms/85xx/corenet_generic.c
> +++ b/arch/powerpc/platforms/85xx/corenet_generic.c
> @@ -59,6 +59,21 @@ void __init corenet_gen_pic_init(void)
> }
> }
>
> +static void setup_spec_barrier(void)
> +{
> + struct device_node *np = of_find_node_by_name(NULL, "cpus");
> +
> + if (np) {
> + struct property *prop;
> +
> + prop = of_find_property(np,
> + "fsl,needs-spec-barrier-for-bounds-check", NULL);
> + if (prop)
> + setup_barrier_nospec();
> + of_node_put(np);
> + }
> +}
Why is this in board code?
Should there be a way for the user to choose not to enable this (editing the
device tree doesn't count), for a use case that is not sufficiently security
sensitive to justify the performance loss? What is the performance impact of
this patch?
-Scott
^ permalink raw reply
* Re: [RFC v5 6/6] migration/memory: Update memory for assoc changes
From: Thomas Falcon @ 2018-05-22 21:11 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev
Cc: Nathan Fontenot, John Allen, Tyrel Datwyler
In-Reply-To: <8b17b806-abe7-49cf-0f0f-9b23905cdfd1@linux.vnet.ibm.com>
On 05/21/2018 12:52 PM, Michael Bringmann wrote:
> migration/memory: This patch adds more recognition for changes to
> the associativity of memory blocks described by the device-tree
> properties and updates local and general kernel data structures to
> reflect those changes. These differences may include:
>
> * Evaluating 'ibm,dynamic-memory' properties when processing the
> topology of LPARS in Post Migration events. Previous efforts
> only recognized whether a memory block's assignment had changed
> in the property. Changes here include checking the aa_index
> values for each drc_index of the old/new LMBs and to 'readd'
> any block for which the setting has changed.
>
> * In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
> property may change. In the event that a row of the array differs,
> locate all assigned memory blocks with that 'aa_index' and 're-add'
> them to the system memory block data structures. In the process of
> the 're-add', the system routines will update the corresponding entry
> for the memory in the LMB structures and any other relevant kernel
> data structures.
>
> * Extend the previous work for the 'ibm,associativity-lookup-array'
> and 'ibm,dynamic-memory' properties to support the property
> 'ibm,dynamic-memory-v2' by means of the DRMEM LMB interpretation
> code.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in RFC:
> -- Simplify code to update memory nodes during mobility checks.
> -- Reuse code from DRMEM changes to scan for LMBs when updating
> aa_index
> -- Combine common code for properties 'ibm,dynamic-memory' and
> 'ibm,dynamic-memory-v2' after integrating DRMEM features.
> -- Rearrange patches to co-locate memory property-related changes.
> -- Use new paired list iterator for the drmem info arrays.
> -- Use direct calls to add/remove memory from the update drconf
> function as those operations are only intended for user DLPAR
> ops, and should not occur during Migration reconfig notifier
> changes.
> -- Correct processing bug in processing of ibm,associativity-lookup-arrays
> -- Rebase to 4.17-rc5 kernel
> -- Apply minor code cleanups
> ---
> arch/powerpc/platforms/pseries/hotplug-memory.c | 153 ++++++++++++++++++-----
> 1 file changed, 121 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f5..ac329aa 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -994,13 +994,11 @@ static int pseries_add_mem_node(struct device_node *np)
> return (ret < 0) ? -EINVAL : 0;
> }
>
> -static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
> {
> - struct of_drconf_cell_v1 *new_drmem, *old_drmem;
> + struct drmem_lmb *old_lmb, *new_lmb;
> unsigned long memblock_size;
> - u32 entries;
> - __be32 *p;
> - int i, rc = -EINVAL;
> + int rc = 0;
>
> if (rtas_hp_event)
> return 0;
> @@ -1009,42 +1007,124 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
> if (!memblock_size)
> return -EINVAL;
>
> - p = (__be32 *) pr->old_prop->value;
> - if (!p)
> - return -EINVAL;
> + /* Arrays should have the same size and DRC indexes */
> + for_each_pair_drmem_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
>
> - /* The first int of the property is the number of lmb's described
> - * by the property. This is followed by an array of of_drconf_cell
> - * entries. Get the number of entries and skip to the array of
> - * of_drconf_cell's.
> - */
> - entries = be32_to_cpu(*p++);
> - old_drmem = (struct of_drconf_cell_v1 *)p;
> -
> - p = (__be32 *)pr->prop->value;
> - p++;
> - new_drmem = (struct of_drconf_cell_v1 *)p;
> + if (new_lmb->drc_index != old_lmb->drc_index)
> + continue;
>
> - for (i = 0; i < entries; i++) {
> - if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
> - (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
> + if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
> + (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
> rc = pseries_remove_memblock(
> - be64_to_cpu(old_drmem[i].base_addr),
> - memblock_size);
> + old_lmb->base_addr, memblock_size);
> break;
> - } else if ((!(be32_to_cpu(old_drmem[i].flags) &
> - DRCONF_MEM_ASSIGNED)) &&
> - (be32_to_cpu(new_drmem[i].flags) &
> - DRCONF_MEM_ASSIGNED)) {
> - rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
> - memblock_size);
> + } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
> + (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> + rc = memblock_add(old_lmb->base_addr,
> + memblock_size);
> rc = (rc < 0) ? -EINVAL : 0;
> break;
> + } else if ((old_lmb->aa_index != new_lmb->aa_index) &&
> + (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> + dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_MEM,
> + PSERIES_HP_ELOG_ACTION_READD,
> + new_lmb->drc_index);
> }
> }
> return rc;
> }
>
> +static void pseries_update_ala_memory_aai(int aa_index)
> +{
> + struct drmem_lmb *lmb;
> +
> + /* Readd all LMBs which were previously using the
> + * specified aa_index value.
> + */
> + for_each_drmem_lmb(lmb) {
> + if ((lmb->aa_index == aa_index) &&
> + (lmb->flags & DRCONF_MEM_ASSIGNED)) {
> + dlpar_queue_action(PSERIES_HP_ELOG_RESOURCE_MEM,
> + PSERIES_HP_ELOG_ACTION_READD,
> + lmb->drc_index);
> + }
> + }
> +}
> +
> +struct assoc_arrays {
> + u32 n_arrays;
> + u32 array_sz;
> + const __be32 *arrays;
> +};
> +
> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
> +{
> + struct assoc_arrays new_ala, old_ala;
> + __be32 *p;
> + int i, lim;
> +
> + if (rtas_hp_event)
> + return 0;
> +
> + /*
> + * The layout of the ibm,associativity-lookup-arrays
> + * property is a number N indicating the number of
> + * associativity arrays, followed by a number M
> + * indicating the size of each associativity array,
> + * followed by a list of N associativity arrays.
> + */
> +
> + p = (__be32 *) pr->old_prop->value;
> + if (!p)
> + return -EINVAL;
> + old_ala.n_arrays = of_read_number(p++, 1);
> + old_ala.array_sz = of_read_number(p++, 1);
> + old_ala.arrays = p;
> +
> + p = (__be32 *) pr->prop->value;
> + if (!p)
> + return -EINVAL;
> + new_ala.n_arrays = of_read_number(p++, 1);
> + new_ala.array_sz = of_read_number(p++, 1);
> + new_ala.arrays = p;
> +
I don't know how often associativity lookup arrays needs to be parsed, but maybe it would be helpful to create a helper function to parse those here.
> + lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
> + new_ala.n_arrays;
> +
> + if (old_ala.array_sz == new_ala.array_sz) {
> +
> + /* Reset any entries where the old and new rows
> + * the array have changed.
> + */
> + for (i = 0; i < lim; i++) {
> + int index = (i * new_ala.array_sz);
> +
> + if (!memcmp(&old_ala.arrays[index],
> + &new_ala.arrays[index],
> + new_ala.array_sz))
> + continue;
> +
> + pseries_update_ala_memory_aai(i);
> + }
> +
> + /* Reset any entries representing the extra rows.
> + * There shouldn't be any, but just in case ...
> + */
> + for (i = lim; i < new_ala.n_arrays; i++)
> + pseries_update_ala_memory_aai(i);
> +
> + } else {
> + /* Update all entries representing these rows;
> + * as all rows have different sizes, none can
> + * have equivalent values.
> + */
> + for (i = 0; i < lim; i++)
> + pseries_update_ala_memory_aai(i);
> + }
> +
> + return 0;
> +}
> +
> static int pseries_memory_notifier(struct notifier_block *nb,
> unsigned long action, void *data)
> {
> @@ -1059,8 +1139,17 @@ static int pseries_memory_notifier(struct notifier_block *nb,
> err = pseries_remove_mem_node(rd->dn);
> break;
> case OF_RECONFIG_UPDATE_PROPERTY:
> - if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
> - err = pseries_update_drconf_memory(rd);
> + if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
> + !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
> + struct drmem_lmb_info *dinfo =
> + drmem_lmbs_init(rd->prop);
> + if (!dinfo)
> + return -EINVAL;
> + err = pseries_update_drconf_memory(dinfo);
> + drmem_lmbs_free(dinfo);
Is this block above related to the other associativity changes? It seems to be an update for dynamic-memory-v2, so should probably be in a separate patch.
Thanks,
Tom
> + } else if (!strcmp(rd->prop->name,
> + "ibm,associativity-lookup-arrays"))
> + err = pseries_update_ala_memory(rd);
> break;
> }
> return notifier_from_errno(err);
^ permalink raw reply
* Re: [RFC v5 3/6] migration/dlpar: Add device readd queuing function
From: Thomas Falcon @ 2018-05-22 20:24 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev
Cc: Nathan Fontenot, John Allen, Tyrel Datwyler
In-Reply-To: <a44f908b-03f9-9042-afaa-2a8bb593db4b@linux.vnet.ibm.com>
On 05/21/2018 12:52 PM, Michael Bringmann wrote:
> migration/dlpar: This patch adds function dlpar_readd_action()
> which will queue a worker function to 'readd' a device in the
> system. Such devices must be identified by a 'resource' type
> and a drc_index to be readded.
The function in the commit message and the patch have different names. The patch seems to queue a generic action instead of a readd. The commit message needs to be updated to describe this new function.
Tom
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/pseries/dlpar.c | 14 ++++++++++++++
> arch/powerpc/platforms/pseries/pseries.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index a0b20c0..a14684e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -407,6 +407,20 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> }
> }
>
> +int dlpar_queue_action(int resource, int action, u32 drc_index)
> +{
> + struct pseries_hp_errorlog hp_elog;
> +
> + hp_elog.resource = resource;
> + hp_elog.action = action;
> + hp_elog.id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
> + hp_elog._drc_u.drc_index = drc_index;
> +
> + queue_hotplug_event(&hp_elog, NULL, NULL);
> +
> + return 0;
> +}
> +
> static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog *hp_elog)
> {
> char *arg;
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index 60db2ee..cb2beb1 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -61,6 +61,7 @@ extern struct device_node *dlpar_configure_connector(__be32,
>
> void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> struct completion *hotplug_done, int *rc);
> +extern int dlpar_queue_action(int resource, int action, u32 drc_index);
> #ifdef CONFIG_MEMORY_HOTPLUG
> int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
> #else
^ permalink raw reply
* Re: [RFC v4 2/4] hotplug/drcinfo: Provide parser with callback
From: Nathan Fontenot @ 2018-05-22 21:02 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <2e2261b4-c0b3-3b6d-d649-541bc8802546@linux.vnet.ibm.com>
On 05/22/2018 11:37 AM, Michael Bringmann wrote:
> This patch provides a common parse function for the ibm,drc-info
> property that can be modified by a callback function. The caller
> provides a pointer to the function and a pointer to their unique
> data, and the parser provides the current lmb set from the struct.
> The callback function may return codes indicating that the parsing
> is complete, or should continue, along with an error code that may
> be returned to the caller.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info firmwar
> e feature" -- end of patch series applied to powerpc next)
> ---
> Changes in V4:
> -- Update code to account for latest kernel checkins.
> -- Rebased to 4.17-rc5 kernel
> -- Some patch cleanup including file combination
> ---
> arch/powerpc/include/asm/prom.h | 7 +++++
> arch/powerpc/platforms/pseries/of_helpers.c | 37 +++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index b04c5ce..2e947b3 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -94,6 +94,13 @@ struct of_drc_info {
> extern int of_read_drc_info_cell(struct property **prop,
> const __be32 **curval, struct of_drc_info *data);
>
> +extern int drc_info_parser(struct device_node *dn,
> + int (*usercb)(struct of_drc_info *drc,
> + void *data,
> + void *optional_data,
The optional_data parameter to the callback routine doesn't seem to be used.
-Nathan
> + int *ret_code),
> + char *opt_drc_type,
> + void *data);
>
> /*
> * There are two methods for telling firmware what our capabilities are.
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 11b2ef1..a588ee6 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -6,6 +6,9 @@
> #include <asm/prom.h>
>
> #include "of_helpers.h"
> +#include "pseries.h"
> +
> +#define MAX_DRC_NAME_LEN 64
>
> /**
> * pseries_of_derive_parent - basically like dirname(1)
> @@ -87,3 +90,37 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
> return 0;
> }
> EXPORT_SYMBOL(of_read_drc_info_cell);
> +
> +int drc_info_parser(struct device_node *dn,
> + int (*usercb)(struct of_drc_info *drc,
> + void *data,
> + void *optional_data,
> + int *ret_code),
> + char *opt_drc_type,
> + void *data)
> +{
> + struct property *info;
> + unsigned int entries;
> + struct of_drc_info drc;
> + const __be32 *value;
> + int j, done = 0, ret_code = -EINVAL;
> +
> + info = of_find_property(dn, "ibm,drc-info", NULL);
> + if (info == NULL)
> + return -EINVAL;
> +
> + value = info->value;
> + entries = of_read_number(value++, 1);
> +
> + for (j = 0, done = 0; (j < entries) && (!done); j++) {
> + of_read_drc_info_cell(&info, &value, &drc);
> +
> + if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
> + continue;
> +
> + done = usercb(&drc, data, NULL, &ret_code);
> + }
> +
> + return ret_code;
> +}
> +EXPORT_SYMBOL(drc_info_parser);
>
^ permalink raw reply
* Re: [RFC v4 1/4] hotplug/drcinfo: Simplify parse ibm, drc-info structs
From: Nathan Fontenot @ 2018-05-22 20:32 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <66fd5dbd-4bca-274a-b0e0-c9d6b52e6567@linux.vnet.ibm.com>
On 05/22/2018 11:37 AM, Michael Bringmann wrote:
> Replace use of of_prop_next_u32() in when parsing 'ibm,drc-info'
> structure to simplify and reduce parsing code.
>
You mention that this patch is to fix the parsing of the drc-info struct, but you end up
making changes to the parsing code in pseries_energy.c and rpaphp_core.c. If there is a
bug in the parsing code in those files that should be submitted as a separate patch
outside of the drc-info fixups.
-Nathan
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info firmware feature" -- end of patch series applied to powerpc next)
> ---
> Changes in V4:
> -- Rebased patch to 4.17-rc5 kernel
> -- Replace of_prop_next_u32() by of_read_number()
> ---
> arch/powerpc/platforms/pseries/of_helpers.c | 20 +++++---------------
> arch/powerpc/platforms/pseries/pseries_energy.c | 10 ++++------
> drivers/pci/hotplug/rpaphp_core.c | 5 ++---
> 3 files changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
> index 6df192f..11b2ef1 100644
> --- a/arch/powerpc/platforms/pseries/of_helpers.c
> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
> @@ -65,29 +65,19 @@ 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 = of_read_number(p2++, 1);
>
> /* Get drc-name-suffix-start:encode-int */
> - p2 = of_prop_next_u32(*prop, p2, &data->drc_name_suffix_start);
> - if (!p2)
> - return -EINVAL;
> + data->drc_name_suffix_start = of_read_number(p2++, 1);
>
> /* Get number-sequential-elements:encode-int */
> - p2 = of_prop_next_u32(*prop, p2, &data->num_sequential_elems);
> - if (!p2)
> - return -EINVAL;
> + data->num_sequential_elems = of_read_number(p2++, 1);
>
> /* Get sequential-increment:encode-int */
> - p2 = of_prop_next_u32(*prop, p2, &data->sequential_inc);
> - if (!p2)
> - return -EINVAL;
> + data->sequential_inc = of_read_number(p2++, 1);
>
> /* Get drc-power-domain:encode-int */
> - p2 = of_prop_next_u32(*prop, p2, &data->drc_power_domain);
> - if (!p2)
> - return -EINVAL;
> + data->drc_power_domain = of_read_number(p2++, 1);
>
> /* Should now know end of current entry */
> (*curval) = (void *)p2;
> diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
> index 6ed2212..5261975 100644
> --- a/arch/powerpc/platforms/pseries/pseries_energy.c
> +++ b/arch/powerpc/platforms/pseries/pseries_energy.c
> @@ -61,9 +61,8 @@ static u32 cpu_to_drc_index(int cpu)
> 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;
> + value = info->value;
> + num_set_entries = of_read_number(value++, 1);
>
> for (j = 0; j < num_set_entries; j++) {
>
> @@ -123,9 +122,8 @@ static int drc_index_to_cpu(u32 drc_index)
> 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;
> + value = info->value;
> + num_set_entries = of_read_number(value++, 1);
>
> for (j = 0; j < num_set_entries; j++) {
>
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index fb5e084..435c1a0 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -236,9 +236,8 @@ static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
> if (info == NULL)
> return -EINVAL;
>
> - value = of_prop_next_u32(info, NULL, &entries);
> - if (!value)
> - return -EINVAL;
> + value = info->value;
> + entries = of_read_number(value++, 1);
>
> for (j = 0; j < entries; j++) {
> of_read_drc_info_cell(&info, &value, &drc);
>
^ permalink raw reply
* Re: [RFC v5 2/6] powerpc/cpu: Conditionally acquire/release DRC index
From: Nathan Fontenot @ 2018-05-22 20:17 UTC (permalink / raw)
To: Michael Bringmann, linuxppc-dev; +Cc: John Allen, Tyrel Datwyler, Thomas Falcon
In-Reply-To: <70c6aae1-6f91-45d5-361b-0d066101d3d0@linux.vnet.ibm.com>
On 05/21/2018 12:52 PM, Michael Bringmann wrote:
> powerpc/cpu: Modify dlpar_cpu_add and dlpar_cpu_remove to allow the
> skipping of DRC index acquire or release operations during the CPU
> add or remove operations. This is intended to support subsequent
> changes to provide a 'CPU readd' operation.
>
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 71 +++++++++++++++-----------
> 1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index a408217..ec78cc6 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -474,7 +474,7 @@ static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
> &cdata);
> }
>
> -static ssize_t dlpar_cpu_add(u32 drc_index)
> +static ssize_t dlpar_cpu_add(u32 drc_index, bool acquire_drc)
> {
> struct device_node *dn, *parent;
> int rc, saved_rc;
> @@ -499,19 +499,22 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
> return -EINVAL;
> }
>
> - rc = dlpar_acquire_drc(drc_index);
> - if (rc) {
> - pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
> - rc, drc_index);
> - of_node_put(parent);
> - return -EINVAL;
> + if (acquire_drc) {
> + rc = dlpar_acquire_drc(drc_index);
> + if (rc) {
> + pr_warn("Failed to acquire DRC, rc: %d, drc index: %x\n",
> + rc, drc_index);
> + of_node_put(parent);
> + return -EINVAL;
> + }
> }
>
> dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
> if (!dn) {
> pr_warn("Failed call to configure-connector, drc index: %x\n",
> drc_index);
> - dlpar_release_drc(drc_index);
> + if (acquire_drc)
> + dlpar_release_drc(drc_index);
> of_node_put(parent);
> return -EINVAL;
> }
> @@ -526,8 +529,9 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
> pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
> dn->name, rc, drc_index);
>
> - rc = dlpar_release_drc(drc_index);
> - if (!rc)
> + if (acquire_drc)
> + rc = dlpar_release_drc(drc_index);
> + if (!rc || acquire_drc)
> dlpar_free_cc_nodes(dn);
>
> return saved_rc;
> @@ -540,7 +544,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
> dn->name, rc, drc_index);
>
> rc = dlpar_detach_node(dn);
> - if (!rc)
> + if (!rc && acquire_drc)
> dlpar_release_drc(drc_index);
>
> return saved_rc;
> @@ -608,7 +612,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
>
> }
>
> -static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
> +static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index,
> + bool release_drc)
> {
> int rc;
>
> @@ -621,12 +626,14 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
> return -EINVAL;
> }
>
> - rc = dlpar_release_drc(drc_index);
> - if (rc) {
> - pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
> - drc_index, dn->name, rc);
> - dlpar_online_cpu(dn);
> - return rc;
> + if (release_drc) {
> + rc = dlpar_release_drc(drc_index);
> + if (rc) {
> + pr_warn("Failed to release drc (%x) for CPU %s, rc: %d\n",
> + drc_index, dn->name, rc);
> + dlpar_online_cpu(dn);
> + return rc;
> + }
> }
>
> rc = dlpar_detach_node(dn);
> @@ -635,7 +642,10 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 drc_index)
>
> pr_warn("Failed to detach CPU %s, rc: %d", dn->name, rc);
>
> - rc = dlpar_acquire_drc(drc_index);
> + if (release_drc)
> + rc = dlpar_acquire_drc(drc_index);
> + else
> + rc = 0;
> if (!rc)
> dlpar_online_cpu(dn);
>
> @@ -664,7 +674,7 @@ static struct device_node *cpu_drc_index_to_dn(u32 drc_index)
> return dn;
> }
>
> -static int dlpar_cpu_remove_by_index(u32 drc_index)
> +static int dlpar_cpu_remove_by_index(u32 drc_index, bool release_drc)
> {
> struct device_node *dn;
> int rc;
> @@ -676,7 +686,7 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
> return -ENODEV;
> }
>
> - rc = dlpar_cpu_remove(dn, drc_index);
> + rc = dlpar_cpu_remove(dn, drc_index, release_drc);
> of_node_put(dn);
> return rc;
> }
> @@ -741,7 +751,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
> }
>
> for (i = 0; i < cpus_to_remove; i++) {
> - rc = dlpar_cpu_remove_by_index(cpu_drcs[i]);
> + rc = dlpar_cpu_remove_by_index(cpu_drcs[i], true);
> if (rc)
> break;
>
> @@ -752,7 +762,7 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
> pr_warn("CPU hot-remove failed, adding back removed CPUs\n");
>
> for (i = 0; i < cpus_removed; i++)
> - dlpar_cpu_add(cpu_drcs[i]);
> + dlpar_cpu_add(cpu_drcs[i], true);
>
> rc = -EINVAL;
> } else {
> @@ -843,7 +853,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
> }
>
> for (i = 0; i < cpus_to_add; i++) {
> - rc = dlpar_cpu_add(cpu_drcs[i]);
> + rc = dlpar_cpu_add(cpu_drcs[i], true);
> if (rc)
> break;
>
> @@ -854,7 +864,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
> pr_warn("CPU hot-add failed, removing any added CPUs\n");
>
> for (i = 0; i < cpus_added; i++)
> - dlpar_cpu_remove_by_index(cpu_drcs[i]);
> + dlpar_cpu_remove_by_index(cpu_drcs[i], true);
>
> rc = -EINVAL;
> } else {
> @@ -880,7 +890,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
> if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
> rc = dlpar_cpu_remove_by_count(count);
> else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
> - rc = dlpar_cpu_remove_by_index(drc_index);
> + rc = dlpar_cpu_remove_by_index(drc_index, true);
> else
> rc = -EINVAL;
> break;
> @@ -888,7 +898,7 @@ int dlpar_cpu(struct pseries_hp_errorlog *hp_elog)
> if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT)
> rc = dlpar_cpu_add_by_count(count);
> else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX)
> - rc = dlpar_cpu_add(drc_index);
> + rc = dlpar_cpu_add(drc_index, true);
> else
> rc = -EINVAL;
> break;
> @@ -913,7 +923,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
> if (rc)
> return -EINVAL;
>
> - rc = dlpar_cpu_add(drc_index);
> + rc = dlpar_cpu_add(drc_index, true);
>
> return rc ? rc : count;
> }
> @@ -934,7 +944,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
> return -EINVAL;
> }
>
> - rc = dlpar_cpu_remove(dn, drc_index);
> + rc = dlpar_cpu_remove(dn, drc_index, true);
> of_node_put(dn);
>
> return rc ? rc : count;
> @@ -948,6 +958,9 @@ static int pseries_smp_notifier(struct notifier_block *nb,
> struct of_reconfig_data *rd = data;
> int err = 0;
>
> + if (strcmp(rd->dn->type, "cpu"))
> + return notifier_from_errno(err);> +
This last change doesn't seem to fit in this patch, should this be a part of a different patch?
-Nathan
> switch (action) {
> case OF_RECONFIG_ATTACH_NODE:
> err = pseries_add_processor(rd->dn);
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 10/10] tools: bpftool: add delimiters to multi-function JITed dumps
From: Jakub Kicinski @ 2018-05-22 19:55 UTC (permalink / raw)
To: Sandipan Das
Cc: ast, daniel, netdev, linuxppc-dev, mpe, naveen.n.rao,
Quentin Monnet
In-Reply-To: <88b61b11ebca5b44bad0c34225b6f2383e5983a5.1527008647.git.sandipan@linux.vnet.ibm.com>
On Tue, 22 May 2018 22:46:13 +0530, Sandipan Das wrote:
> + if (info.nr_jited_func_lens && info.jited_func_lens) {
> + struct kernel_sym *sym = NULL;
> + unsigned char *img = buf;
> + __u64 *ksyms = NULL;
> + __u32 *lens;
> + __u32 i;
> +
> + if (info.nr_jited_ksyms) {
> + kernel_syms_load(&dd);
> + ksyms = (__u64 *) info.jited_ksyms;
> + }
> +
> + lens = (__u32 *) info.jited_func_lens;
> + for (i = 0; i < info.nr_jited_func_lens; i++) {
> + if (ksyms) {
> + sym = kernel_syms_search(&dd, ksyms[i]);
> + if (sym)
> + printf("%s:\n", sym->name);
> + else
> + printf("%016llx:\n", ksyms[i]);
> + }
> +
> + disasm_print_insn(img, lens[i], opcodes, name);
> + img += lens[i];
> + printf("\n");
> + }
> + } else {
The output doesn't seem to be JSON-compatible :( We try to make sure
all bpftool command can produce valid JSON when run with -j (or -p)
switch.
Would it be possible to make each function a separate JSON object with
"name" and "insn" array? Would that work?
^ permalink raw reply
* Re: [PATCH bpf-next v3 07/10] bpf: fix multi-function JITed dump obtained via syscall
From: Jakub Kicinski @ 2018-05-22 19:47 UTC (permalink / raw)
To: Sandipan Das; +Cc: ast, daniel, netdev, linuxppc-dev, mpe, naveen.n.rao
In-Reply-To: <6f245a366d5a2957e2256f4bd89ab56ade6508d5.1527008647.git.sandipan@linux.vnet.ibm.com>
On Tue, 22 May 2018 22:46:10 +0530, Sandipan Das wrote:
> Currently, for multi-function programs, we cannot get the JITed
> instructions using the bpf system call's BPF_OBJ_GET_INFO_BY_FD
> command. Because of this, userspace tools such as bpftool fail
> to identify a multi-function program as being JITed or not.
>
> With the JIT enabled and the test program running, this can be
> verified as follows:
>
> # cat /proc/sys/net/core/bpf_jit_enable
> 1
>
> Before applying this patch:
>
> # bpftool prog list
> 1: kprobe name foo tag b811aab41a39ad3d gpl
> loaded_at 2018-05-16T11:43:38+0530 uid 0
> xlated 216B not jited memlock 65536B
> ...
>
> # bpftool prog dump jited id 1
> no instructions returned
>
> After applying this patch:
>
> # bpftool prog list
> 1: kprobe name foo tag b811aab41a39ad3d gpl
> loaded_at 2018-05-16T12:13:01+0530 uid 0
> xlated 216B jited 308B memlock 65536B
> ...
>
> # bpftool prog dump jited id 1
> 0: nop
> 4: nop
> 8: mflr r0
> c: std r0,16(r1)
> 10: stdu r1,-112(r1)
> 14: std r31,104(r1)
> 18: addi r31,r1,48
> 1c: li r3,10
> ...
>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
> ---
> kernel/bpf/syscall.c | 36 +++++++++++++++++++++++++++++++++---
> 1 file changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index f0ad4b5f0224..1c4cba91e523 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1970,13 +1970,43 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
> * for offload.
> */
> ulen = info.jited_prog_len;
> - info.jited_prog_len = prog->jited_len;
> + if (prog->aux->func_cnt) {
> + u32 i;
> +
> + info.jited_prog_len = 0;
> + for (i = 0; i < prog->aux->func_cnt; i++)
> + info.jited_prog_len += prog->aux->func[i]->jited_len;
> + } else {
> + info.jited_prog_len = prog->jited_len;
> + }
> +
> if (info.jited_prog_len && ulen) {
> if (bpf_dump_raw_ok()) {
> uinsns = u64_to_user_ptr(info.jited_prog_insns);
> ulen = min_t(u32, info.jited_prog_len, ulen);
> - if (copy_to_user(uinsns, prog->bpf_func, ulen))
> - return -EFAULT;
> +
> + /* for multi-function programs, copy the JITed
> + * instructions for all the functions
> + */
> + if (prog->aux->func_cnt) {
> + u32 len, free, i;
> + u8 *img;
> +
> + free = ulen;
> + for (i = 0; i < prog->aux->func_cnt; i++) {
> + len = prog->aux->func[i]->jited_len;
> + img = (u8 *) prog->aux->func[i]->bpf_func;
> + if (len > free)
> + break;
nit: interesting, the previous code used to fill up the space
completely, I would personally vote to keep that behaviour and do:
len = min(len, free);
copy();
free -= len;
if (!free)
break;
otherwise the user space doesn't know when to stop disassembling
truncated output. But that's really a corner case, so not sure we care.
> + if (copy_to_user(uinsns, img, len))
> + return -EFAULT;
> + uinsns += len;
> + free -= len;
> + }
> + } else {
> + if (copy_to_user(uinsns, prog->bpf_func, ulen))
> + return -EFAULT;
> + }
> } else {
> info.jited_prog_insns = 0;
> }
^ permalink raw reply
* Re: [PATCH bpf-next v3 06/10] tools: bpftool: resolve calls without using imm field
From: Jakub Kicinski @ 2018-05-22 19:36 UTC (permalink / raw)
To: Sandipan Das; +Cc: ast, daniel, netdev, linuxppc-dev, mpe, naveen.n.rao
In-Reply-To: <02b16a573269b492d4449c0e587ccc0020973e8c.1527008647.git.sandipan@linux.vnet.ibm.com>
On Tue, 22 May 2018 22:46:09 +0530, Sandipan Das wrote:
> Currently, we resolve the callee's address for a JITed function
> call by using the imm field of the call instruction as an offset
> from __bpf_call_base. If bpf_jit_kallsyms is enabled, we further
> use this address to get the callee's kernel symbol's name.
>
> For some architectures, such as powerpc64, the imm field is not
> large enough to hold this offset. So, instead of assigning this
> offset to the imm field, the verifier now assigns the subprog
> id. Also, a list of kernel symbol addresses for all the JITed
> functions is provided in the program info. We now use the imm
> field as an index for this list to lookup a callee's symbol's
> address and resolve its name.
>
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* [PATCH bpf-next v3 10/10] tools: bpftool: add delimiters to multi-function JITed dumps
From: Sandipan Das @ 2018-05-22 17:16 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, linuxppc-dev, mpe, naveen.n.rao, jakub.kicinski
In-Reply-To: <cover.1527008646.git.sandipan@linux.vnet.ibm.com>
This splits up the contiguous JITed dump obtained via the bpf
system call into more relatable chunks for each function in
the program. If the kernel symbols corresponding to these are
known, they are printed in the header for each JIT image dump
otherwise the masked start address is printed.
Before applying this patch:
# bpftool prog dump jited id 1
0: nop
4: nop
8: mflr r0
c: std r0,16(r1)
10: stdu r1,-112(r1)
14: std r31,104(r1)
...
a8: mr r3,r8
ac: blr
b0: nop
b4: nop
b8: mflr r0
bc: std r0,16(r1)
c0: stdu r1,-112(r1)
c4: std r31,104(r1)
...
138: mr r3,r8
13c: blr
After applying this patch:
# echo 0 > /proc/sys/net/core/bpf_jit_kallsyms
# bpftool prog dump jited id 1
d00000000acc0000:
0: nop
4: nop
8: mflr r0
c: std r0,16(r1)
10: stdu r1,-112(r1)
14: std r31,104(r1)
...
a8: mr r3,r8
ac: blr
d00000000ad20000:
0: nop
4: nop
8: mflr r0
c: std r0,16(r1)
10: stdu r1,-112(r1)
14: std r31,104(r1)
...
88: mr r3,r8
8c: blr
# echo 1 > /proc/sys/net/core/bpf_jit_kallsyms
# bpftool prog dump jited id 1
bpf_prog_8852b2ccb8ec75a7_F:
0: nop
4: nop
8: mflr r0
c: std r0,16(r1)
10: stdu r1,-112(r1)
14: std r31,104(r1)
...
a8: mr r3,r8
ac: blr
bpf_prog_196af774a3477707_F:
0: nop
4: nop
8: mflr r0
c: std r0,16(r1)
10: stdu r1,-112(r1)
14: std r31,104(r1)
...
88: mr r3,r8
8c: blr
Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
tools/bpf/bpftool/prog.c | 51 ++++++++++++++++++++++++++++++++++++++-
tools/bpf/bpftool/xlated_dumper.c | 4 +--
tools/bpf/bpftool/xlated_dumper.h | 1 +
3 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e05ab58d39e2..8ab7a683ac67 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -422,7 +422,9 @@ static int do_dump(int argc, char **argv)
{
unsigned long *func_ksyms = NULL;
struct bpf_prog_info info = {};
+ unsigned int *func_lens = NULL;
unsigned int nr_func_ksyms;
+ unsigned int nr_func_lens;
struct dump_data dd = {};
__u32 len = sizeof(info);
unsigned int buf_size;
@@ -508,12 +510,24 @@ static int do_dump(int argc, char **argv)
}
}
+ nr_func_lens = info.nr_jited_func_lens;
+ if (nr_func_lens) {
+ func_lens = malloc(nr_func_lens * sizeof(__u32));
+ if (!func_lens) {
+ p_err("mem alloc failed");
+ close(fd);
+ goto err_free;
+ }
+ }
+
memset(&info, 0, sizeof(info));
*member_ptr = ptr_to_u64(buf);
*member_len = buf_size;
info.jited_ksyms = ptr_to_u64(func_ksyms);
info.nr_jited_ksyms = nr_func_ksyms;
+ info.jited_func_lens = ptr_to_u64(func_lens);
+ info.nr_jited_func_lens = nr_func_lens;
err = bpf_obj_get_info_by_fd(fd, &info, &len);
close(fd);
@@ -532,6 +546,11 @@ static int do_dump(int argc, char **argv)
goto err_free;
}
+ if (info.nr_jited_func_lens > nr_func_lens) {
+ p_err("too many values returned");
+ goto err_free;
+ }
+
if ((member_len == &info.jited_prog_len &&
info.jited_prog_insns == 0) ||
(member_len == &info.xlated_prog_len &&
@@ -569,7 +588,35 @@ static int do_dump(int argc, char **argv)
goto err_free;
}
- disasm_print_insn(buf, *member_len, opcodes, name);
+ if (info.nr_jited_func_lens && info.jited_func_lens) {
+ struct kernel_sym *sym = NULL;
+ unsigned char *img = buf;
+ __u64 *ksyms = NULL;
+ __u32 *lens;
+ __u32 i;
+
+ if (info.nr_jited_ksyms) {
+ kernel_syms_load(&dd);
+ ksyms = (__u64 *) info.jited_ksyms;
+ }
+
+ lens = (__u32 *) info.jited_func_lens;
+ for (i = 0; i < info.nr_jited_func_lens; i++) {
+ if (ksyms) {
+ sym = kernel_syms_search(&dd, ksyms[i]);
+ if (sym)
+ printf("%s:\n", sym->name);
+ else
+ printf("%016llx:\n", ksyms[i]);
+ }
+
+ disasm_print_insn(img, lens[i], opcodes, name);
+ img += lens[i];
+ printf("\n");
+ }
+ } else {
+ disasm_print_insn(buf, *member_len, opcodes, name);
+ }
} else if (visual) {
if (json_output)
jsonw_null(json_wtr);
@@ -589,11 +636,13 @@ static int do_dump(int argc, char **argv)
free(buf);
free(func_ksyms);
+ free(func_lens);
return 0;
err_free:
free(buf);
free(func_ksyms);
+ free(func_lens);
return -1;
}
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index efdc8fecf2bb..b97f1da60dd1 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -102,8 +102,8 @@ void kernel_syms_destroy(struct dump_data *dd)
free(dd->sym_mapping);
}
-static struct kernel_sym *kernel_syms_search(struct dump_data *dd,
- unsigned long key)
+struct kernel_sym *kernel_syms_search(struct dump_data *dd,
+ unsigned long key)
{
struct kernel_sym sym = {
.address = key,
diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h
index eafbb49c8d0b..33d86e2b369b 100644
--- a/tools/bpf/bpftool/xlated_dumper.h
+++ b/tools/bpf/bpftool/xlated_dumper.h
@@ -56,6 +56,7 @@ struct dump_data {
void kernel_syms_load(struct dump_data *dd);
void kernel_syms_destroy(struct dump_data *dd);
+struct kernel_sym *kernel_syms_search(struct dump_data *dd, unsigned long key);
void dump_xlated_json(struct dump_data *dd, void *buf, unsigned int len,
bool opcodes);
void dump_xlated_plain(struct dump_data *dd, void *buf, unsigned int len,
--
2.14.3
^ permalink raw reply related
* [PATCH bpf-next v3 09/10] tools: bpf: sync bpf uapi header
From: Sandipan Das @ 2018-05-22 17:16 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, linuxppc-dev, mpe, naveen.n.rao, jakub.kicinski
In-Reply-To: <cover.1527008646.git.sandipan@linux.vnet.ibm.com>
Syncing the bpf.h uapi header with tools so that struct
bpf_prog_info has the two new fields for passing on the
JITed image lengths of each function in a multi-function
program.
Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
tools/include/uapi/linux/bpf.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index c44105f27da9..8c3109b5d6d3 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2206,7 +2206,9 @@ struct bpf_prog_info {
__u64 netns_dev;
__u64 netns_ino;
__u32 nr_jited_ksyms;
+ __u32 nr_jited_func_lens;
__aligned_u64 jited_ksyms;
+ __aligned_u64 jited_func_lens;
} __attribute__((aligned(8)));
struct bpf_map_info {
--
2.14.3
^ permalink raw reply related
* [PATCH bpf-next v3 08/10] bpf: get JITed image lengths of functions via syscall
From: Sandipan Das @ 2018-05-22 17:16 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, linuxppc-dev, mpe, naveen.n.rao, jakub.kicinski
In-Reply-To: <cover.1527008646.git.sandipan@linux.vnet.ibm.com>
This adds new two new fields to struct bpf_prog_info. For
multi-function programs, these fields can be used to pass
a list of the JITed image lengths of each function for a
given program to userspace using the bpf system call with
the BPF_OBJ_GET_INFO_BY_FD command.
This can be used by userspace applications like bpftool
to split up the contiguous JITed dump, also obtained via
the system call, into more relatable chunks corresponding
to each function.
Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
include/uapi/linux/bpf.h | 2 ++
kernel/bpf/syscall.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c44105f27da9..8c3109b5d6d3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2206,7 +2206,9 @@ struct bpf_prog_info {
__u64 netns_dev;
__u64 netns_ino;
__u32 nr_jited_ksyms;
+ __u32 nr_jited_func_lens;
__aligned_u64 jited_ksyms;
+ __aligned_u64 jited_func_lens;
} __attribute__((aligned(8)));
struct bpf_map_info {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 1c4cba91e523..faadbcd90191 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2036,6 +2036,26 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
}
}
+ ulen = info.nr_jited_func_lens;
+ info.nr_jited_func_lens = prog->aux->func_cnt;
+ if (info.nr_jited_func_lens && ulen) {
+ if (bpf_dump_raw_ok()) {
+ u32 __user *user_lens;
+ u32 func_len, i;
+
+ /* copy the JITed image lengths for each function */
+ ulen = min_t(u32, info.nr_jited_func_lens, ulen);
+ user_lens = u64_to_user_ptr(info.jited_func_lens);
+ for (i = 0; i < ulen; i++) {
+ func_len = prog->aux->func[i]->jited_len;
+ if (put_user(func_len, &user_lens[i]))
+ return -EFAULT;
+ }
+ } else {
+ info.jited_func_lens = 0;
+ }
+ }
+
done:
if (copy_to_user(uinfo, &info, info_len) ||
put_user(info_len, &uattr->info.info_len))
--
2.14.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox