> Date: Mon, 27 Jul 2020 17:14:21 +0200
> From: Christian Weisgerber <[email protected]>
>
> Scott Cheloha:
>
> > --- lib/libc/arch/amd64/gen/usertc.c 8 Jul 2020 09:17:48 -0000
> > 1.2
> > +++ lib/libc/arch/amd64/gen/usertc.c 25 Jul 2020 17:50:38 -0000
> > @@ -21,9 +21,12 @@
> > static inline u_int
> > rdtsc(void)
> > {
> > - uint32_t hi, lo;
> > - asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
> > - return ((uint64_t)lo)|(((uint64_t)hi)<<32);
> > + uint32_t lo;
> > +
> > + asm volatile("lfence");
> > + asm volatile("rdtsc" : "=a"(lo) : : "rdx");
>
> Is there a guarantee that two separate asm()s will not be reordered?
I believe that is true for "volatile" asm statements. But this is all
not very well documented and I believe that the compiler may hoist
bits of C code in between, which is probably not what you want.
Note that since "asm" is non-standard C, we favour spelling it as
"__asm" since that makes the compiler shut up about it even if you
request stricter C standard compliance.
And given the kernel bit nelow...
> > +
> > + return lo;
> > }
> >
> > static int
> > --- sys/arch/amd64/amd64/tsc.c 6 Jul 2020 13:33:06 -0000 1.19
> > +++ sys/arch/amd64/amd64/tsc.c 25 Jul 2020 17:50:38 -0000
> > @@ -211,7 +211,12 @@ cpu_recalibrate_tsc(struct timecounter *
> > u_int
> > tsc_get_timecount(struct timecounter *tc)
> > {
> > - return rdtsc() + curcpu()->ci_tsc_skew;
> > + uint32_t lo;
> > +
> > + asm volatile("lfence");
> > + asm volatile("rdtsc" : "=a"(lo) : : "rdx");
> > +
> > + return lo + curcpu()->ci_tsc_skew;
> > }
> >
> > void
> >
>
> I'd just do s/rdtsc/rdtsc_lfence/, which would agree well with the
> rest of the file.
Agreed. And I would really prefer that the libc code stays as close
to the kernel code as possible.
