* [PATCH v3 for 4.5] arm32: fix build after 063188f4b3
@ 2014-10-14 16:32 Julien Grall
2014-10-15 8:11 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Julien Grall @ 2014-10-14 16:32 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, Julien Grall, tim, stefano.stabellini, Jan Beulich
"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 */
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3 for 4.5] arm32: fix build after 063188f4b3
2014-10-14 16:32 [PATCH v3 for 4.5] arm32: fix build after 063188f4b3 Julien Grall
@ 2014-10-15 8:11 ` Ian Campbell
2014-10-15 13:14 ` Julien Grall
0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2014-10-15 8:11 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, tim, Jan Beulich, stefano.stabellini
On Tue, 2014-10-14 at 17:32 +0100, Julien Grall wrote:
> @@ -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);
>
Have you confirmed that none of these zeroes (throughout the patch, not
just here) are actually unused and not just formal parameters which
happen to be zero.
For example:
> - 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));
CPU_ON takes three arguments, target_cpu, entry_point_address and
context_id, so the zero here is actually the context_id parameter and
not a dummy argument.
> +#elif defined(CONFIG_ARM_64)
> +/* Not specific ARM64 macros for now */
"No specific..."
> +
> +int do_smc(register_t function_id, ...);
Stray?
Ian.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 for 4.5] arm32: fix build after 063188f4b3
2014-10-15 8:11 ` Ian Campbell
@ 2014-10-15 13:14 ` Julien Grall
0 siblings, 0 replies; 5+ messages in thread
From: Julien Grall @ 2014-10-15 13:14 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, tim, Jan Beulich, stefano.stabellini
On 10/15/2014 09:11 AM, Ian Campbell wrote:
> On Tue, 2014-10-14 at 17:32 +0100, Julien Grall wrote:
>> @@ -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);
>>
>
> Have you confirmed that none of these zeroes (throughout the patch, not
> just here) are actually unused and not just formal parameters which
> happen to be zero.
I can't find any documentation the exynos SMC call on the web.
I will send a new version by removing call_smc<n> and always use
call_smc with 4 parameters.
>> +#elif defined(CONFIG_ARM_64)
>> +/* Not specific ARM64 macros for now */
>
> "No specific..."
Ok.
>> +
>> +int do_smc(register_t function_id, ...);
>
> Stray?
Yes.
--
Julien Grall
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 for 4.5] arm32: fix build after 063188f4b3
@ 2014-10-15 14:08 Julien Grall
2014-10-15 14:13 ` Julien Grall
0 siblings, 1 reply; 5+ messages in thread
From: Julien Grall @ 2014-10-15 14:08 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, Julien Grall, tim, stefano.stabellini, Jan Beulich
"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 */
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3 for 4.5] arm32: fix build after 063188f4b3
2014-10-15 14:08 Julien Grall
@ 2014-10-15 14:13 ` Julien Grall
0 siblings, 0 replies; 5+ messages in thread
From: Julien Grall @ 2014-10-15 14:13 UTC (permalink / raw)
To: xen-devel; +Cc: stefano.stabellini, tim, ian.campbell, Jan Beulich
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-15 14:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-14 16:32 [PATCH v3 for 4.5] arm32: fix build after 063188f4b3 Julien Grall
2014-10-15 8:11 ` Ian Campbell
2014-10-15 13:14 ` Julien Grall
-- strict thread matches above, loose matches on Subject: below --
2014-10-15 14:08 Julien Grall
2014-10-15 14:13 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.