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@cloud.com>
> ---
> 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