All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] powerpc/of: split out new_property() for reusing
@ 2020-02-28  5:53 Pingfan Liu
  2020-02-28  5:53 ` [PATCH 2/3] powerpc/of: coding style cleanup Pingfan Liu
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Pingfan Liu @ 2020-02-28  5:53 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kexec, Pingfan Liu, Paul Mackerras, Aneesh Kumar K . V,
	Oliver O'Halloran, Dan Williams, Hari Bathini

Since new_property() is used in several calling sites, splitting it out for
reusing.

To ease the review, although the split out part has coding style issue,
keeping it untouched and fixed in next patch.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: kexec@lists.infradead.org
---
 arch/powerpc/include/asm/prom.h           |  2 ++
 arch/powerpc/kernel/Makefile              |  2 +-
 arch/powerpc/kernel/of_property.c         | 32 +++++++++++++++++++++++++++++++
 arch/powerpc/mm/drmem.c                   | 26 -------------------------
 arch/powerpc/platforms/pseries/reconfig.c | 26 -------------------------
 5 files changed, 35 insertions(+), 53 deletions(-)
 create mode 100644 arch/powerpc/kernel/of_property.c

diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 94e3fd5..02f7b1b 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -90,6 +90,8 @@ struct of_drc_info {
 extern int of_read_drc_info_cell(struct property **prop,
 			const __be32 **curval, struct of_drc_info *data);
 
+extern struct property *new_property(const char *name, const int length,
+		const unsigned char *value, struct property *last);
 
 /*
  * There are two methods for telling firmware what our capabilities are.
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 157b014..23375fd 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -47,7 +47,7 @@ obj-y				:= cputable.o ptrace.o syscalls.o \
 				   signal.o sysfs.o cacheinfo.o time.o \
 				   prom.o traps.o setup-common.o \
 				   udbg.o misc.o io.o misc_$(BITS).o \
-				   of_platform.o prom_parse.o
+				   of_platform.o prom_parse.o of_property.o
 obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
 				   signal_64.o ptrace32.o \
 				   paca.o nvram_64.o firmware.o note.o
diff --git a/arch/powerpc/kernel/of_property.c b/arch/powerpc/kernel/of_property.c
new file mode 100644
index 0000000..e56c832
--- /dev/null
+++ b/arch/powerpc/kernel/of_property.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/of.h>
+#include <linux/string.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+struct property *new_property(const char *name, const int length,
+				     const unsigned char *value, struct property *last)
+{
+	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
+
+	if (!new)
+		return NULL;
+
+	if (!(new->name = kstrdup(name, GFP_KERNEL)))
+		goto cleanup;
+	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
+		goto cleanup;
+
+	memcpy(new->value, value, length);
+	*(((char *)new->value) + length) = 0;
+	new->length = length;
+	new->next = last;
+	return new;
+
+cleanup:
+	kfree(new->name);
+	kfree(new->value);
+	kfree(new);
+	return NULL;
+}
+
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 85b088a..888227e 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
 
 extern int test_hotplug;
 
-static struct property *new_property(const char *name, const int length,
-				     const unsigned char *value, struct property *last)
-{
-	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
-	if (!new)
-		return NULL;
-
-	if (!(new->name = kstrdup(name, GFP_KERNEL)))
-		goto cleanup;
-	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
-		goto cleanup;
-
-	memcpy(new->value, value, length);
-	*(((char *)new->value) + length) = 0;
-	new->length = length;
-	new->next = last;
-	return new;
-
-cleanup:
-	kfree(new->name);
-	kfree(new->value);
-	kfree(new);
-	return NULL;
-}
-
 static int drmem_update_dt_v2(struct device_node *memory,
 			      struct property *prop)
 {
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 7f7369f..8e5a2ba 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
 	return tmp;
 }
 
-static struct property *new_property(const char *name, const int length,
-				     const unsigned char *value, struct property *last)
-{
-	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
-
-	if (!new)
-		return NULL;
-
-	if (!(new->name = kstrdup(name, GFP_KERNEL)))
-		goto cleanup;
-	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
-		goto cleanup;
-
-	memcpy(new->value, value, length);
-	*(((char *)new->value) + length) = 0;
-	new->length = length;
-	new->next = last;
-	return new;
-
-cleanup:
-	kfree(new->name);
-	kfree(new->value);
-	kfree(new);
-	return NULL;
-}
-
 static int do_add_node(char *buf, size_t bufsize)
 {
 	char *path, *end, *name;
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] powerpc/of: coding style cleanup
  2020-02-28  5:53 [PATCH 1/3] powerpc/of: split out new_property() for reusing Pingfan Liu
@ 2020-02-28  5:53 ` Pingfan Liu
  2020-02-28  5:53 ` [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Pingfan Liu @ 2020-02-28  5:53 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kexec, Pingfan Liu, Paul Mackerras, Aneesh Kumar K . V,
	Oliver O'Halloran, Dan Williams, Hari Bathini

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: kexec@lists.infradead.org
---
 arch/powerpc/kernel/of_property.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/of_property.c b/arch/powerpc/kernel/of_property.c
index e56c832..c6abf7e 100644
--- a/arch/powerpc/kernel/of_property.c
+++ b/arch/powerpc/kernel/of_property.c
@@ -5,16 +5,18 @@
 #include <linux/slab.h>
 
 struct property *new_property(const char *name, const int length,
-				     const unsigned char *value, struct property *last)
+		const unsigned char *value, struct property *last)
 {
 	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
 
 	if (!new)
 		return NULL;
 
-	if (!(new->name = kstrdup(name, GFP_KERNEL)))
+	new->name = kstrdup(name, GFP_KERNEL);
+	if (!new->name)
 		goto cleanup;
-	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
+	new->value = kmalloc(length + 1, GFP_KERNEL);
+	if (!new->value)
 		goto cleanup;
 
 	memcpy(new->value, value, length);
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
  2020-02-28  5:53 [PATCH 1/3] powerpc/of: split out new_property() for reusing Pingfan Liu
  2020-02-28  5:53 ` [PATCH 2/3] powerpc/of: coding style cleanup Pingfan Liu
