lkml.org 
[lkml]   [2008]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] Fix i486 suspend to disk CR4 oops

    * 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

    \
     
     \ /
      Last update: 2008-08-18 08:45    [W:3.215 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site