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

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

[Rafael J. Wysocki]

Hi,

The appended patch fixes a regression and is considered as 2.6.26 material.
Everyone having a box with working suspend to RAM is gently requested to test
it and verify if it doesn't break things.

The patch applies to the current -git.

Thanks,
Rafael

---
From: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>

x86 acpi: normalize segment descriptor register on resume

Some Dell laptops enter resume with apparent garbage in the segment
descriptor registers (almost certainly the result of a botched
transition from protected to real mode.)  The only way to clean that
up is to enter protected mode ourselves and clean out the descriptor
registers.

This fixes resume on Dell XPS M1210 and Dell D620.

Reference: http://bugzilla.kernel.org/show_bug.cgi?id=10927

Signed-off-by: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
Tested-by: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
Signed-off-by: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
---
 arch/x86/kernel/acpi/realmode/wakeup.S |   38 ++++++++++++++++++++++++++++++++-
 arch/x86/kernel/acpi/realmode/wakeup.h |    5 ++++
 arch/x86/kernel/acpi/sleep.c           |   16 +++++++++++++
 drivers/acpi/sleep/main.c              |    5 +---
 4 files changed, 59 insertions(+), 5 deletions(-)

Index: linux-2.6/arch/x86/kernel/acpi/realmode/wakeup.S
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/realmode/wakeup.S
+++ linux-2.6/arch/x86/kernel/acpi/realmode/wakeup.S
@@ -5,6 +5,7 @@
 #include <asm/msr-index.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
+#include <asm/processor-flags.h>
 
 	.code16
 	.section ".header", "a"
@@ -24,6 +25,11 @@ 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_gdt:	.quad	0, 0, 0
 signature:	.long	0x51ee1111
 
 	.text
@@ -34,11 +40,34 @@ _start:
 	cli
 	cld
 
+	/* Apparently some dimwit BIOS programmers don't know how to
+	   program a PM to RM transition, and we might end up here with
+	   junk in the data segment descriptor registers.  The only way
+	   to repair that is to go into PM and fix it ourselves... */
+	movw	$16, %cx
+	lgdtl	%cs:wakeup_gdt
+	movl	%cr0, %eax
+	orb	$X86_CR0_PE, %al
+	movl	%eax, %cr0
+	jmp	1f
+1:	ljmpw	$8, $2f
+2:
+	movw	%cx, %ds
+	movw	%cx, %es
+	movw	%cx, %ss
+	movw	%cx, %fs
+	movw	%cx, %gs
+
+	andb	$~X86_CR0_PE, %al
+	movl	%eax, %cr0
+	jmp	wakeup_jmp
+3:
 	/* Set up segments */
 	movw	%cs, %ax
 	movw	%ax, %ds
 	movw	%ax, %es
 	movw	%ax, %ss
+	lidtl	wakeup_idt
 
 	movl	$wakeup_stack_end, %esp
 
@@ -98,7 +127,14 @@ bogus_real_magic:
 	jmp	1b
 
 	.data
-	.balign	4
+	.balign	8
+
+	/* This is the standard real-mode IDT */
+wakeup_idt:
+	.word	0xffff		/* limit */
+	.long	0		/* address */
+	.word	0
+
 	.globl	HEAP, heap_end
 HEAP:
 	.long	wakeup_heap
Index: linux-2.6/arch/x86/kernel/acpi/realmode/wakeup.h
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/realmode/wakeup.h
+++ linux-2.6/arch/x86/kernel/acpi/realmode/wakeup.h
@@ -24,6 +24,11 @@ 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;
+	u64 wakeup_gdt[3];
 	u32 signature;		/* To check we have correct structure */
 } __attribute__((__packed__));
 
Index: linux-2.6/arch/x86/kernel/acpi/sleep.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c
+++ linux-2.6/arch/x86/kernel/acpi/sleep.c
@@ -50,6 +50,20 @@ int acpi_save_state_mem(void)
 
 	header->video_mode = saved_video_mode;
 
