Re: [Xen-devel] [PATCH 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention

[Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>]

Hi Julien,

On 24.08.18 19:58, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>   xen/arch/arm/psci.c              | 4 ++++
>   xen/include/asm-arm/cpufeature.h | 3 ++-
>   xen/include/asm-arm/smccc.h      | 8 ++++++++
>   3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 3cf5ecf0f3..941eec921b 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -21,6 +21,7 @@
>   #include <xen/types.h>
>   #include <xen/mm.h>
>   #include <xen/smp.h>
> +#include <asm/cpufeature.h>
>   #include <asm/psci.h>
>   #include <asm/acpi.h>
>   
> @@ -118,6 +119,9 @@ static void __init psci_init_smccc(void)
>               smccc_ver = ret;
>       }
>   
> +    if ( smccc_ver >= SMCCC_VERSION(1, 1) )
> +        cpus_set_cap(ARM_SMCCC_1_1);
> +
>       printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
>              SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
>   }
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 9c297c521c..c9c4046f5f 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -44,8 +44,9 @@
>   #define SKIP_CTXT_SWITCH_SERROR_SYNC 6
>   #define ARM_HARDEN_BRANCH_PREDICTOR 7
>   #define ARM_SSBD 8
> +#define ARM_SMCCC_1_1 9
>   
> -#define ARM_NCAPS           9
> +#define ARM_NCAPS           10
>   
>   #ifndef __ASSEMBLY__
>   
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 1ed6cbaa48..7c39c530e2 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -213,6 +213,7 @@ struct arm_smccc_res {
>    */
>   #ifdef CONFIG_ARM_32
>   #define arm_smccc_1_0_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
> +#define arm_smccc_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
>   #else
>   
>   void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
> @@ -254,6 +255,13 @@ void __arm_smccc_1_0_smc(register_t a0, register_t a1, 
> register_t a2,
>   #define arm_smccc_1_0_smc(...)                                              \
>           __arm_smccc_1_0_smc_count(__count_args(__VA_ARGS__), __VA_ARGS__)
>   
> +#define arm_smccc_smc(...)                                      \
> +    do {                                                        \
> +        if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )              \
> +            arm_smccc_1_0_smc(__VA_ARGS__);                     \
> +        else                                                    \
> +            arm_smccc_1_1_smc(__VA_ARGS__);                     \
> +    } while ( 0 )
>   #endif /* CONFIG_ARM_64 */
>   
This will generate lots of code for every arm_smccc_smc(). Can we have 
function pointer arm_smccc_smc instead and assign it to either 
arm_smccc_1_1_smc() or arm_asmccc_1_0_smc() at boot?
I know that currently we have no function arm_smccc_1_1_smc() because it 
is being constructed inline every time. But we can write it explicitly 
and then have one indirect call to (maybe cached) function instead of
lots inlined code with conditional branches.

>   #endif /* __ASSEMBLY__ */
--
Volodymyr Babchuk

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