| Message ID | 1413301941-31853-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
>>> On 14.10.14 at 17:52, <julien.grall@linaro.org> 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> > Signed-off-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. > > Jan: I kept your Signed-off-by for the commit message. > > Changes in v2: > - Write the SMC call in assembly > - Consolidate the code in a single place To me it very much looks like (not just because of the subject) you sent v2 again, rather than v3. Jan
On 10/14/2014 05:29 PM, Jan Beulich wrote: >>>> On 14.10.14 at 17:52, <julien.grall@linaro.org> 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> >> Signed-off-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. >> >> Jan: I kept your Signed-off-by for the commit message. >> >> Changes in v2: >> - Write the SMC call in assembly >> - Consolidate the code in a single place > > To me it very much looks like (not just because of the subject) you > sent v2 again, rather than v3. Oh right. I will send the v3 in a couple of minutes. Regards,
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile index df0e7de..51b64f9 100644 --- a/xen/arch/arm/arm32/Makefile +++ b/xen/arch/arm/arm32/Makefile @@ -8,5 +8,6 @@ obj-y += domain.o obj-y += vfp.o obj-y += smpboot.o obj-y += domctl.o +obj-y += smc.o obj-$(EARLY_PRINTK) += debug.o diff --git a/xen/arch/arm/arm32/smc.S b/xen/arch/arm/arm32/smc.S new file mode 100644 index 0000000..ea1dba5 --- /dev/null +++ b/xen/arch/arm/arm32/smc.S @@ -0,0 +1,19 @@ +/* + * xen/arch/arm/arm32/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. + */ + +GLOBAL(do_smc) + smc #0 + mov pc, lr diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile index c7243f5..4debaf4 100644 --- a/xen/arch/arm/arm64/Makefile +++ b/xen/arch/arm/arm64/Makefile @@ -1,6 +1,7 @@ subdir-y += lib obj-y += entry.o +obj-y += smc.o obj-y += traps.o obj-y += domain.o diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S new file mode 100644 index 0000000..dfe3f02 --- /dev/null +++ b/xen/arch/arm/arm64/smc.S @@ -0,0 +1,19 @@ +/* + * xen/arch/arm/arm64/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. + */ + +GLOBAL(do_smc) + smc #0 + ret diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c index ac556cb..0b6e884 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); + do_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0); return cpu_up_send_sgi(cpu); } diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c index edfc391..91b3c47 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); + do_smc(PSCI_0_2_FN_SYSTEM_RESET); } static void seattle_system_off(void) { - seattle_smc_psci(PSCI_0_2_FN_SYSTEM_OFF); + do_smc(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 b6360d5..7f1f628 100644 --- a/xen/arch/arm/psci.c +++ b/xen/arch/arm/psci.c @@ -25,37 +25,12 @@ bool_t psci_available; -#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 do_smc(psci_cpu_on_nr, cpu_logical_map(cpu), + __pa(init_secondary), 0); } int __init psci_init(void) diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h index e719c26..5b7385c 100644 --- a/xen/include/asm-arm/processor.h +++ b/xen/include/asm-arm/processor.h @@ -614,6 +614,8 @@ 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 do_smc(register_t function_id, ...); + #endif /* __ASSEMBLY__ */ #endif /* __ASM_ARM_PROCESSOR_H */ /*