Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
On Sat, Dec 10, 2022 at 10:24:52AM -0700, Theo de Raadt wrote:
> Scott Cheloha wrote:
>
> > On Sat, Dec 10, 2022 at 09:33:55AM -0700, Theo de Raadt wrote:
> > > Mark Kettenis 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.
>
> Really. Are you willing to put money on that? That on x86, none of the
> clocks are variant anymore, especially after a resume? How much money.
> Anyone else want to put money on it? We'd be talking a 1 year long bet to
> discover a mistake...
There may be other problems remaining on x86, but that isn't relevant
to this thread.
Getting back to sparc64:
What I'm hearing from kettenis@ is that %tick might have a variable
frequency on *all* SPARC systems that have the %sys_tick register.
So, UltraSPARC IIe and everything after that.
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 (ASR24)
or the PCI-based %stick on the Hummingbird. You only need one working
timecounter to boot. There is no reason to *also* offer a %tick-based
timecounter if we know out of the gate that it doesn't have a constant
frequency.
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 - 1.72
+++ clock.c 10 Dec 2022 17:15:55 -
@@ -567,21 +567,28 @@ cpu_initclocks(void)
/* Default to 200MHz clock X */
cpu_clockrate = 2;
- tick_timecounter.tc_frequency = cpu_clockrate;
- tc_init(_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(_tick_timecounter);
+ if (sys_tick_rate > 0) {
+ if (impl != IMPL_HUMMINGBIRD) {
+ sys_tick_timecounter.tc_frequency = sys_tick_rate;
+ tc_init(_tick_timecounter);
+ }
+ } else {
+ tick_timecounter.tc_frequency = cpu_clockrate;
+ tc_init(_timecounter);
}
/*
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
Scott Cheloha wrote: > On Sat, Dec 10, 2022 at 09:33:55AM -0700, Theo de Raadt wrote: > > Mark Kettenis 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. Really. Are you willing to put money on that? That on x86, none of the clocks are variant anymore, especially after a resume? How much money. Anyone else want to put money on it? We'd be talking a 1 year long bet to discover a mistake...
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
On Sat, Dec 10, 2022 at 09:33:55AM -0700, Theo de Raadt wrote:
> Mark Kettenis 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 - 1.72
+++ clock.c 10 Dec 2022 17:15:55 -
@@ -567,21 +567,28 @@ cpu_initclocks(void)
/* Default to 200MHz clock X */
cpu_clockrate = 2;
- tick_timecounter.tc_frequency = cpu_clockrate;
- tc_init(_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(_tick_timecounter);
+ if (sys_tick_rate > 0) {
+ if (impl != IMPL_HUMMINGBIRD) {
+ sys_tick_timecounter.tc_frequency = sys_tick_rate;
+ tc_init(_tick_timecounter);
+ }
+ } else {
+ tick_timecounter.tc_frequency = cpu_clockrate;
+ tc_init(_timecounter);
}
/*
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
Mark Kettenis 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)
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
> Date: Sat, 10 Dec 2022 09:43:32 -0600
> From: Scott Cheloha
>
> On Sat, Dec 10, 2022 at 03:14:37PM +0100, Mark Kettenis wrote:
> > > Date: Fri, 9 Dec 2022 16:27:59 -0600
> > > From: Scott Cheloha
> > >
> > > The UltraSPARC IIe's %TICK register has a variable frequency. See
> > > section 2.3 in this document here:
> > >
> > > https://web.archive.org/web/20221028065731/https://www.oracle.com/technetwork/server-storage/sun-sparc-enterprise/documentation/ultrasparc-iie-2516664.pdf
> > >
> > > Timecounters need to have a constant frequency, so we should not
> > > install tick_timecounter if the implementation is an UltraSPARC IIe
> > > ("Hummingbird").
> > >
> > > As far as I know this issue is unique to the IIe. I can't find any
> > > reference to a varying %TICK frequency in the documentation for the
> > > IIi or the UltraSPARC III.
> > >
> > > miod@ confirmed that the problem is real.
> > >
> > > ok?
> >
> > I don't think so.
> >
> > IIRC, UltraSPARC IIi and UltraSPARC IIIi also have a potentially
> > varying %tick frequency.
>
> Where would this be written down? And how do we know that later
> revisions have a fixed %tick frequency?
>
> > Those chips implement %sys_tick though and
> > since we give the associated timecounter a higher quality, it wins.
>
> Gotcha.
Not sure if it is documented anywhere. But my interpretation has
always been that %sys_tick was introduced when Sun started building
SPARCv9 CPUs that had clock frequencies that could be changed. The
UltraSPARC-IIe processor was part of that transition. A timecounter
that didn't scale with the CPU frequency was introduced in the
hardware, but the %sys_tick ASR wasn't added to the instruction set
yet.
Anyway, I think one has to assume that when %sys_tick exists, %tick
may not run at a constant frequency.
> > With your diff UltraSPARC IIe will end up without a timecounter. That
> > would bad isn't it?
>
> We have a stick_timecounter in dev/psycho.c that uses stick(). Is
> that sufficient?
Ah, wait, that's the one that gets used. I probably wrote that code.
I'm getting old.
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.
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
On Sat, Dec 10, 2022 at 02:43:50PM +0100, Christian Weisgerber wrote: > Scott Cheloha: > > > The UltraSPARC IIe's %TICK register has a variable frequency. > > miod@ confirmed that the problem is real. > > Indeed, I remember clocking down the Blade 100 with apm(8). > > Somebody with access to the hardware might want to adapt the NetBSD > code that adds timecounter support for the STICK register. > Unfortunately it can't be read from userland. I have terrific news for you! Check out: sys/arch/sparc64/dev/psycho.c :)
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
On Sat, Dec 10, 2022 at 03:14:37PM +0100, Mark Kettenis wrote:
> > Date: Fri, 9 Dec 2022 16:27:59 -0600
> > From: Scott Cheloha
> >
> > The UltraSPARC IIe's %TICK register has a variable frequency. See
> > section 2.3 in this document here:
> >
> > https://web.archive.org/web/20221028065731/https://www.oracle.com/technetwork/server-storage/sun-sparc-enterprise/documentation/ultrasparc-iie-2516664.pdf
> >
> > Timecounters need to have a constant frequency, so we should not
> > install tick_timecounter if the implementation is an UltraSPARC IIe
> > ("Hummingbird").
> >
> > As far as I know this issue is unique to the IIe. I can't find any
> > reference to a varying %TICK frequency in the documentation for the
> > IIi or the UltraSPARC III.
> >
> > miod@ confirmed that the problem is real.
> >
> > ok?
>
> I don't think so.
>
> IIRC, UltraSPARC IIi and UltraSPARC IIIi also have a potentially
> varying %tick frequency.
Where would this be written down? And how do we know that later
revisions have a fixed %tick frequency?
> Those chips implement %sys_tick though and
> since we give the associated timecounter a higher quality, it wins.
Gotcha.
> With your diff UltraSPARC IIe will end up without a timecounter. That
> would bad isn't it?
We have a stick_timecounter in dev/psycho.c that uses stick(). Is
that sufficient?
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
> Date: Fri, 9 Dec 2022 16:27:59 -0600
> From: Scott Cheloha
>
> The UltraSPARC IIe's %TICK register has a variable frequency. See
> section 2.3 in this document here:
>
> https://web.archive.org/web/20221028065731/https://www.oracle.com/technetwork/server-storage/sun-sparc-enterprise/documentation/ultrasparc-iie-2516664.pdf
>
> Timecounters need to have a constant frequency, so we should not
> install tick_timecounter if the implementation is an UltraSPARC IIe
> ("Hummingbird").
>
> As far as I know this issue is unique to the IIe. I can't find any
> reference to a varying %TICK frequency in the documentation for the
> IIi or the UltraSPARC III.
>
> miod@ confirmed that the problem is real.
>
> ok?
I don't think so.
IIRC, UltraSPARC IIi and UltraSPARC IIIi also have a potentially
varying %tick frequency. Those chips implement %sys_tick though and
since we give the associated timecounter a higher quality, it wins.
With your diff UltraSPARC IIe will end up without a timecounter. That
would bad isn't it?
So I think there are two options:
1. Implement a timecounter for Hummingbird's STICK logic. Shouldn't
be too difficult as we have a stick() function that returns the
counter.
2. Disable the hummingbird_setperf() implementation.
Cheers,
Mark
> 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 - 1.72
> +++ clock.c 9 Dec 2022 22:19:33 -
> @@ -567,17 +567,23 @@ cpu_initclocks(void)
> /* Default to 200MHz clock X */
> cpu_clockrate = 2;
>
> - tick_timecounter.tc_frequency = cpu_clockrate;
> - tc_init(_timecounter);
> + if (CPU_ISSUN4U || CPU_ISSUN4US)
> + impl = (getver() & VER_IMPL) >> VER_IMPL_SHIFT;
> +
> + /*
> + * The TICK frequency varies on the UltraSPARc IIe, so it isn't
> + * a suitable timecounter.
> + */
> + if (impl != IMPL_HUMMINGBIRD) {
> + tick_timecounter.tc_frequency = cpu_clockrate;
> + tc_init(_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;
> -
> sys_tick_rate = getpropint(findroot(), "stick-frequency", 0);
> if (sys_tick_rate > 0 && impl != IMPL_HUMMINGBIRD) {
> sys_tick_timecounter.tc_frequency = sys_tick_rate;
>
>
Re: sparc64: don't install %TICK timecounter on UltraSPARC IIe
Scott Cheloha: > The UltraSPARC IIe's %TICK register has a variable frequency. > miod@ confirmed that the problem is real. Indeed, I remember clocking down the Blade 100 with apm(8). Somebody with access to the hardware might want to adapt the NetBSD code that adds timecounter support for the STICK register. Unfortunately it can't be read from userland. > ok? I don't have a sparc64 to test, but this makes sense. ok naddy@ -- Christian "naddy" Weisgerber na...@mips.inka.de
