[RFC v2 0/3] powerpc/hotplug: Fix affinity assoc for LPAR migration

[RFC v2 0/3] powerpc/hotplug: Fix affinity assoc for LPAR migration

[Michael Bringmann]

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):
  hotplug/mobility: Apply assoc lookup updates for Post Migration Topo
  postmigration/memory: Review assoc lookup array changes
  postmigration/memory: Associativity & 'ibm,dynamic-memory-v2'
---
Changes in RFC:
  -- 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.
  -- Make use of drmem accessor functions to read dynamic memory
     properties.
  -- Change section of a support function + variable from __init 
     to normal runtime to make them visible to migration code.

[RFC v2 1/3] hotplug/mobility: Apply assoc updates for Post Migration Topo

[Michael Bringmann]

hotplug/mobility: Recognize more changes to the associativity of
memory blocks described by the 'ibm,dynamic-memory' and 'cpu'
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 of the old/new properties and 'readd'
  any block for which the setting has changed.
* Checking for changes in cpu associativity and making 'readd' calls
  when differences are observed.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in RFC:
  -- 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.
  -- Move check for "cpu" node type from pseries_update_cpu to
     pseries_smp_notifier in 'hotplug-cpu.c'
  -- Remove functions 'pseries_memory_readd_by_index' and
     'pseries_cpu_readd_by_index' as no longer needed outside of
     'mobility.c'.
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c    |   69 +++++++++++++++++++++++
 arch/powerpc/platforms/pseries/hotplug-memory.c |    6 ++
 2 files changed, 75 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index a7d14aa7..91ef22a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -636,6 +636,27 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
 	return rc;
 }
 
+static int dlpar_cpu_readd_by_index(u32 drc_index)
+{
+	int rc = 0;
+
+	pr_info("Attempting to update CPU, drc index %x\n", drc_index);
+
+	if (dlpar_cpu_remove_by_index(drc_index))
+		rc = -EINVAL;
+	else if (dlpar_cpu_add(drc_index))
+		rc = -EINVAL;
+
+	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;
@@ -826,6 +847,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;
@@ -876,12 +900,53 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 
 #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
 
+static int pseries_update_cpu(struct of_reconfig_data *pr)
+{
+	u32 old_entries, new_entries;
+	__be32 *p, *old_assoc, *new_assoc;
+	int rc = 0;
+
+	/* So far, we only handle the 'ibm,associativity' property,
+	 * here.
+	 * The first int of the property is the number of domains
+	 * described.  This is followed by an array of level values.
+	 */
+	p = (__be32 *) pr->old_prop->value;
+	if (!p)
+		return -EINVAL;
+	old_entries = be32_to_cpu(*p++);
+	old_assoc = p;
+
+	p = (__be32 *)pr->prop->value;
+	if (!p)
+		return -EINVAL;
+	new_entries = be32_to_cpu(*p++);
+	new_assoc = p;
+
+	if (old_entries == new_entries) {
+		int sz = old_entries * sizeof(int);
+
+		if (!memcmp(old_assoc, new_assoc, sz))
+			rc = dlpar_cpu_readd_by_index(
+					be32_to_cpu(pr->dn->phandle));
+
+	} else {
+		rc = dlpar_cpu_readd_by_index(
+					be32_to_cpu(pr->dn->phandle));
+	}
+
+	return rc;
+}
+
 static int pseries_smp_notifier(struct notifier_block *nb,
 				unsigned long action, void *data)
 {
 	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);
@@ -889,6 +954,10 @@ 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"))
+			err = pseries_update_cpu(rd);
+		break;
 	}
 	return notifier_from_errno(err);
 }
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c1578f5..2341eae 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1040,6 +1040,12 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
 					  memblock_size);
 			rc = (rc < 0) ? -EINVAL : 0;
 			break;
+		} else if ((be32_to_cpu(old_drmem[i].aa_index) !=
+					be32_to_cpu(new_drmem[i].aa_index)) &&
+				(be32_to_cpu(new_drmem[i].flags) &
+					DRCONF_MEM_ASSIGNED)) {
+			rc = dlpar_memory_readd_by_index(
+				be32_to_cpu(new_drmem[i].drc_index));
 		}
 	}
 	return rc;

Re: [RFC v2 1/3] hotplug/mobility: Apply assoc updates for Post Migration Topo

[Tyrel Datwyler]

On 02/26/2018 12:52 PM, Michael Bringmann wrote:
> hotplug/mobility: Recognize more changes to the associativity of
> memory blocks described by the 'ibm,dynamic-memory' and 'cpu'
> 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 of the old/new properties and 'readd'
>   any block for which the setting has changed.
> * Checking for changes in cpu associativity and making 'readd' calls
>   when differences are observed.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in RFC:
>   -- 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.
>   -- Move check for "cpu" node type from pseries_update_cpu to
>      pseries_smp_notifier in 'hotplug-cpu.c'
>   -- Remove functions 'pseries_memory_readd_by_index' and
>      'pseries_cpu_readd_by_index' as no longer needed outside of
>      'mobility.c'.
> ---
>  arch/powerpc/platforms/pseries/hotplug-cpu.c    |   69 +++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/hotplug-memory.c |    6 ++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index a7d14aa7..91ef22a 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -636,6 +636,27 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>  	return rc;
>  }
>  
> +static int dlpar_cpu_readd_by_index(u32 drc_index)
> +{
> +	int rc = 0;
> +
> +	pr_info("Attempting to update CPU, drc index %x\n", drc_index);
> +
> +	if (dlpar_cpu_remove_by_index(drc_index))
> +		rc = -EINVAL;
> +	else if (dlpar_cpu_add(drc_index))
> +		rc = -EINVAL;

While this if block appears to do the right thing it looks a little icky to me as I find it hard to follow the flow. To me the natural way of thinking about this is if the remove succeeds then add the cpu back. Further, you are masking the return codes from the dlpar code by reporting EINVAL instead of capturing the actual return values. EINVAL implies that their was something wrong with the drc_index supplied. I would do something more like the following which captures the return codes and only relies on a single conditional if statement.

rc = dlpar_cpu_remove_by_index(drc_index);
if (!rc)
	rc = dlpar_cpu_add(drc_index);

-Tyrel

> +
> +	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;
> @@ -826,6 +847,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;
> @@ -876,12 +900,53 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>  
>  #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>  
> +static int pseries_update_cpu(struct of_reconfig_data *pr)
> +{
> +	u32 old_entries, new_entries;
> +	__be32 *p, *old_assoc, *new_assoc;
> +	int rc = 0;
> +
> +	/* So far, we only handle the 'ibm,associativity' property,
> +	 * here.
> +	 * The first int of the property is the number of domains
> +	 * described.  This is followed by an array of level values.
> +	 */
> +	p = (__be32 *) pr->old_prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	old_entries = be32_to_cpu(*p++);
> +	old_assoc = p;
> +
> +	p = (__be32 *)pr->prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	new_entries = be32_to_cpu(*p++);
> +	new_assoc = p;
> +
> +	if (old_entries == new_entries) {
> +		int sz = old_entries * sizeof(int);
> +
> +		if (!memcmp(old_assoc, new_assoc, sz))
> +			rc = dlpar_cpu_readd_by_index(
> +					be32_to_cpu(pr->dn->phandle));
> +
> +	} else {
> +		rc = dlpar_cpu_readd_by_index(
> +					be32_to_cpu(pr->dn->phandle));
> +	}
> +
> +	return rc;
> +}
> +
>  static int pseries_smp_notifier(struct notifier_block *nb,
>  				unsigned long action, void *data)
>  {
>  	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);
> @@ -889,6 +954,10 @@ 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"))
> +			err = pseries_update_cpu(rd);
> +		break;
>  	}
>  	return notifier_from_errno(err);
>  }
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f5..2341eae 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -1040,6 +1040,12 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>  					  memblock_size);
>  			rc = (rc < 0) ? -EINVAL : 0;
>  			break;
> +		} else if ((be32_to_cpu(old_drmem[i].aa_index) !=
> +					be32_to_cpu(new_drmem[i].aa_index)) &&
> +				(be32_to_cpu(new_drmem[i].flags) &
> +					DRCONF_MEM_ASSIGNED)) {
> +			rc = dlpar_memory_readd_by_index(
> +				be32_to_cpu(new_drmem[i].drc_index));
>  		}
>  	}
>  	return rc;
> 

Re: [RFC v2 1/3] hotplug/mobility: Apply assoc updates for Post Migration Topo

[Michael Bringmann]

Accepted Tyrel's change to dlpar_cpu_readd_by_index.  The amendment
will be included in the next version of the RFC.

Michael

On 03/07/2018 01:32 PM, Tyrel Datwyler wrote:
> On 02/26/2018 12:52 PM, Michael Bringmann wrote:
>> hotplug/mobility: Recognize more changes to the associativity of
>> memory blocks described by the 'ibm,dynamic-memory' and 'cpu'
>> 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 of the old/new properties and 'readd'
>>   any block for which the setting has changed.
>> * Checking for changes in cpu associativity and making 'readd' calls
>>   when differences are observed.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in RFC:
>>   -- 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.
>>   -- Move check for "cpu" node type from pseries_update_cpu to
>>      pseries_smp_notifier in 'hotplug-cpu.c'
>>   -- Remove functions 'pseries_memory_readd_by_index' and
>>      'pseries_cpu_readd_by_index' as no longer needed outside of
>>      'mobility.c'.
>> ---
>>  arch/powerpc/platforms/pseries/hotplug-cpu.c    |   69 +++++++++++++++++++++++
>>  arch/powerpc/platforms/pseries/hotplug-memory.c |    6 ++
>>  2 files changed, 75 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> index a7d14aa7..91ef22a 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -636,6 +636,27 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>>  	return rc;
>>  }
>>  
>> +static int dlpar_cpu_readd_by_index(u32 drc_index)
>> +{
>> +	int rc = 0;
>> +
>> +	pr_info("Attempting to update CPU, drc index %x\n", drc_index);
>> +
>> +	if (dlpar_cpu_remove_by_index(drc_index))
>> +		rc = -EINVAL;
>> +	else if (dlpar_cpu_add(drc_index))
>> +		rc = -EINVAL;
> 
> While this if block appears to do the right thing it looks a little icky to me as I find it hard to follow the flow. To me the natural way of thinking about this is if the remove succeeds then add the cpu back. Further, you are masking the return codes from the dlpar code by reporting EINVAL instead of capturing the actual return values. EINVAL implies that their was something wrong with the drc_index supplied. I would do something more like the following which captures the return codes and only relies on a single conditional if statement.
> 
> rc = dlpar_cpu_remove_by_index(drc_index);
> if (!rc)
> 	rc = dlpar_cpu_add(drc_index);
> 
> -Tyrel
> 
>> +
>> +	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;
>> @@ -826,6 +847,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;
>> @@ -876,12 +900,53 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>>  
>>  #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>>  
>> +static int pseries_update_cpu(struct of_reconfig_data *pr)
>> +{
>> +	u32 old_entries, new_entries;
>> +	__be32 *p, *old_assoc, *new_assoc;
>> +	int rc = 0;
>> +
>> +	/* So far, we only handle the 'ibm,associativity' property,
>> +	 * here.
>> +	 * The first int of the property is the number of domains
>> +	 * described.  This is followed by an array of level values.
>> +	 */
>> +	p = (__be32 *) pr->old_prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	old_entries = be32_to_cpu(*p++);
>> +	old_assoc = p;
>> +
>> +	p = (__be32 *)pr->prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	new_entries = be32_to_cpu(*p++);
>> +	new_assoc = p;
>> +
>> +	if (old_entries == new_entries) {
>> +		int sz = old_entries * sizeof(int);
>> +
>> +		if (!memcmp(old_assoc, new_assoc, sz))
>> +			rc = dlpar_cpu_readd_by_index(
>> +					be32_to_cpu(pr->dn->phandle));
>> +
>> +	} else {
>> +		rc = dlpar_cpu_readd_by_index(
>> +					be32_to_cpu(pr->dn->phandle));
>> +	}
>> +
>> +	return rc;
>> +}
>> +
>>  static int pseries_smp_notifier(struct notifier_block *nb,
>>  				unsigned long action, void *data)
>>  {
>>  	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);
>> @@ -889,6 +954,10 @@ 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"))
>> +			err = pseries_update_cpu(rd);
>> +		break;
>>  	}
>>  	return notifier_from_errno(err);
>>  }
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index c1578f5..2341eae 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -1040,6 +1040,12 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>>  					  memblock_size);
>>  			rc = (rc < 0) ? -EINVAL : 0;
>>  			break;
>> +		} else if ((be32_to_cpu(old_drmem[i].aa_index) !=
>> +					be32_to_cpu(new_drmem[i].aa_index)) &&
>> +				(be32_to_cpu(new_drmem[i].flags) &
>> +					DRCONF_MEM_ASSIGNED)) {
>> +			rc = dlpar_memory_readd_by_index(
>> +				be32_to_cpu(new_drmem[i].drc_index));
>>  		}
>>  	}
>>  	return rc;
>>
> 
> 

