diff mbox series

[kvm-unit-tests,v2,8/8] arm/arm64: psci: don't assume method is hvc

Message ID 20210420190002.383444-9-drjones@redhat.com (mailing list archive)
State New
Headers show
Series arm/arm64: Prepare for target-efi | expand

Commit Message

Andrew Jones April 20, 2021, 7 p.m. UTC
The method can also be smc and it will be when running on bare metal.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/cstart.S       | 22 ++++++++++++++++++++++
 arm/cstart64.S     | 22 ++++++++++++++++++++++
 arm/selftest.c     | 34 +++++++---------------------------
 lib/arm/asm/psci.h | 10 ++++++++--
 lib/arm/psci.c     | 37 +++++++++++++++++++++++++++++--------
 lib/arm/setup.c    |  2 ++
 6 files changed, 90 insertions(+), 37 deletions(-)

Comments

Andrew Jones April 21, 2021, 7:02 a.m. UTC | #1
On Tue, Apr 20, 2021 at 09:00:02PM +0200, Andrew Jones wrote:
> The method can also be smc and it will be when running on bare metal.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/cstart.S       | 22 ++++++++++++++++++++++
>  arm/cstart64.S     | 22 ++++++++++++++++++++++
>  arm/selftest.c     | 34 +++++++---------------------------
>  lib/arm/asm/psci.h | 10 ++++++++--
>  lib/arm/psci.c     | 37 +++++++++++++++++++++++++++++--------
>  lib/arm/setup.c    |  2 ++
>  6 files changed, 90 insertions(+), 37 deletions(-)
> 
> diff --git a/arm/cstart.S b/arm/cstart.S
> index 446966de350d..2401d92cdadc 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -95,6 +95,28 @@ start:
>  
>  .text
>  
> +/*
> + * psci_invoke_hvc / psci_invoke_smc
> + *
> + * Inputs:
> + *   r0 -- function_id
> + *   r1 -- arg0
> + *   r2 -- arg1
> + *   r3 -- arg2
> + *
> + * Outputs:
> + *   r0 -- return code
> + */
> +.globl psci_invoke_hvc
> +psci_invoke_hvc:
> +	hvc	#0
> +	mov	pc, lr
> +
> +.globl psci_invoke_smc
> +psci_invoke_smc:
> +	smc	#0
> +	mov	pc, lr
> +
>  enable_vfp:
>  	/* Enable full access to CP10 and CP11: */
>  	mov	r0, #(3 << 22 | 3 << 20)
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index 42ba3a3ca249..7610e28f06dd 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -109,6 +109,28 @@ start:
>  
>  .text
>  
> +/*
> + * psci_invoke_hvc / psci_invoke_smc
> + *
> + * Inputs:
> + *   x0 -- function_id
> + *   x1 -- arg0
> + *   x2 -- arg1
> + *   x3 -- arg2
> + *
> + * Outputs:
> + *   x0 -- return code
> + */
> +.globl psci_invoke_hvc
> +psci_invoke_hvc:
> +	hvc	#0
> +	ret
> +
> +.globl psci_invoke_smc
> +psci_invoke_smc:
> +	smc	#0
> +	ret
> +
>  get_mmu_off:
>  	adrp	x0, auxinfo
>  	ldr	x0, [x0, :lo12:auxinfo + 8]
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 4495b161cdd5..9f459ed3d571 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -400,33 +400,13 @@ static void check_vectors(void *arg __unused)
>  	exit(report_summary());
>  }
>  
> -static bool psci_check(void)
> +static void psci_print(void)
>  {
> -	const struct fdt_property *method;
> -	int node, len, ver;
> -
> -	node = fdt_node_offset_by_compatible(dt_fdt(), -1, "arm,psci-0.2");
> -	if (node < 0) {
> -		printf("PSCI v0.2 compatibility required\n");
> -		return false;
> -	}
> -
> -	method = fdt_get_property(dt_fdt(), node, "method", &len);
> -	if (method == NULL) {
> -		printf("bad psci device tree node\n");
> -		return false;
> -	}
> -
> -	if (len < 4 || strcmp(method->data, "hvc") != 0) {
> -		printf("psci method must be hvc\n");
> -		return false;
> -	}
> -
> -	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> -	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
> -				       PSCI_VERSION_MINOR(ver));
> -
> -	return true;
> +	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> +	report_info("PSCI version: %d.%d", PSCI_VERSION_MAJOR(ver),
> +					  PSCI_VERSION_MINOR(ver));
> +	report_info("PSCI method: %s", psci_invoke == psci_invoke_hvc ?
> +				       "hvc" : "smc");
>  }
>  
>  static void cpu_report(void *data __unused)
> @@ -465,7 +445,7 @@ int main(int argc, char **argv)
>  
>  	} else if (strcmp(argv[1], "smp") == 0) {
>  
> -		report(psci_check(), "PSCI version");
> +		psci_print();
>  		on_cpus(cpu_report, NULL);
>  		while (!cpumask_full(&ready))
>  			cpu_relax();
> diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
> index 7b956bf5987d..2820c0a3afc7 100644
> --- a/lib/arm/asm/psci.h
> +++ b/lib/arm/asm/psci.h
> @@ -3,8 +3,14 @@
>  #include <libcflat.h>
>  #include <linux/psci.h>
>  
> -extern int psci_invoke(unsigned long function_id, unsigned long arg0,
> -		       unsigned long arg1, unsigned long arg2);
> +typedef int (*psci_invoke_fn)(unsigned long function_id, unsigned long arg0,
> +			      unsigned long arg1, unsigned long arg2);
> +extern psci_invoke_fn psci_invoke;
> +extern int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
> +			   unsigned long arg1, unsigned long arg2);
> +extern int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
> +			   unsigned long arg1, unsigned long arg2);

Hmm, I forgot to change function_id to 'unsigned int'.

