On 02/05/2023 10:22 am, Roger Pau Monne wrote: > Ensure that the base address is 2M aligned, or else the page table > entries created would be corrupt as reserved bits on the PDE end up > set. > > We have found a broken firmware where the loader would end up loading > Xen at a non 2M aligned region, and that caused a very difficult to > debug triple fault.
It's probably worth saying that in this case, the OEM has fixed their firmware. > > If the alignment is not as required by the page tables print an error > message and stop the boot. > > The check could be performed earlier, but so far the alignment is > required by the page tables, and hence feels more natural that the > check lives near to the piece of code that requires it. > > Note that when booted as an EFI application from the PE entry point > the alignment check is already performed by > efi_arch_load_addr_check(), and hence there's no need to add another > check at the point where page tables get built in > efi_arch_memory_setup(). > > Signed-off-by: Roger Pau Monné <[email protected]> > --- > xen/arch/x86/boot/head.S | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index 0fb7dd3029f2..ff73c1d274c4 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -121,6 +121,7 @@ multiboot2_header: > .Lbad_ldr_nst: .asciz "ERR: EFI SystemTable is not provided by bootloader!" > .Lbad_ldr_nih: .asciz "ERR: EFI ImageHandle is not provided by bootloader!" > .Lbad_efi_msg: .asciz "ERR: EFI IA-32 platforms are not supported!" > +.Lbag_alg_msg: .asciz "ERR: Xen must be loaded at a 2Mb boundary!" > > .section .init.data, "aw", @progbits > .align 4 > @@ -146,6 +147,9 @@ bad_cpu: > not_multiboot: > add $sym_offs(.Lbad_ldr_msg),%esi # Error message > jmp .Lget_vtb > +not_aligned: > + add $sym_offs(.Lbag_alg_msg),%esi # Error message > + jmp .Lget_vtb > .Lmb2_no_st: > /* > * Here we are on EFI platform. vga_text_buffer was zapped earlier > @@ -670,6 +674,11 @@ trampoline_setup: > cmp %edi, %eax > jb 1b > > + /* Check that the image base is aligned. */ > + lea sym_esi(_start), %eax > + and $(1 << L2_PAGETABLE_SHIFT) - 1, %eax > + jnz not_aligned You just want to check the value in %esi, which is the base of the Xen image. Something like: mov %esi, %eax and ... jnz No need to reference the _start label, or use sym_esi(). Otherwise, LGTM. ~Andrew
