Re: Using %stick where available

>>> Michael <[email protected]> wrote

> Hello,
> 
> the attached patch adds support for the system timer interrupt present  
> in UltraSPARC-III and some later II ( like IIe and IIi with on chip  
> ecache ). It hasn't seen much testing beyond 'works on my Blade 2500'.  
> The purpose is to have a timer interrupt / time counter that's  
> independent of the CPU's clock rate, so we can change it without  
> worrying about time keeping.

It looks ok to me, but as Eric reported it needs some fix.


(snip)
> Index: sparc64/clock.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/clock.c,v
> retrieving revision 1.106
> diff -u -w -r1.106 clock.c
> --- sparc64/clock.c	4 Sep 2011 12:17:46 -0000	1.106
> +++ sparc64/clock.c	6 Nov 2012 19:20:24 -0000
> @@ -99,6 +99,7 @@
>   *  counter-timer	 timer#0	 timer#1	 %tick
>   *  counter-timer + SMP	 timer#0/%tick	 -		 timer#1 or %tick
>   *  no counter-timer	 %tick		 -		 %tick
> + *  US-IIIi		 %stick		 -		 %stick
>   */
>  
>  /*
> @@ -137,6 +138,7 @@
>  int timerblurb = 10; /* Guess a value; used before clock is attached */
>  
>  static u_int tick_get_timecount(struct timecounter *);
> +static u_int stick_get_timecount(struct timecounter *);
>  
>  /*
>   * define timecounter "tick-counter"
> @@ -153,6 +155,17 @@
>  	NULL			/* next timecounter */
>  };
>  
> +static struct timecounter stick_timecounter = {
> +	stick_get_timecount,	/* get_timecount */
> +	0,			/* no poll_pps */
> +	~0u,			/* counter_mask */
> +	0,                      /* frequency - set at initialisation */
> +	"stick-counter",	/* name */
> +	100,			/* quality */
> +	0,			/* private reference - UNUSED */
> +	NULL			/* next timecounter */
> +};
> +
>  /*
>   * tick_get_timecount provide current tick counter value
>   */
> @@ -162,6 +175,12 @@
>  	return cpu_counter();
>  }
>  
> +static u_int
> +stick_get_timecount(struct timecounter *tc)
> +{
> +	return getstick();
> +}
> +
>  #ifdef MULTIPROCESSOR
>  static u_int counter_get_timecount(struct timecounter *);
>  
> @@ -329,6 +348,27 @@
>  	intr_restore(s);
>  }
>  
> +void
> +stickintr_establish(int pil, int (*fun)(void *))
> +{
> +	int s;
> +	struct intrhand *ih;
> +	struct cpu_info *ci = curcpu();
> +
> +	ih = sparc_softintr_establish(pil, fun, NULL);
> +	ih->ih_number = 1;
> +	if (CPU_IS_PRIMARY(ci))
> +		intr_establish(pil, true, ih);
> +	ci->ci_tick_ih = ih;
> +
> +	/* set the next interrupt time */
> +	ci->ci_tick_increment = ci->ci_system_clockrate[0] / hz;
> +
> +	s = intr_disable();
> +	next_stick(ci->ci_tick_increment);
> +	intr_restore(s);
> +}
> +
>  /*
>   * Set up the real-time and statistics clocks.  Leave stathz 0 only if
>   * no alternative timer is available.
> @@ -338,6 +378,7 @@
>  void
>  cpu_initclocks(void)
>  {
> +	struct cpu_info *ci = curcpu();

Replace the following curcpu()s with ci.

>  #ifndef MULTIPROCESSOR
>  	int statint, minint;
>  #endif
> @@ -370,8 +411,15 @@
>  	/* Initialize the %tick register */
>  	settick(0);
>  
> +	if (ci->ci_system_clockrate[0] == 0) {
>  	tick_timecounter.tc_frequency = curcpu()->ci_cpu_clockrate[0];
                                        ~~~~~~~~
>  	tc_init(&tick_timecounter);
> +	} else {
> +		setstick(0);
> +		stick_timecounter.tc_frequency = 
> +		    curcpu()->ci_system_clockrate[0];
                    ~~~~~~~~
> +		tc_init(&stick_timecounter);
> +	}
>  
>  	/*
>  	 * Now handle machines w/o counter-timers.
> @@ -379,13 +427,21 @@
>  
>  	if (!timerreg_4u.t_timer || !timerreg_4u.t_clrintr) {
>  
> -		aprint_normal("No counter-timer -- using %%tick at %luMHz as "
> -			"system clock.\n",
> +		if (ci->ci_system_clockrate[0] == 0) {
> +			aprint_normal("No counter-timer -- using %%tick "
> +			    "at %luMHz as system clock.\n",
>  			(unsigned long)curcpu()->ci_cpu_clockrate[1]);
                                       ~~~~~~~~
>  
>  		/* We don't have a counter-timer -- use %tick */
>  		tickintr_establish(PIL_CLOCK, tickintr);
> +		} else {
> +			aprint_normal("No counter-timer -- using %%stick "
> +			    "at %luMHz as system clock.\n",
> +			    (unsigned long)curcpu()->ci_system_clockrate[1]);
                                           ~~~~~~~~
>  
> +			/* We don't have a counter-timer -- use %tick */
> +			stickintr_establish(PIL_CLOCK, stickintr);
> +		}
>  		/* We only have one timer so we have no statclock */
>  		stathz = 0;
>  
> @@ -525,6 +581,22 @@
>  	return (1);
>  }
>  
> +int
> +stickintr(void *cap)
> +{
> +	int s;
> +
> +	hardclock((struct clockframe *)cap);
> +
> +	s = intr_disable();
> +	/* Reset the interrupt */
> +	next_stick(curcpu()->ci_tick_increment);
> +	intr_restore(s);
> +	curcpu()->ci_tick_evcnt.ev_count++;
> +
> +	return (1);
> +}
> +
>  #ifndef MULTIPROCESSOR
>  /*
>   * Level 14 (stat clock) interrupt handler.
> Index: sparc64/cpu.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/cpu.c,v
> retrieving revision 1.102
> diff -u -w -r1.102 cpu.c
> --- sparc64/cpu.c	27 Oct 2012 17:18:12 -0000	1.102
> +++ sparc64/cpu.c	6 Nov 2012 19:20:24 -0000
> @@ -250,7 +250,7 @@
>  	int bigcache, cachesize;
>  	char buf[100];
>  	int 	totalsize = 0;
> -	int 	linesize, dcachesize, icachesize;
> +	int 	linesize, dcachesize, icachesize, sclk;

I think the following is better for consistency.

-	long clk;
+	long clk, sclk;

>  
>  	/* tell them what we have */
>  	node = ma->ma_node;
> @@ -299,12 +299,21 @@
>  		ci->ci_cpu_clockrate[1] = clk / 1000000;
>  	}
>  
> +	sclk = prom_getpropint(findroot(), "stick-frequency", 0);
> +	ci->ci_system_clockrate[0] = sclk;
> +	ci->ci_system_clockrate[1] = sclk / 1000000;

