RE: [patch] Fix GDT re-load on ACPI resume

RE: [patch] Fix GDT re-load on ACPI resume

[Li, Shaohua]

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

>The ACPI resume code currently uses a real-mode 16-bit lgdt instruction
to
>reload the GDT.  This only restores the lower 24 bits of the GDT base
>address.  In recent kernels, the GDT seems to have moved out of the
lower
>16 megs, thereby causing the ACPI resume to fail -- an invalid GDT was
>being loaded.
>
>This simple patch adds the 0x66 prefix to lgdt, which forces it to load
>all 32 bits of the GDT base address, thereby removing any restrictions
on
>where the GDT can be placed in memory.  This makes ACPI resume work for
me
>on a Thinkpad T40 laptop.
>
>-- kolya
>
>--- linux-2.6.9/arch/i386/kernel/acpi/wakeup.S	2004/11/15 09:00:34
1.1
>+++ linux-2.6.9/arch/i386/kernel/acpi/wakeup.S	2004/11/15 20:33:27
>@@ -67,6 +67,8 @@
> 	movw	$0x0e00 + 'i', %fs:(0x12)
>
> 	# need a gdt
>+	.byte	0x66			# force 32-bit operands in case
>+					# the GDT is past 16 megabytes
> 	lgdt	real_save_gdt - wakeup_code
>
> 	movl	real_save_cr0 - wakeup_code, %eax
There is a patch from hiroit@mcn.ne.jp to fix the GDT issue. You can try
it.
Please cc 'acpi-devel@lists.sourceforge.net' for suspend/resume issue.

Thanks,
Shaohua

[-- Attachment #2: wakeup_gdt2.patch --]
[-- Type: application/octet-stream, Size: 2214 bytes --]

diff -Nru a/arch/i386/kernel/acpi/sleep.c b/arch/i386/kernel/acpi/sleep.c
--- a/arch/i386/kernel/acpi/sleep.c	2004-10-15 11:29:15.000000000 +0900
+++ b/arch/i386/kernel/acpi/sleep.c	2004-10-19 00:59:32.000000000 +0900
@@ -17,7 +17,7 @@
 
 extern void zap_low_mappings(void);
 
-extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long));
+extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long,unsigned long));
 
 static void init_low_mapping(pgd_t *pgd, int pgd_limit)
 {
@@ -41,7 +41,8 @@
 		return 1;
 	init_low_mapping(swapper_pg_dir, USER_PTRS_PER_PGD);
 	memcpy((void *) acpi_wakeup_address, &wakeup_start, &wakeup_end - &wakeup_start);
-	acpi_copy_wakeup_routine(acpi_wakeup_address);
+	acpi_copy_wakeup_routine(acpi_wakeup_address,
+				 virt_to_phys((void *)acpi_wakeup_address));
 
 	return 0;
 }
diff -Nru a/arch/i386/kernel/acpi/wakeup.S b/arch/i386/kernel/acpi/wakeup.S
--- a/arch/i386/kernel/acpi/wakeup.S	2004-08-14 14:36:31.000000000 +0900
+++ b/arch/i386/kernel/acpi/wakeup.S	2004-10-19 05:24:42.000000000 +0900
@@ -89,6 +89,7 @@
 real_magic:	.long 0
 video_mode:	.long 0
 video_flags:	.long 0
+real_gdt_table: .fill GDT_ENTRIES, 8, 0
 
 bogus_real_magic:
 	movw	$0x0e00 + 'B', %fs:(0x12)
@@ -213,6 +214,7 @@
 #
 # Parameters:
 # %eax:	place to copy wakeup routine to
+# %edx: the second argument (physical address)
 #
 # Returned address is location of code in low memory (past data and stack)
 #
@@ -223,6 +225,9 @@
 	sldt	saved_ldt
 	str	saved_tss
 
+	# save wakeup_start physical address in ecx
+	movl	%edx, %ecx
+
 	movl    %cr3, %edx
 	movl    %edx, real_save_cr3 - wakeup_start (%eax)
 	movl    %cr4, %edx
