diff mbox series

MSR_FE mask change broke KVM PR MacOS guest

Message ID 9a0d1b68-47fb-70bc-7fb4-521d0179da03@ilande.co.uk (mailing list archive)
State Superseded
Headers show
Series MSR_FE mask change broke KVM PR MacOS guest | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch warning next/apply_patch Patch failed to apply
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch

Commit Message

Mark Cave-Ayland Jan. 25, 2019, 11:42 a.m. UTC 
Hi all,

Referencing my bug report over on the kvm-ppc list at
https://www.spinics.net/lists/kvm-ppc/msg14971.html, I've been experiencing a hard
lockup and panic on a G4 Mac Mini when trying run MacOS X under KVM PR which I've
bisected down to the following commit:


$ git bisect bad
8792468da5e12e77e76e1edf081acf0392abb331 is the first bad commit
commit 8792468da5e12e77e76e1edf081acf0392abb331
Author: Cyril Bur <cyrilbur@gmail.com>
Date:   Mon Feb 29 17:53:49 2016 +1100

    powerpc: Add the ability to save FPU without giving it up

    This patch adds the ability to be able to save the FPU registers to the
    thread struct without giving up (disabling the facility) next time the
    process returns to userspace.

    This patch optimises the thread copy path (as a result of a fork() or
    clone()) so that the parent thread can return to userspace with hot
    registers avoiding a possibly pointless reload of FPU register state.

    Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>


Working through the changes which can be found at
https://github.com/torvalds/linux/commit/8792468da5e12e77e76e1edf081acf0392abb331,
I've discovered that the lockup is being caused by the removal of the the MSR_FE0 and
MSR_FE1 masks from the previous task during giveup_fpu().

Before this patch:


_GLOBAL(giveup_fpu)
        mfmsr   r5
        ori     r5,r5,MSR_FP
#ifdef CONFIG_VSX
BEGIN_FTR_SECTION
        oris    r5,r5,MSR_VSX@h
END_FTR_SECTION_IFSET(CPU_FTR_VSX)
#endif
        SYNC_601
        ISYNC_601
        MTMSRD(r5)                      /* enable use of fpu now */
        SYNC_601
        isync
        PPC_LCMPI       0,r3,0
        beqlr-                          /* if no previous owner, done */
        addi    r3,r3,THREAD            /* want THREAD of task */
        PPC_LL  r6,THREAD_FPSAVEAREA(r3)
        PPC_LL  r5,PT_REGS(r3)
        PPC_LCMPI       0,r6,0
        bne     2f
        addi    r6,r3,THREAD_FPSTATE
2:      PPC_LCMPI       0,r5,0
        SAVE_32FPVSRS(0, R4, R6)
        mffs    fr0
        stfd    fr0,FPSTATE_FPSCR(r6)
        beq     1f
        PPC_LL  r4,_MSR-STACK_FRAME_OVERHEAD(r5)
        li      r3,MSR_FP|MSR_FE0|MSR_FE1

                ^^^^^^^^^^^^^^^^^^^^^^^^^
#ifdef CONFIG_VSX
BEGIN_FTR_SECTION
        oris    r3,r3,MSR_VSX@h
END_FTR_SECTION_IFSET(CPU_FTR_VSX)
#endif
        andc    r4,r4,r3                /* disable FP for previous task */


and after the patch:

void __giveup_fpu(struct task_struct *tsk)
{
	save_fpu(tsk);
	tsk->thread.regs->msr &= ~MSR_FP;
                                 ^^^^^^^

#ifdef CONFIG_VSX
	if (cpu_has_feature(CPU_FTR_VSX))
		tsk->thread.regs->msr &= ~MSR_VSX;
#endif
}


Reinstating the MSR_FE0 and MSR_FE1 bitmasks fixes the issue allowing MacOS X to boot
without incident again:




Would the above be accepted as a patch? The commit message for the patch above fails
to mention why the MSR_FE* constants were removed from the MSR bitmask, so I'm not
sure whether this was deliberate? Certainly running with the above patch applied, I
haven't noticed anything obviously broken. And could this patch cause any ill-effects
on 64-bit PPC CPUs?


ATB,

Mark.
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 14c09d25de98..f9ac44621dda 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -136,7 +136,7 @@  EXPORT_SYMBOL(__msr_check_and_clear);
 void __giveup_fpu(struct task_struct *tsk)
 {
        save_fpu(tsk);
-       tsk->thread.regs->msr &= ~MSR_FP;
+       tsk->thread.regs->msr &= ~(MSR_FP|MSR_FE0|MSR_FE1);
 #ifdef CONFIG_VSX
        if (cpu_has_feature(CPU_FTR_VSX))
                tsk->thread.regs->msr &= ~MSR_VSX;