[PATCH v03 0/5] powerpc/migration: Affinity fix for memory

[PATCH v03 0/5] powerpc/migration: Affinity fix for memory

[Michael Bringmann]

The migration of LPARs across Power systems affects many attributes
including that of the associativity of memory blocks.  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 recorded in
  internal data structures when compared to the latest copies in
  the device tree (e.g. ibm,dynamic-memory, ibm,dynamic-memory-v2).
* 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 memory.
* Re-register the 'changed' entities into the target system.
  Re-registration of memory blocks mostly entails acting as if they
  have been newly hot-added into the target system.

This code builds upon features introduced in a previous patch set
that updates CPUs for affinity changes that may occur during LPM.

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

Michael Bringmann (5):
  powerpc/drmem: Export 'dynamic-memory' loader
  powerpc/drmem: Add internal_flags feature
  migration/memory: Add hotplug flags READD_MULTIPLE
  migration/memory: Evaluate LMB assoc changes
  migration/memory: Support 'ibm,dynamic-memory-v2'
---
Changes in v03:
  -- Change operation to tag changed LMBs in DRMEM array instead
     of queuing a potentially huge number of structures.
  -- Added another hotplug queue event for CPU/memory operations
  -- Added internal_flags feature to DRMEM
  -- Improve the patch description language for the patch set.
  -- Revise patch set to queue worker for memory association
     updates directly to pseries worker queue.

[PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader

[Michael Bringmann]

powerpc/drmem: Export many of the functions of DRMEM to parse
"ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
operations and for Post Migration events.

Also modify the DRMEM initialization code to allow it to,

* Be called after system initialization
* Provide a separate user copy of the LMB array that is produces
* Free the user copy upon request

In addition, a couple of changes were made to make the creation
of additional copies of the LMB array more useful including,

* Add new iterator to work through a pair of drmem_info arrays.
* Modify DRMEM code to replace usages of dt_root_addr_cells, and
  dt_mem_next_cell, as these are only available at first boot.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/drmem.h |   15 ++++++++
 arch/powerpc/mm/drmem.c          |   75 ++++++++++++++++++++++++++++----------
 2 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index ce242b9..b0e70fd 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -35,6 +35,18 @@ struct drmem_lmb_info {
 		&drmem_info->lmbs[0],				\
 		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
 
+#define for_each_dinfo_lmb(dinfo, lmb)				\
+	for_each_drmem_lmb_in_range((lmb),			\
+		&dinfo->lmbs[0],				\
+		&dinfo->lmbs[dinfo->n_lmbs - 1])
+
+#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
+	for ((lmb1) = (&dinfo1->lmbs[0]),			\
+	     (lmb2) = (&dinfo2->lmbs[0]);			\
+	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
+	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
+	     (lmb1)++, (lmb2)++)
+
 /*
  * The of_drconf_cell_v1 struct defines the layout of the LMB data
  * specified in the ibm,dynamic-memory device tree property.
@@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
 			void (*func)(struct drmem_lmb *, const __be32 **));
 int drmem_update_dt(void);
 
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
+
 #ifdef CONFIG_PPC_PSERIES
 void __init walk_drmem_lmbs_early(unsigned long node,
 			void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 3f18036..13d2abb 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -20,6 +20,7 @@
 
 static struct drmem_lmb_info __drmem_info;
 struct drmem_lmb_info *drmem_info = &__drmem_info;
+static int n_root_addr_cells;
 
 u64 drmem_lmb_memory_max(void)
 {
@@ -193,12 +194,13 @@ int drmem_update_dt(void)
 	return rc;
 }
 
-static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
+static void read_drconf_v1_cell(struct drmem_lmb *lmb,
 				       const __be32 **prop)
 {
 	const __be32 *p = *prop;
 
-	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+	lmb->base_addr = of_read_number(p, n_root_addr_cells);
+	p += n_root_addr_cells;
 	lmb->drc_index = of_read_number(p++, 1);
 
 	p++; /* skip reserved field */
@@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
 	*prop = p;
 }
 
-static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
 			void (*func)(struct drmem_lmb *, const __be32 **))
 {
 	struct drmem_lmb lmb;
@@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
 	}
 }
 
-static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
 				       const __be32 **prop)
 {
 	const __be32 *p = *prop;
 
 	dr_cell->seq_lmbs = of_read_number(p++, 1);
-	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
+	p += n_root_addr_cells;
 	dr_cell->drc_index = of_read_number(p++, 1);
 	dr_cell->aa_index = of_read_number(p++, 1);
 	dr_cell->flags = of_read_number(p++, 1);
@@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
 	*prop = p;
 }
 
-static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
 			void (*func)(struct drmem_lmb *, const __be32 **))
 {
 	struct of_drconf_cell_v2 dr_cell;
@@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
 	const __be32 *prop, *usm;
 	int len;
 
+	if (n_root_addr_cells == 0)
+		n_root_addr_cells = dt_root_addr_cells;
+
 	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
 	if (!prop || len < dt_root_size_cells * sizeof(__be32))
 		return;
@@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
 	}
 }
 
-static void __init init_drmem_v1_lmbs(const __be32 *prop)
+static void init_drmem_v1_lmbs(const __be32 *prop,
+				struct drmem_lmb_info *dinfo)
 {
 	struct drmem_lmb *lmb;
 
-	drmem_info->n_lmbs = of_read_number(prop++, 1);
-	if (drmem_info->n_lmbs == 0)
+	dinfo->n_lmbs = of_read_number(prop++, 1);
+	if (dinfo->n_lmbs == 0)
 		return;
 
-	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
 				   GFP_KERNEL);
-	if (!drmem_info->lmbs)
+	if (!dinfo->lmbs)
 		return;
 
-	for_each_drmem_lmb(lmb)
+	for_each_dinfo_lmb(dinfo, lmb)
 		read_drconf_v1_cell(lmb, &prop);
 }
 
-static void __init init_drmem_v2_lmbs(const __be32 *prop)
+static void init_drmem_v2_lmbs(const __be32 *prop,
+				struct drmem_lmb_info *dinfo)
 {
 	struct drmem_lmb *lmb;
 	struct of_drconf_cell_v2 dr_cell;
@@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
 	p = prop;
 	for (i = 0; i < lmb_sets; i++) {
 		read_drconf_v2_cell(&dr_cell, &p);
-		drmem_info->n_lmbs += dr_cell.seq_lmbs;
+		dinfo->n_lmbs += dr_cell.seq_lmbs;
 	}
 
-	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
 				   GFP_KERNEL);
-	if (!drmem_info->lmbs)
+	if (!dinfo->lmbs)
 		return;
 
 	/* second pass, read in the LMB information */
@@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
 		read_drconf_v2_cell(&dr_cell, &p);
 
 		for (j = 0; j < dr_cell.seq_lmbs; j++) {
-			lmb = &drmem_info->lmbs[lmb_index++];
+			lmb = &dinfo->lmbs[lmb_index++];
 
 			lmb->base_addr = dr_cell.base_addr;
-			dr_cell.base_addr += drmem_info->lmb_size;
+			dr_cell.base_addr += dinfo->lmb_size;
 
 			lmb->drc_index = dr_cell.drc_index;
 			dr_cell.drc_index++;
@@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
 	}
 }
 
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
+{
+	if (dinfo) {
+		kfree(dinfo->lmbs);
+		kfree(dinfo);
+	}
+}
+
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
+{
+	struct drmem_lmb_info *dinfo;
+
+	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
+	if (!dinfo)
+		return NULL;
+
+	if (!strcmp("ibm,dynamic-memory", prop->name))
+		init_drmem_v1_lmbs(prop->value, dinfo);
+	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
+		init_drmem_v2_lmbs(prop->value, dinfo);
+
+	return dinfo;
+}
+
 static int __init drmem_init(void)
 {
 	struct device_node *dn;
 	const __be32 *prop;
 
+	if (n_root_addr_cells == 0)
+		n_root_addr_cells = dt_root_addr_cells;
+
 	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
 	if (!dn) {
 		pr_info("No dynamic reconfiguration memory found\n");
@@ -434,11 +469,11 @@ static int __init drmem_init(void)
 
 	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
 	if (prop) {
-		init_drmem_v1_lmbs(prop);
+		init_drmem_v1_lmbs(prop, drmem_info);
 	} else {
 		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
 		if (prop)
-			init_drmem_v2_lmbs(prop);
+			init_drmem_v2_lmbs(prop, drmem_info);
 	}
 
 	of_node_put(dn);

Re: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader

[Tyrel Datwyler]

On 10/01/2018 05:59 AM, Michael Bringmann wrote:
> powerpc/drmem: Export many of the functions of DRMEM to parse
> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
> operations and for Post Migration events.
> 
> Also modify the DRMEM initialization code to allow it to,
> 
> * Be called after system initialization
> * Provide a separate user copy of the LMB array that is produces
> * Free the user copy upon request
> 
> In addition, a couple of changes were made to make the creation
> of additional copies of the LMB array more useful including,
> 
> * Add new iterator to work through a pair of drmem_info arrays.
> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>   dt_mem_next_cell, as these are only available at first boot.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/drmem.h |   15 ++++++++
>  arch/powerpc/mm/drmem.c          |   75 ++++++++++++++++++++++++++++----------
>  2 files changed, 70 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index ce242b9..b0e70fd 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>  		&drmem_info->lmbs[0],				\
>  		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
> 
> +#define for_each_dinfo_lmb(dinfo, lmb)				\
> +	for_each_drmem_lmb_in_range((lmb),			\
> +		&dinfo->lmbs[0],				\
> +		&dinfo->lmbs[dinfo->n_lmbs - 1])
> +
> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
> +	for ((lmb1) = (&dinfo1->lmbs[0]),			\
> +	     (lmb2) = (&dinfo2->lmbs[0]);			\
> +	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
> +	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
> +	     (lmb1)++, (lmb2)++)
> +
>  /*
>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
>   * specified in the ibm,dynamic-memory device tree property.
> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
>  int drmem_update_dt(void);
> 
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
> +
>  #ifdef CONFIG_PPC_PSERIES
>  void __init walk_drmem_lmbs_early(unsigned long node,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 3f18036..13d2abb 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -20,6 +20,7 @@
> 
>  static struct drmem_lmb_info __drmem_info;
>  struct drmem_lmb_info *drmem_info = &__drmem_info;
> +static int n_root_addr_cells;

What is the point of this new global? I see two places were it gets initialized if it is null, and both those initializers simply set it to "dt_root_addr_cells". I also checked the rest of the patches in the series and none of those even reference this variable.

> 
>  u64 drmem_lmb_memory_max(void)
>  {
> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>  	return rc;
>  }
> 
> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
> 
> -	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> +	lmb->base_addr = of_read_number(p, n_root_addr_cells);
> +	p += n_root_addr_cells;

Unnecessary code churn do to the introduction of "n_root_addr_cells".

>  	lmb->drc_index = of_read_number(p++, 1);
> 
>  	p++; /* skip reserved field */
> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>  	*prop = p;
>  }
> 
> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct drmem_lmb lmb;
> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>  	}
>  }
> 
> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
> 
>  	dr_cell->seq_lmbs = of_read_number(p++, 1);
> -	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> +	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
> +	p += n_root_addr_cells;