@@ -231,6 +236,16 @@
 	movl	%edx, real_save_cr0 - wakeup_start (%eax)
 	sgdt    real_save_gdt - wakeup_start (%eax)
 
+	# gdt body must be addressable from real mode by
+	# copying it to the lower mem
+	lea     real_gdt_table - wakeup_start (%ecx), %ecx
+	movl    %ecx, real_save_gdt + 2 - wakeup_start (%eax)
+	xor     %ecx, %ecx
+	movw    saved_gdt, %cx
+	movl    saved_gdt + 2, %esi
+	lea     real_gdt_table - wakeup_start (%eax), %edi
+	rep movsb
+
 	movl	saved_videomode, %edx
 	movl	%edx, video_mode - wakeup_start (%eax)
 	movl	acpi_video_flags, %edx

RE: [patch] Fix GDT re-load on ACPI resume

[Maciej W. Rozycki]

On Tue, 16 Nov 2004, Li, Shaohua wrote:

> There is a patch from hiroit@mcn.ne.jp to fix the GDT issue. You can try
> it.
> Please cc 'acpi-devel@lists.sourceforge.net' for suspend/resume issue.

 What is the "gdt body must be addressable from real mode" requirement
about?  GDT is addressed by the CPU using a linear address as obtained
from GDTR (bypassing segmentation, for obvious reasons) and is accessible
regardless of its placement within the 32-bit linear address space in all
CPU modes.  As its a linear address it only undergoes translation at the
page level, if enabled.  The same applies to IDT.

  Maciej

Re: [patch] Fix GDT re-load on ACPI resume

[Hiroshi Itoh]

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

Hi, Maciej-san

> What is the "gdt body must be addressable from real mode" requirement
>about?  GDT is addressed by the CPU using a linear address as obtained
>from GDTR (bypassing segmentation, for obvious reasons) and is accessible
>regardless of its placement within the 32-bit linear address space in all
>CPU modes.  As its a linear address it only undergoes translation at the
>page level, if enabled.  The same applies to IDT.

I believe this patch is required because the original gdt is not addressable
via the low mapping page table (set by acpi_save_state_mem and used for wakeup
code), not the GDTR's linear address size reason.

cc:Shaohua and Len

wakeup_gdt2.patch should be slightestly modified to wakeup_gdt_2.6.10.patch
because %edx is conflict with msr's patch in 2.6.10-rc2 or later.

thanks
-Hiro

[-- Attachment #2: wakeup_gdt_2.6.10.patch --]
[-- Type: application/octet-stream, Size: 2216 bytes --]

--- a/arch/i386/kernel/acpi/sleep.c	2004-11-04 04:54:41.000000000 +0900
+++ b/arch/i386/kernel/acpi/sleep.c	2004-11-04 05:20:34.864501960 +0900
@@ -17,7 +17,7 @@
 
 extern void zap_low_mappings(void);
 
-extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long));
+extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long,unsigned long));
 
 static void init_low_mapping(pgd_t *pgd, int pgd_limit)
 {
@@ -41,7 +41,8 @@
 		return 1;
 	init_low_mapping(swapper_pg_dir, USER_PTRS_PER_PGD);
 	memcpy((void *) acpi_wakeup_address, &wakeup_start, &wakeup_end - &wakeup_start);
-	acpi_copy_wakeup_routine(acpi_wakeup_address);
+	acpi_copy_wakeup_routine(acpi_wakeup_address,
+				 virt_to_phys((void *)acpi_wakeup_address));
 
 	return 0;
 }
--- a/arch/i386/kernel/acpi/wakeup.S	2004-11-04 04:54:41.000000000 +0900
+++ b/arch/i386/kernel/acpi/wakeup.S	2004-11-04 05:23:46.490370400 +0900
@@ -100,6 +100,7 @@
 real_efer_save_restore:	.long 0
 real_save_efer_edx: 	.long 0
 real_save_efer_eax: 	.long 0
+real_gdt_table: .fill GDT_ENTRIES, 8, 0
 
 bogus_real_magic:
 	movw	$0x0e00 + 'B', %fs:(0x12)
@@ -224,6 +225,7 @@
 #
 # Parameters:
 # %eax:	place to copy wakeup routine to
+# %edx: the second argument (physical address)
 #
 # Returned address is location of code in low memory (past data and stack)
 #
@@ -234,6 +236,9 @@
 	sldt	saved_ldt
 	str	saved_tss
 
+	# save wakeup_start physical address in edx
+	pushl   %edx
+
 	movl	nx_enabled, %edx
 	movl	%edx, real_efer_save_restore - wakeup_start (%eax)
 	testl	$1, real_efer_save_restore - wakeup_start (%eax)
@@ -256,6 +261,17 @@
 	movl	%edx, real_save_cr0 - wakeup_start (%eax)
 	sgdt    real_save_gdt - wakeup_start (%eax)
 
+	# gdt body must be addressable from real mode by
+	# copying it to the lower mem
+	popl	%edx
+	lea     real_gdt_table - wakeup_start (%edx), %edx
+	movl    %edx, real_save_gdt + 2 - wakeup_start (%eax)
+	xor     %ecx, %ecx
+	movw    saved_gdt, %cx
+	movl    saved_gdt + 2, %esi
+	lea     real_gdt_table - wakeup_start (%eax), %edi
+	rep movsb
+
 	movl	saved_videomode, %edx
 	movl	%edx, video_mode - wakeup_start (%eax)
 	movl	acpi_video_flags, %edx

Re: [patch] Fix GDT re-load on ACPI resume

[Maciej W. Rozycki]

Hi Hiroshi-san,

> I believe this patch is required because the original gdt is not addressable
> via the low mapping page table (set by acpi_save_state_mem and used for wakeup
> code), not the GDTR's linear address size reason.

 That makes sense, indeed.  Using "lgdtl" (and likewise "lidtl")  
universally is a bit safer anyway.

  Maciej

Re: [patch] Fix GDT re-load on ACPI resume

[Pavel Machek]

Hi!

> > 	movw	_0x0e00 + 'i', %fs:(0x12)
> >
> > 	# need a gdt
> >+	.byte	0x66			# force 32-bit operands in case
> >+					# the GDT is past 16 megabytes
> > 	lgdt	real_save_gdt - wakeup_code
> >
> > 	movl	real_save_cr0 - wakeup_code, %eax
> There is a patch from hiroit@mcn.ne.jp to fix the GDT issue. You can try
> it.

Well, replacing lgdt with lgdtl (above) seems like nicer solution than
attachment...
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

RE: [patch] Fix GDT re-load on ACPI resume

[Li, Shaohua]

>> > 	movw	_0x0e00 + 'i', %fs:(0x12)
>> >
>> > 	# need a gdt
>> >+	.byte	0x66			# force 32-bit operands in case
>> >+					# the GDT is past 16 megabytes
>> > 	lgdt	real_save_gdt - wakeup_code
>> >
>> > 	movl	real_save_cr0 - wakeup_code, %eax
>> There is a patch from hiroit@mcn.ne.jp to fix the GDT issue. You can
try
>> it.
>
>Well, replacing lgdt with lgdtl (above) seems like nicer solution than
>attachment...
Copy GDT to low mem seems safer. Now the GDT table is in per-cpu region,
possibly it's not in low memory. Or am I missing anything?

Shaohua

[patch] Fix GDT re-load on ACPI resume

[Nickolai Zeldovich]

The ACPI resume code currently uses a real-mode 16-bit lgdt instruction to
reload the GDT.  This only restores the lower 24 bits of the GDT base
address.  In recent kernels, the GDT seems to have moved out of the lower
16 megs, thereby causing the ACPI resume to fail -- an invalid GDT was
being loaded.

This simple patch adds the 0x66 prefix to lgdt, which forces it to load
all 32 bits of the GDT base address, thereby removing any restrictions on
where the GDT can be placed in memory.  This makes ACPI resume work for me
on a Thinkpad T40 laptop.

-- kolya

--- linux-2.6.9/arch/i386/kernel/acpi/wakeup.S	2004/11/15 09:00:34	1.1
+++ linux-2.6.9/arch/i386/kernel/acpi/wakeup.S	2004/11/15 20:33:27
@@ -67,6 +67,8 @@
 	movw	$0x0e00 + 'i', %fs:(0x12)

 	# need a gdt
+	.byte	0x66			# force 32-bit operands in case
+					# the GDT is past 16 megabytes
 	lgdt	real_save_gdt - wakeup_code

 	movl	real_save_cr0 - wakeup_code, %eax

Re: [patch] Fix GDT re-load on ACPI resume

[David Weinehall]

On Mon, Nov 15, 2004 at 03:35:14PM -0500, Nickolai Zeldovich wrote:
> The ACPI resume code currently uses a real-mode 16-bit lgdt instruction to
> reload the GDT.  This only restores the lower 24 bits of the GDT base
> address.  In recent kernels, the GDT seems to have moved out of the lower
> 16 megs, thereby causing the ACPI resume to fail -- an invalid GDT was
> being loaded.
> 
> This simple patch adds the 0x66 prefix to lgdt, which forces it to load
> all 32 bits of the GDT base address, thereby removing any restrictions on
> where the GDT can be placed in memory.  This makes ACPI resume work for me
> on a Thinkpad T40 laptop.

Sadly doesn't work for me.  ACPI resume broke for me with
2.6.10-rc1-bk15 (possibly 14, but that one didn't compile), and this
patch does not fix it.


Regards: David Weinehall
-- 
 /) David Weinehall <tao@acc.umu.se> /) Northern lights wander      (\
//  Maintainer of the v2.0 kernel   //  Dance across the winter sky //
\)  http://www.acc.umu.se/~tao/    (/   Full colour fire           (/

Re: [patch] Fix GDT re-load on ACPI resume

[Nickolai Zeldovich]

On Tue, 16 Nov 2004, David Weinehall wrote:

> Sadly doesn't work for me.  ACPI resume broke for me with
> 2.6.10-rc1-bk15 (possibly 14, but that one didn't compile), and this
> patch does not fix it.

For the record, the symptoms of the problem fixed by the GDT patch in
question are that, upon ACPI resume, your screen shows the letters "Lin"
in yellow, hangs for a little bit, and then reboots.

-- kolya

Re: [patch] Fix GDT re-load on ACPI resume

[Maciej W. Rozycki]

On Mon, 15 Nov 2004, Nickolai Zeldovich wrote:

> This simple patch adds the 0x66 prefix to lgdt, which forces it to load
> all 32 bits of the GDT base address, thereby removing any restrictions on
> where the GDT can be placed in memory.  This makes ACPI resume work for me
> on a Thinkpad T40 laptop.
[...]
> +	.byte	0x66			# force 32-bit operands in case
> +					# the GDT is past 16 megabytes
>  	lgdt	real_save_gdt - wakeup_code

 You should use "lgdtl" and let gas figure out the rest.

  Maciej

Re: [patch] Fix GDT re-load on ACPI resume

[Nickolai Zeldovich]

On Mon, 15 Nov 2004, Maciej W. Rozycki wrote:

>  You should use "lgdtl" and let gas figure out the rest.

Thanks for the pointer; here's an updated patch.

-- kolya

--- linux-2.6.9/arch/i386/kernel/acpi/wakeup.S	2004/11/15 09:00:34	1.1
+++ linux-2.6.9/arch/i386/kernel/acpi/wakeup.S	2004/11/15 23:50:38
@@ -66,8 +66,9 @@
 	movw	%ax,%fs
 	movw	$0x0e00 + 'i', %fs:(0x12)

-	# need a gdt
-	lgdt	real_save_gdt - wakeup_code
+	# need a gdt -- use lgdtl to force 32-bit operands, in case
+	# the GDT is located past 16 megabytes.
+	lgdtl	real_save_gdt - wakeup_code

 	movl	real_save_cr0 - wakeup_code, %eax
 	movl	%eax, %cr0