-- 
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

Re: [RFC v2 1/3] hotplug/mobility: Apply assoc updates for Post Migration Topo

[Nathan Fontenot]

On 02/26/2018 02:52 PM, Michael Bringmann wrote:
> hotplug/mobility: Recognize more changes to the associativity of
> memory blocks described by the 'ibm,dynamic-memory' and 'cpu'
> 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 of the old/new properties and 'readd'
>   any block for which the setting has changed.
> * Checking for changes in cpu associativity and making 'readd' calls
>   when differences are observed.

As part of the post-migration updates do you need to hold a lock
so that we don't attempt to process any of the cpu/memory changes
while the device tree is being updated?

You may be able to grab the device hotplug lock for this.

> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in RFC:
>   -- 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.
>   -- Move check for "cpu" node type from pseries_update_cpu to
>      pseries_smp_notifier in 'hotplug-cpu.c'
>   -- Remove functions 'pseries_memory_readd_by_index' and
>      'pseries_cpu_readd_by_index' as no longer needed outside of
>      'mobility.c'.
> ---
>  arch/powerpc/platforms/pseries/hotplug-cpu.c    |   69 +++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/hotplug-memory.c |    6 ++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index a7d14aa7..91ef22a 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -636,6 +636,27 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>  	return rc;
>  }
> 
> +static int dlpar_cpu_readd_by_index(u32 drc_index)
> +{
> +	int rc = 0;
> +
> +	pr_info("Attempting to update CPU, drc index %x\n", drc_index);

Should make this say we are re-adding the CPU, it's a bit more specific as
to what is really happening.

> +
> +	if (dlpar_cpu_remove_by_index(drc_index))
> +		rc = -EINVAL;
> +	else if (dlpar_cpu_add(drc_index))
> +		rc = -EINVAL;
> +
> +	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;
> @@ -826,6 +847,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;
> @@ -876,12 +900,53 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
> 
>  #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
> 
> +static int pseries_update_cpu(struct of_reconfig_data *pr)
> +{
> +	u32 old_entries, new_entries;
> +	__be32 *p, *old_assoc, *new_assoc;
> +	int rc = 0;
> +
> +	/* So far, we only handle the 'ibm,associativity' property,
> +	 * here.
> +	 * The first int of the property is the number of domains
> +	 * described.  This is followed by an array of level values.
> +	 */
> +	p = (__be32 *) pr->old_prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	old_entries = be32_to_cpu(*p++);
> +	old_assoc = p;
> +
> +	p = (__be32 *)pr->prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	new_entries = be32_to_cpu(*p++);
> +	new_assoc = p;
> +
> +	if (old_entries == new_entries) {
> +		int sz = old_entries * sizeof(int);
> +
> +		if (!memcmp(old_assoc, new_assoc, sz))
> +			rc = dlpar_cpu_readd_by_index(
> +					be32_to_cpu(pr->dn->phandle));
> +
> +	} else {
> +		rc = dlpar_cpu_readd_by_index(
> +					be32_to_cpu(pr->dn->phandle));
> +	}
> +
> +	return rc;
> +}

Do we need to do the full compare of the new vs. the old affinity property?

I would think we would only get an updated property if the property changes.
We don't care what changes in the property at this point, only that it changed.
You could just call dlpar_cpu_readd_by_index() directly.

-Nathan

> +
>  static int pseries_smp_notifier(struct notifier_block *nb,
>  				unsigned long action, void *data)
>  {
>  	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);
> @@ -889,6 +954,10 @@ 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"))
> +			err = pseries_update_cpu(rd);
> +		break;
>  	}
>  	return notifier_from_errno(err);
>  }
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f5..2341eae 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -1040,6 +1040,12 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>  					  memblock_size);
>  			rc = (rc < 0) ? -EINVAL : 0;
>  			break;
> +		} else if ((be32_to_cpu(old_drmem[i].aa_index) !=
> +					be32_to_cpu(new_drmem[i].aa_index)) &&
> +				(be32_to_cpu(new_drmem[i].flags) &
> +					DRCONF_MEM_ASSIGNED)) {
> +			rc = dlpar_memory_readd_by_index(
> +				be32_to_cpu(new_drmem[i].drc_index))>  		}
>  	}
>  	return rc;
> 

Re: [RFC v2 1/3] hotplug/mobility: Apply assoc updates for Post Migration Topo

[Michael Bringmann]

See comments below:

On 04/24/2018 11:56 AM, Nathan Fontenot wrote:
> On 02/26/2018 02:52 PM, Michael Bringmann wrote:
>> hotplug/mobility: Recognize more changes to the associativity of
>> memory blocks described by the 'ibm,dynamic-memory' and 'cpu'
>> 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 of the old/new properties and 'readd'
>>   any block for which the setting has changed.
>> * Checking for changes in cpu associativity and making 'readd' calls
>>   when differences are observed.
> 
> As part of the post-migration updates do you need to hold a lock
> so that we don't attempt to process any of the cpu/memory changes
> while the device tree is being updated?
> 
> You may be able to grab the device hotplug lock for this.

The CPU Re-add process reuses the dlpar_cpu_remove / dlpar_cpu_add
code for POWERPC.  These functions end up invoking device_online() /
device_offline() which in turn end up invoking the 'cpus_write_lock/unlock'
around every kernel change to the CPU structures.  It was modeled
on the Memory Re-add process as we discussed a while back, which
also uses device_online and a corresponding write lock for each
LMB processed.

Do you see a need for a coarser granularity of locking around
all or a group of the cpu/memory changes?  The data structures
that the kernel outside of powerpc uses for CPUs and LMBs seem
to be quite well isolated from the device-tree properties.

> 
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in RFC:
>>   -- 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.
>>   -- Move check for "cpu" node type from pseries_update_cpu to
>>      pseries_smp_notifier in 'hotplug-cpu.c'
>>   -- Remove functions 'pseries_memory_readd_by_index' and
>>      'pseries_cpu_readd_by_index' as no longer needed outside of
>>      'mobility.c'.
>> ---
>>  arch/powerpc/platforms/pseries/hotplug-cpu.c    |   69 +++++++++++++++++++++++
>>  arch/powerpc/platforms/pseries/hotplug-memory.c |    6 ++
>>  2 files changed, 75 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> index a7d14aa7..91ef22a 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -636,6 +636,27 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>>  	return rc;
>>  }
>>
>> +static int dlpar_cpu_readd_by_index(u32 drc_index)
>> +{
>> +	int rc = 0;
>> +
>> +	pr_info("Attempting to update CPU, drc index %x\n", drc_index);
> 
> Should make this say we are re-adding the CPU, it's a bit more specific as
> to what is really happening.

Okay.  I will update the notice from dlpar_memory_readd_by_index, as well.
> 
>> +
>> +	if (dlpar_cpu_remove_by_index(drc_index))
>> +		rc = -EINVAL;
>> +	else if (dlpar_cpu_add(drc_index))
>> +		rc = -EINVAL;
>> +
>> +	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;
>> @@ -826,6 +847,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;
>> @@ -876,12 +900,53 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>>
>>  #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>>
>> +static int pseries_update_cpu(struct of_reconfig_data *pr)
>> +{
>> +	u32 old_entries, new_entries;
>> +	__be32 *p, *old_assoc, *new_assoc;
>> +	int rc = 0;
>> +
>> +	/* So far, we only handle the 'ibm,associativity' property,
>> +	 * here.
>> +	 * The first int of the property is the number of domains
>> +	 * described.  This is followed by an array of level values.
>> +	 */
>> +	p = (__be32 *) pr->old_prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	old_entries = be32_to_cpu(*p++);
>> +	old_assoc = p;
>> +
>> +	p = (__be32 *)pr->prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	new_entries = be32_to_cpu(*p++);
>> +	new_assoc = p;
>> +
>> +	if (old_entries == new_entries) {
>> +		int sz = old_entries * sizeof(int);
>> +
>> +		if (!memcmp(old_assoc, new_assoc, sz))
>> +			rc = dlpar_cpu_readd_by_index(
>> +					be32_to_cpu(pr->dn->phandle));
>> +
>> +	} else {
>> +		rc = dlpar_cpu_readd_by_index(
>> +					be32_to_cpu(pr->dn->phandle));
>> +	}
>> +
>> +	return rc;
>> +}
> 
> Do we need to do the full compare of the new vs. the old affinity property?
> 
> I would think we would only get an updated property if the property changes.
> We don't care what changes in the property at this point, only that it changed.
> You could just call dlpar_cpu_readd_by_index() directly.

Okay.

> 
> -Nathan

Thanks.
Michael

> 
>> +
>>  static int pseries_smp_notifier(struct notifier_block *nb,
>>  				unsigned long action, void *data)
>>  {
>>  	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);
>> @@ -889,6 +954,10 @@ 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"))
>> +			err = pseries_update_cpu(rd);
>> +		break;
>>  	}
>>  	return notifier_from_errno(err);
>>  }
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index c1578f5..2341eae 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -1040,6 +1040,12 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>>  					  memblock_size);
>>  			rc = (rc < 0) ? -EINVAL : 0;
>>  			break;
>> +		} else if ((be32_to_cpu(old_drmem[i].aa_index) !=
>> +					be32_to_cpu(new_drmem[i].aa_index)) &&
>> +				(be32_to_cpu(new_drmem[i].flags) &
>> +					DRCONF_MEM_ASSIGNED)) {
>> +			rc = dlpar_memory_readd_by_index(
>> +				be32_to_cpu(new_drmem[i].drc_index))>  		}
>>  	}
>>  	return rc;
>>
> 

-- 
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

Re: [RFC v2 1/3] hotplug/mobility: Apply assoc updates for Post Migration Topo

[Nathan Fontenot]

On 04/24/2018 04:33 PM, Michael Bringmann wrote:
> See comments below:
> 
> On 04/24/2018 11:56 AM, Nathan Fontenot wrote:
>> On 02/26/2018 02:52 PM, Michael Bringmann wrote:
>>> hotplug/mobility: Recognize more changes to the associativity of
>>> memory blocks described by the 'ibm,dynamic-memory' and 'cpu'
>>> 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 of the old/new properties and 'readd'
>>>   any block for which the setting has changed.
>>> * Checking for changes in cpu associativity and making 'readd' calls
>>>   when differences are observed.
>>
>> As part of the post-migration updates do you need to hold a lock
>> so that we don't attempt to process any of the cpu/memory changes
>> while the device tree is being updated?
>>
>> You may be able to grab the device hotplug lock for this.
> 
> The CPU Re-add process reuses the dlpar_cpu_remove / dlpar_cpu_add
> code for POWERPC.  These functions end up invoking device_online() /
> device_offline() which in turn end up invoking the 'cpus_write_lock/unlock'
> around every kernel change to the CPU structures.  It was modeled
> on the Memory Re-add process as we discussed a while back, which
> also uses device_online and a corresponding write lock for each
> LMB processed.
> 
> Do you see a need for a coarser granularity of locking around
> all or a group of the cpu/memory changes?  The data structures
> that the kernel outside of powerpc uses for CPUs and LMBs seem
> to be quite well isolated from the device-tree properties.

