[PATCH] Fix i486 suspend to disk CR4 oops

[PATCH] Fix i486 suspend to disk CR4 oops

[David Fries]

arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
only a i486 CPU doesn't have the CR4 register.  Trying to read it
produces an invalid opcode oops during suspend to disk.

Added the check (boot_cpu_data.x86 >= 5) before reading the
register.  If the value to be written is zero the write is
skipped.

arch/x86/power/hibernate_asm_32.S
done: swapped the use of %eax and %ecx to use jecxz for
the zero test and jump over store to %cr4.
restore_image: s/%ecx/%eax/ to be consistent with done:

In addition to __save_processor_state, acpi_save_state_mem,
efi_call_phys_prelog, and efi_call_phys_epilog had checks added
(acpi restore was in assembly and already had a check for
non-zero).  There were other reads and writes of CR4, but MCE and
virtualization shouldn't be executed on a i486 anyway.

Signed-off-by: David Fries <david@fries.net>
---
 arch/x86/kernel/acpi/sleep.c      |    4 +++-
 arch/x86/kernel/efi_32.c          |    6 ++++--
 arch/x86/power/cpu_32.c           |    8 ++++++--
 arch/x86/power/hibernate_asm_32.S |   26 +++++++++++++++-----------
 4 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 81e5ab6..bd0f2a3 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -86,7 +86,9 @@ int acpi_save_state_mem(void)
 #endif /* !CONFIG_64BIT */
 
 	header->pmode_cr0 = read_cr0();
-	header->pmode_cr4 = read_cr4();
+	/* cr4 was introduced in the Pentium CPU */
+	if (boot_cpu_data.x86 >= 5)
+		header->pmode_cr4 = read_cr4();
 	header->realmode_flags = acpi_realmode_flags;
 	header->real_magic = 0x12345678;
 
diff --git a/arch/x86/kernel/efi_32.c b/arch/x86/kernel/efi_32.c
index 4b63c8e..ad62d31 100644
--- a/arch/x86/kernel/efi_32.c
+++ b/arch/x86/kernel/efi_32.c
@@ -53,7 +53,8 @@ void efi_call_phys_prelog(void)
 	 * directory. If I have PAE, I just need to duplicate one entry in
 	 * page directory.
 	 */
-	cr4 = read_cr4();
+	/* cr4 was introduced in the Pentium CPU */
+	cr4 = boot_cpu_data.x86 >= 5 ? read_cr4() : 0;
 
 	if (cr4 & X86_CR4_PAE) {
 		efi_bak_pg_dir_pointer[0].pgd =
@@ -91,7 +92,8 @@ void efi_call_phys_epilog(void)
 	gdt_descr.size = GDT_SIZE - 1;
 	load_gdt(&gdt_descr);
 
-	cr4 = read_cr4();
+	/* cr4 was introduced in the Pentium CPU */
+	cr4 = boot_cpu_data.x86 >= 5 ? read_cr4() : 0;
 
 	if (cr4 & X86_CR4_PAE) {
 		swapper_pg_dir[pgd_index(0)].pgd =
diff --git a/arch/x86/power/cpu_32.c b/arch/x86/power/cpu_32.c
index 7dc5d5c..c5647ea 100644
--- a/arch/x86/power/cpu_32.c
+++ b/arch/x86/power/cpu_32.c
@@ -45,7 +45,9 @@ static void __save_processor_state(struct saved_context *ctxt)
 	ctxt->cr0 = read_cr0();
 	ctxt->cr2 = read_cr2();
 	ctxt->cr3 = read_cr3();
-	ctxt->cr4 = read_cr4();
+	/* cr4 was introduced in the Pentium CPU */
+	if (boot_cpu_data.x86 >= 5)
+		ctxt->cr4 = read_cr4();
 }
 
 /* Needed by apm.c */
@@ -98,7 +100,9 @@ static void __restore_processor_state(struct saved_context *ctxt)
 	/*
 	 * control registers
 	 */
-	write_cr4(ctxt->cr4);
+	/* cr4 was introduced in the Pentium CPU */
+	if (ctxt->cr4)
+		write_cr4(ctxt->cr4);
 	write_cr3(ctxt->cr3);
 	write_cr2(ctxt->cr2);
 	write_cr0(ctxt->cr0);
diff --git a/arch/x86/power/hibernate_asm_32.S b/arch/x86/power/hibernate_asm_32.S
index b95aa6c..4fc7e87 100644
--- a/arch/x86/power/hibernate_asm_32.S
+++ b/arch/x86/power/hibernate_asm_32.S
@@ -28,9 +28,9 @@ ENTRY(swsusp_arch_suspend)
 	ret
 
 ENTRY(restore_image)
-	movl	resume_pg_dir, %ecx
-	subl	$__PAGE_OFFSET, %ecx
-	movl	%ecx, %cr3
+	movl	resume_pg_dir, %eax
+	subl	$__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
 
 	movl	restore_pblist, %edx
 	.p2align 4,,7
@@ -52,17 +52,21 @@ copy_loop:
 
 done:
 	/* go back to the original page tables */
-	movl	$swapper_pg_dir, %ecx
-	subl	$__PAGE_OFFSET, %ecx
-	movl	%ecx, %cr3
+	movl	$swapper_pg_dir, %eax
+	subl	$__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
 	/* Flush TLB, including "global" things (vmalloc) */
-	movl	mmu_cr4_features, %eax
-	movl	%eax, %edx
+	movl	mmu_cr4_features, %ecx
+	jecxz	1f	# cr4 Pentium and higher, skip if zero
+	movl	%ecx, %edx
 	andl	$~(1<<7), %edx;  # PGE
 	movl	%edx, %cr4;  # turn off PGE
-	movl	%cr3, %ecx;  # flush TLB
-	movl	%ecx, %cr3
-	movl	%eax, %cr4;  # turn PGE back on
+1:
+	movl	%cr3, %eax;  # flush TLB
+	movl	%eax, %cr3
+	jecxz	1f	# cr4 Pentium and higher, skip if zero
+	movl	%ecx, %cr4;  # turn PGE back on
+1:
 
 	movl saved_context_esp, %esp
 	movl saved_context_ebp, %ebp
-- 
1.4.4.4

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Maciej W. Rozycki]

