[PATCH] Revert "powerpc/irq: Don't open code irq_soft_mask helpers"

[Christophe Leroy]

Le 31/08/2022 à 15:10, Michael Ellerman a écrit :
> 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 at gmail.com>
> [mpe: Construct change log based on Zhouyi's original report]
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> ---
>   arch/powerpc/include/asm/hw_irq.h | 43 ++++++++++++++++++++++++++-----
>   1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 3c8cb48f88ae..983551859891 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -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");
>   }

Only the above changes need to be reverted, below ones should remain as 
they are.

>   
>   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;
>   }