Same comment as above.

>  	dr_cell->drc_index = of_read_number(p++, 1);
>  	dr_cell->aa_index = of_read_number(p++, 1);
>  	dr_cell->flags = of_read_number(p++, 1);
> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  	*prop = p;
>  }
> 
> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>  	const __be32 *prop, *usm;
>  	int len;
> 
> +	if (n_root_addr_cells == 0)
> +		n_root_addr_cells = dt_root_addr_cells;
> +

As I mentioned initially whats the point? Why not just use "dt_root_addr_cells"?

>  	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>  	if (!prop || len < dt_root_size_cells * sizeof(__be32))
>  		return;
> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  	}
>  }
> 
> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
> +static void init_drmem_v1_lmbs(const __be32 *prop,
> +				struct drmem_lmb_info *dinfo)
>  {
>  	struct drmem_lmb *lmb;
> 
> -	drmem_info->n_lmbs = of_read_number(prop++, 1);
> -	if (drmem_info->n_lmbs == 0)
> +	dinfo->n_lmbs = of_read_number(prop++, 1);
> +	if (dinfo->n_lmbs == 0)
>  		return;
> 
> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>  				   GFP_KERNEL);
> -	if (!drmem_info->lmbs)
> +	if (!dinfo->lmbs)
>  		return;
> 
> -	for_each_drmem_lmb(lmb)
> +	for_each_dinfo_lmb(dinfo, lmb)
>  		read_drconf_v1_cell(lmb, &prop);
>  }
> 
> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
> +static void init_drmem_v2_lmbs(const __be32 *prop,
> +				struct drmem_lmb_info *dinfo)
>  {
>  	struct drmem_lmb *lmb;
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  	p = prop;
>  	for (i = 0; i < lmb_sets; i++) {
>  		read_drconf_v2_cell(&dr_cell, &p);
> -		drmem_info->n_lmbs += dr_cell.seq_lmbs;
> +		dinfo->n_lmbs += dr_cell.seq_lmbs;
>  	}
> 
> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>  				   GFP_KERNEL);
> -	if (!drmem_info->lmbs)
> +	if (!dinfo->lmbs)
>  		return;
> 
>  	/* second pass, read in the LMB information */
> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  		read_drconf_v2_cell(&dr_cell, &p);
> 
>  		for (j = 0; j < dr_cell.seq_lmbs; j++) {
> -			lmb = &drmem_info->lmbs[lmb_index++];
> +			lmb = &dinfo->lmbs[lmb_index++];
> 
>  			lmb->base_addr = dr_cell.base_addr;
> -			dr_cell.base_addr += drmem_info->lmb_size;
> +			dr_cell.base_addr += dinfo->lmb_size;
> 
>  			lmb->drc_index = dr_cell.drc_index;
>  			dr_cell.drc_index++;
> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  	}
>  }
> 
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
> +{
> +	if (dinfo) {
> +		kfree(dinfo->lmbs);
> +		kfree(dinfo);
> +	}
> +}
> +
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
> +{
> +	struct drmem_lmb_info *dinfo;
> +
> +	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
> +	if (!dinfo)
> +		return NULL;
> +
> +	if (!strcmp("ibm,dynamic-memory", prop->name))
> +		init_drmem_v1_lmbs(prop->value, dinfo);
> +	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
> +		init_drmem_v2_lmbs(prop->value, dinfo);
> +
> +	return dinfo;
> +}
> +
>  static int __init drmem_init(void)
>  {
>  	struct device_node *dn;
>  	const __be32 *prop;
> 
> +	if (n_root_addr_cells == 0)
> +		n_root_addr_cells = dt_root_addr_cells;
> +

See previous comment.

-Tyrel

>  	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>  	if (!dn) {
>  		pr_info("No dynamic reconfiguration memory found\n");
> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
> 
>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>  	if (prop) {
> -		init_drmem_v1_lmbs(prop);
> +		init_drmem_v1_lmbs(prop, drmem_info);
>  	} else {
>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>  		if (prop)
> -			init_drmem_v2_lmbs(prop);
> +			init_drmem_v2_lmbs(prop, drmem_info);
>  	}
> 
>  	of_node_put(dn);
> 

Re: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader

[Michael Bringmann]

On 10/02/2018 03:56 PM, Tyrel Datwyler wrote:
> On 10/01/2018 05:59 AM, Michael Bringmann wrote:
>> powerpc/drmem: Export many of the functions of DRMEM to parse
>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>> operations and for Post Migration events.
>>
>> Also modify the DRMEM initialization code to allow it to,
>>
>> * Be called after system initialization
>> * Provide a separate user copy of the LMB array that is produces
>> * Free the user copy upon request
>>
>> In addition, a couple of changes were made to make the creation
>> of additional copies of the LMB array more useful including,
>>
>> * Add new iterator to work through a pair of drmem_info arrays.
>> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>>   dt_mem_next_cell, as these are only available at first boot.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/drmem.h |   15 ++++++++
>>  arch/powerpc/mm/drmem.c          |   75 ++++++++++++++++++++++++++++----------
>>  2 files changed, 70 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
>> index ce242b9..b0e70fd 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>>  		&drmem_info->lmbs[0],				\
>>  		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
>>
>> +#define for_each_dinfo_lmb(dinfo, lmb)				\
>> +	for_each_drmem_lmb_in_range((lmb),			\
>> +		&dinfo->lmbs[0],				\
>> +		&dinfo->lmbs[dinfo->n_lmbs - 1])
>> +
>> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
>> +	for ((lmb1) = (&dinfo1->lmbs[0]),			\
>> +	     (lmb2) = (&dinfo2->lmbs[0]);			\
>> +	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
>> +	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
>> +	     (lmb1)++, (lmb2)++)
>> +
>>  /*
>>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
>>   * specified in the ibm,dynamic-memory device tree property.
>> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>>  int drmem_update_dt(void);
>>
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
>> +
>>  #ifdef CONFIG_PPC_PSERIES
>>  void __init walk_drmem_lmbs_early(unsigned long node,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>> index 3f18036..13d2abb 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -20,6 +20,7 @@
>>
>>  static struct drmem_lmb_info __drmem_info;
>>  struct drmem_lmb_info *drmem_info = &__drmem_info;
>> +static int n_root_addr_cells;
> 
> What is the point of this new global? I see two places were it gets initialized if it is null, and both those initializers simply set it to "dt_root_addr_cells". I also checked the rest of the patches in the series and none of those even reference this variable.

The point of these changes is to introduce code that can be used after
an LPAR migration.  The variable dt_root_addr_cells was observed to be
zero (0) after migration on powerpc.  As it is defined as,

./drivers/of/fdt.c:int __initdata dt_root_addr_cells;

its definition may be replaced by other code/variables after initial boot.
We needed the value to persist, and it was cheaper to cache it.

> 
>>
>>  u64 drmem_lmb_memory_max(void)
>>  {
>> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>>  	return rc;
>>  }
>>
>> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>>
>> -	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +	lmb->base_addr = of_read_number(p, n_root_addr_cells);
>> +	p += n_root_addr_cells;
> 
> Unnecessary code churn do to the introduction of "n_root_addr_cells".'

See note above.

> 
>>  	lmb->drc_index = of_read_number(p++, 1);
>>
>>  	p++; /* skip reserved field */
>> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>>  	*prop = p;
>>  }
>>
>> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct drmem_lmb lmb;
>> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>  	}
>>  }
>>
>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>>
>>  	dr_cell->seq_lmbs = of_read_number(p++, 1);
>> -	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
>> +	p += n_root_addr_cells;
> 
> Same comment as above.

See note above.

> 
>>  	dr_cell->drc_index = of_read_number(p++, 1);
>>  	dr_cell->aa_index = of_read_number(p++, 1);
>>  	dr_cell->flags = of_read_number(p++, 1);
>> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  	*prop = p;
>>  }
>>
>> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>>  	const __be32 *prop, *usm;
>>  	int len;
>>
>> +	if (n_root_addr_cells == 0)
>> +		n_root_addr_cells = dt_root_addr_cells;
>> +
> 
> As I mentioned initially whats the point? Why not just use "dt_root_addr_cells"?

See note above.

