diff mbox series

[RFC,1/7] x86/wakeup: Stop using %fs for lidt/lgdt

Message ID 22e1febec473b55fd7e43ffe02fb3a81f70a5e86.1556708226.git.dwmw2@infradead.org (mailing list archive)
State Superseded
Headers show
Series Clean up x86_64 boot code | expand

Commit Message

David Woodhouse May 1, 2019, 11:17 a.m. UTC 
From: David Woodhouse <dwmw@amazon.co.uk>

The wakeup code is now relocated alongside the trampoline code, so %ds
is just fine here.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 xen/arch/x86/boot/wakeup.S | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

Andrew Cooper May 1, 2019, 12:02 p.m. UTC | #1 
On 01/05/2019 12:17, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The wakeup code is now relocated alongside the trampoline code, so %ds
> is just fine here.

AFACIT, this is as a consequence of c/s bc4a68f21f43c2 "x86: make
run-time part of trampoline relocatable" ?

> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

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

I don't have a sensible way of testing this ATM, but the change looks
correct.

~Andrew
Andrew Cooper May 1, 2019, 4:09 p.m. UTC | #2 
On 01/05/2019 13:02, Andrew Cooper wrote:
> On 01/05/2019 12:17, David Woodhouse wrote:
>> From: David Woodhouse <dwmw@amazon.co.uk>
>>
>> The wakeup code is now relocated alongside the trampoline code, so %ds
>> is just fine here.
> AFACIT, this is as a consequence of c/s bc4a68f21f43c2 "x86: make
> run-time part of trampoline relocatable" ?
>
>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> I don't have a sensible way of testing this ATM, but the change looks
> correct.

I'm afraid testing says no.  S3 works fine without this change, and
resets with it.

~Andrew
David Woodhouse May 1, 2019, 6:33 p.m. UTC | #3 
On Wed, 2019-05-01 at 17:09 +0100, Andrew Cooper wrote:
> I'm afraid testing says no.  S3 works fine without this change, and
> resets with it.

Thanks for testing. That's obvious in retrospect — although the wakeup
code is relocated alongside the trampoline code, it runs in real mode
with its own segment, and %ip=0000. So the idt_48 and gdt_48 really do
need to use "wakesym". For which they need to be shifted later in
trampline.S so that they're actually within the range of that segment.

However, neither staging-4.11 nor master are working here even before
the patch, so while I can fix my patch to go back to the existing
breakage and not add my own, I haven't managed to test. This is what I
get:

