Re: [PATCH v2 1/2] x86/head: check base address alignment

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.05.2023 16:59, 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 encountered a broken firmware where grub2 would end up loading
> Xen at a non 2M aligned region when using the multiboot2 protocol, and
> that caused a very difficult to debug triple fault.
> 
> If the alignment is not as required by the page tables print an error
> message and stop the boot.  Also add a build time check that the
> calculation of symbol offsets don't break alignment of passed
> addresses.
> 
> 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é <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Would you mind if, while committing, ...

> @@ -146,6 +148,9 @@ bad_cpu:
>  not_multiboot:
>          add     $sym_offs(.Lbad_ldr_msg),%esi   # Error message
>          jmp     .Lget_vtb
> +not_aligned:

... a .L prefix was added to this label, bringing it out of sync with the
earlier one, but in line with e.g. ...

> +        add     $sym_offs(.Lbag_alg_msg),%esi   # Error message
> +        jmp     .Lget_vtb
>  .Lmb2_no_st:

... this one? I don't think the label is particularly useful to have in
the symbol table (nor are not_multiboot and likely a few others).

Jan