> Date: Fri, 16 Dec 2022 16:28:03 -0600
> From: Scott Cheloha <[email protected]>
>
> 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.
No, I can't spot the bug. Unless reading STICK_REG_LOW has some of
the bits in the upper half of the 64-bit register set...
> miod: Can you confirm that this patch still works? I tweaked it to
> match the existing assembly like you asked.
>
> ok?
ok kettenis@
> 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)
>
>
