Revert "powerpc/irq: Don't open code irq_soft_mask helpers"
authorMichael Ellerman <mpe@ellerman.id.au>
Wed, 31 Aug 2022 13:10:52 +0000 (23:10 +1000)
committerMichael Ellerman <mpe@ellerman.id.au>
Fri, 2 Sep 2022 08:54:53 +0000 (18:54 +1000)
This reverts commit ef5b570d3700fbb8628a58da0487486ceeb713cd.

Zhouyi reported that commit is causing crashes when running rcutorture
with KASAN enabled:

  BUG: using smp_processor_id() in preemptible [00000000] code: rcu_torture_rea/100
  caller is rcu_preempt_deferred_qs_irqrestore+0x74/0xed0
  CPU: 4 PID: 100 Comm: rcu_torture_rea Tainted: G        W          5.19.0-rc5-next-20220708-dirty #253
  Call Trace:
    dump_stack_lvl+0xbc/0x108 (unreliable)
    check_preemption_disabled+0x154/0x160
    rcu_preempt_deferred_qs_irqrestore+0x74/0xed0
    __rcu_read_unlock+0x290/0x3b0
    rcu_torture_read_unlock+0x30/0xb0
    rcutorture_one_extend+0x198/0x810
    rcu_torture_one_read+0x58c/0xc90
    rcu_torture_reader+0x12c/0x360
    kthread+0x1e8/0x220
    ret_from_kernel_thread+0x5c/0x64

KASAN will generate instrumentation instructions around the
WRITE_ONCE(local_paca->irq_soft_mask, mask):

   0xc000000000295cb0 <+0>: addis   r2,r12,774
   0xc000000000295cb4 <+4>: addi    r2,r2,16464
   0xc000000000295cb8 <+8>: mflr    r0
   0xc000000000295cbc <+12>: bl      0xc00000000008bb4c <mcount>
   0xc000000000295cc0 <+16>: mflr    r0
   0xc000000000295cc4 <+20>: std     r31,-8(r1)
   0xc000000000295cc8 <+24>: addi    r3,r13,2354
   0xc000000000295ccc <+28>: mr      r31,r13
   0xc000000000295cd0 <+32>: std     r0,16(r1)
   0xc000000000295cd4 <+36>: stdu    r1,-48(r1)
   0xc000000000295cd8 <+40>: bl      0xc000000000609b98 <__asan_store1+8>
   0xc000000000295cdc <+44>: nop
   0xc000000000295ce0 <+48>: li      r9,1
   0xc000000000295ce4 <+52>: stb     r9,2354(r31)
   0xc000000000295ce8 <+56>: addi    r1,r1,48
   0xc000000000295cec <+60>: ld      r0,16(r1)
   0xc000000000295cf0 <+64>: ld      r31,-8(r1)
   0xc000000000295cf4 <+68>: mtlr    r0

If there is a context switch before "stb     r9,2354(r31)", r31 may
not equal to r13, in such case, irq soft mask will not work.

The usual solution of marking the code ineligible for instrumentation
forces the code out-of-line, which we would prefer to avoid. Christophe
proposed a partial revert, but Nick raised some concerns with that. So
for now do a full revert.

Reported-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
[mpe: Construct change log based on Zhouyi's original report]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20220831131052.42250-1-mpe@ellerman.id.au
arch/powerpc/include/asm/hw_irq.h

index 3c8cb48f88aee82e134eb6d88719f50500e347cb..983551859891a9525551ae477630b7eee21ef3f6 100644 (file)
@@ -113,7 +113,14 @@ static inline void __hard_RI_enable(void)
 
 static inline notrace unsigned long irq_soft_mask_return(void)
 {
-       return READ_ONCE(local_paca->irq_soft_mask);
+       unsigned long flags;
+
+       asm volatile(
+               "lbz %0,%1(13)"
+               : "=r" (flags)
+               : "i" (offsetof(struct paca_struct, irq_soft_mask)));
+
+       return flags;
 }
 
 /*
@@ -140,24 +147,46 @@ static inline notrace void irq_soft_mask_set(unsigned long mask)
        if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
                WARN_ON(mask && !(mask & IRQS_DISABLED));
 
-       WRITE_ONCE(local_paca->irq_soft_mask, mask);
-       barrier();
+       asm volatile(
+               "stb %0,%1(13)"
+               :
+               : "r" (mask),
+                 "i" (offsetof(struct paca_struct, irq_soft_mask))
+               : "memory");
 }
 
 static inline notrace unsigned long irq_soft_mask_set_return(unsigned long mask)
 {
-       unsigned long flags = irq_soft_mask_return();
+       unsigned long flags;
 
-       irq_soft_mask_set(mask);
+#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
+       WARN_ON(mask && !(mask & IRQS_DISABLED));
+#endif
+
+       asm volatile(
+               "lbz %0,%1(13); stb %2,%1(13)"
+               : "=&r" (flags)
+               : "i" (offsetof(struct paca_struct, irq_soft_mask)),
+                 "r" (mask)
+               : "memory");
 
        return flags;
 }
 
 static inline notrace unsigned long irq_soft_mask_or_return(unsigned long mask)
 {
-       unsigned long flags = irq_soft_mask_return();
+       unsigned long flags, tmp;
+
+       asm volatile(
+               "lbz %0,%2(13); or %1,%0,%3; stb %1,%2(13)"
+               : "=&r" (flags), "=r" (tmp)
+               : "i" (offsetof(struct paca_struct, irq_soft_mask)),
+                 "r" (mask)
+               : "memory");
 
-       irq_soft_mask_set(flags | mask);
+#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
+       WARN_ON((mask | flags) && !((mask | flags) & IRQS_DISABLED));
+#endif
 
        return flags;
 }