On Sat, Dec 10, 2022 at 09:33:55AM -0700, Theo de Raadt wrote: > Mark Kettenis <mark.kette...@xs4all.nl> 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); } /*