Re: [PATCH] powerpc/vdso64: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE

[Segher Boessenkool <segher@kernel.crashing.org>]

On Fri, Jul 21, 2017 at 07:17:39AM +1000, Benjamin Herrenschmidt wrote:
> > Great patch! Always good to see asm replaced with C.
> 
> Yeah ewll ... when C becomes some kind of weird glorifed asm like
> below, I don't see much of a point ;-)

Yeah.

> > > diff --git a/arch/powerpc/kernel/vdso64/gettime.c b/arch/powerpc/kernel/vdso64/gettime.c
> > > new file mode 100644
> > > index 0000000..01f411f
> > > --- /dev/null
> > > +++ b/arch/powerpc/kernel/vdso64/gettime.c
> > > @@ -0,0 +1,162 @@
> > 
> > ...
> > > +static notrace int gettime_syscall_fallback(clockid_t clk_id,
> > > +					     struct timespec *tp)
> > > +{
> > > +	register clockid_t id asm("r3") = clk_id;
> > > +	register struct timespec *t asm("r4") = tp;
> > > +	register int nr asm("r0") = __NR_clock_gettime;
> > > +	register int ret asm("r3");
> > 
> > I guess this works. I've always been a bit nervous about register
> > variables TBH.
> 
> Does it really work ? That really makes me nervous too, I woudn't do
> this without a strong ack from a toolchain person... Segher ?

Local register variables work perfectly well, but only for one thing:
those variables are guaranteed to be in those registers, _as arguments
to an asm_.

> > > +	asm volatile("sc"
> > > +		     : "=r" (ret)
> > > +		     : "r"(nr), "r"(id), "r"(t)
> > > +		     : "memory");
> > 
> > Not sure we need the memory clobber?
> > 
> > It can clobber more registers than that though.

It needs the memory clobber (if the system call accessess any of "your"
memory).  You need more register clobbers: also for most CR fields,
CTR, etc.

One trick that often works is doing the system call from within an
assembler function that uses the C ABI, since that has almost the same
calling conventions.

Something as simple as

	.globl syscall42
syscall42:
	li 0,42
	sc
	blr

(and yeah handle the CR bit 3 thing somehow)

and declare it as

int syscall42(some_type r3_arg, another_type r4_arg);


Inline asm is good when you want the asm code inlined into the callers,
potentially with arguments optimised etc.  The only overhead making the
syscall a function has is that single blr; the only optimisation you
miss is you could potentially load GPR0 a bit earlier (and you can get
a tiny bit more scheduling flexibility).


Segher