* [PATCH 0/3] arm64: alternative: add auto-nop support
@ 2016-09-07 10:07 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
In some cases where we use pairs of alternative sequences, only one sequence
does valuable work, and the other consists solely of NOPs. We have to manually
ensure that both sequences are the same size, and some NOP sleds are partially
contained unddef ifdefs.
Maintaining this balance is tedious, and the presence of these makes the code
more painful to read than is necessary. These patches add helpers to handle
these cases automatically, making the code more maintainable, and easier to
read.
Thanks,
Mark.
Mark Rutland (3):
arm64: alternative: add auto-nop infrastructure
arm64: use alternative auto-nop
arm64/kvm: use alternative auto-nop
arch/arm64/include/asm/alternative.h | 71 +++++++++++++++++++++++++++---------
arch/arm64/include/asm/kvm_mmu.h | 10 ++---
arch/arm64/kernel/entry.S | 10 +----
arch/arm64/kvm/hyp.S | 6 +--
arch/arm64/lib/copy_page.S | 13 ++-----
arch/arm64/mm/proc.S | 9 +----
6 files changed, 66 insertions(+), 53 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
2016-09-07 10:07 ` Mark Rutland
@ 2016-09-07 10:07 ` Mark Rutland
-1 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
Cc: marc.zyngier, andre.przywara, will.deacon, dave.martin,
catalin.marinas, kvmarm
In some cases, one side of an alternative sequence is simply a number of
NOPs used to balance the other side. Keeping track of this manually is
tedious, and the presence of large chains of NOPs makes the code more
painful to read than necessary.
To ameliorate matters, this patch adds a new alternative_else_nop_endif,
which automatically balances an alternative sequence with a trivial NOP
sled.
In many cases, we would like a NOP-sled in the default case, and
instructions patched in in the presence of a feature. To enable the NOPs
to be generated automatically for this case, this patch also adds a new
alternative_if, and updates alternative_else and alternative_endif to
work with either alternative_if or alternative_endif.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Martin <dave.martin@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/include/asm/alternative.h | 71 +++++++++++++++++++++++++++---------
1 file changed, 54 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 8746ff6..4ddbf1c 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -90,34 +90,55 @@ void apply_alternatives(void *start, size_t length);
.endm
/*
- * Begin an alternative code sequence.
+ * Alternative sequences
+ *
+ * The code for the case where the capability is not present will be
+ * assembled and linked as normal. There are no restrictions on this
+ * code.
+ *
+ * The code for the case where the capability is present will be
+ * assembled into a special section to be used for dynamic patching.
+ * Code for that case must:
+ *
+ * 1. Be exactly the same length (in bytes) as the default code
+ * sequence.
*
- * The code that follows this macro will be assembled and linked as
- * normal. There are no restrictions on this code.
+ * 2. Not contain a branch target that is used outside of the
+ * alternative sequence it is defined in (branches into an
+ * alternative sequence are not fixed up).
+ */
+
+/*
+ * Begin an alternative code sequence.
*/
.macro alternative_if_not cap
+ .set .Lasm_alt_mode, 0
.pushsection .altinstructions, "a"
altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
.popsection
661:
.endm
+.macro alternative_if cap
+ .set .Lasm_alt_mode, 1
+ .pushsection .altinstructions, "a"
+ altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
+ .popsection
+ .pushsection .altinstr_replacement, "ax"
+ .align 2 /* So GAS knows label 661 is suitably aligned */
+661:
+.endm
+
/*
- * Provide the alternative code sequence.
- *
- * The code that follows this macro is assembled into a special
- * section to be used for dynamic patching. Code that follows this
- * macro must:
- *
- * 1. Be exactly the same length (in bytes) as the default code
- * sequence.
- *
- * 2. Not contain a branch target that is used outside of the
- * alternative sequence it is defined in (branches into an
- * alternative sequence are not fixed up).
+ * Provide the other half of the alternative code sequence.
*/
.macro alternative_else
-662: .pushsection .altinstr_replacement, "ax"
+662:
+ .if .Lasm_alt_mode==0
+ .pushsection .altinstr_replacement, "ax"
+ .else
+ .popsection
+ .endif
663:
.endm
@@ -125,11 +146,27 @@ void apply_alternatives(void *start, size_t length);
* Complete an alternative code sequence.
*/
.macro alternative_endif
-664: .popsection
+664:
+ .if .Lasm_alt_mode==0
+ .popsection
+ .endif
.org . - (664b-663b) + (662b-661b)
.org . - (662b-661b) + (664b-663b)
.endm
+/*
+ * Provides a trivial alternative or default sequence consisting solely
+ * of NOPs. The number of NOPs is chosen automatically to match the
+ * previous case.
+ */
+.macro alternative_else_nop_endif
+alternative_else
+.rept (662b-661b) / 4
+ nop
+.endr
+alternative_endif
+.endm
+
#define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...) \
alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
@ 2016-09-07 10:07 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
In some cases, one side of an alternative sequence is simply a number of
NOPs used to balance the other side. Keeping track of this manually is
tedious, and the presence of large chains of NOPs makes the code more
painful to read than necessary.
To ameliorate matters, this patch adds a new alternative_else_nop_endif,
which automatically balances an alternative sequence with a trivial NOP
sled.
In many cases, we would like a NOP-sled in the default case, and
instructions patched in in the presence of a feature. To enable the NOPs
to be generated automatically for this case, this patch also adds a new
alternative_if, and updates alternative_else and alternative_endif to
work with either alternative_if or alternative_endif.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Martin <dave.martin@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/include/asm/alternative.h | 71 +++++++++++++++++++++++++++---------
1 file changed, 54 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 8746ff6..4ddbf1c 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -90,34 +90,55 @@ void apply_alternatives(void *start, size_t length);
.endm
/*
- * Begin an alternative code sequence.
+ * Alternative sequences
+ *
+ * The code for the case where the capability is not present will be
+ * assembled and linked as normal. There are no restrictions on this
+ * code.
+ *
+ * The code for the case where the capability is present will be
+ * assembled into a special section to be used for dynamic patching.
+ * Code for that case must:
+ *
+ * 1. Be exactly the same length (in bytes) as the default code
+ * sequence.
*
- * The code that follows this macro will be assembled and linked as
- * normal. There are no restrictions on this code.
+ * 2. Not contain a branch target that is used outside of the
+ * alternative sequence it is defined in (branches into an
+ * alternative sequence are not fixed up).
+ */
+
+/*
+ * Begin an alternative code sequence.
*/
.macro alternative_if_not cap
+ .set .Lasm_alt_mode, 0
.pushsection .altinstructions, "a"
altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
.popsection
661:
.endm
+.macro alternative_if cap
+ .set .Lasm_alt_mode, 1
+ .pushsection .altinstructions, "a"
+ altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
+ .popsection
+ .pushsection .altinstr_replacement, "ax"
+ .align 2 /* So GAS knows label 661 is suitably aligned */
+661:
+.endm
+
/*
- * Provide the alternative code sequence.
- *
- * The code that follows this macro is assembled into a special
- * section to be used for dynamic patching. Code that follows this
- * macro must:
- *
- * 1. Be exactly the same length (in bytes) as the default code
- * sequence.
- *
- * 2. Not contain a branch target that is used outside of the
- * alternative sequence it is defined in (branches into an
- * alternative sequence are not fixed up).
+ * Provide the other half of the alternative code sequence.
*/
.macro alternative_else
-662: .pushsection .altinstr_replacement, "ax"
+662:
+ .if .Lasm_alt_mode==0
+ .pushsection .altinstr_replacement, "ax"
+ .else
+ .popsection
+ .endif
663:
.endm
@@ -125,11 +146,27 @@ void apply_alternatives(void *start, size_t length);
* Complete an alternative code sequence.
*/
.macro alternative_endif
-664: .popsection
+664:
+ .if .Lasm_alt_mode==0
+ .popsection
+ .endif
.org . - (664b-663b) + (662b-661b)
.org . - (662b-661b) + (664b-663b)
.endm
+/*
+ * Provides a trivial alternative or default sequence consisting solely
+ * of NOPs. The number of NOPs is chosen automatically to match the
+ * previous case.
+ */
+.macro alternative_else_nop_endif
+alternative_else
+.rept (662b-661b) / 4
+ nop
+.endr
+alternative_endif
+.endm
+
#define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...) \
alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
2016-09-07 10:07 ` Mark Rutland
@ 2016-09-13 8:36 ` Ard Biesheuvel
-1 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2016-09-13 8:36 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Andre Przywara, Will Deacon, kvmarm,
Catalin Marinas, Dave Martin, linux-arm-kernel
On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
> In some cases, one side of an alternative sequence is simply a number of
> NOPs used to balance the other side. Keeping track of this manually is
> tedious, and the presence of large chains of NOPs makes the code more
> painful to read than necessary.
>
> To ameliorate matters, this patch adds a new alternative_else_nop_endif,
> which automatically balances an alternative sequence with a trivial NOP
> sled.
>
> In many cases, we would like a NOP-sled in the default case, and
> instructions patched in in the presence of a feature. To enable the NOPs
> to be generated automatically for this case, this patch also adds a new
> alternative_if, and updates alternative_else and alternative_endif to
> work with either alternative_if or alternative_endif.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Martin <dave.martin@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm64/include/asm/alternative.h | 71 +++++++++++++++++++++++++++---------
> 1 file changed, 54 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 8746ff6..4ddbf1c 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -90,34 +90,55 @@ void apply_alternatives(void *start, size_t length);
> .endm
>
> /*
> - * Begin an alternative code sequence.
> + * Alternative sequences
> + *
> + * The code for the case where the capability is not present will be
> + * assembled and linked as normal. There are no restrictions on this
> + * code.
> + *
> + * The code for the case where the capability is present will be
> + * assembled into a special section to be used for dynamic patching.
> + * Code for that case must:
> + *
> + * 1. Be exactly the same length (in bytes) as the default code
> + * sequence.
> *
> - * The code that follows this macro will be assembled and linked as
> - * normal. There are no restrictions on this code.
> + * 2. Not contain a branch target that is used outside of the
> + * alternative sequence it is defined in (branches into an
> + * alternative sequence are not fixed up).
> + */
> +
> +/*
> + * Begin an alternative code sequence.
> */
> .macro alternative_if_not cap
> + .set .Lasm_alt_mode, 0
Given that only a single copy of this symbol will exist in an object
file, is it still possible to use both variants in a single
compilation/assembly unit?
> .pushsection .altinstructions, "a"
> altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
> .popsection
> 661:
> .endm
>
> +.macro alternative_if cap
> + .set .Lasm_alt_mode, 1
> + .pushsection .altinstructions, "a"
> + altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
> + .popsection
> + .pushsection .altinstr_replacement, "ax"
> + .align 2 /* So GAS knows label 661 is suitably aligned */
> +661:
> +.endm
> +
> /*
> - * Provide the alternative code sequence.
> - *
> - * The code that follows this macro is assembled into a special
> - * section to be used for dynamic patching. Code that follows this
> - * macro must:
> - *
> - * 1. Be exactly the same length (in bytes) as the default code
> - * sequence.
> - *
> - * 2. Not contain a branch target that is used outside of the
> - * alternative sequence it is defined in (branches into an
> - * alternative sequence are not fixed up).
> + * Provide the other half of the alternative code sequence.
> */
> .macro alternative_else
> -662: .pushsection .altinstr_replacement, "ax"
> +662:
> + .if .Lasm_alt_mode==0
> + .pushsection .altinstr_replacement, "ax"
> + .else
> + .popsection
> + .endif
> 663:
> .endm
>
> @@ -125,11 +146,27 @@ void apply_alternatives(void *start, size_t length);
> * Complete an alternative code sequence.
> */
> .macro alternative_endif
> -664: .popsection
> +664:
> + .if .Lasm_alt_mode==0
> + .popsection
> + .endif
> .org . - (664b-663b) + (662b-661b)
> .org . - (662b-661b) + (664b-663b)
> .endm
>
> +/*
> + * Provides a trivial alternative or default sequence consisting solely
> + * of NOPs. The number of NOPs is chosen automatically to match the
> + * previous case.
> + */
> +.macro alternative_else_nop_endif
> +alternative_else
> +.rept (662b-661b) / 4
> + nop
> +.endr
> +alternative_endif
> +.endm
> +
> #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...) \
> alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
@ 2016-09-13 8:36 ` Ard Biesheuvel
0 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2016-09-13 8:36 UTC (permalink / raw)
To: linux-arm-kernel
On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
> In some cases, one side of an alternative sequence is simply a number of
> NOPs used to balance the other side. Keeping track of this manually is
> tedious, and the presence of large chains of NOPs makes the code more
> painful to read than necessary.
>
> To ameliorate matters, this patch adds a new alternative_else_nop_endif,
> which automatically balances an alternative sequence with a trivial NOP
> sled.
>
> In many cases, we would like a NOP-sled in the default case, and
> instructions patched in in the presence of a feature. To enable the NOPs
> to be generated automatically for this case, this patch also adds a new
> alternative_if, and updates alternative_else and alternative_endif to
> work with either alternative_if or alternative_endif.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Martin <dave.martin@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
> arch/arm64/include/asm/alternative.h | 71 +++++++++++++++++++++++++++---------
> 1 file changed, 54 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 8746ff6..4ddbf1c 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -90,34 +90,55 @@ void apply_alternatives(void *start, size_t length);
> .endm
>
> /*
> - * Begin an alternative code sequence.
> + * Alternative sequences
> + *
> + * The code for the case where the capability is not present will be
> + * assembled and linked as normal. There are no restrictions on this
> + * code.
> + *
> + * The code for the case where the capability is present will be
> + * assembled into a special section to be used for dynamic patching.
> + * Code for that case must:
> + *
> + * 1. Be exactly the same length (in bytes) as the default code
> + * sequence.
> *
> - * The code that follows this macro will be assembled and linked as
> - * normal. There are no restrictions on this code.
> + * 2. Not contain a branch target that is used outside of the
> + * alternative sequence it is defined in (branches into an
> + * alternative sequence are not fixed up).
> + */
> +
> +/*
> + * Begin an alternative code sequence.
> */
> .macro alternative_if_not cap
> + .set .Lasm_alt_mode, 0
Given that only a single copy of this symbol will exist in an object
file, is it still possible to use both variants in a single
compilation/assembly unit?
> .pushsection .altinstructions, "a"
> altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
> .popsection
> 661:
> .endm
>
> +.macro alternative_if cap
> + .set .Lasm_alt_mode, 1
> + .pushsection .altinstructions, "a"
> + altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
> + .popsection
> + .pushsection .altinstr_replacement, "ax"
> + .align 2 /* So GAS knows label 661 is suitably aligned */
> +661:
> +.endm
> +
> /*
> - * Provide the alternative code sequence.
> - *
> - * The code that follows this macro is assembled into a special
> - * section to be used for dynamic patching. Code that follows this
> - * macro must:
> - *
> - * 1. Be exactly the same length (in bytes) as the default code
> - * sequence.
> - *
> - * 2. Not contain a branch target that is used outside of the
> - * alternative sequence it is defined in (branches into an
> - * alternative sequence are not fixed up).
> + * Provide the other half of the alternative code sequence.
> */
> .macro alternative_else
> -662: .pushsection .altinstr_replacement, "ax"
> +662:
> + .if .Lasm_alt_mode==0
> + .pushsection .altinstr_replacement, "ax"
> + .else
> + .popsection
> + .endif
> 663:
> .endm
>
> @@ -125,11 +146,27 @@ void apply_alternatives(void *start, size_t length);
> * Complete an alternative code sequence.
> */
> .macro alternative_endif
> -664: .popsection
> +664:
> + .if .Lasm_alt_mode==0
> + .popsection
> + .endif
> .org . - (664b-663b) + (662b-661b)
> .org . - (662b-661b) + (664b-663b)
> .endm
>
> +/*
> + * Provides a trivial alternative or default sequence consisting solely
> + * of NOPs. The number of NOPs is chosen automatically to match the
> + * previous case.
> + */
> +.macro alternative_else_nop_endif
> +alternative_else
> +.rept (662b-661b) / 4
> + nop
> +.endr
> +alternative_endif
> +.endm
> +
> #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...) \
> alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
2016-09-13 8:36 ` Ard Biesheuvel
@ 2016-09-13 8:57 ` Mark Rutland
-1 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-13 8:57 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Marc Zyngier, Andre Przywara, Will Deacon, kvmarm,
Catalin Marinas, Dave Martin, linux-arm-kernel
On Tue, Sep 13, 2016 at 09:36:14AM +0100, Ard Biesheuvel wrote:
> On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
> > In some cases, one side of an alternative sequence is simply a number of
> > NOPs used to balance the other side. Keeping track of this manually is
> > tedious, and the presence of large chains of NOPs makes the code more
> > painful to read than necessary.
> >
> > To ameliorate matters, this patch adds a new alternative_else_nop_endif,
> > which automatically balances an alternative sequence with a trivial NOP
> > sled.
> >
> > In many cases, we would like a NOP-sled in the default case, and
> > instructions patched in in the presence of a feature. To enable the NOPs
> > to be generated automatically for this case, this patch also adds a new
> > alternative_if, and updates alternative_else and alternative_endif to
> > work with either alternative_if or alternative_endif.
[...]
> > +/*
> > + * Begin an alternative code sequence.
> > */
> > .macro alternative_if_not cap
> > + .set .Lasm_alt_mode, 0
>
> Given that only a single copy of this symbol will exist in an object
> file, is it still possible to use both variants in a single
> compilation/assembly unit?
Yes.
GAS allows the symbol to be set multiple times (so long as the
assignments are constant values). The last assignment "wins" when it
comes to output, but assembler macros are evaluated before this, and use
the most recent assignment.
In testing I hacked __kvm_call_hyp to use both:
ENTRY(__kvm_call_hyp)
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
str lr, [sp, #-16]!
hvc #0
ldr lr, [sp], #16
ret
alternative_else_nop_endif
alternative_if ARM64_HAS_VIRT_HOST_EXTN
hvc #0x539
alternative_else_nop_endif
b __vhe_hyp_call
ENDPROC(__kvm_call_hyp)
Which, according to objdump gives me the expected result:
Disassembly of section .text:
0000000000000000 <__kvm_call_hyp>:
0: f81f0ffe str x30, [sp,#-16]!
4: d4000002 hvc #0x0
8: f84107fe ldr x30, [sp],#16
c: d65f03c0 ret
10: d503201f nop
14: 14000000 b 0 <__vhe_hyp_call>
Disassembly of section .altinstr_replacement:
0000000000000000 <.altinstr_replacement>:
0: d503201f nop
4: d503201f nop
8: d503201f nop
c: d503201f nop
10: d400a722 hvc #0x539
Thanks,
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
@ 2016-09-13 8:57 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-13 8:57 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 13, 2016 at 09:36:14AM +0100, Ard Biesheuvel wrote:
> On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
> > In some cases, one side of an alternative sequence is simply a number of
> > NOPs used to balance the other side. Keeping track of this manually is
> > tedious, and the presence of large chains of NOPs makes the code more
> > painful to read than necessary.
> >
> > To ameliorate matters, this patch adds a new alternative_else_nop_endif,
> > which automatically balances an alternative sequence with a trivial NOP
> > sled.
> >
> > In many cases, we would like a NOP-sled in the default case, and
> > instructions patched in in the presence of a feature. To enable the NOPs
> > to be generated automatically for this case, this patch also adds a new
> > alternative_if, and updates alternative_else and alternative_endif to
> > work with either alternative_if or alternative_endif.
[...]
> > +/*
> > + * Begin an alternative code sequence.
> > */
> > .macro alternative_if_not cap
> > + .set .Lasm_alt_mode, 0
>
> Given that only a single copy of this symbol will exist in an object
> file, is it still possible to use both variants in a single
> compilation/assembly unit?
Yes.
GAS allows the symbol to be set multiple times (so long as the
assignments are constant values). The last assignment "wins" when it
comes to output, but assembler macros are evaluated before this, and use
the most recent assignment.
In testing I hacked __kvm_call_hyp to use both:
ENTRY(__kvm_call_hyp)
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
str lr, [sp, #-16]!
hvc #0
ldr lr, [sp], #16
ret
alternative_else_nop_endif
alternative_if ARM64_HAS_VIRT_HOST_EXTN
hvc #0x539
alternative_else_nop_endif
b __vhe_hyp_call
ENDPROC(__kvm_call_hyp)
Which, according to objdump gives me the expected result:
Disassembly of section .text:
0000000000000000 <__kvm_call_hyp>:
0: f81f0ffe str x30, [sp,#-16]!
4: d4000002 hvc #0x0
8: f84107fe ldr x30, [sp],#16
c: d65f03c0 ret
10: d503201f nop
14: 14000000 b 0 <__vhe_hyp_call>
Disassembly of section .altinstr_replacement:
0000000000000000 <.altinstr_replacement>:
0: d503201f nop
4: d503201f nop
8: d503201f nop
c: d503201f nop
10: d400a722 hvc #0x539
Thanks,
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
2016-09-13 8:57 ` Mark Rutland
@ 2016-09-13 8:59 ` Ard Biesheuvel
-1 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2016-09-13 8:59 UTC (permalink / raw)
To: Mark Rutland
Cc: Marc Zyngier, Andre Przywara, Will Deacon, kvmarm,
Catalin Marinas, Dave Martin, linux-arm-kernel
On 13 September 2016 at 09:57, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Sep 13, 2016 at 09:36:14AM +0100, Ard Biesheuvel wrote:
>> On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
>> > In some cases, one side of an alternative sequence is simply a number of
>> > NOPs used to balance the other side. Keeping track of this manually is
>> > tedious, and the presence of large chains of NOPs makes the code more
>> > painful to read than necessary.
>> >
>> > To ameliorate matters, this patch adds a new alternative_else_nop_endif,
>> > which automatically balances an alternative sequence with a trivial NOP
>> > sled.
>> >
>> > In many cases, we would like a NOP-sled in the default case, and
>> > instructions patched in in the presence of a feature. To enable the NOPs
>> > to be generated automatically for this case, this patch also adds a new
>> > alternative_if, and updates alternative_else and alternative_endif to
>> > work with either alternative_if or alternative_endif.
>
> [...]
>
>> > +/*
>> > + * Begin an alternative code sequence.
>> > */
>> > .macro alternative_if_not cap
>> > + .set .Lasm_alt_mode, 0
>>
>> Given that only a single copy of this symbol will exist in an object
>> file, is it still possible to use both variants in a single
>> compilation/assembly unit?
>
> Yes.
>
> GAS allows the symbol to be set multiple times (so long as the
> assignments are constant values). The last assignment "wins" when it
> comes to output, but assembler macros are evaluated before this, and use
> the most recent assignment.
>
> In testing I hacked __kvm_call_hyp to use both:
>
> ENTRY(__kvm_call_hyp)
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> str lr, [sp, #-16]!
> hvc #0
> ldr lr, [sp], #16
> ret
> alternative_else_nop_endif
> alternative_if ARM64_HAS_VIRT_HOST_EXTN
> hvc #0x539
> alternative_else_nop_endif
> b __vhe_hyp_call
> ENDPROC(__kvm_call_hyp)
>
> Which, according to objdump gives me the expected result:
>
> Disassembly of section .text:
>
> 0000000000000000 <__kvm_call_hyp>:
> 0: f81f0ffe str x30, [sp,#-16]!
> 4: d4000002 hvc #0x0
> 8: f84107fe ldr x30, [sp],#16
> c: d65f03c0 ret
> 10: d503201f nop
> 14: 14000000 b 0 <__vhe_hyp_call>
>
> Disassembly of section .altinstr_replacement:
>
> 0000000000000000 <.altinstr_replacement>:
> 0: d503201f nop
> 4: d503201f nop
> 8: d503201f nop
> c: d503201f nop
> 10: d400a722 hvc #0x539
>
Thanks for the clarification,
Ard.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] arm64: alternative: add auto-nop infrastructure
@ 2016-09-13 8:59 ` Ard Biesheuvel
0 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2016-09-13 8:59 UTC (permalink / raw)
To: linux-arm-kernel
On 13 September 2016 at 09:57, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Sep 13, 2016 at 09:36:14AM +0100, Ard Biesheuvel wrote:
>> On 7 September 2016 at 11:07, Mark Rutland <mark.rutland@arm.com> wrote:
>> > In some cases, one side of an alternative sequence is simply a number of
>> > NOPs used to balance the other side. Keeping track of this manually is
>> > tedious, and the presence of large chains of NOPs makes the code more
>> > painful to read than necessary.
>> >
>> > To ameliorate matters, this patch adds a new alternative_else_nop_endif,
>> > which automatically balances an alternative sequence with a trivial NOP
>> > sled.
>> >
>> > In many cases, we would like a NOP-sled in the default case, and
>> > instructions patched in in the presence of a feature. To enable the NOPs
>> > to be generated automatically for this case, this patch also adds a new
>> > alternative_if, and updates alternative_else and alternative_endif to
>> > work with either alternative_if or alternative_endif.
>
> [...]
>
>> > +/*
>> > + * Begin an alternative code sequence.
>> > */
>> > .macro alternative_if_not cap
>> > + .set .Lasm_alt_mode, 0
>>
>> Given that only a single copy of this symbol will exist in an object
>> file, is it still possible to use both variants in a single
>> compilation/assembly unit?
>
> Yes.
>
> GAS allows the symbol to be set multiple times (so long as the
> assignments are constant values). The last assignment "wins" when it
> comes to output, but assembler macros are evaluated before this, and use
> the most recent assignment.
>
> In testing I hacked __kvm_call_hyp to use both:
>
> ENTRY(__kvm_call_hyp)
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> str lr, [sp, #-16]!
> hvc #0
> ldr lr, [sp], #16
> ret
> alternative_else_nop_endif
> alternative_if ARM64_HAS_VIRT_HOST_EXTN
> hvc #0x539
> alternative_else_nop_endif
> b __vhe_hyp_call
> ENDPROC(__kvm_call_hyp)
>
> Which, according to objdump gives me the expected result:
>
> Disassembly of section .text:
>
> 0000000000000000 <__kvm_call_hyp>:
> 0: f81f0ffe str x30, [sp,#-16]!
> 4: d4000002 hvc #0x0
> 8: f84107fe ldr x30, [sp],#16
> c: d65f03c0 ret
> 10: d503201f nop
> 14: 14000000 b 0 <__vhe_hyp_call>
>
> Disassembly of section .altinstr_replacement:
>
> 0000000000000000 <.altinstr_replacement>:
> 0: d503201f nop
> 4: d503201f nop
> 8: d503201f nop
> c: d503201f nop
> 10: d400a722 hvc #0x539
>
Thanks for the clarification,
Ard.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] arm64: use alternative auto-nop
2016-09-07 10:07 ` Mark Rutland
@ 2016-09-07 10:07 ` Mark Rutland
-1 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
Cc: marc.zyngier, andre.przywara, will.deacon, dave.martin,
catalin.marinas, kvmarm
Make use of the new alternative_if and alternative_else_nop_endif and
get rid of our homebew NOP sleds, making the code simpler to read.
Note that for cpu_do_switch_mm the ret has been moved out of the
alternative sequence, and in the default case there will be three
additional NOPs executed.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/kernel/entry.S | 10 ++--------
arch/arm64/lib/copy_page.S | 13 ++++---------
arch/arm64/mm/proc.S | 9 ++-------
3 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 441420c..1c76066 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -150,13 +150,7 @@
ldr x23, [sp, #S_SP] // load return stack pointer
msr sp_el0, x23
#ifdef CONFIG_ARM64_ERRATUM_845719
-alternative_if_not ARM64_WORKAROUND_845719
- nop
- nop
-#ifdef CONFIG_PID_IN_CONTEXTIDR
- nop
-#endif
-alternative_else
+alternative_if ARM64_WORKAROUND_845719
tbz x22, #4, 1f
#ifdef CONFIG_PID_IN_CONTEXTIDR
mrs x29, contextidr_el1
@@ -165,7 +159,7 @@ alternative_else
msr contextidr_el1, xzr
#endif
1:
-alternative_endif
+alternative_else_nop_endif
#endif
.endif
msr elr_el1, x21 // set up the return data
diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
index 4c1e700..c3cd65e 100644
--- a/arch/arm64/lib/copy_page.S
+++ b/arch/arm64/lib/copy_page.S
@@ -29,14 +29,11 @@
* x1 - src
*/
ENTRY(copy_page)
-alternative_if_not ARM64_HAS_NO_HW_PREFETCH
- nop
- nop
-alternative_else
+alternative_if ARM64_HAS_NO_HW_PREFETCH
# Prefetch two cache lines ahead.
prfm pldl1strm, [x1, #128]
prfm pldl1strm, [x1, #256]
-alternative_endif
+alternative_else_nop_endif
ldp x2, x3, [x1]
ldp x4, x5, [x1, #16]
@@ -52,11 +49,9 @@ alternative_endif
1:
subs x18, x18, #128
-alternative_if_not ARM64_HAS_NO_HW_PREFETCH
- nop
-alternative_else
+alternative_if ARM64_HAS_NO_HW_PREFETCH
prfm pldl1strm, [x1, #384]
-alternative_endif
+alternative_else_nop_endif
stnp x2, x3, [x0]
ldp x2, x3, [x1]
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5bb61de..2bb088f 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -125,17 +125,12 @@ ENTRY(cpu_do_switch_mm)
bfi x0, x1, #48, #16 // set the ASID
msr ttbr0_el1, x0 // set TTBR0
isb
-alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
- ret
- nop
- nop
- nop
-alternative_else
+alternative_if ARM64_WORKAROUND_CAVIUM_27456
ic iallu
dsb nsh
isb
+alternative_else_nop_endif
ret
-alternative_endif
ENDPROC(cpu_do_switch_mm)
.pushsection ".idmap.text", "ax"
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] arm64: use alternative auto-nop
@ 2016-09-07 10:07 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
Make use of the new alternative_if and alternative_else_nop_endif and
get rid of our homebew NOP sleds, making the code simpler to read.
Note that for cpu_do_switch_mm the ret has been moved out of the
alternative sequence, and in the default case there will be three
additional NOPs executed.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/kernel/entry.S | 10 ++--------
arch/arm64/lib/copy_page.S | 13 ++++---------
arch/arm64/mm/proc.S | 9 ++-------
3 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 441420c..1c76066 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -150,13 +150,7 @@
ldr x23, [sp, #S_SP] // load return stack pointer
msr sp_el0, x23
#ifdef CONFIG_ARM64_ERRATUM_845719
-alternative_if_not ARM64_WORKAROUND_845719
- nop
- nop
-#ifdef CONFIG_PID_IN_CONTEXTIDR
- nop
-#endif
-alternative_else
+alternative_if ARM64_WORKAROUND_845719
tbz x22, #4, 1f
#ifdef CONFIG_PID_IN_CONTEXTIDR
mrs x29, contextidr_el1
@@ -165,7 +159,7 @@ alternative_else
msr contextidr_el1, xzr
#endif
1:
-alternative_endif
+alternative_else_nop_endif
#endif
.endif
msr elr_el1, x21 // set up the return data
diff --git a/arch/arm64/lib/copy_page.S b/arch/arm64/lib/copy_page.S
index 4c1e700..c3cd65e 100644
--- a/arch/arm64/lib/copy_page.S
+++ b/arch/arm64/lib/copy_page.S
@@ -29,14 +29,11 @@
* x1 - src
*/
ENTRY(copy_page)
-alternative_if_not ARM64_HAS_NO_HW_PREFETCH
- nop
- nop
-alternative_else
+alternative_if ARM64_HAS_NO_HW_PREFETCH
# Prefetch two cache lines ahead.
prfm pldl1strm, [x1, #128]
prfm pldl1strm, [x1, #256]
-alternative_endif
+alternative_else_nop_endif
ldp x2, x3, [x1]
ldp x4, x5, [x1, #16]
@@ -52,11 +49,9 @@ alternative_endif
1:
subs x18, x18, #128
-alternative_if_not ARM64_HAS_NO_HW_PREFETCH
- nop
-alternative_else
+alternative_if ARM64_HAS_NO_HW_PREFETCH
prfm pldl1strm, [x1, #384]
-alternative_endif
+alternative_else_nop_endif
stnp x2, x3, [x0]
ldp x2, x3, [x1]
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5bb61de..2bb088f 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -125,17 +125,12 @@ ENTRY(cpu_do_switch_mm)
bfi x0, x1, #48, #16 // set the ASID
msr ttbr0_el1, x0 // set TTBR0
isb
-alternative_if_not ARM64_WORKAROUND_CAVIUM_27456
- ret
- nop
- nop
- nop
-alternative_else
+alternative_if ARM64_WORKAROUND_CAVIUM_27456
ic iallu
dsb nsh
isb
+alternative_else_nop_endif
ret
-alternative_endif
ENDPROC(cpu_do_switch_mm)
.pushsection ".idmap.text", "ax"
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] arm64/kvm: use alternative auto-nop
2016-09-07 10:07 ` Mark Rutland
@ 2016-09-07 10:07 ` Mark Rutland
-1 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
Cc: marc.zyngier, andre.przywara, will.deacon, dave.martin,
catalin.marinas, kvmarm
Make use of the new alternative_if and alternative_else_nop_endif and
get rid of our homebew NOP sleds, making the code simpler to read.
Note that for __kvm_call_hyp the branch to __vhe_hyp_call has been moved
out of the alternative sequence, and in the default case there will be
four additional NOPs executed.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: kvmarm@lists.cs.columbia.edu
---
arch/arm64/include/asm/kvm_mmu.h | 10 +++-------
arch/arm64/kvm/hyp.S | 6 +-----
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index b6bb834..dff1098 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -99,14 +99,10 @@
.macro kern_hyp_va reg
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
and \reg, \reg, #HYP_PAGE_OFFSET_HIGH_MASK
-alternative_else
- nop
-alternative_endif
-alternative_if_not ARM64_HYP_OFFSET_LOW
- nop
-alternative_else
+alternative_else_nop_endif
+alternative_if ARM64_HYP_OFFSET_LOW
and \reg, \reg, #HYP_PAGE_OFFSET_LOW_MASK
-alternative_endif
+alternative_else_nop_endif
.endm
#else
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 7ce9315..2726635 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -46,10 +46,6 @@ alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
hvc #0
ldr lr, [sp], #16
ret
-alternative_else
+alternative_else_nop_endif
b __vhe_hyp_call
- nop
- nop
- nop
-alternative_endif
ENDPROC(__kvm_call_hyp)
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] arm64/kvm: use alternative auto-nop
@ 2016-09-07 10:07 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-07 10:07 UTC (permalink / raw)
To: linux-arm-kernel
Make use of the new alternative_if and alternative_else_nop_endif and
get rid of our homebew NOP sleds, making the code simpler to read.
Note that for __kvm_call_hyp the branch to __vhe_hyp_call has been moved
out of the alternative sequence, and in the default case there will be
four additional NOPs executed.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: kvmarm at lists.cs.columbia.edu
---
arch/arm64/include/asm/kvm_mmu.h | 10 +++-------
arch/arm64/kvm/hyp.S | 6 +-----
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index b6bb834..dff1098 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -99,14 +99,10 @@
.macro kern_hyp_va reg
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
and \reg, \reg, #HYP_PAGE_OFFSET_HIGH_MASK
-alternative_else
- nop
-alternative_endif
-alternative_if_not ARM64_HYP_OFFSET_LOW
- nop
-alternative_else
+alternative_else_nop_endif
+alternative_if ARM64_HYP_OFFSET_LOW
and \reg, \reg, #HYP_PAGE_OFFSET_LOW_MASK
-alternative_endif
+alternative_else_nop_endif
.endm
#else
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 7ce9315..2726635 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -46,10 +46,6 @@ alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
hvc #0
ldr lr, [sp], #16
ret
-alternative_else
+alternative_else_nop_endif
b __vhe_hyp_call
- nop
- nop
- nop
-alternative_endif
ENDPROC(__kvm_call_hyp)
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] arm64/kvm: use alternative auto-nop
2016-09-07 10:07 ` Mark Rutland
@ 2016-09-08 11:16 ` Christoffer Dall
-1 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2016-09-08 11:16 UTC (permalink / raw)
To: Mark Rutland
Cc: marc.zyngier, andre.przywara, will.deacon, dave.martin,
catalin.marinas, kvmarm, linux-arm-kernel
On Wed, Sep 07, 2016 at 11:07:10AM +0100, Mark Rutland wrote:
> Make use of the new alternative_if and alternative_else_nop_endif and
> get rid of our homebew NOP sleds, making the code simpler to read.
homebew?
>
> Note that for __kvm_call_hyp the branch to __vhe_hyp_call has been moved
> out of the alternative sequence, and in the default case there will be
> four additional NOPs executed.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: kvmarm@lists.cs.columbia.edu
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> arch/arm64/include/asm/kvm_mmu.h | 10 +++-------
> arch/arm64/kvm/hyp.S | 6 +-----
> 2 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index b6bb834..dff1098 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -99,14 +99,10 @@
> .macro kern_hyp_va reg
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> and \reg, \reg, #HYP_PAGE_OFFSET_HIGH_MASK
> -alternative_else
> - nop
> -alternative_endif
> -alternative_if_not ARM64_HYP_OFFSET_LOW
> - nop
> -alternative_else
> +alternative_else_nop_endif
> +alternative_if ARM64_HYP_OFFSET_LOW
> and \reg, \reg, #HYP_PAGE_OFFSET_LOW_MASK
> -alternative_endif
> +alternative_else_nop_endif
> .endm
>
> #else
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 7ce9315..2726635 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -46,10 +46,6 @@ alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> hvc #0
> ldr lr, [sp], #16
> ret
> -alternative_else
> +alternative_else_nop_endif
> b __vhe_hyp_call
> - nop
> - nop
> - nop
> -alternative_endif
> ENDPROC(__kvm_call_hyp)
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] arm64/kvm: use alternative auto-nop
@ 2016-09-08 11:16 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2016-09-08 11:16 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Sep 07, 2016 at 11:07:10AM +0100, Mark Rutland wrote:
> Make use of the new alternative_if and alternative_else_nop_endif and
> get rid of our homebew NOP sleds, making the code simpler to read.
homebew?
>
> Note that for __kvm_call_hyp the branch to __vhe_hyp_call has been moved
> out of the alternative sequence, and in the default case there will be
> four additional NOPs executed.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: kvmarm at lists.cs.columbia.edu
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> arch/arm64/include/asm/kvm_mmu.h | 10 +++-------
> arch/arm64/kvm/hyp.S | 6 +-----
> 2 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index b6bb834..dff1098 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -99,14 +99,10 @@
> .macro kern_hyp_va reg
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> and \reg, \reg, #HYP_PAGE_OFFSET_HIGH_MASK
> -alternative_else
> - nop
> -alternative_endif
> -alternative_if_not ARM64_HYP_OFFSET_LOW
> - nop
> -alternative_else
> +alternative_else_nop_endif
> +alternative_if ARM64_HYP_OFFSET_LOW
> and \reg, \reg, #HYP_PAGE_OFFSET_LOW_MASK
> -alternative_endif
> +alternative_else_nop_endif
> .endm
>
> #else
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 7ce9315..2726635 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -46,10 +46,6 @@ alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> hvc #0
> ldr lr, [sp], #16
> ret
> -alternative_else
> +alternative_else_nop_endif
> b __vhe_hyp_call
> - nop
> - nop
> - nop
> -alternative_endif
> ENDPROC(__kvm_call_hyp)
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] arm64/kvm: use alternative auto-nop
2016-09-08 11:16 ` Christoffer Dall
@ 2016-09-08 11:33 ` Mark Rutland
-1 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-08 11:33 UTC (permalink / raw)
To: Christoffer Dall
Cc: marc.zyngier, andre.przywara, will.deacon, dave.martin,
catalin.marinas, kvmarm, linux-arm-kernel
On Thu, Sep 08, 2016 at 01:16:48PM +0200, Christoffer Dall wrote:
> On Wed, Sep 07, 2016 at 11:07:10AM +0100, Mark Rutland wrote:
> > Make use of the new alternative_if and alternative_else_nop_endif and
> > get rid of our homebew NOP sleds, making the code simpler to read.
>
> homebew?
Whoops. Should have been 'homebrew', though I'll change that to
'open-coded' to make this clearer.
> > Note that for __kvm_call_hyp the branch to __vhe_hyp_call has been moved
> > out of the alternative sequence, and in the default case there will be
> > four additional NOPs executed.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: Christoffer Dall <christoffer.dall@linaro.org>
> > Cc: kvmarm@lists.cs.columbia.edu
>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Cheers!
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] arm64/kvm: use alternative auto-nop
@ 2016-09-08 11:33 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2016-09-08 11:33 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 08, 2016 at 01:16:48PM +0200, Christoffer Dall wrote:
> On Wed, Sep 07, 2016 at 11:07:10AM +0100, Mark Rutland wrote:
> > Make use of the new alternative_if and alternative_else_nop_endif and
> > get rid of our homebew NOP sleds, making the code simpler to read.
>
> homebew?
Whoops. Should have been 'homebrew', though I'll change that to
'open-coded' to make this clearer.
> > Note that for __kvm_call_hyp the branch to __vhe_hyp_call has been moved
> > out of the alternative sequence, and in the default case there will be
> > four additional NOPs executed.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: Christoffer Dall <christoffer.dall@linaro.org>
> > Cc: kvmarm at lists.cs.columbia.edu
>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Cheers!
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread