Hi,
In the future when the LAPIC timer is run in oneshot mode there will
be no lapic_delay().
This is fine if you have a constant TSC, because we have tsc_delay().
This is *very* bad for older amd64 machines, because you are left with
i8254_delay().
I would like to offer a less awful delay(9) implementation for this
class of hardware. Otherwise we may trip over bizarre phantom bugs on
MP kernels because only one CPU can read the i8254 at a time.
I think patrick@ was struggling with some version of that problem last
year, but in a VM.
Real i386 hardware should be fine. Later models with an ACPI PM timer
will be fine using acpitimer_delay() instead of i8254_delay().
If this seems reasonable to people I will come back with a cleaned up
patch for testing.
Thoughts? Preferences?
-Scott
Here are the sample measurements from my 2017 laptop (kaby lake
refresh) running the attached patch. It takes longer than a
microsecond to read either of the ACPI timers. The PM timer is better
than the HPET. The HPET is a bit better than the i8254. I hope the
numbers are a little better on older hardware.
acpitimer_test_delay: expected 0.000001000 actual 0.000010638 error
0.000009638
acpitimer_test_delay: expected 0.000010000 actual 0.000015464 error
0.000005464
acpitimer_test_delay: expected 0.000100000 actual 0.000107619 error
0.000007619
acpitimer_test_delay: expected 0.001000000 actual 0.001007275 error
0.000007275
acpitimer_test_delay: expected 0.010000000 actual 0.010007891 error
0.000007891
acpihpet_test_delay: expected 0.000001000 actual 0.000022208 error
0.000021208
acpihpet_test_delay: expected 0.000010000 actual 0.000031690 error
0.000021690
acpihpet_test_delay: expected 0.000100000 actual 0.000112647 error
0.000012647
acpihpet_test_delay: expected 0.001000000 actual 0.001021480 error
0.000021480
acpihpet_test_delay: expected 0.010000000 actual 0.010013736 error
0.000013736
i8254_test_delay: expected 0.000001000 actual 0.000040110 error
0.000039110
i8254_test_delay: expected 0.000010000 actual 0.000039471 error
0.000029471
i8254_test_delay: expected 0.000100000 actual 0.000128031 error
0.000028031
i8254_test_delay: expected 0.001000000 actual 0.001024586 error
0.000024586
i8254_test_delay: expected 0.010000000 actual 0.010021859 error
0.000021859
Index: 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
--- dev/acpi/acpihpet.c 6 Apr 2022 18:59:27 -0000 1.26
+++ dev/acpi/acpihpet.c 15 Aug 2022 04:21:58 -0000
@@ -18,6 +18,7 @@
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/device.h>
+#include <sys/stdint.h>
#include <sys/timetc.h>
#include <machine/bus.h>
@@ -31,8 +32,9 @@ 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 acpiphet_delay(u_int);
u_int acpihpet_gettime(struct timecounter *tc);
+void acpihpet_test_delay(u_int);
uint64_t acpihpet_r(bus_space_tag_t _iot, bus_space_handle_t _ioh,
bus_size_t _ioa);
@@ -262,7 +264,7 @@ 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;
hpet_timecounter.tc_priv = sc;
hpet_timecounter.tc_name = sc->sc_dev.dv_xname;
tc_init(&hpet_timecounter);
@@ -273,10 +275,43 @@ acpihpet_attach(struct device *parent, s
acpihpet_attached++;
}
+void
+acpihpet_delay(u_int usecs)
+{
+ uint64_t d, s;
+ struct acpihpet_softc *sc = hpet_timecounter.tc_priv;
+
+ d = usecs * hpet_timecounter.tc_frequency / 1000000;
+ s = acpihpet_r(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER);
+ while (acpihpet_r(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER) - s < d)
+ CPU_BUSY_CYCLE();
+}
+
u_int
acpihpet_gettime(struct timecounter *tc)
{
struct acpihpet_softc *sc = tc->tc_priv;
return (bus_space_read_4(sc->sc_iot, sc->sc_ioh, HPET_MAIN_COUNTER));
+}
+
+void
+acpihpet_test_delay(u_int usecs)
+{
+ struct timespec ac, er, ex, t0, t1;
+
+ if (!acpihpet_attached) {
+ printf("%s: (no hpet attached)\n", __func__);
+ return;
+ }
+
+ nanouptime(&t0);
+ acpihpet_delay(usecs);
+ nanouptime(&t1);
+ timespecsub(&t1, &t0, &ac);
+ NSEC_TO_TIMESPEC(usecs * 1000ULL, &ex);
+ timespecsub(&ac, &ex, &er);
+ printf("%s: expected %lld.%09ld actual %lld.%09ld error %lld.%09ld\n",
+ __func__, ex.tv_sec, ex.tv_nsec, ac.tv_sec, ac.tv_nsec,
+ er.tv_sec, er.tv_nsec);
}
Index: 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
--- dev/acpi/acpitimer.c 6 Apr 2022 18:59:27 -0000 1.15
+++ dev/acpi/acpitimer.c 15 Aug 2022 04:21:58 -0000
@@ -25,10 +25,14 @@
#include <dev/acpi/acpireg.h>
#include <dev/acpi/acpivar.h>
+struct acpitimer_softc;
+
int acpitimermatch(struct device *, void *, void *);
void acpitimerattach(struct device *, struct device *, void *);
-
+void acpitimer_delay(u_int);
+void acpitimer_test_delay(u_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,
@@ -104,12 +108,34 @@ acpitimerattach(struct device *parent, s
#endif
}
+void
+acpitimer_delay(u_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);
@@ -120,4 +146,20 @@ acpi_get_timecount(struct timecounter *t
} while (u1 > u2 || u2 > u3);
return (u2);
+}
+
+void
+acpitimer_test_delay(u_int usecs)
+{
+ struct timespec ac, er, ex, t0, t1;
+
+ nanouptime(&t0);
+ acpitimer_delay(usecs);
+ nanouptime(&t1);
+ timespecsub(&t1, &t0, &ac);
+ NSEC_TO_TIMESPEC(usecs * 1000ULL, &ex);
+ timespecsub(&ac, &ex, &er);
+ printf("%s: expected %lld.%09ld actual %lld.%09ld error %lld.%09ld\n",
+ __func__, ex.tv_sec, ex.tv_nsec, ac.tv_sec, ac.tv_nsec,
+ er.tv_sec, er.tv_nsec);
}
Index: arch/amd64/amd64/lapic.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.59
diff -u -p -r1.59 lapic.c
--- arch/amd64/amd64/lapic.c 31 Aug 2021 15:53:36 -0000 1.59
+++ arch/amd64/amd64/lapic.c 15 Aug 2022 04:21:58 -0000
@@ -468,6 +468,19 @@ lapic_initclocks(void)
lapic_startclock();
i8254_inittimecounter_simple();
+
+ extern void acpitimer_test_delay(u_int);
+ extern void acpihpet_test_delay(u_int);
+ extern void i8254_test_delay(u_int);
+ u_int usec[] = { 1, 10, 100, 1000, 10000 };
+ size_t i;
+ delay(20000); /* wait for real timecounter to activate */
+ for (i = 0; i < nitems(usec); i++)
+ acpitimer_test_delay(usec[i]);
+ for (i = 0; i < nitems(usec); i++)
+ acpihpet_test_delay(usec[i]);
+ for (i = 0; i < nitems(usec); i++)
+ i8254_test_delay(usec[i]);
}
Index: arch/amd64/isa/clock.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/isa/clock.c,v
retrieving revision 1.36
diff -u -p -r1.36 clock.c
--- arch/amd64/isa/clock.c 13 Feb 2022 19:15:09 -0000 1.36
+++ arch/amd64/isa/clock.c 15 Aug 2022 04:21:59 -0000
@@ -266,6 +266,22 @@ i8254_delay(int n)
}
void
+i8254_test_delay(u_int usecs)
+{
+ struct timespec ac, er, ex, t0, t1;
+
+ nanouptime(&t0);
+ i8254_delay(usecs);
+ nanouptime(&t1);
+ timespecsub(&t1, &t0, &ac);
+ NSEC_TO_TIMESPEC(usecs * 1000ULL, &ex);
+ timespecsub(&ac, &ex, &er);
+ printf("%s: expected %lld.%09ld actual %lld.%09ld error %lld.%09ld\n",
+ __func__, ex.tv_sec, ex.tv_nsec, ac.tv_sec, ac.tv_nsec,
+ er.tv_sec, er.tv_nsec);
+}
+
+void
rtcdrain(void *v)
{
struct timeout *to = (struct timeout *)v;
