On Sat, Dec 10, 2022 at 09:33:55AM -0700, Theo de Raadt wrote:
> Mark Kettenis <[email protected]> wrote:
>
> > So really the question is whether we should register the "tick"
> > timecounter if we also provide the "stick" or "sys_tick" timecounter.
> >
> > Personaly I think the code we currently have is fine. If you make the
> > sconscious decision to use "tick" on a system that provides "stick" or
> > "sys_tick" you'd better make the conscious decision not to use the CPU
> > frequency scaling stuff.
>
> Right. Is it really any different than choosing a clock with known or unknown
> quirks on an x86 machine:
>
> kern.timecounter.choice=i8254(0) tsc(2000) acpihpet0(1000) acpitimer0(1000)
I think this situation is different from the one on i386/amd64.
When we offer a timecounter to the end user, we're saying (1) this
clock is monotonic, and (2) this clock counts at a fixed frequency.
The quality number then gives you a hint about which clock is best.
So, yes, some of those x86 clocks are slow compared with the TSC, but
they still count time correctly.
What I'm hearing from kettenis@ is that %tick might have a variable
frequency on SPARC systems that have %sys_tick. If this is true, then
%tick is not a real timecounter on those systems and we should not
offer it as a choice to the end user.
We have a suitable timecounter on those systems, %sys_tick or the
PCI-based %stick... why offer a second clock we know is unsuitable?
It just leaves room for end-user error. It's a footgun.
Index: clock.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/clock.c,v
retrieving revision 1.72
diff -u -p -r1.72 clock.c
--- clock.c 10 Nov 2022 07:08:01 -0000 1.72
+++ clock.c 10 Dec 2022 17:15:55 -0000
@@ -567,21 +567,28 @@ cpu_initclocks(void)
/* Default to 200MHz clock XXXXX */
cpu_clockrate = 200000000;
- tick_timecounter.tc_frequency = cpu_clockrate;
- tc_init(&tick_timecounter);
-
- /*
- * UltraSPARC IIe processors do have a STICK register, but it
- * lives on the PCI host bridge and isn't accessible through
- * ASR24.
- */
if (CPU_ISSUN4U || CPU_ISSUN4US)
impl = (getver() & VER_IMPL) >> VER_IMPL_SHIFT;
+ /*
+ * The %tick frequency is variable on systems with a %sys_tick
+ * register, so if stick-frequency is non-zero we don't install
+ * tick_timecounter.
+ *
+ * UltraSPARC IIe ("Hummingbird") processors have a %sys_tick
+ * register, but it lives on the PCI host bridge and isn't
+ * accessible through ASR24. psycho(4) provides a special
+ * timecounter for those systems.
+ */
sys_tick_rate = getpropint(findroot(), "stick-frequency", 0);
- if (sys_tick_rate > 0 && impl != IMPL_HUMMINGBIRD) {
- sys_tick_timecounter.tc_frequency = sys_tick_rate;
- tc_init(&sys_tick_timecounter);
+ if (sys_tick_rate > 0) {
+ if (impl != IMPL_HUMMINGBIRD) {
+ sys_tick_timecounter.tc_frequency = sys_tick_rate;
+ tc_init(&sys_tick_timecounter);
+ }
+ } else {
+ tick_timecounter.tc_frequency = cpu_clockrate;
+ tc_init(&tick_timecounter);
}
/*