> +extern void psci_set_conduit(void);
>  extern int psci_cpu_on(unsigned long cpuid, unsigned long entry_point);
>  extern void psci_system_reset(void);
>  extern int cpu_psci_cpu_boot(unsigned int cpu);
> diff --git a/lib/arm/psci.c b/lib/arm/psci.c
> index 936c83948b6a..168786dcf792 100644
> --- a/lib/arm/psci.c
> +++ b/lib/arm/psci.c
> @@ -6,22 +6,23 @@
>   *
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
> +#include <devicetree.h>
>  #include <asm/psci.h>
>  #include <asm/setup.h>
>  #include <asm/page.h>
>  #include <asm/smp.h>
>  
> -__attribute__((noinline))
> -int psci_invoke(unsigned long function_id, unsigned long arg0,
> -		unsigned long arg1, unsigned long arg2)
> +extern void halt(void);

And, this is a left over from an earlier version of psci_invoke_none.

Will fix these things for v3.

> +
> +static int psci_invoke_none(unsigned long function_id, unsigned long arg0,
> +			    unsigned long arg1, unsigned long arg2)
>  {
> -	asm volatile(
> -		"hvc #0"
> -	: "+r" (function_id)
> -	: "r" (arg0), "r" (arg1), "r" (arg2));
> -	return function_id;
> +	printf("No PSCI method configured! Can't invoke...\n");
> +	return PSCI_RET_NOT_PRESENT;
>  }
>  
> +psci_invoke_fn psci_invoke = psci_invoke_none;
> +
>  int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
>  {
>  #ifdef __arm__
> @@ -56,3 +57,23 @@ void psci_system_off(void)
>  	int err = psci_invoke(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
>  	printf("CPU%d unable to do system off (error = %d)\n", smp_processor_id(), err);
>  }
> +
> +void psci_set_conduit(void)
> +{
> +	const void *fdt = dt_fdt();
> +	const struct fdt_property *method;
> +	int node, len;
> +
> +	node = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-0.2");
> +	assert_msg(node >= 0, "PSCI v0.2 compatibility required");
> +
> +	method = fdt_get_property(fdt, node, "method", &len);
> +	assert(method != NULL && len == 4);
> +
> +	if (strcmp(method->data, "hvc") == 0)
> +		psci_invoke = psci_invoke_hvc;
> +	else if (strcmp(method->data, "smc") == 0)
> +		psci_invoke = psci_invoke_smc;
> +	else
> +		assert_msg(false, "Unknown PSCI conduit: %s", method->data);
> +}
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index a5ebec3c5a12..07d52d2e5fe6 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -25,6 +25,7 @@
>  #include <asm/processor.h>
>  #include <asm/smp.h>
>  #include <asm/timer.h>
> +#include <asm/psci.h>
>  
>  #include "io.h"
>  
> @@ -266,6 +267,7 @@ void setup(const void *fdt, phys_addr_t freemem_start)
>  	mem_regions_add_assumed();
>  	mem_init(PAGE_ALIGN((unsigned long)freemem));
>  
> +	psci_set_conduit();
>  	cpu_init();
>  
>  	/* cpu_init must be called before thread_info_init */
> -- 
> 2.30.2
> 

Thanks,
drew
Andrew Jones April 22, 2021, 4:17 p.m. UTC | #2
For v3, I've done the following changes (inline)

On Wed, Apr 21, 2021 at 09:02:06AM +0200, Andrew Jones wrote:
> On Tue, Apr 20, 2021 at 09:00:02PM +0200, Andrew Jones wrote:
> > The method can also be smc and it will be when running on bare metal.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  arm/cstart.S       | 22 ++++++++++++++++++++++
> >  arm/cstart64.S     | 22 ++++++++++++++++++++++
> >  arm/selftest.c     | 34 +++++++---------------------------
> >  lib/arm/asm/psci.h | 10 ++++++++--
> >  lib/arm/psci.c     | 37 +++++++++++++++++++++++++++++--------
> >  lib/arm/setup.c    |  2 ++
> >  6 files changed, 90 insertions(+), 37 deletions(-)
> > 
> > diff --git a/arm/cstart.S b/arm/cstart.S
> > index 446966de350d..2401d92cdadc 100644
> > --- a/arm/cstart.S
> > +++ b/arm/cstart.S
> > @@ -95,6 +95,28 @@ start:
> >  
> >  .text
> >  
> > +/*
> > + * psci_invoke_hvc / psci_invoke_smc
> > + *
> > + * Inputs:
> > + *   r0 -- function_id
> > + *   r1 -- arg0
> > + *   r2 -- arg1
> > + *   r3 -- arg2
> > + *
> > + * Outputs:
> > + *   r0 -- return code
> > + */
> > +.globl psci_invoke_hvc
> > +psci_invoke_hvc:
> > +	hvc	#0
> > +	mov	pc, lr
> > +
> > +.globl psci_invoke_smc
> > +psci_invoke_smc:
> > +	smc	#0
> > +	mov	pc, lr
> > +
> >  enable_vfp:
> >  	/* Enable full access to CP10 and CP11: */
> >  	mov	r0, #(3 << 22 | 3 << 20)
> > diff --git a/arm/cstart64.S b/arm/cstart64.S
> > index 42ba3a3ca249..7610e28f06dd 100644
> > --- a/arm/cstart64.S
> > +++ b/arm/cstart64.S
> > @@ -109,6 +109,28 @@ start:
> >  
> >  .text
> >  
> > +/*
> > + * psci_invoke_hvc / psci_invoke_smc
> > + *
> > + * Inputs:
> > + *   x0 -- function_id

changed this comment to be 'w0 -- function_id'

> > + *   x1 -- arg0
> > + *   x2 -- arg1
> > + *   x3 -- arg2
> > + *
> > + * Outputs:
> > + *   x0 -- return code
> > + */
> > +.globl psci_invoke_hvc
> > +psci_invoke_hvc:
> > +	hvc	#0
> > +	ret
> > +
> > +.globl psci_invoke_smc
> > +psci_invoke_smc:
> > +	smc	#0
> > +	ret
> > +
> >  get_mmu_off:
> >  	adrp	x0, auxinfo
> >  	ldr	x0, [x0, :lo12:auxinfo + 8]
> > diff --git a/arm/selftest.c b/arm/selftest.c
> > index 4495b161cdd5..9f459ed3d571 100644
> > --- a/arm/selftest.c
> > +++ b/arm/selftest.c
> > @@ -400,33 +400,13 @@ static void check_vectors(void *arg __unused)
> >  	exit(report_summary());
> >  }
> >  
> > -static bool psci_check(void)
> > +static void psci_print(void)
> >  {
> > -	const struct fdt_property *method;
> > -	int node, len, ver;
> > -
> > -	node = fdt_node_offset_by_compatible(dt_fdt(), -1, "arm,psci-0.2");
> > -	if (node < 0) {
> > -		printf("PSCI v0.2 compatibility required\n");
> > -		return false;
> > -	}
> > -
> > -	method = fdt_get_property(dt_fdt(), node, "method", &len);
> > -	if (method == NULL) {
> > -		printf("bad psci device tree node\n");
> > -		return false;
> > -	}
> > -
> > -	if (len < 4 || strcmp(method->data, "hvc") != 0) {
> > -		printf("psci method must be hvc\n");
> > -		return false;
> > -	}
> > -
> > -	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> > -	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
> > -				       PSCI_VERSION_MINOR(ver));
> > -
> > -	return true;
> > +	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> > +	report_info("PSCI version: %d.%d", PSCI_VERSION_MAJOR(ver),
> > +					  PSCI_VERSION_MINOR(ver));
> > +	report_info("PSCI method: %s", psci_invoke == psci_invoke_hvc ?
> > +				       "hvc" : "smc");
> >  }
> >  
> >  static void cpu_report(void *data __unused)
> > @@ -465,7 +445,7 @@ int main(int argc, char **argv)
> >  
> >  	} else if (strcmp(argv[1], "smp") == 0) {
> >  
> > -		report(psci_check(), "PSCI version");
> > +		psci_print();
> >  		on_cpus(cpu_report, NULL);
> >  		while (!cpumask_full(&ready))
> >  			cpu_relax();
> > diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
> > index 7b956bf5987d..2820c0a3afc7 100644
> > --- a/lib/arm/asm/psci.h
> > +++ b/lib/arm/asm/psci.h
> > @@ -3,8 +3,14 @@
> >  #include <libcflat.h>
> >  #include <linux/psci.h>
> >  
> > -extern int psci_invoke(unsigned long function_id, unsigned long arg0,
> > -		       unsigned long arg1, unsigned long arg2);
> > +typedef int (*psci_invoke_fn)(unsigned long function_id, unsigned long arg0,
> > +			      unsigned long arg1, unsigned long arg2);
> > +extern psci_invoke_fn psci_invoke;
> > +extern int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
> > +			   unsigned long arg1, unsigned long arg2);
> > +extern int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
> > +			   unsigned long arg1, unsigned long arg2);

