miod@'s UltraSPARC IIe machine really, really hates the following
code:
stickcmpr_set(stick());
When we try that code, invariably the clock interrupt does not fire
and the machine hangs.
I think stickcmpr_set() fails to ensure that %STICK_CMPR is larger
than %STICK before returning to the caller. That, or miod@'s machine
is weird. In either case, we've been stuck for a few weeks trying to
figure it out.
After some back and forth, we eventually tried pulling the retry logic
out of stickcmpr_set() into a C function. It worked! His UltraSPARC
IIe machine (probably a Sun Blade 100?) finally booted. That patch is
attached below.
If this change is accepted I intend to change tickcmpr_set() and
sys_tickcmpr_set() in identical ways to keep the three clocks' code
similar.
I am including this bit:
> @@ -920,9 +922,24 @@ stick_start(void)
> */
>
> s = intr_disable();
> - ci->ci_tick = roundup(stick(), tick_increment);
> - stickcmpr_set(ci->ci_tick);
> + ci->ci_tick = stick();
> + stick_rearm(ci->ci_tick);
... to demonstrate that the new logic actually works. I suspect that
the retry logic in stickcmpr_set() has a bug that has remained hidden
by roundup() since it was committed in 2008.
kettenis: You wrote stickcmpr_set(). Am I nuts? Can you spot a bug
in in stickcmpr_set()? I cannot, but OTOH I was born yesterday.
miod: Can you confirm that this patch still works? I tweaked it to
match the existing assembly like you asked.
ok?
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 16 Dec 2022 22:25:06 -0000
@@ -150,6 +150,8 @@ void tick_start(void);
void sys_tick_start(void);
void stick_start(void);
+void stick_rearm(uint64_t);
+
int tickintr(void *);
int sys_tickintr(void *);
int stickintr(void *);
@@ -810,7 +812,7 @@ stickintr(void *cap)
/* Reset the interrupt. */
s = intr_disable();
- stickcmpr_set(ci->ci_tick);
+ stick_rearm(ci->ci_tick);
intr_restore(s);
return (1);
@@ -920,9 +922,24 @@ stick_start(void)
*/
s = intr_disable();
- ci->ci_tick = roundup(stick(), tick_increment);
- stickcmpr_set(ci->ci_tick);
+ ci->ci_tick = stick();
+ stick_rearm(ci->ci_tick);
intr_restore(s);
+}
+
+void
+stick_rearm(uint64_t cmp)
+{
+ uint64_t now, off = 8;
+
+ stickcmpr_set(cmp);
+ now = stick();
+ while (cmp <= now) {
+ cmp += off;
+ stickcmpr_set(cmp);
+ now = stick();
+ off *= 2;
+ }
}
u_int
Index: locore.s
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/locore.s,v
retrieving revision 1.194
diff -u -p -r1.194 locore.s
--- locore.s 8 Dec 2022 01:25:45 -0000 1.194
+++ locore.s 16 Dec 2022 22:25:07 -0000
@@ -7568,26 +7568,10 @@ END(stick)
ENTRY(stickcmpr_set)
setx STICK_CMP_HIGH, %o1, %o3
- mov 8, %o2 ! Initial step size
-1:
srlx %o0, 32, %o1
stxa %o1, [%o3] ASI_PHYS_NON_CACHED
add %o3, (STICK_CMP_LOW - STICK_CMP_HIGH), %o4
stxa %o0, [%o4] ASI_PHYS_NON_CACHED
-
- add %o3, (STICK_REG_LOW - STICK_CMP_HIGH), %o4
- ldxa [%o4] ASI_PHYS_NON_CACHED, %o1
- add %o3, (STICK_REG_HIGH - STICK_CMP_HIGH), %o4
- ldxa [%o4] ASI_PHYS_NON_CACHED, %o5
- sllx %o5, 32, %o5
- or %o1, %o5, %o1
-
- cmp %o0, %o1 ! Make sure the value we wrote
- bg,pt %xcc, 2f ! was in the future
- add %o0, %o2, %o0 ! If not, add the step size, double
- ba,pt %xcc, 1b ! the step size and try again.
- sllx %o2, 1, %o2
-2:
retl
nop
END(stickcmpr_set)