My thinking was for memory and CPU updates, the idea being that all
updates are queued up until after the post-LPM device tree updates happens.
Grabbing the device_hotplug lock while updating the device tree would
prevent any of the queued CPU/memory updates from happening.

> 
>>
>>>
>>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>>> ---
>>> Changes in RFC:
>>>   -- 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.
>>>   -- Move check for "cpu" node type from pseries_update_cpu to
>>>      pseries_smp_notifier in 'hotplug-cpu.c'
>>>   -- Remove functions 'pseries_memory_readd_by_index' and
>>>      'pseries_cpu_readd_by_index' as no longer needed outside of
>>>      'mobility.c'.
>>> ---
>>>  arch/powerpc/platforms/pseries/hotplug-cpu.c    |   69 +++++++++++++++++++++++
>>>  arch/powerpc/platforms/pseries/hotplug-memory.c |    6 ++
>>>  2 files changed, 75 insertions(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>>> index a7d14aa7..91ef22a 100644
>>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>>> @@ -636,6 +636,27 @@ static int dlpar_cpu_remove_by_index(u32 drc_index)
>>>  	return rc;
>>>  }
>>>
>>> +static int dlpar_cpu_readd_by_index(u32 drc_index)
>>> +{
>>> +	int rc = 0;
>>> +
>>> +	pr_info("Attempting to update CPU, drc index %x\n", drc_index);
>>
>> Should make this say we are re-adding the CPU, it's a bit more specific as
>> to what is really happening.
> 
> Okay.  I will update the notice from dlpar_memory_readd_by_index, as well.

Looks like your current message mirrors what the memory readd routine has,
let's just keep the message as is.

-Nathan

>>
>>> +
>>> +	if (dlpar_cpu_remove_by_index(drc_index))
>>> +		rc = -EINVAL;
>>> +	else if (dlpar_cpu_add(drc_index))
>>> +		rc = -EINVAL;
>>> +
>>> +	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;
>>> @@ -826,6 +847,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;
>>> @@ -876,12 +900,53 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>>>
>>>  #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
>>>
>>> +static int pseries_update_cpu(struct of_reconfig_data *pr)
>>> +{
>>> +	u32 old_entries, new_entries;
>>> +	__be32 *p, *old_assoc, *new_assoc;
>>> +	int rc = 0;
>>> +
>>> +	/* So far, we only handle the 'ibm,associativity' property,
>>> +	 * here.
>>> +	 * The first int of the property is the number of domains
>>> +	 * described.  This is followed by an array of level values.
>>> +	 */
>>> +	p = (__be32 *) pr->old_prop->value;
>>> +	if (!p)
>>> +		return -EINVAL;
>>> +	old_entries = be32_to_cpu(*p++);
>>> +	old_assoc = p;
>>> +
>>> +	p = (__be32 *)pr->prop->value;
>>> +	if (!p)
>>> +		return -EINVAL;
>>> +	new_entries = be32_to_cpu(*p++);
>>> +	new_assoc = p;
>>> +
>>> +	if (old_entries == new_entries) {
>>> +		int sz = old_entries * sizeof(int);
>>> +
>>> +		if (!memcmp(old_assoc, new_assoc, sz))
>>> +			rc = dlpar_cpu_readd_by_index(
>>> +					be32_to_cpu(pr->dn->phandle));
>>> +
>>> +	} else {
>>> +		rc = dlpar_cpu_readd_by_index(
>>> +					be32_to_cpu(pr->dn->phandle));
>>> +	}
>>> +
>>> +	return rc;
>>> +}
>>
>> Do we need to do the full compare of the new vs. the old affinity property?
>>
>> I would think we would only get an updated property if the property changes.
>> We don't care what changes in the property at this point, only that it changed.
>> You could just call dlpar_cpu_readd_by_index() directly.
> 
> Okay.
> 
>>
>> -Nathan
> 
> Thanks.
> Michael
> 
>>
>>> +
>>>  static int pseries_smp_notifier(struct notifier_block *nb,
>>>  				unsigned long action, void *data)
>>>  {
>>>  	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);
>>> @@ -889,6 +954,10 @@ 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"))
>>> +			err = pseries_update_cpu(rd);
>>> +		break;
>>>  	}
>>>  	return notifier_from_errno(err);
>>>  }
>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> index c1578f5..2341eae 100644
>>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> @@ -1040,6 +1040,12 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>>>  					  memblock_size);
>>>  			rc = (rc < 0) ? -EINVAL : 0;
>>>  			break;
>>> +		} else if ((be32_to_cpu(old_drmem[i].aa_index) !=
>>> +					be32_to_cpu(new_drmem[i].aa_index)) &&
>>> +				(be32_to_cpu(new_drmem[i].flags) &
>>> +					DRCONF_MEM_ASSIGNED)) {
>>> +			rc = dlpar_memory_readd_by_index(
>>> +				be32_to_cpu(new_drmem[i].drc_index))>  		}
>>>  	}
>>>  	return rc;
>>>
>>
> 

[RFC v2 2/3] postmigration/memory: Review assoc lookup array changes

[Michael Bringmann]

postmigration/memory: In an LPAR migration scenario, the property
"ibm,associativity-lookup-arrays" 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 appropriate entry of the property
'ibm,dynamic-memory' would be updated as well as any other applicable
system data structures.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in RFC v2:
  -- Simplify code to update memory nodes during mobility checks.
     Remove functions to generate extra HP_ELOG messages in favor
     of direct function calls to dlpar_memory_readd_by_index.
---
 arch/powerpc/platforms/pseries/hotplug-memory.c |  120 +++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 2341eae..b63181d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1051,6 +1051,123 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
 	return rc;
 }
 