The prototypes are now

long invoke_fn(unsigned int function_id, unsigned long arg0,
               unsigned long arg1, unsigned long arg2)

Notice the return value changed to long and the function_id to
unsigned int.


I also improved the commit message by adding the following:

   The method can be smc in addition to hvc, and it will be when running
   on bare metal. Additionally, we move the invocations to assembly so
   we don't have to rely on compiler assumptions. We also fix the
   prototype of psci_invoke. It should return long, not int, and
   function_id should be an unsigned int, not an unsigned long.

Thanks,
drew
Alexandru Elisei April 26, 2021, 2:57 p.m. UTC | #3
Hi Drew,

On 4/22/21 5:17 PM, Andrew Jones wrote:
> For v3, I've done the following changes (inline)
>
> On Wed, Apr 21, 2021 at 09:02:06AM +0200, Andrew Jones wrote:
>> On Tue, Apr 20, 2021 at 09:00:02PM +0200, Andrew Jones wrote:
>>> The method can also be smc and it will be when running on bare metal.
>>>
>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>> ---
>>>  arm/cstart.S       | 22 ++++++++++++++++++++++
>>>  arm/cstart64.S     | 22 ++++++++++++++++++++++
>>>  arm/selftest.c     | 34 +++++++---------------------------
>>>  lib/arm/asm/psci.h | 10 ++++++++--
>>>  lib/arm/psci.c     | 37 +++++++++++++++++++++++++++++--------
>>>  lib/arm/setup.c    |  2 ++
>>>  6 files changed, 90 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/arm/cstart.S b/arm/cstart.S
>>> index 446966de350d..2401d92cdadc 100644
>>> --- a/arm/cstart.S
>>> +++ b/arm/cstart.S
>>> @@ -95,6 +95,28 @@ start:
>>>  
>>>  .text
>>>  
>>> +/*
>>> + * psci_invoke_hvc / psci_invoke_smc
>>> + *
>>> + * Inputs:
>>> + *   r0 -- function_id
>>> + *   r1 -- arg0
>>> + *   r2 -- arg1
>>> + *   r3 -- arg2
>>> + *
>>> + * Outputs:
>>> + *   r0 -- return code
>>> + */
>>> +.globl psci_invoke_hvc
>>> +psci_invoke_hvc:
>>> +	hvc	#0
>>> +	mov	pc, lr
>>> +
>>> +.globl psci_invoke_smc
>>> +psci_invoke_smc:
>>> +	smc	#0
>>> +	mov	pc, lr
>>> +
>>>  enable_vfp:
>>>  	/* Enable full access to CP10 and CP11: */
>>>  	mov	r0, #(3 << 22 | 3 << 20)
>>> diff --git a/arm/cstart64.S b/arm/cstart64.S
>>> index 42ba3a3ca249..7610e28f06dd 100644
>>> --- a/arm/cstart64.S
>>> +++ b/arm/cstart64.S
>>> @@ -109,6 +109,28 @@ start:
>>>  
>>>  .text
>>>  
>>> +/*
>>> + * psci_invoke_hvc / psci_invoke_smc
>>> + *
>>> + * Inputs:
>>> + *   x0 -- function_id
> changed this comment to be 'w0 -- function_id'
>
>>> + *   x1 -- arg0
>>> + *   x2 -- arg1
>>> + *   x3 -- arg2
>>> + *
>>> + * Outputs:
>>> + *   x0 -- return code
>>> + */
>>> +.globl psci_invoke_hvc
>>> +psci_invoke_hvc:
>>> +	hvc	#0
>>> +	ret
>>> +
>>> +.globl psci_invoke_smc
>>> +psci_invoke_smc:
>>> +	smc	#0
>>> +	ret
>>> +
>>>  get_mmu_off:
>>>  	adrp	x0, auxinfo
>>>  	ldr	x0, [x0, :lo12:auxinfo + 8]
>>> diff --git a/arm/selftest.c b/arm/selftest.c
>>> index 4495b161cdd5..9f459ed3d571 100644
>>> --- a/arm/selftest.c
>>> +++ b/arm/selftest.c
>>> @@ -400,33 +400,13 @@ static void check_vectors(void *arg __unused)
>>>  	exit(report_summary());
>>>  }
>>>  
>>> -static bool psci_check(void)
>>> +static void psci_print(void)
>>>  {
>>> -	const struct fdt_property *method;
>>> -	int node, len, ver;
>>> -
>>> -	node = fdt_node_offset_by_compatible(dt_fdt(), -1, "arm,psci-0.2");
>>> -	if (node < 0) {
>>> -		printf("PSCI v0.2 compatibility required\n");
>>> -		return false;
>>> -	}
>>> -
>>> -	method = fdt_get_property(dt_fdt(), node, "method", &len);
>>> -	if (method == NULL) {
>>> -		printf("bad psci device tree node\n");
>>> -		return false;
>>> -	}
>>> -
>>> -	if (len < 4 || strcmp(method->data, "hvc") != 0) {
>>> -		printf("psci method must be hvc\n");
>>> -		return false;
>>> -	}
>>> -
>>> -	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>>> -	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
>>> -				       PSCI_VERSION_MINOR(ver));
>>> -
>>> -	return true;
>>> +	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>>> +	report_info("PSCI version: %d.%d", PSCI_VERSION_MAJOR(ver),
>>> +					  PSCI_VERSION_MINOR(ver));
>>> +	report_info("PSCI method: %s", psci_invoke == psci_invoke_hvc ?
>>> +				       "hvc" : "smc");
>>>  }
>>>  
>>>  static void cpu_report(void *data __unused)
>>> @@ -465,7 +445,7 @@ int main(int argc, char **argv)
>>>  
>>>  	} else if (strcmp(argv[1], "smp") == 0) {
>>>  
>>> -		report(psci_check(), "PSCI version");
>>> +		psci_print();
>>>  		on_cpus(cpu_report, NULL);
>>>  		while (!cpumask_full(&ready))
>>>  			cpu_relax();
>>> diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
>>> index 7b956bf5987d..2820c0a3afc7 100644
>>> --- a/lib/arm/asm/psci.h
>>> +++ b/lib/arm/asm/psci.h
>>> @@ -3,8 +3,14 @@
>>>  #include <libcflat.h>
>>>  #include <linux/psci.h>
>>>  
>>> -extern int psci_invoke(unsigned long function_id, unsigned long arg0,
>>> -		       unsigned long arg1, unsigned long arg2);
>>> +typedef int (*psci_invoke_fn)(unsigned long function_id, unsigned long arg0,
>>> +			      unsigned long arg1, unsigned long arg2);
>>> +extern psci_invoke_fn psci_invoke;
>>> +extern int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
>>> +			   unsigned long arg1, unsigned long arg2);
>>> +extern int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
>>> +			   unsigned long arg1, unsigned long arg2);
> The prototypes are now
>
> long invoke_fn(unsigned int function_id, unsigned long arg0,
>                unsigned long arg1, unsigned long arg2)
>
> Notice the return value changed to long and the function_id to
> unsigned int.

