Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel

2020-02-28 Thread Pingfan Liu
On Fri, Feb 28, 2020 at 2:52 PM Christophe Leroy
 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 
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Benjamin Herrenschmidt 
> > Cc: Paul Mackerras 
> > Cc: Michael Ellerman 
> > Cc: Hari Bathini 
> > Cc: Aneesh Kumar K.V 
> > Cc: Oliver O'Halloran 
> > Cc: Dan Williams 
> > Cc: ke...@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", _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(>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(>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), ,
> > + 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


Re: [PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel

2020-02-27 Thread Christophe Leroy




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 
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Hari Bathini 
Cc: Aneesh Kumar K.V 
Cc: Oliver O'Halloran 
Cc: Dan Williams 
Cc: ke...@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", _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(>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(>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), ,
+   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


[PATCH 3/3] pseries/scm: buffer pmem's bound addr in dt for kexec kernel

2020-02-27 Thread Pingfan Liu
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 
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Hari Bathini 
Cc: Aneesh Kumar K.V 
Cc: Oliver O'Halloran 
Cc: Dan Williams 
Cc: ke...@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", _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(>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(>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), ,
+   NULL);
+   of_add_property(dn, property);
}
 
/* setup the resource for the newly bound range */
-- 
2.7.5