Re: [PATCH v4 2/2] powerpc/irq: inline call_do_irq() and call_do_softirq()
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-11-21 06:14:52
Also in:
lkml
Christophe Leroy [off-list ref] writes:
call_do_irq() and call_do_softirq() are quite similar on PPC32 and PPC64 and are simple enough to be worth inlining. Inlining them avoids an mflr/mtlr pair plus a save/reload on stack. This is inspired from S390 arch. Several other arches do more or less the same. The way sparc arch does seems odd thought. Signed-off-by: Christophe Leroy <redacted> Reviewed-by: Segher Boessenkool <redacted> --- v2: no change. v3: no change. v4: - comment reminding the purpose of the inline asm block. - added r2 as clobbered reg
That breaks 64-bit with GCC9:
arch/powerpc/kernel/irq.c: In function 'do_IRQ':
arch/powerpc/kernel/irq.c:650:2: error: PIC register clobbered by 'r2' in 'asm'
650 | asm volatile(
| ^~~
arch/powerpc/kernel/irq.c: In function 'do_softirq_own_stack':
arch/powerpc/kernel/irq.c:711:2: error: PIC register clobbered by 'r2' in 'asm'
711 | asm volatile(
| ^~~
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index 04204be49577..d62fe18405a0 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c@@ -642,6 +642,22 @@ void __do_irq(struct pt_regs *regs) irq_exit(); } +static inline void call_do_irq(struct pt_regs *regs, void *sp) +{ + register unsigned long r3 asm("r3") = (unsigned long)regs; + + /* Temporarily switch r1 to sp, call __do_irq() then restore r1 */ + asm volatile( + " "PPC_STLU" 1, %2(%1);\n" + " mr 1, %1;\n" + " bl %3;\n" + " "PPC_LL" 1, 0(1);\n" : + "+r"(r3) : + "b"(sp), "i"(THREAD_SIZE - STACK_FRAME_OVERHEAD), "i"(__do_irq) : + "lr", "xer", "ctr", "memory", "cr0", "cr1", "cr5", "cr6", "cr7", + "r0", "r2", "r4", "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12"); +}
If we add a nop after the bl, so the linker could insert a TOC restore, then I don't think there's any circumstance under which we expect this to actually clobber r2, is there? cheers