> 
>>  	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>>  	if (!prop || len < dt_root_size_cells * sizeof(__be32))
>>  		return;
>> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  	}
>>  }
>>
>> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
>> +static void init_drmem_v1_lmbs(const __be32 *prop,
>> +				struct drmem_lmb_info *dinfo)
>>  {
>>  	struct drmem_lmb *lmb;
>>
>> -	drmem_info->n_lmbs = of_read_number(prop++, 1);
>> -	if (drmem_info->n_lmbs == 0)
>> +	dinfo->n_lmbs = of_read_number(prop++, 1);
>> +	if (dinfo->n_lmbs == 0)
>>  		return;
>>
>> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>>  				   GFP_KERNEL);
>> -	if (!drmem_info->lmbs)
>> +	if (!dinfo->lmbs)
>>  		return;
>>
>> -	for_each_drmem_lmb(lmb)
>> +	for_each_dinfo_lmb(dinfo, lmb)
>>  		read_drconf_v1_cell(lmb, &prop);
>>  }
>>
>> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> +static void init_drmem_v2_lmbs(const __be32 *prop,
>> +				struct drmem_lmb_info *dinfo)
>>  {
>>  	struct drmem_lmb *lmb;
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  	p = prop;
>>  	for (i = 0; i < lmb_sets; i++) {
>>  		read_drconf_v2_cell(&dr_cell, &p);
>> -		drmem_info->n_lmbs += dr_cell.seq_lmbs;
>> +		dinfo->n_lmbs += dr_cell.seq_lmbs;
>>  	}
>>
>> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>>  				   GFP_KERNEL);
>> -	if (!drmem_info->lmbs)
>> +	if (!dinfo->lmbs)
>>  		return;
>>
>>  	/* second pass, read in the LMB information */
>> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  		read_drconf_v2_cell(&dr_cell, &p);
>>
>>  		for (j = 0; j < dr_cell.seq_lmbs; j++) {
>> -			lmb = &drmem_info->lmbs[lmb_index++];
>> +			lmb = &dinfo->lmbs[lmb_index++];
>>
>>  			lmb->base_addr = dr_cell.base_addr;
>> -			dr_cell.base_addr += drmem_info->lmb_size;
>> +			dr_cell.base_addr += dinfo->lmb_size;
>>
>>  			lmb->drc_index = dr_cell.drc_index;
>>  			dr_cell.drc_index++;
>> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  	}
>>  }
>>
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
>> +{
>> +	if (dinfo) {
>> +		kfree(dinfo->lmbs);
>> +		kfree(dinfo);
>> +	}
>> +}
>> +
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
>> +{
>> +	struct drmem_lmb_info *dinfo;
>> +
>> +	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
>> +	if (!dinfo)
>> +		return NULL;
>> +
>> +	if (!strcmp("ibm,dynamic-memory", prop->name))
>> +		init_drmem_v1_lmbs(prop->value, dinfo);
>> +	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
>> +		init_drmem_v2_lmbs(prop->value, dinfo);
>> +
>> +	return dinfo;
>> +}
>> +
>>  static int __init drmem_init(void)
>>  {
>>  	struct device_node *dn;
>>  	const __be32 *prop;
>>
>> +	if (n_root_addr_cells == 0)
>> +		n_root_addr_cells = dt_root_addr_cells;
>> +
> 
> See previous comment.

See note above.

> 
> -Tyrel

Michael

> 
>>  	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>>  	if (!dn) {
>>  		pr_info("No dynamic reconfiguration memory found\n");
>> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
>>
>>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>  	if (prop) {
>> -		init_drmem_v1_lmbs(prop);
>> +		init_drmem_v1_lmbs(prop, drmem_info);
>>  	} else {
>>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>>  		if (prop)
>> -			init_drmem_v2_lmbs(prop);
>> +			init_drmem_v2_lmbs(prop, drmem_info);
>>  	}
>>
>>  	of_node_put(dn);
>>
> 
> 

-- 
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: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader

[Michael Ellerman]

Michael Bringmann <mwb@linux.vnet.ibm.com> writes:

> powerpc/drmem: Export many of the functions of DRMEM to parse
> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
> operations and for Post Migration events.

This isn't a criticism of your patch, but I think the drmem.c code
should be moved into platforms/pseries.

That would then make most of it private to platforms/pseries and we
wouldn't need to export things in arch/powerpc/include/asm.


> Also modify the DRMEM initialization code to allow it to,
>
> * Be called after system initialization
> * Provide a separate user copy of the LMB array that is produces
> * Free the user copy upon request

Is there any reason those can't be done as separate patches?

> In addition, a couple of changes were made to make the creation
> of additional copies of the LMB array more useful including,
>
> * Add new iterator to work through a pair of drmem_info arrays.
> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>   dt_mem_next_cell, as these are only available at first boot.

Likewise?

cheers

> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index ce242b9..b0e70fd 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>  		&drmem_info->lmbs[0],				\
>  		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
>  
> +#define for_each_dinfo_lmb(dinfo, lmb)				\
> +	for_each_drmem_lmb_in_range((lmb),			\
> +		&dinfo->lmbs[0],				\
> +		&dinfo->lmbs[dinfo->n_lmbs - 1])
> +
> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
> +	for ((lmb1) = (&dinfo1->lmbs[0]),			\
> +	     (lmb2) = (&dinfo2->lmbs[0]);			\
> +	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
> +	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
> +	     (lmb1)++, (lmb2)++)
> +
>  /*
>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
>   * specified in the ibm,dynamic-memory device tree property.
> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
>  int drmem_update_dt(void);
>  
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
> +
>  #ifdef CONFIG_PPC_PSERIES
>  void __init walk_drmem_lmbs_early(unsigned long node,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 3f18036..13d2abb 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -20,6 +20,7 @@
>  
>  static struct drmem_lmb_info __drmem_info;
>  struct drmem_lmb_info *drmem_info = &__drmem_info;
> +static int n_root_addr_cells;
>  
>  u64 drmem_lmb_memory_max(void)
>  {
> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>  	return rc;
>  }
>  
> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
>  
> -	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> +	lmb->base_addr = of_read_number(p, n_root_addr_cells);
> +	p += n_root_addr_cells;
>  	lmb->drc_index = of_read_number(p++, 1);
>  
>  	p++; /* skip reserved field */
> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>  	*prop = p;
>  }
>  
> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct drmem_lmb lmb;
> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>  	}
>  }
>  
> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
>  
>  	dr_cell->seq_lmbs = of_read_number(p++, 1);
> -	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> +	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
> +	p += n_root_addr_cells;
>  	dr_cell->drc_index = of_read_number(p++, 1);
>  	dr_cell->aa_index = of_read_number(p++, 1);
>  	dr_cell->flags = of_read_number(p++, 1);
> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  	*prop = p;
>  }
>  
> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>  	const __be32 *prop, *usm;
>  	int len;
>  
> +	if (n_root_addr_cells == 0)
> +		n_root_addr_cells = dt_root_addr_cells;
> +
>  	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>  	if (!prop || len < dt_root_size_cells * sizeof(__be32))
>  		return;
> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  	}
>  }
>  
> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
> +static void init_drmem_v1_lmbs(const __be32 *prop,
> +				struct drmem_lmb_info *dinfo)
>  {
>  	struct drmem_lmb *lmb;
>  
> -	drmem_info->n_lmbs = of_read_number(prop++, 1);
> -	if (drmem_info->n_lmbs == 0)
> +	dinfo->n_lmbs = of_read_number(prop++, 1);
> +	if (dinfo->n_lmbs == 0)
>  		return;
>  
> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>  				   GFP_KERNEL);
> -	if (!drmem_info->lmbs)
> +	if (!dinfo->lmbs)
>  		return;
>  
> -	for_each_drmem_lmb(lmb)
> +	for_each_dinfo_lmb(dinfo, lmb)
>  		read_drconf_v1_cell(lmb, &prop);
>  }
>  
> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
> +static void init_drmem_v2_lmbs(const __be32 *prop,
> +				struct drmem_lmb_info *dinfo)
>  {
>  	struct drmem_lmb *lmb;
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  	p = prop;
>  	for (i = 0; i < lmb_sets; i++) {
>  		read_drconf_v2_cell(&dr_cell, &p);
> -		drmem_info->n_lmbs += dr_cell.seq_lmbs;
> +		dinfo->n_lmbs += dr_cell.seq_lmbs;
>  	}
>  
> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>  				   GFP_KERNEL);
> -	if (!drmem_info->lmbs)
> +	if (!dinfo->lmbs)
>  		return;
>  
>  	/* second pass, read in the LMB information */
> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  		read_drconf_v2_cell(&dr_cell, &p);
>  
>  		for (j = 0; j < dr_cell.seq_lmbs; j++) {
> -			lmb = &drmem_info->lmbs[lmb_index++];
> +			lmb = &dinfo->lmbs[lmb_index++];
>  
>  			lmb->base_addr = dr_cell.base_addr;
> -			dr_cell.base_addr += drmem_info->lmb_size;
> +			dr_cell.base_addr += dinfo->lmb_size;
>  
>  			lmb->drc_index = dr_cell.drc_index;
>  			dr_cell.drc_index++;
> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  	}
>  }
>  
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
> +{
> +	if (dinfo) {
> +		kfree(dinfo->lmbs);
> +		kfree(dinfo);
> +	}
> +}
> +
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
> +{
> +	struct drmem_lmb_info *dinfo;
> +
> +	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
> +	if (!dinfo)
> +		return NULL;
> +
> +	if (!strcmp("ibm,dynamic-memory", prop->name))
> +		init_drmem_v1_lmbs(prop->value, dinfo);
> +	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
> +		init_drmem_v2_lmbs(prop->value, dinfo);
> +
> +	return dinfo;
> +}
> +
>  static int __init drmem_init(void)
>  {
>  	struct device_node *dn;
>  	const __be32 *prop;
>  
> +	if (n_root_addr_cells == 0)
> +		n_root_addr_cells = dt_root_addr_cells;
> +
>  	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>  	if (!dn) {
>  		pr_info("No dynamic reconfiguration memory found\n");
> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
>  
>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>  	if (prop) {
> -		init_drmem_v1_lmbs(prop);
> +		init_drmem_v1_lmbs(prop, drmem_info);
>  	} else {
>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>  		if (prop)
> -			init_drmem_v2_lmbs(prop);
> +			init_drmem_v2_lmbs(prop, drmem_info);
>  	}
>  
>  	of_node_put(dn);

Re: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader

[Nathan Fontenot]

On 10/02/2018 08:00 PM, Michael Ellerman wrote:
> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
> 
>> powerpc/drmem: Export many of the functions of DRMEM to parse
>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>> operations and for Post Migration events.
> 
> This isn't a criticism of your patch, but I think the drmem.c code
> should be moved into platforms/pseries.
> 
> That would then make most of it private to platforms/pseries and we
> wouldn't need to export things in arch/powerpc/include/asm.

I don't have an issue with moving it to platform/pseries. I originally
put it in arch/powerpc/mm because the numa code also uses the drmem code.

The numa code was updated so that it could just be given a lmb struct
instead of having to parse the device tree directly for dynamic
reconfiguration memory. Having to support two versions of this dt
property this made more sense.

-Nathan

> 
> 
>> Also modify the DRMEM initialization code to allow it to,
>>
>> * Be called after system initialization
>> * Provide a separate user copy of the LMB array that is produces
>> * Free the user copy upon request
> 
> Is there any reason those can't be done as separate patches?
> 
>> In addition, a couple of changes were made to make the creation
>> of additional copies of the LMB array more useful including,
>>
>> * Add new iterator to work through a pair of drmem_info arrays.
>> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>>   dt_mem_next_cell, as these are only available at first boot.
> 
> Likewise?
> 
> cheers
> 
>> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
>> index ce242b9..b0e70fd 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>>  		&drmem_info->lmbs[0],				\
>>  		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
>>  
>> +#define for_each_dinfo_lmb(dinfo, lmb)				\
>> +	for_each_drmem_lmb_in_range((lmb),			\
>> +		&dinfo->lmbs[0],				\
>> +		&dinfo->lmbs[dinfo->n_lmbs - 1])
>> +
>> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
>> +	for ((lmb1) = (&dinfo1->lmbs[0]),			\
>> +	     (lmb2) = (&dinfo2->lmbs[0]);			\
>> +	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
>> +	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
>> +	     (lmb1)++, (lmb2)++)
>> +
>>  /*
>>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
>>   * specified in the ibm,dynamic-memory device tree property.
>> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>>  int drmem_update_dt(void);
>>  
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
>> +
>>  #ifdef CONFIG_PPC_PSERIES
>>  void __init walk_drmem_lmbs_early(unsigned long node,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>> index 3f18036..13d2abb 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -20,6 +20,7 @@
>>  
>>  static struct drmem_lmb_info __drmem_info;
>>  struct drmem_lmb_info *drmem_info = &__drmem_info;
>> +static int n_root_addr_cells;
>>  
>>  u64 drmem_lmb_memory_max(void)
>>  {
>> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>>  	return rc;
>>  }
>>  
>> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>>  
>> -	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +	lmb->base_addr = of_read_number(p, n_root_addr_cells);
>> +	p += n_root_addr_cells;
>>  	lmb->drc_index = of_read_number(p++, 1);
>>  
>>  	p++; /* skip reserved field */
>> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>>  	*prop = p;
>>  }
>>  
>> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct drmem_lmb lmb;
>> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>  	}
>>  }
>>  
>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>>  
>>  	dr_cell->seq_lmbs = of_read_number(p++, 1);
>> -	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
>> +	p += n_root_addr_cells;
>>  	dr_cell->drc_index = of_read_number(p++, 1);
>>  	dr_cell->aa_index = of_read_number(p++, 1);
>>  	dr_cell->flags = of_read_number(p++, 1);
>> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  	*prop = p;
>>  }
>>  
>> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>>  	const __be32 *prop, *usm;
>>  	int len;
>>  
>> +	if (n_root_addr_cells == 0)
>> +		n_root_addr_cells = dt_root_addr_cells;
>> +
>>  	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>>  	if (!prop || len < dt_root_size_cells * sizeof(__be32))
>>  		return;
>> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  	}
>>  }
>>  
>> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
>> +static void init_drmem_v1_lmbs(const __be32 *prop,
>> +				struct drmem_lmb_info *dinfo)
>>  {
>>  	struct drmem_lmb *lmb;
>>  
>> -	drmem_info->n_lmbs = of_read_number(prop++, 1);
>> -	if (drmem_info->n_lmbs == 0)
>> +	dinfo->n_lmbs = of_read_number(prop++, 1);
>> +	if (dinfo->n_lmbs == 0)
>>  		return;
>>  
>> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>>  				   GFP_KERNEL);
>> -	if (!drmem_info->lmbs)
>> +	if (!dinfo->lmbs)
>>  		return;
>>  
>> -	for_each_drmem_lmb(lmb)
>> +	for_each_dinfo_lmb(dinfo, lmb)
>>  		read_drconf_v1_cell(lmb, &prop);
>>  }
>>  
>> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> +static void init_drmem_v2_lmbs(const __be32 *prop,
>> +				struct drmem_lmb_info *dinfo)
>>  {
>>  	struct drmem_lmb *lmb;
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  	p = prop;
>>  	for (i = 0; i < lmb_sets; i++) {
>>  		read_drconf_v2_cell(&dr_cell, &p);
>> -		drmem_info->n_lmbs += dr_cell.seq_lmbs;
>> +		dinfo->n_lmbs += dr_cell.seq_lmbs;
>>  	}
>>  
>> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>>  				   GFP_KERNEL);
>> -	if (!drmem_info->lmbs)
>> +	if (!dinfo->lmbs)
>>  		return;
>>  
>>  	/* second pass, read in the LMB information */
>> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  		read_drconf_v2_cell(&dr_cell, &p);
>>  
>>  		for (j = 0; j < dr_cell.seq_lmbs; j++) {
>> -			lmb = &drmem_info->lmbs[lmb_index++];
>> +			lmb = &dinfo->lmbs[lmb_index++];
>>  
>>  			lmb->base_addr = dr_cell.base_addr;
>> -			dr_cell.base_addr += drmem_info->lmb_size;
>> +			dr_cell.base_addr += dinfo->lmb_size;
>>  
>>  			lmb->drc_index = dr_cell.drc_index;
>>  			dr_cell.drc_index++;
>> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  	}
>>  }
>>  
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
>> +{
>> +	if (dinfo) {
>> +		kfree(dinfo->lmbs);
>> +		kfree(dinfo);
>> +	}
>> +}
>> +
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
>> +{
>> +	struct drmem_lmb_info *dinfo;
>> +
>> +	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
>> +	if (!dinfo)
>> +		return NULL;
>> +
>> +	if (!strcmp("ibm,dynamic-memory", prop->name))
>> +		init_drmem_v1_lmbs(prop->value, dinfo);
>> +	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
>> +		init_drmem_v2_lmbs(prop->value, dinfo);
>> +
>> +	return dinfo;
>> +}
>> +
>>  static int __init drmem_init(void)
>>  {
>>  	struct device_node *dn;
>>  	const __be32 *prop;
>>  
>> +	if (n_root_addr_cells == 0)
>> +		n_root_addr_cells = dt_root_addr_cells;
>> +
>>  	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>>  	if (!dn) {
>>  		pr_info("No dynamic reconfiguration memory found\n");
>> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
>>  
>>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>  	if (prop) {
>> -		init_drmem_v1_lmbs(prop);
>> +		init_drmem_v1_lmbs(prop, drmem_info);
>>  	} else {
>>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>>  		if (prop)
>> -			init_drmem_v2_lmbs(prop);
>> +			init_drmem_v2_lmbs(prop, drmem_info);
>>  	}
>>  
>>  	of_node_put(dn);
> 

Re: [PATCH v03 1/5] powerpc/drmem: Export 'dynamic-memory' loader

[Michael Ellerman]

Nathan Fontenot <nfont@linux.vnet.ibm.com> writes:

> On 10/02/2018 08:00 PM, Michael Ellerman wrote:
>> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>> 
>>> powerpc/drmem: Export many of the functions of DRMEM to parse
>>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>>> operations and for Post Migration events.
>> 
>> This isn't a criticism of your patch, but I think the drmem.c code
>> should be moved into platforms/pseries.
>> 
>> That would then make most of it private to platforms/pseries and we
>> wouldn't need to export things in arch/powerpc/include/asm.
>
> I don't have an issue with moving it to platform/pseries. I originally
> put it in arch/powerpc/mm because the numa code also uses the drmem code.

Yeah, originally the NUMA code was only used on pseries so the
distinction between NUMA and pseries-specific NUMA didn't exist.

But these days we're getting more and more code that is really pseries
(or PAPR) specific, so it might make sense to move it.

I may be wrong, perhaps there isn't a clean split there, but it would be
worth trying.

cheers

[PATCH v03 2/5] powerpc/drmem: Add internal_flags feature

[Michael Bringmann]

powerpc/drmem: Add internal_flags field to each LMB to allow
marking of kernel software-specific operations that need not
be exported to other users.  For instance, if information about
selected LMBs needs to be maintained for subsequent passes
through the system, it can be encoded into the LMB array itself
without requiring the allocation and maintainance of additional
data structures.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/drmem.h |   18 ++++++++++++++++++
 arch/powerpc/mm/drmem.c          |    2 ++
 2 files changed, 20 insertions(+)

diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index b0e70fd..acb6539 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -17,6 +17,7 @@ struct drmem_lmb {
 	u32     drc_index;
 	u32     aa_index;
 	u32     flags;
+	u32     internal_flags;
 };
 
 struct drmem_lmb_info {
@@ -101,6 +102,23 @@ static inline bool drmem_lmb_reserved(struct drmem_lmb *lmb)
 	return lmb->flags & DRMEM_LMB_RESERVED;
 }
 
+#define DRMEM_LMBINT_UPDATE	0x00000001
+
+static inline void drmem_mark_lmb_update(struct drmem_lmb *lmb)
+{
+	lmb->internal_flags |= DRMEM_LMBINT_UPDATE;
+}
+
+static inline void drmem_remove_lmb_update(struct drmem_lmb *lmb)
+{
+	lmb->internal_flags &= ~DRMEM_LMBINT_UPDATE;
+}
+
+static inline bool drmem_lmb_update(struct drmem_lmb *lmb)
+{
+	return lmb->internal_flags & DRMEM_LMBINT_UPDATE;
+}
+
 u64 drmem_lmb_memory_max(void);
 void __init walk_drmem_lmbs(struct device_node *dn,
 			void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 13d2abb..fd2cae92 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -207,6 +207,7 @@ static void read_drconf_v1_cell(struct drmem_lmb *lmb,
 
 	lmb->aa_index = of_read_number(p++, 1);
 	lmb->flags = of_read_number(p++, 1);
+	lmb->internal_flags = 0;
 
 	*prop = p;
 }
@@ -265,6 +266,7 @@ static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
 
 			lmb.aa_index = dr_cell.aa_index;
 			lmb.flags = dr_cell.flags;
+			lmb.internal_flags = 0;
 
 			func(&lmb, &usm);
 		}

[PATCH v03 3/5] migration/memory: Add hotplug READD_MULTIPLE

[Michael Bringmann]

migration/memory: This patch adds a new pseries hotplug action
for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
This is a variant of the READD operation which performs the action
upon multiple instances of the resource at one time.  The operation
is to be triggered by device-tree analysis of updates by RTAS events
analyzed by 'migation_store' during post-migration processing.  It
will be used for memory updates, initially.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/rtas.h |    1 +
 arch/powerpc/mm/drmem.c         |    1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 71e393c..e510d82 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -320,6 +320,7 @@ struct pseries_hp_errorlog {
 #define PSERIES_HP_ELOG_ACTION_ADD	1
 #define PSERIES_HP_ELOG_ACTION_REMOVE	2
 #define PSERIES_HP_ELOG_ACTION_READD	3
+#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE	4
 
 #define PSERIES_HP_ELOG_ID_DRC_NAME	1
 #define PSERIES_HP_ELOG_ID_DRC_INDEX	2
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index fd2cae92..2228586 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -422,6 +422,7 @@ static void init_drmem_v2_lmbs(const __be32 *prop,
 
 			lmb->aa_index = dr_cell.aa_index;
 			lmb->flags = dr_cell.flags;
+			lmb->internal_flags = 0;
 		}
 	}
 }

Re: [PATCH v03 3/5] migration/memory: Add hotplug READD_MULTIPLE

[Tyrel Datwyler]

On 10/01/2018 05:59 AM, Michael Bringmann wrote:
> migration/memory: This patch adds a new pseries hotplug action
> for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
> This is a variant of the READD operation which performs the action
> upon multiple instances of the resource at one time.  The operation
> is to be triggered by device-tree analysis of updates by RTAS events
> analyzed by 'migation_store' during post-migration processing.  It
> will be used for memory updates, initially.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/rtas.h |    1 +
>  arch/powerpc/mm/drmem.c         |    1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 71e393c..e510d82 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -320,6 +320,7 @@ struct pseries_hp_errorlog {
>  #define PSERIES_HP_ELOG_ACTION_ADD	1
>  #define PSERIES_HP_ELOG_ACTION_REMOVE	2
>  #define PSERIES_HP_ELOG_ACTION_READD	3
> +#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE	4

I'm confused, you have only added a define and not the actual implementation. I really think this should be squashed into your 4th patch where the operation is actually implemented.

> 
>  #define PSERIES_HP_ELOG_ID_DRC_NAME	1
>  #define PSERIES_HP_ELOG_ID_DRC_INDEX	2
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index fd2cae92..2228586 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -422,6 +422,7 @@ static void init_drmem_v2_lmbs(const __be32 *prop,
> 
>  			lmb->aa_index = dr_cell.aa_index;
>  			lmb->flags = dr_cell.flags;
> +			lmb->internal_flags = 0;

And this should have been squashed into the previous patch where you added the internal_flags field to the lmb struct.

-Tyrel 

>  		}
>  	}
>  }
> 

