[PATCH 4/4] arm64: head: tidy up the Image header definition

[Robin Murphy]

On 2020-10-28 14:17, Will Deacon wrote:
> On Tue, Oct 27, 2020 at 08:32:09AM +0100, Ard Biesheuvel wrote:
>> Even though support for EFI boot remains entirely optional for arm64,
>> it is unlikely that we will ever be able to repurpose the image header
>> fields that the EFI loader relies on, i.e., the magic NOP at offset
>> 0x0 and the PE header address at offset 0x3c.
>>
>> So let's factor out the differences into a 'magic_nop' macro and a local
>> symbol representing the PE header address, and move the conditional
>> definitions into efi-header.S, taking into account whether CONFIG_EFI is
>> enabled or not.
> 
> How many architectures can claim to have both a "magic nop" and a
> "mysterious nop", hey?

It's fun 'n'all, but putting my serious hat on for a moment, if the name 
still requires a comment to explain it at point of use, it's not a very 
good name :(

At worst magic_nop is even potentially misleading, since it's not 
necessarily a nop; there's no mention of the implicit dependency on a 
context where the side-effect of executing it wouldn't affect anything 
important.

Could we call the macro itself something clear and self-explanatory like 
efi_signature_insn please? I'm happy for it to be *commented* as "Magic 
NOP" if you want parity with the VDSO :D

Robin.

>> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
>> ---
>>   arch/arm64/kernel/efi-header.S | 43 +++++++++++++++-----
>>   arch/arm64/kernel/head.S       | 19 +--------
>>   2 files changed, 35 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
>> index ddaf57d825b5..7b7ac4316d95 100644
>> --- a/arch/arm64/kernel/efi-header.S
>> +++ b/arch/arm64/kernel/efi-header.S
>> @@ -7,7 +7,27 @@
>>   #include <linux/pe.h>
>>   #include <linux/sizes.h>
>>   
>> +	.macro	magic_nop
>> +#ifdef CONFIG_EFI
>> +.L_head:
>> +	/*
>> +	 * This add instruction has no meaningful effect except that
>> +	 * its opcode forms the magic "MZ" signature required by UEFI.
>> +	 */
>> +	add	x13, x18, #0x16
> 
> It's probably faster too ;)
> 
>> +#else
>> +	/*
>> +	 * Bootloaders may inspect the opcode at the start of the kernel
>> +	 * image to decide if the kernel is capable of booting via UEFI.
>> +	 * So put an ordinary NOP here, not the "MZ.." pseudo-nop above.
>> +	 */
>> +	nop
> 
> Let's just hope nobody was decoding the branch instruction...
> 
> Acked-by: Will Deacon >will at kernel.org>
> 
> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>