On Tue, Aug 16, 2022 at 11:53:51AM -0500, Scott Cheloha wrote:
> On Sun, Aug 14, 2022 at 11:24:37PM -0500, Scott Cheloha wrote:
> >
> > In the future when the LAPIC timer is run in oneshot mode there will
> > be no lapic_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.
> >
> > [...]
> >
> > Real i386 hardware should be fine. Later models with an ACPI PM timer
> > will be fine using acpitimer_delay() instead of i8254_delay().
> >
> > [...]
>
> Attched is an updated patch. I left the test measurement code in
> place because I would like to see a test on a real i386 machine, just
> to make sure it works as expected. I can't imagine why it wouldn't
> work, but we should never assume anything.
>
> [...]
>
> One remaining question I have:
>
> Is there a nice way to test whether ACPI PMT support is compiled into
> the kernel? We can assume the existence of i8254_delay() because
> clock.c is required on amd64 and i386. However, acpitimer.c is a
> optional, so acpitimer_delay() isn't necessarily there.
>
> I would rather not introduce a hard requirement on acpitimer.c into
> acpihpet.c if there's an easy way to check for the latter.
>
> Any ideas?
And here's the cleaned up patch. Just in case nobody tests i386.
Pretty straightforward. acpitimer is preferable to i8254, hpet is
preferable to acpitimer and i8254.
The only obvious problem I see is the hard dependency this creates in
acpihpet.c on acpitimer.c.
Index: acpitimer.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpitimer.c,v
retrieving revision 1.15
diff -u -p -r1.15 acpitimer.c
--- acpitimer.c 6 Apr 2022 18:59:27 -0000 1.15
+++ acpitimer.c 17 Aug 2022 02:56:10 -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>
@@ -25,10 +26,13 @@
#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(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 +102,45 @@ acpitimerattach(struct device *parent, s
acpi_timecounter.tc_priv = sc;
acpi_timecounter.tc_name = sc->sc_dev.dv_xname;
tc_init(&acpi_timecounter);
+
+#if defined(__amd64__) || defined(__i386__)
+ if (delay_func == i8254_delay)
+ delay_func = acpitimer_delay;
+#endif
#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: acpihpet.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpihpet.c,v
retrieving revision 1.26
diff -u -p -r1.26 acpihpet.c
--- acpihpet.c 6 Apr 2022 18:59:27 -0000 1.26
+++ acpihpet.c 17 Aug 2022 02:56:10 -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,7 +32,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 +263,37 @@ 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);
+
+#if defined(__amd64__) || defined(__i386__)
+ if (delay_func == i8254_delay)
+ delay_func = acpihpet_delay;
+ /* XXX what if the kernel has no acpitimer support? */
+ extern void acpitimer_delay(int);
+ if (delay_func == acpitimer_delay)
+ delay_func = acpihpet_delay;
+#endif
+
#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