On Sun, 17 Aug 2008, David Fries wrote:

> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
> only a i486 CPU doesn't have the CR4 register.  Trying to read it
> produces an invalid opcode oops during suspend to disk.
[...]
> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> index 81e5ab6..bd0f2a3 100644
> --- a/arch/x86/kernel/acpi/sleep.c
> +++ b/arch/x86/kernel/acpi/sleep.c
> @@ -86,7 +86,9 @@ int acpi_save_state_mem(void)
>  #endif /* !CONFIG_64BIT */
>  
>  	header->pmode_cr0 = read_cr0();
> -	header->pmode_cr4 = read_cr4();
> +	/* cr4 was introduced in the Pentium CPU */
> +	if (boot_cpu_data.x86 >= 5)
> +		header->pmode_cr4 = read_cr4();
>  	header->realmode_flags = acpi_realmode_flags;
>  	header->real_magic = 0x12345678;
>  

 NACK.  Later i486 chips do have CR4 -- for PSE, VME, etc. (the set of
features varies across the line).  Use a fixup as elsewhere or something.

  Maciej

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[H. Peter Anvin]

Maciej W. Rozycki wrote:
> On Sun, 17 Aug 2008, David Fries wrote:
> 
>> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
>> only a i486 CPU doesn't have the CR4 register.  Trying to read it
>> produces an invalid opcode oops during suspend to disk.
> [...]
>> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
>> index 81e5ab6..bd0f2a3 100644
>> --- a/arch/x86/kernel/acpi/sleep.c
>> +++ b/arch/x86/kernel/acpi/sleep.c
>> @@ -86,7 +86,9 @@ int acpi_save_state_mem(void)
>>  #endif /* !CONFIG_64BIT */
>>  
>>  	header->pmode_cr0 = read_cr0();
>> -	header->pmode_cr4 = read_cr4();
>> +	/* cr4 was introduced in the Pentium CPU */
>> +	if (boot_cpu_data.x86 >= 5)
>> +		header->pmode_cr4 = read_cr4();
>>  	header->realmode_flags = acpi_realmode_flags;
>>  	header->real_magic = 0x12345678;
>>  
> 
>  NACK.  Later i486 chips do have CR4 -- for PSE, VME, etc. (the set of
> features varies across the line).  Use a fixup as elsewhere or something.
> 

The other alternative is to probe for the CPUID instruction (via 
EFLAGS.ID) -- CR4 is present if and only if CPUID exists.

	-hpa

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Andi Kleen]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Maciej W. Rozycki wrote:
>> On Sun, 17 Aug 2008, David Fries wrote:
>>
>>> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
>>> only a i486 CPU doesn't have the CR4 register.  Trying to read it
>>> produces an invalid opcode oops during suspend to disk.
>> [...]
>>> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
>>> index 81e5ab6..bd0f2a3 100644
>>> --- a/arch/x86/kernel/acpi/sleep.c
>>> +++ b/arch/x86/kernel/acpi/sleep.c
>>> @@ -86,7 +86,9 @@ int acpi_save_state_mem(void)
>>>  #endif /* !CONFIG_64BIT */
>>>   	header->pmode_cr0 = read_cr0();
>>> -	header->pmode_cr4 = read_cr4();
>>> +	/* cr4 was introduced in the Pentium CPU */
>>> +	if (boot_cpu_data.x86 >= 5)
>>> +		header->pmode_cr4 = read_cr4();
>>>  	header->realmode_flags = acpi_realmode_flags;
>>>  	header->real_magic = 0x12345678;
>>>
>>  NACK.  Later i486 chips do have CR4 -- for PSE, VME, etc. (the set
>> of
>> features varies across the line).  Use a fixup as elsewhere or something.
>>
>
> The other alternative is to probe for the CPUID instruction (via
> EFLAGS.ID) -- CR4 is present if and only if CPUID exists.

Can be already checked for with boot_cpu_data.extended_cpuid_level.

-Andi

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[H. Peter Anvin]

Andi Kleen wrote:
>>>
>> The other alternative is to probe for the CPUID instruction (via
>> EFLAGS.ID) -- CR4 is present if and only if CPUID exists.
> 
> Can be already checked for with boot_cpu_data.extended_cpuid_level.
> 

I believe you mean just plain cpuid_level (which is set to -1 if CPUID 
doesn't exist.)  extended_cpuid_level is only defined on x86-64, but all 
x86-64 CPUs have CR4.

	-hpa

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Ingo Molnar]

* David Fries <david@fries.net> wrote:

> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4() only a 
> i486 CPU doesn't have the CR4 register.  Trying to read it produces an 
> invalid opcode oops during suspend to disk.
> 
> Added the check (boot_cpu_data.x86 >= 5) before reading the register.  
> If the value to be written is zero the write is skipped.
> 
> arch/x86/power/hibernate_asm_32.S
> done: swapped the use of %eax and %ecx to use jecxz for
> the zero test and jump over store to %cr4.
> restore_image: s/%ecx/%eax/ to be consistent with done:
> 
> In addition to __save_processor_state, acpi_save_state_mem, 
> efi_call_phys_prelog, and efi_call_phys_epilog had checks added (acpi 
> restore was in assembly and already had a check for non-zero).  There 
> were other reads and writes of CR4, but MCE and virtualization 
> shouldn't be executed on a i486 anyway.
> 
> Signed-off-by: David Fries <david@fries.net>

applied to tip/x86/urgent, thanks David. I've changed the conditions to 
read_cr4_safe() instead - that's cleaner. Could you please check whether 
the patch below works fine too on your box?

Rafael, Pavel - does the commit below look good to you too?

	Ingo

---------------------->
>From e437fa5586f2e3b2aaeba649fae52be1f9a6eabb Mon Sep 17 00:00:00 2001
From: David Fries <david@fries.net>
Date: Sun, 17 Aug 2008 23:03:40 -0500
Subject: [PATCH] x86: fix i486 suspend to disk CR4 oops

arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
only a i486 CPU doesn't have the CR4 register.  Trying to read it
produces an invalid opcode oops during suspend to disk.

Use the safe rc4 reading op instead. If the value to be written is
zero the write is skipped.

arch/x86/power/hibernate_asm_32.S
done: swapped the use of %eax and %ecx to use jecxz for
the zero test and jump over store to %cr4.
restore_image: s/%ecx/%eax/ to be consistent with done:

In addition to __save_processor_state, acpi_save_state_mem,
efi_call_phys_prelog, and efi_call_phys_epilog had checks added
(acpi restore was in assembly and already had a check for
non-zero).  There were other reads and writes of CR4, but MCE and
virtualization shouldn't be executed on a i486 anyway.

Signed-off-by: David Fries <david@fries.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/acpi/sleep.c      |    2 +-
 arch/x86/kernel/efi_32.c          |    4 ++--
 arch/x86/power/cpu_32.c           |    6 ++++--
 arch/x86/power/hibernate_asm_32.S |   26 +++++++++++++++-----------
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 81e5ab6..426e5d9 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -86,7 +86,7 @@ int acpi_save_state_mem(void)
 #endif /* !CONFIG_64BIT */
 
 	header->pmode_cr0 = read_cr0();
-	header->pmode_cr4 = read_cr4();
+	header->pmode_cr4 = read_cr4_safe();
 	header->realmode_flags = acpi_realmode_flags;
 	header->real_magic = 0x12345678;
 
diff --git a/arch/x86/kernel/efi_32.c b/arch/x86/kernel/efi_32.c
index 4b63c8e..5cab48e 100644
--- a/arch/x86/kernel/efi_32.c
+++ b/arch/x86/kernel/efi_32.c
@@ -53,7 +53,7 @@ void efi_call_phys_prelog(void)
 	 * directory. If I have PAE, I just need to duplicate one entry in
 	 * page directory.
 	 */
-	cr4 = read_cr4();
+	cr4 = read_cr4_safe();
 
 	if (cr4 & X86_CR4_PAE) {
 		efi_bak_pg_dir_pointer[0].pgd =
@@ -91,7 +91,7 @@ void efi_call_phys_epilog(void)
 	gdt_descr.size = GDT_SIZE - 1;
 	load_gdt(&gdt_descr);
 
-	cr4 = read_cr4();
+	cr4 = read_cr4_safe();
 
 	if (cr4 & X86_CR4_PAE) {
 		swapper_pg_dir[pgd_index(0)].pgd =
diff --git a/arch/x86/power/cpu_32.c b/arch/x86/power/cpu_32.c
index 7dc5d5c..d3e083d 100644
--- a/arch/x86/power/cpu_32.c
+++ b/arch/x86/power/cpu_32.c
@@ -45,7 +45,7 @@ static void __save_processor_state(struct saved_context *ctxt)
 	ctxt->cr0 = read_cr0();
 	ctxt->cr2 = read_cr2();
 	ctxt->cr3 = read_cr3();
-	ctxt->cr4 = read_cr4();
+	ctxt->cr4 = read_cr4_safe();
 }
 
 /* Needed by apm.c */
@@ -98,7 +98,9 @@ static void __restore_processor_state(struct saved_context *ctxt)
 	/*
 	 * control registers
 	 */
-	write_cr4(ctxt->cr4);
+	/* cr4 was introduced in the Pentium CPU */
+	if (ctxt->cr4)
+		write_cr4(ctxt->cr4);
 	write_cr3(ctxt->cr3);
 	write_cr2(ctxt->cr2);
 	write_cr0(ctxt->cr0);
diff --git a/arch/x86/power/hibernate_asm_32.S b/arch/x86/power/hibernate_asm_32.S
index b95aa6c..4fc7e87 100644
--- a/arch/x86/power/hibernate_asm_32.S
+++ b/arch/x86/power/hibernate_asm_32.S
@@ -28,9 +28,9 @@ ENTRY(swsusp_arch_suspend)
 	ret
 
 ENTRY(restore_image)
-	movl	resume_pg_dir, %ecx
-	subl	$__PAGE_OFFSET, %ecx
-	movl	%ecx, %cr3
+	movl	resume_pg_dir, %eax
+	subl	$__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
 
 	movl	restore_pblist, %edx
 	.p2align 4,,7
@@ -52,17 +52,21 @@ copy_loop:
 
 done:
 	/* go back to the original page tables */
-	movl	$swapper_pg_dir, %ecx
-	subl	$__PAGE_OFFSET, %ecx
-	movl	%ecx, %cr3
+	movl	$swapper_pg_dir, %eax
+	subl	$__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
 	/* Flush TLB, including "global" things (vmalloc) */
-	movl	mmu_cr4_features, %eax
-	movl	%eax, %edx
+	movl	mmu_cr4_features, %ecx
+	jecxz	1f	# cr4 Pentium and higher, skip if zero
+	movl	%ecx, %edx
 	andl	$~(1<<7), %edx;  # PGE
 	movl	%edx, %cr4;  # turn off PGE
-	movl	%cr3, %ecx;  # flush TLB
-	movl	%ecx, %cr3
-	movl	%eax, %cr4;  # turn PGE back on
+1:
+	movl	%cr3, %eax;  # flush TLB
+	movl	%eax, %cr3
+	jecxz	1f	# cr4 Pentium and higher, skip if zero
+	movl	%ecx, %cr4;  # turn PGE back on
+1:
 
 	movl saved_context_esp, %esp
 	movl saved_context_ebp, %ebp

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Andi Kleen]

On Sun, Aug 17, 2008 at 11:34:50PM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >>>
> >>The other alternative is to probe for the CPUID instruction (via
> >>EFLAGS.ID) -- CR4 is present if and only if CPUID exists.
> >
> >Can be already checked for with boot_cpu_data.extended_cpuid_level.
> >
> 
> I believe you mean just plain cpuid_level (which is set to -1 if CPUID 
> doesn't exist.)  extended_cpuid_level is only defined on x86-64, but all 
> x86-64 CPUs have CR4.

Yes I meant plain cpuid_level. Just wanted to mention that before someone
opencodes another cpuid test.

-Andi

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[H. Peter Anvin]

Ingo Molnar wrote:
> 
> applied to tip/x86/urgent, thanks David. I've changed the conditions to 
> read_cr4_safe() instead - that's cleaner. Could you please check whether 
> the patch below works fine too on your box?
> 
> Rafael, Pavel - does the commit below look good to you too?
> 

Looks good to me.

	-hpa

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Pavel Machek]

> 
> * David Fries <david@fries.net> wrote:
> 
> > arch/x86/power/cpu_32.c __save_processor_state calls read_cr4() only a 
> > i486 CPU doesn't have the CR4 register.  Trying to read it produces an 
> > invalid opcode oops during suspend to disk.
> > 
> > Added the check (boot_cpu_data.x86 >= 5) before reading the register.  
> > If the value to be written is zero the write is skipped.
> > 
> > arch/x86/power/hibernate_asm_32.S
> > done: swapped the use of %eax and %ecx to use jecxz for
> > the zero test and jump over store to %cr4.
> > restore_image: s/%ecx/%eax/ to be consistent with done:
> > 
> > In addition to __save_processor_state, acpi_save_state_mem, 
> > efi_call_phys_prelog, and efi_call_phys_epilog had checks added (acpi 
> > restore was in assembly and already had a check for non-zero).  There 
> > were other reads and writes of CR4, but MCE and virtualization 
> > shouldn't be executed on a i486 anyway.
> > 
> > Signed-off-by: David Fries <david@fries.net>
> 
> applied to tip/x86/urgent, thanks David. I've changed the conditions to 
> read_cr4_safe() instead - that's cleaner. Could you please check whether 
> the patch below works fine too on your box?
> 
> Rafael, Pavel - does the commit below look good to you too?
> 
> 	Ingo
> 
> ---------------------->
> >From e437fa5586f2e3b2aaeba649fae52be1f9a6eabb Mon Sep 17 00:00:00 2001
> From: David Fries <david@fries.net>
> Date: Sun, 17 Aug 2008 23:03:40 -0500
> Subject: [PATCH] x86: fix i486 suspend to disk CR4 oops
> 
> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
> only a i486 CPU doesn't have the CR4 register.  Trying to read it
> produces an invalid opcode oops during suspend to disk.
> 
> Use the safe rc4 reading op instead. If the value to be written is
> zero the write is skipped.
> 
> arch/x86/power/hibernate_asm_32.S
> done: swapped the use of %eax and %ecx to use jecxz for
> the zero test and jump over store to %cr4.
> restore_image: s/%ecx/%eax/ to be consistent with done:
> 
> In addition to __save_processor_state, acpi_save_state_mem,
> efi_call_phys_prelog, and efi_call_phys_epilog had checks added
> (acpi restore was in assembly and already had a check for
> non-zero).  There were other reads and writes of CR4, but MCE and
> virtualization shouldn't be executed on a i486 anyway.
> 
> Signed-off-by: David Fries <david@fries.net>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>

Acked-by: Pavel Machek <pavel@suse.cz>


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Rafael J. Wysocki]

On Monday, 18 of August 2008, Ingo Molnar wrote:
> 
> * David Fries <david@fries.net> wrote:
> 
> > arch/x86/power/cpu_32.c __save_processor_state calls read_cr4() only a 
> > i486 CPU doesn't have the CR4 register.  Trying to read it produces an 
> > invalid opcode oops during suspend to disk.
> > 
> > Added the check (boot_cpu_data.x86 >= 5) before reading the register.  
> > If the value to be written is zero the write is skipped.
> > 
> > arch/x86/power/hibernate_asm_32.S
> > done: swapped the use of %eax and %ecx to use jecxz for
> > the zero test and jump over store to %cr4.
> > restore_image: s/%ecx/%eax/ to be consistent with done:
> > 
> > In addition to __save_processor_state, acpi_save_state_mem, 
> > efi_call_phys_prelog, and efi_call_phys_epilog had checks added (acpi 
> > restore was in assembly and already had a check for non-zero).  There 
> > were other reads and writes of CR4, but MCE and virtualization 
> > shouldn't be executed on a i486 anyway.
> > 
> > Signed-off-by: David Fries <david@fries.net>
> 
> applied to tip/x86/urgent, thanks David. I've changed the conditions to 
> read_cr4_safe() instead - that's cleaner. Could you please check whether 
> the patch below works fine too on your box?
> 
> Rafael, Pavel - does the commit below look good to you too?
> 
> 	Ingo
> 
> ---------------------->
> From e437fa5586f2e3b2aaeba649fae52be1f9a6eabb Mon Sep 17 00:00:00 2001
> From: David Fries <david@fries.net>
> Date: Sun, 17 Aug 2008 23:03:40 -0500
> Subject: [PATCH] x86: fix i486 suspend to disk CR4 oops
> 
> arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
> only a i486 CPU doesn't have the CR4 register.  Trying to read it
> produces an invalid opcode oops during suspend to disk.
> 
> Use the safe rc4 reading op instead. If the value to be written is
> zero the write is skipped.
> 
> arch/x86/power/hibernate_asm_32.S
> done: swapped the use of %eax and %ecx to use jecxz for
> the zero test and jump over store to %cr4.
> restore_image: s/%ecx/%eax/ to be consistent with done:
> 
> In addition to __save_processor_state, acpi_save_state_mem,
> efi_call_phys_prelog, and efi_call_phys_epilog had checks added
> (acpi restore was in assembly and already had a check for
> non-zero).  There were other reads and writes of CR4, but MCE and
> virtualization shouldn't be executed on a i486 anyway.
> 
> Signed-off-by: David Fries <david@fries.net>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  arch/x86/kernel/acpi/sleep.c      |    2 +-
>  arch/x86/kernel/efi_32.c          |    4 ++--
>  arch/x86/power/cpu_32.c           |    6 ++++--
>  arch/x86/power/hibernate_asm_32.S |   26 +++++++++++++++-----------
>  4 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> index 81e5ab6..426e5d9 100644
> --- a/arch/x86/kernel/acpi/sleep.c
> +++ b/arch/x86/kernel/acpi/sleep.c
> @@ -86,7 +86,7 @@ int acpi_save_state_mem(void)
>  #endif /* !CONFIG_64BIT */
>  
>  	header->pmode_cr0 = read_cr0();
> -	header->pmode_cr4 = read_cr4();
> +	header->pmode_cr4 = read_cr4_safe();
>  	header->realmode_flags = acpi_realmode_flags;
>  	header->real_magic = 0x12345678;
>  
> diff --git a/arch/x86/kernel/efi_32.c b/arch/x86/kernel/efi_32.c
> index 4b63c8e..5cab48e 100644
> --- a/arch/x86/kernel/efi_32.c
> +++ b/arch/x86/kernel/efi_32.c
> @@ -53,7 +53,7 @@ void efi_call_phys_prelog(void)
>  	 * directory. If I have PAE, I just need to duplicate one entry in
>  	 * page directory.
>  	 */
> -	cr4 = read_cr4();
> +	cr4 = read_cr4_safe();
>  
>  	if (cr4 & X86_CR4_PAE) {
>  		efi_bak_pg_dir_pointer[0].pgd =
> @@ -91,7 +91,7 @@ void efi_call_phys_epilog(void)
>  	gdt_descr.size = GDT_SIZE - 1;
>  	load_gdt(&gdt_descr);
>  
> -	cr4 = read_cr4();
> +	cr4 = read_cr4_safe();
>  
>  	if (cr4 & X86_CR4_PAE) {
>  		swapper_pg_dir[pgd_index(0)].pgd =
> diff --git a/arch/x86/power/cpu_32.c b/arch/x86/power/cpu_32.c
> index 7dc5d5c..d3e083d 100644
> --- a/arch/x86/power/cpu_32.c
> +++ b/arch/x86/power/cpu_32.c
> @@ -45,7 +45,7 @@ static void __save_processor_state(struct saved_context *ctxt)
>  	ctxt->cr0 = read_cr0();
>  	ctxt->cr2 = read_cr2();
>  	ctxt->cr3 = read_cr3();
> -	ctxt->cr4 = read_cr4();
> +	ctxt->cr4 = read_cr4_safe();
>  }
>  
>  /* Needed by apm.c */
> @@ -98,7 +98,9 @@ static void __restore_processor_state(struct saved_context *ctxt)
>  	/*
>  	 * control registers
>  	 */
> -	write_cr4(ctxt->cr4);
> +	/* cr4 was introduced in the Pentium CPU */
> +	if (ctxt->cr4)
> +		write_cr4(ctxt->cr4);
>  	write_cr3(ctxt->cr3);
>  	write_cr2(ctxt->cr2);
>  	write_cr0(ctxt->cr0);
> diff --git a/arch/x86/power/hibernate_asm_32.S b/arch/x86/power/hibernate_asm_32.S
> index b95aa6c..4fc7e87 100644
> --- a/arch/x86/power/hibernate_asm_32.S
> +++ b/arch/x86/power/hibernate_asm_32.S
> @@ -28,9 +28,9 @@ ENTRY(swsusp_arch_suspend)
>  	ret
>  
>  ENTRY(restore_image)
> -	movl	resume_pg_dir, %ecx
> -	subl	$__PAGE_OFFSET, %ecx
> -	movl	%ecx, %cr3
> +	movl	resume_pg_dir, %eax
> +	subl	$__PAGE_OFFSET, %eax
> +	movl	%eax, %cr3
>  
>  	movl	restore_pblist, %edx
>  	.p2align 4,,7
> @@ -52,17 +52,21 @@ copy_loop:
>  
>  done:
>  	/* go back to the original page tables */
> -	movl	$swapper_pg_dir, %ecx
> -	subl	$__PAGE_OFFSET, %ecx
> -	movl	%ecx, %cr3
> +	movl	$swapper_pg_dir, %eax
> +	subl	$__PAGE_OFFSET, %eax
> +	movl	%eax, %cr3
>  	/* Flush TLB, including "global" things (vmalloc) */
> -	movl	mmu_cr4_features, %eax
> -	movl	%eax, %edx
> +	movl	mmu_cr4_features, %ecx
> +	jecxz	1f	# cr4 Pentium and higher, skip if zero
> +	movl	%ecx, %edx
>  	andl	$~(1<<7), %edx;  # PGE
>  	movl	%edx, %cr4;  # turn off PGE
> -	movl	%cr3, %ecx;  # flush TLB
> -	movl	%ecx, %cr3
> -	movl	%eax, %cr4;  # turn PGE back on
> +1:
> +	movl	%cr3, %eax;  # flush TLB
> +	movl	%eax, %cr3
> +	jecxz	1f	# cr4 Pentium and higher, skip if zero
> +	movl	%ecx, %cr4;  # turn PGE back on
> +1:
>  
>  	movl saved_context_esp, %esp
>  	movl saved_context_ebp, %ebp
> 
> 

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[David Fries]

On Mon, Aug 18, 2008 at 05:14:50AM +0100, Maciej W. Rozycki wrote:
> On Sun, 17 Aug 2008, David Fries wrote:
> > +	/* cr4 was introduced in the Pentium CPU */
> 
>  NACK.  Later i486 chips do have CR4 -- for PSE, VME, etc. (the set of
> features varies across the line).  Use a fixup as elsewhere or something.
> 
>   Maciej

That's what I get for reading the Intel instruction set reference,
"The CR4 register was added to the Intel Architecture beginning with
the Pentium processor."

Ingo Molnar, thanks, I'll try the read_cr4_safe() version tonight (the
computer is in the trunk of my car and I'm about ready to head to
work).

In light of the above, how about updating the comments
-       /* cr4 was introduced in the Pentium CPU */
-       jecxz   1f      # cr4 Pentium and higher, skip if zero
+       /* cr4 not in i386 only some i486, skip if zero */
+       jecxz   1f      # cr4 not in i386 only some i486, skip if zero

I'm not being bit by arch/x86/kernel/relocate_kernel_32.S, but it is
using cr4.  Should that be fixed up as well?

-- 
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Ingo Molnar]

On Mon, Aug 18, 2008 at 05:14:50AM +0100, Maciej W. Rozycki wrote:
> On Sun, 17 Aug 2008, David Fries wrote:
> > +	/* cr4 was introduced in the Pentium CPU */
> 
>  NACK.  Later i486 chips do have CR4 -- for PSE, VME, etc. (the set of 
> features varies across the line).  Use a fixup as elsewhere or 
> something.

the version i committed (reproduced below) should work fine. It uses cr4 
opportunistically (we get a fault if it does not exist), and we write it 
back if the cr4 value is non-zero. (which it must always be on a 
CR4-enabled processor)

agreed?

	Ingo

----------->
>From e532c06f2a835b5cc4f4166f467437d9b09c1d0e Mon Sep 17 00:00:00 2001
From: David Fries <david@fries.net>
Date: Sun, 17 Aug 2008 23:03:40 -0500
Subject: [PATCH] x86: fix i486 suspend to disk CR4 oops

arch/x86/power/cpu_32.c __save_processor_state calls read_cr4()
only a i486 CPU doesn't have the CR4 register.  Trying to read it
produces an invalid opcode oops during suspend to disk.

Use the safe rc4 reading op instead. If the value to be written is
zero the write is skipped.

arch/x86/power/hibernate_asm_32.S
done: swapped the use of %eax and %ecx to use jecxz for
the zero test and jump over store to %cr4.
restore_image: s/%ecx/%eax/ to be consistent with done:

In addition to __save_processor_state, acpi_save_state_mem,
efi_call_phys_prelog, and efi_call_phys_epilog had checks added
(acpi restore was in assembly and already had a check for
non-zero).  There were other reads and writes of CR4, but MCE and
virtualization shouldn't be executed on a i486 anyway.

Signed-off-by: David Fries <david@fries.net>
Acked-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/acpi/sleep.c      |    2 +-
 arch/x86/kernel/efi_32.c          |    4 ++--
 arch/x86/power/cpu_32.c           |    6 ++++--
 arch/x86/power/hibernate_asm_32.S |   26 +++++++++++++++-----------
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 81e5ab6..426e5d9 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -86,7 +86,7 @@ int acpi_save_state_mem(void)
 #endif /* !CONFIG_64BIT */
 
 	header->pmode_cr0 = read_cr0();
-	header->pmode_cr4 = read_cr4();
+	header->pmode_cr4 = read_cr4_safe();
 	header->realmode_flags = acpi_realmode_flags;
 	header->real_magic = 0x12345678;
 
diff --git a/arch/x86/kernel/efi_32.c b/arch/x86/kernel/efi_32.c
index 4b63c8e..5cab48e 100644
--- a/arch/x86/kernel/efi_32.c
+++ b/arch/x86/kernel/efi_32.c
@@ -53,7 +53,7 @@ void efi_call_phys_prelog(void)
 	 * directory. If I have PAE, I just need to duplicate one entry in
 	 * page directory.
 	 */
-	cr4 = read_cr4();
+	cr4 = read_cr4_safe();
 
 	if (cr4 & X86_CR4_PAE) {
 		efi_bak_pg_dir_pointer[0].pgd =
@@ -91,7 +91,7 @@ void efi_call_phys_epilog(void)
 	gdt_descr.size = GDT_SIZE - 1;
 	load_gdt(&gdt_descr);
 
-	cr4 = read_cr4();
+	cr4 = read_cr4_safe();
 
 	if (cr4 & X86_CR4_PAE) {
 		swapper_pg_dir[pgd_index(0)].pgd =
diff --git a/arch/x86/power/cpu_32.c b/arch/x86/power/cpu_32.c
index 7dc5d5c..d3e083d 100644
--- a/arch/x86/power/cpu_32.c
+++ b/arch/x86/power/cpu_32.c
@@ -45,7 +45,7 @@ static void __save_processor_state(struct saved_context *ctxt)
 	ctxt->cr0 = read_cr0();
 	ctxt->cr2 = read_cr2();
 	ctxt->cr3 = read_cr3();
-	ctxt->cr4 = read_cr4();
+	ctxt->cr4 = read_cr4_safe();
 }
 
 /* Needed by apm.c */
@@ -98,7 +98,9 @@ static void __restore_processor_state(struct saved_context *ctxt)
 	/*
 	 * control registers
 	 */
-	write_cr4(ctxt->cr4);
+	/* cr4 was introduced in the Pentium CPU */
+	if (ctxt->cr4)
+		write_cr4(ctxt->cr4);
 	write_cr3(ctxt->cr3);
 	write_cr2(ctxt->cr2);
 	write_cr0(ctxt->cr0);
diff --git a/arch/x86/power/hibernate_asm_32.S b/arch/x86/power/hibernate_asm_32.S
index b95aa6c..4fc7e87 100644
--- a/arch/x86/power/hibernate_asm_32.S
+++ b/arch/x86/power/hibernate_asm_32.S
@@ -28,9 +28,9 @@ ENTRY(swsusp_arch_suspend)
 	ret
 
 ENTRY(restore_image)
-	movl	resume_pg_dir, %ecx
-	subl	$__PAGE_OFFSET, %ecx
-	movl	%ecx, %cr3
+	movl	resume_pg_dir, %eax
+	subl	$__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
 
 	movl	restore_pblist, %edx
 	.p2align 4,,7
@@ -52,17 +52,21 @@ copy_loop:
 
 done:
 	/* go back to the original page tables */
-	movl	$swapper_pg_dir, %ecx
-	subl	$__PAGE_OFFSET, %ecx
-	movl	%ecx, %cr3
+	movl	$swapper_pg_dir, %eax
+	subl	$__PAGE_OFFSET, %eax
+	movl	%eax, %cr3
 	/* Flush TLB, including "global" things (vmalloc) */
-	movl	mmu_cr4_features, %eax
-	movl	%eax, %edx
+	movl	mmu_cr4_features, %ecx
+	jecxz	1f	# cr4 Pentium and higher, skip if zero
+	movl	%ecx, %edx
 	andl	$~(1<<7), %edx;  # PGE
 	movl	%edx, %cr4;  # turn off PGE
-	movl	%cr3, %ecx;  # flush TLB
-	movl	%ecx, %cr3
-	movl	%eax, %cr4;  # turn PGE back on
+1:
+	movl	%cr3, %eax;  # flush TLB
+	movl	%eax, %cr3
+	jecxz	1f	# cr4 Pentium and higher, skip if zero
+	movl	%ecx, %cr4;  # turn PGE back on
+1:
 
 	movl saved_context_esp, %esp
 	movl saved_context_ebp, %ebp

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Maciej W. Rozycki]

On Mon, 18 Aug 2008, David Fries wrote:

> That's what I get for reading the Intel instruction set reference,
> "The CR4 register was added to the Intel Architecture beginning with
> the Pentium processor."

 Indeed.  It started with the Pentium and some of the i486
models/steppings that were released after the Pentium got the CR4 register
implemented too.  These include at the least the write-back enhanced 
versions of the iDX2 and iDX4 processors as well later steppings of the 
i486SX, which I encountered myself.  There could be others -- iSX2?

 Unfortunately as usually with Intel you have to read all the documents
they release to hope to get a full image and even then you'll probably
come out confused and with bits of data missing -- if in doubt, check the
silicon: it has the most definite information.  However these additions
were actually documented quite well in the respective i486-class processor
datasheets -- IIRC Intel have released a combined version at one point.  
I am not sure whether it has ever been available on-line.

  Maciej

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Maciej W. Rozycki]

On Mon, 18 Aug 2008, Ingo Molnar wrote:

> the version i committed (reproduced below) should work fine. It uses cr4 
> opportunistically (we get a fault if it does not exist), and we write it 
> back if the cr4 value is non-zero. (which it must always be on a 
> CR4-enabled processor)
> 
> agreed?

Acked-by: Maciej W. Rozycki <macro@linux-mips.org>

  Maciej

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Dave Jones]

On Mon, Aug 18, 2008 at 08:41:20AM +0200, Ingo Molnar wrote:

 > diff --git a/arch/x86/kernel/efi_32.c b/arch/x86/kernel/efi_32.c
 > index 4b63c8e..5cab48e 100644
 > --- a/arch/x86/kernel/efi_32.c
 > +++ b/arch/x86/kernel/efi_32.c
 > @@ -53,7 +53,7 @@ void efi_call_phys_prelog(void)
 >  	 * directory. If I have PAE, I just need to duplicate one entry in
 >  	 * page directory.
 >  	 */
 > -	cr4 = read_cr4();
 > +	cr4 = read_cr4_safe();
 >  
 >  	if (cr4 & X86_CR4_PAE) {
 >  		efi_bak_pg_dir_pointer[0].pgd =
 > @@ -91,7 +91,7 @@ void efi_call_phys_epilog(void)
 >  	gdt_descr.size = GDT_SIZE - 1;
 >  	load_gdt(&gdt_descr);
 >  
 > -	cr4 = read_cr4();
 > +	cr4 = read_cr4_safe();
 >  
 >  	if (cr4 & X86_CR4_PAE) {
 >  		swapper_pg_dir[pgd_index(0)].pgd =

Is this part really necessary ?

Why would a 486 be in EFI code?

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Lennart Sorensen]

On Mon, Aug 18, 2008 at 11:24:40AM -0400, Dave Jones wrote:
> Is this part really necessary ?
> 
> Why would a 486 be in EFI code?

Someone has a hobby writing replacement firmware for their 486?

-- 
Len Sorensen

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Dave Jones]

On Mon, Aug 18, 2008 at 12:04:26PM -0400, Lennart Sorensen wrote:
 > On Mon, Aug 18, 2008 at 11:24:40AM -0400, Dave Jones wrote:
 > > Is this part really necessary ?
 > > 
 > > Why would a 486 be in EFI code?
 > 
 > Someone has a hobby writing replacement firmware for their 486?

Anyone who writes EFI code for a hobby really needs to see
a mental health specialist.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[H. Peter Anvin]

Dave Jones wrote:
> 
> Is this part really necessary ?
> 
> Why would a 486 be in EFI code?
> 

Necessary, almost certainly not, but (a) it doesn't hurt anything, (b) 
it's good to be consistent and (c) it might still be useful to virtualizers.

EFI is an abomination no matter how you look at it, of course.

	-hpa

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Pavel Machek]

On Mon 2008-08-18 10:32:24, H. Peter Anvin wrote:
> Dave Jones wrote:
>>
>> Is this part really necessary ?
>>
>> Why would a 486 be in EFI code?
>>
>
> Necessary, almost certainly not, but (a) it doesn't hurt anything, (b) it's 
> good to be consistent and (c) it might still be useful to virtualizers.

...it may be useful for something obscure... like system-on-chip...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[Pavel Machek]

On Mon 2008-08-18 07:58:03, David Fries wrote:
> On Mon, Aug 18, 2008 at 05:14:50AM +0100, Maciej W. Rozycki wrote:
> > On Sun, 17 Aug 2008, David Fries wrote:
> > > +	/* cr4 was introduced in the Pentium CPU */
> > 
> >  NACK.  Later i486 chips do have CR4 -- for PSE, VME, etc. (the set of
> > features varies across the line).  Use a fixup as elsewhere or something.

> 
> That's what I get for reading the Intel instruction set reference,
> "The CR4 register was added to the Intel Architecture beginning with
> the Pentium processor."
> 
> Ingo Molnar, thanks, I'll try the read_cr4_safe() version tonight (the
> computer is in the trunk of my car and I'm about ready to head to
> work).
> 
> In light of the above, how about updating the comments
> -       /* cr4 was introduced in the Pentium CPU */
> -       jecxz   1f      # cr4 Pentium and higher, skip if zero
> +       /* cr4 not in i386 only some i486, skip if zero */
> +       jecxz   1f      # cr4 not in i386 only some i486, skip if zero

Okay, can it happen that that cr4 is zero legitimately? If newer 486SX
chips support cr4 but not coprocessor...?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Fix i486 suspend to disk CR4 oops

[H. Peter Anvin]

Pavel Machek wrote:
> 
> Okay, can it happen that that cr4 is zero legitimately? If newer 486SX
> chips support cr4 but not coprocessor...?
> 									Pavel

Theoretically it can, but that means no features are enabled, so there 
is no need to enable the features.

The real question is if the following can happen: can it be such that we 
want CR4 to be zero in a situation where CR4 is nonzero to start out with?

The main bit in CR4 that could be set that we wouldn't want set would be 
CR4.PAE, so this could happen if there is a CPU with CR4.PAE but none of 
the other CR4 bits that we would normally set unconditionally.

I'm pretty sure this can't happen on any physical CPUs, since all 
physical CPUs supporting PAE would also support DE, MCE, and PGE.  It 
could possibly happen on a virtual CPU, although it is of course 
extremely unlikely we'd get there with CR4 not zero to start out with.

Still, it is at least theoretically wrong.

	-hpa

Re: [PATCH] i486 CR4 oops, no_console_suspend

[David Fries]

On Mon, Aug 18, 2008 at 08:41:20AM +0200, Ingo Molnar wrote:
> 
> applied to tip/x86/urgent, thanks David. I've changed the conditions to 
> read_cr4_safe() instead - that's cleaner. Could you please check whether 
> the patch below works fine too on your box?

Yes the 486 suspends and resumes with this patch.

Is there any known problem with no_console_suspend and serial
consoles?  It worked to print the oops for me to track this down, and
Documentation/kernel-parameters.txt says it is known to work with
serial consoles, but on resume only kernel messages can output to the
serial console.  The getty on the serial port can't raed or write and
processes trying to write to the port just hang without getting any
data across.  The serial port works fine across suspends without
the no_console_suspend argument.  Does anyone else see this?

-- 
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)

Re: [PATCH] i486 CR4 oops, no_console_suspend

[Ingo Molnar]

* David Fries <david@fries.net> wrote:

> On Mon, Aug 18, 2008 at 08:41:20AM +0200, Ingo Molnar wrote:
> > 
> > applied to tip/x86/urgent, thanks David. I've changed the conditions 
> > to read_cr4_safe() instead - that's cleaner. Could you please check 
> > whether the patch below works fine too on your box?
> 
> Yes the 486 suspends and resumes with this patch.

good - it's now upstream and should show up in 2.6.27-rc4 as well.

> Is there any known problem with no_console_suspend and serial 
> consoles?  It worked to print the oops for me to track this down, and 
> Documentation/kernel-parameters.txt says it is known to work with 
> serial consoles, but on resume only kernel messages can output to the 
> serial console.  The getty on the serial port can't raed or write and 
> processes trying to write to the port just hang without getting any 
> data across.  The serial port works fine across suspends without the 
> no_console_suspend argument.  Does anyone else see this?

i've had trouble no end with getting even kernel messages out to the 
serial console during critical phases of suspend/resume. (especially in 
combination with earlyprintk=ttyS0 - not surprisingly)

Especially during resume the UART is initialized back to something 
really slow - 300 bauds or 9600 bauds. (depends on the chipset i guess) 
So even though it works, it will only worked reliably when i 
standardized all my baud settings to that very low setting.

	Ingo

Re: [PATCH] i486 CR4 oops, no_console_suspend

[H. Peter Anvin]

Ingo Molnar wrote:
> 
> i've had trouble no end with getting even kernel messages out to the 
> serial console during critical phases of suspend/resume. (especially in 
> combination with earlyprintk=ttyS0 - not surprisingly)
> 
> Especially during resume the UART is initialized back to something 
> really slow - 300 bauds or 9600 bauds. (depends on the chipset i guess) 
> So even though it works, it will only worked reliably when i 
> standardized all my baud settings to that very low setting.
> 

Probably depends on the BIOS, actually; I suspect you end up with the 
standard BIOS setting, which is *usually* 9600 bps.

	-hpa

Re: [PATCH] i486 CR4 oops, no_console_suspend

[David Fries]

On Tue, Aug 19, 2008 at 09:07:26AM -0700, H. Peter Anvin wrote:
> Ingo Molnar wrote:
> >
> >i've had trouble no end with getting even kernel messages out to the 
> >serial console during critical phases of suspend/resume. (especially in 
> >combination with earlyprintk=ttyS0 - not surprisingly)
> >
> >Especially during resume the UART is initialized back to something 
> >really slow - 300 bauds or 9600 bauds. (depends on the chipset i guess) 
> >So even though it works, it will only worked reliably when i 
> >standardized all my baud settings to that very low setting.
> >
> 
> Probably depends on the BIOS, actually; I suspect you end up with the 
> standard BIOS setting, which is *usually* 9600 bps.

I have grub, the kernel, and a getty all set to 115200, for suspend,
resume, or reboot it's always 115200.  With grub there's no BIOS
guessing, it sets it to a configured state.

-- 
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)

Re: [PATCH] i486 CR4 oops, no_console_suspend

[H. Peter Anvin]

David Fries wrote:
> On Tue, Aug 19, 2008 at 09:07:26AM -0700, H. Peter Anvin wrote:
>> Ingo Molnar wrote:
>>> i've had trouble no end with getting even kernel messages out to the 
>>> serial console during critical phases of suspend/resume. (especially in 
>>> combination with earlyprintk=ttyS0 - not surprisingly)
>>>
>>> Especially during resume the UART is initialized back to something 
>>> really slow - 300 bauds or 9600 bauds. (depends on the chipset i guess) 
>>> So even though it works, it will only worked reliably when i 
>>> standardized all my baud settings to that very low setting.
>>>
>> Probably depends on the BIOS, actually; I suspect you end up with the 
>> standard BIOS setting, which is *usually* 9600 bps.
> 
> I have grub, the kernel, and a getty all set to 115200, for suspend,
> resume, or reboot it's always 115200.  With grub there's no BIOS
> guessing, it sets it to a configured state.
> 

Most likely it never gets reinitialized during resume.  This is a bug, 
obviously.

	-hpa