US-IIe has system tick register, but its implementation is
different to US-III one.  It can be used via memory mapped system
registers, not via ancillary state register (%asr24).

So, I suggest not to use it on US-IIe as below.

	if (!CPU_IS_HUMMINGBIRD()) {
		sclk = prom_getpropint(findroot(), "stick-frequency", 0);
		ci->ci_system_clockrate[0] = sclk;
		ci->ci_system_clockrate[1] = sclk / 1000000;
	}

And, put the following into include/psl.h.

#define CPU_IS_HUMMINGBIRD()	(GETVER_CPU_IMPL() == IMPL_HUMMINGBIRD)


> +
>  	snprintf(buf, sizeof buf, "%s @ %s MHz",
>  		prom_getpropstring(node, "name"), clockfreq(clk));
>  	snprintf(cpu_model, sizeof cpu_model, "%s (%s)", machine_model, buf);
>  
>  	aprint_normal(": %s, UPA id %d\n", buf, ci->ci_cpuid);
>  	aprint_naive("\n");
> +
> +	if (ci->ci_system_clockrate[0] != 0) {
> +		aprint_normal_dev(dev, "system tick frequency %d MHz\n", 
> +		    (int)ci->ci_system_clockrate[1]);
> +	}
>  	aprint_normal_dev(dev, "");
>  
>  	bigcache = 0;
> @@ -452,6 +461,8 @@
>  		sync_tick = 1;
>  		membar_Sync();
>  		settick(0);
> +		if (ci->ci_system_clockrate[0] != 0)
> +			setstick(0);
>  
>  		setpstate(pstate);
>  
> @@ -480,8 +491,12 @@
>  		/* we do nothing here */
>  	}
>  	settick(0);
> -
> +	if (curcpu()->ci_system_clockrate[0] != 0) {
> +		setstick(0);
> +		stickintr_establish(PIL_CLOCK, stickintr);
> +	} else {
>  	tickintr_establish(PIL_CLOCK, tickintr);
> +	}
>  	spl0();
>  }
>  #endif /* MULTIPROCESSOR */
> Index: sparc64/genassym.cf
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/genassym.cf,v
> retrieving revision 1.66
> diff -u -w -r1.66 genassym.cf
> --- sparc64/genassym.cf	20 Jul 2011 12:06:00 -0000	1.66
> +++ sparc64/genassym.cf	6 Nov 2012 19:20:24 -0000
> @@ -157,6 +157,7 @@
>  define	CI_NFAULT	offsetof(struct cpu_info, ci_data.cpu_nfault)
>  define	CI_NINTR	offsetof(struct cpu_info, ci_data.cpu_nintr)
>  define	CI_CLOCKRATE	offsetof(struct cpu_info, ci_cpu_clockrate)
> +define	CI_SYSCLOCKRATE	offsetof(struct cpu_info, ci_system_clockrate)
>  define	CI_IDEPTH	offsetof(struct cpu_info, ci_idepth)
>  define	CI_INTRPENDING	offsetof(struct cpu_info, ci_intrpending)
>  define	CI_TICK_IH	offsetof(struct cpu_info, ci_tick_ih)

