xen: arm: introduce assembly helper to call smc

Commit 063188f4b3 "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.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
[ ijc -- reworded commit message as discussed ]

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 9a25290a0871a91d5992252119cc0bd3f8ca0b25..41aba2eb2c250e4ef452f51cb5f53dc9a6553ac0 100644 (file)
--- 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 ac556cb6a047ebee1f8592aca3a6453cc674f584..79e3a5faf9fa6f37d99040f85c6dccbd9312c856 100644 (file)
--- 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_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 edfc3914071ee30e327235b8d9128e66cf79bf48..6cc53625ae357af3d3eb7709aa2da04288d65bb1 100644 (file)
--- 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_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
 }
 
 static void seattle_system_off(void)
 {
-    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_OFF);
+    call_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
 }
 
 PLATFORM_START(seattle, "SEATTLE")

diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 604ff4c88f4747bafb15f1a0d9c7c68cd817286c..40663093bbd1956c0c0bc6677859d1cc0a9e393d 100644 (file)
--- 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_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary), 0);
 }
 
 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_smc(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
 }
 
 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_smc(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
 }
 
 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_smc(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
 
     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 (file)
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 (file)
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 (file)
index 0000000..5d837cb
--- /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)
+/* No 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 e719c26f22a554b18e37f8897c438009af48ab45..b7ef817fd2ab5217e66e76c2a4420e8f5ef3b810 100644 (file)
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -614,6 +614,9 @@ 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);
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_ARM_PROCESSOR_H */
 /*