Strictly speaking, arm always returns an unsigned long (32bits), but arm64 can
return either an unsigned long (64bits) when using SMC64/HVC64, or an unsigned int
(32bits) when using SMC32/HVC32. But, I did check C99 and it says that the
original value is unchanged if it fits in the new type (section 6.3.1.3), so we
can just cast the result to an unsigned if we ever want to use SMC32/HVC32 on arm64.

The declaration for psci_invoke_none should also be changed, but otherwise looks good.

Thanks,

Alex

>
>
> I also improved the commit message by adding the following:
>
>    The method can be smc in addition to hvc, and it will be when running
>    on bare metal. Additionally, we move the invocations to assembly so
>    we don't have to rely on compiler assumptions. We also fix the
>    prototype of psci_invoke. It should return long, not int, and
>    function_id should be an unsigned int, not an unsigned long.
>
> Thanks,
> drew
>
Andrew Jones April 26, 2021, 4:35 p.m. UTC | #4
On Mon, Apr 26, 2021 at 03:57:34PM +0100, Alexandru Elisei wrote:
> Hi Drew,
> 
> On 4/22/21 5:17 PM, Andrew Jones wrote:
> > For v3, I've done the following changes (inline)
> >
> > On Wed, Apr 21, 2021 at 09:02:06AM +0200, Andrew Jones wrote:
> >> On Tue, Apr 20, 2021 at 09:00:02PM +0200, Andrew Jones wrote:
> >>> The method can also be smc and it will be when running on bare metal.
> >>>
> >>> Signed-off-by: Andrew Jones <drjones@redhat.com>
> >>> ---
> >>>  arm/cstart.S       | 22 ++++++++++++++++++++++
> >>>  arm/cstart64.S     | 22 ++++++++++++++++++++++
> >>>  arm/selftest.c     | 34 +++++++---------------------------
> >>>  lib/arm/asm/psci.h | 10 ++++++++--
> >>>  lib/arm/psci.c     | 37 +++++++++++++++++++++++++++++--------
> >>>  lib/arm/setup.c    |  2 ++
> >>>  6 files changed, 90 insertions(+), 37 deletions(-)
> >>>
> >>> diff --git a/arm/cstart.S b/arm/cstart.S
> >>> index 446966de350d..2401d92cdadc 100644
> >>> --- a/arm/cstart.S
> >>> +++ b/arm/cstart.S
> >>> @@ -95,6 +95,28 @@ start:
> >>>  
> >>>  .text
> >>>  
> >>> +/*
> >>> + * psci_invoke_hvc / psci_invoke_smc
> >>> + *
> >>> + * Inputs:
> >>> + *   r0 -- function_id
> >>> + *   r1 -- arg0
> >>> + *   r2 -- arg1
> >>> + *   r3 -- arg2
> >>> + *
> >>> + * Outputs:
> >>> + *   r0 -- return code
> >>> + */
> >>> +.globl psci_invoke_hvc
> >>> +psci_invoke_hvc:
> >>> +	hvc	#0
> >>> +	mov	pc, lr
> >>> +
> >>> +.globl psci_invoke_smc
> >>> +psci_invoke_smc:
> >>> +	smc	#0
> >>> +	mov	pc, lr
> >>> +
> >>>  enable_vfp:
> >>>  	/* Enable full access to CP10 and CP11: */
> >>>  	mov	r0, #(3 << 22 | 3 << 20)
> >>> diff --git a/arm/cstart64.S b/arm/cstart64.S
> >>> index 42ba3a3ca249..7610e28f06dd 100644
> >>> --- a/arm/cstart64.S
> >>> +++ b/arm/cstart64.S
> >>> @@ -109,6 +109,28 @@ start:
> >>>  
> >>>  .text
> >>>  
> >>> +/*
> >>> + * psci_invoke_hvc / psci_invoke_smc
> >>> + *
> >>> + * Inputs:
> >>> + *   x0 -- function_id
> > changed this comment to be 'w0 -- function_id'
> >
> >>> + *   x1 -- arg0
> >>> + *   x2 -- arg1
> >>> + *   x3 -- arg2
> >>> + *
> >>> + * Outputs:
> >>> + *   x0 -- return code
> >>> + */
> >>> +.globl psci_invoke_hvc
> >>> +psci_invoke_hvc:
> >>> +	hvc	#0
> >>> +	ret
> >>> +
> >>> +.globl psci_invoke_smc
> >>> +psci_invoke_smc:
> >>> +	smc	#0
> >>> +	ret
> >>> +
> >>>  get_mmu_off:
> >>>  	adrp	x0, auxinfo
> >>>  	ldr	x0, [x0, :lo12:auxinfo + 8]
> >>> diff --git a/arm/selftest.c b/arm/selftest.c
> >>> index 4495b161cdd5..9f459ed3d571 100644
> >>> --- a/arm/selftest.c
> >>> +++ b/arm/selftest.c
> >>> @@ -400,33 +400,13 @@ static void check_vectors(void *arg __unused)
> >>>  	exit(report_summary());
> >>>  }
> >>>  
> >>> -static bool psci_check(void)
> >>> +static void psci_print(void)
> >>>  {
> >>> -	const struct fdt_property *method;
> >>> -	int node, len, ver;
> >>> -
> >>> -	node = fdt_node_offset_by_compatible(dt_fdt(), -1, "arm,psci-0.2");
> >>> -	if (node < 0) {
> >>> -		printf("PSCI v0.2 compatibility required\n");
> >>> -		return false;
> >>> -	}
> >>> -
> >>> -	method = fdt_get_property(dt_fdt(), node, "method", &len);
> >>> -	if (method == NULL) {
> >>> -		printf("bad psci device tree node\n");
> >>> -		return false;
> >>> -	}
> >>> -
> >>> -	if (len < 4 || strcmp(method->data, "hvc") != 0) {
> >>> -		printf("psci method must be hvc\n");
> >>> -		return false;
> >>> -	}
> >>> -
> >>> -	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> >>> -	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
> >>> -				       PSCI_VERSION_MINOR(ver));
> >>> -
> >>> -	return true;
> >>> +	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> >>> +	report_info("PSCI version: %d.%d", PSCI_VERSION_MAJOR(ver),
> >>> +					  PSCI_VERSION_MINOR(ver));
> >>> +	report_info("PSCI method: %s", psci_invoke == psci_invoke_hvc ?
> >>> +				       "hvc" : "smc");
> >>>  }
> >>>  
> >>>  static void cpu_report(void *data __unused)
> >>> @@ -465,7 +445,7 @@ int main(int argc, char **argv)
> >>>  
> >>>  	} else if (strcmp(argv[1], "smp") == 0) {
> >>>  
> >>> -		report(psci_check(), "PSCI version");
> >>> +		psci_print();
> >>>  		on_cpus(cpu_report, NULL);
> >>>  		while (!cpumask_full(&ready))
> >>>  			cpu_relax();
> >>> diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
> >>> index 7b956bf5987d..2820c0a3afc7 100644
> >>> --- a/lib/arm/asm/psci.h
> >>> +++ b/lib/arm/asm/psci.h
> >>> @@ -3,8 +3,14 @@
> >>>  #include <libcflat.h>
> >>>  #include <linux/psci.h>
> >>>  
> >>> -extern int psci_invoke(unsigned long function_id, unsigned long arg0,
> >>> -		       unsigned long arg1, unsigned long arg2);
> >>> +typedef int (*psci_invoke_fn)(unsigned long function_id, unsigned long arg0,
> >>> +			      unsigned long arg1, unsigned long arg2);
> >>> +extern psci_invoke_fn psci_invoke;
> >>> +extern int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
> >>> +			   unsigned long arg1, unsigned long arg2);
> >>> +extern int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
> >>> +			   unsigned long arg1, unsigned long arg2);
> > The prototypes are now
> >
> > long invoke_fn(unsigned int function_id, unsigned long arg0,
> >                unsigned long arg1, unsigned long arg2)
> >
> > Notice the return value changed to long and the function_id to
> > unsigned int.
> 
> Strictly speaking, arm always returns an unsigned long (32bits), but arm64 can
> return either an unsigned long (64bits) when using SMC64/HVC64, or an unsigned int
> (32bits) when using SMC32/HVC32.