Re: [PATCH v03 3/5] migration/memory: Add hotplug READD_MULTIPLE

[Michael Bringmann]

On 10/02/2018 04:03 PM, Tyrel Datwyler wrote:
> On 10/01/2018 05:59 AM, Michael Bringmann wrote:
>> migration/memory: This patch adds a new pseries hotplug action
>> for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
>> This is a variant of the READD operation which performs the action
>> upon multiple instances of the resource at one time.  The operation
>> is to be triggered by device-tree analysis of updates by RTAS events
>> analyzed by 'migation_store' during post-migration processing.  It
>> will be used for memory updates, initially.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/rtas.h |    1 +
>>  arch/powerpc/mm/drmem.c         |    1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>> index 71e393c..e510d82 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -320,6 +320,7 @@ struct pseries_hp_errorlog {
>>  #define PSERIES_HP_ELOG_ACTION_ADD	1
>>  #define PSERIES_HP_ELOG_ACTION_REMOVE	2
>>  #define PSERIES_HP_ELOG_ACTION_READD	3
>> +#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE	4
> 
> I'm confused, you have only added a define and not the actual implementation. I really think this should be squashed into your 4th patch where the operation is actually implemented.

Okay.

> 
>>
>>  #define PSERIES_HP_ELOG_ID_DRC_NAME	1
>>  #define PSERIES_HP_ELOG_ID_DRC_INDEX	2
>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>> index fd2cae92..2228586 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -422,6 +422,7 @@ static void init_drmem_v2_lmbs(const __be32 *prop,
>>
>>  			lmb->aa_index = dr_cell.aa_index;
>>  			lmb->flags = dr_cell.flags;
>> +			lmb->internal_flags = 0;
> 
> And this should have been squashed into the previous patch where you added the internal_flags field to the lmb struct.

Okay.

> 
> -Tyrel 
> 
>>  		}
>>  	}
>>  }
>>
> 
> 

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

