[PATCH 0/3] arm64: alternative: add auto-nop support

[PATCH 0/3] arm64: alternative: add auto-nop support

[Mark Rutland]

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

[PATCH 0/3] arm64: alternative: add auto-nop support

[Mark Rutland]

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

[PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Mark Rutland]

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

[PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Mark Rutland]

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

Re: [PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Ard Biesheuvel]

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

[PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Ard Biesheuvel]

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

Re: [PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Mark Rutland]

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.

[PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Mark Rutland]

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.

Re: [PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Ard Biesheuvel]

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.

[PATCH 1/3] arm64: alternative: add auto-nop infrastructure

[Ard Biesheuvel]

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.

[PATCH 2/3] arm64: use alternative auto-nop

[Mark Rutland]

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

[PATCH 2/3] arm64: use alternative auto-nop

[Mark Rutland]

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

[PATCH 3/3] arm64/kvm: use alternative auto-nop

[Mark Rutland]

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

[PATCH 3/3] arm64/kvm: use alternative auto-nop

[Mark Rutland]

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

Re: [PATCH 3/3] arm64/kvm: use alternative auto-nop

[Christoffer Dall]

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
> 

[PATCH 3/3] arm64/kvm: use alternative auto-nop

[Christoffer Dall]

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
> 

Re: [PATCH 3/3] arm64/kvm: use alternative auto-nop

[Mark Rutland]

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.

[PATCH 3/3] arm64/kvm: use alternative auto-nop

[Mark Rutland]

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.