Re: [PATCH v4 2/2] powerpc/irq: inline call_do_irq() and call_do_softirq()

[Christophe Leroy]

Le 25/11/2019 Ã 15:25, Segher Boessenkool a ÃcritÂ:
> On Mon, Nov 25, 2019 at 09:32:23PM +1100, Michael Ellerman wrote:
> > Segher Boessenkool <segher@xxxxxxxxxxxxxxxxxxx> writes:
> > > > > +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?
> > >
> > > That is mostly correct.
> >
> > That's the standard I aspire to :P
> >
> > > If call_do_irq was a no-inline function, there would not be problems.
> > >
> > > What TOC does __do_irq require in r2 on entry, and what will be there
> > > when it returns?
> >
> > The kernel TOC, and also the kernel TOC, unless something's gone wrong
> > or I'm missing something.
>
> If that is the case, we can just do the bl, no nop at all?  And that works
> for all of our ABIs.
>
> If we can be certain that we have the kernel TOC in r2 on entry to
> call_do_irq, that is!  (Or it establishes it itself).

So what do we do ? We just drop the "r2" clobber ?

Otherwise, to be on the safe side we can just save r2 in a local var 
before the bl and restore it after. I guess it won't collapse CPU time 
on a performant PPC64.

Christophe