On Mon, Dec 03, 2018 at 04:56:10PM -0800, Chris Cappuccio wrote:
> Reyk Floeter [r...@openbsd.org] wrote:
> >
> > Yes, KVM???s stable bit is not a reliable indication as it is seems to
> > depend on the capabilities of the KVM version and not the actual
> > availability of the feature on the particular hardware. How annoying.
> >
> > As mentioned before: I???d like to disable pvclock for now and I can do
> > that in the morning CET if nobody beats me to it.
> >
> > I have an idea how to deal with old platforms afterwards but this needs
> > some more tests and thoughts.
> >
>
> Perhaps the solution is as "simple" as checking the status of the bit
> after the presence of the bit is established ?
>
The approach makes sense but it is not the right way to read the
value. It only works if you don't hit a refresh cycle of the host.
Better diff below.
But I'm not sure if this is enough and if the "stable" flag mit appear
occasionally. So I'd like to disable pvclock for now until we did
some more testing and tried different options.
Reyk
Index: sys/dev/pv/pvclock.c
===================================================================
RCS file: /cvs/src/sys/dev/pv/pvclock.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 pvclock.c
--- sys/dev/pv/pvclock.c 24 Nov 2018 13:12:29 -0000 1.2
+++ sys/dev/pv/pvclock.c 4 Dec 2018 12:09:06 -0000
@@ -70,6 +70,11 @@ uint pvclock_get_timecount(struct timec
void pvclock_read_time_info(struct pvclock_softc *,
struct pvclock_time_info *);
+static inline uint32_t
+ pvclock_read_begin(const struct pvclock_time_info *);
+static inline int
+ pvclock_read_done(const struct pvclock_time_info *, uint32_t);
+
struct cfattach pvclock_ca = {
sizeof(struct pvclock_softc),
pvclock_match,
@@ -127,8 +132,11 @@ pvclock_match(struct device *parent, voi
void
pvclock_attach(struct device *parent, struct device *self, void *aux)
{
- struct pvclock_softc *sc = (struct pvclock_softc *)self;
- paddr_t pa;
+ struct pvclock_softc *sc = (struct pvclock_softc *)self;
+ struct pvclock_time_info *ti;
+ paddr_t pa;
+ uint32_t version;
+ uint8_t flags;
if ((sc->sc_time = km_alloc(PAGE_SIZE,
&kv_any, &kp_zero, &kd_nowait)) == NULL) {
@@ -143,6 +151,19 @@ pvclock_attach(struct device *parent, st
wrmsr(KVM_MSR_SYSTEM_TIME, pa | PVCLOCK_SYSTEM_TIME_ENABLE);
sc->sc_paddr = pa;
+
+ ti = sc->sc_time;
+ do {
+ version = pvclock_read_begin(ti);
+ 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);
+ panic(": unstable clock\n");
+ return;
+ }
sc->sc_tc = &pvclock_timecounter;
sc->sc_tc->tc_name = DEVNAME(sc);
