[FIX PATCH v2 0/2] Fix powerpc,numa: Fix memory_hotplug_max()

[FIX PATCH v2 0/2] Fix powerpc,numa: Fix memory_hotplug_max()

[Bharata B Rao]

This patchset fixes memory_hotplug_max() routine to return correct
value of maximum hotpluggable address.

In this version, whitespace fixes are separated into a different patch.

v2: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg103342.html

Bharata B Rao (2):
  powerpc,numa: Fix whitespace in hot_add_drconf_memory_max()
  powerpc,numa: Fix memory_hotplug_max()

 arch/powerpc/mm/numa.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

-- 
2.1.0

[FIX PATCH v2 1/2] powerpc, numa: Fix whitespace in hot_add_drconf_memory_max()

[Bharata B Rao]

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 669a15e..4a87ccb 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1163,18 +1163,18 @@ int hot_add_scn_to_nid(unsigned long scn_addr)
 
 static u64 hot_add_drconf_memory_max(void)
 {
-        struct device_node *memory = NULL;
-        unsigned int drconf_cell_cnt = 0;
-        u64 lmb_size = 0;
+	struct device_node *memory = NULL;
+	unsigned int drconf_cell_cnt = 0;
+	u64 lmb_size = 0;
 	const __be32 *dm = NULL;
 
-        memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
-        if (memory) {
-                drconf_cell_cnt = of_get_drconf_memory(memory, &dm);
-                lmb_size = of_get_lmb_size(memory);
-                of_node_put(memory);
-        }
-        return lmb_size * drconf_cell_cnt;
+	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
+	if (memory) {
+		drconf_cell_cnt = of_get_drconf_memory(memory, &dm);
+		lmb_size = of_get_lmb_size(memory);
+		of_node_put(memory);
+	}
+	return lmb_size * drconf_cell_cnt;
 }
 
 /*
-- 
2.1.0

Re: [FIX PATCH v2 1/2] powerpc,numa: Fix whitespace in hot_add_drconf_memory_max()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Thu, May 12, 2016 at 07:04:14PM +0530, Bharata B Rao wrote:
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  arch/powerpc/mm/numa.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 669a15e..4a87ccb 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1163,18 +1163,18 @@ int hot_add_scn_to_nid(unsigned long scn_addr)
>  
>  static u64 hot_add_drconf_memory_max(void)
>  {
> -        struct device_node *memory = NULL;
> -        unsigned int drconf_cell_cnt = 0;
> -        u64 lmb_size = 0;
> +	struct device_node *memory = NULL;
> +	unsigned int drconf_cell_cnt = 0;
> +	u64 lmb_size = 0;
>  	const __be32 *dm = NULL;
>  
> -        memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> -        if (memory) {
> -                drconf_cell_cnt = of_get_drconf_memory(memory, &dm);
> -                lmb_size = of_get_lmb_size(memory);
> -                of_node_put(memory);
> -        }
> -        return lmb_size * drconf_cell_cnt;
> +	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> +	if (memory) {
> +		drconf_cell_cnt = of_get_drconf_memory(memory, &dm);
> +		lmb_size = of_get_lmb_size(memory);
> +		of_node_put(memory);
> +	}
> +	return lmb_size * drconf_cell_cnt;
>  }
>  
>  /*

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [FIX, v2, 1/2] powerpc, numa: Fix whitespace in hot_add_drconf_memory_max()

[Michael Ellerman]

On Thu, 2016-12-05 at 13:34:14 UTC, Bharata B Rao wrote:
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e70bd3ae914ec40d8505ed842d

cheers

[FIX PATCH v2 2/2] powerpc,numa: Fix memory_hotplug_max()

[Bharata B Rao]

memory_hotplug_max() uses hot_add_drconf_memory_max() to get maxmimum
addressable memory by referring to ibm,dyanamic-memory property. There
are three problems with the current approach:

1 hot_add_drconf_memory_max() assumes that ibm,dynamic-memory includes
  all the LMBs of the guest, but that is not true for PowerKVM which
  populates only DR LMBs (LMBs that can be hotplugged/removed) in that
  property.
2 hot_add_drconf_memory_max() multiplies lmb-size with lmb-count to arrive
  at the max possible address. Since ibm,dynamic-memory doesn't include
  RMA LMBs, the address thus obtained will be less than the actual max
  address. For example, if max possible memory size is 32G, with lmb-size
  of 256MB there can be 127 LMBs in ibm,dynamic-memory (1 LMB for RMA
  which won't be present here).  hot_add_drconf_memory_max() would then
  return the max addressable memory as 127 * 256MB = 31.75GB, the max
  address should have been 32G which is what ibm,lrdr-capacity shows.
3 In PowerKVM, there can be a gap between the end of boot time RAM and
  beginning of hotplug RAM area. So just multiplying lmb-count with
  lmb-size will not provide the correct max possible address for PowerKVM.

This patch fixes 1 by using ibm,lrdr-capacity property to return the max
addressable memory whenever the property is present. Then it fixes 2 & 3
by fetching the address of the last LMB in ibm,dynamic-memory property.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 4a87ccb..f8b1da7 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1164,17 +1164,33 @@ int hot_add_scn_to_nid(unsigned long scn_addr)
 static u64 hot_add_drconf_memory_max(void)
 {
 	struct device_node *memory = NULL;
+	struct device_node *dn = NULL;
 	unsigned int drconf_cell_cnt = 0;
 	u64 lmb_size = 0;
 	const __be32 *dm = NULL;
+	const __be64 *lrdr = NULL;
+	struct of_drconf_cell drmem;
+
+	dn = of_find_node_by_path("/rtas");
+	if (dn) {
+		lrdr = of_get_property(dn, "ibm,lrdr-capacity", NULL);
+		of_node_put(dn);
+		if (lrdr)
+			return be64_to_cpup(lrdr);
+	}
 
 	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
 	if (memory) {
 		drconf_cell_cnt = of_get_drconf_memory(memory, &dm);
 		lmb_size = of_get_lmb_size(memory);
+
+		/* Advance to the last cell, each cell has 6 32 bit integers */
+		dm += (drconf_cell_cnt - 1) * 6;
+		read_drconf_cell(&drmem, &dm);
 		of_node_put(memory);
