Message ID | fb412d760810301104s228a6036s50adba8605a13a98@mail.gmail.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Delegated to: | Josh Boyer |
Headers | show |
On Thu, 2008-10-30 at 13:04 -0500, Hollis Blanchard wrote: > > I don't think it's necessary at all to disable ME/CE/DE inside > _tlbie() on 440, because the interrupt handlers for those types save > and restore MMUCR (they're all the same code path; see > mcheck_transfer_to_handler in entry_32.S). This was written before the saving of MMUCR was added I think. > However, I think EE does need to be disabled, since the normal EE > handler doesn't deal with MMUCR. So instead of all these MSR > manipulations, I think a simple wrteei 0/1 pair should do the trick? > Or maybe mfmsr/wrteei/wrtee, in case _tlbie() happens to be called > with interrupts disabled already. Yes. Ben.
On Fri, 31 Oct 2008 07:14:00 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Thu, 2008-10-30 at 13:04 -0500, Hollis Blanchard wrote: > > > > I don't think it's necessary at all to disable ME/CE/DE inside > > _tlbie() on 440, because the interrupt handlers for those types save > > and restore MMUCR (they're all the same code path; see > > mcheck_transfer_to_handler in entry_32.S). > > This was written before the saving of MMUCR was added I think. I was thinking that but git was being annoying. > > However, I think EE does need to be disabled, since the normal EE > > handler doesn't deal with MMUCR. So instead of all these MSR > > manipulations, I think a simple wrteei 0/1 pair should do the trick? > > Or maybe mfmsr/wrteei/wrtee, in case _tlbie() happens to be called > > with interrupts disabled already. > > Yes. Agreed. Hollis and I had this discussion on IRC and I pointed out that the patch originally just started with wrteei's. (And aren't you supposed to be on vacation...) josh
On Oct 30, 2008, at 3:36 PM, Josh Boyer wrote: > > (And aren't you supposed to be on vacation...) There is no vacation from the community.. you will be assimilated. :) - k
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S index e708ab7..8533de5 100644 --- a/arch/powerpc/kernel/misc_32.S +++ b/arch/powerpc/kernel/misc_32.S @@ -301,9 +301,19 @@ _GLOBAL(_tlbie) mfspr r4,SPRN_MMUCR mfspr r5,SPRN_PID /* Get PID */ rlwimi r4,r5,0,24,31 /* Set TID */ - mtspr SPRN_MMUCR,r4 + /* We have to run the search with interrupts disabled, even critical + * and debug interrupts (in fact the only critical exceptions we have + * are debug and machine check). Otherwise an interrupt which causes + * a TLB miss can clobber the MMUCR between the mtspr and the tlbsx. */ + mfmsr r5 + lis r6,(MSR_EE|MSR_CE|MSR_ME|MSR_DE)@ha + addi r6,r6,(MSR_EE|MSR_CE|MSR_ME|MSR_DE)@l + andc r6,r5,r6 + mtmsr r6 + mtspr SPRN_MMUCR,r4 tlbsx. r3, 0, r3 + mtmsr r5 bne 10f sync /* There are only 64 TLB entries, so r3 < 64, diff --git a/arch/ppc/kernel/misc.S b/arch/ppc/kernel/misc.S index 0da5536..a22e1f4 100644 --- a/arch/ppc/kernel/misc.S +++ b/arch/ppc/kernel/misc.S @@ -237,9 +237,19 @@ _GLOBAL(_tlbie) mfspr r4,SPRN_MMUCR mfspr r5,SPRN_PID /* Get PID */ rlwimi r4,r5,0,24,31 /* Set TID */ - mtspr SPRN_MMUCR,r4 + /* We have to run the search with interrupts disabled, even critical + * and debug interrupts (in fact the only critical exceptions we have + * are debug and machine check). Otherwise an interrupt which causes + * a TLB miss can clobber the MMUCR between the mtspr and the tlbsx. */ + mfmsr r5 + lis r6,(MSR_EE|MSR_CE|MSR_ME|MSR_DE)@ha + addi r6,r6,(MSR_EE|MSR_CE|MSR_ME|MSR_DE)@l + andc r6,r5,r6 + mtmsr r6 + mtspr SPRN_MMUCR,r4 tlbsx. r3, 0, r3 + mtmsr r5 bne 10f sync /* There are only 64 TLB entries, so r3 < 64,