[RFC][PATCH] powerpc/64s: Leave IRQs hard enabled over context switch

[RFC][PATCH] powerpc/64s: Leave IRQs hard enabled over context switch

[Nicholas Piggin]

Commit 4387e9ff25 ("[POWERPC] Fix PMU + soft interrupt disable bug")
hard disabled interrupts over the low level context switch, because
the SLB management can't cope with a PMU interrupt accesing the stack
in that window.

Radix based kernel mapping does not use the SLB so it does not require
interrupts disabled here. This is worth a % or so in context switch
performance, and also allows the low level context switch code to be
profiled.

Extending the soft IRQ disable to cover PMU interrupts will allow this
hard disable to be removed from hash based kernels too, but they will
still have to soft-disable PMU interrupts.

- Q1: Can we do this? It gives nice profiles of context switch code
  rather than assigning it all to local_irq_enable.

- Q2: What is the unrecoverable SLB miss on exception entry? Is there
  anywhere we access the kernel stack with RI disabled? Something else?

---
 arch/powerpc/kernel/process.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d645da302bf2..915ec20a18a9 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1176,12 +1176,14 @@ struct task_struct *__switch_to(struct task_struct *prev,
 
 	__switch_to_tm(prev, new);
 
-	/*
-	 * We can't take a PMU exception inside _switch() since there is a
-	 * window where the kernel stack SLB and the kernel stack are out
-	 * of sync. Hard disable here.
-	 */
-	hard_irq_disable();
+	if (!radix_enabled()) {
+		/*
+		 * We can't take a PMU exception inside _switch() since there
+		 * is a window where the kernel stack SLB and the kernel stack
+		 * are out of sync. Hard disable here.
+		 */
+		hard_irq_disable();
+	}
 
 	/*
 	 * Call restore_sprs() before calling _switch(). If we move it after
-- 
2.11.0

Re: [RFC][PATCH] powerpc/64s: Leave IRQs hard enabled over context switch

[Benjamin Herrenschmidt]

On Wed, 2017-05-03 at 17:34 +1000, Nicholas Piggin wrote:
> Extending the soft IRQ disable to cover PMU interrupts will allow this
> hard disable to be removed from hash based kernels too, but they will
> still have to soft-disable PMU interrupts.
> 
> - Q1: Can we do this? It gives nice profiles of context switch code
>   rather than assigning it all to local_irq_enable.

Probably ok with radix yes.

> - Q2: What is the unrecoverable SLB miss on exception entry? Is there
>   anywhere we access the kernel stack with RI disabled? Something else?

Not sure what you mean by Q2, but the original problem is an occurrence
of what we call the 'megabug' which hit us in different forms over the
years, and happens when we get a kernel stack SLB entry wrong.

Normally, the segment containing the current kernel stack is always
bolted in the SLB in a specific slot. If we accidentally lose that
"bolt", we can end up faulting it into the wrong slot, thus making it
possible for it to get evicted later on etc...

That in turns hits the exception return path which accesses the kernel
stack after clearing RI and setting SRR0/1 to the return values.

Cheers,
Ben.

Re: [RFC][PATCH] powerpc/64s: Leave IRQs hard enabled over context switch

[Nicholas Piggin]

On Wed, 03 May 2017 10:28:27 +0200
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Wed, 2017-05-03 at 17:34 +1000, Nicholas Piggin wrote:
> > Extending the soft IRQ disable to cover PMU interrupts will allow this
> > hard disable to be removed from hash based kernels too, but they will
> > still have to soft-disable PMU interrupts.
> > 
> > - Q1: Can we do this? It gives nice profiles of context switch code
> >   rather than assigning it all to local_irq_enable.  
> 
> Probably ok with radix yes.

Cool.

> > - Q2: What is the unrecoverable SLB miss on exception entry? Is there
> >   anywhere we access the kernel stack with RI disabled? Something else?  
> 
> Not sure what you mean by Q2, but the original problem is an occurrence
> of what we call the 'megabug' which hit us in different forms over the
> years, and happens when we get a kernel stack SLB entry wrong.
> 
> Normally, the segment containing the current kernel stack is always
> bolted in the SLB in a specific slot. If we accidentally lose that
> "bolt", we can end up faulting it into the wrong slot, thus making it
> possible for it to get evicted later on etc...
> 
> That in turns hits the exception return path which accesses the kernel
> stack after clearing RI and setting SRR0/1 to the return values.

This is exactly my question. The original patch said there was an
unrecoverable SLB miss at exception entry. Either I missed that or
it has since been removed. The stack access at exit would do it. I
didn't want to change anything for hash, just wondering where the
bug was (subject line got truncated but is supposed to say "for radix").

Thanks,
Nick

Re: [RFC][PATCH] powerpc/64s: Leave IRQs hard enabled over context switch

[Michael Ellerman]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Wed, 2017-05-03 at 17:34 +1000, Nicholas Piggin wrote:
>> Extending the soft IRQ disable to cover PMU interrupts will allow this
>> hard disable to be removed from hash based kernels too, but they will
>> still have to soft-disable PMU interrupts.
>>=20
>> - Q1: Can we do this? It gives nice profiles of context switch code
>> =C2=A0 rather than assigning it all to local_irq_enable.
>
> Probably ok with radix yes.
>
>> - Q2: What is the unrecoverable SLB miss on exception entry? Is there
>> =C2=A0 anywhere we access the kernel stack with RI disabled? Something e=
lse?
>
> Not sure what you mean by Q2, but the original problem is an occurrence
> of what we call the 'megabug' which hit us in different forms over the
> years, and happens when we get a kernel stack SLB entry wrong.
>
> Normally, the segment containing the current kernel stack is always
> bolted in the SLB in a specific slot. If we accidentally lose that
> "bolt", we can end up faulting it into the wrong slot, thus making it
> possible for it to get evicted later on etc...
>
> That in turns hits the exception return path which accesses the kernel
> stack after clearing RI and setting SRR0/1 to the return values.

Couldn't we avoid the whole problem by just having two bolted slots for
the stack, meaning we could have the current and next stack bolted at
all times.

That would mean we'd be using 4 slots for bolted entries, which is one
more than 3 - but it might be a good trade off.

Or we could make the SLB insertion algorithm smarter so that we could
later free the slot that was used for previous kernel stack.

cheers

Re: [RFC][PATCH] powerpc/64s: Leave IRQs hard enabled over context switch

[Benjamin Herrenschmidt]

On Wed, 2017-05-03 at 20:26 +1000, Michael Ellerman wrote:
> Couldn't we avoid the whole problem by just having two bolted slots for
> the stack, meaning we could have the current and next stack bolted at
> all times.
> 
> That would mean we'd be using 4 slots for bolted entries, which is one
> more than 3 - but it might be a good trade off.
> 
> Or we could make the SLB insertion algorithm smarter so that we could
> later free the slot that was used for previous kernel stack.

Changing the insertion algo is a bit nasty, it's nice and simple as-is,
an option would be to replace the bolted "threshold" by a bolted "map"
with a bit (or a byte) per entry.

Sadly, we have no way that I know of to search an slb entry that tells
you in which slot it is, do we ? So even if the "new" stack is already
somewhere in there, we can't know it and just "mark it bolted". We'd
still have to invalidate just in case before bolting it in.

Ben.