On Sat, Jul 11, 2020 at 01:16:43PM +0200, Mark Kettenis wrote:
> > From: Paul Irofti <[email protected]>
> > Date: Sat, 11 Jul 2020 13:50:37 +0300
> >
> > On 2020-07-11 13:46, Mark Kettenis wrote:
> > >> From: Paul Irofti <[email protected]>
> > >> Date: Sat, 11 Jul 2020 13:32:22 +0300
> > >>
> > >> Hi,
> > >>
> > >> Getting lots of messages about people loving the new timekeep
> > >> functionality, which I am very happy about, but also some that have the
> > >> skew too large for it to be enabled.
> > >>
> > >> I plan on sending a diff next week to improve the situation via RDTSCP
> > >> on the machines that have it. Which is basically all modern machines.
> > >>
> > >> The plan is to have an auxiliary value returned by RDTSCP which
> > >> identifies the CPU we got the info from so that we can look-up its
> > >> associated skew in a table saved at init inside the timekeep structure:
> > >
> > > I think that is the wrong approach. Instead we should synchronize the
> > > TSC counters themselves. There are special MSRs you can write the
> > > offset into IIRC. That seems to be what FreeBSD does.
> >
> > Yes, that is another option. I have not looked to see which are more
> > popular in terms of hardware. Did the MSRs come with RDTSCP? Before? Or
> > after? We should choose the most inclusive solution I guess. Or we could
> > have both...
>
> One thing to keep in mind is that we only use the TSC as a timecounter
> on CPUs that have the ITSC feature.
In the meantime...
Can we add the missing LFENCE instructions to userspace and the
kernel? And can we excise the upper 32 bits? IIRC there was talk of
doing these things anyway, regardless of how the skew problem is
addressed.
My understanding is that you need an "LFENCE sandwich" to prevent the
RDTSC from being reordered in the pipeline.
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 16 Jul 2020 01:31:31 -0000
@@ -21,9 +21,13 @@
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));
+ asm volatile("lfence");
+
+ 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 16 Jul 2020 01:31:31 -0000
@@ -211,7 +211,13 @@ 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));
+ asm volatile("lfence");
+
+ return lo + curcpu()->ci_tsc_skew;
}
void
