On Wed, Aug 24, 2022 at 05:51:14PM +1000, Jonathan Gray wrote:
> On Tue, Aug 23, 2022 at 12:20:39PM -0500, Scott Cheloha wrote:
> > > Hyper-V generation 1 VMs are bios boot with emulation of the usual
> > > devices. 32-bit and 64-bit guests.
> > >
> > > Hyper-V generation 2 VMs are 64-bit uefi with paravirtualised devices.
> > > 64-bit guests only.
> > >
> > > There is no 8254 in generation 2.
> > > No HPET in either generation.
> > >
> > > hv_delay uses the "Partition Reference Counter MSR" described in
> > > https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/timers
> > > It seems it is available in both generations and could be used from i386?
> > >
> > > From reading that page hv_delay() should be preferred over lapic_delay()
> >
> > Alright, I have nudged hv_delay's quality up over lapic_delay's
> > quality.
>
> Before these changes, tsc is probed before pvbus. Do the tsc sanity
> checks result in it not being considered an option on hyper-v? I think
> the tsc_delay and hv_delay numbers should be swapped in a later commit.
> It is unclear if that would change the final delay_func setting.
Why would we prefer hv_delay() to tsc_delay() if we had a
constant/invariant TSC available in our Hyper-V guest?
When patrick@ emailed me last year about issues with delay(9) on
Hyper-V, he started by saying that the root of the problem was that
the OpenBSD guest was not opting to use tsc_delay() because the host
wasn't reporting a constant/invariant TSC. So the guest was trying to
use i8254_delay(), which was impossible because Hyper-V Gen2 guests
don't have an i8254. Hence, hv_delay() was added to the tree.
So, my understanding is that the addition of hv_delay() does not mean
tsc_delay() is worse than hv_delay(). hv_delay() was added because
tsc_delay() isn't always an option and (to our surprise) neither is
i8254_delay().
> It would be a good idea to have different commits for the places new
> delay callbacks are introduced.
>
> - add delay_init()
> - use delay_init() in lapic, tsc, hv_delay
> - commit acpihpet
> - commit acpitimer
I had planned to do separate commits. This ordering seems right.
> - swap tsc and hv_delay numbers
See above.
> > How are we looking now?
>
> some minor suggestions inline
>
> have you built a release with this?
Just finished building a release and upgrading with it from physical
media. I think we are good to go. I incorporated your suggestions
below and I'm going to do the first four suggested commits tomorrow
unless I hear otherwise.
Current combined patch is attached.
> > Index: sys/arch/amd64/amd64/lapic.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> > retrieving revision 1.60
> > diff -u -p -r1.60 lapic.c
> > --- sys/arch/amd64/amd64/lapic.c 15 Aug 2022 04:17:50 -0000 1.60
> > +++ sys/arch/amd64/amd64/lapic.c 23 Aug 2022 17:18:30 -0000
> > @@ -486,8 +486,6 @@ wait_next_cycle(void)
> > }
> > }
> >
> > -extern void tsc_delay(int);
> > -
>
> this cleanup is unrelated and should be a different diff/commit
Ack, will do it separately.
> > /*
> > * Calibrate the local apic count-down timer (which is running at
> > * bus-clock speed) vs. the i8254 counter/timer (which is running at
> > @@ -592,8 +590,7 @@ skip_calibration:
> > * Now that the timer's calibrated, use the apic timer routines
> > * for all our timing needs..
> > */
> > - if (delay_func == i8254_delay)
> > - delay_func = lapic_delay;
> > + delay_init(lapic_delay, 3000);
> > initclock_func = lapic_initclocks;
> > }
> > }
> > Index: sys/arch/amd64/amd64/machdep.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
> > retrieving revision 1.279
> > diff -u -p -r1.279 machdep.c
> > --- sys/arch/amd64/amd64/machdep.c 7 Aug 2022 23:56:06 -0000 1.279
> > +++ sys/arch/amd64/amd64/machdep.c 23 Aug 2022 17:18:31 -0000
> > @@ -2069,3 +2069,13 @@ check_context(const struct reg *regs, st
> >
> > return 0;
> > }
> > +
> > +void
> > +delay_init(void(*fn)(int), int fn_quality)
> > +{
> > + static int cur_quality = 0;
> > + if (fn_quality > cur_quality) {
> > + delay_func = fn;
> > + cur_quality = fn_quality;
> > + }
> > +}
> > Index: sys/arch/amd64/amd64/tsc.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> > retrieving revision 1.25
> > diff -u -p -r1.25 tsc.c
> > --- sys/arch/amd64/amd64/tsc.c 12 Aug 2022 02:20:36 -0000 1.25
> > +++ sys/arch/amd64/amd64/tsc.c 23 Aug 2022 17:18:31 -0000
> > @@ -109,7 +109,7 @@ tsc_identify(struct cpu_info *ci)
> >
> > tsc_frequency = tsc_freq_cpuid(ci);
> > if (tsc_frequency > 0)
> > - delay_func = tsc_delay;
> > + delay_init(tsc_delay, 5000);
> > }
> >
> > static inline int
> > Index: sys/arch/amd64/include/cpu.h
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
> > retrieving revision 1.148
> > diff -u -p -r1.148 cpu.h
> > --- sys/arch/amd64/include/cpu.h 22 Aug 2022 08:57:54 -0000 1.148
> > +++ sys/arch/amd64/include/cpu.h 23 Aug 2022 17:18:31 -0000
> > @@ -359,6 +359,7 @@ void signotify(struct proc *);
> > * We need a machine-independent name for this.
> > */
> > extern void (*delay_func)(int);
> > +void delay_init(void (*)(int), int);
> > struct timeval;
> >
> > #define DELAY(x) (*delay_func)(x)
> > Index: sys/arch/i386/i386/lapic.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/i386/i386/lapic.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 lapic.c
> > --- sys/arch/i386/i386/lapic.c 15 Aug 2022 04:17:50 -0000 1.49
> > +++ sys/arch/i386/i386/lapic.c 23 Aug 2022 17:18:31 -0000
> > @@ -395,7 +395,7 @@ lapic_calibrate_timer(struct cpu_info *c
> > * Now that the timer's calibrated, use the apic timer routines
> > * for all our timing needs..
> > */
> > - delay_func = lapic_delay;
> > + delay_init(lapic_delay, 3000);
> > initclock_func = lapic_initclocks;
> > }
> > }
> > Index: sys/arch/i386/i386/machdep.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> > retrieving revision 1.655
> > diff -u -p -r1.655 machdep.c
> > --- sys/arch/i386/i386/machdep.c 22 Aug 2022 08:53:55 -0000 1.655
> > +++ sys/arch/i386/i386/machdep.c 23 Aug 2022 17:18:33 -0000
> > @@ -3974,3 +3974,13 @@ cpu_rnd_messybits(void)
> > nanotime(&ts);
> > return (ts.tv_nsec ^ (ts.tv_sec << 20));
> > }
> > +
> > +void
> > +delay_init(void(*fn)(int), int fn_quality)
> > +{
> > + static int cur_quality = 0;
> > + if (fn_quality > cur_quality) {
> > + delay_func = fn;
> > + cur_quality = fn_quality;
> > + }
> > +}
> > Index: sys/arch/i386/include/cpu.h
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/i386/include/cpu.h,v
> > retrieving revision 1.177
> > diff -u -p -r1.177 cpu.h
> > --- sys/arch/i386/include/cpu.h 22 Aug 2022 08:53:55 -0000 1.177
> > +++ sys/arch/i386/include/cpu.h 23 Aug 2022 17:18:33 -0000
> > @@ -302,6 +302,7 @@ void signotify(struct proc *);
> > * We need a machine-independent name for this.
> > */
> > extern void (*delay_func)(int);
> > +void delay_init(void(*)(int), int);
> > struct timeval;
> >
> > #define DELAY(x) (*delay_func)(x)
> > Index: sys/dev/acpi/acpitimer.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/acpitimer.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 acpitimer.c
> > --- sys/dev/acpi/acpitimer.c 6 Apr 2022 18:59:27 -0000 1.15
> > +++ sys/dev/acpi/acpitimer.c 23 Aug 2022 17:18:33 -0000
> > @@ -18,17 +18,22 @@
> > #include <sys/param.h>
> > #include <sys/systm.h>
> > #include <sys/device.h>
> > +#include <sys/stdint.h>
> > #include <sys/timetc.h>
> >
> > #include <machine/bus.h>
> > +#include <machine/cpu.h>
> >
> > #include <dev/acpi/acpireg.h>
> > #include <dev/acpi/acpivar.h>
> >
> > +struct acpitimer_softc;
>
> move acpitimer_softc here instead
Done.
> > +
> > int acpitimermatch(struct device *, void *, void *);
> > void acpitimerattach(struct device *, struct device *, void *);
> > -
> > +void acpitimer_delay(int);
> > u_int acpi_get_timecount(struct timecounter *tc);
> > +uint32_t acpitimer_read(struct acpitimer_softc *);
> >
> > static struct timecounter acpi_timecounter = {
> > .tc_get_timecount = acpi_get_timecount,
> > @@ -98,18 +103,43 @@ acpitimerattach(struct device *parent, s
> > acpi_timecounter.tc_priv = sc;
> > acpi_timecounter.tc_name = sc->sc_dev.dv_xname;
> > tc_init(&acpi_timecounter);
> > +
> > + delay_init(acpitimer_delay, 1000);
> > +
> > #if defined(__amd64__)
> > extern void cpu_recalibrate_tsc(struct timecounter *);
> > cpu_recalibrate_tsc(&acpi_timecounter);
> > #endif
> > }
> >
> > +void
> > +acpitimer_delay(int usecs)
> > +{
> > + uint64_t count = 0, cycles;
> > + struct acpitimer_softc *sc = acpi_timecounter.tc_priv;
> > + uint32_t mask = acpi_timecounter.tc_counter_mask;
> > + uint32_t val1, val2;
> > +
> > + val2 = acpitimer_read(sc);
> > + cycles = usecs * acpi_timecounter.tc_frequency / 1000000;
> > + while (count < cycles) {
> > + CPU_BUSY_CYCLE();
> > + val1 = val2;
> > + val2 = acpitimer_read(sc);
> > + count += (val2 - val1) & mask;
> > + }
> > +}
> >
> > u_int
> > acpi_get_timecount(struct timecounter *tc)
> > {
> > - struct acpitimer_softc *sc = tc->tc_priv;
> > - u_int u1, u2, u3;
> > + return acpitimer_read(tc->tc_priv);
> > +}
> > +
> > +uint32_t
> > +acpitimer_read(struct acpitimer_softc *sc)
> > +{
> > + uint32_t u1, u2, u3;
> >
> > u2 = bus_space_read_4(sc->sc_iot, sc->sc_ioh, 0);
> > u3 = bus_space_read_4(sc->sc_iot, sc->sc_ioh, 0);
> > Index: sys/dev/acpi/acpihpet.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/acpi/acpihpet.c,v
> > retrieving revision 1.26
> > diff -u -p -r1.26 acpihpet.c
> > --- sys/dev/acpi/acpihpet.c 6 Apr 2022 18:59:27 -0000 1.26
> > +++ sys/dev/acpi/acpihpet.c 23 Aug 2022 17:18:33 -0000
> > @@ -18,9 +18,11 @@
> > #include <sys/param.h>
> > #include <sys/systm.h>
> > #include <sys/device.h>
> > +#include <sys/stdint.h>
> > #include <sys/timetc.h>
> >
> > #include <machine/bus.h>
> > +#include <machine/cpu.h>
> >
> > #include <dev/acpi/acpireg.h>
> > #include <dev/acpi/acpivar.h>
> > @@ -31,7 +33,7 @@ int acpihpet_attached;
> > int acpihpet_match(struct device *, void *, void *);
> > void acpihpet_attach(struct device *, struct device *, void *);
> > int acpihpet_activate(struct device *, int);
> > -
> > +void acpihpet_delay(int);
> > u_int acpihpet_gettime(struct timecounter *tc);
> >
> > uint64_t acpihpet_r(bus_space_tag_t _iot, bus_space_handle_t _ioh,
> > @@ -262,15 +264,30 @@ acpihpet_attach(struct device *parent, s
> > freq = 1000000000000000ull / period;
> > printf(": %lld Hz\n", freq);
> >
> > - hpet_timecounter.tc_frequency = (uint32_t)freq;
> > + hpet_timecounter.tc_frequency = freq;
>
> this is unrelated and should be a different diff/commit
Ack, will do it separately.
--
Index: sys/arch/amd64/amd64/lapic.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.60
diff -u -p -r1.60 lapic.c
--- sys/arch/amd64/amd64/lapic.c 15 Aug 2022 04:17:50 -0000 1.60
+++ sys/arch/amd64/amd64/lapic.c 25 Aug 2022 03:56:47 -0000
@@ -592,8 +592,7 @@ skip_calibration:
* Now that the timer's calibrated, use the apic timer routines
* for all our timing needs..
*/
- if (delay_func == i8254_delay)
- delay_func = lapic_delay;
+ delay_init(lapic_delay, 3000);
initclock_func = lapic_initclocks;
}
}
Index: sys/arch/amd64/amd64/machdep.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.279
diff -u -p -r1.279 machdep.c
--- sys/arch/amd64/amd64/machdep.c 7 Aug 2022 23:56:06 -0000 1.279
+++ sys/arch/amd64/amd64/machdep.c 25 Aug 2022 03:56:47 -0000
@@ -2069,3 +2069,13 @@ check_context(const struct reg *regs, st
return 0;
}
+
+void
+delay_init(void(*fn)(int), int fn_quality)
+{
+ static int cur_quality = 0;
+ if (fn_quality > cur_quality) {
+ delay_func = fn;
+ cur_quality = fn_quality;
+ }
+}
Index: sys/arch/amd64/amd64/tsc.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.25
diff -u -p -r1.25 tsc.c
--- sys/arch/amd64/amd64/tsc.c 12 Aug 2022 02:20:36 -0000 1.25
+++ sys/arch/amd64/amd64/tsc.c 25 Aug 2022 03:56:48 -0000
@@ -109,7 +109,7 @@ tsc_identify(struct cpu_info *ci)
tsc_frequency = tsc_freq_cpuid(ci);
if (tsc_frequency > 0)
- delay_func = tsc_delay;
+ delay_init(tsc_delay, 5000);
}
static inline int
Index: sys/arch/amd64/include/cpu.h
===================================================================
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.148
diff -u -p -r1.148 cpu.h
--- sys/arch/amd64/include/cpu.h 22 Aug 2022 08:57:54 -0000 1.148
+++ sys/arch/amd64/include/cpu.h 25 Aug 2022 03:56:48 -0000
@@ -359,6 +359,7 @@ void signotify(struct proc *);
* We need a machine-independent name for this.
*/
extern void (*delay_func)(int);
+void delay_init(void (*)(int), int);
struct timeval;
#define DELAY(x) (*delay_func)(x)
Index: sys/arch/i386/i386/lapic.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/lapic.c,v
retrieving revision 1.49
diff -u -p -r1.49 lapic.c
--- sys/arch/i386/i386/lapic.c 15 Aug 2022 04:17:50 -0000 1.49
+++ sys/arch/i386/i386/lapic.c 25 Aug 2022 03:56:48 -0000
@@ -395,7 +395,7 @@ lapic_calibrate_timer(struct cpu_info *c
* Now that the timer's calibrated, use the apic timer routines
* for all our timing needs..
*/
- delay_func = lapic_delay;
+ delay_init(lapic_delay, 3000);
initclock_func = lapic_initclocks;
}
}
Index: sys/arch/i386/i386/machdep.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.655
diff -u -p -r1.655 machdep.c
--- sys/arch/i386/i386/machdep.c 22 Aug 2022 08:53:55 -0000 1.655
+++ sys/arch/i386/i386/machdep.c 25 Aug 2022 03:56:49 -0000
@@ -3974,3 +3974,13 @@ cpu_rnd_messybits(void)
nanotime(&ts);
return (ts.tv_nsec ^ (ts.tv_sec << 20));
}
+
+void
+delay_init(void(*fn)(int), int fn_quality)
+{
+ static int cur_quality = 0;
+ if (fn_quality > cur_quality) {
+ delay_func = fn;
+ cur_quality = fn_quality;
+ }
+}
Index: sys/arch/i386/include/cpu.h
===================================================================
RCS file: /cvs/src/sys/arch/i386/include/cpu.h,v
retrieving revision 1.177
diff -u -p -r1.177 cpu.h
--- sys/arch/i386/include/cpu.h 22 Aug 2022 08:53:55 -0000 1.177
+++ sys/arch/i386/include/cpu.h 25 Aug 2022 03:56:49 -0000
@@ -302,6 +302,7 @@ void signotify(struct proc *);
* We need a machine-independent name for this.
*/
extern void (*delay_func)(int);
+void delay_init(void(*)(int), int);
struct timeval;
#define DELAY(x) (*delay_func)(x)
Index: sys/dev/acpi/acpitimer.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpitimer.c,v
retrieving revision 1.15
diff -u -p -r1.15 acpitimer.c
--- sys/dev/acpi/acpitimer.c 6 Apr 2022 18:59:27 -0000 1.15
+++ sys/dev/acpi/acpitimer.c 25 Aug 2022 03:56:49 -0000
@@ -18,17 +18,27 @@
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/device.h>
+#include <sys/stdint.h>
#include <sys/timetc.h>
#include <machine/bus.h>
+#include <machine/cpu.h>
#include <dev/acpi/acpireg.h>
#include <dev/acpi/acpivar.h>
+struct acpitimer_softc {
+ struct device sc_dev;
+
+ bus_space_tag_t sc_iot;
+ bus_space_handle_t sc_ioh;
+};
+
int acpitimermatch(struct device *, void *, void *);
void acpitimerattach(struct device *, struct device *, void *);
-
+void acpitimer_delay(int);
u_int acpi_get_timecount(struct timecounter *tc);
+uint32_t acpitimer_read(struct acpitimer_softc *);
static struct timecounter acpi_timecounter = {
.tc_get_timecount = acpi_get_timecount,
@@ -41,13 +51,6 @@ static struct timecounter acpi_timecount
.tc_user = 0,
};
-struct acpitimer_softc {
- struct device sc_dev;
-
- bus_space_tag_t sc_iot;
- bus_space_handle_t sc_ioh;
-};
-
const struct cfattach acpitimer_ca = {
sizeof(struct acpitimer_softc), acpitimermatch, acpitimerattach
};
@@ -98,18 +101,43 @@ acpitimerattach(struct device *parent, s
acpi_timecounter.tc_priv = sc;
acpi_timecounter.tc_name = sc->sc_dev.dv_xname;
tc_init(&acpi_timecounter);
+
+ delay_init(acpitimer_delay, 1000);
+
#if defined(__amd64__)
extern void cpu_recalibrate_tsc(struct timecounter *);
cpu_recalibrate_tsc(&acpi_timecounter);
#endif
}
+void
+acpitimer_delay(int usecs)
+{
+ uint64_t count = 0, cycles;
+ struct acpitimer_softc *sc = acpi_timecounter.tc_priv;
+ uint32_t mask = acpi_timecounter.tc_counter_mask;
+ uint32_t val1, val2;
+
+ val2 = acpitimer_read(sc);
+ cycles = usecs * acpi_timecounter.tc_frequency / 1000000;
+ while (count < cycles) {
+ CPU_BUSY_CYCLE();
+ val1 = val2;
+ val2 = acpitimer_read(sc);
+ count += (val2 - val1) & mask;
+ }
+}
u_int
acpi_get_timecount(struct timecounter *tc)
{
- struct acpitimer_softc *sc = tc->tc_priv;
- u_int u1, u2, u3;
+ return acpitimer_read(tc->tc_priv);
+}
+
+uint32_t
+acpitimer_read(struct acpitimer_softc *sc)
+{
+ uint32_t u1, u2, u3;
u2 = bus_space_read_4(sc->sc_iot, sc->sc_ioh, 0);
u3 = bus_space_read_4(sc->sc_iot, sc->sc_ioh, 0);
Index: sys/dev/acpi/acpihpet.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpihpet.c,v
retrieving revision 1.26
diff -u -p -r1.26 acpihpet.c
--- sys/dev/acpi/acpihpet.c 6 Apr 2022 18:59:27 -0000 1.26
+++ sys/dev/acpi/acpihpet.c 25 Aug 2022 03:56:49 -0000
@@ -18,9 +18,11 @@
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/device.h>
+#include <sys/stdint.h>
#include <sys/timetc.h>
#include <machine/bus.h>
+#include <machine/cpu.h>
#include <dev/acpi/acpireg.h>
#include <dev/acpi/acpivar.h>
@@ -31,7 +33,7 @@ int acpihpet_attached;
int acpihpet_match(struct device *, void *, void *);
void acpihpet_attach(struct device *, struct device *, void *);
int acpihpet_activate(struct device *, int);
-
+void acpihpet_delay(int);
u_int acpihpet_gettime(struct timecounter *tc);
uint64_t acpihpet_r(bus_space_tag_t _iot, bus_space_handle_t _ioh,
@@ -266,11 +268,26 @@ acpihpet_attach(struct device *parent, s
hpet_timecounter.tc_priv = sc;
hpet_timecounter.tc_name = sc->sc_dev.dv_xname;
tc_init(&hpet_timecounter);
+
+ delay_init(acpihpet_delay, 2000);
+
#if defined(__amd64__)
extern void cpu_recalibrate_tsc(struct timecounter *);
cpu_recalibrate_tsc(&hpet_timecounter);
#endif
acpihpet_attached++;
+}
+
+void
+acpihpet_delay(int usecs)
+{
+ uint64_t c, s;
+ struct acpihpet_softc *sc = hpet_timecounter.tc_priv;
+
+ s = acpihpet_r(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER);
+ c = usecs * hpet_timecounter.tc_frequency / 1000000;
+ while (acpihpet_r(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER) - s < c)
+ CPU_BUSY_CYCLE();
}
u_int
Index: sys/dev/pv/pvbus.c
===================================================================
RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
retrieving revision 1.24
diff -u -p -r1.24 pvbus.c
--- sys/dev/pv/pvbus.c 5 Nov 2021 11:38:29 -0000 1.24
+++ sys/dev/pv/pvbus.c 25 Aug 2022 03:56:49 -0000
@@ -319,9 +319,8 @@ pvbus_hyperv(struct pvbus_hv *hv)
HYPERV_VERSION_EBX_MINOR_S;
#if NHYPERV > 0
- if (hv->hv_features & CPUID_HV_MSR_TIME_REFCNT &&
- delay_func == i8254_delay)
- delay_func = hv_delay;
+ if (hv->hv_features & CPUID_HV_MSR_TIME_REFCNT)
+ delay_init(hv_delay, 4000);
#endif
}
