On Fri, Jul 17, 2020 at 10:47:50AM -0900, Philip Guenther wrote:
> On Thu, Jul 16, 2020 at 4:55 PM Scott Cheloha <[email protected]>
> wrote:
>
> > > On Jul 16, 2020, at 19:36, Theo de Raadt <[email protected]> wrote:
> > >
> > >> Note the third sentence.
> > >>
> > >> Given that, I reason that a serializing instruction before *and* after
> > >> the RDTSC should freeze it in place.
> > >
> > > I haven't seen anyone read it that way.
> >
> > They say that instructions after RDTSC can run before it because
> > it isn't a serializing instruction.
> >
> > Do we want that?
> >
> > And then, consider this bit of programming advice. Also from the
> > ISA reference (Vol. 2B 4-547):
> >
> > > If software requires RDTSC to be executed prior to execution of any
> > > subsequent instruction (including any memory accesses), it can
> > > execute the sequence LFENCE immediately after RDTSC.
> >
> > What other reading is possible given this documentation?
> >
> > What is your interpretation? Am I missing something?
>
> If you're trying to time a sequence of instructions via rdtsc, then you
> have to put an lfence after the first rdtsc (so that the sequence being
> timed doesn't get lifted to before it completes) and an lfence before the
> second rdtsc (so that the sequence is actually complete before the second
> one). In this case, is it a problem if instructions after the rdtsc that
> are not dependent on its result may be started before it's actually
> complete? If not, then there's no reason to bracket that side.
Oh, okay.
So, just a single preceding LFENCE for the timecounter will suffice.
There's no reason to wait for RDTSC to complete before doing the other
stuff in the loop.
As for rdtsc_lfence(), it sounds like it's entirely context-dependent
whether you want to plant a second LFENCE or not. I guess I'll just
leave it alone.
Updated patch below.
Anyone ok?
Index: lib/libc/arch/amd64/gen/usertc.c
===================================================================
RCS file: /cvs/src/lib/libc/arch/amd64/gen/usertc.c,v
retrieving revision 1.2
diff -u -p -r1.2 usertc.c
--- 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");
+
+ return lo;
}
static int
Index: sys/arch/amd64/amd64/tsc.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.19
diff -u -p -r1.19 tsc.c
--- 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
