Re: [RFT] x86 acpi: normalize segment descriptor register on resume

Andy Lutomirski
2008-07-12T20:31:18Z

Rafael J. Wysocki wrote:
> On Saturday, 12 of July 2008, Andy Lutomirski wrote:
>> My Lenovo X61s fails to resume if I suspend it from within X, on both
>> 2.6.26-rc9 and recent wireless-testing. 2.6.26-rc8 is fine, as is
>> wireless-testing with 4b4f7280 reverted. My in-progress bisect between
>> -rc8 and -rc9 is also consistent with this being the problem.
>>
>> The symptom is that, when I push the power button to resume, the hard
>> drive light turns on, the fan turns on, then the hard drive light turns
>> off, the sleep light stays on, and the fan keeps running. Sometimes the
>> battery light will blink off very briefly (1/4 sec, maybe) every few
>> seconds. The system is locked hard at this point.
>>
>> I'm using Ubuntu Hardy userspace.
>
> Well, that's bad.
>
> There is the bugzilla entry at http://bugzilla.kernel.org/show_bug.cgi?id=11064
> for this bug and you've just confirmed my suspicion that this particular
> commit is to blame.
>
> Can you please see if the appended patch changes anything?

I suspended and resumed OK. Then I rebooted and tried again, and it failed.

--Andy

>
> Thanks,
> Rafael
>
> ---
> From: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> Date: Mon, 30 Jun 2008 23:48:35 -0700
> Subject: [PATCH] x86 acpi: on wakeup, ljmp directly after writing CR0.PE
>
> Impact: possible resume failures on AMD Elan, others?
>
> Intel documents that writing cr0 should be immediately followed by a
> ljmp, and that "failures are readily seen" if the processor enters SMM
> at this point. We believe this has been observed on the AMD Elan, so
> stick strictly to the script and do an ljmp immediately after a change
> to CR0.PE in all circumstances.
>
> Signed-off-by: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> ---
> arch/x86/kernel/acpi/realmode/wakeup.S | 11 ++++-------
> arch/x86/kernel/acpi/realmode/wakeup.h | 6 ++----
> arch/x86/kernel/acpi/sleep.c | 4 +++-
> 3 files changed, 9 insertions(+), 12 deletions(-)
>
> Index: linux-next/arch/x86/kernel/acpi/realmode/wakeup.S
> ===================================================================
> --- linux-next.orig/arch/x86/kernel/acpi/realmode/wakeup.S
> +++ linux-next/arch/x86/kernel/acpi/realmode/wakeup.S
> @@ -25,10 +25,8 @@ pmode_gdt: .quad 0
> realmode_flags: .long 0
> real_magic: .long 0
> trampoline_segment: .word 0
> -_pad1: .byte 0
> -wakeup_jmp: .byte 0xea /* ljmpw */
> -wakeup_jmp_off: .word 3f
> -wakeup_jmp_seg: .word 0
> +wakeup_seg_ptr: .word 3f-2 /* the segment in the ljmpw */
> +_pad: .long 0
> wakeup_gdt: .quad 0, 0, 0
> signature: .long 0x51ee1111
>
> @@ -49,8 +47,7 @@ _start:
> movl %cr0, %eax
> orb $X86_CR0_PE, %al
> movl %eax, %cr0
> - jmp 1f
> -1: ljmpw $8, $2f
> + ljmpw $8, $2f
> 2:
> movw %cx, %ds
> movw %cx, %es
> @@ -60,7 +57,7 @@ _start:
>
> andb $~X86_CR0_PE, %al
> movl %eax, %cr0
> - jmp wakeup_jmp
> + ljmpw $0, $3f
> 3:
> /* Set up segments */
> movw %cs, %ax
> Index: linux-next/arch/x86/kernel/acpi/realmode/wakeup.h
> ===================================================================
> --- linux-next.orig/arch/x86/kernel/acpi/realmode/wakeup.h
> +++ linux-next/arch/x86/kernel/acpi/realmode/wakeup.h
> @@ -24,10 +24,8 @@ struct wakeup_header {
> u32 realmode_flags;
> u32 real_magic;
> u16 trampoline_segment; /* segment with trampoline code, 64-bit only */
> - u8 _pad1;
> - u8 wakeup_jmp;
> - u16 wakeup_jmp_off;
> - u16 wakeup_jmp_seg;
> + u16 wakeup_seg_ptr;
> + u32 _pad;
> u64 wakeup_gdt[3];
> u32 signature; /* To check we have correct structure */
> } __attribute__((__packed__));
> Index: linux-next/arch/x86/kernel/acpi/sleep.c
> ===================================================================
> --- linux-next.orig/arch/x86/kernel/acpi/sleep.c
> +++ linux-next/arch/x86/kernel/acpi/sleep.c
> @@ -34,6 +34,7 @@ static char temp_stack[10240];
> int acpi_save_state_mem(void)
> {
> struct wakeup_header *header;
> + u16 *wakeup_seg;
>
> if (!acpi_realmode) {
> printk(KERN_ERR "Could not allocate memory during boot, "
> @@ -43,6 +44,7 @@ int acpi_save_state_mem(void)
> memcpy((void *)acpi_realmode, &wakeup_code_start, WAKEUP_SIZE);
>
> header = (struct wakeup_header *)(acpi_realmode + HEADER_OFFSET);
> + wakeup_seg = (u16 *)(acpi_realmode + header->wakeup_seg_ptr);
> if (header->signature != 0x51ee1111) {
> printk(KERN_ERR "wakeup header does not match\n");
> return -EINVAL;
> @@ -50,7 +52,7 @@ int acpi_save_state_mem(void)
>
> header->video_mode = saved_video_mode;
>
> - header->wakeup_jmp_seg = acpi_wakeup_address >> 4;
> + *wakeup_seg = acpi_wakeup_address >> 4;
> /* GDT[0]: GDT self-pointer */
> header->wakeup_gdt[0] =
> (u64)(sizeof(header->wakeup_gdt) - 1) +