Hmm, where did you see that? Because section 5.1 of the SMC calling
convention disagrees

"""
5.1 Error codes
Errors codes that are returned in R0, W0 and X0 are signed integers of the
appropriate size:
* In AArch32:
  o When using the SMC32/HVC32 calling convention, error codes, which are
    returned in R0, are 32-bit signed integers.
* In AArch64:
  o When using the SMC64/HVC64 calling convention, error codes, which are
    returned in X0, are 64-bit signed integers.
  o When using the SMC32/HVC32 calling convention, error codes, which are
    returned in W0, are 32-bit signed integers. X0[63:32] is UNDEFINED.
"""

And 5.2.2 from the Power State Coordination Interface manual

"""
5.2.2 Return error codes
Table 6 defines the values for error codes used with PSCI functions. All
errors are considered to be 32-bit signed integers.
"""

Thanks,
drew
Alexandru Elisei April 27, 2021, 3:47 p.m. UTC | #5
Hi Drew,

On 4/26/21 5:35 PM, Andrew Jones wrote:
> On Mon, Apr 26, 2021 at 03:57:34PM +0100, Alexandru Elisei wrote:
>> Hi Drew,
>>
>> On 4/22/21 5:17 PM, Andrew Jones wrote:
>>> For v3, I've done the following changes (inline)
>>>
>>> On Wed, Apr 21, 2021 at 09:02:06AM +0200, Andrew Jones wrote:
>>>> On Tue, Apr 20, 2021 at 09:00:02PM +0200, Andrew Jones wrote:
>>>>> The method can also be smc and it will be when running on bare metal.
>>>>>
>>>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>>>> ---
>>>>>  arm/cstart.S       | 22 ++++++++++++++++++++++
>>>>>  arm/cstart64.S     | 22 ++++++++++++++++++++++
>>>>>  arm/selftest.c     | 34 +++++++---------------------------
>>>>>  lib/arm/asm/psci.h | 10 ++++++++--
>>>>>  lib/arm/psci.c     | 37 +++++++++++++++++++++++++++++--------
>>>>>  lib/arm/setup.c    |  2 ++
>>>>>  6 files changed, 90 insertions(+), 37 deletions(-)
>>>>>
>>>>> diff --git a/arm/cstart.S b/arm/cstart.S
>>>>> index 446966de350d..2401d92cdadc 100644
>>>>> --- a/arm/cstart.S
>>>>> +++ b/arm/cstart.S
>>>>> @@ -95,6 +95,28 @@ start:
>>>>>  
>>>>>  .text
>>>>>  
>>>>> +/*
>>>>> + * psci_invoke_hvc / psci_invoke_smc
>>>>> + *
>>>>> + * Inputs:
>>>>> + *   r0 -- function_id
>>>>> + *   r1 -- arg0
>>>>> + *   r2 -- arg1
>>>>> + *   r3 -- arg2
>>>>> + *
>>>>> + * Outputs:
>>>>> + *   r0 -- return code
>>>>> + */
>>>>> +.globl psci_invoke_hvc
>>>>> +psci_invoke_hvc:
>>>>> +	hvc	#0
>>>>> +	mov	pc, lr
>>>>> +
>>>>> +.globl psci_invoke_smc
>>>>> +psci_invoke_smc:
>>>>> +	smc	#0
>>>>> +	mov	pc, lr
>>>>> +
>>>>>  enable_vfp:
>>>>>  	/* Enable full access to CP10 and CP11: */
>>>>>  	mov	r0, #(3 << 22 | 3 << 20)
>>>>> diff --git a/arm/cstart64.S b/arm/cstart64.S
>>>>> index 42ba3a3ca249..7610e28f06dd 100644
>>>>> --- a/arm/cstart64.S
>>>>> +++ b/arm/cstart64.S
>>>>> @@ -109,6 +109,28 @@ start:
>>>>>  
>>>>>  .text
>>>>>  
>>>>> +/*
>>>>> + * psci_invoke_hvc / psci_invoke_smc
>>>>> + *
>>>>> + * Inputs:
>>>>> + *   x0 -- function_id
>>> changed this comment to be 'w0 -- function_id'
>>>
>>>>> + *   x1 -- arg0
>>>>> + *   x2 -- arg1
>>>>> + *   x3 -- arg2
>>>>> + *
>>>>> + * Outputs:
>>>>> + *   x0 -- return code
>>>>> + */
>>>>> +.globl psci_invoke_hvc
>>>>> +psci_invoke_hvc:
>>>>> +	hvc	#0
>>>>> +	ret
>>>>> +
>>>>> +.globl psci_invoke_smc
>>>>> +psci_invoke_smc:
>>>>> +	smc	#0
>>>>> +	ret
>>>>> +
>>>>>  get_mmu_off:
>>>>>  	adrp	x0, auxinfo
>>>>>  	ldr	x0, [x0, :lo12:auxinfo + 8]
>>>>> diff --git a/arm/selftest.c b/arm/selftest.c
>>>>> index 4495b161cdd5..9f459ed3d571 100644
>>>>> --- a/arm/selftest.c
>>>>> +++ b/arm/selftest.c
>>>>> @@ -400,33 +400,13 @@ static void check_vectors(void *arg __unused)
>>>>>  	exit(report_summary());
>>>>>  }
>>>>>  
>>>>> -static bool psci_check(void)
>>>>> +static void psci_print(void)
>>>>>  {
>>>>> -	const struct fdt_property *method;
>>>>> -	int node, len, ver;
>>>>> -
>>>>> -	node = fdt_node_offset_by_compatible(dt_fdt(), -1, "arm,psci-0.2");
>>>>> -	if (node < 0) {
>>>>> -		printf("PSCI v0.2 compatibility required\n");
>>>>> -		return false;
>>>>> -	}
>>>>> -
>>>>> -	method = fdt_get_property(dt_fdt(), node, "method", &len);
>>>>> -	if (method == NULL) {
>>>>> -		printf("bad psci device tree node\n");
>>>>> -		return false;
>>>>> -	}
>>>>> -
>>>>> -	if (len < 4 || strcmp(method->data, "hvc") != 0) {
>>>>> -		printf("psci method must be hvc\n");
>>>>> -		return false;
>>>>> -	}
>>>>> -
>>>>> -	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>>>>> -	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
>>>>> -				       PSCI_VERSION_MINOR(ver));
>>>>> -
>>>>> -	return true;
>>>>> +	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
>>>>> +	report_info("PSCI version: %d.%d", PSCI_VERSION_MAJOR(ver),
>>>>> +					  PSCI_VERSION_MINOR(ver));
>>>>> +	report_info("PSCI method: %s", psci_invoke == psci_invoke_hvc ?
>>>>> +				       "hvc" : "smc");
>>>>>  }
>>>>>  
>>>>>  static void cpu_report(void *data __unused)
>>>>> @@ -465,7 +445,7 @@ int main(int argc, char **argv)
>>>>>  
>>>>>  	} else if (strcmp(argv[1], "smp") == 0) {
>>>>>  
>>>>> -		report(psci_check(), "PSCI version");
>>>>> +		psci_print();
>>>>>  		on_cpus(cpu_report, NULL);
>>>>>  		while (!cpumask_full(&ready))
>>>>>  			cpu_relax();
>>>>> diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
>>>>> index 7b956bf5987d..2820c0a3afc7 100644
>>>>> --- a/lib/arm/asm/psci.h
>>>>> +++ b/lib/arm/asm/psci.h
>>>>> @@ -3,8 +3,14 @@
>>>>>  #include <libcflat.h>
>>>>>  #include <linux/psci.h>
>>>>>  
>>>>> -extern int psci_invoke(unsigned long function_id, unsigned long arg0,
>>>>> -		       unsigned long arg1, unsigned long arg2);
>>>>> +typedef int (*psci_invoke_fn)(unsigned long function_id, unsigned long arg0,
>>>>> +			      unsigned long arg1, unsigned long arg2);
>>>>> +extern psci_invoke_fn psci_invoke;
>>>>> +extern int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
>>>>> +			   unsigned long arg1, unsigned long arg2);
>>>>> +extern int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
>>>>> +			   unsigned long arg1, unsigned long arg2);
>>> The prototypes are now
>>>
>>> long invoke_fn(unsigned int function_id, unsigned long arg0,
>>>                unsigned long arg1, unsigned long arg2)
>>>
>>> Notice the return value changed to long and the function_id to
>>> unsigned int.
>> Strictly speaking, arm always returns an unsigned long (32bits), but arm64 can
>> return either an unsigned long (64bits) when using SMC64/HVC64, or an unsigned int
>> (32bits) when using SMC32/HVC32.
> Hmm, where did you see that? Because section 5.1 of the SMC calling
> convention disagrees
>
> """
> 5.1 Error codes
> Errors codes that are returned in R0, W0 and X0 are signed integers of the
> appropriate size:
> * In AArch32:
>   o When using the SMC32/HVC32 calling convention, error codes, which are
>     returned in R0, are 32-bit signed integers.
> * In AArch64:
>   o When using the SMC64/HVC64 calling convention, error codes, which are
>     returned in X0, are 64-bit signed integers.
>   o When using the SMC32/HVC32 calling convention, error codes, which are
>     returned in W0, are 32-bit signed integers. X0[63:32] is UNDEFINED.
> """

