* [PATCHv2 1/2] powerpc/of: split out new_property() for reusing @ 2020-02-28 9:41 Pingfan Liu 2020-02-28 9:41 ` [PATCHv2 2/2] pSeries/papr_scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 0/2] pseries/scm: " Pingfan Liu 0 siblings, 2 replies; 12+ messages in thread From: Pingfan Liu @ 2020-02-28 9:41 UTC (permalink / raw) To: linuxppc-dev Cc: Andrew Donnellan, kexec, Pingfan Liu, Paul Mackerras, Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams, Hari Bathini Splitting out new_property() for coming reusing and moving it to of_helpers.c. Also do some coding style cleanup. 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: Andrew Donnellan <ajd@linux.ibm.com> Cc: Christophe Leroy <christophe.leroy@c-s.fr> Cc: kexec@lists.infradead.org --- arch/powerpc/platforms/pseries/of_helpers.c | 28 ++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/of_helpers.h | 3 +++ arch/powerpc/platforms/pseries/reconfig.c | 26 -------------------------- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c index 66dfd82..1022e0f 100644 --- a/arch/powerpc/platforms/pseries/of_helpers.c +++ b/arch/powerpc/platforms/pseries/of_helpers.c @@ -7,6 +7,34 @@ #include "of_helpers.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; + + new->name = kstrdup(name, GFP_KERNEL); + if (!new->name) + goto cleanup; + new->value = kmalloc(length + 1, GFP_KERNEL); + if (!new->value) + 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; +} + /** * pseries_of_derive_parent - basically like dirname(1) * @path: the full_name of a node to be added to the tree diff --git a/arch/powerpc/platforms/pseries/of_helpers.h b/arch/powerpc/platforms/pseries/of_helpers.h index decad65..34add82 100644 --- a/arch/powerpc/platforms/pseries/of_helpers.h +++ b/arch/powerpc/platforms/pseries/of_helpers.h @@ -4,6 +4,9 @@ #include <linux/of.h> +struct property *new_property(const char *name, const int length, + const unsigned char *value, struct property *last); + struct device_node *pseries_of_derive_parent(const char *path); #endif /* _PSERIES_OF_HELPERS_H */ 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] 12+ messages in thread
* [PATCHv2 2/2] pSeries/papr_scm: buffer pmem's bound addr in dt for kexec kernel 2020-02-28 9:41 [PATCHv2 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu @ 2020-02-28 9:41 ` Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 0/2] pseries/scm: " Pingfan Liu 1 sibling, 0 replies; 12+ messages in thread From: Pingfan Liu @ 2020-02-28 9:41 UTC (permalink / raw) To: linuxppc-dev Cc: Andrew Donnellan, 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: Andrew Donnellan <ajd@linux.ibm.com> Cc: Christophe Leroy <christophe.leroy@c-s.fr> Cc: kexec@lists.infradead.org --- note: This patch has not been tested since I can not get such a pseries with pmem. Please kindly to give some suggestion, thanks. 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 0b4467e..40cd214 100644 --- a/arch/powerpc/platforms/pseries/papr_scm.c +++ b/arch/powerpc/platforms/pseries/papr_scm.c @@ -14,6 +14,7 @@ #include <linux/delay.h> #include <asm/plpar_wrappers.h> +#include "of_helpers.h" #define BIND_ANY_ADDR (~0ul) @@ -383,7 +384,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]; @@ -440,17 +441,28 @@ 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] 12+ messages in thread
* [PATCHv3 0/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel 2020-02-28 9:41 [PATCHv2 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu 2020-02-28 9:41 ` [PATCHv2 2/2] pSeries/papr_scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu @ 2020-03-04 8:47 ` Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu 1 sibling, 2 replies; 12+ messages in thread From: Pingfan Liu @ 2020-03-04 8:47 UTC (permalink / raw) To: linuxppc-dev Cc: Andrew Donnellan, kexec, Pingfan Liu, Rob Herring, Paul Mackerras, Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams, Frank Rowand, Hari Bathini V2 -> V3: in [2/2], EXPORT_SYMBOL(new_property) and EXPORT_SYMBOL_GPL(of_add_property) 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: Andrew Donnellan <ajd@linux.ibm.com> Cc: Christophe Leroy <christophe.leroy@c-s.fr> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: kexec@lists.infradead.org Pingfan Liu (2): powerpc/of: split out new_property() for reusing pseries/scm: buffer pmem's bound addr in dt for kexec kernel arch/powerpc/platforms/pseries/of_helpers.c | 29 +++++++++++++++++++++++++ arch/powerpc/platforms/pseries/of_helpers.h | 3 +++ arch/powerpc/platforms/pseries/papr_scm.c | 33 ++++++++++++++++++++--------- arch/powerpc/platforms/pseries/reconfig.c | 26 ----------------------- drivers/of/base.c | 1 + 5 files changed, 56 insertions(+), 36 deletions(-) -- 2.7.5 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 1/2] powerpc/of: split out new_property() for reusing 2020-03-04 8:47 ` [PATCHv3 0/2] pseries/scm: " Pingfan Liu @ 2020-03-04 8:47 ` Pingfan Liu 2020-03-05 3:58 ` Andrew Donnellan 2020-03-06 19:59 ` Nathan Lynch 2020-03-04 8:47 ` [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu 1 sibling, 2 replies; 12+ messages in thread From: Pingfan Liu @ 2020-03-04 8:47 UTC (permalink / raw) To: linuxppc-dev Cc: Andrew Donnellan, kexec, Pingfan Liu, Rob Herring, Paul Mackerras, Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams, Frank Rowand, Hari Bathini Splitting out new_property() for coming reusing and moving it to of_helpers.c. Also do some coding style cleanup. 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: Andrew Donnellan <ajd@linux.ibm.com> Cc: Christophe Leroy <christophe.leroy@c-s.fr> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: kexec@lists.infradead.org --- arch/powerpc/platforms/pseries/of_helpers.c | 28 ++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/of_helpers.h | 3 +++ arch/powerpc/platforms/pseries/reconfig.c | 26 -------------------------- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c index 66dfd82..1022e0f 100644 --- a/arch/powerpc/platforms/pseries/of_helpers.c +++ b/arch/powerpc/platforms/pseries/of_helpers.c @@ -7,6 +7,34 @@ #include "of_helpers.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; + + new->name = kstrdup(name, GFP_KERNEL); + if (!new->name) + goto cleanup; + new->value = kmalloc(length + 1, GFP_KERNEL); + if (!new->value) + 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; +} + /** * pseries_of_derive_parent - basically like dirname(1) * @path: the full_name of a node to be added to the tree diff --git a/arch/powerpc/platforms/pseries/of_helpers.h b/arch/powerpc/platforms/pseries/of_helpers.h index decad65..34add82 100644 --- a/arch/powerpc/platforms/pseries/of_helpers.h +++ b/arch/powerpc/platforms/pseries/of_helpers.h @@ -4,6 +4,9 @@ #include <linux/of.h> +struct property *new_property(const char *name, const int length, + const unsigned char *value, struct property *last); + struct device_node *pseries_of_derive_parent(const char *path); #endif /* _PSERIES_OF_HELPERS_H */ 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] 12+ messages in thread
* Re: [PATCHv3 1/2] powerpc/of: split out new_property() for reusing 2020-03-04 8:47 ` [PATCHv3 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu @ 2020-03-05 3:58 ` Andrew Donnellan 2020-03-06 19:59 ` Nathan Lynch 1 sibling, 0 replies; 12+ messages in thread From: Andrew Donnellan @ 2020-03-05 3:58 UTC (permalink / raw) To: Pingfan Liu, linuxppc-dev Cc: kexec, Rob Herring, Oliver O'Halloran, Aneesh Kumar K . V, Paul Mackerras, Dan Williams, Frank Rowand, Hari Bathini On 4/3/20 7:47 pm, Pingfan Liu wrote: > Splitting out new_property() for coming reusing and moving it to > of_helpers.c. > > Also do some coding style cleanup. > > 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: Andrew Donnellan <ajd@linux.ibm.com> > Cc: Christophe Leroy <christophe.leroy@c-s.fr> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: kexec@lists.infradead.org Would this be useful to move into generic OF code? Also if more functions are moving into of_helpers.c perhaps there should be a common function name prefix. Otherwise your style cleanup looks good. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 1/2] powerpc/of: split out new_property() for reusing 2020-03-04 8:47 ` [PATCHv3 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu 2020-03-05 3:58 ` Andrew Donnellan @ 2020-03-06 19:59 ` Nathan Lynch 2020-03-09 1:50 ` Pingfan Liu 1 sibling, 1 reply; 12+ messages in thread From: Nathan Lynch @ 2020-03-06 19:59 UTC (permalink / raw) To: Pingfan Liu Cc: Andrew Donnellan, Aneesh Kumar K . V, kexec, linuxppc-dev, Pingfan Liu, Rob Herring, Paul Mackerras, Oliver O'Halloran, Dan Williams, Frank Rowand, Hari Bathini Hi, Pingfan Liu <kernelfans@gmail.com> writes: > Splitting out new_property() for coming reusing and moving it to > of_helpers.c. [...] > +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; > + > + new->name = kstrdup(name, GFP_KERNEL); > + if (!new->name) > + goto cleanup; > + new->value = kmalloc(length + 1, GFP_KERNEL); > + if (!new->value) > + 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; > +} This function in its current form isn't suitable for more general use: * It appears to be tailored to string properties - note the char * value parameter, the length + 1 allocation and nul termination. * Most code shouldn't need the 'last' argument. The code where this currently resides builds a list of properties and attaches it to a new node, bypassing of_add_property(). Let's look at the call site you add in your next patch: + big = cpu_to_be64(p->bound_addr); + property = new_property("bound-addr", sizeof(u64), (const unsigned char *)&big, + NULL); + of_add_property(dn, property); So you have to use a cast, and this is going to allocate (sizeof(u64) + 1) for the value, is that what you want? I think you should leave that legacy pseries reconfig code undisturbed (frankly that stuff should get deprecated and removed) and if you want a generic helper it should look more like: struct property *of_property_new(const char *name, size_t length, const void *value, gfp_t allocflags) __of_prop_dup() looks like a good model/guide here. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 1/2] powerpc/of: split out new_property() for reusing 2020-03-06 19:59 ` Nathan Lynch @ 2020-03-09 1:50 ` Pingfan Liu 0 siblings, 0 replies; 12+ messages in thread From: Pingfan Liu @ 2020-03-09 1:50 UTC (permalink / raw) To: Nathan Lynch Cc: Andrew Donnellan, Aneesh Kumar K . V, Kexec Mailing List, linuxppc-dev, Rob Herring, Paul Mackerras, Oliver O'Halloran, Dan Williams, Frank Rowand, Hari Bathini On Sat, Mar 7, 2020 at 3:59 AM Nathan Lynch <nathanl@linux.ibm.com> wrote: > > Hi, > > Pingfan Liu <kernelfans@gmail.com> writes: > > Splitting out new_property() for coming reusing and moving it to > > of_helpers.c. > > [...] > > > +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; > > + > > + new->name = kstrdup(name, GFP_KERNEL); > > + if (!new->name) > > + goto cleanup; > > + new->value = kmalloc(length + 1, GFP_KERNEL); > > + if (!new->value) > > + 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; > > +} > > This function in its current form isn't suitable for more general use: > > * It appears to be tailored to string properties - note the char * value > parameter, the length + 1 allocation and nul termination. > > * Most code shouldn't need the 'last' argument. The code where this > currently resides builds a list of properties and attaches it to a new > node, bypassing of_add_property(). > > Let's look at the call site you add in your next patch: > > + big = cpu_to_be64(p->bound_addr); > + property = new_property("bound-addr", sizeof(u64), (const unsigned char *)&big, > + NULL); > + of_add_property(dn, property); > > So you have to use a cast, and this is going to allocate (sizeof(u64) + 1) > for the value, is that what you want? > > I think you should leave that legacy pseries reconfig code undisturbed > (frankly that stuff should get deprecated and removed) and if you want a > generic helper it should look more like: > > struct property *of_property_new(const char *name, size_t length, > const void *value, gfp_t allocflags) > > __of_prop_dup() looks like a good model/guide here. Thanks for your good suggestion. I will re-code based on your suggestion, if [2/2] turns out acceptable. Regards, Pingfan ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel 2020-03-04 8:47 ` [PATCHv3 0/2] pseries/scm: " Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu @ 2020-03-04 8:47 ` Pingfan Liu 2020-03-13 3:17 ` Oliver O'Halloran 2020-03-16 2:53 ` Aneesh Kumar K.V 1 sibling, 2 replies; 12+ messages in thread From: Pingfan Liu @ 2020-03-04 8:47 UTC (permalink / raw) To: linuxppc-dev Cc: Andrew Donnellan, kexec, Pingfan Liu, Rob Herring, Paul Mackerras, Aneesh Kumar K . V, Oliver O'Halloran, Dan Williams, Frank Rowand, 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: Andrew Donnellan <ajd@linux.ibm.com> Cc: Christophe Leroy <christophe.leroy@c-s.fr> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: kexec@lists.infradead.org --- note: This patch has not been tested since I can not get such a pseries with pmem. Please kindly to give some suggestion, thanks. --- arch/powerpc/platforms/pseries/of_helpers.c | 1 + arch/powerpc/platforms/pseries/papr_scm.c | 33 ++++++++++++++++++++--------- drivers/of/base.c | 1 + 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c index 1022e0f..2c7bab4 100644 --- a/arch/powerpc/platforms/pseries/of_helpers.c +++ b/arch/powerpc/platforms/pseries/of_helpers.c @@ -34,6 +34,7 @@ struct property *new_property(const char *name, const int length, kfree(new); return NULL; } +EXPORT_SYMBOL(new_property); /** * pseries_of_derive_parent - basically like dirname(1) diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c index 0b4467e..54ae903 100644 --- a/arch/powerpc/platforms/pseries/papr_scm.c +++ b/arch/powerpc/platforms/pseries/papr_scm.c @@ -14,6 +14,7 @@ #include <linux/delay.h> #include <asm/plpar_wrappers.h> +#include "of_helpers.h" #define BIND_ANY_ADDR (~0ul) @@ -383,7 +384,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]; @@ -440,17 +441,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), (const unsigned char *)&big, + NULL); + of_add_property(dn, property); } /* setup the resource for the newly bound range */ diff --git a/drivers/of/base.c b/drivers/of/base.c index ae03b12..602d2a9 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1817,6 +1817,7 @@ int of_add_property(struct device_node *np, struct property *prop) return rc; } +EXPORT_SYMBOL_GPL(of_add_property); int __of_remove_property(struct device_node *np, struct property *prop) { -- 2.7.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel 2020-03-04 8:47 ` [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu @ 2020-03-13 3:17 ` Oliver O'Halloran 2020-03-16 2:49 ` Pingfan Liu 2020-03-16 2:53 ` Aneesh Kumar K.V 1 sibling, 1 reply; 12+ messages in thread From: Oliver O'Halloran @ 2020-03-13 3:17 UTC (permalink / raw) To: Pingfan Liu Cc: Andrew Donnellan, Frank Rowand, kexec, Rob Herring, Paul Mackerras, Aneesh Kumar K . V, Dan Williams, linuxppc-dev, Hari Bathini On Wed, Mar 4, 2020 at 7:50 PM Pingfan Liu <kernelfans@gmail.com> wrote: > > 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: Andrew Donnellan <ajd@linux.ibm.com> > Cc: Christophe Leroy <christophe.leroy@c-s.fr> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: kexec@lists.infradead.org > --- > note: This patch has not been tested since I can not get such a pseries with pmem. > Please kindly to give some suggestion, thanks. There was some qemu patches to implement the Hcall interface floating around a while ago. I'm not sure they ever made it into upstream qemu though. > --- > arch/powerpc/platforms/pseries/of_helpers.c | 1 + > arch/powerpc/platforms/pseries/papr_scm.c | 33 ++++++++++++++++++++--------- > drivers/of/base.c | 1 + > 3 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c > index 1022e0f..2c7bab4 100644 > --- a/arch/powerpc/platforms/pseries/of_helpers.c > +++ b/arch/powerpc/platforms/pseries/of_helpers.c > @@ -34,6 +34,7 @@ struct property *new_property(const char *name, const int length, > kfree(new); > return NULL; > } > +EXPORT_SYMBOL(new_property); > > /** > * pseries_of_derive_parent - basically like dirname(1) > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c > index 0b4467e..54ae903 100644 > --- a/arch/powerpc/platforms/pseries/papr_scm.c > +++ b/arch/powerpc/platforms/pseries/papr_scm.c > @@ -14,6 +14,7 @@ > #include <linux/delay.h> > > #include <asm/plpar_wrappers.h> > +#include "of_helpers.h" > > #define BIND_ANY_ADDR (~0ul) > > @@ -383,7 +384,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]; > @@ -440,17 +441,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), (const unsigned char *)&big, > + NULL); That should probably be "linux,bound-addr" The other thing that stands out to me is that you aren't removing the property when the region is unbound. As a general rule I'd prefer we didn't hack the DT at runtime, but if we are going to then we should make sure we're not putting anything wrong in there. > + of_add_property(dn, property); > } > > /* setup the resource for the newly bound range */ > diff --git a/drivers/of/base.c b/drivers/of/base.c > index ae03b12..602d2a9 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -1817,6 +1817,7 @@ int of_add_property(struct device_node *np, struct property *prop) > > return rc; > } > +EXPORT_SYMBOL_GPL(of_add_property); > > int __of_remove_property(struct device_node *np, struct property *prop) > { > -- > 2.7.5 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel 2020-03-13 3:17 ` Oliver O'Halloran @ 2020-03-16 2:49 ` Pingfan Liu 0 siblings, 0 replies; 12+ messages in thread From: Pingfan Liu @ 2020-03-16 2:49 UTC (permalink / raw) To: Oliver O'Halloran Cc: Andrew Donnellan, Frank Rowand, Kexec Mailing List, Rob Herring, Paul Mackerras, Aneesh Kumar K . V, Dan Williams, linuxppc-dev, Hari Bathini Appreciate for your kind review. And I have some comment as below. On Fri, Mar 13, 2020 at 11:18 AM Oliver O'Halloran <oohall@gmail.com> wrote: > > On Wed, Mar 4, 2020 at 7:50 PM Pingfan Liu <kernelfans@gmail.com> wrote: > > > > 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: Andrew Donnellan <ajd@linux.ibm.com> > > Cc: Christophe Leroy <christophe.leroy@c-s.fr> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: kexec@lists.infradead.org > > --- > > note: This patch has not been tested since I can not get such a pseries with pmem. > > Please kindly to give some suggestion, thanks. > > There was some qemu patches to implement the Hcall interface floating > around a while ago. I'm not sure they ever made it into upstream qemu > though. Unfortunately, it does not appear in latest qemu code. I think probably virt-pmem has achieved the same feature. > > > --- > > arch/powerpc/platforms/pseries/of_helpers.c | 1 + > > arch/powerpc/platforms/pseries/papr_scm.c | 33 ++++++++++++++++++++--------- > > drivers/of/base.c | 1 + > > 3 files changed, 25 insertions(+), 10 deletions(-) > > > > diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c > > index 1022e0f..2c7bab4 100644 > > --- a/arch/powerpc/platforms/pseries/of_helpers.c > > +++ b/arch/powerpc/platforms/pseries/of_helpers.c > > @@ -34,6 +34,7 @@ struct property *new_property(const char *name, const int length, > > kfree(new); > > return NULL; > > } > > +EXPORT_SYMBOL(new_property); > > > > /** > > * pseries_of_derive_parent - basically like dirname(1) > > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c > > index 0b4467e..54ae903 100644 > > --- a/arch/powerpc/platforms/pseries/papr_scm.c > > +++ b/arch/powerpc/platforms/pseries/papr_scm.c > > @@ -14,6 +14,7 @@ > > #include <linux/delay.h> > > > > #include <asm/plpar_wrappers.h> > > +#include "of_helpers.h" > > > > #define BIND_ANY_ADDR (~0ul) > > > > @@ -383,7 +384,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]; > > @@ -440,17 +441,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), (const unsigned char *)&big, > > + NULL); > > That should probably be "linux,bound-addr" OK, thanks for suggestion. > > The other thing that stands out to me is that you aren't removing the Yes, you are right. I will fix it in V2. > property when the region is unbound. As a general rule I'd prefer we > didn't hack the DT at runtime, but if we are going to then we should > make sure we're not putting anything wrong in there. Actually, the dynamically building of DT is widely used by "kexec -l". The pre-condition for the hacked method is that the bound pmem-addr will not change. And on pseries, during kexec -l/-p, a machine reset will not be invoked, so the bound address should be changed. Thanks, Pingfan [...] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel 2020-03-04 8:47 ` [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu 2020-03-13 3:17 ` Oliver O'Halloran @ 2020-03-16 2:53 ` Aneesh Kumar K.V 2020-03-16 8:37 ` Pingfan Liu 1 sibling, 1 reply; 12+ messages in thread From: Aneesh Kumar K.V @ 2020-03-16 2:53 UTC (permalink / raw) To: Pingfan Liu, linuxppc-dev Cc: Andrew Donnellan, kexec, Rob Herring, Oliver O'Halloran, Paul Mackerras, Dan Williams, Frank Rowand, Hari Bathini On 3/4/20 2:17 PM, Pingfan Liu wrote: > 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. > that should be fixed by faa6d21153fd11e139dd880044521389b34a24f2 Author: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> AuthorDate: Tue Sep 3 18:04:52 2019 +0530 Commit: Michael Ellerman <mpe@ellerman.id.au> CommitDate: Wed Sep 25 08:32:59 2019 +1000 powerpc/nvdimm: use H_SCM_QUERY hcall on H_OVERLAP error Right now we force an unbind of SCM memory at drcindex on H_OVERLAP error. This really slows down operations like kexec where we get the H_OVERLAP error because we don't go through a full hypervisor re init. H_OVERLAP error for a H_SCM_BIND_MEM hcall indicates that SCM memory at drc index is already bound. Since we don't specify a logical memory address for bind hcall, we can use the H_SCM_QUERY hcall to query the already bound logical address. > 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'. > -aneesh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel 2020-03-16 2:53 ` Aneesh Kumar K.V @ 2020-03-16 8:37 ` Pingfan Liu 0 siblings, 0 replies; 12+ messages in thread From: Pingfan Liu @ 2020-03-16 8:37 UTC (permalink / raw) To: Aneesh Kumar K.V Cc: Andrew Donnellan, Frank Rowand, Kexec Mailing List, Rob Herring, Oliver O'Halloran, Paul Mackerras, Dan Williams, linuxppc-dev, Hari Bathini On Mon, Mar 16, 2020 at 10:53 AM Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> wrote: > > On 3/4/20 2:17 PM, Pingfan Liu wrote: > > 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. > > > > > that should be fixed by > > faa6d21153fd11e139dd880044521389b34a24f2 > Author: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > AuthorDate: Tue Sep 3 18:04:52 2019 +0530 > Commit: Michael Ellerman <mpe@ellerman.id.au> > CommitDate: Wed Sep 25 08:32:59 2019 +1000 > > powerpc/nvdimm: use H_SCM_QUERY hcall on H_OVERLAP error > > Right now we force an unbind of SCM memory at drcindex on H_OVERLAP error. > This really slows down operations like kexec where we get the H_OVERLAP > error because we don't go through a full hypervisor re init. > > H_OVERLAP error for a H_SCM_BIND_MEM hcall indicates that SCM memory at > drc index is already bound. Since we don't specify a logical memory > address for bind hcall, we can use the H_SCM_QUERY hcall to query > the already bound logical address. Good to know it. Thanks, Pingfan > > > > > > 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'. > > > > -aneesh > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-03-16 8:40 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-28 9:41 [PATCHv2 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu 2020-02-28 9:41 ` [PATCHv2 2/2] pSeries/papr_scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 0/2] pseries/scm: " Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu 2020-03-05 3:58 ` Andrew Donnellan 2020-03-06 19:59 ` Nathan Lynch 2020-03-09 1:50 ` Pingfan Liu 2020-03-04 8:47 ` [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu 2020-03-13 3:17 ` Oliver O'Halloran 2020-03-16 2:49 ` Pingfan Liu 2020-03-16 2:53 ` Aneesh Kumar K.V 2020-03-16 8:37 ` Pingfan Liu
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.