From: David Gibson <hidden> Date: 2007-08-07 04:20:50
The 440 family of processors don't have a tlbie instruction. So, we
implement TLB invalidates by explicitly searching the TLB with tlbsx.,
then clobbering the relevant entry, if any. Unfortunately the PID for
the search needs to be stored in the MMUCR register, which is also
used by the TLB miss handler. Interrupts were enabled in _tlbie(), so
an interrupt between loading the MMUCR and the tlbsx could cause
incorrect search results, and thus a failure to invalide TLB entries
which needed to be invalidated.
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
Signed-off-by: David Gibson <redacted>
---
Paul, this one's a bugfix, which I think should go into 2.6.23.
Index: working-2.6/arch/powerpc/kernel/misc_32.S
===================================================================
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Tue, 7 Aug 2007 14:20:50 +1000
David Gibson [off-list ref] wrote:
The 440 family of processors don't have a tlbie instruction. So, we
implement TLB invalidates by explicitly searching the TLB with tlbsx.,
then clobbering the relevant entry, if any. Unfortunately the PID for
the search needs to be stored in the MMUCR register, which is also
used by the TLB miss handler. Interrupts were enabled in _tlbie(), so
an interrupt between loading the MMUCR and the tlbsx could cause
incorrect search results, and thus a failure to invalide TLB entries
which needed to be invalidated.
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
Signed-off-by: David Gibson <redacted>
Acked-by: Josh Boyer <redacted>
And I agree this should go into 2.6.23.
josh
From: Kumar Gala <hidden> Date: 2007-08-08 15:20:45
On Aug 6, 2007, at 11:20 PM, David Gibson wrote:
The 440 family of processors don't have a tlbie instruction. So, we
implement TLB invalidates by explicitly searching the TLB with tlbsx.,
then clobbering the relevant entry, if any. Unfortunately the PID for
the search needs to be stored in the MMUCR register, which is also
used by the TLB miss handler. Interrupts were enabled in _tlbie(), so
an interrupt between loading the MMUCR and the tlbsx could cause
incorrect search results, and thus a failure to invalide TLB entries
which needed to be invalidated.
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
Signed-off-by: David Gibson <redacted>
---
Paul, this one's a bugfix, which I think should go into 2.6.23.
On Wed, 8 Aug 2007 10:20:45 -0500
Kumar Gala [off-list ref] wrote:
On Aug 6, 2007, at 11:20 PM, David Gibson wrote:
quoted
The 440 family of processors don't have a tlbie instruction. So, we
implement TLB invalidates by explicitly searching the TLB with tlbsx.,
then clobbering the relevant entry, if any. Unfortunately the PID for
the search needs to be stored in the MMUCR register, which is also
used by the TLB miss handler. Interrupts were enabled in _tlbie(), so
an interrupt between loading the MMUCR and the tlbsx could cause
incorrect search results, and thus a failure to invalide TLB entries
which needed to be invalidated.
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
Signed-off-by: David Gibson <redacted>
---
Paul, this one's a bugfix, which I think should go into 2.6.23.
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
--
Hollis Blanchard
IBM Linux Technology Center
On Wed, 8 Aug 2007 20:43:25 +0000 (UTC)
Hollis Blanchard [off-list ref] wrote:
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
quoted
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
Interrupts from UICs can be configured as critical. If one of those
triggers, (or any other CE triggers) and causes a tlb miss, you have a
race. The watchdog timer interrupt also is a CE IIRC.
CE and DE are admittedly a much smaller race, but still possible.
Masking EE off is the largest one.
josh
On Wed, 2007-08-08 at 16:29 -0500, Josh Boyer wrote:
On Wed, 8 Aug 2007 20:43:25 +0000 (UTC)
Hollis Blanchard [off-list ref] wrote:
quoted
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
quoted
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
Interrupts from UICs can be configured as critical. If one of those
triggers, (or any other CE triggers) and causes a tlb miss, you have a
race. The watchdog timer interrupt also is a CE IIRC.
By "causes a tlb miss", you mean the interrupt handler associated with
the critical-priority UIC interrupt performs MMIO which causes a TLB
miss? Regular code couldn't cause a TLB miss AFAICS, since the kernel is
always mapped, and an interrupt handler doesn't access userspace.
--
Hollis Blanchard
IBM Linux Technology Center
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2007-08-08 23:01:35
On Wed, 2007-08-08 at 16:29 -0500, Josh Boyer wrote:
On Wed, 8 Aug 2007 20:43:25 +0000 (UTC)
Hollis Blanchard [off-list ref] wrote:
quoted
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
quoted
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
Interrupts from UICs can be configured as critical. If one of those
triggers, (or any other CE triggers) and causes a tlb miss, you have a
race. The watchdog timer interrupt also is a CE IIRC.
CE and DE are admittedly a much smaller race, but still possible.
Masking EE off is the largest one.
There is a much bigger problem if CEs can do tlb misses though... they
can interrupt the tlb miss handler itself, either between the two halves
of a tlb write, or between the write to MMUCR and the write to the tlb,
and I suspect both cases will cause trouble.
We might want to check if we were in the TLB miss handler upon return
from the CE and MCE handlers, and in this case, restart them (just
return to the faulting instruction, that is use srr0 instead of
csrr0/mcsrr0).
Ben.
On Wed, Aug 08, 2007 at 05:11:09PM -0500, Hollis Blanchard wrote:
On Wed, 2007-08-08 at 16:29 -0500, Josh Boyer wrote:
quoted
On Wed, 8 Aug 2007 20:43:25 +0000 (UTC)
Hollis Blanchard [off-list ref] wrote:
quoted
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
quoted
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
Interrupts from UICs can be configured as critical. If one of those
triggers, (or any other CE triggers) and causes a tlb miss, you have a
race. The watchdog timer interrupt also is a CE IIRC.
By "causes a tlb miss", you mean the interrupt handler associated with
the critical-priority UIC interrupt performs MMIO which causes a TLB
miss? Regular code couldn't cause a TLB miss AFAICS, since the kernel is
always mapped, and an interrupt handler doesn't access userspace.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2007-08-08 23:30:22
On Wed, 2007-08-08 at 17:11 -0500, Hollis Blanchard wrote:
On Wed, 2007-08-08 at 16:29 -0500, Josh Boyer wrote:
quoted
On Wed, 8 Aug 2007 20:43:25 +0000 (UTC)
Hollis Blanchard [off-list ref] wrote:
quoted
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
quoted
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
Interrupts from UICs can be configured as critical. If one of those
triggers, (or any other CE triggers) and causes a tlb miss, you have a
race. The watchdog timer interrupt also is a CE IIRC.
By "causes a tlb miss", you mean the interrupt handler associated with
the critical-priority UIC interrupt performs MMIO which causes a TLB
miss? Regular code couldn't cause a TLB miss AFAICS, since the kernel is
always mapped, and an interrupt handler doesn't access userspace.
ioremap is an example, vmalloc space is another...
Ben.
On Thu, Aug 09, 2007 at 09:01:29AM +1000, Benjamin Herrenschmidt wrote:
On Wed, 2007-08-08 at 16:29 -0500, Josh Boyer wrote:
quoted
On Wed, 8 Aug 2007 20:43:25 +0000 (UTC)
Hollis Blanchard [off-list ref] wrote:
quoted
On Tue, 07 Aug 2007 14:20:50 +1000, David Gibson wrote:
quoted
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across the
relevant instructions.
How could a critical or debug interrupt modify the contents of MMUCR?
Interrupts from UICs can be configured as critical. If one of those
triggers, (or any other CE triggers) and causes a tlb miss, you have a
race. The watchdog timer interrupt also is a CE IIRC.
CE and DE are admittedly a much smaller race, but still possible.
Masking EE off is the largest one.
There is a much bigger problem if CEs can do tlb misses though... they
can interrupt the tlb miss handler itself, either between the two halves
of a tlb write, or between the write to MMUCR and the write to the tlb,
and I suspect both cases will cause trouble.
Yes.
We might want to check if we were in the TLB miss handler upon return
from the CE and MCE handlers, and in this case, restart them (just
return to the faulting instruction, that is use srr0 instead of
csrr0/mcsrr0).
From: Kumar Gala <hidden> Date: 2007-08-09 05:27:21
On Aug 8, 2007, at 11:00 AM, Josh Boyer wrote:
On Wed, 8 Aug 2007 10:20:45 -0500
Kumar Gala [off-list ref] wrote:
quoted
On Aug 6, 2007, at 11:20 PM, David Gibson wrote:
quoted
The 440 family of processors don't have a tlbie instruction. So, we
implement TLB invalidates by explicitly searching the TLB with
tlbsx.,
then clobbering the relevant entry, if any. Unfortunately the
PID for
the search needs to be stored in the MMUCR register, which is also
used by the TLB miss handler. Interrupts were enabled in _tlbie
(), so
an interrupt between loading the MMUCR and the tlbsx could cause
incorrect search results, and thus a failure to invalide TLB entries
which needed to be invalidated.
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across
the
relevant instructions.
Signed-off-by: David Gibson <redacted>
---
Paul, this one's a bugfix, which I think should go into 2.6.23.
Did you actually see this happen?
Yes.
When?
We don't have critical wired to anything, I don't expect watchdog to
cause another fault.. so just wondering.
- k
From: David Gibson <hidden> Date: 2007-08-09 05:39:30
On Thu, Aug 09, 2007 at 12:28:20AM -0500, Kumar Gala wrote:
On Aug 8, 2007, at 11:00 AM, Josh Boyer wrote:
quoted
On Wed, 8 Aug 2007 10:20:45 -0500
Kumar Gala [off-list ref] wrote:
quoted
On Aug 6, 2007, at 11:20 PM, David Gibson wrote:
quoted
The 440 family of processors don't have a tlbie instruction. So, we
implement TLB invalidates by explicitly searching the TLB with
tlbsx.,
then clobbering the relevant entry, if any. Unfortunately the
PID for
the search needs to be stored in the MMUCR register, which is also
used by the TLB miss handler. Interrupts were enabled in _tlbie
(), so
an interrupt between loading the MMUCR and the tlbsx could cause
incorrect search results, and thus a failure to invalide TLB entries
which needed to be invalidated.
This patch fixes the problem in both arch/ppc and arch/powerpc by
inhibiting interrupts (even critical and debug interrupts) across
the
relevant instructions.
Signed-off-by: David Gibson <redacted>
---
Paul, this one's a bugfix, which I think should go into 2.6.23.
Did you actually see this happen?
Yes.
When?
We don't have critical wired to anything, I don't expect watchdog to
cause another fault.. so just wondering.
On debug (trace) interrupts on blue gene.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Thu, 09 Aug 2007 23:05:36 +1000
Benjamin Herrenschmidt [off-list ref] wrote:
On Thu, 2007-08-09 at 07:04 -0500, Josh Boyer wrote:
quoted
quoted
We don't have critical wired to anything, I don't expect watchdog
to
quoted
cause another fault.. so just wondering.
We being who? I'm slightly confused here.
I think Kumar doesn't know that we are talking about the BG kernel
which has more things "wired" to CRIT than what is upstream at the
moment :-)
Ah, sure. But even though we don't have much upstream that uses CE,
that doesn't mean someone can't reprogram the UICs on their boards to
use CE for some things, for example. I know of at least one project
that has done that in the past.
josh