+	header->wakeup_jmp_seg = acpi_wakeup_address >> 4;
+	/* GDT[0]: GDT self-pointer */
+	header->wakeup_gdt[0] =
+		(u64)(sizeof(header->wakeup_gdt) - 1) +
+		((u64)(acpi_wakeup_address +
+			((char *)&header->wakeup_gdt - (char *)acpi_realmode))
+				<< 16);
+	/* GDT[1]: real-mode-like code segment */
+	header->wakeup_gdt[1] = (0x009bULL << 40) +
+		((u64)acpi_wakeup_address << 16) + 0xffff;
+	/* GDT[2]: real-mode-like data segment */
+	header->wakeup_gdt[2] = (0x0093ULL << 40) +
+		((u64)acpi_wakeup_address << 16) + 0xffff;
+
 #ifndef CONFIG_64BIT
 	store_gdt((struct desc_ptr *)&header->pmode_gdt);
 
@@ -111,7 +125,7 @@ void __init acpi_reserve_bootmem(void)
 		return;
 	}
 
-	acpi_wakeup_address = acpi_realmode;
+	acpi_wakeup_address = virt_to_phys((void *)acpi_realmode);
 }
 
 
Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c
+++ linux-2.6/drivers/acpi/sleep/main.c
@@ -32,9 +32,8 @@ static int acpi_sleep_prepare(u32 acpi_s
 		if (!acpi_wakeup_address) {
 			return -EFAULT;
 		}
-		acpi_set_firmware_waking_vector((acpi_physical_address)
-						virt_to_phys((void *)
-							     acpi_wakeup_address));
+		acpi_set_firmware_waking_vector(
+				(acpi_physical_address)acpi_wakeup_address);
 
 	}
 	ACPI_FLUSH_CPU_CACHE();

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

[H. Peter Anvin]

Rafael J. Wysocki wrote:
> Hi,
> 
> The appended patch fixes a regression and is considered as 2.6.26 material.
> Everyone having a box with working suspend to RAM is gently requested to test
> it and verify if it doesn't break things.
> 
> The patch applies to the current -git.
> 

Given the Elan report from earlier today, I'm worried this patch might 
have the same problem.  If so, it's a relatively easy change; I'll try 
to have an updated patch later this evening.

	-hpa

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

[Ingo Molnar]

* Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org> wrote:

> Hi,
> 
> The appended patch fixes a regression and is considered as 2.6.26 
> material. Everyone having a box with working suspend to RAM is gently 
> requested to test it and verify if it doesn't break things.
> 
> The patch applies to the current -git.

it's been under testing in tip/out-of-tree for about a week:

| commit ee901dc1b9ab94a37ba2efc296fe9ba72bc75adf
| Author:     H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
| AuthorDate: Tue Jun 24 23:03:48 2008 +0200
| Commit:     Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
| CommitDate: Wed Jun 25 18:48:13 2008 +0200
|
|     x86 ACPI: normalize segment descriptor register on resume

no problems caused by it so far.

	Ingo

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

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

Ingo Molnar wrote:
> * Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> 
>> Hi,
>>
>> The appended patch fixes a regression and is considered as 2.6.26 
>> material. Everyone having a box with working suspend to RAM is gently 
>> requested to test it and verify if it doesn't break things.
>>
>> The patch applies to the current -git.
> 
> it's been under testing in tip/out-of-tree for about a week:
> 
> | commit ee901dc1b9ab94a37ba2efc296fe9ba72bc75adf
> | Author:     H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> | AuthorDate: Tue Jun 24 23:03:48 2008 +0200
> | Commit:     Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> | CommitDate: Wed Jun 25 18:48:13 2008 +0200
> |
> |     x86 ACPI: normalize segment descriptor register on resume
> 
> no problems caused by it so far.
> 

Here is the incremental patch which should stick "strictly to the 
script" of ljmp immediately after writing CR0.PE.  This should be done 
to the boot code as well; I'm waiting for confirmation from the Elan 
original reporter before submitting that patch.

I decided to make this an incremental patch to make it bisectable versus 
the other one, however, it should probably be considered the right thing.

Note: I have not tested this beyond compilation, I'm afraid.

	-hpa

