[PATCH 2/2] arm64: alternatives: Work around NOP generation with broken assembler

[will.deacon@arm.com (Will Deacon)]

On Sat, Dec 03, 2016 at 02:05:38PM +0000, Marc Zyngier wrote:
> When compiling a .inst directive in an alternative clause with
> a rather old version of gas (circa 2.24), and when used with
> the alternative_else_nop_endif feature, the compilation fails
> with a rather cryptic message such as:
> 
> arch/arm64/lib/clear_user.S:33: Error: bad or irreducible absolute expression
> 
> which is caused by the bug described in eb7c11ee3c5c ("arm64:
> alternative: Work around .inst assembler bugs").
> 
> This effectively prevents the use of the "nops" macro, which
> requires the number of instruction as a parameter (the assembler
> is confused and unable to compute the difference between two labels).
> 
> As an alternative(!), use the .fill directive to output the number
> of required NOPs (.fill has the good idea to output the fill pattern
> in the endianness of the instruction stream).

Are you sure about that? The gas docs say:

`The contents of each repeat bytes is taken from an 8-byte number. The
 highest order 4 bytes are zero. The lowest order 4 bytes are value rendered
 in the byte-order of an integer on the computer as is assembling for.'

and I'd expect "integer" to refer to data values, rather than instructions.

> 
> Fixes: 792d47379f4d ("arm64: alternative: add auto-nop infrastructure")
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/include/asm/alternative.h | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 39feb85..39f8c98 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -159,9 +159,19 @@ void apply_alternatives(void *start, size_t length);
>   * of NOPs. The number of NOPs is chosen automatically to match the
>   * previous case.
>   */
> +
> +#define NOP_INST	0xd503201f

If we define this, let's put it in insn.h or something. It could even be
used in the vdso linker script, which open codes this constant.

> +
>  .macro alternative_else_nop_endif
>  alternative_else
> -	nops	(662b-661b) / AARCH64_INSN_SIZE
> +	/*
> +	 * The same assembler bug strikes again here if the first half
> +	 * of the alternative sequence contains a .inst, leading to a
> +	 * bizarre error message. Fortulately, .fill replaces the

Fortunately

> +	 * "nops" macro by inserting padding with the target machine
> +	 * endianness.
> +	 */
> +	.fill	(662b-661b) / AARCH64_INSN_SIZE, AARCH64_INSN_SIZE, NOP_INST

Assuming we can get .fill to do the right thing, we should probably use
it in __nops rather than open-coding it here. Or is the issue that we're
unable to pass (662b-661b) / AARCH64_INSN_SIZE to any macro?

Will