On Tue, Sep 01, 2020 at 06:14:05PM +0200, Mark Kettenis wrote:
> > Date: Tue, 1 Sep 2020 11:05:26 -0500
> > From: Scott Cheloha
> >
> > Hi,
> >
> > At boot, if we don't know the lapic frequency offhand we compute it by
> > waiting for a known clock (the i8254) with a known frequency to cycle
> > a few times.
> >
> > Currently we cycle hz times. This doesn't make sense. There is
> > little to no benefit to waiting additional cycles if your kernel is
> > compiled with a larger HZ. Mostly it just makes the calibration take
> > longer.
> >
> > Consider the common HZ=1000 case. What is the benefit of looping an
> > additional 900 times? The point of diminishing returns is well under
> > 1000 loops.
> >
> > 20-50 loops is probably sufficient to limit our error, but I don't
> > want to break anything so let's use 100, like we do on default
> > kernels.
> >
> > ok?
>
> Sorry, but this makes no sense to me. The current code waits for 1
> second regarless of what HZ is.
>
> And I expect that the accuracy of the measurement depends on the total
> number elapsed time, so I expect a less acurate results if you only
> wait 100 cycles at HZ=1000 (which is 0.1 second).
Whoops.
Yes, you're right, I'm having a slow moment. Nevermind that last
patch.
But this gives me an idea.
Given that we're waiting the same amount of time (1 second) no matter
the value of hz, and we're using the same clock to do the wait... why
are we fussing with the inner workings of reading the i8254? We have
a function for this: i8254_delay().
If we just use that and tell it to wait a million microseconds we can
throw out a bunch of code.
Index: lapic.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.55
diff -u -p -r1.55 lapic.c
--- lapic.c 3 Aug 2019 14:57:51 - 1.55
+++ lapic.c 1 Sep 2020 16:40:39 -
@@ -451,24 +451,6 @@ lapic_initclocks(void)
i8254_inittimecounter_simple();
}
-
-extern int gettick(void); /* XXX put in header file */
-extern u_long rtclock_tval; /* XXX put in header file */
-
-static __inline void
-wait_next_cycle(void)
-{
- unsigned int tick, tlast;
-
- tlast = (1 << 16); /* i8254 counter has 16 bits at most */
- for (;;) {
- tick = gettick();
- if (tick > tlast)
- return;
- tlast = tick;
- }
-}
-
/*
* Calibrate the local apic count-down timer (which is running at
* bus-clock speed) vs. the i8254 counter/timer (which is running at
@@ -484,7 +466,7 @@ void
lapic_calibrate_timer(struct cpu_info *ci)
{
unsigned int startapic, endapic;
- u_int64_t dtick, dapic, tmp;
+ u_int64_t tmp;
u_long s;
int i;
@@ -504,29 +486,13 @@ lapic_calibrate_timer(struct cpu_info *c
s = intr_disable();
- /* wait for current cycle to finish */
- wait_next_cycle();
-
startapic = lapic_gettick();
-
- /* wait the next hz cycles */
- for (i = 0; i < hz; i++)
- wait_next_cycle();
-
+ i8254_delay(100);
endapic = lapic_gettick();
intr_restore(s);
- dtick = hz * rtclock_tval;
- dapic = startapic-endapic;
-
- /*
-* there are TIMER_FREQ ticks per second.
-* in dtick ticks, there are dapic bus clocks.
-*/
- tmp = (TIMER_FREQ * dapic) / dtick;
-
- lapic_per_second = tmp;
+ lapic_per_second = startapic - endapic;
skip_calibration:
printf("%s: apic clock running at %dMHz\n",
