Re: [PATCH] x86/boot: Setup correctly fs segment for bogus_real_magic

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.11.2024 10:17, Frediano Ziglio wrote:
> bogus_real_magic code uses fs segment so it should be initialised.

Like for Andrew's fix:
Fixes: d8c8fef09054 ("Provide basic Xen PM infrastructure")

> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> ---
>  xen/arch/x86/boot/wakeup.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> ---
> OT: Seen another similar patch on ML I suppose this part of code is not that
> tested. Also, considering EFI code, do we always have VGA available in these
> cases?

No, we can't really assume so. This is "best effort" only, with the hope that
if there's no VGA there, then there's also nothing else there which might
dislike the writes.

> --- a/xen/arch/x86/boot/wakeup.S
> +++ b/xen/arch/x86/boot/wakeup.S
> @@ -20,6 +20,8 @@ ENTRY(wakeup_start)
>          movw    %ax, %ds
>          movw    %ax, %ss        # A stack required for BIOS call
>          movw    $wakesym(wakeup_stack), %sp
> +        movw    $0xb800, %ax
> +        movw    %ax, %fs
>  
>          pushl   $0              # Kill dangerous flag early
>          popfl
> @@ -44,8 +46,6 @@ ENTRY(wakeup_start)
>          call    mode_setw
>  
>  1:      # Show some progress if VGA is resumed
> -        movw    $0xb800, %ax
> -        movw    %ax, %fs
>          movw    $0x0e00 + 'L', %fs:(0x10)
>  
>          lidt    wakesym(idt_48)

Between these two hunks we have

        lcall   $0xc000, $3
        movw    %cs, %ax        # In case messed by BIOS
        movw    %ax, %ds
        movw    %ax, %ss        # Need this? How to ret if clobbered?

I'd guess that the loading of %fs was deliberately after that point, in
case some BIOSes messed with that, too. Hence I'm unconvinced we can
simply move the loading of %fs; I think it needs duplicating instead.
(That way it's probably also safer wrt the mode_setw() invocation. That
doesn't touch %fs right now, but it's also not said anywhere that it
shouldn't.)

Jan