Re: [PATCH] x86: General protection fault after STR (32 bit systems only)

H. Peter Anvin
2015-06-12T08:17:48Z

%es is used implicitly by string instructions.

On June 12, 2015 12:50:13 AM PDT, Ingo Molnar <mingo@kernel.org> wrote:
>
>* Andy Lutomirski <luto@amacapital.net> wrote:
>
>> > --- a/arch/x86/kernel/acpi/wakeup_32.S
>> > +++ b/arch/x86/kernel/acpi/wakeup_32.S
>> > @@ -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.
>
>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
> wbinvd
>
>which 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

--
Sent from my mobile phone. Please pardon brevity and lack of formatting.