Re: [PATCH v3 for 4.5] arm32: fix build after 063188f4b3

[Julien Grall <julien.grall@linaro.org>]

I sent again the wrong version :/. Sorry for the noise.

On 10/15/2014 03:08 PM, Julien Grall wrote:
> "xen: arm: Add support for the Exynos secure firmware" introduced code
> assuming that exynos_smc() would get called with arguments in certain
> registers. While the "noinline" attribute guarantees the function to
> not get inlined, it does not guarantee that all arguments arrive in the
> assumed registers: gcc's interprocedural analysis can result in clone
> functions to be created where some of the incoming arguments (commonly
> when they have constant values) get replaced by putting in place the
> respective values inside the clone.
> 
> Xen contains in multiple place of this SMC function: consolidate the function
> in a single place and write it in assembly.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Reported-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
> This is a fix for Xen 4.5 to compile the hypervisor with GCC 4.9.1, used by
> Fedora & co.
> 
> Changes in v3:
>     - Introduce macros assembly helper
>     - Consolidate the SMC code in a single assembly file
>     - Rename do_scm into call_smc and create helper for different number
>     of argument
>     - Rebase on top of PSCI v0.2 host support
> 
> Changes in v2:
>     - Write the SMC call in assembly
>     - Consolidate the code in a single place
> ---
>  xen/arch/arm/Makefile              |    1 +
>  xen/arch/arm/platforms/exynos5.c   |   15 +--------------
>  xen/arch/arm/platforms/seattle.c   |   12 ++----------
>  xen/arch/arm/psci.c                |   34 ++++------------------------------
>  xen/arch/arm/smc.S                 |   21 +++++++++++++++++++++
>  xen/include/asm-arm/arm32/macros.h |    8 ++++++++
>  xen/include/asm-arm/macros.h       |   16 ++++++++++++++++
>  xen/include/asm-arm/processor.h    |    9 +++++++++
>  8 files changed, 62 insertions(+), 54 deletions(-)
>  create mode 100644 xen/arch/arm/smc.S
>  create mode 100644 xen/include/asm-arm/arm32/macros.h
>  create mode 100644 xen/include/asm-arm/macros.h
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 9a25290..41aba2e 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -37,6 +37,7 @@ obj-y += hvm.o
>  obj-y += device.o
>  obj-y += decode.o
>  obj-y += processor.o
> +obj-y += smc.o
>  
>  #obj-bin-y += ....o
>  
> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
> index ac556cb..cfb293d 100644
> --- a/xen/arch/arm/platforms/exynos5.c
> +++ b/xen/arch/arm/platforms/exynos5.c
> @@ -37,19 +37,6 @@ static bool_t secure_firmware;
>  
>  #define SMC_CMD_CPU1BOOT            (-4)
>  
> -static noinline void exynos_smc(register_t function_id, register_t arg0,
> -                                register_t arg1, register_t arg2)
> -{
> -    asm volatile(
> -        __asmeq("%0", "r0")
> -        __asmeq("%1", "r1")
> -        __asmeq("%2", "r2")
> -        __asmeq("%3", "r3")
> -        "smc #0"
> -        :
> -        : "r" (function_id), "r" (arg0), "r" (arg1), "r" (arg2));
> -}
> -
>  static int exynos5_init_time(void)
>  {
>      uint32_t reg;
> @@ -263,7 +250,7 @@ static int exynos5_cpu_up(int cpu)
>      iounmap(power);
>  
>      if ( secure_firmware )
> -        exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> +        call_smc1(SMC_CMD_CPU1BOOT, cpu);
>  
>      return cpu_up_send_sgi(cpu);
>  }
> diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
> index edfc391..f717039 100644
> --- a/xen/arch/arm/platforms/seattle.c
> +++ b/xen/arch/arm/platforms/seattle.c
> @@ -31,22 +31,14 @@ static const char * const seattle_dt_compat[] __initconst =
>   * This is temporary until full PSCI-0.2 is supported.
>   * Then, these function will be removed.
>   */
> -static noinline void seattle_smc_psci(register_t func_id)
> -{
> -    asm volatile(
> -        "smc #0"
> -        : "+r" (func_id)
> -        :);
> -}
> -
>  static void seattle_system_reset(void)
>  {
> -    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_RESET);
> +    call_smc0(PSCI_0_2_FN_SYSTEM_RESET);
>  }
>  
>  static void seattle_system_off(void)
>  {
> -    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_OFF);
> +    call_smc0(PSCI_0_2_FN_SYSTEM_OFF);
>  }
>  
>  PLATFORM_START(seattle, "SEATTLE")
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 604ff4c..c186c13 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -25,49 +25,23 @@
>  
>  uint32_t psci_ver;
>  
> -#ifdef CONFIG_ARM_32
> -#define REG_PREFIX "r"
> -#else
> -#define REG_PREFIX "x"
> -#endif
> -
> -static noinline int __invoke_psci_fn_smc(register_t function_id,
> -                                         register_t arg0,
> -                                         register_t arg1,
> -                                         register_t arg2)
> -{
> -    asm volatile(
> -        __asmeq("%0", REG_PREFIX"0")
> -        __asmeq("%1", REG_PREFIX"1")
> -        __asmeq("%2", REG_PREFIX"2")
> -        __asmeq("%3", REG_PREFIX"3")
> -        "smc #0"
> -        : "+r" (function_id)
> -        : "r" (arg0), "r" (arg1), "r" (arg2));
> -
> -    return function_id;
> -}
> -
> -#undef REG_PREFIX
> -
>  static uint32_t psci_cpu_on_nr;
>  
>  int call_psci_cpu_on(int cpu)
>  {
> -    return __invoke_psci_fn_smc(psci_cpu_on_nr,
> -                                cpu_logical_map(cpu), __pa(init_secondary), 0);
> +    return call_smc2(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary));
>  }
>  
>  void call_psci_system_off(void)
>  {
>      if ( psci_ver > XEN_PSCI_V_0_1 )
> -        __invoke_psci_fn_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +        call_smc0(PSCI_0_2_FN_SYSTEM_OFF);
>  }
>  
>  void call_psci_system_reset(void)
>  {
>      if ( psci_ver > XEN_PSCI_V_0_1 )
> -        __invoke_psci_fn_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +        call_smc0(PSCI_0_2_FN_SYSTEM_RESET);
>  }
>  
>  int __init psci_is_smc_method(const struct dt_device_node *psci)
> @@ -134,7 +108,7 @@ int __init psci_init_0_2(void)
>      if ( ret )
>          return -EINVAL;
>  
> -    psci_ver = __invoke_psci_fn_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> +    psci_ver = call_smc0(PSCI_0_2_FN_PSCI_VERSION);
>  
>      if ( psci_ver != XEN_PSCI_V_0_2 )
>      {
> diff --git a/xen/arch/arm/smc.S b/xen/arch/arm/smc.S
> new file mode 100644
> index 0000000..b8f1822
> --- /dev/null
> +++ b/xen/arch/arm/smc.S
> @@ -0,0 +1,21 @@
> +/*
> + * xen/arch/arm/smc.S
> + *
> + * Wrapper for Secure Monitors Calls
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <asm/macros.h>
> +
> +ENTRY(call_smc)
> +        smc   #0
> +        ret
> diff --git a/xen/include/asm-arm/arm32/macros.h b/xen/include/asm-arm/arm32/macros.h
> new file mode 100644
> index 0000000..a4e20aa
> --- /dev/null
> +++ b/xen/include/asm-arm/arm32/macros.h
> @@ -0,0 +1,8 @@
> +#ifndef __ASM_ARM_ARM32_MACROS_H
> +#define __ASM_ARM_ARM32_MACROS_H
> +
> +    .macro ret
> +        mov     pc, lr
> +    .endm
> +
> +#endif /* __ASM_ARM_ARM32_MACROS_H */
> diff --git a/xen/include/asm-arm/macros.h b/xen/include/asm-arm/macros.h
> new file mode 100644
> index 0000000..f92f905
> --- /dev/null
> +++ b/xen/include/asm-arm/macros.h
> @@ -0,0 +1,16 @@
> +#ifndef __ASM_MACROS_H
> +#define __ASM_MACROS_H
> +
> +#ifndef __ASSEMBLY__
> +# error "This file should only be included in assembly file"
> +#endif
> +
> +#if defined (CONFIG_ARM_32)
> +# include <asm/arm32/macros.h>
> +#elif defined(CONFIG_ARM_64)
> +/* Not specific ARM64 macros for now */
> +#else
> +# error "unknown ARM variant"
> +#endif
> +
> +#endif /* __ASM_ARM_MACROS_H */
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index e719c26..90c2647 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -614,6 +614,15 @@ void vcpu_regs_hyp_to_user(const struct vcpu *vcpu,
>  void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>                             const struct vcpu_guest_core_regs *regs);
>  
> +int call_smc(register_t function_id, register_t arg0, register_t arg1,
> +             register_t arg2);
> +
> +#define call_smc0(func)         call_smc((func), 0, 0, 0)
> +#define call_smc1(func, a0)     call_smc((func), (a0), 0, 0)
> +#define call_smc2(func, a0, a1) call_smc((func), (a0), (a1), 0)
> +
> +int do_smc(register_t function_id, ...);
> +
>  #endif /* __ASSEMBLY__ */
>  #endif /* __ASM_ARM_PROCESSOR_H */
>  /*
> 


-- 
Julien Grall