On Thu, Mar 17, 2016 at 04:03:25PM +0900, Masao Uebayashi wrote: > On Wed, Mar 16, 2016 at 11:49:32PM -0700, Mike Larkin wrote: > > On Thu, Mar 17, 2016 at 03:38:20PM +0900, Masao Uebayashi wrote: > > > On Wed, Mar 16, 2016 at 11:25:11PM -0700, Mike Larkin wrote: > > > > On Thu, Mar 17, 2016 at 09:44:22AM +0900, Masao Uebayashi wrote: > > > > > Implement delay() using TSC > > > > > > > > > > - Calculate delay using 64-bit RDTSC instruction > > > > > - Enable tsc_delay() as delay(9) backend > > > > > - Use tsc_delay() only when TSC is invariant > > > > > - Configure tsc_delay() after primary CPU is identified > > > > > - Assume all CPUs are identical > > > > > > > > Why is this needed, or wanted? There is no explanation given above > > > > aside from "let's make things different". Are you claiming that > > > > delay(9) is somehow inaccurate or incorrect without this? > > > > > > I think LAPIC delay() is just fine. Accuracy is not a problem for me. > > > > > > In the long run, I want to use LAPIC for per-CPU one-shot timer. This > > > will come later ... but it'd be nice for me to know if using TSC for > > > delay() is not a problem. > > > > > > > So are you asking people to just test this to see if there are side > > effects (in order to switch to this "in the long run"), or are you > > asking for oks? > > > > The way the diff is written, it looks like if it were committed, it > > would force every machine to use TSC. Is that what you want? > > I don't need this to be committed & enabled for everyone right now, but > tests and feedbacks are really appreciated.
I'll put it on a few machines and let you know. -ml > > (Unfortunately using TSC for delay() itself has nothing visible or > beneficial for users.) > > > -ml > > > > > > -ml > > > > > > > > > > > > > > diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c > > > > > index df6c623..0863306 100644 > > > > > --- a/sys/arch/amd64/amd64/cpu.c > > > > > +++ b/sys/arch/amd64/amd64/cpu.c > > > > > @@ -330,6 +330,7 @@ cpu_attach(struct device *parent, struct device > > > > > *self, void *aux) > > > > > vaddr_t kstack; > > > > > struct pcb *pcb; > > > > > #endif > > > > > + void tsc_delay_init(struct cpu_info *); > > > > > > > > > > /* > > > > > * If we're an Application Processor, allocate a cpu_info > > > > > @@ -409,6 +410,7 @@ cpu_attach(struct device *parent, struct device > > > > > *self, void *aux) > > > > > #endif /* MTRR */ > > > > > cpu_init(ci); > > > > > cpu_init_mwait(sc); > > > > > + tsc_delay_init(ci); > > > > > break; > > > > > > > > > > case CPU_ROLE_BP: > > > > > @@ -432,6 +434,7 @@ cpu_attach(struct device *parent, struct device > > > > > *self, void *aux) > > > > > ioapic_bsp_id = caa->cpu_number; > > > > > #endif > > > > > cpu_init_mwait(sc); > > > > > + tsc_delay_init(ci); > > > > > break; > > > > > > > > > > case CPU_ROLE_AP: > > > > > diff --git a/sys/arch/amd64/amd64/tsc.c b/sys/arch/amd64/amd64/tsc.c > > > > > new file mode 100644 > > > > > index 0000000..15242ca > > > > > --- /dev/null > > > > > +++ b/sys/arch/amd64/amd64/tsc.c > > > > > @@ -0,0 +1,68 @@ > > > > > +/* > > > > > + * Copyright (c) 2016 Masao Uebayashi <[email protected]> > > > > > + * > > > > > + * Permission to use, copy, modify, and distribute this software for > > > > > any > > > > > + * purpose with or without fee is hereby granted, provided that the > > > > > above > > > > > + * copyright notice and this permission notice appear in all copies. > > > > > + * > > > > > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL > > > > > WARRANTIES > > > > > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > > > > > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE > > > > > LIABLE FOR > > > > > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY > > > > > DAMAGES > > > > > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER > > > > > IN AN > > > > > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING > > > > > OUT OF > > > > > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > > > > > + */ > > > > > + > > > > > +#include <sys/param.h> > > > > > +#include <sys/systm.h> > > > > > +#include <sys/stdint.h> > > > > > + > > > > > +#include <machine/cpu.h> > > > > > +#include <machine/cpufunc.h> > > > > > + > > > > > +void tsc_delay_init(struct cpu_info *); > > > > > +void tsc_delay(int); > > > > > +uint64_t tsc2usec(uint64_t n); > > > > > + > > > > > +int64_t tsc_delay_mult; > > > > > + > > > > > +void > > > > > +tsc_delay_init(struct cpu_info *ci) > > > > > +{ > > > > > +#ifdef DIAGNOSTIC > > > > > + KASSERT(ci->ci_tsc_freq != 0); > > > > > +#endif > > > > > + if ((ci->ci_flags & CPUF_CONST_TSC) == 0) > > > > > + return; > > > > > + > > > > > + tsc_delay_mult = ci->ci_tsc_freq / 1000000; > > > > > + delay_func = tsc_delay; > > > > > +} > > > > > + > > > > > +void > > > > > +tsc_delay(int usec) > > > > > +{ > > > > > + int64_t n; > > > > > + uint64_t now, prev; > > > > > + > > > > > + n = tsc_delay_mult * usec; > > > > > + prev = rdtsc(); > > > > > + while (n > 0) { > > > > > + CPU_BUSY_CYCLE(); > > > > > + now = rdtsc(); > > > > > + if (now < prev) > > > > > + n -= UINT64_MAX - (prev - now); > > > > > + else > > > > > + n -= now - prev; > > > > > + prev = now; > > > > > + } > > > > > +} > > > > > + > > > > > +uint64_t > > > > > +tsc2usec(uint64_t n) > > > > > +{ > > > > > + struct cpu_info *ci = curcpu(); > > > > > + > > > > > + return n / ci->ci_tsc_freq * 1000000; > > > > > +} > > > > > diff --git a/sys/arch/amd64/conf/files.amd64 > > > > > b/sys/arch/amd64/conf/files.amd64 > > > > > index 5d58b34..d101cf2 100644 > > > > > --- a/sys/arch/amd64/conf/files.amd64 > > > > > +++ b/sys/arch/amd64/conf/files.amd64 > > > > > @@ -97,6 +97,7 @@ define cpu {[apid = -1]} > > > > > device cpu > > > > > attach cpu at mainbus > > > > > file arch/amd64/amd64/cpu.c cpu > > > > > +file arch/amd64/amd64/tsc.c cpu > > > > > > > > > > > > > > > define lapic > > > > > > > > > >
