From: Ingo Molnar <mingo@elte.hu>
To: David Fries <david@fries.net>,
"Maciej W. Rozycki" <macro@linux-mips.org>
Cc: linux-kernel@vger.kernel.org, Pavel Machek <pavel@ucw.cz>,
"H. Peter Anvin" <hpa@zytor.com>,
Thomas Gleixner <tglx@linutronix.de>,
"Rafael J. Wysocki" <rjw@sisk.pl>
Subject: Re: [PATCH] Fix i486 suspend to disk CR4 oops
Date: Mon, 18 Aug 2008 15:25:59 +0200 [thread overview]
Message-ID: <20080818132559.GA19578@elte.hu> (raw)
In-Reply-To: <20080818125803.GC17528@spacedout.fries.net>
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
next prev parent reply other threads:[~2008-08-18 13:26 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-18 4:03 [PATCH] Fix i486 suspend to disk CR4 oops David Fries
2008-08-18 4:14 ` Maciej W. Rozycki
2008-08-18 4:35 ` H. Peter Anvin
2008-08-18 6:04 ` Andi Kleen
2008-08-18 6:34 ` H. Peter Anvin
2008-08-18 6:42 ` Andi Kleen
2008-08-18 6:41 ` Ingo Molnar
2008-08-18 6:45 ` H. Peter Anvin
2008-08-18 9:15 ` Pavel Machek
2008-08-18 10:16 ` Rafael J. Wysocki
2008-08-18 12:58 ` David Fries
2008-08-18 13:25 ` Ingo Molnar [this message]
2008-08-18 14:41 ` Maciej W. Rozycki
2008-08-18 14:38 ` Maciej W. Rozycki
2008-08-18 22:04 ` Pavel Machek
2008-08-18 22:10 ` H. Peter Anvin
2008-08-18 15:24 ` Dave Jones
2008-08-18 16:04 ` Lennart Sorensen
2008-08-18 17:17 ` Dave Jones
2008-08-18 17:32 ` H. Peter Anvin
2008-08-18 22:02 ` Pavel Machek
2008-08-19 3:37 ` [PATCH] i486 CR4 oops, no_console_suspend David Fries
2008-08-19 9:34 ` Ingo Molnar
2008-08-19 16:07 ` H. Peter Anvin
2008-08-21 4:17 ` David Fries
2008-08-21 5:37 ` H. Peter Anvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080818132559.GA19578@elte.hu \
--to=mingo@elte.hu \
--cc=david@fries.net \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=macro@linux-mips.org \
--cc=pavel@ucw.cz \
--cc=rjw@sisk.pl \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).