Re: [PATCH v2 09/15] xen/arm: psci: Detect SMCCC version

[Volodymyr Babchuk <volodymyr_babchuk@epam.com>]

Julien,


On 08.02.18 21:21, Julien Grall wrote:
> PSCI 1.0 and later allows the SMCCC version to be (indirectly) probed
> via PSCI_FEATURES. If the PSCI_FEATURES does not exist (PSCI 0.2 or
> earlier) and the function return an error, then we considered SMCCC 1.0
> is implemented.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>      Changes in v2:
>          - Patch added
> ---
>   xen/arch/arm/psci.c         | 34 +++++++++++++++++++++++++++++++++-
>   xen/include/asm-arm/smccc.h |  5 ++++-
>   2 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c

I find it strange to determine SMCCC version in PSCI code. psci.c is not 
the first place, where I will look for SMCCC version discovery.

I think it is better to add smccc.c, where such functions can reside.

> index 5dda35cd7c..bc7b2260e8 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -37,6 +37,7 @@
>   #endif
>   
>   uint32_t psci_ver;
> +uint32_t smccc_ver;

And this variable actually is not related to PSCI.
>   
>   static uint32_t psci_cpu_on_nr;
>   
> @@ -57,6 +58,14 @@ void call_psci_system_reset(void)
>           call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
>   }
>   
> +static int __init psci_features(uint32_t psci_func_id)
> +{
> +    if ( psci_ver < PSCI_VERSION(1, 0) )
> +        return PSCI_NOT_SUPPORTED;
> +
> +    return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
> +}
> +
>   int __init psci_is_smc_method(const struct dt_device_node *psci)
>   {
>       int ret;
> @@ -82,6 +91,24 @@ int __init psci_is_smc_method(const struct dt_device_node *psci)
>       return 0;
>   }
>   
> +static void __init psci_init_smccc(void)
> +{
> +    /* PSCI is using at least SMCC 1.0 calling convention. */
> +    smccc_ver = ARM_SMCCC_VERSION_1_0;
> +
> +    if ( psci_features(ARM_SMCCC_VERSION_FID) != PSCI_NOT_SUPPORTED )
> +    {
> +        uint32_t ret;
> +
> +        ret = call_smc(ARM_SMCCC_VERSION_FID, 0, 0, 0);
> +        if ( ret != ARM_SMCCC_NOT_SUPPORTED )
> +            smccc_ver = ret;
> +    }
> +
> +    printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
> +           SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
> +}
> +
>   int __init psci_init_0_1(void)
>   {
>       int ret;
> @@ -173,7 +200,12 @@ int __init psci_init(void)
>       if ( ret )
>           ret = psci_init_0_1();
>   
> -    return ret;
> +    if ( ret )
> +        return ret;
> +
> +    psci_init_smccc();
> +
> +    return 0;
>   }
>   
>   /*
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index caa2c9cc1b..bc067892c7 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -52,6 +52,8 @@
>   
>   #ifndef __ASSEMBLY__
>   
> +extern uint32_t smccc_ver;
> +
>   /* Check if this is fast call. */
>   static inline bool smccc_is_fast_call(register_t funcid)
>   {
> @@ -137,8 +139,9 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>                         ARM_SMCCC_OWNER_ARCH,         \
>                         0x8000)
>   
> -/* Only one error code defined in SMCCC */
> +/* SMCCC error codes */
>   #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
> +#define ARM_SMCCC_NOT_SUPPORTED         (-1)

In patch "xen/arm: vsmc: Implement SMCCC 1.1" you return plain -1 in 
static bool handle_arch(struct cpu_user_regs *regs)

Could you please move definition of ARM_SMCCC_NOT_SUPPORTED into that 
patch and use it in mentioned function or add new patch that changes -1 
to ARM_SMCCC_NOT_SUPPORTED ?

>   
>   /* SMCCC function identifier range which is reserved for existing APIs */
>   #define ARM_SMCCC_RESERVED_RANGE_START  0x0
> 

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel