Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1681963 > unrolled thread

Re: [PATCH v2 2/4] MIPS: VDSO: Add implementation of clock_gettime() fallback

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 2/4] MIPS: VDSO: Add implementation of clock_gettime()  fallback "Maciej W. Rozycki" <macro@imgtec.com> - 2017-07-06 02:10 +0200
    Re: [PATCH v2 2/4] MIPS: VDSO: Add implementation of clock_gettime()  fallback James Hogan <james.hogan@imgtec.com> - 2017-07-06 11:10 +0200
      Re: [PATCH v2 2/4] MIPS: VDSO: Add implementation of clock_gettime()  fallback "Maciej W. Rozycki" <macro@imgtec.com> - 2017-07-06 15:20 +0200
        Re: [PATCH v2 2/4] MIPS: VDSO: Add implementation of clock_gettime()  fallback James Hogan <james.hogan@imgtec.com> - 2017-07-06 16:10 +0200

#1681963 — Re: [PATCH v2 2/4] MIPS: VDSO: Add implementation of clock_gettime() fallback

On Wed, 28 Jun 2017, Aleksandar Markovic wrote:

> diff --git a/arch/mips/vdso/gettimeofday.c b/arch/mips/vdso/gettimeofday.c
> index fd7d433..5f63375 100644
> --- a/arch/mips/vdso/gettimeofday.c
> +++ b/arch/mips/vdso/gettimeofday.c
> @@ -20,6 +20,24 @@
>  #include <asm/unistd.h>
>  #include <asm/vdso.h>
>  
> +static __always_inline long clock_gettime_fallback(clockid_t _clkid,
> +					   struct timespec *_ts)
> +{
> +	register struct timespec *ts asm("a1") = _ts;
> +	register clockid_t clkid asm("a0") = _clkid;
> +	register long ret asm("v0");
> +	register long nr asm("v0") = __NR_clock_gettime;
> +	register long error asm("a3");
> +
> +	asm volatile(
> +	"       syscall\n"
> +	: "=r" (ret), "=r" (error)
> +	: "r" (clkid), "r" (ts), "r" (nr)
> +	: "memory");
> +
> +	return error ? -ret : ret;
> +}

 Hmm, are you sure it is safe nowadays WRT the syscall restart convention 
to leave out the instruction explicitly loading the syscall number that 
would normally immediately precede SYSCALL (and would have to forcibly use 
the 32-bit encoding in the microMIPS case)?

  Maciej

[toc] | [next] | [standalone]

#1682212

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jul 06, 2017 at 01:00:34AM +0100, Maciej W. Rozycki wrote:
> On Wed, 28 Jun 2017, Aleksandar Markovic wrote:
> 
> > diff --git a/arch/mips/vdso/gettimeofday.c b/arch/mips/vdso/gettimeofday.c
> > index fd7d433..5f63375 100644
> > --- a/arch/mips/vdso/gettimeofday.c
> > +++ b/arch/mips/vdso/gettimeofday.c
> > @@ -20,6 +20,24 @@
> >  #include <asm/unistd.h>
> >  #include <asm/vdso.h>
> >  
> > +static __always_inline long clock_gettime_fallback(clockid_t _clkid,
> > +					   struct timespec *_ts)
> > +{
> > +	register struct timespec *ts asm("a1") = _ts;
> > +	register clockid_t clkid asm("a0") = _clkid;
> > +	register long ret asm("v0");
> > +	register long nr asm("v0") = __NR_clock_gettime;
> > +	register long error asm("a3");
> > +
> > +	asm volatile(
> > +	"       syscall\n"
> > +	: "=r" (ret), "=r" (error)
> > +	: "r" (clkid), "r" (ts), "r" (nr)
> > +	: "memory");
> > +
> > +	return error ? -ret : ret;
> > +}
> 
>  Hmm, are you sure it is safe nowadays WRT the syscall restart convention 
> to leave out the instruction explicitly loading the syscall number that 
> would normally immediately precede SYSCALL

It should be fine. syscall restart only rewinds one (32-bit)
instruction, and it preserves the syscall number in pt_regs::regs[0]
(see handle_signal() / do_signal() and this code in e.g. scall32-o32.S:)

sw      t1, PT_R0(sp)           # save it for syscall restarting

> (and would have to forcibly use the 32-bit encoding in the microMIPS
> case)?

I don't believe there is a 16-bit SYSCALL encoding in microMIPS, at
least I can't see one in the 5.04 manual.

However, the clobber list is incomplete.
The following registers are written as outputs:
	$2 (v0), $7 (a3)
The following registers are used as arguments and should be preserved:
	$4-$6 (a0-a2), [$8-$9 (a4-a5)] (n32 / n64 only)
And the following other registers are preserved:
	$16-$23, $28-$31
So assuming you already have $2 and $7 as outputs, the clobber list
should be:
	"$1", "$3", ["$8", "$9",] "$10", "$11", "$12", "$13", "$14",
	"$15", "$24", "$25", "hi", "lo", "memory"

(only o32 needs to mark $8-$9 clobbered, but no harm doing so on n32/n64
too)

Cheers
James

[toc] | [prev] | [next] | [standalone]

#1682426

On Thu, 6 Jul 2017, James Hogan wrote:

> > > +	asm volatile(
> > > +	"       syscall\n"
> > > +	: "=r" (ret), "=r" (error)
> > > +	: "r" (clkid), "r" (ts), "r" (nr)
> > > +	: "memory");
> > > +
> > > +	return error ? -ret : ret;
> > > +}
> > 
> >  Hmm, are you sure it is safe nowadays WRT the syscall restart convention 
> > to leave out the instruction explicitly loading the syscall number that 
> > would normally immediately precede SYSCALL
> 
> It should be fine. syscall restart only rewinds one (32-bit)
> instruction, and it preserves the syscall number in pt_regs::regs[0]
> (see handle_signal() / do_signal() and this code in e.g. scall32-o32.S:)
> 
> sw      t1, PT_R0(sp)           # save it for syscall restarting

 Fair enough, I just wanted to be sure.

 [This user code is bundled with the kernel, so it can assume whatever the 
kernel does, however general user code does have to conform to the legacy 
restart convention, unless it also requires a kernel version that is new 
enough and has a safety check in place.]

> > (and would have to forcibly use the 32-bit encoding in the microMIPS
> > case)?
> 
> I don't believe there is a 16-bit SYSCALL encoding in microMIPS, at
> least I can't see one in the 5.04 manual.

 I referred to the preceding instruction, presumably LI, that does have a 
16-bit variant in the microMIPS instruction set.

  Maciej

[toc] | [prev] | [next] | [standalone]

#1682455

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jul 06, 2017 at 02:12:37PM +0100, Maciej W. Rozycki wrote:
> On Thu, 6 Jul 2017, James Hogan wrote:
> > > (and would have to forcibly use the 32-bit encoding in the microMIPS
> > > case)?
> > 
> > I don't believe there is a 16-bit SYSCALL encoding in microMIPS, at
> > least I can't see one in the 5.04 manual.
> 
>  I referred to the preceding instruction, presumably LI, that does have a 
> 16-bit variant in the microMIPS instruction set.

Ah yes, I see what you mean.

Hopefully microMIPS support is new enough for that not to matter.

It appears that the restart behaviour was improved in v2.6.36 in commit
8f5a00eb422e ("MIPS: Sanitize restart logics"), whereas first mentions
of micromips are in v3.9, in commit f8fa4811dbb2 ("MIPS: Add support for
the M14KEc core.").

Cheers
James

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web