[-- Attachment #2: 0002-x86-acpi-on-wakeup-ljmp-directly-after-writing-CR0.patch --]
[-- Type: text/x-patch, Size: 3589 bytes --]

From 220cb3b652abcf6a68b20c4bda4cfd426703d3c1 Mon Sep 17 00:00:00 2001
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 |   13 +++++--------
 arch/x86/kernel/acpi/realmode/wakeup.h |    6 ++----
 arch/x86/kernel/acpi/sleep.c           |    4 +++-
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/acpi/realmode/wakeup.S b/arch/x86/kernel/acpi/realmode/wakeup.S
index f2c16d2..7e54e40 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.S
+++ b/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
@@ -134,7 +131,7 @@ wakeup_idt:
 	.word	0xffff		/* limit */
 	.long	0		/* address */
 	.word	0
-	
+
 	.globl	HEAP, heap_end
 HEAP:
 	.long	wakeup_heap
diff --git a/arch/x86/kernel/acpi/realmode/wakeup.h b/arch/x86/kernel/acpi/realmode/wakeup.h
index 69d38d0..0dcdbc7 100644
--- a/arch/x86/kernel/acpi/realmode/wakeup.h
+++ b/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__));
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index c8c7e7a..08698ca 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/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]  =
 		(sizeof header->wakeup_gdt - 1) +
-- 
1.5.4.1

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

[H. Peter Anvin]

Ingo Molnar wrote:
> * Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> 
>> Hi,
>>
>> The appended patch fixes a regression and is considered as 2.6.26 
>> material. Everyone having a box with working suspend to RAM is gently 
>> requested to test it and verify if it doesn't break things.
>>
>> The patch applies to the current -git.
> 
> it's been under testing in tip/out-of-tree for about a week:
> 
> | commit ee901dc1b9ab94a37ba2efc296fe9ba72bc75adf
> | Author:     H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> | AuthorDate: Tue Jun 24 23:03:48 2008 +0200
> | Commit:     Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> | CommitDate: Wed Jun 25 18:48:13 2008 +0200
> |
> |     x86 ACPI: normalize segment descriptor register on resume
> 
> no problems caused by it so far.
> 

It still seems incredibly risky to push this for 2.6.26, especially 
given the Elan revelation.  I think it needs to be tested on the 2.6.27 
track, and then possibly be pushed back via the 2.6.26-stable route.

	-hpa

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

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> Hi,
> 
> The appended patch fixes a regression and is considered as 2.6.26 
> material. Everyone having a box with working suspend to RAM is gently 
> requested to test it and verify if it doesn't break things.
> 
> The patch applies to the current -git.

The fix is _really_ tempting, but i think it's 2.6.26.1 material at the 
earliest. I just counted about 8 red flag items in that commit:

  - "assembly code"
  - "fresh change"
  - "suspend/resume"
  - "real-mode code"
  - "ACPI"
  - "SMM"
  - "CPU erratas"
  - "boot code"

I'd say it's probably 90% fine, but it's just too much risk at this 
stage i think. The regression was only found 2 weeks ago, and the commit 
that broke it was upstream for 2 months (and was under testing for about 
4 months).

