2020-06-18 12:30:02

by David Brazdil

[permalink] [raw]
Subject: [PATCH v3 02/15] arm64: kvm: Move __smccc_workaround_1_smc to .rodata

This snippet of assembly is used by cpu_errata.c to overwrite parts of KVM hyp
vector. Move it to its own source file and change its ELF section to .rodata.

Signed-off-by: David Brazdil <[email protected]>
---
arch/arm64/kvm/hyp/Makefile | 1 +
arch/arm64/kvm/hyp/hyp-entry.S | 16 ----------------
arch/arm64/kvm/hyp/smccc_wa.S | 30 ++++++++++++++++++++++++++++++
3 files changed, 31 insertions(+), 16 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/smccc_wa.S

diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 8c9880783839..5d8357ddc234 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -7,6 +7,7 @@ ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
$(DISABLE_STACKLEAK_PLUGIN)

obj-$(CONFIG_KVM) += hyp.o
+obj-$(CONFIG_KVM_INDIRECT_VECTORS) += smccc_wa.o

hyp-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o sysreg-sr.o \
debug-sr.o entry.o switch.o fpsimd.o tlb.o hyp-entry.o
diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
index 9c5cfb04170e..d362fad97cc8 100644
--- a/arch/arm64/kvm/hyp/hyp-entry.S
+++ b/arch/arm64/kvm/hyp/hyp-entry.S
@@ -318,20 +318,4 @@ SYM_CODE_START(__bp_harden_hyp_vecs)
1: .org __bp_harden_hyp_vecs + __BP_HARDEN_HYP_VECS_SZ
.org 1b
SYM_CODE_END(__bp_harden_hyp_vecs)
-
- .popsection
-
-SYM_CODE_START(__smccc_workaround_1_smc)
- esb
- sub sp, sp, #(8 * 4)
- stp x2, x3, [sp, #(8 * 0)]
- stp x0, x1, [sp, #(8 * 2)]
- mov w0, #ARM_SMCCC_ARCH_WORKAROUND_1
- smc #0
- ldp x2, x3, [sp, #(8 * 0)]
- ldp x0, x1, [sp, #(8 * 2)]
- add sp, sp, #(8 * 4)
-1: .org __smccc_workaround_1_smc + __SMCCC_WORKAROUND_1_SMC_SZ
- .org 1b
-SYM_CODE_END(__smccc_workaround_1_smc)
#endif
diff --git a/arch/arm64/kvm/hyp/smccc_wa.S b/arch/arm64/kvm/hyp/smccc_wa.S
new file mode 100644
index 000000000000..aa25b5428e77
--- /dev/null
+++ b/arch/arm64/kvm/hyp/smccc_wa.S
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2015-2018 - ARM Ltd
+ * Author: Marc Zyngier <[email protected]>
+ */
+
+#include <linux/arm-smccc.h>
+
+#include <asm/kvm_asm.h>
+#include <asm/kvm_mmu.h>
+
+ /*
+ * This is not executed directly and is instead copied into the vectors
+ * by install_bp_hardening_cb().
+ */
+ .data
+ .pushsection .rodata
+ .global __smccc_workaround_1_smc
+__smccc_workaround_1_smc:
+ esb
+ sub sp, sp, #(8 * 4)
+ stp x2, x3, [sp, #(8 * 0)]
+ stp x0, x1, [sp, #(8 * 2)]
+ mov w0, #ARM_SMCCC_ARCH_WORKAROUND_1
+ smc #0
+ ldp x2, x3, [sp, #(8 * 0)]
+ ldp x0, x1, [sp, #(8 * 2)]
+ add sp, sp, #(8 * 4)
+1: .org __smccc_workaround_1_smc + __SMCCC_WORKAROUND_1_SMC_SZ
+ .org 1b
--
2.27.0


2020-06-18 14:00:03

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 02/15] arm64: kvm: Move __smccc_workaround_1_smc to .rodata

Hi David,

On 2020-06-18 13:25, David Brazdil wrote:
> This snippet of assembly is used by cpu_errata.c to overwrite parts of
> KVM hyp
> vector. Move it to its own source file and change its ELF section to
> .rodata.
>
> Signed-off-by: David Brazdil <[email protected]>
> ---
> arch/arm64/kvm/hyp/Makefile | 1 +
> arch/arm64/kvm/hyp/hyp-entry.S | 16 ----------------
> arch/arm64/kvm/hyp/smccc_wa.S | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 31 insertions(+), 16 deletions(-)
> create mode 100644 arch/arm64/kvm/hyp/smccc_wa.S
>
> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> index 8c9880783839..5d8357ddc234 100644
> --- a/arch/arm64/kvm/hyp/Makefile
> +++ b/arch/arm64/kvm/hyp/Makefile
> @@ -7,6 +7,7 @@ ccflags-y += -fno-stack-protector
> -DDISABLE_BRANCH_PROFILING \
> $(DISABLE_STACKLEAK_PLUGIN)
>
> obj-$(CONFIG_KVM) += hyp.o
> +obj-$(CONFIG_KVM_INDIRECT_VECTORS) += smccc_wa.o
>
> hyp-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o
> sysreg-sr.o \
> debug-sr.o entry.o switch.o fpsimd.o tlb.o hyp-entry.o
> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S
> b/arch/arm64/kvm/hyp/hyp-entry.S
> index 9c5cfb04170e..d362fad97cc8 100644
> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> @@ -318,20 +318,4 @@ SYM_CODE_START(__bp_harden_hyp_vecs)
> 1: .org __bp_harden_hyp_vecs + __BP_HARDEN_HYP_VECS_SZ
> .org 1b
> SYM_CODE_END(__bp_harden_hyp_vecs)
> -
> - .popsection

I'd be tempted to leave the .popsection in place, if only for symmetry
with the initial .pushsection.

> -
> -SYM_CODE_START(__smccc_workaround_1_smc)
> - esb
> - sub sp, sp, #(8 * 4)
> - stp x2, x3, [sp, #(8 * 0)]
> - stp x0, x1, [sp, #(8 * 2)]
> - mov w0, #ARM_SMCCC_ARCH_WORKAROUND_1
> - smc #0
> - ldp x2, x3, [sp, #(8 * 0)]
> - ldp x0, x1, [sp, #(8 * 2)]
> - add sp, sp, #(8 * 4)
> -1: .org __smccc_workaround_1_smc + __SMCCC_WORKAROUND_1_SMC_SZ
> - .org 1b
> -SYM_CODE_END(__smccc_workaround_1_smc)
> #endif
> diff --git a/arch/arm64/kvm/hyp/smccc_wa.S
> b/arch/arm64/kvm/hyp/smccc_wa.S
> new file mode 100644
> index 000000000000..aa25b5428e77
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/smccc_wa.S
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2015-2018 - ARM Ltd
> + * Author: Marc Zyngier <[email protected]>
> + */
> +
> +#include <linux/arm-smccc.h>
> +
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_mmu.h>
> +
> + /*
> + * This is not executed directly and is instead copied into the
> vectors
> + * by install_bp_hardening_cb().
> + */
> + .data
> + .pushsection .rodata
> + .global __smccc_workaround_1_smc
> +__smccc_workaround_1_smc:

You probably want to replace this with SYM_DATA_START (and SYM_DATA_END
at the end).

> + esb
> + sub sp, sp, #(8 * 4)
> + stp x2, x3, [sp, #(8 * 0)]
> + stp x0, x1, [sp, #(8 * 2)]
> + mov w0, #ARM_SMCCC_ARCH_WORKAROUND_1
> + smc #0
> + ldp x2, x3, [sp, #(8 * 0)]
> + ldp x0, x1, [sp, #(8 * 2)]
> + add sp, sp, #(8 * 4)
> +1: .org __smccc_workaround_1_smc + __SMCCC_WORKAROUND_1_SMC_SZ
> + .org 1b

Otherwise, looks good.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2020-06-19 10:12:01

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v3 02/15] arm64: kvm: Move __smccc_workaround_1_smc to .rodata

On 2020-06-19 10:51, David Brazdil wrote:
> Hey Marc,
>
>> > - .popsection
>>
>> I'd be tempted to leave the .popsection in place, if only for symmetry
>> with
>> the initial .pushsection.
>
> I removed it because other .S files don't pop either. It must have been
> added
> here purely for the smccc workaround code. Happy to add it back if you
> prefer,
> but the pushsection is removed later in the series, so this would
> disappear
> as well.

Don't bother then.

>
>> > + .pushsection .rodata
>> > + .global __smccc_workaround_1_smc
>> > +__smccc_workaround_1_smc:
>>
>> You probably want to replace this with SYM_DATA_START (and
>> SYM_DATA_END at
>> the end).
>
> Done

Thanks!

M.
--
Jazz is not dead. It just smells funny...

2020-06-19 11:42:04

by David Brazdil

[permalink] [raw]
Subject: Re: [PATCH v3 02/15] arm64: kvm: Move __smccc_workaround_1_smc to .rodata

Hey Marc,

> > - .popsection
>
> I'd be tempted to leave the .popsection in place, if only for symmetry with
> the initial .pushsection.

I removed it because other .S files don't pop either. It must have been added
here purely for the smccc workaround code. Happy to add it back if you prefer,
but the pushsection is removed later in the series, so this would disappear
as well.

> > + .pushsection .rodata
> > + .global __smccc_workaround_1_smc
> > +__smccc_workaround_1_smc:
>
> You probably want to replace this with SYM_DATA_START (and SYM_DATA_END at
> the end).

Done

Thanks for reviewing,
David