On Tue, Apr 21, 2020 at 03:25:41PM +0100, Catalin Marinas wrote:
> There are situations where we do not want to disable the whole block
> based on a config option, only the alternative part while keeping the
> first instruction. Improve the alternative_insn assembler macro to take
> a 'first_insn' argument, default 0, to preserve the current behaviour.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/alternative.h | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 5e5dc05d63a0..67d7cc608336 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -111,7 +111,11 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
> .byte \alt_len
> .endm
>
> -.macro alternative_insn insn1, insn2, cap, enable = 1
> +/*
> + * Disable the whole block if enable == 0, unless first_insn == 1 in which
> + * case insn1 will always be issued but without an alternative insn2.
> + */
> +.macro alternative_insn insn1, insn2, cap, enable = 1, first_insn = 0
> .if \enable
> 661: \insn1
> 662: .pushsection .altinstructions, "a"
> @@ -122,6 +126,8 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
> 664: .popsection
> .org . - (664b-663b) + (662b-661b)
> .org . - (662b-661b) + (664b-663b)
> + .elseif \first_insn
> + \insn1
This becomes quite unreadable at the invocation site, especially when
invoked as "alternative_insn ..., 1". "... first_insn=1" is not much
better either).
I'm struggling to find non-trivial users of this that actually want the
whole block to be deleted dependent on the config.
Can we instead just always behave as if first_insn=1 instead? This this
works intuitively as an alternative, not the current weird 3-way choice
between insn1, insn2 and nothing at all. The only time that makes sense
is when one of the insns is a branch that skips the block, but that's
handled via the alternative_if macros instead.
Behaving always like first_insn=1 provides an if-else that is statically
optimised if the relevant feature is configured out, which I think is
the only thing people are ever going to want.
Maybe something depends on the current behaviour, but I can't see it so
far...
[...]
Cheers
---Dave
