x86: General protection fault after STR (32 bit systems only)
Subsystems (4)
PATCH 1/1 x86: General protection fault after STR (32 bit systems only)
Suspend to RAM process is returning to userspsace with DS set to KERNEL_DS after resume, this cause general protection fault. This is very difficult to reproduce, but this does happen on 32 bit systems. This crash is not reproduced after save/restore DS during calls to _save_processor_state/ __restore_processor_state respectively.
Similar issue was reported in the past
https://bugzilla.kernel.org/show_bug.cgi?id=61781, for which the root cause was not identified.
--- arch/x86/include/asm/suspend_32.h | 2 +- arch/x86/power/cpu.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
arch/x86/include/asm/suspend_32.h +1 -1
@@ -11,7 +11,7 @@/* image of the saved processor state */ struct saved_context { -u16 es, fs, gs, ss; +u16 ds, es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;u64 misc_enable;bool misc_enable_saved;
arch/x86/power/cpu.c +2 -0
@@ -79,6 +79,7 @@ static void __save_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +savesegment (ds, ctxt->ds);savesegment (es, ctxt->es);savesegment (fs, ctxt->fs);savesegment (gs, ctxt->gs);
@@ -198,6 +199,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +loadsegment (ds, ctxt->ds);loadsegment (es, ctxt->es);loadsegment (fs, ctxt->fs);loadsegment (gs, ctxt->gs);
Signature · 1 line
1.9.3
Quoted 2 lines · Srinivas Pandruvada
Suspend to RAM process is returning to userspsace with DS set to
KERNEL_DSafter resume, this cause general protection fault. [...]
But s2ram has no influence on 'returning to user-space'. So unless I'm missing something this changelog makes no sense.
Unfortunately the patch seems to be completely bogus as well:
Quoted 41 lines
[...] This is very difficult to reproduce, but this does happen on 32 bit systems. This crash is not reproduced after save/restore DS during calls to _save_processor_state/
__restore_processor_staterespectively.Similar issue was reported in the past
https://bugzilla.kernel.org/show_bug.cgi?id=61781, for which the root cause was not identified.Signed-off-by Srinivas Pandruvada <[email protected]>Reviewed-by Andi Kleen <[email protected]>--- arch/x86/include/asm/suspend_32.h | 2 +- arch/x86/power/cpu.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)arch/x86/include/asm/suspend_32.h +1
-1@@ -11,7 +11,7 @@/* image of the saved processor state */ struct saved_context { -u16 es, fs, gs, ss; +u16 ds, es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;u64 misc_enable;bool misc_enable_saved;arch/x86/power/cpu.c +1
-0@@ -79,6 +79,7 @@ static void __save_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +savesegment (ds, ctxt->ds);savesegment (es, ctxt->es);savesegment (fs, ctxt->fs);savesegment (gs, ctxt->gs);
arch/x86/power/cpu.c static void notrace __restore_processor_state(struct saved_context *ctxt) · line 205
Quoted 7 lines
arch/x86/power/cpu.c +1
-0@@ -198,6 +199,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +loadsegment (ds, ctxt->ds);loadsegment (es, ctxt->es);loadsegment (fs, ctxt->fs);loadsegment (gs, ctxt->gs);
So save_processor_state() is used by 32-bit s2ram in the following place:
arch/x86/kernel/acpi/wakeup_32.S: call save_processor_state
Other uses are:
arch/x86/kernel/acpi/wakeup_64.S: call save_processor_state arch/x86/kernel/apm_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_64.c: save_processor_state(); arch/x86/platform/olpc/xo1-wakeup.S: call save_processor_state kernel/power/hibernate.c: save_processor_state(); kernel/power/hibernate.c: save_processor_state();
but neither of these call sites seems to matter to the bug: the 32-bit system does not use APM, kexec, is not an OLPC and does not use hibernation.
And if we look at arch/x86/kernel/acpi/wakeup_32.S it's a straightforward full state save/restore before ACPI (BIOS) downcalls.
Furthermore, the bug report in:
https://bugzilla.kernel.org/show_bug.cgi?id=61781
talks about SIGSEGVs, and claims that this regression triggers in v3.11 but does not trigger in v3.10.
1)
So the first critical question is: if the ACPI/BIOS suspend code corrupts the kernel's DS, how can we get so far as to resume fully, return to user-space, and segfault there so that it can all be reported?
So neither the explanation nor the code makes any sense in the context of the reported bugs. Can anyone else offer any plausible theory about why this patch would fix 32-bit user-space segfaults?
2)
Btw., I don't mind the idea of the patch itself per se: saving/restoring DS is prudent to avoid the BIOS stomping on our DS.
But the restoration done in this patch is bogus, it's done way too late in a C function, there's a number of places where we might use the kernel DS before it's restored, such as restore_registers():
restore_registers:
movl saved_context_ebp, %ebp
movl saved_context_ebx, %ebx
movl saved_context_esi, %esi
movl saved_context_edi, %edi
pushl saved_context_eflags popfl
ret
this is called before restore_processor_state().
those 'saved_context_*' references are already using the kernel DS! So if it's corrupted, we'll crash there already. So your patch seems totally nonsensical.
3)
So the patch below (totally untested) does the DS restoration early on, as the first thing after we emerge from the sleep. This should be equivalent to your patch, but is more robust and much simpler: there's no need to save DS, because we know that it has to be __KERNEL_DS.
Could you please test whether this solves the problem as well? Also, could you please describe how the failure triggers in your system: how many times do you have to suspend/resume to trigger the segfaults, and is there anything that makes the failures less or more likely?
Thanks,
Ingo
=================>
arch/x86/kernel/acpi/wakeup_32.S | 4 ++++ 1 file changed, 4 insertions(+)
arch/x86/kernel/acpi/wakeup_32.S +4 -0
@@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel) jmp ret_point .p2align 4,,7 ret_point: + /* In case the BIOS corrupted DS, make the kernel context minimally functional: */ + movl $__KERNEL_DS, %eax + movl %eax, %ds + call restore_registers call restore_processor_state ret
Quoted 89 lines · Ingo Molnar
Quoted 2 lines · Srinivas Pandruvada
Suspend to RAM process is returning to userspsace with DS set to
KERNEL_DSafter resume, this cause general protection fault. [...]But s2ram has no influence on 'returning to user-space'. So unless I'm missing something this changelog makes no sense.
Unfortunately the patch seems to be completely bogus as well:
Quoted 41 lines
[...] This is very difficult to reproduce, but this does happen on 32 bit systems. This crash is not reproduced after save/restore DS during calls to _save_processor_state/
__restore_processor_staterespectively.Similar issue was reported in the past
https://bugzilla.kernel.org/show_bug.cgi?id=61781, for which the root cause was not identified.Signed-off-by Srinivas Pandruvada <[email protected]>Reviewed-by Andi Kleen <[email protected]>--- arch/x86/include/asm/suspend_32.h | 2 +- arch/x86/power/cpu.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)arch/x86/include/asm/suspend_32.h +1
-1@@ -11,7 +11,7 @@/* image of the saved processor state */ struct saved_context { -u16 es, fs, gs, ss; +u16 ds, es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;u64 misc_enable;bool misc_enable_saved;arch/x86/power/cpu.c +1
-0@@ -79,6 +79,7 @@ static void __save_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +savesegment (ds, ctxt->ds);savesegment (es, ctxt->es);savesegment (fs, ctxt->fs);savesegment (gs, ctxt->gs);arch/x86/power/cpu.c static void notrace __restore_processor_state(struct saved_context *ctxt) · line 205
Quoted 7 lines
arch/x86/power/cpu.c +1
-0@@ -198,6 +199,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +loadsegment (ds, ctxt->ds);loadsegment (es, ctxt->es);loadsegment (fs, ctxt->fs);loadsegment (gs, ctxt->gs);So
save_processor_state()is used by 32-bit s2ram in the following place:
arch/x86/kernel/acpi/wakeup_32.S: callsave_processor_stateOther uses are:
arch/x86/kernel/acpi/wakeup_64.S: call save_processor_state arch/x86/kernel/apm_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_64.c: save_processor_state(); arch/x86/platform/olpc/xo1-wakeup.S: call save_processor_state kernel/power/hibernate.c: save_processor_state(); kernel/power/hibernate.c: save_processor_state();but neither of these call sites seems to matter to the bug: the 32-bit system does not use APM, kexec, is not an OLPC and does not use hibernation.
And if we look at
arch/x86/kernel/acpi/wakeup_32.Sit's a straightforward full state save/restore before ACPI (BIOS) downcalls.Furthermore, the bug report in:
https://bugzilla.kernel.org/show_bug.cgi?id=61781talks about SIGSEGVs, and claims that this regression triggers in v3.11 but does not trigger in v3.10.
1)
Quoted 7 lines
So the first critical question is: if the ACPI/BIOS suspend code corrupts the kernel's DS, how can we get so far as to resume fully, return to user-space, and segfault there so that it can all be reported?
So neither the explanation nor the code makes any sense in the context of the reported bugs. Can anyone else offer any plausible theory about why this patch would fix 32-bit user-space segfaults?
I'm too tired to look at this intelligently right now, but this reminds me of the sysret_ss_attrs thing. What if we have a situation where, after suspend/resume, we end up with a perfectly valid ss *selector* (or, on 64-bit kernels, a ds selector that does not matter one whit) but a somehow-screwed-up ds *cached hidden descriptor*. (On 32-bit kernels, this could be something exotic like grows-down limit 2^31.)
Now we do the very first return. If we're on AMD hardware and that return is SYSRET, then we end up with some complete random garbage loaded in the hidden DS descriptor if SYSRET on 32-bit mode is indeed screwed up on AMD.
Of course, this is apparently DS and not SS, but maybe something similar is happening. How easily reproducible is this, and what cpu is it on? Would something like 'push %ds; pop %ds' after SYSENTER in vdso32/sysenter.S fix it?
Also, damnit systemd, stop catching SEGV. I want to know all the SEGV details, and you helpfully threw them away. Good job.
Quoted 16 lines
2)
Btw., I don't mind the idea of the patch itself per se: saving/restoring DS is prudent to avoid the BIOS stomping on our DS.
But the restoration done in this patch is bogus, it's done way too late in a C function, there's a number of places where we might use the kernel DS before it's restored, such as
restore_registers():
restore_registers:movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esi movl saved_context_edi, %edi pushl saved_context_eflags
Quoted 7 lines
popfl
retthis is called before
restore_processor_state().those 'saved_context_*' references are already using the kernel DS! So if it's corrupted, we'll crash there already. So your patch seems totally nonsensical.
Don't even bother saving it. Just load the known value on resume.
This is a bit odd, in that we don't do the X86_BUG_SYSRET_SS_ATTRS fixup on 32-bit kernels, and yet no one reported the sysret_ss_attrs_32 test failing on 32-bit kernels. I assume that what's going on is that 32-bit kernels don't have any ways to enter the kernel with SS=0 (because ss0 is in the TSS and it isn't zero), but we never thought of the other way to have a bogus SS descriptor in kernel mode: change the GDT without reloading SS.
I'm presently mystified about DS. The 32-bit sysenter path loads __KERNEL_DS into ds (in a macro helpfully called SAVE_ALL), but what, if anything, restores DS on return? Could we actually be executing in 32-bit userspace with DS & 3 == 0 after every SYSEXIT on 32-bit kernels? If so, yikes!
Should we load __USER_DS into DS and ES right before SYSEXIT?
Here's my full-fledged half-asleep theory:
We suspend to RAM. We resume. DS and/or ES contains something unusual but not unusual enough to crash us. Our first entry to userspace is via SYSEXIT. Because we're daft, we don't reload DS or ES at any point along the way. Now we're in userspace with an even more screwed up DS or ES than usual. We get SIGSEGV (presumably #GP) and try to deliver the signal. We end up with impossible pt_regs (bogus RPL) but who cares? We get to __setup_frame, which fixes the garbage in pt_regs and we re-enter user mode through an IRET patch, so we finally reload DS and ES. As a result, we successfully deliver the signal. The saved regs would reveal the damage, but systemd throws them away, and we remain confused for a full ten kernel versions.
--Andy
Quoted 10 lines · Andy Lutomirski
Quoted 9 lines
1)
So the first critical question is: if the ACPI/BIOS suspend code corrupts the kernel's DS, how can we get so far as to resume fully, return to user-space, and segfault there so that it can all be reported?
So neither the explanation nor the code makes any sense in the context of the reported bugs. Can anyone else offer any plausible theory about why this patch would fix 32-bit user-space segfaults?
Quoted 6 lines
I'm too tired to look at this intelligently right now, but this reminds me of the
sysret_ss_attrsthing. What if we have a situation where, after suspend/resume, we end up with a perfectly valid ss *selector* (or, on 64-bit kernels, a ds selector that does not matter one whit) but a somehow-screwed-up ds *cached hidden descriptor*. (On 32-bit kernels, this could be something exotic like grows-down limit 2^31.)
Yes, that theory is what my patch tests, by reloading DS with __KERNEL_DS.
This should be safe as the first thing to execute after re-entry, as we don't save/restore the GDT. (If the BIOS mucks with the GDT without restoring it to our value we are probably screwed in any case.)
Quoted 3 lines
Now we do the very first return. If we're on AMD hardware and that return is SYSRET, then we end up with some complete random garbage loaded in the hidden DS descriptor if SYSRET on 32-bit mode is indeed screwed up on AMD.
But why would this change from v3.10 to v3.11? I cannot see any low level x86 change that should make a difference there.
Quoted 1 line
Don't even bother saving it. Just load the known value on resume.
Yeah, so that's what my simple patch does.
Quoted 12 lines
Here's my full-fledged half-asleep theory:
We suspend to RAM. We resume. DS and/or ES contains something unusual but not unusual enough to crash us. Our first entry to userspace is via SYSEXIT. Because we're daft, we don't reload DS or ES at any point along the way. Now we're in userspace with an even more screwed up DS or ES than usual. We get SIGSEGV (presumably #GP) and try to deliver the signal. We end up with impossible
pt_regs(bogus RPL) but who cares? We get to__setup_frame, which fixes the garbage inpt_regsand we re-enter user mode through an IRET patch, so we finally reload DS and ES. As a result, we successfully deliver the signal. The saved regs would reveal the damage, but systemd throws them away, and we remain confused for a full ten kernel versions.
That's indeed plausible.
If so then the DS reloading patch I sent should help.
So we should also do a full review of all the DS/ES save/restore paths, everywhere, as they don't seem to be very consistently done.
Thanks,
Ingo
Quoted 145 lines · Ingo Molnar
Quoted 2 lines · Srinivas Pandruvada
Suspend to RAM process is returning to userspsace with DS set to
KERNEL_DSafter resume, this cause general protection fault. [...]But s2ram has no influence on 'returning to user-space'. So unless I'm missing something this changelog makes no sense.
Unfortunately the patch seems to be completely bogus as well:
Quoted 41 lines
[...] This is very difficult to reproduce, but this does happen on 32 bit systems. This crash is not reproduced after save/restore DS during calls to _save_processor_state/
__restore_processor_staterespectively.Similar issue was reported in the past
https://bugzilla.kernel.org/show_bug.cgi?id=61781, for which the root cause was not identified.Signed-off-by Srinivas Pandruvada <[email protected]>Reviewed-by Andi Kleen <[email protected]>--- arch/x86/include/asm/suspend_32.h | 2 +- arch/x86/power/cpu.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)arch/x86/include/asm/suspend_32.h +1
-1@@ -11,7 +11,7 @@/* image of the saved processor state */ struct saved_context { -u16 es, fs, gs, ss; +u16 ds, es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;u64 misc_enable;bool misc_enable_saved;arch/x86/power/cpu.c +1
-0@@ -79,6 +79,7 @@ static void __save_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +savesegment (ds, ctxt->ds);savesegment (es, ctxt->es);savesegment (fs, ctxt->fs);savesegment (gs, ctxt->gs);arch/x86/power/cpu.c static void notrace __restore_processor_state(struct saved_context *ctxt) · line 205
Quoted 7 lines
arch/x86/power/cpu.c +1
-0@@ -198,6 +199,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +loadsegment (ds, ctxt->ds);loadsegment (es, ctxt->es);loadsegment (fs, ctxt->fs);loadsegment (gs, ctxt->gs);So
save_processor_state()is used by 32-bit s2ram in the following place:
arch/x86/kernel/acpi/wakeup_32.S: callsave_processor_stateOther uses are:
arch/x86/kernel/acpi/wakeup_64.S: call save_processor_state arch/x86/kernel/apm_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_64.c: save_processor_state(); arch/x86/platform/olpc/xo1-wakeup.S: call save_processor_state kernel/power/hibernate.c: save_processor_state(); kernel/power/hibernate.c: save_processor_state();but neither of these call sites seems to matter to the bug: the 32-bit system does not use APM, kexec, is not an OLPC and does not use hibernation.
And if we look at
arch/x86/kernel/acpi/wakeup_32.Sit's a straightforward full state save/restore before ACPI (BIOS) downcalls.Furthermore, the bug report in:
https://bugzilla.kernel.org/show_bug.cgi?id=61781talks about SIGSEGVs, and claims that this regression triggers in v3.11 but does not trigger in v3.10.
1)
So the first critical question is: if the ACPI/BIOS suspend code corrupts the kernel's DS, how can we get so far as to resume fully, return to user-space, and segfault there so that it can all be reported?
So neither the explanation nor the code makes any sense in the context of the reported bugs. Can anyone else offer any plausible theory about why this patch would fix 32-bit user-space segfaults?
2)
Btw., I don't mind the idea of the patch itself per se: saving/restoring DS is prudent to avoid the BIOS stomping on our DS.
But the restoration done in this patch is bogus, it's done way too late in a C function, there's a number of places where we might use the kernel DS before it's restored, such as
restore_registers():
restore_registers:movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esi movl saved_context_edi, %edi pushl saved_context_eflagspopfl
retthis is called before
restore_processor_state().those 'saved_context_*' references are already using the kernel DS! So if it's corrupted, we'll crash there already. So your patch seems totally nonsensical.
3)
So the patch below (totally untested) does the DS restoration early on, as the first thing after we emerge from the sleep. This should be equivalent to your patch, but is more robust and much simpler: there's no need to save DS, because we know that it has to be
__KERNEL_DS.Could you please test whether this solves the problem as well? Also, could you please describe how the failure triggers in your system: how many times do you have to suspend/resume to trigger the segfaults, and is there anything that makes the failures less or more likely?
Thanks,
Ingo
=================>
arch/x86/kernel/acpi/wakeup_32.S | 4 ++++ 1 file changed, 4 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +0
-0
arch/x86/kernel/acpi/wakeup_32.S ENTRY(do_suspend_lowlevel) · line 87
Quoted 7 lines
arch/x86/kernel/acpi/wakeup_32.S +4
-0@@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel) jmp ret_point .p2align 4,,7 ret_point: + /* In case the BIOS corrupted DS, make the kernel context minimally functional: */ + movl $__KERNEL_DS, %eax + movl %eax, %ds +
On further thought, I think you want movl $__USER_DS, %eax. The 32-bit kernel is a strange beast. Also, you should probably fix up %es as well.
--Andy
Quoted 3 lines
arch/x86/kernel/acpi/wakeup_32.S +0
-0call restore_registers call restore_processor_state ret
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Quoted 6 lines · Andy Lutomirski
Quoted 6 lines
arch/x86/kernel/acpi/wakeup_32.S +0
-0@@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel) jmp ret_point .p2align 4,,7 ret_point:
Quoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(do_suspend_lowlevel) · line 87
Quoted 4 lines
arch/x86/kernel/acpi/wakeup_32.S +4
-0+ /* In case the BIOS corrupted DS, make the kernel context minimally functional: */ + movl $__KERNEL_DS, %eax + movl %eax, %ds +On further thought, I think you want movl $__USER_DS, %eax. The 32-bit kernel is a strange beast. Also, you should probably fix up %es as well.
So restore_processor_state() already restores ES. The idea here was to reload DS early on, because the kernel implicitly uses it for data access so we need it to be good to be able to continue executing any generic kernel code.
We don't use %es: prefixed assembly AFAICS, what are the implicit users of ES?
Also, to further confuse things, we also have:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:
movw $__KERNEL_DS, %ax
movw %ax, %ss
movw %ax, %ds
movw %ax, %es
movw %ax, %fs
movw %ax, %gs # reload the gdt, as we need the full 32 bit address
lidt saved_idt
lldt saved_ldt
ljmp $(__KERNEL_CS), $1f
1:
movl %cr3, %eax
movl %eax, %cr3
wbinvdwhich seems to be another layer of restoration - but it possibly does not trigger in the S2RAM case here.
Oh, funny the 'reload the gdt' comment: do you see an LGDT there? It reloads all segment selectors, the IDT, LDT and CR3, but does not seem to reload the GDT - the only thing the comment describes.
Thanks,
Ingo
H. Peter Anvin
reviewer%es is used implicitly by string instructions.
Quoted 61 lines · Ingo Molnar
Quoted 7 lines · Andy Lutomirski
arch/x86/kernel/acpi/wakeup_32.S ENTRY(do_suspend_lowlevel) · line 84
Quoted 7 lines
arch/x86/kernel/acpi/wakeup_32.S +1
-0@@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel) jmp ret_point .p2align 4,,7 ret_point: + /* In case the BIOS corrupted DS, make the kernel contextminimally functional: */
Quoted 7 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(do_suspend_lowlevel) · line 87
Quoted 3 lines
arch/x86/kernel/acpi/wakeup_32.S +3
-0+ movl $__KERNEL_DS, %eax + movl %eax, %ds +On further thought, I think you want movl $__USER_DS, %eax. The 32-bit kernel is a strange beast. Also, you should probably fix up %es as well.
So
restore_processor_state()already restores ES. The idea here was to reload DS
early on, because the kernel implicitly uses it for data access so we need it to
be good to be able to continue executing any generic kernel code.We don't use %es: prefixed assembly AFAICS, what are the implicit users of ES?
Also, to further confuse things, we also have:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds movw %ax, %es movw %ax, %fs movw %ax, %gs# reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt ljmp $(__KERNEL_CS), $1f 1: movl %cr3, %eax movl %eax, %cr3 wbinvdwhich seems to be another layer of restoration - but it possibly does not trigger
in the S2RAM case here.Oh, funny the 'reload the gdt' comment: do you see an LGDT there? It reloads all
segment selectors, the IDT, LDT and CR3, but does not seem to reload the GDT - the
only thing the comment describes.Thanks,
Ingo
Signature · 1 line
Sent from my mobile phone. Please pardon brevity and lack of formatting.
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in restore_processor_state(), which is a generic C function?
Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/x86/kernel/acpi/wakeup_32.S +13 -0
@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ss + movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esi
Quoted 38 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +6
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 10 lines
arch/x86/kernel/acpi/wakeup_32.S +7
-0+ movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ss + movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esi
If you follow the convoluted flow of the calls in this file, wakeup_pmode_return is the first thing called from the trampoline on resume, and that loads the data segments with __KERNEL_DS. The better fix would be to set ds/es to __USER_DS there. Also since we are in the kernel, fs is fixed at __KERNEL_PERCPU, and gs is either __KERNEL_STACK_CANARY or user's gs.
--
Brian Gerst
Quoted 48 lines · Brian Gerst
Quoted 47 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +13
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ss + movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx
Quoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 1 line
arch/x86/kernel/acpi/wakeup_32.S +0
-0movl saved_context_esi, %esiIf you follow the convoluted flow of the calls in this file,
wakeup_pmode_returnis the first thing called from the trampoline on resume, and that loads the data segments with__KERNEL_DS. The better fix would be to set ds/es to__USER_DSthere. Also since we are in the kernel, fs is fixed at__KERNEL_PERCPU, and gs is either__KERNEL_STACK_CANARYor user's gs.
So why is this issue rare? Is it just that the first entry to userspace after resume is only very rarely via SYSEXIT? Or is there some other code path that usually, but not quite always, fixes up the segment registers?
--Andy
Quoted 58 lines · Andy Lutomirski
Quoted 48 lines · Brian Gerst
Quoted 47 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +13
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ss + movl saved_context_ebp, %ebp movl saved_context_ebx, %ebxQuoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 1 line
arch/x86/kernel/acpi/wakeup_32.S +0
-0movl saved_context_esi, %esiIf you follow the convoluted flow of the calls in this file,
wakeup_pmode_returnis the first thing called from the trampoline on resume, and that loads the data segments with__KERNEL_DS. The better fix would be to set ds/es to__USER_DSthere. Also since we are in the kernel, fs is fixed at__KERNEL_PERCPU, and gs is either__KERNEL_STACK_CANARYor user's gs.
Quoted 4 lines
So why is this issue rare? Is it just that the first entry to userspace after resume is only very rarely via SYSEXIT? Or is there some other code path that usually, but not quite always, fixes up the segment registers?
We spent a quite a bit of time debugging this. Any small debug code or printk causes problem to disappear. We noticed that we can reproduce this more frequently when the we run just one user space application to just do suspend/resume test, which supports yours theory.
Quoted 2 lines
--Andy
Quoted 64 lines · Srinivas Pandruvada
Quoted 58 lines · Andy Lutomirski
Quoted 48 lines · Brian Gerst
Quoted 47 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +13
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ss + movl saved_context_ebp, %ebp movl saved_context_ebx, %ebxQuoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 1 line
arch/x86/kernel/acpi/wakeup_32.S +0
-0movl saved_context_esi, %esiIf you follow the convoluted flow of the calls in this file,
wakeup_pmode_returnis the first thing called from the trampoline on resume, and that loads the data segments with__KERNEL_DS. The better fix would be to set ds/es to__USER_DSthere. Also since we are in the kernel, fs is fixed at__KERNEL_PERCPU, and gs is either__KERNEL_STACK_CANARYor user's gs.Quoted 4 lines
So why is this issue rare? Is it just that the first entry to userspace after resume is only very rarely via SYSEXIT? Or is there some other code path that usually, but not quite always, fixes up the segment registers?
Quoted 4 lines
We spent a quite a bit of time debugging this. Any small debug code or printk causes problem to disappear. We noticed that we can reproduce this more frequently when the we run just one user space application to just do suspend/resume test, which supports yours theory.
So what happens if you add a couple of CPUID (or NOP) calls? If it's timing related, not segment register related, then that might 'fix' the bug too.
Which would suggest that it might be some hardware problem, or a race with something (which could not really be on the Linux side, as we are the only thread of execution at this point)?
Thanks,
Ingo
Quoted 2 lines · Brian Gerst
If you follow the convoluted flow of the calls in this file,
...
Speaking of which. It is indeed quite bad.
For one, saved_eip is only ever set to point to ret_point:
ENTRY(saved_eip) .long 0
...
movl $ret_point, saved_eip
and it has just a single user, where an indirect jump through it is performed:
# jump to place where we left off
movl saved_eip, %eax
jmp *%eaxNo comments why it is so.
All this seems to be equivalent to trivial
# jump to place where we left off
jmp ret_pointAm I missing something?
Quoted 20 lines · Denys Vlasenko
Quoted 2 lines · Brian Gerst
If you follow the convoluted flow of the calls in this file,
...Speaking of which. It is indeed quite bad.
For one,
saved_eipis only ever set to point toret_point:
ENTRY(saved_eip).long 0
...movl $ret_point,
saved_eipand it has just a single user, where an indirect jump through it is performed:
# jump to place where we left off movl saved_eip, %eax jmp *%eax
Quoted 8 lines
No comments why it is so.
All this seems to be equivalent to trivial
# jump to place where we left off jmp ret_pointAm I missing something?
I don't think so. Its just that slowdown was not bad enough tofix it.
...patch would be welcome, and even better if you could check the issue on 64-bit kernel, too...
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 45 lines · Brian Gerst
Quoted 44 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +12
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ss
Quoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 4 lines
arch/x86/kernel/acpi/wakeup_32.S +1
-0+ movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esiIf you follow the convoluted flow of the calls in this file,
wakeup_pmode_returnis the first thing called from the trampoline on resume, and that loads the data segments with__KERNEL_DS. [...]
So if wakeup_pmode_return is really the first thing called then the whole premise of shadow descriptor corruption goes out the window: we reload all relevant segment registers.
Which leaves us with only two small channels through which the patch might make a bug go away:
- timing, as it introduces a small delay
- code/cache layout, as it slightly rearranges the code
... but both of these are in the 'grasping at straws' category of hypotheses really.
Thanks,
Ingo
Quoted 57 lines · Ingo Molnar
Quoted 45 lines · Brian Gerst
Quoted 44 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +12
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ssQuoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 4 lines
arch/x86/kernel/acpi/wakeup_32.S +1
-0+ movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esiIf you follow the convoluted flow of the calls in this file,
wakeup_pmode_returnis the first thing called from the trampoline on resume, and that loads the data segments with__KERNEL_DS. [...]
Quoted 3 lines
So if
wakeup_pmode_returnis really the first thing called then the whole premise of shadow descriptor corruption goes out the window: we reload all relevant segment registers.
True, but it still leaves the fact that we're loading __KERNEL_DS instead of __USER_DS, right? So we end up in the kernel in some context (I have no clue what context) with __KERNEL_DS loaded. It's very easy for us to inadvertently fix it: we could return to userspace by any means whatsoever except SYSEXIT, or we could even return back to some preempted kernel context.
I still think we should replace __KERNEL_DS with __USER_DS in wakeup_pmode_return and see if the problem goes away.
--Andy
Quoted 69 lines · Andy Lutomirski
Quoted 57 lines · Ingo Molnar
Quoted 45 lines · Brian Gerst
Quoted 44 lines · Ingo Molnar
Quoted 1 line · H. Peter Anvin
%es is used implicitly by string instructions.
Ok, so we are probably better off reloading ES as well early, right when we return from the firmware, just in case something does a copy before we hit the ES restore in
restore_processor_state(), which is a generic C function?Something like the patch below?
I also added FS/GS/SS reloading to make it complete. If this (or a variant thereof, it's still totally untested) works then we can remove the segment save/restore layer in __save/restore_processor_state().
Thanks,
Ingo
===========>
arch/x86/kernel/acpi/wakeup_32.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +12
-0@@ -61,6 +61,19 @@ ENTRY(wakeup_pmode_return) restore_registers: + /* + * In case the BIOS corrupted our segment descriptors, + * reload them to clear out any shadow descriptor + * state: + */ + movl $__USER_DS, %eax + movl %eax, %ds + movl %eax, %es + movl %eax, %fs + movl %eax, %gs + movl $__KERNEL_DS, %eax + movl %eax, %ssQuoted 8 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 79
Quoted 4 lines
arch/x86/kernel/acpi/wakeup_32.S +1
-0+ movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esiIf you follow the convoluted flow of the calls in this file,
wakeup_pmode_returnis the first thing called from the trampoline on resume, and that loads the data segments with__KERNEL_DS. [...]Quoted 3 lines
So if
wakeup_pmode_returnis really the first thing called then the whole premise of shadow descriptor corruption goes out the window: we reload all relevant segment registers.True, but it still leaves the fact that we're loading
__KERNEL_DSinstead of__USER_DS, right? So we end up in the kernel in some context (I have no clue what context) with__KERNEL_DSloaded. It's very easy for us to inadvertently fix it: we could return to userspace by any means whatsoever except SYSEXIT, or we could even return back to some preempted kernel context.
Quoted 2 lines
I still think we should replace
__KERNEL_DSwith__USER_DSinwakeup_pmode_returnand see if the problem goes away.
I'm pretty sure that's what the problem is. If you look at the sysexit path, it never reloads ds/es. It assumes they are still __USER_DS set at sysenter. The iret path does restore all the user segments.
--
Brian Gerst
PATCH 1/1 x86: Load __USER_DS into DS/ES after resume
Quoted 10 lines · Brian Gerst
Quoted 10 lines
Quoted 3 lines
So if
wakeup_pmode_returnis really the first thing called then the whole premise of shadow descriptor corruption goes out the window: we reload all relevant segment registers.True, but it still leaves the fact that we're loading
__KERNEL_DSinstead of__USER_DS, right? So we end up in the kernel in some context (I have no clue what context) with__KERNEL_DSloaded. It's very easy for us to inadvertently fix it: we could return to userspace by any means whatsoever except SYSEXIT, or we could even return back to some preempted kernel context.
Quoted 6 lines
Quoted 2 lines
I still think we should replace
__KERNEL_DSwith__USER_DSinwakeup_pmode_returnand see if the problem goes away.I'm pretty sure that's what the problem is. If you look at the sysexit path, it never reloads ds/es. It assumes they are still
__USER_DSset at sysenter. The iret path does restore all the user segments.
Ok, so something like the patch below, right?
Thanks,
Ingo
=====================>
arch/x86/kernel/acpi/wakeup_32.S | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
arch/x86/kernel/acpi/wakeup_32.S +4 -2
@@ -12,11 +12,13 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return: movw $__KERNEL_DS, %ax movw %ax, %ss - movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs + movw $__KERNEL_DS, %ax + movw %ax, %ds + movw %ax, %es + # reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt
Re: [PATCH] x86: Load __USER_DS into DS/ES after resume
Quoted 36 lines · Ingo Molnar
Quoted 10 lines · Brian Gerst
Quoted 10 lines
Quoted 3 lines
So if
wakeup_pmode_returnis really the first thing called then the whole premise of shadow descriptor corruption goes out the window: we reload all relevant segment registers.True, but it still leaves the fact that we're loading
__KERNEL_DSinstead of__USER_DS, right? So we end up in the kernel in some context (I have no clue what context) with__KERNEL_DSloaded. It's very easy for us to inadvertently fix it: we could return to userspace by any means whatsoever except SYSEXIT, or we could even return back to some preempted kernel context.Quoted 6 lines
Quoted 2 lines
I still think we should replace
__KERNEL_DSwith__USER_DSinwakeup_pmode_returnand see if the problem goes away.I'm pretty sure that's what the problem is. If you look at the sysexit path, it never reloads ds/es. It assumes they are still
__USER_DSset at sysenter. The iret path does restore all the user segments.Ok, so something like the patch below, right?
Thanks,
Ingo
=====================>
arch/x86/kernel/acpi/wakeup_32.S | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)arch/x86/kernel/acpi/wakeup_32.S +0
-0@@ -12,11 +12,13 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return:
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 20
Quoted 10 lines
arch/x86/kernel/acpi/wakeup_32.S +3
-2movw $__KERNEL_DS, %ax movw %ax, %ss - movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs + movw $__KERNEL_DS, %ax + movw %ax, %ds + movw %ax, %es
Umm. Are you sure? :-).
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
A message that is not in the archive (sent off-list or not yet received): CA+55aFzB9dYidEf_7Hs47FOF7WPPJnJQwj_RiwL--c5Gb1uqyw@mail.gmail.com
PATCH v2 1/1 x86: Load __USER_DS into DS/ES after resume
Quoted 11 lines · Linus Torvalds
Quoted 10 lines · Ingo Molnar
Ok, so something like the patch below, right?
movw $__KERNEL_DS, %ax
movw %ax, %ss(quoted diff) +0
-2- movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs
Quoted 7 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ movw $__KERNEL_DS, %ax + movw %ax, %ds + movw %ax, %es.. except with less cut-and-paste bugs.
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Thanks,
Ingo
=====
arch/x86/kernel/acpi/wakeup_32.S | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
arch/x86/kernel/acpi/wakeup_32.S +4 -2
@@ -12,11 +12,13 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return: movw $__KERNEL_DS, %ax movw %ax, %ss - movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs + movw $__USER_DS, %ax + movw %ax, %ds + movw %ax, %es + # reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt
Re: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
Quoted 18 lines · Ingo Molnar
Quoted 15 lines · Linus Torvalds
Quoted 13 lines · Ingo Molnar
Ok, so something like the patch below, right?
movw $__KERNEL_DS, %ax
movw %ax, %ss(quoted diff) +3
-2- movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs + movw $__KERNEL_DS, %ax + movw %ax, %ds + movw %ax, %es
Quoted 5 lines
Quoted 3 lines
.. except with less cut-and-paste bugs.
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Do we want similar patch for 64-bit, too?
arch/x86/kernel/acpi/wakeup_64.S +4 -2
@@ -18,10 +18,12 @@ ENTRY(wakeup_long64) cmpq %rdx, %rax jne bogus_64_magic - movw $__KERNEL_DS, %ax - movw %ax, %ss + movw $__USER_DS, %ax movw %ax, %ds movw %ax, %es + + movw $__KERNEL_DS, %ax + movw %ax, %ss movw %ax, %fs movw %ax, %gs movq saved_rsp, %rsp
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
Quoted 38 lines · Pavel Machek
Quoted 18 lines · Ingo Molnar
Quoted 15 lines · Linus Torvalds
Quoted 13 lines · Ingo Molnar
Ok, so something like the patch below, right?
movw $__KERNEL_DS, %ax
movw %ax, %ss(quoted diff) +3
-2- movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs + movw $__KERNEL_DS, %ax + movw %ax, %ds + movw %ax, %esQuoted 5 lines
Quoted 3 lines
.. except with less cut-and-paste bugs.
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Do we want similar patch for 64-bit, too?
Signed-off-by Pavel Machek <[email protected]>arch/x86/kernel/acpi/wakeup_64.S +0
-1@@ -18,10 +18,12 @@ ENTRY(wakeup_long64) cmpq %rdx, %rax jne bogus_64_magic - movw $__KERNEL_DS, %ax
arch/x86/kernel/acpi/wakeup_64.S ENTRY(wakeup_long64) · line 29
Quoted 10 lines
arch/x86/kernel/acpi/wakeup_64.S +4
-1- movw %ax, %ss + movw $__USER_DS, %ax movw %ax, %ds movw %ax, %es + + movw $__KERNEL_DS, %ax + movw %ax, %ss movw %ax, %fs movw %ax, %gs movq saved_rsp, %rsp
64-bit is a different animal. DS/ES are ignored in 64-bit mode, so they are always userspace-owned, and only matter for 32-bit user processes. 64-bit does currently save/restore these in x86/power/cpu.c.
--
Brian Gerst
Re: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
In reply to Ingo MolnarQuoted 41 lines · Ingo Molnar
Quoted 11 lines · Linus Torvalds
Quoted 10 lines · Ingo Molnar
Ok, so something like the patch below, right?
movw $__KERNEL_DS, %ax
movw %ax, %ssarch/x86/kernel/acpi/wakeup_32.S +0
-2- movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gsQuoted 7 lines
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 20
Quoted 3 lines
arch/x86/kernel/acpi/wakeup_32.S +3
-0+ movw $__KERNEL_DS, %ax + movw %ax, %ds + movw %ax, %es.. except with less cut-and-paste bugs.
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Thanks,
Ingo
=====
arch/x86/kernel/acpi/wakeup_32.S | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)arch/x86/kernel/acpi/wakeup_32.S +0
-2@@ -12,11 +12,13 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return: movw $__KERNEL_DS, %ax movw %ax, %ss - movw %ax, %ds - movw %ax, %es
arch/x86/kernel/acpi/wakeup_32.S ENTRY(wakeup_pmode_return) · line 24
Quoted 10 lines
arch/x86/kernel/acpi/wakeup_32.S +4
-0movw %ax, %fs movw %ax, %gs + movw $__USER_DS, %ax + movw %ax, %ds + movw %ax, %es + # reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt
We did suggest this patch to our customer and initial test results showed no crash. But they didn't merge this because they did months of testing with my earlier patch with load/save segment of ds, which will store restore _USER_DS.
I tried this patch again today on another 32 bit system and STR didn't break, so if you apply this change, during next kernel upgrade this will be picked up.
Thanks to you, Andy, Brian for helping on this issue.
- Srinivas
Re: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
In reply to Ingo MolnarQuoted 7 lines
Quoted 1 line
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Thanks,
Ingo
[of course, it worked for me before, so I only tested it also works with the patch.]
Rafael, can you apply it?
Pavel
Quoted 20 lines
arch/x86/kernel/acpi/wakeup_32.S +4
-2@@ -12,11 +12,13 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return: movw $__KERNEL_DS, %ax movw %ax, %ss - movw %ax, %ds - movw %ax, %es movw %ax, %fs movw %ax, %gs + movw $__USER_DS, %ax + movw %ax, %ds + movw %ax, %es + # reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
Quoted 8 lines · Pavel Machek
Quoted 7 lines
Quoted 1 line
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Thanks,
Ingo
Quoted 7 lines
Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>[of course, it worked for me before, so I only tested it also works with the patch.]
Rafael, can you apply it?
Yes, I can, but I thought Ingo would take it into the tip tree.
Ingo, what's your plan regarding this one?
Rafael
x86: allow using different kernel version for 32-bit, too
Rafael, would something like this be acceptable?
I think I'd like to unify hibernate_32/64.c into hibernate.c with ifdefs as a next step.
Best regards,
Pavel
commit e4d6e488069b1452fdaab06ecc812969d76ef777
Author: Pavel <[email protected]> Date: Wed Jun 17 10:35:01 2015 +0200
Port d158cbdf39ffaec9dd5299fdfdfdd2c7897a71dc to i386.
(Thanks go to czech SUSE, Vojtech Pavlik and Jiri Bohac for this one, altough they probably have no chance to understand why.)
arch/x86/Kconfig +1 -1
@@ -2074,7 +2074,7 @@ menu "Power management and ACPI options" config ARCH_HIBERNATION_HEADER def_bool y - depends on X86_64 && HIBERNATION + depends on HIBERNATION source "kernel/power/Kconfig"
arch/x86/include/asm/suspend_32.h +3 -0
@@ -24,4 +24,7 @@ struct saved_context {unsigned long return_address; } __attribute__((packed)); +extern char core_restore_code; +extern char restore_registers; +#endif /* _ASM_X86_SUSPEND_32_H */
arch/x86/power/hibernate.c new file +46 -0
46 lines not on this page · raw message
arch/x86/power/hibernate_32.c +23 -0
@@ -4,6 +4,7 @@* Distribute under GPLv2 * * Copyright (c) 2006 Rafael J. Wysocki <[email protected]> +* Copyright (c) 2015 Pavel Machek <[email protected]> */ #include <linux/gfp.h>
@@ -14,13 +15,28 @@#include <asm/pgtable.h> #include <asm/mmzone.h> #include <asm/sections.h> +#include <asm/suspend.h> /* Defined in hibernate_asm_32.S */ extern int restore_image (void ); +/* +* Address to jump to in the last phase of restore in order to get to the image +* kernel's text (this value is passed in the image header). +*/ +unsigned long restore_jump_address __visible; + +/* +* Value of the cr3 register from before the hibernation (this value is passed +* in the image header). +*/ +unsigned long restore_cr3 __visible; +/* Pointer to the temporary resume page tables */ pgd_t *resume_pg_dir; +void *relocated_restore_code __visible; +/* The following three functions are based on the analogous code in * arch/x86/mm/init_32.c */
@@ -142,6 +158,9 @@ static inline void resume_init_first_level_page_table(pgd_t *pg_dir)#endif } +#define RESTORE_MAGIC 0x2468aceUL +#include "hibernate.c" +int swsusp_arch_resume (void ) {int error;
@@ -155,6 +174,10 @@ int swsusp_arch_resume(void)if (error)return error; + error =reallocate_restore_code (); +if (error) +return error; +/* We have got enough memory and from now on we cannot recover */ restore_image ();return 0 ;
arch/x86/power/hibernate_64.c +6 -42
64 lines not on this page · raw message
arch/x86/power/hibernate_asm_32.S +44 -2
81 lines not on this page · raw message
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Ingo Molnar
Acked-byRe: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
In reply to Rafael J. WysockiQuoted 12 lines · Rafael J. Wysocki
Quoted 11 lines · Pavel Machek
Quoted 7 lines
Quoted 1 line
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Thanks,
Ingo
Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>
Quoted 8 lines
Quoted 4 lines
[of course, it worked for me before, so I only tested it also works with the patch.]
Rafael, can you apply it?
Yes, I can, but I thought Ingo would take it into the tip tree.
Ingo, what's your plan regarding this one?
Feel free to push it!
Thanks,
Ingo
Re: [PATCH v2] x86: Load __USER_DS into DS/ES after resume
Quoted 20 lines · Ingo Molnar
Quoted 17 lines · Rafael J. Wysocki
Quoted 15 lines · Pavel Machek
Quoted 7 lines
Quoted 1 line
That second KERNEL should be USER.
Yeah, doh :-/ Updated patch below.
Thanks,
Ingo
Acked-by Pavel Machek <[email protected]>Tested-by Pavel Machek <[email protected]>[of course, it worked for me before, so I only tested it also works with the patch.]
Rafael, can you apply it?
Quoted 7 lines
Quoted 3 lines
Yes, I can, but I thought Ingo would take it into the tip tree.
Ingo, what's your plan regarding this one?
Feel free to push it!
Acked-by Ingo Molnar <[email protected]>
OK, applied, thanks!
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
Quoted 126 lines · Ingo Molnar
Quoted 2 lines · Srinivas Pandruvada
Suspend to RAM process is returning to userspsace with DS set to
KERNEL_DSafter resume, this cause general protection fault. [...]But s2ram has no influence on 'returning to user-space'. So unless I'm missing something this changelog makes no sense.
Unfortunately the patch seems to be completely bogus as well:
Quoted 41 lines
[...] This is very difficult to reproduce, but this does happen on 32 bit systems. This crash is not reproduced after save/restore DS during calls to _save_processor_state/
__restore_processor_staterespectively.Similar issue was reported in the past
https://bugzilla.kernel.org/show_bug.cgi?id=61781, for which the root cause was not identified.Signed-off-by Srinivas Pandruvada <[email protected]>Reviewed-by Andi Kleen <[email protected]>--- arch/x86/include/asm/suspend_32.h | 2 +- arch/x86/power/cpu.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)arch/x86/include/asm/suspend_32.h +1
-1@@ -11,7 +11,7 @@/* image of the saved processor state */ struct saved_context { -u16 es, fs, gs, ss; +u16 ds, es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;u64 misc_enable;bool misc_enable_saved;arch/x86/power/cpu.c +1
-0@@ -79,6 +79,7 @@ static void __save_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +savesegment (ds, ctxt->ds);savesegment (es, ctxt->es);savesegment (fs, ctxt->fs);savesegment (gs, ctxt->gs);arch/x86/power/cpu.c static void notrace __restore_processor_state(struct saved_context *ctxt) · line 205
Quoted 7 lines
arch/x86/power/cpu.c +1
-0@@ -198,6 +199,7 @@ static void notrace __restore_processor_state(struct saved_context *ctxt)* segment registers */ #ifdef CONFIG_X86_32 +loadsegment (ds, ctxt->ds);loadsegment (es, ctxt->es);loadsegment (fs, ctxt->fs);loadsegment (gs, ctxt->gs);So
save_processor_state()is used by 32-bit s2ram in the following place:
arch/x86/kernel/acpi/wakeup_32.S: callsave_processor_stateOther uses are:
arch/x86/kernel/acpi/wakeup_64.S: call save_processor_state arch/x86/kernel/apm_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_32.c: save_processor_state(); arch/x86/kernel/machine_kexec_64.c: save_processor_state(); arch/x86/platform/olpc/xo1-wakeup.S: call save_processor_state kernel/power/hibernate.c: save_processor_state(); kernel/power/hibernate.c: save_processor_state();but neither of these call sites seems to matter to the bug: the 32-bit system does not use APM, kexec, is not an OLPC and does not use hibernation.
And if we look at
arch/x86/kernel/acpi/wakeup_32.Sit's a straightforward full state save/restore before ACPI (BIOS) downcalls.Furthermore, the bug report in:
https://bugzilla.kernel.org/show_bug.cgi?id=61781talks about SIGSEGVs, and claims that this regression triggers in v3.11 but does not trigger in v3.10.
1)
So the first critical question is: if the ACPI/BIOS suspend code corrupts the kernel's DS, how can we get so far as to resume fully, return to user-space, and segfault there so that it can all be reported?
So neither the explanation nor the code makes any sense in the context of the reported bugs. Can anyone else offer any plausible theory about why this patch would fix 32-bit user-space segfaults?
2)
Btw., I don't mind the idea of the patch itself per se: saving/restoring DS is prudent to avoid the BIOS stomping on our DS.
But the restoration done in this patch is bogus, it's done way too late in a C function, there's a number of places where we might use the kernel DS before it's restored, such as
restore_registers():
restore_registers:movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx movl saved_context_esi, %esi movl saved_context_edi, %edi pushl saved_context_eflagspopfl
retthis is called before
restore_processor_state().those 'saved_context_*' references are already using the kernel DS! So if it's corrupted, we'll crash there already. So your patch seems totally nonsensical.
3)
So the patch below (totally untested) does the DS restoration early on, as the first thing after we emerge from the sleep. This should be equivalent to your patch, but is more robust and much simpler: there's no need to save DS, because we know that it has to be
__KERNEL_DS.
Quoted 4 lines
Could you please test whether this solves the problem as well? Also, could you please describe how the failure triggers in your system: how many times do you have to suspend/resume to trigger the segfaults, and is there anything that makes the failures less or more likely?
It is very random. Sometimes only few hundred trys reproduce this issue. Some other times it requires thousands of trys (sometimes not reproducible at all for days)
It is very time sensitive. A small delay or some debug code in resume path prevents this to crash.
The BIOS folks created special version to check if they are corrupting any DS, but they were not able to catch any corruption. Since these are special deployed systems running critical application, need to request the tests again with your changes. May take long time.
Thanks,
Srinivas
Quoted 24 lines
Thanks,
Ingo
=================>
arch/x86/kernel/acpi/wakeup_32.S | 4 ++++ 1 file changed, 4 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +4
-0@@ -81,6 +81,10 @@ ENTRY(do_suspend_lowlevel) jmp ret_point .p2align 4,,7 ret_point: + /* In case the BIOS corrupted DS, make the kernel context minimally functional: */ + movl $__KERNEL_DS, %eax + movl %eax, %ds + call restore_registers call restore_processor_state ret
PATCH 1/1 x86/32: Add small delay after resume
Quoted 7 lines · Srinivas Pandruvada
Quoted 3 lines
Also, could you please describe how the failure triggers in your system: how
many times do you have to suspend/resume to trigger the segfaults, and is there anything that makes the failures less or more likely?It is very random. Sometimes only few hundred trys reproduce this issue. Some other times it requires thousands of trys (sometimes not reproducible at all for days) It is very time sensitive.
So the very same kernel image will produce different crash patterns depending on the time of day? That suggests heat/hardware problems.
Quoted 1 line
[...] A small delay or some debug code in resume path prevents this to crash.
Fun...
Quoted 2 lines
The BIOS folks created special version to check if they are corrupting any DS, but they were not able to catch any corruption. [...]
So is it true that we always execute wakeup_pmode_return first after we return from the BIOS?
If so then the BIOS touching DS cannot be an issue, as we re-initialize all segment selectors, which reloads the descriptors:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:
movw $__KERNEL_DS, %ax
movw %ax, %ss
movw %ax, %ds
movw %ax, %es
movw %ax, %fs
movw %ax, %gs # reload the gdt, as we need the full 32 bit address
lidt saved_idt
lldt saved_ldt
ljmp $(__KERNEL_CS), $1fQuoted 2 lines
[...] Since these are special deployed systems running critical application, need to request the tests again with your changes. May take long time.
So my second patch is clearly broken as per Brian Gerst's comments.
What I would suggest is to try a patch that adds just 100 NOPs or so - attached below. This patch will add a small delay without any side effects (other than changing the kernel image layout).
If that makes the crash go away, then I'd say the likelihood that it's hardware related increases substantially: maybe a PLL has not stabilized yet sufficiently after resume, or there's some latent heat sensitivity and the fan has not started up yet, etc.
( You can then use this simple delay generating patch in production systems as
well, to work around the problem. Maybe convince the BIOS folks to add a delay like this to their resume path before they call Linux. )
Thanks,
Ingo
=================>
arch/x86/kernel/acpi/wakeup_32.S | 6 ++++++ 1 file changed, 6 insertions(+)
arch/x86/kernel/acpi/wakeup_32.S +6 -0
@@ -10,6 +10,12 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return: + + /* Timing delay of a few dozen cycles: give the hardware some time to recover */ + .rept 100 + nop + .endr + movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds
Re: [PATCH, DEBUG] x86/32: Add small delay after resume
Quoted 49 lines · Ingo Molnar
Quoted 7 lines · Srinivas Pandruvada
Quoted 3 lines
Also, could you please describe how the failure triggers in your system: how
many times do you have to suspend/resume to trigger the segfaults, and is there anything that makes the failures less or more likely?It is very random. Sometimes only few hundred trys reproduce this issue. Some other times it requires thousands of trys (sometimes not reproducible at all for days) It is very time sensitive.
So the very same kernel image will produce different crash patterns depending on the time of day? That suggests heat/hardware problems.
Quoted 1 line
[...] A small delay or some debug code in resume path prevents this to crash.
Fun...
Quoted 2 lines
The BIOS folks created special version to check if they are corrupting any DS, but they were not able to catch any corruption. [...]
So is it true that we always execute
wakeup_pmode_returnfirst after we return from the BIOS?If so then the BIOS touching DS cannot be an issue, as we re-initialize all segment selectors, which reloads the descriptors:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds movw %ax, %es movw %ax, %fs movw %ax, %gs# reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt ljmp $(__KERNEL_CS), $1fQuoted 2 lines
[...] Since these are special deployed systems running critical application, need to request the tests again with your changes. May take long time.
So my second patch is clearly broken as per Brian Gerst's comments.
What I would suggest is to try a patch that adds just 100 NOPs or so - attached below. This patch will add a small delay without any side effects (other than changing the kernel image layout).
Quoted 8 lines
If that makes the crash go away, then I'd say the likelihood that it's hardware related increases substantially: maybe a PLL has not stabilized yet sufficiently after resume, or there's some latent heat sensitivity and the fan has not started up yet, etc.
( You can then use this simple delay generating patch in production systems as
well, to work around the problem. Maybe convince the BIOS folks to add a delay like this to their resume path before they call Linux. )
This was already experimented. They added delay in BIOS before handing over to OS, the crash still occurred.
We were thinking that BIOS SMI handler responsible for suspend/wake up corrupted the DS even after control passed over to OS. But couldn't prove it.
Thanks for your valuable debugging suggestions.
Thanks,
Srinivas
Quoted 27 lines
Thanks,
Ingo
=================>
arch/x86/kernel/acpi/wakeup_32.S | 6 ++++++ 1 file changed, 6 insertions(+)arch/x86/kernel/acpi/wakeup_32.S +6
-0@@ -10,6 +10,12 @@ ENTRY(wakeup_pmode_return) wakeup_pmode_return: + + /* Timing delay of a few dozen cycles: give the hardware some time to recover */ + .rept 100 + nop + .endr + movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds
H. Peter Anvin
reviewerRe: [PATCH, DEBUG] x86/32: Add small delay after resume
Quoted 16 lines · Srinivas Pandruvada
Quoted 16 lines
So is it true that we always execute
wakeup_pmode_returnfirst after we return from the BIOS?If so then the BIOS touching DS cannot be an issue, as we re-initialize all segment selectors, which reloads the descriptors:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds movw %ax, %es movw %ax, %fs movw %ax, %gs
Quoted 5 lines
Quoted 5 lines
# reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt ljmp $(__KERNEL_CS), $1f
Where does the GDT get initialized?
-hpa
Re: [PATCH, DEBUG] x86/32: Add small delay after resume
Quoted 17 lines · H. Peter Anvin
Quoted 16 lines · Srinivas Pandruvada
Quoted 16 lines
So is it true that we always execute
wakeup_pmode_returnfirst after we return from the BIOS?If so then the BIOS touching DS cannot be an issue, as we re-initialize all segment selectors, which reloads the descriptors:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds movw %ax, %es movw %ax, %fs movw %ax, %gs
Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
# reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt ljmp $(__KERNEL_CS), $1fWhere does the GDT get initialized?
In wakeup_start
During acpi_sleep_prepare we set a 32 bit FW waking vector which points to wakeup_start.
Thanks,
Srinivas
Quoted 8 lines
-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH, DEBUG] x86/32: Add small delay after resume
In reply to H. Peter AnvinQuoted 23 lines · H. Peter Anvin
Quoted 16 lines · Srinivas Pandruvada
Quoted 16 lines
So is it true that we always execute
wakeup_pmode_returnfirst after we return from the BIOS?If so then the BIOS touching DS cannot be an issue, as we re-initialize all segment selectors, which reloads the descriptors:
ENTRY(wakeup_pmode_return)wakeup_pmode_return:movw $__KERNEL_DS, %ax movw %ax, %ss movw %ax, %ds movw %ax, %es movw %ax, %fs movw %ax, %gsQuoted 5 lines
Quoted 5 lines
# reload the gdt, as we need the full 32 bit address lidt saved_idt lldt saved_ldt ljmp $(__KERNEL_CS), $1f
Quoted 3 lines
Where does the GDT get initialized?
-hpa
mit 84e70971e67d97bc2db18a4e76d42846272a54bd
Author: Konrad Rzeszutek Wilk <[email protected]> Date: Fri Apr 5 16:42:22 2013 -0400
x86-32, gdt: Store/load GDT for ACPI S3 or hibernation/resume path is not needed
Quoted 7 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
H. Peter Anvin
reviewerRe: [PATCH, DEBUG] x86/32: Add small delay after resume
Quoted 4 lines · Konrad Rzeszutek Wilk
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Where does the GDT get initialized?
Quoted 8 lines
Quoted 1 line
-hpa
mit
84e70971e67d97bc2db18a4e76d42846272a54bdAuthor: Konrad Rzeszutek Wilk <[email protected]> Date: Fri Apr 5 16:42:22 2013 -0400x86-32, gdt: Store/load GDT for ACPI S3 or hibernation/resume path is not needed
Store, no. LOAD?
-hpa
Re: [PATCH, DEBUG] x86/32: Add small delay after resume
Quoted 7 lines · H. Peter Anvin
Quoted 6 lines · Konrad Rzeszutek Wilk
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Where does the GDT get initialized?
-hpa
Quoted 8 lines
Quoted 6 lines
mit
84e70971e67d97bc2db18a4e76d42846272a54bdAuthor: Konrad Rzeszutek Wilk <[email protected]> Date: Fri Apr 5 16:42:22 2013 -0400x86-32, gdt: Store/load GDT for ACPI S3 or hibernation/resume path is not needed
Store, no. LOAD?
__save_processor_state:
..
/*
* We save it here, but restore it only in the hibernate case.
* For ACPI S3 resume, this is loaded via 'early_gdt_desc' in
* 64-bit
* mode in "secondary_startup_64". In 32-bit mode it is done via
* 'pmode_gdt' in wakeup_start.
*/
Quoted 4 lines
-hpa