[PATCH v03 4/5] migration/memory: Evaluate LMB assoc changes

[Michael Bringmann]

migration/memory: This patch adds code that recognizes changes to
the associativity of memory blocks described by the device-tree
properties in order to drive equivalent 'hotplug' operations to
update local and general kernel data structures to reflect those
changes.  These differences may include:

* Evaluate 'ibm,dynamic-memory' properties when processing the
  updated device-tree properties of the system during Post Migration
  events (migration_store).  The new functionality looks for changes
  to the aa_index values for each drc_index/LMB to identify any memory
  blocks that should be readded.

* In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
  property may change.  In the event that a row of the array differs,
  locate all assigned memory blocks with that 'aa_index' and 're-add'
  them to the system memory block data structures.  In the process of
  the 're-add', the system routines will update the corresponding entry
  for the memory in the LMB structures and any other relevant kernel
  data structures.

A number of previous extensions made to the DRMEM code for scanning
device-tree properties and creating LMB arrays are used here to
ensure that the resulting code is simpler and more usable:

* Use new paired list iterator for the DRMEM LMB info arrays to find
  differences in old and new versions of properties.
* Use new iterator for copies of the DRMEM info arrays to evaluate
  completely new structures.
* Combine common code for parsing and evaluating memory description
  properties based on the DRMEM LMB array model to greatly simplify
  extension from the older property 'ibm,dynamic-memory' to the new
  property model of 'ibm,dynamic-memory-v2'.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v03:
  -- Modify the code that parses the memory affinity attributes to
     mark relevant DRMEM LMB array entries using the internal_flags
     mechanism instead of generate unique hotplug actions for each
     memory block to be readded.  The change is intended to both
     simplify the code, and to require fewer resources on systems
     with huge amounts of memory.
  -- Save up notice about any all LMB entries until the end of the
     'migration_store' operation at which point a single action is
     queued to scan the entire DRMEM array.
  -- Add READD_MULTIPLE function for memory that scans the DRMEM
     array to identify multiple entries that were marked previously.
     The corresponding memory blocks are to be readded to the system
     to update relevant data structures outside of the powerpc-
     specific code.
  -- Change dlpar_memory_pmt_changes_action to directly queue worker
     to pseries work queue.
---
 arch/powerpc/platforms/pseries/hotplug-memory.c |  220 +++++++++++++++++++----
 arch/powerpc/platforms/pseries/mobility.c       |    4 
 arch/powerpc/platforms/pseries/pseries.h        |    4 
 3 files changed, 194 insertions(+), 34 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c1578f5..68bde2e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -561,8 +561,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
 		}
 	}
 
-	if (!lmb_found)
-		rc = -EINVAL;
+	if (!lmb_found) {
+		pr_info("Failed to update memory for drc index %lx\n",
+			(unsigned long) drc_index);
+		return -EINVAL;
+	}
 
 	if (rc)
 		pr_info("Failed to update memory at %llx\n",
@@ -573,6 +576,30 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
 	return rc;
 }
 
+static int dlpar_memory_readd_multiple(void)
+{
+	struct drmem_lmb *lmb;
+	int rc;
+
+	pr_info("Attempting to update multiple LMBs\n");
+
+	for_each_drmem_lmb(lmb) {
+		if (drmem_lmb_update(lmb)) {
+			rc = dlpar_remove_lmb(lmb);
+
+			if (!rc) {
+				rc = dlpar_add_lmb(lmb);
+				if (rc)
+					dlpar_release_drc(lmb->drc_index);
+			}
+
+			drmem_remove_lmb_update(lmb);
+		}
+	}
+
+	return rc;
+}
+
 static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
 {
 	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
@@ -673,6 +700,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
 {
 	return -EOPNOTSUPP;
 }
+static int dlpar_memory_readd_multiple(void)
+{
+	return -EOPNOTSUPP;
+}
 
 static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
 {
@@ -952,6 +983,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
 		drc_index = hp_elog->_drc_u.drc_index;
 		rc = dlpar_memory_readd_by_index(drc_index);
 		break;
+	case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
+		rc = dlpar_memory_readd_multiple();
+		break;
 	default:
 		pr_err("Invalid action (%d) specified\n", hp_elog->action);
 		rc = -EINVAL;
@@ -994,13 +1028,43 @@ static int pseries_add_mem_node(struct device_node *np)
 	return (ret < 0) ? -EINVAL : 0;
 }
 
-static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
+static int pmt_changes = 0;
+
+void dlpar_memory_pmt_changes_set(void)
+{
+	pmt_changes = 1;
+}
+
+void dlpar_memory_pmt_changes_clear(void)
+{
+	pmt_changes = 0;
+}
+
+int dlpar_memory_pmt_changes(void)
+{
+	return pmt_changes;
+}
+
+void dlpar_memory_pmt_changes_action(void)
+{
+	if (dlpar_memory_pmt_changes()) {
+		struct pseries_hp_errorlog hp_errlog;
+
+        	hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
+        	hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
+        	hp_errlog.id_type = 0;
+
+        	queue_hotplug_event(&hp_errlog, NULL, NULL);
+
+		dlpar_memory_pmt_changes_clear();
+	}
+}
+
+static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
 {
-	struct of_drconf_cell_v1 *new_drmem, *old_drmem;
+	struct drmem_lmb *old_lmb, *new_lmb;
 	unsigned long memblock_size;
-	u32 entries;
-	__be32 *p;
-	int i, rc = -EINVAL;
+	int rc = 0;
 
 	if (rtas_hp_event)
 		return 0;
@@ -1009,42 +1073,122 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
 	if (!memblock_size)
 		return -EINVAL;
 
-	p = (__be32 *) pr->old_prop->value;
-	if (!p)
-		return -EINVAL;
-
-	/* The first int of the property is the number of lmb's described
-	 * by the property. This is followed by an array of of_drconf_cell
-	 * entries. Get the number of entries and skip to the array of
-	 * of_drconf_cell's.
-	 */
-	entries = be32_to_cpu(*p++);
-	old_drmem = (struct of_drconf_cell_v1 *)p;
+	/* Arrays should have the same size and DRC indexes */
+	for_each_pair_dinfo_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
 
-	p = (__be32 *)pr->prop->value;
-	p++;
-	new_drmem = (struct of_drconf_cell_v1 *)p;
+		if (new_lmb->drc_index != old_lmb->drc_index)
+			continue;
 
-	for (i = 0; i < entries; i++) {
-		if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
-		    (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
+		if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
+		    (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
 			rc = pseries_remove_memblock(
-				be64_to_cpu(old_drmem[i].base_addr),
-						     memblock_size);
+				old_lmb->base_addr, memblock_size);
 			break;
-		} else if ((!(be32_to_cpu(old_drmem[i].flags) &
-			    DRCONF_MEM_ASSIGNED)) &&
-			    (be32_to_cpu(new_drmem[i].flags) &
-			    DRCONF_MEM_ASSIGNED)) {
-			rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
-					  memblock_size);
+		} else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
+			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+			rc = memblock_add(old_lmb->base_addr,
+					memblock_size);
 			rc = (rc < 0) ? -EINVAL : 0;
 			break;
+		} else if ((old_lmb->aa_index != new_lmb->aa_index) &&
+			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+			drmem_mark_lmb_update(old_lmb);
+			dlpar_memory_pmt_changes_set();
 		}
 	}
 	return rc;
 }
 