[ We have to try to shorten the test cycle for such problems. Hopefully 
  in v2.6.27 we'll have CONFIG_PM_TEST_SUSPEND=y :-) ]

	Ingo

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

[Andrew Morton]

On Tue, 1 Jul 2008 09:20:24 +0200 Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org> wrote:

> * Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> 
> > Hi,
> > 
> > The appended patch fixes a regression and is considered as 2.6.26 
> > material. Everyone having a box with working suspend to RAM is gently 
> > requested to test it and verify if it doesn't break things.
> > 
> > The patch applies to the current -git.
> 
> The fix is _really_ tempting, but i think it's 2.6.26.1 material at the 
> earliest. I just counted about 8 red flag items in that commit:
> 
>   - "assembly code"
>   - "fresh change"
>   - "suspend/resume"
>   - "real-mode code"
>   - "ACPI"
>   - "SMM"
>   - "CPU erratas"
>   - "boot code"
> 
> I'd say it's probably 90% fine, but it's just too much risk at this 
> stage i think. The regression was only found 2 weeks ago, and the commit 
> that broke it was upstream for 2 months (and was under testing for about 
> 4 months).

Merge it into 2.6.27-rc1 and add Cc: <stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> to the changelog with
a note "needed in 2.6.26.x after a couple of weeks testing in mainline" or
something like that.

I expect 2.6.25.x will be maintained for a while yet too...

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

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 1 Jul 2008 09:20:24 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> 
> > * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > 
> > > Hi,
> > > 
> > > The appended patch fixes a regression and is considered as 2.6.26 
> > > material. Everyone having a box with working suspend to RAM is gently 
> > > requested to test it and verify if it doesn't break things.
> > > 
> > > The patch applies to the current -git.
> > 
> > The fix is _really_ tempting, but i think it's 2.6.26.1 material at the 
> > earliest. I just counted about 8 red flag items in that commit:
> > 
> >   - "assembly code"
> >   - "fresh change"
> >   - "suspend/resume"
> >   - "real-mode code"
> >   - "ACPI"
> >   - "SMM"
> >   - "CPU erratas"
> >   - "boot code"
> > 
> > I'd say it's probably 90% fine, but it's just too much risk at this 
> > stage i think. The regression was only found 2 weeks ago, and the commit 
> > that broke it was upstream for 2 months (and was under testing for about 
> > 4 months).
> 
> Merge it into 2.6.27-rc1 and add Cc: <stable@kernel.org> to the changelog with
> a note "needed in 2.6.26.x after a couple of weeks testing in mainline" or
> something like that.

ok.

> I expect 2.6.25.x will be maintained for a while yet too...

2.6.25.x is not affected by the suspend+resume aspect of this problem.

	Ingo

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

[Pavel Machek]

Hi!

>>> The appended patch fixes a regression and is considered as 2.6.26 
>>> material. Everyone having a box with working suspend to RAM is gently 
>>> requested to test it and verify if it doesn't break things.
>>>
>>> The patch applies to the current -git.
>>
>> it's been under testing in tip/out-of-tree for about a week:
>>
>> | commit ee901dc1b9ab94a37ba2efc296fe9ba72bc75adf
>> | Author:     H. Peter Anvin <hpa@zytor.com>
>> | AuthorDate: Tue Jun 24 23:03:48 2008 +0200
>> | Commit:     Ingo Molnar <mingo@elte.hu>
>> | CommitDate: Wed Jun 25 18:48:13 2008 +0200
>> |
>> |     x86 ACPI: normalize segment descriptor register on resume
>>
>> no problems caused by it so far.
>>
>
> Here is the incremental patch which should stick "strictly to the script" 
> of ljmp immediately after writing CR0.PE.  This should be done to the boot 
> code as well; I'm waiting for confirmation from the Elan original reporter 
> before submitting that patch.
>
> I decided to make this an incremental patch to make it bisectable versus 
> the other one, however, it should probably be considered the right thing.
>
> Note: I have not tested this beyond compilation, I'm afraid.

I had to apply it by hand, then I tested it along with Rafael's patch
and it does not seem to break anything here. So feel free to add my
ACK to both.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Andi Kleen]

> 
> It still seems incredibly risky to push this for 2.6.26, especially
> given the Elan revelation.

Do Elans even support S3?

>  I think it needs to be tested on the 2.6.27
> track, and then possibly be pushed back via the 2.6.26-stable route.

I'm just not sure how many suspend/resume cycles people really do
on a early (pre -rc) mainline kernel (or in linux-next for that
matter). You usually have to install on a laptop and actually
use it.

Since this code is only executed on resume some directed testing
would be better. That is what Rafael asked for in this mail.

I think it would be ok for .26 if we can get confirmation it works
on a few systems with S3 suspend/resume.

-Andi

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

[H. Peter Anvin]

Andi Kleen wrote:
>> It still seems incredibly risky to push this for 2.6.26, especially
>> given the Elan revelation.
> 
> Do Elans even support S3?

I don't know if they do, but I don't know offhand the extent of machines 
that may have that problem, especially since Intel now document it as 
"failures are readily seen".

>>  I think it needs to be tested on the 2.6.27
>> track, and then possibly be pushed back via the 2.6.26-stable route.
> 
> I'm just not sure how many suspend/resume cycles people really do
> on a early (pre -rc) mainline kernel (or in linux-next for that
> matter). You usually have to install on a laptop and actually
> use it.
 >
> Since this code is only executed on resume some directed testing
> would be better. That is what Rafael asked for in this mail.

The issue is mostly if it breaks some obscure system.  I have put it on 
my laptop, Ingo has it on this test system with a suspend-testing cycle, 
and so on, but the number of systems exposed is going to be small.

> I think it would be ok for .26 if we can get confirmation it works
> on a few systems with S3 suspend/resume.

That we already know it does, but it took a long time even until the 
regression came to light.

	-hpa

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

[Andi Kleen]

