Re: [PATCH] x86: fix early boot output

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/07/2023 8:38 am, Jan Beulich wrote:
> Loading the VGA base address involves sym_esi(), i.e. %esi still needs
> to hold the relocation base address. Therefore the address of the
> message to output cannot be "passed" in %esi. Put the message offset in
> %ecx instead, adding it into %esi _after_ its last use as base address.
>
> Fixes: b28044226e1c ("x86: make Xen early boot code relocatable")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

When I was doing the label cleanup, I did wonder how this worked, given
that it clobbered %esi.  I guess this is the answer...

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Although it occurs to me that probably want to (optionally) use one of
the IO-port/Hypercall protocols too to get these messages in PVH boot
case too.

> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -184,12 +184,15 @@ early_error: /* Here to improve the disa
>           * Here we are on EFI IA-32 platform. Then reliable vga_text_buffer 
> zap is
>           * impossible in Multiboot2 scanning loop and we have to zero %edi 
> below.
>           */
> -        add     $sym_offs(.Lbad_efi_msg), %esi
> +        mov     $sym_offs(.Lbad_efi_msg), %ecx
>          xor     %edi,%edi                       # No VGA text buffer
>          jmp     .Lprint_err
>  .Lget_vtb:
>          mov     sym_esi(vga_text_buffer), %edi
>  .Lprint_err:
> +        add     %ecx, %esi     # Add string offset to relocation base.
> +        # NOTE: No further use of sym_esi() till the end of the "function"!

Minor, but I'd phrase this as "Note: sym_esi() no longer useable".

It is obviously limited in scope, but "until the end of the function"
gives an implication that it's fine thereafter which isn't really true.

~Andrew