I made a mistake, I read "long" and thought unsigned integer instead of signed.

I went over the PSCI spec again, and PSCI_STAT_RESIDENCY and PSCI_STAT_COUNT
return an uint64_t or uint32_t, depending on the convention. Those were introduced
in PSCI 1.0, they are optional and none of the tests use them, so I guess we can
ignore them for now and revisit the prototypes if we ever need to.

Thanks,

Alex

>
> And 5.2.2 from the Power State Coordination Interface manual
>
> """
> 5.2.2 Return error codes
> Table 6 defines the values for error codes used with PSCI functions. All
> errors are considered to be 32-bit signed integers.
> """
>
> Thanks,
> drew
>
diff mbox series

Patch

diff --git a/arm/cstart.S b/arm/cstart.S
index 446966de350d..2401d92cdadc 100644
--- a/arm/cstart.S
+++ b/arm/cstart.S
@@ -95,6 +95,28 @@  start:
 
 .text
 
+/*
+ * psci_invoke_hvc / psci_invoke_smc
+ *
+ * Inputs:
+ *   r0 -- function_id
+ *   r1 -- arg0
+ *   r2 -- arg1
+ *   r3 -- arg2
+ *
+ * Outputs:
+ *   r0 -- return code
+ */
+.globl psci_invoke_hvc
+psci_invoke_hvc:
+	hvc	#0
+	mov	pc, lr
+
+.globl psci_invoke_smc
+psci_invoke_smc:
+	smc	#0
+	mov	pc, lr
+
 enable_vfp:
 	/* Enable full access to CP10 and CP11: */
 	mov	r0, #(3 << 22 | 3 << 20)