H. Peter Anvin wrote:
> Andi Kleen wrote:
>>> It still seems incredibly risky to push this for 2.6.26, especially
>>> given the Elan revelation.
>>
>> Do Elans even support S3?
> 
> I don't know if they do, but I don't know offhand the extent of machines
> that may have that problem, especially since Intel now document it as
> "failures are readily seen".

What document is that exactly?


> 
>>>  I think it needs to be tested on the 2.6.27
>>> track, and then possibly be pushed back via the 2.6.26-stable route.
>>
>> I'm just not sure how many suspend/resume cycles people really do
>> on a early (pre -rc) mainline kernel (or in linux-next for that
>> matter). You usually have to install on a laptop and actually
>> use it.
>>
>> Since this code is only executed on resume some directed testing
>> would be better. That is what Rafael asked for in this mail.
> 
> The issue is mostly if it breaks some obscure system.  I have put it on
> my laptop, Ingo has it on this test system with a suspend-testing cycle,
> and so on, but the number of systems exposed is going to be small.

Right now it looks like a significant number of Dell laptops
are affected by this regression. That's a serious issue.

-Andi

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

[H. Peter Anvin]

Andi Kleen wrote:
> H. Peter Anvin wrote:
>> Andi Kleen wrote:
>>>> It still seems incredibly risky to push this for 2.6.26, especially
>>>> given the Elan revelation.
>>> Do Elans even support S3?
>> I don't know if they do, but I don't know offhand the extent of machines
>> that may have that problem, especially since Intel now document it as
>> "failures are readily seen".
> 
> What document is that exactly?
> 

Per Jeremy's recent email, volume 3a, section 8.9.1:

>    Random failures can occur if other instructions exist between steps
>    3 and 4 above.  Failures will be readily seen in some situations,
>    such as when instructions that reference memory are inserted between
>    steps 3 and 4 while in system management mode. 

	-hpa

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

[Rafael J. Wysocki]

On Tuesday, 1 of July 2008, H. Peter Anvin wrote:
> Ingo Molnar wrote:
> > * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > 
> >> Hi,
> >>
> >> The appended patch fixes a regression and is considered as 2.6.26 
> >> material. Everyone having a box with working suspend to RAM is gently 
> >> requested to test it and verify if it doesn't break things.
> >>
> >> The patch applies to the current -git.
> > 
> > it's been under testing in tip/out-of-tree for about a week:
> > 
> > | commit ee901dc1b9ab94a37ba2efc296fe9ba72bc75adf
> > | Author:     H. Peter Anvin <hpa@zytor.com>
> > | AuthorDate: Tue Jun 24 23:03:48 2008 +0200
> > | Commit:     Ingo Molnar <mingo@elte.hu>
> > | CommitDate: Wed Jun 25 18:48:13 2008 +0200
> > |
> > |     x86 ACPI: normalize segment descriptor register on resume
> > 
> > no problems caused by it so far.
> > 
> 
> Here is the incremental patch which should stick "strictly to the 
> script" of ljmp immediately after writing CR0.PE.  This should be done 
> to the boot code as well; I'm waiting for confirmation from the Elan 
> original reporter before submitting that patch.
> 
> I decided to make this an incremental patch to make it bisectable versus 
> the other one, however, it should probably be considered the right thing.
> 
> Note: I have not tested this beyond compilation, I'm afraid.

I have tested it on the nx6325 (64-bit) on top of the today's linux-next.  It
works fine for me.

Thanks,
Rafael

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

[Andi Kleen]

> I have tested it on the nx6325 (64-bit) on top of the today's linux-next.  It
> works fine for me.

Could a few more people with different laptops please test too?

-Andi

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

[Rafael J. Wysocki]

On Tuesday, 1 of July 2008, Andi Kleen wrote:
> 
> > I have tested it on the nx6325 (64-bit) on top of the today's linux-next.  It
> > works fine for me.
> 
> Could a few more people with different laptops please test too?

Well, I think we can put it into the 2.6.27 queue in the hope it gets more
testing in linux-next over time.

Thanks,
Rafael

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

[Andi Kleen]

Rafael J. Wysocki wrote:
> On Tuesday, 1 of July 2008, Andi Kleen wrote:
>>> I have tested it on the nx6325 (64-bit) on top of the today's linux-next.  It
>>> works fine for me.
>> Could a few more people with different laptops please test too?
> 
> Well, I think we can put it into the 2.6.27 queue in the hope it gets more
> testing in linux-next over time.

