Re: [PATCH v2] xen:arm: use SMC64 version function ID on ARM64

[Julien Grall <julien.grall@citrix.com>]

Hi,

>From the commit title, it's not clear that you are touching the PSCI code.

I would rewrite it like:

"xen/arm: psci: use SMC64 function ID when available on ARM64"

On 05/10/15 16:27, Brijesh Singh wrote:
> As per PSCI 0.2 spec, if CPU_ON entry_point_address is 64-bit then SMC
> call function ID parameter should be set to SMC64 version.
> 
> Signed-off-by: Brijesh Singh <brijeshkumar.singh@amd.com>
> ---
>  xen/arch/arm/psci.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 7ad6a43..654b89f 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -23,6 +23,17 @@
>  #include <xen/smp.h>
>  #include <asm/psci.h>
>  
> +/* While a 64-bit OS can make SMC32 call convension but some calls which

s/convension/convention/

> + * requires passing function addresses (e.g CPU_ON) should set the
> + * parameter ID to SMC64. The macro will be used to choose the appropriate
> + * parameter ID.

This comment gives the impression that the only reason to use SMC64
convention is to support 64bit address entry point.
But this is not true. SMC64 also brings the support of 64bit target
affinity, change the type of the return...

Please make a generic comment similar to the Linux one. I.e:

"While a 64-bit OS can make calls with SMC32 calling conventions, for
some calls it is necessary to use SMC64 to pass or return 64-bit values.
For such calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate
(native-width) function ID."

> + */
> +#ifdef CONFIG_ARM_64
> +#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN64_##name
> +#else
> +#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN_##name
> +#endif
> +
>  uint32_t psci_ver;
>  
>  static uint32_t psci_cpu_on_nr;
> @@ -116,7 +127,7 @@ int __init psci_init_0_2(void)
>          return -EOPNOTSUPP;
>      }
>  
> -    psci_cpu_on_nr = PSCI_0_2_FN_CPU_ON;
> +    psci_cpu_on_nr = PSCI_0_2_FN_NATIVE(CPU_ON);
>  
>      printk(XENLOG_INFO "Using PSCI-0.2 for SMP bringup\n");
>  
> 

Regards,


-- 
Julien Grall