| Message ID | 20241111091739.4885-1-frediano.ziglio@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/boot: Setup correctly fs segment for bogus_real_magic | expand |
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
diff --git a/xen/arch/x86/boot/wakeup.S b/xen/arch/x86/boot/wakeup.S index 08447e1934..b73b947bdf 100644 --- 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)
bogus_real_magic code uses fs segment so it should be initialised. 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?