+		return drmem.base_addr + lmb_size;
 	}
-	return lmb_size * drconf_cell_cnt;
+	return 0;
 }
 
 /*
-- 
2.1.0

Re: [FIX PATCH v2 2/2] powerpc,numa: Fix memory_hotplug_max()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]

On Thu, May 12, 2016 at 07:04:15PM +0530, Bharata B Rao wrote:
> memory_hotplug_max() uses hot_add_drconf_memory_max() to get maxmimum
> addressable memory by referring to ibm,dyanamic-memory property. There
> are three problems with the current approach:
> 
> 1 hot_add_drconf_memory_max() assumes that ibm,dynamic-memory includes
>   all the LMBs of the guest, but that is not true for PowerKVM which
>   populates only DR LMBs (LMBs that can be hotplugged/removed) in that
>   property.
> 2 hot_add_drconf_memory_max() multiplies lmb-size with lmb-count to arrive
>   at the max possible address. Since ibm,dynamic-memory doesn't include
>   RMA LMBs, the address thus obtained will be less than the actual max
>   address. For example, if max possible memory size is 32G, with lmb-size
>   of 256MB there can be 127 LMBs in ibm,dynamic-memory (1 LMB for RMA
>   which won't be present here).  hot_add_drconf_memory_max() would then
>   return the max addressable memory as 127 * 256MB = 31.75GB, the max
>   address should have been 32G which is what ibm,lrdr-capacity shows.
> 3 In PowerKVM, there can be a gap between the end of boot time RAM and
>   beginning of hotplug RAM area. So just multiplying lmb-count with
>   lmb-size will not provide the correct max possible address for PowerKVM.
> 
> This patch fixes 1 by using ibm,lrdr-capacity property to return the max
> addressable memory whenever the property is present. Then it fixes 2 & 3
> by fetching the address of the last LMB in ibm,dynamic-memory property.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  arch/powerpc/mm/numa.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 4a87ccb..f8b1da7 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1164,17 +1164,33 @@ int hot_add_scn_to_nid(unsigned long scn_addr)
>  static u64 hot_add_drconf_memory_max(void)
>  {
>  	struct device_node *memory = NULL;
> +	struct device_node *dn = NULL;
>  	unsigned int drconf_cell_cnt = 0;
>  	u64 lmb_size = 0;
>  	const __be32 *dm = NULL;
> +	const __be64 *lrdr = NULL;
> +	struct of_drconf_cell drmem;
> +
> +	dn = of_find_node_by_path("/rtas");
> +	if (dn) {
> +		lrdr = of_get_property(dn, "ibm,lrdr-capacity", NULL);
> +		of_node_put(dn);
> +		if (lrdr)
> +			return be64_to_cpup(lrdr);
> +	}
>  
>  	memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>  	if (memory) {
>  		drconf_cell_cnt = of_get_drconf_memory(memory, &dm);
>  		lmb_size = of_get_lmb_size(memory);
> +
> +		/* Advance to the last cell, each cell has 6 32 bit integers */
> +		dm += (drconf_cell_cnt - 1) * 6;
> +		read_drconf_cell(&drmem, &dm);
>  		of_node_put(memory);
> +		return drmem.base_addr + lmb_size;
>  	}
> -	return lmb_size * drconf_cell_cnt;
> +	return 0;
>  }
>  
>  /*

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [FIX,v2,2/2] powerpc,numa: Fix memory_hotplug_max()

[Michael Ellerman]

On Thu, 2016-12-05 at 13:34:15 UTC, Bharata B Rao wrote:
> memory_hotplug_max() uses hot_add_drconf_memory_max() to get maxmimum
> addressable memory by referring to ibm,dyanamic-memory property. There
> are three problems with the current approach:
> 
> 1 hot_add_drconf_memory_max() assumes that ibm,dynamic-memory includes
>   all the LMBs of the guest, but that is not true for PowerKVM which
>   populates only DR LMBs (LMBs that can be hotplugged/removed) in that
>   property.
> 2 hot_add_drconf_memory_max() multiplies lmb-size with lmb-count to arrive
>   at the max possible address. Since ibm,dynamic-memory doesn't include
>   RMA LMBs, the address thus obtained will be less than the actual max
>   address. For example, if max possible memory size is 32G, with lmb-size
>   of 256MB there can be 127 LMBs in ibm,dynamic-memory (1 LMB for RMA
>   which won't be present here).  hot_add_drconf_memory_max() would then
>   return the max addressable memory as 127 * 256MB = 31.75GB, the max
>   address should have been 32G which is what ibm,lrdr-capacity shows.
> 3 In PowerKVM, there can be a gap between the end of boot time RAM and
>   beginning of hotplug RAM area. So just multiplying lmb-count with
>   lmb-size will not provide the correct max possible address for PowerKVM.
> 
> This patch fixes 1 by using ibm,lrdr-capacity property to return the max
> addressable memory whenever the property is present. Then it fixes 2 & 3
> by fetching the address of the last LMB in ibm,dynamic-memory property.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/45b64ee64970dee9392229302e

cheers

Re: [FIX PATCH v2 0/2] Fix powerpc,numa: Fix memory_hotplug_max()

[Bharata B Rao]

On Thu, May 12, 2016 at 07:04:13PM +0530, Bharata B Rao wrote:
> This patchset fixes memory_hotplug_max() routine to return correct
> value of maximum hotpluggable address.
> 
> In this version, whitespace fixes are separated into a different patch.
> 
> v2: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg103342.html
> 
> Bharata B Rao (2):
>   powerpc,numa: Fix whitespace in hot_add_drconf_memory_max()
>   powerpc,numa: Fix memory_hotplug_max()
> 
>  arch/powerpc/mm/numa.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)

Can this fix be considered for inclusion ? Should I be posting this against
latest git now ?

Regards,
Bharata.

Re: [FIX PATCH v2 0/2] Fix powerpc,numa: Fix memory_hotplug_max()

[Michael Ellerman]

On Tue, 2016-06-14 at 10:16 +0530, Bharata B Rao wrote:
> On Thu, May 12, 2016 at 07:04:13PM +0530, Bharata B Rao wrote:
> > This patchset fixes memory_hotplug_max() routine to return correct
> > value of maximum hotpluggable address.
> > 
> > In this version, whitespace fixes are separated into a different patch.
> > 
> > v2: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg103342.html
> > 
> > Bharata B Rao (2):
> >   powerpc,numa: Fix whitespace in hot_add_drconf_memory_max()
> >   powerpc,numa: Fix memory_hotplug_max()
> > 
> >  arch/powerpc/mm/numa.c | 36 ++++++++++++++++++++++++++----------
> >  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> Can this fix be considered for inclusion ? Should I be posting this against
> latest git now ?

No that's fine, it still applies cleanly.

Have you tested this on PowerVM ?

Has this always been broken, or did we break it in a particular commit? If the
latter can you tell me which commit.

Should we be sending this to stable?

cheers

Re: [FIX PATCH v2 0/2] Fix powerpc,numa: Fix memory_hotplug_max()

[Bharata B Rao]

On Tue, Jun 14, 2016 at 03:19:27PM +1000, Michael Ellerman wrote:
> On Tue, 2016-06-14 at 10:16 +0530, Bharata B Rao wrote:
> > On Thu, May 12, 2016 at 07:04:13PM +0530, Bharata B Rao wrote:
> > > This patchset fixes memory_hotplug_max() routine to return correct
> > > value of maximum hotpluggable address.
> > > 
> > > In this version, whitespace fixes are separated into a different patch.
> > > 
> > > v2: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg103342.html
> > > 
> > > Bharata B Rao (2):
> > >   powerpc,numa: Fix whitespace in hot_add_drconf_memory_max()
> > >   powerpc,numa: Fix memory_hotplug_max()
> > > 
> > >  arch/powerpc/mm/numa.c | 36 ++++++++++++++++++++++++++----------
> > >  1 file changed, 26 insertions(+), 10 deletions(-)
> > 
> > Can this fix be considered for inclusion ? Should I be posting this against
> > latest git now ?
> 
> No that's fine, it still applies cleanly.
> 
> Have you tested this on PowerVM ?

No. Let me grab a PowerVM box, test and get back.

> 
> Has this always been broken, or did we break it in a particular commit? If the
> latter can you tell me which commit.

memory_hotplug_max() got introduced in cd34206e9 and has been buggy since then.
Just that it got exposed by memory hotplug and DDW features on PowerKVM.

> 
> Should we be sending this to stable?

I am not sure as only PowerKVM is affected with the introducion of DDW feature
in yet-to-be-released QEMU-2.7. Even there, we are working around the problem
within QEMU itself, but better to fix this in the kernel once.

Regards,
Bharata.