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?

-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
> > > 
> 

Reply via email to