+static void pseries_update_ala_memory_aai(int aa_index)
+{
+	struct drmem_lmb *lmb;
+
+	/* Readd all LMBs which were previously using the
+	 * specified aa_index value.
+	 */
+	for_each_drmem_lmb(lmb) {
+		if ((lmb->aa_index == aa_index) &&
+			(lmb->flags & DRCONF_MEM_ASSIGNED)) {
+			drmem_mark_lmb_update(lmb);
+			dlpar_memory_pmt_changes_set();
+		}
+	}
+}
+
+struct assoc_arrays {
+	u32 n_arrays;
+	u32 array_sz;
+	const __be32 *arrays;
+};
+
+static int pseries_update_ala_memory(struct of_reconfig_data *pr)
+{
+	struct assoc_arrays new_ala, old_ala;
+	__be32 *p;
+	int i, lim;
+
+	if (rtas_hp_event)
+		return 0;
+
+	/*
+	 * The layout of the ibm,associativity-lookup-arrays
+	 * property is a number N indicating the number of
+	 * associativity arrays, followed by a number M
+	 * indicating the size of each associativity array,
+	 * followed by a list of N associativity arrays.
+	 */
+
+	p = (__be32 *) pr->old_prop->value;
+	if (!p)
+		return -EINVAL;
+	old_ala.n_arrays = of_read_number(p++, 1);
+	old_ala.array_sz = of_read_number(p++, 1);
+	old_ala.arrays = p;
+
+	p = (__be32 *) pr->prop->value;
+	if (!p)
+		return -EINVAL;
+	new_ala.n_arrays = of_read_number(p++, 1);
+	new_ala.array_sz = of_read_number(p++, 1);
+	new_ala.arrays = p;
+
+	lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
+			new_ala.n_arrays;
+
+	if (old_ala.array_sz == new_ala.array_sz) {
+
+		/* Reset any entries where the old and new rows
+		 * the array have changed.
+		 */
+		for (i = 0; i < lim; i++) {
+			int index = (i * new_ala.array_sz);
+
+			if (!memcmp(&old_ala.arrays[index],
+				&new_ala.arrays[index],
+				new_ala.array_sz))
+				continue;
+
+			pseries_update_ala_memory_aai(i);
+		}
+
+		/* Reset any entries representing the extra rows.
+		 * There shouldn't be any, but just in case ...
+		 */
+		for (i = lim; i < new_ala.n_arrays; i++)
+			pseries_update_ala_memory_aai(i);
+
+	} else {
+		/* Update all entries representing these rows;
+		 * as all rows have different sizes, none can
+		 * have equivalent values.
+		 */
+		for (i = 0; i < lim; i++)
+			pseries_update_ala_memory_aai(i);
+	}
+
+	return 0;
+}
+
 static int pseries_memory_notifier(struct notifier_block *nb,
 				   unsigned long action, void *data)
 {
@@ -1059,8 +1203,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
 		err = pseries_remove_mem_node(rd->dn);
 		break;
 	case OF_RECONFIG_UPDATE_PROPERTY:
-		if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
-			err = pseries_update_drconf_memory(rd);
+		if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+			struct drmem_lmb_info *dinfo =
+				drmem_lmbs_init(rd->prop);
+			if (!dinfo)
+				return -EINVAL;
+			err = pseries_update_drconf_memory(dinfo);
+			drmem_lmbs_free(dinfo);
+		} else if (!strcmp(rd->prop->name,
+				"ibm,associativity-lookup-arrays"))
+			err = pseries_update_ala_memory(rd);
 		break;
 	}
 	return notifier_from_errno(err);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 23fb9ac..6c0456f 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -383,6 +383,10 @@ static ssize_t migration_store(struct class *class,
 		return rc;
 
 	post_mobility_fixup();
+
+	/* Apply any necessary changes identified during fixup */
+	dlpar_memory_pmt_changes_action();
+
 	return count;
 }
 
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 60db2ee..b238634 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -69,6 +69,10 @@ static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
 	return -EOPNOTSUPP;
 }
 #endif
+void dlpar_memory_pmt_changes_set(void);
+void dlpar_memory_pmt_changes_clear(void);
+int dlpar_memory_pmt_changes(void);
+void dlpar_memory_pmt_changes_action(void);
 
 #ifdef CONFIG_HOTPLUG_CPU
 int dlpar_cpu(struct pseries_hp_errorlog *hp_elog);

Re: [PATCH v03 4/5] migration/memory: Evaluate LMB assoc changes

[Tyrel Datwyler]

On 10/01/2018 06:00 AM, Michael Bringmann wrote:
> migration/memory: This patch adds code that recognizes changes to
> the associativity of memory blocks described by the device-tree
> properties in order to drive equivalent 'hotplug' operations to
> update local and general kernel data structures to reflect those
> changes.  These differences may include:
> 
> * Evaluate 'ibm,dynamic-memory' properties when processing the
>   updated device-tree properties of the system during Post Migration
>   events (migration_store).  The new functionality looks for changes
>   to the aa_index values for each drc_index/LMB to identify any memory
>   blocks that should be readded.
> 
> * In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
>   property may change.  In the event that a row of the array differs,
>   locate all assigned memory blocks with that 'aa_index' and 're-add'
>   them to the system memory block data structures.  In the process of
>   the 're-add', the system routines will update the corresponding entry
>   for the memory in the LMB structures and any other relevant kernel
>   data structures.
> 
> A number of previous extensions made to the DRMEM code for scanning
> device-tree properties and creating LMB arrays are used here to
> ensure that the resulting code is simpler and more usable:
> 
> * Use new paired list iterator for the DRMEM LMB info arrays to find
>   differences in old and new versions of properties.
> * Use new iterator for copies of the DRMEM info arrays to evaluate
>   completely new structures.
> * Combine common code for parsing and evaluating memory description
>   properties based on the DRMEM LMB array model to greatly simplify
>   extension from the older property 'ibm,dynamic-memory' to the new
>   property model of 'ibm,dynamic-memory-v2'.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in v03:
>   -- Modify the code that parses the memory affinity attributes to
>      mark relevant DRMEM LMB array entries using the internal_flags
>      mechanism instead of generate unique hotplug actions for each
>      memory block to be readded.  The change is intended to both
>      simplify the code, and to require fewer resources on systems
>      with huge amounts of memory.
>   -- Save up notice about any all LMB entries until the end of the
>      'migration_store' operation at which point a single action is
>      queued to scan the entire DRMEM array.
>   -- Add READD_MULTIPLE function for memory that scans the DRMEM
>      array to identify multiple entries that were marked previously.
>      The corresponding memory blocks are to be readded to the system
>      to update relevant data structures outside of the powerpc-
>      specific code.
>   -- Change dlpar_memory_pmt_changes_action to directly queue worker
>      to pseries work queue.
> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c |  220 +++++++++++++++++++----
>  arch/powerpc/platforms/pseries/mobility.c       |    4 
>  arch/powerpc/platforms/pseries/pseries.h        |    4 
>  3 files changed, 194 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f5..68bde2e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -561,8 +561,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>  		}
>  	}
> 
> -	if (!lmb_found)
> -		rc = -EINVAL;
> +	if (!lmb_found) {
> +		pr_info("Failed to update memory for drc index %lx\n",
> +			(unsigned long) drc_index);
> +		return -EINVAL;
> +	}
> 
>  	if (rc)
>  		pr_info("Failed to update memory at %llx\n",
> @@ -573,6 +576,30 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>  	return rc;
>  }
> 
> +static int dlpar_memory_readd_multiple(void)
> +{
> +	struct drmem_lmb *lmb;
> +	int rc;
> +
> +	pr_info("Attempting to update multiple LMBs\n");
> +
> +	for_each_drmem_lmb(lmb) {
> +		if (drmem_lmb_update(lmb)) {
> +			rc = dlpar_remove_lmb(lmb);
> +
> +			if (!rc) {
> +				rc = dlpar_add_lmb(lmb);
> +				if (rc)
> +					dlpar_release_drc(lmb->drc_index);
> +			}
> +
> +			drmem_remove_lmb_update(lmb);
> +		}
> +	}
> +
> +	return rc;
> +}
> +

Actually, in retrospect this function should have been independently implemented in your 3rd patch where you define the flag. So, disregard moving the define into this patch and instead move the function implementation into patch 3 where the commit message talks about adding this operation.