CI_SYSCLOCKRATE is unused.


> Index: sparc64/locore.s
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/locore.s,v
> retrieving revision 1.341
> diff -u -w -r1.341 locore.s
> --- sparc64/locore.s	17 Mar 2012 22:19:53 -0000	1.341
> +++ sparc64/locore.s	6 Nov 2012 19:20:25 -0000
> @@ -3270,13 +3270,18 @@
>  	wrpr	%g0, PSTATE_KERN|PSTATE_IG, %pstate	! DEBUG
>  #endif
>  	/*
> -	 * If this is a %tick softint, clear it then call interrupt_vector.
> +	 * If this is a %tick or %stick softint, clear it then call
> +	 * interrupt_vector. Only one of them should be enabled at any given
> +	 * time.
>  	 */
>  	rd	SOFTINT, %g1
> -	btst	1, %g1
> +	mov	1, %g5
> +	sllx	%g5, 16, %g3
> +	or	%g5, %g3, %g5
> +	andcc	%g5, %g1, %g5
>  	bz,pt	%icc, 0f
>  	 sethi	%hi(CPUINFO_VA+CI_TICK_IH), %g3
> -	wr	%g0, 1, CLEAR_SOFTINT
> +	wr	%g0, %g5, CLEAR_SOFTINT
>  	ba,pt	%icc, setup_sparcintr
>  	 LDPTR	[%g3 + %lo(CPUINFO_VA+CI_TICK_IH)], %g5
>  0:
> @@ -6058,6 +6063,93 @@
>  	 wr	%o2, TICK_CMPR
>  #endif
>  
> +/*
> + * setstick(long)
> + */
> +ENTRY(setstick)
> +	retl
> +	 wr %o0, STICK
> +
> +/*
> + * long getstick(void)
> + */
> +ENTRY(getstick)
> +	retl
> +	 rd STICK, %o0

I'll replace them with inline functions like settick/gettick after
you commited.


> +
> +/*
> + * next_stick(long increment)
> + *
> + * Sets the %stick_cmpr register to fire off in `increment' machine
> + * cycles in the future.  Also handles %stick wraparound.  In 32-bit
> + * mode we're limited to a 32-bit increment.
> + */
> +ENTRY(next_stick)
> +	rd	STICK_CMPR, %o2
> +	rd	STICK, %o1
> +
> +	mov	1, %o3		! Mask off high bits of these registers
> +	sllx	%o3, 63, %o3
> +	andn	%o1, %o3, %o1
> +	andn	%o2, %o3, %o2
> +	cmp	%o1, %o2	! Did we wrap?  (tick < tick_cmpr)
> +	bgt,pt	%icc, 1f
> +	 add	%o1, 1000, %o1	! Need some slack so we don't lose intrs.
> +
> +	/*
> +	 * Handle the unlikely case of %stick wrapping.
> +	 *
> +	 * This should only happen every 10 years or more.
> +	 *
> +	 * We need to increment the time base by the size of %stick in
> +	 * microseconds.  This will require some divides and multiplies
> +	 * which can take time.  So we re-read %stick.
> +	 *
> +	 */
> +
> +	/* XXXXX NOT IMPLEMENTED */
> +
> +
> +
> +1:
> +	add	%o2, %o0, %o2
> +	andn	%o2, %o3, %o4
> +	brlz,pn	%o4, Lstick_ovflw
> +	 cmp	%o2, %o1	! Has this stick passed?
> +	blt,pn	%xcc, 1b	! Yes
> +	 nop
> +
> +#ifdef BB_ERRATA_1
> +	ba,a	2f
> +	 nop
> +#else
> +	retl
> +	 wr	%o2, STICK_CMPR
> +#endif
> +
> +Lstick_ovflw:
> +/*
> + * When we get here tick_cmpr has wrapped, but we don't know if %stick
> + * has wrapped.  If bit 62 is set then we have not wrapped and we can
> + * use the current value of %o4 as %stick.  Otherwise we need to return
> + * to our loop with %o4 as %stick_cmpr (%o2).
> + */
> +	srlx	%o3, 1, %o5
> +	btst	%o5, %o1
> +	bz,pn	%xcc, 1b
> +	 mov	%o4, %o2
> +#ifdef BB_ERRATA_1
> +	ba,a	2f
> +	 nop
> +	.align	64
> +2:	wr	%o2, STICK_CMPR
> +	rd	STICK_CMPR, %g0
> +	retl
> +	 nop
> +#else
> +	retl
> +	 wr	%o2, STICK_CMPR
> +#endif
>  
>  ENTRY(setjmp)
>  	save	%sp, -CC64FSZ, %sp	! Need a frame to return to.

BB_ERRATA_1 is an errata of US-II, so #ifdef BB_ERRATA_1 cases are
unnecessary.


-- Takeshi Nakayama