Skip to content

Commit be2cf20

Browse files
committed
powerpc: More fixes for lazy IRQ vs. idle
Looks like we still have issues with pSeries and Cell idle code vs. the lazy irq state. In fact, the reset fixes that went upstream are exposing the problem more by causing BUG_ON() to trigger (which this patch turns into a WARN_ON instead). We need to be careful when using a variant of low power state that has the side effect of turning interrupts back on, to properly set all the SW & lazy state to look as if everything is enabled before we enter the low power state with MSR:EE off as we will return with MSR:EE on. If not, we have a discrepancy of state which can cause things to go very wrong later on. This patch moves the logic into a helper and uses it from the pseries and cell idle code. The power4/970 idle code already got things right (in assembly even !) so I'm not touching it. The power7 "bare metal" idle code is subtly different and correct. Remains PA6T and some hypervisor based Cell platforms which have questionable code in there, but they are mostly dead platforms so I'll fix them when I manage to get final answers from the respective maintainers about how the low power state actually works on them. Signed-off-by: Benjamin Herrenschmidt <[email protected]> CC: [email protected] [v3.4]
1 parent 2437fcc commit be2cf20

File tree

4 files changed

+64
-12
lines changed

4 files changed

+64
-12
lines changed

arch/powerpc/include/asm/hw_irq.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
125125
return !regs->softe;
126126
}
127127

128+
extern bool prep_irq_for_idle(void);
129+
128130
#else /* CONFIG_PPC64 */
129131

130132
#define SET_MSR_EE(x) mtmsr(x)

arch/powerpc/kernel/irq.c

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,52 @@ void notrace restore_interrupts(void)
286286
__hard_irq_enable();
287287
}
288288

289+
/*
290+
* This is a helper to use when about to go into idle low-power
291+
* when the latter has the side effect of re-enabling interrupts
292+
* (such as calling H_CEDE under pHyp).
293+
*
294+
* You call this function with interrupts soft-disabled (this is
295+
* already the case when ppc_md.power_save is called). The function
296+
* will return whether to enter power save or just return.
297+
*
298+
* In the former case, it will have notified lockdep of interrupts
299+
* being re-enabled and generally sanitized the lazy irq state,
300+
* and in the latter case it will leave with interrupts hard
301+
* disabled and marked as such, so the local_irq_enable() call
302+
* in cpu_idle() will properly re-enable everything.
303+
*/
304+
bool prep_irq_for_idle(void)
305+
{
306+
/*
307+
* First we need to hard disable to ensure no interrupt
308+
* occurs before we effectively enter the low power state
309+
*/
310+
hard_irq_disable();
311+
312+
/*
313+
* If anything happened while we were soft-disabled,
314+
* we return now and do not enter the low power state.
315+
*/
316+
if (lazy_irq_pending())
317+
return false;
318+
319+
/* Tell lockdep we are about to re-enable */
320+
trace_hardirqs_on();
321+
322+
/*
323+
* Mark interrupts as soft-enabled and clear the
324+
* PACA_IRQ_HARD_DIS from the pending mask since we
325+
* are about to hard enable as well as a side effect
326+
* of entering the low power state.
327+
*/
328+
local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
329+
local_paca->soft_enabled = 1;
330+
331+
/* Tell the caller to enter the low power state */
332+
return true;
333+
}
334+
289335
#endif /* CONFIG_PPC64 */
290336

291337
int arch_show_interrupts(struct seq_file *p, int prec)

arch/powerpc/platforms/cell/pervasive.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,9 @@ static void cbe_power_save(void)
4242
{
4343
unsigned long ctrl, thread_switch_control;
4444

45-
/*
46-
* We need to hard disable interrupts, the local_irq_enable() done by
47-
* our caller upon return will hard re-enable.
48-
*/
49-
hard_irq_disable();
45+
/* Ensure our interrupt state is properly tracked */
46+
if (!prep_irq_for_idle())
47+
return;
5048

5149
ctrl = mfspr(SPRN_CTRLF);
5250

@@ -81,6 +79,9 @@ static void cbe_power_save(void)
8179
*/
8280
ctrl &= ~(CTRL_RUNLATCH | CTRL_TE);
8381
mtspr(SPRN_CTRLT, ctrl);
82+
83+
/* Re-enable interrupts in MSR */
84+
__hard_irq_enable();
8485
}
8586

8687
static int cbe_system_reset_exception(struct pt_regs *regs)

arch/powerpc/platforms/pseries/processor_idle.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,18 @@ static int snooze_loop(struct cpuidle_device *dev,
9999
static void check_and_cede_processor(void)
100100
{
101101
/*
102-
* Interrupts are soft-disabled at this point,
103-
* but not hard disabled. So an interrupt might have
104-
* occurred before entering NAP, and would be potentially
105-
* lost (edge events, decrementer events, etc...) unless
106-
* we first hard disable then check.
102+
* Ensure our interrupt state is properly tracked,
103+
* also checks if no interrupt has occurred while we
104+
* were soft-disabled
107105
*/
108-
hard_irq_disable();
109-
if (!lazy_irq_pending())
106+
if (prep_irq_for_idle()) {
110107
cede_processor();
108+
#ifdef CONFIG_TRACE_IRQFLAGS
109+
/* Ensure that H_CEDE returns with IRQs on */
110+
if (WARN_ON(!(mfmsr() & MSR_EE)))
111+
__hard_irq_enable();
112+
#endif
113+
}
111114
}
112115

113116
static int dedicated_cede_loop(struct cpuidle_device *dev,

0 commit comments

Comments
 (0)