+struct assoc_arrays {
+	u32 n_arrays;
+	u32 array_sz;
+	const __be32 *arrays;
+};
+
+static int pseries_update_ala_memory_aai(int aa_index,
+					struct property *dmprop)
+{
+	struct of_drconf_cell *drmem;
+	u32 entries;
+	__be32 *p;
+	int i;
+	int rc = 0;
+
+	p = (__be32 *) dmprop->value;
+	if (!p)
+		return -EINVAL;
+
+	/* 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++);
+	drmem = (struct of_drconf_cell *)p;
+
+	for (i = 0; i < entries; i++) {
+		if ((be32_to_cpu(drmem[i].aa_index) != aa_index) &&
+			(be32_to_cpu(drmem[i].flags) & DRCONF_MEM_ASSIGNED)) {
+			rc = dlpar_memory_readd_by_index(
+				be32_to_cpu(drmem[i].drc_index));
+		}
+	}
+
+	return rc;
+}
+
+static int pseries_update_ala_memory(struct of_reconfig_data *pr)
+{
+	struct assoc_arrays new_ala, old_ala;
+	struct device_node *dn;
+	struct property *dmprop;
+	__be32 *p;
+	int i, lim;
+
+	if (rtas_hp_event)
+		return 0;
+
+	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
+	if (!dn)
+		return -ENODEV;
+
+	dmprop = of_find_property(dn, "ibm,dynamic-memory", NULL);
+	if (!dmprop) {
+		of_node_put(dn);
+		return -ENODEV;
+	}
+
+	/*
+	 * 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) {
+		of_node_put(dn);
+		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) {
+		of_node_put(dn);
+		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) {
+
+		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, dmprop);
+		}
+
+		for (i = lim; i < new_ala.n_arrays; i++)
+			pseries_update_ala_memory_aai(i, dmprop);
+
+	} 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, dmprop);
+	}
+
+	of_node_put(dn);
+	return 0;
+}
+
 static int pseries_memory_notifier(struct notifier_block *nb,
 				   unsigned long action, void *data)
 {
@@ -1067,6 +1184,9 @@ static int pseries_memory_notifier(struct notifier_block *nb,
 	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,associativity-lookup-arrays"))
+			err = pseries_update_ala_memory(rd);
 		break;
 	}
 	return notifier_from_errno(err);

Re: [RFC v2 2/3] postmigration/memory: Review assoc lookup array changes

[Nathan Fontenot]

On 02/26/2018 02:53 PM, Michael Bringmann wrote:
> postmigration/memory: In an LPAR migration scenario, the property
> "ibm,associativity-lookup-arrays" 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 appropriate entry of the property
> 'ibm,dynamic-memory' would be updated as well as any other applicable
> system data structures.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in RFC v2:
>   -- Simplify code to update memory nodes during mobility checks.
>      Remove functions to generate extra HP_ELOG messages in favor
>      of direct function calls to dlpar_memory_readd_by_index.
> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c |  120 +++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 2341eae..b63181d 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -1051,6 +1051,123 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>  	return rc;
>  }
> 
> +struct assoc_arrays {
> +	u32 n_arrays;
> +	u32 array_sz;
> +	const __be32 *arrays;
> +};
> +
> +static int pseries_update_ala_memory_aai(int aa_index,
> +					struct property *dmprop)
> +{
> +	struct of_drconf_cell *drmem;
> +	u32 entries;
> +	__be32 *p;
> +	int i;
> +	int rc = 0;
> +
> +	p = (__be32 *) dmprop->value;
> +	if (!p)
> +		return -EINVAL;
> +
> +	/* 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++);
> +	drmem = (struct of_drconf_cell *)p;
> +
> +	for (i = 0; i < entries; i++) {
> +		if ((be32_to_cpu(drmem[i].aa_index) != aa_index) &&
> +			(be32_to_cpu(drmem[i].flags) & DRCONF_MEM_ASSIGNED)) {
> +			rc = dlpar_memory_readd_by_index(
> +				be32_to_cpu(drmem[i].drc_index));
> +		}
> +	}
> +
> +	return rc;
> +}
> +
> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
> +{
> +	struct assoc_arrays new_ala, old_ala;
> +	struct device_node *dn;
> +	struct property *dmprop;
> +	__be32 *p;
> +	int i, lim;
> +
> +	if (rtas_hp_event)
> +		return 0;
> +
> +	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> +	if (!dn)
> +		return -ENODEV;
> +
> +	dmprop = of_find_property(dn, "ibm,dynamic-memory", NULL);
> +	if (!dmprop) {
> +		of_node_put(dn);
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * 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) {
> +		of_node_put(dn);
> +		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) {
> +		of_node_put(dn);
> +		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) {
> +
> +		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, dmprop);
> +		}
> +
> +		for (i = lim; i < new_ala.n_arrays; i++)
> +			pseries_update_ala_memory_aai(i, dmprop);
> +
> +	} 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, dmprop);
> +	}
> +
> +	of_node_put(dn);
> +	return 0;
> +}

The two routines above should be updated to use the in-kernel drmem array instead
of looking up the dynamic-memory property in the device tree.

-Nathan

> +
>  static int pseries_memory_notifier(struct notifier_block *nb,
>  				   unsigned long action, void *data)
>  {
> @@ -1067,6 +1184,9 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>  	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,associativity-lookup-arrays"))
> +			err = pseries_update_ala_memory(rd);
>  		break;
>  	}
>  	return notifier_from_errno(err);
> 

Re: [RFC v2 2/3] postmigration/memory: Review assoc lookup array changes

[Michael Bringmann]

On 04/24/2018 12:01 PM, Nathan Fontenot wrote:
>> +};
>> +
>> +static int pseries_update_ala_memory_aai(int aa_index,
>> +					struct property *dmprop)

>> +
>> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)


> The two routines above should be updated to use the in-kernel drmem array instead
> of looking up the dynamic-memory property in the device tree.

Okay.

> 
> -Nathan

Thanks.
Michael

> 
>> +
>>  static int pseries_memory_notifier(struct notifier_block *nb,
>>  				   unsigned long action, void *data)
>>  {
>> @@ -1067,6 +1184,9 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>>  	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,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

[RFC v2 3/3] postmigration/memory: Associativity & ibm,dynamic-memory-v2

[Michael Bringmann]

postmigration/memory: Now apply changes to the associativity of memory
blocks described by the 'ibm,dynamic-memory-v2' property regarding
the topology of LPARS in Post Migration events.

* Extend the previous work done for the 'ibm,associativity-lookup-array'
  to apply to either property 'ibm,dynamic-memory' or
  'ibm,dynamic-memory-v2', whichever is present.
* Add new code to parse the 'ibm,dynamic-memory-v2' property looking
  for differences in block 'assignment', associativity indexes per
  block, and any other difference currently known.
* Rewrite some of the original code to parse the 'ibm,dynamic-memory'
  property to take advantage of LMB parsing code.

When block differences are recognized, the memory block may be removed,
added, or updated depending upon the state of the new device tree
property and differences from the migrated value of the property.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in RFC v2:
  -- Reuse existing parser code from 'drmem.c' in parsing property
     'imb,dynamic-memory-v2' for migration.
  -- Fix crash during migration that occurs on non-VPHN systems
     when attempting to reset topology timer.
  -- Change section of a support function + variable from __init 
     to normal runtime to make them visible to migration code.
---
 arch/powerpc/include/asm/drmem.h                |    8 +
 arch/powerpc/mm/drmem.c                         |   23 ++-
 arch/powerpc/mm/numa.c                          |    3 
 arch/powerpc/platforms/pseries/hotplug-memory.c |  175 +++++++++++++++++++----
 drivers/of/fdt.c                                |    4 -
 include/linux/of_fdt.h                          |    2 
 6 files changed, 170 insertions(+), 45 deletions(-)

diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 47a7012..e4773c9 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -92,6 +92,14 @@ void __init walk_drmem_lmbs(struct device_node *dn,
 			void (*func)(struct drmem_lmb *, const __be32 **));
 int drmem_update_dt(void);
 
+void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
+			void (*func)(struct drmem_lmb *, const __be32 **));
+
+void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+			const __be32 **prop);
+void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
+			void (*func)(struct drmem_lmb *, const __be32 **));
+
 #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 31dbe14..e47a6e0 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -192,7 +192,7 @@ 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;
@@ -208,7 +208,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,
+void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
 			void (*func)(struct drmem_lmb *, const __be32 **))
 {
 	struct drmem_lmb lmb;
@@ -218,11 +218,12 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
 
 	for (i = 0; i < n_lmbs; i++) {
 		read_drconf_v1_cell(&lmb, &prop);
-		func(&lmb, &usm);
+		func(&lmb, &data);
 	}
 }
+EXPORT_SYMBOL(walk_drmem_v1_lmbs);
 
-static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
 				       const __be32 **prop)
 {
 	const __be32 *p = *prop;
@@ -235,8 +236,9 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
 
 	*prop = p;
 }
+EXPORT_SYMBOL(read_drconf_v2_cell);
 
-static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
+void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
 			void (*func)(struct drmem_lmb *, const __be32 **))
 {
 	struct of_drconf_cell_v2 dr_cell;
@@ -258,10 +260,11 @@ static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
 			lmb.aa_index = dr_cell.aa_index;
 			lmb.flags = dr_cell.flags;
 
-			func(&lmb, &usm);
+			func(&lmb, &data);
 		}
 	}
 }
+EXPORT_SYMBOL(walk_drmem_v2_lmbs);
 
 #ifdef CONFIG_PPC_PSERIES
 void __init walk_drmem_lmbs_early(unsigned long node,
@@ -280,12 +283,12 @@ void __init walk_drmem_lmbs_early(unsigned long node,
 
 	prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory", &len);
 	if (prop) {
-		__walk_drmem_v1_lmbs(prop, usm, func);
+		walk_drmem_v1_lmbs(prop, usm, func);
 	} else {
 		prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory-v2",
 					   &len);
 		if (prop)
-			__walk_drmem_v2_lmbs(prop, usm, func);
+			walk_drmem_v2_lmbs(prop, usm, func);
 	}
 
 	memblock_dump_all();
@@ -340,11 +343,11 @@ void __init walk_drmem_lmbs(struct device_node *dn,
 
 	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
 	if (prop) {
-		__walk_drmem_v1_lmbs(prop, usm, func);
+		walk_drmem_v1_lmbs(prop, usm, func);
 	} else {
 		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
 		if (prop)
-			__walk_drmem_v2_lmbs(prop, usm, func);
+			walk_drmem_v2_lmbs(prop, usm, func);
 	}
 }
 
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 0e573f9..2545fea 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1395,7 +1395,8 @@ static void topology_timer_fn(struct timer_list *unused)
 
 static void reset_topology_timer(void)
 {
-	mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
+	if (vphn_enabled)
+		mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
 }
 
 #ifdef CONFIG_SMP
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index b63181d..bf717e2 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1051,49 +1051,155 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
 	return rc;
 }
 
-struct assoc_arrays {
-	u32 n_arrays;
-	u32 array_sz;
-	const __be32 *arrays;
-};
+static inline int pseries_memory_v2_find_drc(u32 drc_index,
+			u64 *base_addr, unsigned long memblock_size,
+			struct of_drconf_cell_v2 *dm)
+{
+	if ((dm->drc_index <= drc_index) &&
+		(drc_index <= (dm->drc_index + dm->seq_lmbs - 1))) {
+		int offset = drc_index - dm->drc_index;
+
+		(*base_addr) = dm->base_addr +
+				(offset * memblock_size);
+	} else if (drc_index > (dm->drc_index +
+				dm->seq_lmbs - 1)) {
+		return -1;
+	} else if (dm->drc_index > drc_index) {
+		return -1;
+	}
+
+	return 0;
+}
 
-static int pseries_update_ala_memory_aai(int aa_index,
-					struct property *dmprop)
+static int pseries_update_drconf_memory_v2(struct of_reconfig_data *pr)
 {
-	struct of_drconf_cell *drmem;
-	u32 entries;
-	__be32 *p;
-	int i;
-	int rc = 0;
+	const __be32 *new_drmem, *old_drmem;
+	unsigned long memblock_size;
+	u32 new_lmb_sets, old_lmb_sets;
+	u64 old_base_addr;
+	int i, rc = 0;
 
-	p = (__be32 *) dmprop->value;
-	if (!p)
+	if (rtas_hp_event)
+		return 0;
+
+	memblock_size = pseries_memory_block_size();
+	if (!memblock_size)
 		return -EINVAL;
 
 	/* 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.
+	 * of of_drconf_cell_v2 entries. Get the number of entries
+	 * and skip to the array of of_drconf_cell_v2's.
 	 */
-	entries = be32_to_cpu(*p++);
-	drmem = (struct of_drconf_cell *)p;
+	old_drmem = (__be32 *) pr->old_prop->value;
+	if (!old_drmem)
+		return -EINVAL;
+	old_lmb_sets = of_read_number(old_drmem++, 1);
 
-	for (i = 0; i < entries; i++) {
-		if ((be32_to_cpu(drmem[i].aa_index) != aa_index) &&
-			(be32_to_cpu(drmem[i].flags) & DRCONF_MEM_ASSIGNED)) {
-			rc = dlpar_memory_readd_by_index(
-				be32_to_cpu(drmem[i].drc_index));
+	new_drmem = (__be32 *)pr->prop->value;
+	new_lmb_sets = of_read_number(new_drmem++, 1);
+
+	for (i = 0; i < old_lmb_sets; i++) {
+		int j;
+		struct of_drconf_cell_v2 old_cell, new_cell;
+
+		read_drconf_v2_cell(&old_cell, &old_drmem);
+		read_drconf_v2_cell(&new_cell, &new_drmem);
+
+		for (j = 0; j < new_cell.seq_lmbs; j++) {
+			if (pseries_memory_v2_find_drc(
+				new_cell.drc_index + j,
+				&old_base_addr,
+				memblock_size,
+				&old_cell))
+				continue;
+
+			if ((old_cell.flags &
+					DRCONF_MEM_ASSIGNED) &&
+			    (!(new_cell.flags &
+					DRCONF_MEM_ASSIGNED))) {
+				rc = pseries_remove_memblock(
+					old_base_addr,
+					memblock_size);
+			} else if ((!(old_cell.flags &
+					DRCONF_MEM_ASSIGNED)) &&
+				   (new_cell.flags &
+					DRCONF_MEM_ASSIGNED)) {
+				rc = memblock_add(
+					old_base_addr, memblock_size);
+			} else if ((old_cell.aa_index !=
+				    new_cell.aa_index) &&
+				   (new_cell.flags &
+					DRCONF_MEM_ASSIGNED)) {
+				dlpar_memory_readd_by_index(
+					new_cell.drc_index + j);
+			}
 		}
 	}
 
-	return rc;
+	return 0;
+}
+
+struct assoc_arrays {
+	u32 n_arrays;
+	u32 array_sz;
+	const __be32 *arrays;
+};
+
+struct update_ala_memory_aai_struct {
+	int aa_index;
+};
+
+static void update_ala_memory_aai_cb(struct drmem_lmb *lmb,
+					const __be32 **data)
+{
+	struct update_ala_memory_aai_struct *updt =
+		(struct update_ala_memory_aai_struct *)*data;
+
+	if ((lmb->aa_index != updt->aa_index) &&
+		(lmb->flags & DRCONF_MEM_ASSIGNED))
+		dlpar_memory_readd_by_index(lmb->drc_index);
+}
+
+static int pseries_update_ala_memory_aai_v1(int aa_index,
+				const __be32 *dmprop)
+{
+	struct update_ala_memory_aai_struct data = {
+		aa_index };
+
+	walk_drmem_v1_lmbs(dmprop, (const __be32 *)&data,
+			update_ala_memory_aai_cb);
+
+	return 0;
+}
+
+static int pseries_update_ala_memory_aai_v2(int aa_index,
+				const __be32 *dmprop)
+{
+	struct update_ala_memory_aai_struct data = {
+		aa_index };
+
+	walk_drmem_v2_lmbs(dmprop, (const __be32 *)&data,
+			update_ala_memory_aai_cb);
+
+	return 0;
+}
+
+static int pseries_update_ala_memory_aai(int v1, int aa_index,
+				const __be32 *dmprop)
+{
+	if (v1)
+		return pseries_update_ala_memory_aai_v1(aa_index, dmprop);
+	else
+		return pseries_update_ala_memory_aai_v2(aa_index, dmprop);
 }
 
 static int pseries_update_ala_memory(struct of_reconfig_data *pr)
 {
 	struct assoc_arrays new_ala, old_ala;
 	struct device_node *dn;
-	struct property *dmprop;
+	const __be32 *dmprop;
+	bool v1 = true;
 	__be32 *p;
 	int i, lim;
 
@@ -1104,10 +1210,15 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
 	if (!dn)
 		return -ENODEV;
 
-	dmprop = of_find_property(dn, "ibm,dynamic-memory", NULL);
+	dmprop = of_get_property(dn, "ibm,dynamic-memory", NULL);
 	if (!dmprop) {
-		of_node_put(dn);
-		return -ENODEV;
+		v1 = false;
+		dmprop = of_get_property(dn, "ibm,dynamic-memory-v2",
+					NULL);
+		if (!dmprop) {
+			of_node_put(dn);
+			return -ENODEV;
+		}
 	}
 
 	/*
@@ -1149,11 +1260,11 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
 						new_ala.array_sz))
 				continue;
 
-			pseries_update_ala_memory_aai(i, dmprop);
+			pseries_update_ala_memory_aai(v1, i, dmprop);
 		}
 
 		for (i = lim; i < new_ala.n_arrays; i++)
-			pseries_update_ala_memory_aai(i, dmprop);
+			pseries_update_ala_memory_aai(v1, i, dmprop);
 
 	} else {
 		/* Update all entries representing these rows;
@@ -1161,7 +1272,7 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
 		 * have equivalent values.
 		 */
 		for (i = 0; i < lim; i++)
-			pseries_update_ala_memory_aai(i, dmprop);
+			pseries_update_ala_memory_aai(v1, i, dmprop);
 	}
 
 	of_node_put(dn);
@@ -1184,6 +1295,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
 	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-v2"))
+			err = pseries_update_drconf_memory_v2(rd);
 		if (!strcmp(rd->prop->name,
 				"ibm,associativity-lookup-arrays"))
 			err = pseries_update_ala_memory(rd);
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 4675e5a..00df576 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -539,7 +539,7 @@ void *of_fdt_unflatten_tree(const unsigned long *blob,
 EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
 
 /* Everything below here references initial_boot_params directly. */
-int __initdata dt_root_addr_cells;
+int dt_root_addr_cells;
 int __initdata dt_root_size_cells;
 
 void *initial_boot_params;
@@ -1013,7 +1013,7 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
 	return 1;
 }
 
-u64 __init dt_mem_next_cell(int s, const __be32 **cellp)
+u64 dt_mem_next_cell(int s, const __be32 **cellp)
 {
 	const __be32 *p = *cellp;
 
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 013c541..14c8681 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -40,7 +40,7 @@ extern void *of_fdt_unflatten_tree(const unsigned long *blob,
 				   struct device_node **mynodes);
 
 /* TBD: Temporary export of fdt globals - remove when code fully merged */
-extern int __initdata dt_root_addr_cells;
+extern int dt_root_addr_cells;
 extern int __initdata dt_root_size_cells;
 extern void *initial_boot_params;
 

Re: [RFC v2 3/3] postmigration/memory: Associativity & ibm,dynamic-memory-v2

[Nathan Fontenot]

On 02/26/2018 02:53 PM, Michael Bringmann wrote:
> postmigration/memory: Now apply changes to the associativity of memory
> blocks described by the 'ibm,dynamic-memory-v2' property regarding
> the topology of LPARS in Post Migration events.
> 
> * Extend the previous work done for the 'ibm,associativity-lookup-array'
>   to apply to either property 'ibm,dynamic-memory' or
>   'ibm,dynamic-memory-v2', whichever is present.
> * Add new code to parse the 'ibm,dynamic-memory-v2' property looking
>   for differences in block 'assignment', associativity indexes per
>   block, and any other difference currently known.
> * Rewrite some of the original code to parse the 'ibm,dynamic-memory'
>   property to take advantage of LMB parsing code.
> 
> When block differences are recognized, the memory block may be removed,
> added, or updated depending upon the state of the new device tree
> property and differences from the migrated value of the property.
> 

The only thing we need to check during LPM is affinity updates, memory
is not added or removed as part of LPM.

I think a slightly different approach to this may be worth considering.
One of the goals of the drmem.c code was to remove the need to parse the
device tree for memory directly. For this update, I think we could modify
the code that builds the drmem_info data so that it can return a drmem_info
struct instead of assuming to set the global one.

This change would allow you to do a straight compare on the global vs. the
new info from the updated device tree property. I think this would be cleaner
and may be able to use the same routine for V1 and V2 properties.

> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in RFC v2:
>   -- Reuse existing parser code from 'drmem.c' in parsing property
>      'imb,dynamic-memory-v2' for migration.
>   -- Fix crash during migration that occurs on non-VPHN systems
>      when attempting to reset topology timer.
>   -- Change section of a support function + variable from __init 
>      to normal runtime to make them visible to migration code.
> ---
>  arch/powerpc/include/asm/drmem.h                |    8 +
>  arch/powerpc/mm/drmem.c                         |   23 ++-
>  arch/powerpc/mm/numa.c                          |    3 
>  arch/powerpc/platforms/pseries/hotplug-memory.c |  175 +++++++++++++++++++----
>  drivers/of/fdt.c                                |    4 -
>  include/linux/of_fdt.h                          |    2 
>  6 files changed, 170 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index 47a7012..e4773c9 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -92,6 +92,14 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
>  int drmem_update_dt(void);
> 
> +void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
> +			void (*func)(struct drmem_lmb *, const __be32 **));
> +
> +void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> +			const __be32 **prop);
> +void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
> +			void (*func)(struct drmem_lmb *, const __be32 **));
> +
>  #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 31dbe14..e47a6e0 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -192,7 +192,7 @@ 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;
> @@ -208,7 +208,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,
> +void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct drmem_lmb lmb;
> @@ -218,11 +218,12 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
> 
>  	for (i = 0; i < n_lmbs; i++) {
>  		read_drconf_v1_cell(&lmb, &prop);
> -		func(&lmb, &usm);
> +		func(&lmb, &data);
>  	}
>  }
> +EXPORT_SYMBOL(walk_drmem_v1_lmbs);
> 
> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> +void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
> @@ -235,8 +236,9 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> 
>  	*prop = p;
>  }
> +EXPORT_SYMBOL(read_drconf_v2_cell);
> 
> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
> +void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -258,10 +260,11 @@ static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>  			lmb.aa_index = dr_cell.aa_index;
>  			lmb.flags = dr_cell.flags;
> 
> -			func(&lmb, &usm);
> +			func(&lmb, &data);
>  		}
>  	}
>  }
> +EXPORT_SYMBOL(walk_drmem_v2_lmbs);
> 
>  #ifdef CONFIG_PPC_PSERIES
>  void __init walk_drmem_lmbs_early(unsigned long node,
> @@ -280,12 +283,12 @@ void __init walk_drmem_lmbs_early(unsigned long node,
> 
>  	prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory", &len);
>  	if (prop) {
> -		__walk_drmem_v1_lmbs(prop, usm, func);
> +		walk_drmem_v1_lmbs(prop, usm, func);
>  	} else {
>  		prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory-v2",
>  					   &len);
>  		if (prop)
> -			__walk_drmem_v2_lmbs(prop, usm, func);
> +			walk_drmem_v2_lmbs(prop, usm, func);
>  	}
> 
>  	memblock_dump_all();
> @@ -340,11 +343,11 @@ void __init walk_drmem_lmbs(struct device_node *dn,
> 
>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>  	if (prop) {
> -		__walk_drmem_v1_lmbs(prop, usm, func);
> +		walk_drmem_v1_lmbs(prop, usm, func);
>  	} else {
>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>  		if (prop)
> -			__walk_drmem_v2_lmbs(prop, usm, func);
> +			walk_drmem_v2_lmbs(prop, usm, func);
>  	}
>  }
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 0e573f9..2545fea 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1395,7 +1395,8 @@ static void topology_timer_fn(struct timer_list *unused)
> 
>  static void reset_topology_timer(void)
>  {
> -	mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
> +	if (vphn_enabled)
> +		mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);

This really looks like a bug fix that should be in a different patch.

-Nathan

>  }
> 
>  #ifdef CONFIG_SMP
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index b63181d..bf717e2 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -1051,49 +1051,155 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>  	return rc;
>  }
> 
> -struct assoc_arrays {
> -	u32 n_arrays;
> -	u32 array_sz;
> -	const __be32 *arrays;
> -};
> +static inline int pseries_memory_v2_find_drc(u32 drc_index,
> +			u64 *base_addr, unsigned long memblock_size,
> +			struct of_drconf_cell_v2 *dm)
> +{
> +	if ((dm->drc_index <= drc_index) &&
> +		(drc_index <= (dm->drc_index + dm->seq_lmbs - 1))) {
> +		int offset = drc_index - dm->drc_index;
> +
> +		(*base_addr) = dm->base_addr +
> +				(offset * memblock_size);
> +	} else if (drc_index > (dm->drc_index +
> +				dm->seq_lmbs - 1)) {
> +		return -1;
> +	} else if (dm->drc_index > drc_index) {
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> 
> -static int pseries_update_ala_memory_aai(int aa_index,
> -					struct property *dmprop)
> +static int pseries_update_drconf_memory_v2(struct of_reconfig_data *pr)
>  {
> -	struct of_drconf_cell *drmem;
> -	u32 entries;
> -	__be32 *p;
> -	int i;
> -	int rc = 0;
> +	const __be32 *new_drmem, *old_drmem;
> +	unsigned long memblock_size;
> +	u32 new_lmb_sets, old_lmb_sets;
> +	u64 old_base_addr;
> +	int i, rc = 0;
> 
> -	p = (__be32 *) dmprop->value;
> -	if (!p)
> +	if (rtas_hp_event)
> +		return 0;
> +
> +	memblock_size = pseries_memory_block_size();
> +	if (!memblock_size)
>  		return -EINVAL;
> 
>  	/* 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.
> +	 * of of_drconf_cell_v2 entries. Get the number of entries
> +	 * and skip to the array of of_drconf_cell_v2's.
>  	 */
> -	entries = be32_to_cpu(*p++);
> -	drmem = (struct of_drconf_cell *)p;
> +	old_drmem = (__be32 *) pr->old_prop->value;
> +	if (!old_drmem)
> +		return -EINVAL;
> +	old_lmb_sets = of_read_number(old_drmem++, 1);
> 
> -	for (i = 0; i < entries; i++) {
> -		if ((be32_to_cpu(drmem[i].aa_index) != aa_index) &&
> -			(be32_to_cpu(drmem[i].flags) & DRCONF_MEM_ASSIGNED)) {
> -			rc = dlpar_memory_readd_by_index(
> -				be32_to_cpu(drmem[i].drc_index));
> +	new_drmem = (__be32 *)pr->prop->value;
> +	new_lmb_sets = of_read_number(new_drmem++, 1);
> +
> +	for (i = 0; i < old_lmb_sets; i++) {
> +		int j;
> +		struct of_drconf_cell_v2 old_cell, new_cell;
> +
> +		read_drconf_v2_cell(&old_cell, &old_drmem);
> +		read_drconf_v2_cell(&new_cell, &new_drmem);
> +
> +		for (j = 0; j < new_cell.seq_lmbs; j++) {
> +			if (pseries_memory_v2_find_drc(
> +				new_cell.drc_index + j,
> +				&old_base_addr,
> +				memblock_size,
> +				&old_cell))
> +				continue;
> +
> +			if ((old_cell.flags &
> +					DRCONF_MEM_ASSIGNED) &&
> +			    (!(new_cell.flags &
> +					DRCONF_MEM_ASSIGNED))) {
> +				rc = pseries_remove_memblock(
> +					old_base_addr,
> +					memblock_size);
> +			} else if ((!(old_cell.flags &
> +					DRCONF_MEM_ASSIGNED)) &&
> +				   (new_cell.flags &
> +					DRCONF_MEM_ASSIGNED)) {
> +				rc = memblock_add(
> +					old_base_addr, memblock_size);
> +			} else if ((old_cell.aa_index !=
> +				    new_cell.aa_index) &&
> +				   (new_cell.flags &
> +					DRCONF_MEM_ASSIGNED)) {
> +				dlpar_memory_readd_by_index(
> +					new_cell.drc_index + j);
> +			}
>  		}
>  	}
> 
> -	return rc;
> +	return 0;
> +}
> +
> +struct assoc_arrays {
> +	u32 n_arrays;
> +	u32 array_sz;
> +	const __be32 *arrays;
> +};
> +
> +struct update_ala_memory_aai_struct {
> +	int aa_index;
> +};
> +
> +static void update_ala_memory_aai_cb(struct drmem_lmb *lmb,
> +					const __be32 **data)
> +{
> +	struct update_ala_memory_aai_struct *updt =
> +		(struct update_ala_memory_aai_struct *)*data;
> +
> +	if ((lmb->aa_index != updt->aa_index) &&
> +		(lmb->flags & DRCONF_MEM_ASSIGNED))
> +		dlpar_memory_readd_by_index(lmb->drc_index);
> +}
> +
> +static int pseries_update_ala_memory_aai_v1(int aa_index,
> +				const __be32 *dmprop)
> +{
> +	struct update_ala_memory_aai_struct data = {
> +		aa_index };
> +
> +	walk_drmem_v1_lmbs(dmprop, (const __be32 *)&data,
> +			update_ala_memory_aai_cb);
> +
> +	return 0;
> +}
> +
> +static int pseries_update_ala_memory_aai_v2(int aa_index,
> +				const __be32 *dmprop)
> +{
> +	struct update_ala_memory_aai_struct data = {
> +		aa_index };
> +
> +	walk_drmem_v2_lmbs(dmprop, (const __be32 *)&data,
> +			update_ala_memory_aai_cb);
> +
> +	return 0;
> +}
> +
> +static int pseries_update_ala_memory_aai(int v1, int aa_index,
> +				const __be32 *dmprop)
> +{
> +	if (v1)
> +		return pseries_update_ala_memory_aai_v1(aa_index, dmprop);
> +	else
> +		return pseries_update_ala_memory_aai_v2(aa_index, dmprop);
>  }
> 
>  static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>  {
>  	struct assoc_arrays new_ala, old_ala;
>  	struct device_node *dn;
> -	struct property *dmprop;
> +	const __be32 *dmprop;
> +	bool v1 = true;
>  	__be32 *p;
>  	int i, lim;
> 
> @@ -1104,10 +1210,15 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>  	if (!dn)
>  		return -ENODEV;
> 
> -	dmprop = of_find_property(dn, "ibm,dynamic-memory", NULL);
> +	dmprop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>  	if (!dmprop) {
> -		of_node_put(dn);
> -		return -ENODEV;
> +		v1 = false;
> +		dmprop = of_get_property(dn, "ibm,dynamic-memory-v2",
> +					NULL);
> +		if (!dmprop) {
> +			of_node_put(dn);
> +			return -ENODEV;
> +		}
>  	}
> 
>  	/*
> @@ -1149,11 +1260,11 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>  						new_ala.array_sz))
>  				continue;
> 
> -			pseries_update_ala_memory_aai(i, dmprop);
> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>  		}
> 
>  		for (i = lim; i < new_ala.n_arrays; i++)
> -			pseries_update_ala_memory_aai(i, dmprop);
> +			pseries_update_ala_memory_aai(v1, i, dmprop);
> 
>  	} else {
>  		/* Update all entries representing these rows;
> @@ -1161,7 +1272,7 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>  		 * have equivalent values.
>  		 */
>  		for (i = 0; i < lim; i++)
> -			pseries_update_ala_memory_aai(i, dmprop);
> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>  	}
> 
>  	of_node_put(dn);
> @@ -1184,6 +1295,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>  	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-v2"))
> +			err = pseries_update_drconf_memory_v2(rd);
>  		if (!strcmp(rd->prop->name,
>  				"ibm,associativity-lookup-arrays"))
>  			err = pseries_update_ala_memory(rd);
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 4675e5a..00df576 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -539,7 +539,7 @@ void *of_fdt_unflatten_tree(const unsigned long *blob,
>  EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
> 
>  /* Everything below here references initial_boot_params directly. */
> -int __initdata dt_root_addr_cells;
> +int dt_root_addr_cells;
>  int __initdata dt_root_size_cells;
> 
>  void *initial_boot_params;
> @@ -1013,7 +1013,7 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
>  	return 1;
>  }
> 
> -u64 __init dt_mem_next_cell(int s, const __be32 **cellp)
> +u64 dt_mem_next_cell(int s, const __be32 **cellp)
>  {
>  	const __be32 *p = *cellp;
> 
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 013c541..14c8681 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -40,7 +40,7 @@ extern void *of_fdt_unflatten_tree(const unsigned long *blob,
>  				   struct device_node **mynodes);
> 
>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
> -extern int __initdata dt_root_addr_cells;
> +extern int dt_root_addr_cells;
>  extern int __initdata dt_root_size_cells;
>  extern void *initial_boot_params;
> 

Re: [RFC v2 3/3] postmigration/memory: Associativity & ibm,dynamic-memory-v2

[Michael Bringmann]

See below.

On 04/24/2018 12:17 PM, Nathan Fontenot wrote:
> On 02/26/2018 02:53 PM, Michael Bringmann wrote:
>> postmigration/memory: Now apply changes to the associativity of memory
>> blocks described by the 'ibm,dynamic-memory-v2' property regarding
>> the topology of LPARS in Post Migration events.
>>
>> * Extend the previous work done for the 'ibm,associativity-lookup-array'
>>   to apply to either property 'ibm,dynamic-memory' or
>>   'ibm,dynamic-memory-v2', whichever is present.
>> * Add new code to parse the 'ibm,dynamic-memory-v2' property looking
>>   for differences in block 'assignment', associativity indexes per
>>   block, and any other difference currently known.
>> * Rewrite some of the original code to parse the 'ibm,dynamic-memory'
>>   property to take advantage of LMB parsing code.
>>
>> When block differences are recognized, the memory block may be removed,
>> added, or updated depending upon the state of the new device tree
>> property and differences from the migrated value of the property.
>>
> 
> The only thing we need to check during LPM is affinity updates, memory
> is not added or removed as part of LPM.
> 
> I think a slightly different approach to this may be worth considering.
> One of the goals of the drmem.c code was to remove the need to parse the
> device tree for memory directly. For this update, I think we could modify
> the code that builds the drmem_info data so that it can return a drmem_info
> struct instead of assuming to set the global one.
> 
> This change would allow you to do a straight compare on the global vs. the
> new info from the updated device tree property. I think this would be cleaner
> and may be able to use the same routine for V1 and V2 properties.

The code dealing with the 'ibm,associativity' array updated cleanly to use
the same function to scan the LMBs regardless of the version of the properties.

The code dealing with changes to 'ibm,dynamic-memory-v2' is a mirror of the
code in 'pseries_update_drconf_memory' that deals with changes to the property
'ibm,dynamic-memory', so it should also be updated.  On the other hand, do we
need to consider the memory requirements of creating/cloning the drmem_info
structure to provide a copy based on the new 'dynamic-memory' property?
Or is this not an issue?

> 
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in RFC v2:
>>   -- Reuse existing parser code from 'drmem.c' in parsing property
>>      'imb,dynamic-memory-v2' for migration.
>>   -- Fix crash during migration that occurs on non-VPHN systems
>>      when attempting to reset topology timer.
>>   -- Change section of a support function + variable from __init 
>>      to normal runtime to make them visible to migration code.
>> ---
>>  arch/powerpc/include/asm/drmem.h                |    8 +
>>  arch/powerpc/mm/drmem.c                         |   23 ++-
>>  arch/powerpc/mm/numa.c                          |    3 
>>  arch/powerpc/platforms/pseries/hotplug-memory.c |  175 +++++++++++++++++++----
>>  drivers/of/fdt.c                                |    4 -
>>  include/linux/of_fdt.h                          |    2 
>>  6 files changed, 170 insertions(+), 45 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
>> index 47a7012..e4773c9 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -92,6 +92,14 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>>  int drmem_update_dt(void);
>>
>> +void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
>> +			void (*func)(struct drmem_lmb *, const __be32 **));
>> +
>> +void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +			const __be32 **prop);
>> +void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
>> +			void (*func)(struct drmem_lmb *, const __be32 **));
>> +
>>  #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 31dbe14..e47a6e0 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -192,7 +192,7 @@ 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;
>> @@ -208,7 +208,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,
>> +void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct drmem_lmb lmb;
>> @@ -218,11 +218,12 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>
>>  	for (i = 0; i < n_lmbs; i++) {
>>  		read_drconf_v1_cell(&lmb, &prop);
>> -		func(&lmb, &usm);
>> +		func(&lmb, &data);
>>  	}
>>  }
>> +EXPORT_SYMBOL(walk_drmem_v1_lmbs);
>>
>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>> @@ -235,8 +236,9 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>
>>  	*prop = p;
>>  }
>> +EXPORT_SYMBOL(read_drconf_v2_cell);
>>
>> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>> +void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -258,10 +260,11 @@ static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>>  			lmb.aa_index = dr_cell.aa_index;
>>  			lmb.flags = dr_cell.flags;
>>
>> -			func(&lmb, &usm);
>> +			func(&lmb, &data);
>>  		}
>>  	}
>>  }
>> +EXPORT_SYMBOL(walk_drmem_v2_lmbs);
>>
>>  #ifdef CONFIG_PPC_PSERIES
>>  void __init walk_drmem_lmbs_early(unsigned long node,
>> @@ -280,12 +283,12 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>>
>>  	prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory", &len);
>>  	if (prop) {
>> -		__walk_drmem_v1_lmbs(prop, usm, func);
>> +		walk_drmem_v1_lmbs(prop, usm, func);
>>  	} else {
>>  		prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory-v2",
>>  					   &len);
>>  		if (prop)
>> -			__walk_drmem_v2_lmbs(prop, usm, func);
>> +			walk_drmem_v2_lmbs(prop, usm, func);
>>  	}
>>
>>  	memblock_dump_all();
>> @@ -340,11 +343,11 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>
>>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>  	if (prop) {
>> -		__walk_drmem_v1_lmbs(prop, usm, func);
>> +		walk_drmem_v1_lmbs(prop, usm, func);
>>  	} else {
>>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>>  		if (prop)
>> -			__walk_drmem_v2_lmbs(prop, usm, func);
>> +			walk_drmem_v2_lmbs(prop, usm, func);
>>  	}
>>  }
>>
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index 0e573f9..2545fea 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -1395,7 +1395,8 @@ static void topology_timer_fn(struct timer_list *unused)
>>
>>  static void reset_topology_timer(void)
>>  {
>> -	mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
>> +	if (vphn_enabled)
>> +		mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
> 
> This really looks like a bug fix that should be in a different patch.

Okay.

> 
> -Nathan

Thanks.
Michael

> 
>>  }
>>
>>  #ifdef CONFIG_SMP
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index b63181d..bf717e2 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -1051,49 +1051,155 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>>  	return rc;
>>  }
>>
>> -struct assoc_arrays {
>> -	u32 n_arrays;
>> -	u32 array_sz;
>> -	const __be32 *arrays;
>> -};
>> +static inline int pseries_memory_v2_find_drc(u32 drc_index,
>> +			u64 *base_addr, unsigned long memblock_size,
>> +			struct of_drconf_cell_v2 *dm)
>> +{
>> +	if ((dm->drc_index <= drc_index) &&
>> +		(drc_index <= (dm->drc_index + dm->seq_lmbs - 1))) {
>> +		int offset = drc_index - dm->drc_index;
>> +
>> +		(*base_addr) = dm->base_addr +
>> +				(offset * memblock_size);
>> +	} else if (drc_index > (dm->drc_index +
>> +				dm->seq_lmbs - 1)) {
>> +		return -1;
>> +	} else if (dm->drc_index > drc_index) {
>> +		return -1;
>> +	}
>> +
>> +	return 0;
>> +}
>>
>> -static int pseries_update_ala_memory_aai(int aa_index,
>> -					struct property *dmprop)
>> +static int pseries_update_drconf_memory_v2(struct of_reconfig_data *pr)
>>  {
>> -	struct of_drconf_cell *drmem;
>> -	u32 entries;
>> -	__be32 *p;
>> -	int i;
>> -	int rc = 0;
>> +	const __be32 *new_drmem, *old_drmem;
>> +	unsigned long memblock_size;
>> +	u32 new_lmb_sets, old_lmb_sets;
>> +	u64 old_base_addr;
>> +	int i, rc = 0;
>>
>> -	p = (__be32 *) dmprop->value;
>> -	if (!p)
>> +	if (rtas_hp_event)
>> +		return 0;
>> +
>> +	memblock_size = pseries_memory_block_size();
>> +	if (!memblock_size)
>>  		return -EINVAL;
>>
>>  	/* 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.
>> +	 * of of_drconf_cell_v2 entries. Get the number of entries
>> +	 * and skip to the array of of_drconf_cell_v2's.
>>  	 */
>> -	entries = be32_to_cpu(*p++);
>> -	drmem = (struct of_drconf_cell *)p;
>> +	old_drmem = (__be32 *) pr->old_prop->value;
>> +	if (!old_drmem)
>> +		return -EINVAL;
>> +	old_lmb_sets = of_read_number(old_drmem++, 1);
>>
>> -	for (i = 0; i < entries; i++) {
>> -		if ((be32_to_cpu(drmem[i].aa_index) != aa_index) &&
>> -			(be32_to_cpu(drmem[i].flags) & DRCONF_MEM_ASSIGNED)) {
>> -			rc = dlpar_memory_readd_by_index(
>> -				be32_to_cpu(drmem[i].drc_index));
>> +	new_drmem = (__be32 *)pr->prop->value;
>> +	new_lmb_sets = of_read_number(new_drmem++, 1);
>> +
>> +	for (i = 0; i < old_lmb_sets; i++) {
>> +		int j;
>> +		struct of_drconf_cell_v2 old_cell, new_cell;
>> +
>> +		read_drconf_v2_cell(&old_cell, &old_drmem);
>> +		read_drconf_v2_cell(&new_cell, &new_drmem);
>> +
>> +		for (j = 0; j < new_cell.seq_lmbs; j++) {
>> +			if (pseries_memory_v2_find_drc(
>> +				new_cell.drc_index + j,
>> +				&old_base_addr,
>> +				memblock_size,
>> +				&old_cell))
>> +				continue;
>> +
>> +			if ((old_cell.flags &
>> +					DRCONF_MEM_ASSIGNED) &&
>> +			    (!(new_cell.flags &
>> +					DRCONF_MEM_ASSIGNED))) {
>> +				rc = pseries_remove_memblock(
>> +					old_base_addr,
>> +					memblock_size);
>> +			} else if ((!(old_cell.flags &
>> +					DRCONF_MEM_ASSIGNED)) &&
>> +				   (new_cell.flags &
>> +					DRCONF_MEM_ASSIGNED)) {
>> +				rc = memblock_add(
>> +					old_base_addr, memblock_size);
>> +			} else if ((old_cell.aa_index !=
>> +				    new_cell.aa_index) &&
>> +				   (new_cell.flags &
>> +					DRCONF_MEM_ASSIGNED)) {
>> +				dlpar_memory_readd_by_index(
>> +					new_cell.drc_index + j);
>> +			}
>>  		}
>>  	}
>>
>> -	return rc;
>> +	return 0;
>> +}
>> +
>> +struct assoc_arrays {
>> +	u32 n_arrays;
>> +	u32 array_sz;
>> +	const __be32 *arrays;
>> +};
>> +
>> +struct update_ala_memory_aai_struct {
>> +	int aa_index;
>> +};
>> +
>> +static void update_ala_memory_aai_cb(struct drmem_lmb *lmb,
>> +					const __be32 **data)
>> +{
>> +	struct update_ala_memory_aai_struct *updt =
>> +		(struct update_ala_memory_aai_struct *)*data;
>> +
>> +	if ((lmb->aa_index != updt->aa_index) &&
>> +		(lmb->flags & DRCONF_MEM_ASSIGNED))
>> +		dlpar_memory_readd_by_index(lmb->drc_index);
>> +}
>> +
>> +static int pseries_update_ala_memory_aai_v1(int aa_index,
>> +				const __be32 *dmprop)
>> +{
>> +	struct update_ala_memory_aai_struct data = {
>> +		aa_index };
>> +
>> +	walk_drmem_v1_lmbs(dmprop, (const __be32 *)&data,
>> +			update_ala_memory_aai_cb);
>> +
>> +	return 0;
>> +}
>> +
>> +static int pseries_update_ala_memory_aai_v2(int aa_index,
>> +				const __be32 *dmprop)
>> +{
>> +	struct update_ala_memory_aai_struct data = {
>> +		aa_index };
>> +
>> +	walk_drmem_v2_lmbs(dmprop, (const __be32 *)&data,
>> +			update_ala_memory_aai_cb);
>> +
>> +	return 0;
>> +}
>> +
>> +static int pseries_update_ala_memory_aai(int v1, int aa_index,
>> +				const __be32 *dmprop)
>> +{
>> +	if (v1)
>> +		return pseries_update_ala_memory_aai_v1(aa_index, dmprop);
>> +	else
>> +		return pseries_update_ala_memory_aai_v2(aa_index, dmprop);
>>  }
>>
>>  static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>  {
>>  	struct assoc_arrays new_ala, old_ala;
>>  	struct device_node *dn;
>> -	struct property *dmprop;
>> +	const __be32 *dmprop;
>> +	bool v1 = true;
>>  	__be32 *p;
>>  	int i, lim;
>>
>> @@ -1104,10 +1210,15 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>  	if (!dn)
>>  		return -ENODEV;
>>
>> -	dmprop = of_find_property(dn, "ibm,dynamic-memory", NULL);
>> +	dmprop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>  	if (!dmprop) {
>> -		of_node_put(dn);
>> -		return -ENODEV;
>> +		v1 = false;
>> +		dmprop = of_get_property(dn, "ibm,dynamic-memory-v2",
>> +					NULL);
>> +		if (!dmprop) {
>> +			of_node_put(dn);
>> +			return -ENODEV;
>> +		}
>>  	}
>>
>>  	/*
>> @@ -1149,11 +1260,11 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>  						new_ala.array_sz))
>>  				continue;
>>
>> -			pseries_update_ala_memory_aai(i, dmprop);
>> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>>  		}
>>
>>  		for (i = lim; i < new_ala.n_arrays; i++)
>> -			pseries_update_ala_memory_aai(i, dmprop);
>> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>>
>>  	} else {
>>  		/* Update all entries representing these rows;
>> @@ -1161,7 +1272,7 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>  		 * have equivalent values.
>>  		 */
>>  		for (i = 0; i < lim; i++)
>> -			pseries_update_ala_memory_aai(i, dmprop);
>> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>>  	}
>>
>>  	of_node_put(dn);
>> @@ -1184,6 +1295,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>>  	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-v2"))
>> +			err = pseries_update_drconf_memory_v2(rd);
>>  		if (!strcmp(rd->prop->name,
>>  				"ibm,associativity-lookup-arrays"))
>>  			err = pseries_update_ala_memory(rd);
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 4675e5a..00df576 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -539,7 +539,7 @@ void *of_fdt_unflatten_tree(const unsigned long *blob,
>>  EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
>>
>>  /* Everything below here references initial_boot_params directly. */
>> -int __initdata dt_root_addr_cells;
>> +int dt_root_addr_cells;
>>  int __initdata dt_root_size_cells;
>>
>>  void *initial_boot_params;
>> @@ -1013,7 +1013,7 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
>>  	return 1;
>>  }
>>
>> -u64 __init dt_mem_next_cell(int s, const __be32 **cellp)
>> +u64 dt_mem_next_cell(int s, const __be32 **cellp)
>>  {
>>  	const __be32 *p = *cellp;
>>
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 013c541..14c8681 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -40,7 +40,7 @@ extern void *of_fdt_unflatten_tree(const unsigned long *blob,
>>  				   struct device_node **mynodes);
>>
>>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
>> -extern int __initdata dt_root_addr_cells;
>> +extern int dt_root_addr_cells;
>>  extern int __initdata dt_root_size_cells;
>>  extern void *initial_boot_params;
>>
> 

-- 
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

Re: [RFC v2 3/3] postmigration/memory: Associativity & ibm,dynamic-memory-v2

[Nathan Fontenot]

On 04/24/2018 04:35 PM, Michael Bringmann wrote:
> See below.
> 
> On 04/24/2018 12:17 PM, Nathan Fontenot wrote:
>> On 02/26/2018 02:53 PM, Michael Bringmann wrote:
>>> postmigration/memory: Now apply changes to the associativity of memory
>>> blocks described by the 'ibm,dynamic-memory-v2' property regarding
>>> the topology of LPARS in Post Migration events.
>>>
>>> * Extend the previous work done for the 'ibm,associativity-lookup-array'
>>>   to apply to either property 'ibm,dynamic-memory' or
>>>   'ibm,dynamic-memory-v2', whichever is present.
>>> * Add new code to parse the 'ibm,dynamic-memory-v2' property looking
>>>   for differences in block 'assignment', associativity indexes per
>>>   block, and any other difference currently known.
>>> * Rewrite some of the original code to parse the 'ibm,dynamic-memory'
>>>   property to take advantage of LMB parsing code.
>>>
>>> When block differences are recognized, the memory block may be removed,
>>> added, or updated depending upon the state of the new device tree
>>> property and differences from the migrated value of the property.
>>>
>>
>> The only thing we need to check during LPM is affinity updates, memory
>> is not added or removed as part of LPM.
>>
>> I think a slightly different approach to this may be worth considering.
>> One of the goals of the drmem.c code was to remove the need to parse the
>> device tree for memory directly. For this update, I think we could modify
>> the code that builds the drmem_info data so that it can return a drmem_info
>> struct instead of assuming to set the global one.
>>
>> This change would allow you to do a straight compare on the global vs. the
>> new info from the updated device tree property. I think this would be cleaner
>> and may be able to use the same routine for V1 and V2 properties.
> 
> The code dealing with the 'ibm,associativity' array updated cleanly to use
> the same function to scan the LMBs regardless of the version of the properties.
> 
> The code dealing with changes to 'ibm,dynamic-memory-v2' is a mirror of the
> code in 'pseries_update_drconf_memory' that deals with changes to the property
> 'ibm,dynamic-memory', so it should also be updated.  On the other hand, do we
> need to consider the memory requirements of creating/cloning the drmem_info
> structure to provide a copy based on the new 'dynamic-memory' property?
> Or is this not an issue?

If done correctly, using the drmem code to create a drmem_info struct from
the new memory property would allow us to use the same comparison routine
for v1 and v2 versions of the property.

The size of the drmem_info data is not big enough to be concerned about
memory requirements when making a copy. 

-Nathan

> 
>>
>>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>>> ---
>>> Changes in RFC v2:
>>>   -- Reuse existing parser code from 'drmem.c' in parsing property
>>>      'imb,dynamic-memory-v2' for migration.
>>>   -- Fix crash during migration that occurs on non-VPHN systems
>>>      when attempting to reset topology timer.
>>>   -- Change section of a support function + variable from __init 
>>>      to normal runtime to make them visible to migration code.
>>> ---
>>>  arch/powerpc/include/asm/drmem.h                |    8 +
>>>  arch/powerpc/mm/drmem.c                         |   23 ++-
>>>  arch/powerpc/mm/numa.c                          |    3 
>>>  arch/powerpc/platforms/pseries/hotplug-memory.c |  175 +++++++++++++++++++----
>>>  drivers/of/fdt.c                                |    4 -
>>>  include/linux/of_fdt.h                          |    2 
>>>  6 files changed, 170 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
>>> index 47a7012..e4773c9 100644
>>> --- a/arch/powerpc/include/asm/drmem.h
>>> +++ b/arch/powerpc/include/asm/drmem.h
>>> @@ -92,6 +92,14 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>>>  int drmem_update_dt(void);
>>>
>>> +void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
>>> +			void (*func)(struct drmem_lmb *, const __be32 **));
>>> +
>>> +void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>> +			const __be32 **prop);
>>> +void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
>>> +			void (*func)(struct drmem_lmb *, const __be32 **));
>>> +
>>>  #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 31dbe14..e47a6e0 100644
>>> --- a/arch/powerpc/mm/drmem.c
>>> +++ b/arch/powerpc/mm/drmem.c
>>> @@ -192,7 +192,7 @@ 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;
>>> @@ -208,7 +208,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,
>>> +void walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *data,
>>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>>  {
>>>  	struct drmem_lmb lmb;
>>> @@ -218,11 +218,12 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>>
>>>  	for (i = 0; i < n_lmbs; i++) {
>>>  		read_drconf_v1_cell(&lmb, &prop);
>>> -		func(&lmb, &usm);
>>> +		func(&lmb, &data);
>>>  	}
>>>  }
>>> +EXPORT_SYMBOL(walk_drmem_v1_lmbs);
>>>
>>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>> +void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>>  				       const __be32 **prop)
>>>  {
>>>  	const __be32 *p = *prop;
>>> @@ -235,8 +236,9 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>>
>>>  	*prop = p;
>>>  }
>>> +EXPORT_SYMBOL(read_drconf_v2_cell);
>>>
>>> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>>> +void walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *data,
>>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>>  {
>>>  	struct of_drconf_cell_v2 dr_cell;
>>> @@ -258,10 +260,11 @@ static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>>>  			lmb.aa_index = dr_cell.aa_index;
>>>  			lmb.flags = dr_cell.flags;
>>>
>>> -			func(&lmb, &usm);
>>> +			func(&lmb, &data);
>>>  		}
>>>  	}
>>>  }
>>> +EXPORT_SYMBOL(walk_drmem_v2_lmbs);
>>>
>>>  #ifdef CONFIG_PPC_PSERIES
>>>  void __init walk_drmem_lmbs_early(unsigned long node,
>>> @@ -280,12 +283,12 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>>>
>>>  	prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory", &len);
>>>  	if (prop) {
>>> -		__walk_drmem_v1_lmbs(prop, usm, func);
>>> +		walk_drmem_v1_lmbs(prop, usm, func);
>>>  	} else {
>>>  		prop = of_get_flat_dt_prop(node, "ibm,dynamic-memory-v2",
>>>  					   &len);
>>>  		if (prop)
>>> -			__walk_drmem_v2_lmbs(prop, usm, func);
>>> +			walk_drmem_v2_lmbs(prop, usm, func);
>>>  	}
>>>
>>>  	memblock_dump_all();
>>> @@ -340,11 +343,11 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>>
>>>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>>  	if (prop) {
>>> -		__walk_drmem_v1_lmbs(prop, usm, func);
>>> +		walk_drmem_v1_lmbs(prop, usm, func);
>>>  	} else {
>>>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>>>  		if (prop)
>>> -			__walk_drmem_v2_lmbs(prop, usm, func);
>>> +			walk_drmem_v2_lmbs(prop, usm, func);
>>>  	}
>>>  }
>>>
>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>>> index 0e573f9..2545fea 100644
>>> --- a/arch/powerpc/mm/numa.c
>>> +++ b/arch/powerpc/mm/numa.c
>>> @@ -1395,7 +1395,8 @@ static void topology_timer_fn(struct timer_list *unused)
>>>
>>>  static void reset_topology_timer(void)
>>>  {
>>> -	mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
>>> +	if (vphn_enabled)
>>> +		mod_timer(&topology_timer, jiffies + topology_timer_secs * HZ);
>>
>> This really looks like a bug fix that should be in a different patch.
> 
> Okay.
> 
>>
>> -Nathan
> 
> Thanks.
> Michael
> 
>>
>>>  }
>>>
>>>  #ifdef CONFIG_SMP
>>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> index b63181d..bf717e2 100644
>>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>>> @@ -1051,49 +1051,155 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>>>  	return rc;
>>>  }
>>>
>>> -struct assoc_arrays {
>>> -	u32 n_arrays;
>>> -	u32 array_sz;
>>> -	const __be32 *arrays;
>>> -};
>>> +static inline int pseries_memory_v2_find_drc(u32 drc_index,
>>> +			u64 *base_addr, unsigned long memblock_size,
>>> +			struct of_drconf_cell_v2 *dm)
>>> +{
>>> +	if ((dm->drc_index <= drc_index) &&
>>> +		(drc_index <= (dm->drc_index + dm->seq_lmbs - 1))) {
>>> +		int offset = drc_index - dm->drc_index;
>>> +
>>> +		(*base_addr) = dm->base_addr +
>>> +				(offset * memblock_size);
>>> +	} else if (drc_index > (dm->drc_index +
>>> +				dm->seq_lmbs - 1)) {
>>> +		return -1;
>>> +	} else if (dm->drc_index > drc_index) {
>>> +		return -1;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>>
>>> -static int pseries_update_ala_memory_aai(int aa_index,
>>> -					struct property *dmprop)
>>> +static int pseries_update_drconf_memory_v2(struct of_reconfig_data *pr)
>>>  {
>>> -	struct of_drconf_cell *drmem;
>>> -	u32 entries;
>>> -	__be32 *p;
>>> -	int i;
>>> -	int rc = 0;
>>> +	const __be32 *new_drmem, *old_drmem;
>>> +	unsigned long memblock_size;
>>> +	u32 new_lmb_sets, old_lmb_sets;
>>> +	u64 old_base_addr;
>>> +	int i, rc = 0;
>>>
>>> -	p = (__be32 *) dmprop->value;
>>> -	if (!p)
>>> +	if (rtas_hp_event)
>>> +		return 0;
>>> +
>>> +	memblock_size = pseries_memory_block_size();
>>> +	if (!memblock_size)
>>>  		return -EINVAL;
>>>
>>>  	/* 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.
>>> +	 * of of_drconf_cell_v2 entries. Get the number of entries
>>> +	 * and skip to the array of of_drconf_cell_v2's.
>>>  	 */
>>> -	entries = be32_to_cpu(*p++);
>>> -	drmem = (struct of_drconf_cell *)p;
>>> +	old_drmem = (__be32 *) pr->old_prop->value;
>>> +	if (!old_drmem)
>>> +		return -EINVAL;
>>> +	old_lmb_sets = of_read_number(old_drmem++, 1);
>>>
>>> -	for (i = 0; i < entries; i++) {
>>> -		if ((be32_to_cpu(drmem[i].aa_index) != aa_index) &&
>>> -			(be32_to_cpu(drmem[i].flags) & DRCONF_MEM_ASSIGNED)) {
>>> -			rc = dlpar_memory_readd_by_index(
>>> -				be32_to_cpu(drmem[i].drc_index));
>>> +	new_drmem = (__be32 *)pr->prop->value;
>>> +	new_lmb_sets = of_read_number(new_drmem++, 1);
>>> +
>>> +	for (i = 0; i < old_lmb_sets; i++) {
>>> +		int j;
>>> +		struct of_drconf_cell_v2 old_cell, new_cell;
>>> +
>>> +		read_drconf_v2_cell(&old_cell, &old_drmem);
>>> +		read_drconf_v2_cell(&new_cell, &new_drmem);
>>> +
>>> +		for (j = 0; j < new_cell.seq_lmbs; j++) {
>>> +			if (pseries_memory_v2_find_drc(
>>> +				new_cell.drc_index + j,
>>> +				&old_base_addr,
>>> +				memblock_size,
>>> +				&old_cell))
>>> +				continue;
>>> +
>>> +			if ((old_cell.flags &
>>> +					DRCONF_MEM_ASSIGNED) &&
>>> +			    (!(new_cell.flags &
>>> +					DRCONF_MEM_ASSIGNED))) {
>>> +				rc = pseries_remove_memblock(
>>> +					old_base_addr,
>>> +					memblock_size);
>>> +			} else if ((!(old_cell.flags &
>>> +					DRCONF_MEM_ASSIGNED)) &&
>>> +				   (new_cell.flags &
>>> +					DRCONF_MEM_ASSIGNED)) {
>>> +				rc = memblock_add(
>>> +					old_base_addr, memblock_size);
>>> +			} else if ((old_cell.aa_index !=
>>> +				    new_cell.aa_index) &&
>>> +				   (new_cell.flags &
>>> +					DRCONF_MEM_ASSIGNED)) {
>>> +				dlpar_memory_readd_by_index(
>>> +					new_cell.drc_index + j);
>>> +			}
>>>  		}
>>>  	}
>>>
>>> -	return rc;
>>> +	return 0;
>>> +}
>>> +
>>> +struct assoc_arrays {
>>> +	u32 n_arrays;
>>> +	u32 array_sz;
>>> +	const __be32 *arrays;
>>> +};
>>> +
>>> +struct update_ala_memory_aai_struct {
>>> +	int aa_index;
>>> +};
>>> +
>>> +static void update_ala_memory_aai_cb(struct drmem_lmb *lmb,
>>> +					const __be32 **data)
>>> +{
>>> +	struct update_ala_memory_aai_struct *updt =
>>> +		(struct update_ala_memory_aai_struct *)*data;
>>> +
>>> +	if ((lmb->aa_index != updt->aa_index) &&
>>> +		(lmb->flags & DRCONF_MEM_ASSIGNED))
>>> +		dlpar_memory_readd_by_index(lmb->drc_index);
>>> +}
>>> +
>>> +static int pseries_update_ala_memory_aai_v1(int aa_index,
>>> +				const __be32 *dmprop)
>>> +{
>>> +	struct update_ala_memory_aai_struct data = {
>>> +		aa_index };
>>> +
>>> +	walk_drmem_v1_lmbs(dmprop, (const __be32 *)&data,
>>> +			update_ala_memory_aai_cb);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int pseries_update_ala_memory_aai_v2(int aa_index,
>>> +				const __be32 *dmprop)
>>> +{
>>> +	struct update_ala_memory_aai_struct data = {
>>> +		aa_index };
>>> +
>>> +	walk_drmem_v2_lmbs(dmprop, (const __be32 *)&data,
>>> +			update_ala_memory_aai_cb);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int pseries_update_ala_memory_aai(int v1, int aa_index,
>>> +				const __be32 *dmprop)
>>> +{
>>> +	if (v1)
>>> +		return pseries_update_ala_memory_aai_v1(aa_index, dmprop);
>>> +	else
>>> +		return pseries_update_ala_memory_aai_v2(aa_index, dmprop);
>>>  }
>>>
>>>  static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>>  {
>>>  	struct assoc_arrays new_ala, old_ala;
>>>  	struct device_node *dn;
>>> -	struct property *dmprop;
>>> +	const __be32 *dmprop;
>>> +	bool v1 = true;
>>>  	__be32 *p;
>>>  	int i, lim;
>>>
>>> @@ -1104,10 +1210,15 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>>  	if (!dn)
>>>  		return -ENODEV;
>>>
>>> -	dmprop = of_find_property(dn, "ibm,dynamic-memory", NULL);
>>> +	dmprop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>>  	if (!dmprop) {
>>> -		of_node_put(dn);
>>> -		return -ENODEV;
>>> +		v1 = false;
>>> +		dmprop = of_get_property(dn, "ibm,dynamic-memory-v2",
>>> +					NULL);
>>> +		if (!dmprop) {
>>> +			of_node_put(dn);
>>> +			return -ENODEV;
>>> +		}
>>>  	}
>>>
>>>  	/*
>>> @@ -1149,11 +1260,11 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>>  						new_ala.array_sz))
>>>  				continue;
>>>
>>> -			pseries_update_ala_memory_aai(i, dmprop);
>>> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>>>  		}
>>>
>>>  		for (i = lim; i < new_ala.n_arrays; i++)
>>> -			pseries_update_ala_memory_aai(i, dmprop);
>>> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>>>
>>>  	} else {
>>>  		/* Update all entries representing these rows;
>>> @@ -1161,7 +1272,7 @@ static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>>>  		 * have equivalent values.
>>>  		 */
>>>  		for (i = 0; i < lim; i++)
>>> -			pseries_update_ala_memory_aai(i, dmprop);
>>> +			pseries_update_ala_memory_aai(v1, i, dmprop);
>>>  	}
>>>
>>>  	of_node_put(dn);
>>> @@ -1184,6 +1295,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>>>  	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-v2"))
>>> +			err = pseries_update_drconf_memory_v2(rd);
>>>  		if (!strcmp(rd->prop->name,
>>>  				"ibm,associativity-lookup-arrays"))
>>>  			err = pseries_update_ala_memory(rd);
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 4675e5a..00df576 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -539,7 +539,7 @@ void *of_fdt_unflatten_tree(const unsigned long *blob,
>>>  EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
>>>
>>>  /* Everything below here references initial_boot_params directly. */
>>> -int __initdata dt_root_addr_cells;
>>> +int dt_root_addr_cells;
>>>  int __initdata dt_root_size_cells;
>>>
>>>  void *initial_boot_params;
>>> @@ -1013,7 +1013,7 @@ int __init early_init_dt_scan_root(unsigned long node, const char *uname,
>>>  	return 1;
>>>  }
>>>
>>> -u64 __init dt_mem_next_cell(int s, const __be32 **cellp)
>>> +u64 dt_mem_next_cell(int s, const __be32 **cellp)
>>>  {
>>>  	const __be32 *p = *cellp;
>>>
>>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>>> index 013c541..14c8681 100644
>>> --- a/include/linux/of_fdt.h
>>> +++ b/include/linux/of_fdt.h
>>> @@ -40,7 +40,7 @@ extern void *of_fdt_unflatten_tree(const unsigned long *blob,
>>>  				   struct device_node **mynodes);
>>>
>>>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
>>> -extern int __initdata dt_root_addr_cells;
>>> +extern int dt_root_addr_cells;
>>>  extern int __initdata dt_root_size_cells;
>>>  extern void *initial_boot_params;
>>>
>>
>