[root@localhost ~]# echo mem > /sys/power/state 
(XEN) Preparing system for ACPI S3 state.
(XEN) Disabling non-boot CPUs ...
(XEN) Broke affinity for irq 14
(XEN) Removing cpu 1 from runqueue 1
(XEN)  No cpus left on runqueue, disabling
(XEN) Entering ACPI S3 state.
QEMU 2.11.1 monitor - type 'help' for more information
(qemu) system_wakeup 
(qemu) (XEN) mce_intel.c:780: MCA Capability: firstbank 1, extended MCE MSR 0, SER
(XEN) Finishing wakeup from ACPI S3 state.
(XEN) Enabling non-boot CPUs  ...
(XEN) Adding cpu 1 to runqueue 0
(XEN) Assertion 'c != old_pool && (c != NULL || old_pool != NULL)' failed at schedule.c:1848
(XEN) ----[ Xen-4.13-unstable  x86_64  debug=y   Not tainted ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82d08023c1b0>] schedule_cpu_switch+0x25b/0x318
(XEN) RFLAGS: 0000000000010246   CONTEXT: hypervisor
(XEN) rax: ffff82d0805c7060   rbx: 0000000000000001   rcx: 00000031bbf05100
(XEN) rdx: ffff83023c509840   rsi: ffff83023c509840   rdi: 0000000000000001
(XEN) rbp: ffff8300bfa8fce0   rsp: ffff8300bfa8fc80   r8:  000000000008f000
(XEN) r9:  ffff82d0805c4ec0   r10: 00000000bf9f7600   r11: 0000000979798d09
(XEN) r12: ffff83023c509840   r13: ffff82d080470720   r14: 0000000000000004
(XEN) r15: 0000000000000000   cr0: 000000008005003b   cr4: 00000000000026e0
(XEN) cr3: 00000000bfa83000   cr2: 0000000000000000
(XEN) fsb: 00007f91b5b6c740   gsb: ffff880075c00000   gss: 0000000000000000
(XEN) ds: 0018   es: 0000   fs: b800   gs: 0000   ss: e010   cs: e008
(XEN) Xen code around <ffff82d08023c1b0> (schedule_cpu_switch+0x25b/0x318):
(XEN)  85 d2 0f 85 f1 fd ff ff <0f> 0b 0f 0b 0f 0b 0f 0b 45 84 c0 75 39 48 8b 46
(XEN) Xen stack trace from rsp=ffff8300bfa8fc80:
(XEN)    0000000000000000 00007cff40570347 ffff82d0803818aa ffff82cfffffb300
(XEN)    ffff82cfffffb280 0000000000000001 0000000000000000 0000000000000001
(XEN)    ffff83023c509840 ffff82d080596128 0000000000000004 0000000000000000
(XEN)    ffff8300bfa8fd10 ffff82d080203f3e 0000000000000286 0000000000000001
(XEN)    0000000000000001 ffff82d080596128 ffff8300bfa8fd40 ffff82d080204108
(XEN)    ffff82d0804859e0 ffff82d080485368 ffff82d080485360 ffff82d080485348
(XEN)    ffff8300bfa8fd90 ffff82d0802251bf 0000000000000000 0000000000000001
(XEN)    ffff8300bfa8fd80 0000000000000000 0000000000000001 ffff82d0803e9e81
(XEN)    0000000000000001 ffff8300bfa8ffff ffff8300bfa8fdc0 ffff82d0802039d0
(XEN)    ffff8300bfa8ffff ffff82d080485340 0000000000000001 0000000000000003
(XEN)    ffff8300bfa8fdf0 ffff82d080203bc9 0000000000000000 0000000000000003
(XEN)    ffff8300bfa8ffff 00000000000026e0 ffff8300bfa8fe40 ffff82d0802dcd03
(XEN)    ffff83023c49b000 0000000000000282 0000000000000000 ffff83023c51ef90
(XEN)    ffff83023c49b000 0000000000000000 0000000000000000 ffff8300bfa8ffff
(XEN)    ffff8300bfa8fe60 ffff82d080208355 ffff83023c49b1d8 ffff82d0805c7170
(XEN)    ffff8300bfa8fe80 ffff82d08023f7b4 ffff8300bfa8fe80 ffff82d0805c7160
(XEN)    ffff8300bfa8feb0 ffff82d08023fadc 0000000000000000 ffff82d0805c7170
(XEN)    ffff82d08059b880 ffff82d0805b20a0 ffff8300bfa8fef0 ffff82d080272397
(XEN)    ffff83023c4fb000 ffff83023c4fb000 ffff83023c49b000 ffff83023c508000
(XEN)    0000000000000000 ffff83023c4b8000 ffff8300bfa8fda8 ffffc900025abd60
(XEN) Xen call trace:
(XEN)    [<ffff82d08023c1b0>] schedule_cpu_switch+0x25b/0x318
(XEN)    [<ffff82d080203f3e>] cpupool.c#cpupool_assign_cpu_locked+0x31/0x126
(XEN)    [<ffff82d080204108>] cpupool.c#cpu_callback+0xd5/0x31d
(XEN)    [<ffff82d0802251bf>] notifier_call_chain+0x64/0x8f
(XEN)    [<ffff82d0802039d0>] cpu_up+0xca/0xec
(XEN)    [<ffff82d080203bc9>] enable_nonboot_cpus+0x87/0xd9
(XEN)    [<ffff82d0802dcd03>] power.c#enter_state_helper+0xd7/0x48f
(XEN)    [<ffff82d080208355>] domain.c#continue_hypercall_tasklet_handler+0x4a/0xab
(XEN)    [<ffff82d08023f7b4>] tasklet.c#do_tasklet_work+0x7a/0xb2
(XEN)    [<ffff82d08023fadc>] do_tasklet+0x58/0x8a
(XEN)    [<ffff82d080272397>] domain.c#idle_loop+0x5e/0xb9
(XEN) 
(XEN) 
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Assertion 'c != old_pool && (c != NULL || old_pool != NULL)' failed at schedule.c:1848
(XEN) ****************************************
(XEN) 
(XEN) Reboot in five seconds...
diff mbox series

Patch

diff --git a/xen/arch/x86/boot/wakeup.S b/xen/arch/x86/boot/wakeup.S
index f9632eef95..8c52819171 100644
--- a/xen/arch/x86/boot/wakeup.S
+++ b/xen/arch/x86/boot/wakeup.S
@@ -40,11 +40,8 @@  ENTRY(wakeup_start)
         movw    %ax, %fs
         movw    $0x0e00 + 'L', %fs:(0x10)
 
-        # boot trampoline is under 1M, and shift its start into
-        # %fs to reference symbols in that area
-        mov     wakesym(trampoline_seg), %fs
-        lidt    %fs:bootsym(idt_48)
-        lgdt    %fs:bootsym(gdt_48)
+        lidt    bootsym(idt_48)
+        lgdt    bootsym(gdt_48)
 
         movw    $1, %ax
         lmsw    %ax             # Turn on CR0.PE 
@@ -102,10 +99,6 @@  GLOBAL(video_mode)
         .long 0
 GLOBAL(video_flags)
         .long 0
-trampoline_seg: .word 0
-        .pushsection .trampoline_seg, "a"
-        .long   trampoline_seg - .
-        .popsection
 
         .code32