On 12.08.2024 23:40, Andrew Cooper wrote: > On 08/08/2024 9:49 am, Jan Beulich wrote: >> --- a/xen/arch/x86/boot/head.S >> +++ b/xen/arch/x86/boot/head.S >> @@ -233,13 +233,11 @@ __efi64_mb2_start: >> >> /* Check for Multiboot2 bootloader. */ >> cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax >> - je .Lefi_multiboot2_proto >> >> /* Jump to .Lnot_multiboot after switching CPU to x86_32 mode. */ >> lea .Lnot_multiboot(%rip), %r15 >> - jmp x86_32_switch >> + jne x86_32_switch >> >> -.Lefi_multiboot2_proto: >> /* Zero EFI SystemTable, EFI ImageHandle addresses and cmdline. */ >> xor %esi,%esi >> xor %edi,%edi > > > You've split the logical in two, and now the comment is in the wrong > position. > > If you're going to make this change, it wants to end up reading: > > /* Jump to .Lnot_multiboot after switching CPU to x86_32 mode. */ > lea .Lnot_multiboot(%rip), %r15 > > /* Check for Multiboot2 bootloader. */ > cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax > jne x86_32_switch > > > Not that it's really relevant, but this form also macrofuses nicely.
All true, yet I'm sure you also read my response to Alejandro: Then we ought to also change the other similar instances. The goal of this simple change, after all, is to get all similar pieces of code into consistent shape. Jan