>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>  {
>  	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
> @@ -673,6 +700,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>  {
>  	return -EOPNOTSUPP;
>  }
> +static int dlpar_memory_readd_multiple(void)
> +{
> +	return -EOPNOTSUPP;
> +}

Same as above commit obviously.

> 
>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>  {
> @@ -952,6 +983,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>  		drc_index = hp_elog->_drc_u.drc_index;
>  		rc = dlpar_memory_readd_by_index(drc_index);
>  		break;
> +	case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
> +		rc = dlpar_memory_readd_multiple();
> +		break;

And this as well.

-Tyrel

>  	default:
>  		pr_err("Invalid action (%d) specified\n", hp_elog->action);
>  		rc = -EINVAL;
> @@ -994,13 +1028,43 @@ static int pseries_add_mem_node(struct device_node *np)
>  	return (ret < 0) ? -EINVAL : 0;
>  }
> 
> -static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
> +static int pmt_changes = 0;
> +
> +void dlpar_memory_pmt_changes_set(void)
> +{
> +	pmt_changes = 1;
> +}
> +
> +void dlpar_memory_pmt_changes_clear(void)
> +{
> +	pmt_changes = 0;
> +}
> +
> +int dlpar_memory_pmt_changes(void)
> +{
> +	return pmt_changes;
> +}
> +
> +void dlpar_memory_pmt_changes_action(void)
> +{
> +	if (dlpar_memory_pmt_changes()) {
> +		struct pseries_hp_errorlog hp_errlog;
> +
> +        	hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
> +        	hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
> +        	hp_errlog.id_type = 0;
> +
> +        	queue_hotplug_event(&hp_errlog, NULL, NULL);
> +
> +		dlpar_memory_pmt_changes_clear();
> +	}
> +}
> +
> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
>  {
> -	struct of_drconf_cell_v1 *new_drmem, *old_drmem;
> +	struct drmem_lmb *old_lmb, *new_lmb;
>  	unsigned long memblock_size;
> -	u32 entries;
> -	__be32 *p;
> -	int i, rc = -EINVAL;
> +	int rc = 0;
> 
>  	if (rtas_hp_event)
>  		return 0;
> @@ -1009,42 +1073,122 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>  	if (!memblock_size)
>  		return -EINVAL;
> 
> -	p = (__be32 *) pr->old_prop->value;
> -	if (!p)
> -		return -EINVAL;
> -
> -	/* The first int of the property is the number of lmb's described
> -	 * by the property. This is followed by an array of of_drconf_cell
> -	 * entries. Get the number of entries and skip to the array of
> -	 * of_drconf_cell's.
> -	 */
> -	entries = be32_to_cpu(*p++);
> -	old_drmem = (struct of_drconf_cell_v1 *)p;
> +	/* Arrays should have the same size and DRC indexes */
> +	for_each_pair_dinfo_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
> 
> -	p = (__be32 *)pr->prop->value;
> -	p++;
> -	new_drmem = (struct of_drconf_cell_v1 *)p;
> +		if (new_lmb->drc_index != old_lmb->drc_index)
> +			continue;
> 
> -	for (i = 0; i < entries; i++) {
> -		if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
> -		    (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
> +		if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
> +		    (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
>  			rc = pseries_remove_memblock(
> -				be64_to_cpu(old_drmem[i].base_addr),
> -						     memblock_size);
> +				old_lmb->base_addr, memblock_size);
>  			break;
> -		} else if ((!(be32_to_cpu(old_drmem[i].flags) &
> -			    DRCONF_MEM_ASSIGNED)) &&
> -			    (be32_to_cpu(new_drmem[i].flags) &
> -			    DRCONF_MEM_ASSIGNED)) {
> -			rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
> -					  memblock_size);
> +		} else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
> +			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> +			rc = memblock_add(old_lmb->base_addr,
> +					memblock_size);
>  			rc = (rc < 0) ? -EINVAL : 0;
>  			break;
> +		} else if ((old_lmb->aa_index != new_lmb->aa_index) &&
> +			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> +			drmem_mark_lmb_update(old_lmb);
> +			dlpar_memory_pmt_changes_set();
>  		}
>  	}
>  	return rc;
>  }
> 
> +static void pseries_update_ala_memory_aai(int aa_index)
> +{
> +	struct drmem_lmb *lmb;
> +
> +	/* Readd all LMBs which were previously using the
> +	 * specified aa_index value.
> +	 */
> +	for_each_drmem_lmb(lmb) {
> +		if ((lmb->aa_index == aa_index) &&
> +			(lmb->flags & DRCONF_MEM_ASSIGNED)) {
> +			drmem_mark_lmb_update(lmb);
> +			dlpar_memory_pmt_changes_set();
> +		}
> +	}
> +}
> +
> +struct assoc_arrays {
> +	u32 n_arrays;
> +	u32 array_sz;
> +	const __be32 *arrays;
> +};
> +
> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
> +{
> +	struct assoc_arrays new_ala, old_ala;
> +	__be32 *p;
> +	int i, lim;
> +
> +	if (rtas_hp_event)
> +		return 0;
> +
> +	/*
> +	 * The layout of the ibm,associativity-lookup-arrays
> +	 * property is a number N indicating the number of
> +	 * associativity arrays, followed by a number M
> +	 * indicating the size of each associativity array,
> +	 * followed by a list of N associativity arrays.
> +	 */
> +
> +	p = (__be32 *) pr->old_prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	old_ala.n_arrays = of_read_number(p++, 1);
> +	old_ala.array_sz = of_read_number(p++, 1);
> +	old_ala.arrays = p;
> +
> +	p = (__be32 *) pr->prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	new_ala.n_arrays = of_read_number(p++, 1);
> +	new_ala.array_sz = of_read_number(p++, 1);
> +	new_ala.arrays = p;
> +
> +	lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
> +			new_ala.n_arrays;
> +
> +	if (old_ala.array_sz == new_ala.array_sz) {
> +
> +		/* Reset any entries where the old and new rows
> +		 * the array have changed.
> +		 */
> +		for (i = 0; i < lim; i++) {
> +			int index = (i * new_ala.array_sz);
> +
> +			if (!memcmp(&old_ala.arrays[index],
> +				&new_ala.arrays[index],
> +				new_ala.array_sz))
> +				continue;
> +
> +			pseries_update_ala_memory_aai(i);
> +		}
> +
> +		/* Reset any entries representing the extra rows.
> +		 * There shouldn't be any, but just in case ...
> +		 */
> +		for (i = lim; i < new_ala.n_arrays; i++)
> +			pseries_update_ala_memory_aai(i);
> +
> +	} else {
> +		/* Update all entries representing these rows;
> +		 * as all rows have different sizes, none can
> +		 * have equivalent values.
> +		 */
> +		for (i = 0; i < lim; i++)
> +			pseries_update_ala_memory_aai(i);
> +	}
> +
> +	return 0;
> +}
> +
>  static int pseries_memory_notifier(struct notifier_block *nb,
>  				   unsigned long action, void *data)
>  {
> @@ -1059,8 +1203,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>  		err = pseries_remove_mem_node(rd->dn);
>  		break;
>  	case OF_RECONFIG_UPDATE_PROPERTY:
> -		if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
> -			err = pseries_update_drconf_memory(rd);
> +		if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
> +			struct drmem_lmb_info *dinfo =
> +				drmem_lmbs_init(rd->prop);
> +			if (!dinfo)
> +				return -EINVAL;
> +			err = pseries_update_drconf_memory(dinfo);
> +			drmem_lmbs_free(dinfo);
> +		} else if (!strcmp(rd->prop->name,
> +				"ibm,associativity-lookup-arrays"))
> +			err = pseries_update_ala_memory(rd);
>  		break;
>  	}
>  	return notifier_from_errno(err);
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 23fb9ac..6c0456f 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -383,6 +383,10 @@ static ssize_t migration_store(struct class *class,
>  		return rc;
> 
>  	post_mobility_fixup();
> +
> +	/* Apply any necessary changes identified during fixup */
> +	dlpar_memory_pmt_changes_action();
> +
>  	return count;
>  }
> 
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index 60db2ee..b238634 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -69,6 +69,10 @@ static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>  	return -EOPNOTSUPP;
>  }
>  #endif
> +void dlpar_memory_pmt_changes_set(void);
> +void dlpar_memory_pmt_changes_clear(void);
> +int dlpar_memory_pmt_changes(void);
> +void dlpar_memory_pmt_changes_action(void);
> 
>  #ifdef CONFIG_HOTPLUG_CPU
>  int dlpar_cpu(struct pseries_hp_errorlog *hp_elog);
> 

Re: [PATCH v03 4/5] migration/memory: Evaluate LMB assoc changes

[Michael Bringmann]

On 10/02/2018 04:08 PM, Tyrel Datwyler wrote:
> On 10/01/2018 06:00 AM, Michael Bringmann wrote:
>> migration/memory: This patch adds code that recognizes changes to
>> the associativity of memory blocks described by the device-tree
>> properties in order to drive equivalent 'hotplug' operations to
>> update local and general kernel data structures to reflect those
>> changes.  These differences may include:
>>
>> * Evaluate 'ibm,dynamic-memory' properties when processing the
>>   updated device-tree properties of the system during Post Migration
>>   events (migration_store).  The new functionality looks for changes
>>   to the aa_index values for each drc_index/LMB to identify any memory
>>   blocks that should be readded.
>>
>> * In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
>>   property may change.  In the event that a row of the array differs,
>>   locate all assigned memory blocks with that 'aa_index' and 're-add'
>>   them to the system memory block data structures.  In the process of
>>   the 're-add', the system routines will update the corresponding entry
>>   for the memory in the LMB structures and any other relevant kernel
>>   data structures.
>>
>> A number of previous extensions made to the DRMEM code for scanning
>> device-tree properties and creating LMB arrays are used here to
>> ensure that the resulting code is simpler and more usable:
>>
>> * Use new paired list iterator for the DRMEM LMB info arrays to find
>>   differences in old and new versions of properties.
>> * Use new iterator for copies of the DRMEM info arrays to evaluate
>>   completely new structures.
>> * Combine common code for parsing and evaluating memory description
>>   properties based on the DRMEM LMB array model to greatly simplify
>>   extension from the older property 'ibm,dynamic-memory' to the new
>>   property model of 'ibm,dynamic-memory-v2'.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in v03:
>>   -- Modify the code that parses the memory affinity attributes to
>>      mark relevant DRMEM LMB array entries using the internal_flags
>>      mechanism instead of generate unique hotplug actions for each
>>      memory block to be readded.  The change is intended to both
>>      simplify the code, and to require fewer resources on systems
>>      with huge amounts of memory.
>>   -- Save up notice about any all LMB entries until the end of the
>>      'migration_store' operation at which point a single action is
>>      queued to scan the entire DRMEM array.
>>   -- Add READD_MULTIPLE function for memory that scans the DRMEM
>>      array to identify multiple entries that were marked previously.
>>      The corresponding memory blocks are to be readded to the system
>>      to update relevant data structures outside of the powerpc-
>>      specific code.
>>   -- Change dlpar_memory_pmt_changes_action to directly queue worker
>>      to pseries work queue.
>> ---
>>  arch/powerpc/platforms/pseries/hotplug-memory.c |  220 +++++++++++++++++++----
>>  arch/powerpc/platforms/pseries/mobility.c       |    4 
>>  arch/powerpc/platforms/pseries/pseries.h        |    4 
>>  3 files changed, 194 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index c1578f5..68bde2e 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -561,8 +561,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>>  		}
>>  	}
>>
>> -	if (!lmb_found)
>> -		rc = -EINVAL;
>> +	if (!lmb_found) {
>> +		pr_info("Failed to update memory for drc index %lx\n",
>> +			(unsigned long) drc_index);
>> +		return -EINVAL;
>> +	}
>>
>>  	if (rc)
>>  		pr_info("Failed to update memory at %llx\n",
>> @@ -573,6 +576,30 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>>  	return rc;
>>  }
>>
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> +	struct drmem_lmb *lmb;
>> +	int rc;
>> +
>> +	pr_info("Attempting to update multiple LMBs\n");
>> +
>> +	for_each_drmem_lmb(lmb) {
>> +		if (drmem_lmb_update(lmb)) {
>> +			rc = dlpar_remove_lmb(lmb);
>> +
>> +			if (!rc) {
>> +				rc = dlpar_add_lmb(lmb);
>> +				if (rc)
>> +					dlpar_release_drc(lmb->drc_index);
>> +			}
>> +
>> +			drmem_remove_lmb_update(lmb);
>> +		}
>> +	}
>> +
>> +	return rc;
>> +}
>> +
> 
> Actually, in retrospect this function should have been independently implemented in your 3rd patch where you define the flag. So, disregard moving the define into this patch and instead move the function implementation into patch 3 where the commit message talks about adding this operation.

