Re: [PATCH] attach pvclock with lower priority if tsc is unstable

[Mike Larkin]

On Fri, Dec 06, 2019 at 02:16:43PM -0800, Pratik Vyas wrote:
> * Pratik Vyas  [2019-11-24 23:07:26 -0800]:
> 
> > Hello tech@,
> > 
> > This diff attaches pvclock with lower priority (500) in case of unstable
> > tsc (PVCLOCK_FLAG_TSC_STABLE) instead of not attaching at all.
> > 
> > For reference current priorities,
> > tsc (variant)  : -2000
> > i8254  : 0
> > acpitimer  : 1000
> > acpihpet0  : 1000
> > pvclock (stable tsc)   : 1500
> > tsc (invariant, stable): 2000
> > 
> > --
> > Pratik
> > 
> 
> Does this look ok? (or any comments?)
> 
> --
> Pratik
> 

That seems like a reasonable compromise. People who still want to force
set their timecounter can do so.

-ml

Re: [PATCH] attach pvclock with lower priority if tsc is unstable

[Pratik Vyas]

* Pratik Vyas  [2019-11-24 23:07:26 -0800]:


Hello tech@,

This diff attaches pvclock with lower priority (500) in case of unstable
tsc (PVCLOCK_FLAG_TSC_STABLE) instead of not attaching at all.

For reference current priorities,
tsc (variant)  : -2000
i8254  : 0
acpitimer  : 1000
acpihpet0  : 1000
pvclock (stable tsc)   : 1500
tsc (invariant, stable): 2000

--
Pratik



Does this look ok? (or any comments?)

--
Pratik

[PATCH] attach pvclock with lower priority if tsc is unstable

[Pratik Vyas]

Hello tech@,

This diff attaches pvclock with lower priority (500) in case of unstable
tsc (PVCLOCK_FLAG_TSC_STABLE) instead of not attaching at all.

For reference current priorities,
tsc (variant)  : -2000
i8254  : 0
acpitimer  : 1000
acpihpet0  : 1000
pvclock (stable tsc)   : 1500
tsc (invariant, stable): 2000

--
Pratik

Index: sys/dev/pv/pvclock.c
===
RCS file: /home/cvs/src/sys/dev/pv/pvclock.c,v
retrieving revision 1.4
diff -u -p -a -u -r1.4 pvclock.c
--- sys/dev/pv/pvclock.c13 May 2019 15:40:34 -  1.4
+++ sys/dev/pv/pvclock.c25 Nov 2019 06:49:09 -
@@ -29,11 +29,14 @@
#include 

#include 
+#include 
#include 

#include 
#include 

+uint pvclock_lastcount;
+
struct pvclock_softc {
struct devicesc_dev;
void*sc_time;
@@ -141,21 +144,22 @@ pvclock_attach(struct device *parent, st
flags = ti->ti_flags;
} while (!pvclock_read_done(ti, version));

-   if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0) {
-   wrmsr(KVM_MSR_SYSTEM_TIME, pa & ~PVCLOCK_SYSTEM_TIME_ENABLE);
-   km_free(sc->sc_time, PAGE_SIZE, &kv_any, &kp_zero);
-   printf(": unstable clock\n");
-   return;
-   }
-
sc->sc_tc = &pvclock_timecounter;
sc->sc_tc->tc_name = DEVNAME(sc);
sc->sc_tc->tc_frequency = 10ULL;
sc->sc_tc->tc_priv = sc;

+   pvclock_lastcount = 0;
+
/* Better than HPET but below TSC */
sc->sc_tc->tc_quality = 1500;

+   if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0) {
+   /* if tsc is not stable, set a lower priority */
+   /* Better than i8254 but below HPET */
+   sc->sc_tc->tc_quality = 500;
+   }
+
tc_init(sc->sc_tc);

printf("\n");
@@ -216,10 +220,6 @@ pvclock_get_timecount(struct timecounter
flags = ti->ti_flags;
} while (!pvclock_read_done(ti, version));

-   /* This bit must be set as we attached based on the stable flag */
-   if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0)
-   panic("%s: unstable result on stable clock", DEVNAME(sc));
-
/*
 * The algorithm is described in
 * linux/Documentation/virtual/kvm/msr.txt
@@ -230,6 +230,14 @@ pvclock_get_timecount(struct timecounter
else
delta <<= shift;
ctr = ((delta * mul_frac) >> 32) + system_time;
+
+   if ((flags & PVCLOCK_FLAG_TSC_STABLE) != 0)
+   return (ctr);
+
+   if (ctr < pvclock_lastcount)
+   return (pvclock_lastcount);
+
+   atomic_swap_uint(&pvclock_lastcount, ctr);

return (ctr);
}