diff --git a/arm/cstart64.S b/arm/cstart64.S
index 42ba3a3ca249..7610e28f06dd 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -109,6 +109,28 @@  start:
 
 .text
 
+/*
+ * psci_invoke_hvc / psci_invoke_smc
+ *
+ * Inputs:
+ *   x0 -- function_id
+ *   x1 -- arg0
+ *   x2 -- arg1
+ *   x3 -- arg2
+ *
+ * Outputs:
+ *   x0 -- return code
+ */
+.globl psci_invoke_hvc
+psci_invoke_hvc:
+	hvc	#0
+	ret
+
+.globl psci_invoke_smc
+psci_invoke_smc:
+	smc	#0
+	ret
+
 get_mmu_off:
 	adrp	x0, auxinfo
 	ldr	x0, [x0, :lo12:auxinfo + 8]
diff --git a/arm/selftest.c b/arm/selftest.c
index 4495b161cdd5..9f459ed3d571 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -400,33 +400,13 @@  static void check_vectors(void *arg __unused)
 	exit(report_summary());
 }
 
-static bool psci_check(void)
+static void psci_print(void)
 {
-	const struct fdt_property *method;
-	int node, len, ver;
-
-	node = fdt_node_offset_by_compatible(dt_fdt(), -1, "arm,psci-0.2");
-	if (node < 0) {
-		printf("PSCI v0.2 compatibility required\n");
-		return false;
-	}
-
-	method = fdt_get_property(dt_fdt(), node, "method", &len);
-	if (method == NULL) {
-		printf("bad psci device tree node\n");
-		return false;
-	}
-
-	if (len < 4 || strcmp(method->data, "hvc") != 0) {
-		printf("psci method must be hvc\n");
-		return false;
-	}
-
-	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
-	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
-				       PSCI_VERSION_MINOR(ver));
-
-	return true;
+	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+	report_info("PSCI version: %d.%d", PSCI_VERSION_MAJOR(ver),
+					  PSCI_VERSION_MINOR(ver));
+	report_info("PSCI method: %s", psci_invoke == psci_invoke_hvc ?
+				       "hvc" : "smc");
 }
 
 static void cpu_report(void *data __unused)