Okay.  It won't hurt to decouple these changes.
> 
>>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>>  {
>>  	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
>> @@ -673,6 +700,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>>  {
>>  	return -EOPNOTSUPP;
>>  }
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
> 
> Same as above commit obviously.

Okay.
> 
>>
>>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>>  {
>> @@ -952,6 +983,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>>  		drc_index = hp_elog->_drc_u.drc_index;
>>  		rc = dlpar_memory_readd_by_index(drc_index);
>>  		break;
>> +	case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
>> +		rc = dlpar_memory_readd_multiple();
>> +		break;
> 
> And this as well.

Okay.

> 
> -Tyrel

Michael

> 
>>  	default:
>>  		pr_err("Invalid action (%d) specified\n", hp_elog->action);
>>  		rc = -EINVAL;
>> @@ -994,13 +1028,43 @@ static int pseries_add_mem_node(struct device_node *np)
>>  	return (ret < 0) ? -EINVAL : 0;
>>  }
>>
>> -static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>> +static int pmt_changes = 0;
>> +
>> +void dlpar_memory_pmt_changes_set(void)
>> +{
>> +	pmt_changes = 1;
>> +}
>> +
>> +void dlpar_memory_pmt_changes_clear(void)
>> +{
>> +	pmt_changes = 0;
>> +}
>> +
>> +int dlpar_memory_pmt_changes(void)
>> +{
>> +	return pmt_changes;
>> +}
>> +
>> +void dlpar_memory_pmt_changes_action(void)
>> +{
>> +	if (dlpar_memory_pmt_changes()) {
>> +		struct pseries_hp_errorlog hp_errlog;
>> +
>> +        	hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
>> +        	hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
>> +        	hp_errlog.id_type = 0;
>> +
>> +        	queue_hotplug_event(&hp_errlog, NULL, NULL);
>> +
>> +		dlpar_memory_pmt_changes_clear();
>> +	}
>> +}
>> +
>> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
>>  {
>> -	struct of_drconf_cell_v1 *new_drmem, *old_drmem;
>> +	struct drmem_lmb *old_lmb, *new_lmb;
>>  	unsigned long memblock_size;
>> -	u32 entries;
>> -	__be32 *p;
>> -	int i, rc = -EINVAL;
>> +	int rc = 0;
>>
>>  	if (rtas_hp_event)
>>  		return 0;
>> @@ -1009,42 +1073,122 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>>  	if (!memblock_size)
>>  		return -EINVAL;
>>
>> -	p = (__be32 *) pr->old_prop->value;
>> -	if (!p)
>> -		return -EINVAL;
>> -
>> -	/* The first int of the property is the number of lmb's described
>> -	 * by the property. This is followed by an array of of_drconf_cell
>> -	 * entries. Get the number of entries and skip to the array of
>> -	 * of_drconf_cell's.
>> -	 */
>> -	entries = be32_to_cpu(*p++);
>> -	old_drmem = (struct of_drconf_cell_v1 *)p;
>> +	/* Arrays should have the same size and DRC indexes */
>> +	for_each_pair_dinfo_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
>>
>> -	p = (__be32 *)pr->prop->value;
>> -	p++;
>> -	new_drmem = (struct of_drconf_cell_v1 *)p;
>> +		if (new_lmb->drc_index != old_lmb->drc_index)
>> +			continue;
>>
>> -	for (i = 0; i < entries; i++) {
>> -		if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
>> -		    (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
>> +		if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
>> +		    (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
>>  			rc = pseries_remove_memblock(
>> -				be64_to_cpu(old_drmem[i].base_addr),
>> -						     memblock_size);
>> +				old_lmb->base_addr, memblock_size);
>>  			break;
>> -		} else if ((!(be32_to_cpu(old_drmem[i].flags) &
>> -			    DRCONF_MEM_ASSIGNED)) &&
>> -			    (be32_to_cpu(new_drmem[i].flags) &
>> -			    DRCONF_MEM_ASSIGNED)) {
>> -			rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
>> -					  memblock_size);
>> +		} else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
>> +			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> +			rc = memblock_add(old_lmb->base_addr,
>> +					memblock_size);
>>  			rc = (rc < 0) ? -EINVAL : 0;
>>  			break;
>> +		} else if ((old_lmb->aa_index != new_lmb->aa_index) &&
>> +			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> +			drmem_mark_lmb_update(old_lmb);
>> +			dlpar_memory_pmt_changes_set();
>>  		}
>>  	}
>>  	return rc;
>>  }
>>
>> +static void pseries_update_ala_memory_aai(int aa_index)
>> +{
>> +	struct drmem_lmb *lmb;
>> +
>> +	/* Readd all LMBs which were previously using the
>> +	 * specified aa_index value.
>> +	 */
>> +	for_each_drmem_lmb(lmb) {
>> +		if ((lmb->aa_index == aa_index) &&
>> +			(lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> +			drmem_mark_lmb_update(lmb);
>> +			dlpar_memory_pmt_changes_set();
>> +		}
>> +	}
>> +}
>> +
>> +struct assoc_arrays {
>> +	u32 n_arrays;
>> +	u32 array_sz;
>> +	const __be32 *arrays;
>> +};
>> +
>> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>> +{
>> +	struct assoc_arrays new_ala, old_ala;
>> +	__be32 *p;
>> +	int i, lim;
>> +
>> +	if (rtas_hp_event)
>> +		return 0;
>> +
>> +	/*
>> +	 * The layout of the ibm,associativity-lookup-arrays
>> +	 * property is a number N indicating the number of
>> +	 * associativity arrays, followed by a number M
>> +	 * indicating the size of each associativity array,
>> +	 * followed by a list of N associativity arrays.
>> +	 */
>> +
>> +	p = (__be32 *) pr->old_prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	old_ala.n_arrays = of_read_number(p++, 1);
>> +	old_ala.array_sz = of_read_number(p++, 1);
>> +	old_ala.arrays = p;
>> +
>> +	p = (__be32 *) pr->prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	new_ala.n_arrays = of_read_number(p++, 1);
>> +	new_ala.array_sz = of_read_number(p++, 1);
>> +	new_ala.arrays = p;
>> +
>> +	lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
>> +			new_ala.n_arrays;
>> +
>> +	if (old_ala.array_sz == new_ala.array_sz) {
>> +
>> +		/* Reset any entries where the old and new rows
>> +		 * the array have changed.
>> +		 */
>> +		for (i = 0; i < lim; i++) {
>> +			int index = (i * new_ala.array_sz);
>> +
>> +			if (!memcmp(&old_ala.arrays[index],
>> +				&new_ala.arrays[index],
>> +				new_ala.array_sz))
>> +				continue;
>> +
>> +			pseries_update_ala_memory_aai(i);
>> +		}
>> +
>> +		/* Reset any entries representing the extra rows.
>> +		 * There shouldn't be any, but just in case ...
>> +		 */
>> +		for (i = lim; i < new_ala.n_arrays; i++)
>> +			pseries_update_ala_memory_aai(i);
>> +
>> +	} else {
>> +		/* Update all entries representing these rows;
>> +		 * as all rows have different sizes, none can
>> +		 * have equivalent values.
>> +		 */
>> +		for (i = 0; i < lim; i++)
>> +			pseries_update_ala_memory_aai(i);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int pseries_memory_notifier(struct notifier_block *nb,
>>  				   unsigned long action, void *data)
>>  {
>> @@ -1059,8 +1203,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>>  		err = pseries_remove_mem_node(rd->dn);
>>  		break;
>>  	case OF_RECONFIG_UPDATE_PROPERTY:
>> -		if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
>> -			err = pseries_update_drconf_memory(rd);
>> +		if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
>> +			struct drmem_lmb_info *dinfo =
>> +				drmem_lmbs_init(rd->prop);
>> +			if (!dinfo)
>> +				return -EINVAL;
>> +			err = pseries_update_drconf_memory(dinfo);
>> +			drmem_lmbs_free(dinfo);
>> +		} else if (!strcmp(rd->prop->name,
>> +				"ibm,associativity-lookup-arrays"))
>> +			err = pseries_update_ala_memory(rd);
>>  		break;
>>  	}
>>  	return notifier_from_errno(err);
>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>> index 23fb9ac..6c0456f 100644
>> --- a/arch/powerpc/platforms/pseries/mobility.c
>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>> @@ -383,6 +383,10 @@ static ssize_t migration_store(struct class *class,
>>  		return rc;
>>
>>  	post_mobility_fixup();
>> +
>> +	/* Apply any necessary changes identified during fixup */
>> +	dlpar_memory_pmt_changes_action();
>> +
>>  	return count;
>>  }
>>
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
>> index 60db2ee..b238634 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -69,6 +69,10 @@ static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>>  	return -EOPNOTSUPP;
>>  }
>>  #endif
>> +void dlpar_memory_pmt_changes_set(void);
>> +void dlpar_memory_pmt_changes_clear(void);
>> +int dlpar_memory_pmt_changes(void);
>> +void dlpar_memory_pmt_changes_action(void);
>>
>>  #ifdef CONFIG_HOTPLUG_CPU
>>  int dlpar_cpu(struct pseries_hp_errorlog *hp_elog);
>>
> 
> 

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

[PATCH v03 5/5] migration/memory: Support 'ibm,dynamic-memory-v2'

[Michael Bringmann]

migration/memory: This patch adds recognition for changes to the
associativity of memory blocks described by 'ibm,dynamic-memory-v2'.
If the associativity of an LMB has changed, it should be readded to
the system in order to update local and general kernel data structures.
This patch builds upon previous enhancements that scan the device-tree
"ibm,dynamic-memory" properties using the base LMB array, and a copy
derived from the updated properties.

Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/hotplug-memory.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 68bde2e..3e65aeb 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1203,7 +1203,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
 		err = pseries_remove_mem_node(rd->dn);
 		break;
 	case OF_RECONFIG_UPDATE_PROPERTY:
-		if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+		if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
+		    !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
 			struct drmem_lmb_info *dinfo =
 				drmem_lmbs_init(rd->prop);
 			if (!dinfo)