Skip to contentSkip to conversation

x86 acpi: on wakeup, ljmp directly after writing CR0.PE

PATCH In review Rafael J. Wysocki 2 participants 5 messages Oct 26, 2008

Subsystems (3)
x86 ACPI Suspend to RAM
lore
Share & tools
lore

Rafael J. Wysocki

author
#0 lore raw

PATCH 1/1 x86 acpi: on wakeup, ljmp directly after writing CR0.PE

Ingo, Peter,

I have this patch in my queue, is it necessary or should I drop it?

Rafael

---
From: H. Peter Anvin <[email protected]>
Subject: 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 <[email protected]>
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 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(-)
arch/x86/kernel/acpi/realmode/wakeup.S +4 -7
index 3355973..7e54e40 100644
@@ -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
arch/x86/kernel/acpi/realmode/wakeup.h +2 -4
index 69d38d0..0dcdbc7 100644
@@ -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__));
arch/x86/kernel/acpi/sleep.c +3 -1
index 806b4e9..f8a12a8 100644
@@ -36,6 +36,7 @@ static char temp_stack[4096];
 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, "

@@ -45,6 +46,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;

@@ -52,7 +54,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;
 
 	/*
 	 * Set up the wakeup GDT.  We set these up as Big Real Mode,
Signature · 1 line
1.5.6
reviewer
#1 lore raw
Quoted 4 lines · Rafael J. Wysocki

Ingo, Peter,

I have this patch in my queue, is it necessary or should I drop it?

It's unclear if it's necessary, but I think it's more compliant and we should do it.

-hpa

reviewer
#2 lore raw
In reply to Rafael J. Wysocki
Quoted 4 lines · Rafael J. Wysocki

Ingo, Peter,

I have this patch in my queue, is it necessary or should I drop it?

It's unclear if it's necessary, but I think it's more compliant and we should do it.

-hpa

author
#3 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Rafael J. Wysocki

Ingo, Peter,

I have this patch in my queue, is it necessary or should I drop it?

It's unclear if it's necessary, but I think it's more compliant and we should do it.

OK, then.

Ingo, please queue. :-)

Thanks,
Rafael

author
#4 lore raw
In reply to H. Peter Anvin
Quoted 8 lines · H. Peter Anvin
Quoted 4 lines · Rafael J. Wysocki

Ingo, Peter,

I have this patch in my queue, is it necessary or should I drop it?

It's unclear if it's necessary, but I think it's more compliant and we should do it.

OK, then.

Ingo, please queue. :-)

Thanks,
Rafael