@ 2020-02-28  5:53 ` Pingfan Liu
  2020-02-28  6:52   ` Christophe Leroy
  2020-02-28  6:03 ` [PATCH 1/3] powerpc/of: split out new_property() for reusing Andrew Donnellan
  2020-02-28  6:47 ` Christophe Leroy
  3 siblings, 1 reply; 8+ messages in thread
From: Pingfan Liu @ 2020-02-28  5:53 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: kexec, Pingfan Liu, Paul Mackerras, Aneesh Kumar K . V,
	Oliver O'Halloran, Dan Williams, Hari Bathini

At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
if dumping to fsdax, it will take a very long time.

Take a closer look, during the papr_scm initialization, the only
configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
...), which helps to set up the bound address.

On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
step can be stepped around to save times.  So the pmem bound address can be
passed to the 2nd kernel through a dynamic added property "bound-addr" in
dt node 'ibm,pmemory'.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: kexec@lists.infradead.org
---
note: I can not find such a pseries machine, and not finish it yet.
---
 arch/powerpc/platforms/pseries/papr_scm.c | 32 +++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index c2ef320..555e746 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
 {
 	struct device_node *dn = pdev->dev.of_node;
 	u32 drc_index, metadata_size;
-	u64 blocks, block_size;
+	u64 blocks, block_size, bound_addr = 0;
 	struct papr_scm_priv *p;
 	const char *uuid_str;
 	u64 uuid[2];
@@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
 	p->metadata_size = metadata_size;
 	p->pdev = pdev;
 
-	/* request the hypervisor to bind this region to somewhere in memory */
-	rc = drc_pmem_bind(p);
+	of_property_read_u64(dn, "bound-addr", &bound_addr);
+	if (bound_addr)
+		p->bound_addr = bound_addr;
+	else {
+		struct property *property;
+		u64 big;
 
-	/* If phyp says drc memory still bound then force unbound and retry */
-	if (rc == H_OVERLAP)
-		rc = drc_pmem_query_n_bind(p);
+		/* request the hypervisor to bind this region to somewhere in memory */
+		rc = drc_pmem_bind(p);
 
-	if (rc != H_SUCCESS) {
-		dev_err(&p->pdev->dev, "bind err: %d\n", rc);
-		rc = -ENXIO;
-		goto err;
+		/* If phyp says drc memory still bound then force unbound and retry */
+		if (rc == H_OVERLAP)
+			rc = drc_pmem_query_n_bind(p);
+
+		if (rc != H_SUCCESS) {
+			dev_err(&p->pdev->dev, "bind err: %d\n", rc);
+			rc = -ENXIO;
+			goto err;
+		}
+		big = cpu_to_be64(p->bound_addr);
+		property = new_property("bound-addr", sizeof(u64), &big,
+			NULL);
+		of_add_property(dn, property);
 	}
 
 	/* setup the resource for the newly bound range */
-- 
2.7.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
  2020-02-28  5:53 [PATCH 1/3] powerpc/of: split out new_property() for reusing Pingfan Liu
  2020-02-28  5:53 ` [PATCH 2/3] powerpc/of: coding style cleanup Pingfan Liu
  2020-02-28  5:53 ` [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu
@ 2020-02-28  6:03 ` Andrew Donnellan
  2020-02-28  6:22   ` Pingfan Liu
  2020-02-28  6:47 ` Christophe Leroy
  3 siblings, 1 reply; 8+ messages in thread
From: Andrew Donnellan @ 2020-02-28  6:03 UTC (permalink / raw)
  To: Pingfan Liu, linuxppc-dev
  Cc: Aneesh Kumar K . V, kexec, Paul Mackerras, Oliver O'Halloran,
	Dan Williams, Hari Bathini

On 28/2/20 4:53 pm, Pingfan Liu wrote:
> Since new_property() is used in several calling sites, splitting it out for
> reusing.
> 
> To ease the review, although the split out part has coding style issue,
> keeping it untouched and fixed in next patch.
> 
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: kexec@lists.infradead.org

Which tree does this apply to? I don't see a new_property() in mm/drmem.c...

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
  2020-02-28  6:03 ` [PATCH 1/3] powerpc/of: split out new_property() for reusing Andrew Donnellan
@ 2020-02-28  6:22   ` Pingfan Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Pingfan Liu @ 2020-02-28  6:22 UTC (permalink / raw)
  To: Andrew Donnellan
  Cc: Aneesh Kumar K . V, Kexec Mailing List, Oliver O'Halloran,
	Paul Mackerras, Dan Williams, linuxppc-dev, Hari Bathini

On Fri, Feb 28, 2020 at 2:03 PM Andrew Donnellan <ajd@linux.ibm.com> wrote:
>
> On 28/2/20 4:53 pm, Pingfan Liu wrote:
> > Since new_property() is used in several calling sites, splitting it out for
> > reusing.
> >
> > To ease the review, although the split out part has coding style issue,
> > keeping it untouched and fixed in next patch.
> >
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > Cc: Oliver O'Halloran <oohall@gmail.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: kexec@lists.infradead.org
>
> Which tree does this apply to? I don't see a new_property() in mm/drmem.c...
Sorry, there is mud in my git tree, I check, either linux git or
powerpc git tree does not have this function.

Nack this series, and I will send out V2 for patch 3/3.

Thanks,
Pingfan
>
> --
> Andrew Donnellan              OzLabs, ADL Canberra
> ajd@linux.ibm.com             IBM Australia Limited
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] powerpc/of: split out new_property() for reusing
  2020-02-28  5:53 [PATCH 1/3] powerpc/of: split out new_property() for reusing Pingfan Liu
                   ` (2 preceding siblings ...)
  2020-02-28  6:03 ` [PATCH 1/3] powerpc/of: split out new_property() for reusing Andrew Donnellan
@ 2020-02-28  6:47 ` Christophe Leroy
  3 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-02-28  6:47 UTC (permalink / raw)
  To: Pingfan Liu, linuxppc-dev
  Cc: Aneesh Kumar K . V, kexec, Paul Mackerras, Oliver O'Halloran,
	Dan Williams, Hari Bathini



Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> Since new_property() is used in several calling sites, splitting it out for
> reusing.
> 
> To ease the review, although the split out part has coding style issue,
> keeping it untouched and fixed in next patch.

The moved function fits in one screen. I don't think it is worth 
splitting out for coding style that applies on the new lines only. You 
can squash the two patches, it will still be easy to review.

> 
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: kexec@lists.infradead.org
> ---
>   arch/powerpc/include/asm/prom.h           |  2 ++
>   arch/powerpc/kernel/Makefile              |  2 +-
>   arch/powerpc/kernel/of_property.c         | 32 +++++++++++++++++++++++++++++++
>   arch/powerpc/mm/drmem.c                   | 26 -------------------------
>   arch/powerpc/platforms/pseries/reconfig.c | 26 -------------------------
>   5 files changed, 35 insertions(+), 53 deletions(-)
>   create mode 100644 arch/powerpc/kernel/of_property.c
> 
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 94e3fd5..02f7b1b 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -90,6 +90,8 @@ struct of_drc_info {
>   extern int of_read_drc_info_cell(struct property **prop,
>   			const __be32 **curval, struct of_drc_info *data);
>   
> +extern struct property *new_property(const char *name, const int length,
> +		const unsigned char *value, struct property *last);

Don't add useless 'extern' keyword.

>   
>   /*
>    * There are two methods for telling firmware what our capabilities are.
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 157b014..23375fd 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -47,7 +47,7 @@ obj-y				:= cputable.o ptrace.o syscalls.o \
>   				   signal.o sysfs.o cacheinfo.o time.o \
>   				   prom.o traps.o setup-common.o \
>   				   udbg.o misc.o io.o misc_$(BITS).o \
> -				   of_platform.o prom_parse.o
> +				   of_platform.o prom_parse.o of_property.o
>   obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
>   				   signal_64.o ptrace32.o \
>   				   paca.o nvram_64.o firmware.o note.o
> diff --git a/arch/powerpc/kernel/of_property.c b/arch/powerpc/kernel/of_property.c
> new file mode 100644
> index 0000000..e56c832
> --- /dev/null
> +++ b/arch/powerpc/kernel/of_property.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +struct property *new_property(const char *name, const int length,
> +				     const unsigned char *value, struct property *last)
> +{
> +	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> +
> +	if (!new)
> +		return NULL;
> +
> +	if (!(new->name = kstrdup(name, GFP_KERNEL)))
> +		goto cleanup;
> +	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> +		goto cleanup;
> +
> +	memcpy(new->value, value, length);
> +	*(((char *)new->value) + length) = 0;
> +	new->length = length;
> +	new->next = last;
> +	return new;
> +
> +cleanup:
> +	kfree(new->name);
> +	kfree(new->value);
> +	kfree(new);
> +	return NULL;
> +}
> +
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 85b088a..888227e 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>   
>   extern int test_hotplug;
>   
> -static struct property *new_property(const char *name, const int length,
> -				     const unsigned char *value, struct property *last)
> -{
> -	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> -
> -	if (!new)
> -		return NULL;
> -
> -	if (!(new->name = kstrdup(name, GFP_KERNEL)))
> -		goto cleanup;
> -	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> -		goto cleanup;
> -
> -	memcpy(new->value, value, length);
> -	*(((char *)new->value) + length) = 0;
> -	new->length = length;
> -	new->next = last;
> -	return new;
> -
> -cleanup:
> -	kfree(new->name);
> -	kfree(new->value);
> -	kfree(new);
> -	return NULL;
> -}
> -
>   static int drmem_update_dt_v2(struct device_node *memory,
>   			      struct property *prop)
>   {
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index 7f7369f..8e5a2ba 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
>   	return tmp;
>   }
>   
> -static struct property *new_property(const char *name, const int length,
> -				     const unsigned char *value, struct property *last)
> -{
> -	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> -
> -	if (!new)
> -		return NULL;
> -
> -	if (!(new->name = kstrdup(name, GFP_KERNEL)))
> -		goto cleanup;
> -	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> -		goto cleanup;
> -
> -	memcpy(new->value, value, length);
> -	*(((char *)new->value) + length) = 0;
> -	new->length = length;
> -	new->next = last;
> -	return new;
> -
> -cleanup:
> -	kfree(new->name);
> -	kfree(new->value);
> -	kfree(new);
> -	return NULL;
> -}
> -
>   static int do_add_node(char *buf, size_t bufsize)
>   {
>   	char *path, *end, *name;
> 

Christophe

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
  2020-02-28  5:53 ` [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu
@ 2020-02-28  6:52   ` Christophe Leroy
  2020-02-28  9:27     ` Pingfan Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Christophe Leroy @ 2020-02-28  6:52 UTC (permalink / raw)
  To: Pingfan Liu, linuxppc-dev
  Cc: Aneesh Kumar K . V, kexec, Paul Mackerras, Oliver O'Halloran,
	Dan Williams, Hari Bathini



Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
> if dumping to fsdax, it will take a very long time.
> 
> Take a closer look, during the papr_scm initialization, the only
> configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
> ...), which helps to set up the bound address.
> 
> On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
> step can be stepped around to save times.  So the pmem bound address can be
> passed to the 2nd kernel through a dynamic added property "bound-addr" in
> dt node 'ibm,pmemory'.
> 
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: kexec@lists.infradead.org
> ---
> note: I can not find such a pseries machine, and not finish it yet.
> ---
>   arch/powerpc/platforms/pseries/papr_scm.c | 32 +++++++++++++++++++++----------
>   1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index c2ef320..555e746 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
>   {
>   	struct device_node *dn = pdev->dev.of_node;
>   	u32 drc_index, metadata_size;
> -	u64 blocks, block_size;
> +	u64 blocks, block_size, bound_addr = 0;
>   	struct papr_scm_priv *p;
>   	const char *uuid_str;
>   	u64 uuid[2];
> @@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
>   	p->metadata_size = metadata_size;
>   	p->pdev = pdev;
>   
> -	/* request the hypervisor to bind this region to somewhere in memory */
> -	rc = drc_pmem_bind(p);
> +	of_property_read_u64(dn, "bound-addr", &bound_addr);
> +	if (bound_addr)
> +		p->bound_addr = bound_addr;
> +	else {

All legs of an if/else must have { } when one leg need them, see codying 
style.

> +		struct property *property;
> +		u64 big;
>   
> -	/* If phyp says drc memory still bound then force unbound and retry */
> -	if (rc == H_OVERLAP)
> -		rc = drc_pmem_query_n_bind(p);
> +		/* request the hypervisor to bind this region to somewhere in memory */
> +		rc = drc_pmem_bind(p);
>   
> -	if (rc != H_SUCCESS) {
> -		dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> -		rc = -ENXIO;
> -		goto err;
> +		/* If phyp says drc memory still bound then force unbound and retry */
> +		if (rc == H_OVERLAP)
> +			rc = drc_pmem_query_n_bind(p);
> +
> +		if (rc != H_SUCCESS) {
> +			dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> +			rc = -ENXIO;
> +			goto err;
> +		}
> +		big = cpu_to_be64(p->bound_addr);
> +		property = new_property("bound-addr", sizeof(u64), &big,
> +			NULL);

Why plitting this line in two parts ? You have lines far longer above.
In powerpc we allow lines 90 chars long.

> +		of_add_property(dn, property);
>   	}
>   
>   	/* setup the resource for the newly bound range */
> 

Christophe

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel
  2020-02-28  6:52   ` Christophe Leroy
@ 2020-02-28  9:27     ` Pingfan Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Pingfan Liu @ 2020-02-28  9:27 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Aneesh Kumar K . V, Kexec Mailing List, Oliver O'Halloran,
	Paul Mackerras, Dan Williams, linuxppc-dev, Hari Bathini

On Fri, Feb 28, 2020 at 2:52 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> > At present, plpar_hcall(H_SCM_BIND_MEM, ...) takes a very long time, so
> > if dumping to fsdax, it will take a very long time.
> >
> > Take a closer look, during the papr_scm initialization, the only
> > configuration is through drc_pmem_bind()-> plpar_hcall(H_SCM_BIND_MEM,
> > ...), which helps to set up the bound address.
> >
> > On pseries, for kexec -l/-p kernel, there is no reset of hardware, and this
> > step can be stepped around to save times.  So the pmem bound address can be
> > passed to the 2nd kernel through a dynamic added property "bound-addr" in
> > dt node 'ibm,pmemory'.
> >
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > Cc: Oliver O'Halloran <oohall@gmail.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: kexec@lists.infradead.org
> > ---
> > note: I can not find such a pseries machine, and not finish it yet.
> > ---
> >   arch/powerpc/platforms/pseries/papr_scm.c | 32 +++++++++++++++++++++----------
> >   1 file changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> > index c2ef320..555e746 100644
> > --- a/arch/powerpc/platforms/pseries/papr_scm.c
> > +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> > @@ -382,7 +382,7 @@ static int papr_scm_probe(struct platform_device *pdev)
> >   {
> >       struct device_node *dn = pdev->dev.of_node;
> >       u32 drc_index, metadata_size;
> > -     u64 blocks, block_size;
> > +     u64 blocks, block_size, bound_addr = 0;
> >       struct papr_scm_priv *p;
> >       const char *uuid_str;
> >       u64 uuid[2];
> > @@ -439,17 +439,29 @@ static int papr_scm_probe(struct platform_device *pdev)
> >       p->metadata_size = metadata_size;
> >       p->pdev = pdev;
> >
> > -     /* request the hypervisor to bind this region to somewhere in memory */
> > -     rc = drc_pmem_bind(p);
> > +     of_property_read_u64(dn, "bound-addr", &bound_addr);
> > +     if (bound_addr)
> > +             p->bound_addr = bound_addr;
> > +     else {
>
> All legs of an if/else must have { } when one leg need them, see codying
> style.
OK,
>
> > +             struct property *property;
> > +             u64 big;
> >
> > -     /* If phyp says drc memory still bound then force unbound and retry */
> > -     if (rc == H_OVERLAP)
> > -             rc = drc_pmem_query_n_bind(p);
> > +             /* request the hypervisor to bind this region to somewhere in memory */
> > +             rc = drc_pmem_bind(p);
> >
> > -     if (rc != H_SUCCESS) {
> > -             dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> > -             rc = -ENXIO;
> > -             goto err;
> > +             /* If phyp says drc memory still bound then force unbound and retry */
> > +             if (rc == H_OVERLAP)
> > +                     rc = drc_pmem_query_n_bind(p);
> > +
> > +             if (rc != H_SUCCESS) {
> > +                     dev_err(&p->pdev->dev, "bind err: %d\n", rc);
> > +                     rc = -ENXIO;
> > +                     goto err;
> > +             }
> > +             big = cpu_to_be64(p->bound_addr);
> > +             property = new_property("bound-addr", sizeof(u64), &big,
> > +                     NULL);
>
> Why plitting this line in two parts ? You have lines far longer above.
> In powerpc we allow lines 90 chars long.
OK, good to know it.

Thanks,
Pingfan

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-02-28  9:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28  5:53 [PATCH 1/3] powerpc/of: split out new_property() for reusing Pingfan Liu
2020-02-28  5:53 ` [PATCH 2/3] powerpc/of: coding style cleanup Pingfan Liu
2020-02-28  5:53 ` [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu
2020-02-28  6:52   ` Christophe Leroy
2020-02-28  9:27     ` Pingfan Liu
2020-02-28  6:03 ` [PATCH 1/3] powerpc/of: split out new_property() for reusing Andrew Donnellan
2020-02-28  6:22   ` Pingfan Liu
2020-02-28  6:47 ` Christophe Leroy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.