From ef2b343e99e772e35f0f9d00f7db318b6629c16e Mon Sep 17 00:00:00 2001 From: Hugh Dickins Date: Fri, 10 Nov 2006 21:32:40 +0000 Subject: [PATCH] [POWERPC] Make soft_enabled irqs preempt safe Rewrite local_get_flags and local_irq_disable to use r13 explicitly, to avoid the risk that gcc will split get_paca()->soft_enabled into a sequence unsafe against preemption. Similar care in local_irq_restore. Signed-off-by: Hugh Dickins Signed-off-by: Paul Mackerras --- arch/powerpc/kernel/irq.c | 57 ++++++++++++++++++++++++++++++++---- include/asm-powerpc/hw_irq.h | 20 +++++++++---- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index e1936952017c..0bd8c7665834 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c @@ -97,22 +97,69 @@ EXPORT_SYMBOL(irq_desc); int distribute_irqs = 1; +static inline unsigned long get_hard_enabled(void) +{ + unsigned long enabled; + + __asm__ __volatile__("lbz %0,%1(13)" + : "=r" (enabled) : "i" (offsetof(struct paca_struct, hard_enabled))); + + return enabled; +} + +static inline void set_soft_enabled(unsigned long enable) +{ + __asm__ __volatile__("stb %0,%1(13)" + : : "r" (enable), "i" (offsetof(struct paca_struct, soft_enabled))); +} + void local_irq_restore(unsigned long en) { - get_paca()->soft_enabled = en; + /* + * get_paca()->soft_enabled = en; + * Is it ever valid to use local_irq_restore(0) when soft_enabled is 1? + * That was allowed before, and in such a case we do need to take care + * that gcc will set soft_enabled directly via r13, not choose to use + * an intermediate register, lest we're preempted to a different cpu. + */ + set_soft_enabled(en); if (!en) return; if (firmware_has_feature(FW_FEATURE_ISERIES)) { - if (get_paca()->lppaca_ptr->int_dword.any_int) + /* + * Do we need to disable preemption here? Not really: in the + * unlikely event that we're preempted to a different cpu in + * between getting r13, loading its lppaca_ptr, and loading + * its any_int, we might call iseries_handle_interrupts without + * an interrupt pending on the new cpu, but that's no disaster, + * is it? And the business of preempting us off the old cpu + * would itself involve a local_irq_restore which handles the + * interrupt to that cpu. + * + * But use "local_paca->lppaca_ptr" instead of "get_lppaca()" + * to avoid any preemption checking added into get_paca(). + */ + if (local_paca->lppaca_ptr->int_dword.any_int) iseries_handle_interrupts(); return; } - if (get_paca()->hard_enabled) + /* + * if (get_paca()->hard_enabled) return; + * But again we need to take care that gcc gets hard_enabled directly + * via r13, not choose to use an intermediate register, lest we're + * preempted to a different cpu in between the two instructions. + */ + if (get_hard_enabled()) return; - /* need to hard-enable interrupts here */ - get_paca()->hard_enabled = en; + + /* + * Need to hard-enable interrupts here. Since currently disabled, + * no need to take further asm precautions against preemption; but + * use local_paca instead of get_paca() to avoid preemption checking. + */ + local_paca->hard_enabled = en; if ((int)mfspr(SPRN_DEC) < 0) mtspr(SPRN_DEC, 1); hard_irq_enable(); diff --git a/include/asm-powerpc/hw_irq.h b/include/asm-powerpc/hw_irq.h index c4a1ab608f6f..fd3f2a206271 100644 --- a/include/asm-powerpc/hw_irq.h +++ b/include/asm-powerpc/hw_irq.h @@ -18,15 +18,25 @@ extern void timer_interrupt(struct pt_regs *); static inline unsigned long local_get_flags(void) { - return get_paca()->soft_enabled; + unsigned long flags; + + __asm__ __volatile__("lbz %0,%1(13)" + : "=r" (flags) + : "i" (offsetof(struct paca_struct, soft_enabled))); + + return flags; } static inline unsigned long local_irq_disable(void) { - unsigned long flag = get_paca()->soft_enabled; - get_paca()->soft_enabled = 0; - barrier(); - return flag; + unsigned long flags, zero; + + __asm__ __volatile__("li %1,0; lbz %0,%2(13); stb %1,%2(13)" + : "=r" (flags), "=&r" (zero) + : "i" (offsetof(struct paca_struct, soft_enabled)) + : "memory"); + + return flags; } extern void local_irq_restore(unsigned long); -- GitLab