@@ -465,7 +445,7 @@  int main(int argc, char **argv)
 
 	} else if (strcmp(argv[1], "smp") == 0) {
 
-		report(psci_check(), "PSCI version");
+		psci_print();
 		on_cpus(cpu_report, NULL);
 		while (!cpumask_full(&ready))
 			cpu_relax();
diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
index 7b956bf5987d..2820c0a3afc7 100644
--- a/lib/arm/asm/psci.h
+++ b/lib/arm/asm/psci.h
@@ -3,8 +3,14 @@ 
 #include <libcflat.h>
 #include <linux/psci.h>
 
-extern int psci_invoke(unsigned long function_id, unsigned long arg0,
-		       unsigned long arg1, unsigned long arg2);
+typedef int (*psci_invoke_fn)(unsigned long function_id, unsigned long arg0,
+			      unsigned long arg1, unsigned long arg2);
+extern psci_invoke_fn psci_invoke;
+extern int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
+			   unsigned long arg1, unsigned long arg2);
+extern int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
+			   unsigned long arg1, unsigned long arg2);
+extern void psci_set_conduit(void);
 extern int psci_cpu_on(unsigned long cpuid, unsigned long entry_point);
 extern void psci_system_reset(void);
 extern int cpu_psci_cpu_boot(unsigned int cpu);
diff --git a/lib/arm/psci.c b/lib/arm/psci.c
index 936c83948b6a..168786dcf792 100644
--- a/lib/arm/psci.c
+++ b/lib/arm/psci.c
@@ -6,22 +6,23 @@ 
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
+#include <devicetree.h>
 #include <asm/psci.h>
 #include <asm/setup.h>
 #include <asm/page.h>
 #include <asm/smp.h>
 
-__attribute__((noinline))
-int psci_invoke(unsigned long function_id, unsigned long arg0,
-		unsigned long arg1, unsigned long arg2)
+extern void halt(void);
+
+static int psci_invoke_none(unsigned long function_id, unsigned long arg0,
+			    unsigned long arg1, unsigned long arg2)
 {
-	asm volatile(
-		"hvc #0"
-	: "+r" (function_id)
-	: "r" (arg0), "r" (arg1), "r" (arg2));
-	return function_id;
+	printf("No PSCI method configured! Can't invoke...\n");
+	return PSCI_RET_NOT_PRESENT;
 }
 
+psci_invoke_fn psci_invoke = psci_invoke_none;
+
 int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
 {
 #ifdef __arm__
@@ -56,3 +57,23 @@  void psci_system_off(void)
 	int err = psci_invoke(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
 	printf("CPU%d unable to do system off (error = %d)\n", smp_processor_id(), err);
 }
+
+void psci_set_conduit(void)
+{
+	const void *fdt = dt_fdt();
+	const struct fdt_property *method;
+	int node, len;
+
+	node = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-0.2");
+	assert_msg(node >= 0, "PSCI v0.2 compatibility required");
+
+	method = fdt_get_property(fdt, node, "method", &len);
+	assert(method != NULL && len == 4);
+
+	if (strcmp(method->data, "hvc") == 0)
+		psci_invoke = psci_invoke_hvc;
+	else if (strcmp(method->data, "smc") == 0)
+		psci_invoke = psci_invoke_smc;
+	else
+		assert_msg(false, "Unknown PSCI conduit: %s", method->data);
+}
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index a5ebec3c5a12..07d52d2e5fe6 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -25,6 +25,7 @@ 
 #include <asm/processor.h>
 #include <asm/smp.h>
 #include <asm/timer.h>
+#include <asm/psci.h>
 
 #include "io.h"
 
@@ -266,6 +267,7 @@  void setup(const void *fdt, phys_addr_t freemem_start)
 	mem_regions_add_assumed();
 	mem_init(PAGE_ALIGN((unsigned long)freemem));
 
+	psci_set_conduit();
 	cpu_init();
 
 	/* cpu_init must be called before thread_info_init */