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

[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 <kernelfans at gmail.com>
> To: linuxppc-dev at lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> Cc: Paul Mackerras <paulus at samba.org>
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: Hari Bathini <hbathini at linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
> Cc: Oliver O'Halloran <oohall at gmail.com>
> Cc: Dan Williams <dan.j.williams at intel.com>
> Cc: kexec at 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