I'm just not sure how many people really put linux-next on their laptops.
Better would be some directed testing.

We'll see if that kernel-testers list actually does something or is just a placebo.

-Andi

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

[Andy Lutomirski]

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.

--Andy

Andi Kleen wrote:
>> I have tested it on the nx6325 (64-bit) on top of the today's linux-next.  It
>> works fine for me.
> 
> Could a few more people with different laptops please test too?
> 
> -Andi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Andi Kleen]

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.

With "this" do you mean the patch being applied or not applied?

As in does it fail with the patch applied?

You can confirm that by only applying the patch and testing.

-Andi

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

[Andy Lutomirski]

Andi Kleen wrote:
> 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.
> 
> With "this" do you mean the patch being applied or not applied?
> 
> As in does it fail with the patch applied?
> 
> You can confirm that by only applying the patch and testing.

The tree as of 4b4f2780 (the title patch of this thread) fails to 
resume.  Its parent works fine.  Which makes this patch the culprit.  (I 
just tried both.)

--Andy

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

[Rafael J. Wysocki]

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?

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) +

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

[Andy Lutomirski]

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?

More correctly:

If I suspend by typing pm-suspend or echo mem >/sys/power/state, then it 
resumes just fine.  If I log in to Gnome and push the suspend button, 
then it does not resume.  This seems to be the case with or without your 
patch.

-rc8 and -rc9 with the original patch 4b4f7280 resume fine no matter how 
I suspend.

--Andy

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

[Rafael J. Wysocki]

On Saturday, 12 of July 2008, Andy Lutomirski wrote:
> 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.

That's very strange.

Well, is suspend/resume 100% reliable without commit
4b4f7280d7fd1feeff134c2cf2db32fd583b6c29 ?

Also, is the system 64-bit or 32-bit and do you use any user-space quirks to
bring the graphics to life during resume?

And what does happen if you suspend it from the console (no X)?

Rafael

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

[Rafael J. Wysocki]

On Saturday, 12 of July 2008, Andy Lutomirski wrote:
> 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?
> 
> More correctly:
> 
> If I suspend by typing pm-suspend or echo mem >/sys/power/state, then it 
> resumes just fine.  If I log in to Gnome and push the suspend button, 
> then it does not resume.  This seems to be the case with or without your 
> patch.

Is there an Intel graphics in your box?

> -rc8 and -rc9 with the original patch 4b4f7280 resume fine no matter how 
> I suspend.

That's _really_ strange.

In fact I have only one explanation, which is that the Gnome suspend button
causes some user-space quirks to be applied, which are harmful and break the
resume.  Also, without commit 4b4f7280 those quirks might have not been really
executed.  Peter, does it sound reasonable?

Thanks,
Rafael

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

[H. Peter Anvin]

It seems like a reasonable guess at least.  I won't have a chance to dig into the details until late this evening or tomorrow, depending on when I get home tonight.

-- 
Sent from my mobile phone (pardon any lack of formatting)


-----Original Message-----
From: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
Sent: Saturday, July 12, 2008 13:53
To: Andy Lutomirski <luto-YSGFQ8SKJZVDPfheJLI6IQ@public.gmane.org>; H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
Cc: Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org>; Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>; kernel-testers-u79uwXL29TY76Z2rM5mHXA-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org; ACPI Devel Maling List <linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; pm list <linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>; Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Subject: Re: [RFT] x86 acpi: normalize segment descriptor register on resume

On Saturday, 12 of July 2008, Andy Lutomirski wrote:
> 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?
> 
> More correctly:
> 
> If I suspend by typing pm-suspend or echo mem >/sys/power/state, then it 
> resumes just fine.  If I log in to Gnome and push the suspend button, 
> then it does not resume.  This seems to be the case with or without your 
> patch.

Is there an Intel graphics in your box?

> -rc8 and -rc9 with the original patch 4b4f7280 resume fine no matter how 
> I suspend.

That's _really_ strange.

In fact I have only one explanation, which is that the Gnome suspend button
causes some user-space quirks to be applied, which are harmful and break the
resume.  Also, without commit 4b4f7280 those quirks might have not been really
executed.  Peter, does it sound reasonable?

Thanks,
Rafael