On Feb 21, 2013, at 6:26 AM, Gleb Smirnoff wrote:

> On Thu, Feb 21, 2013 at 01:09:57PM +0400, Andrey Zonov wrote:
> A> > Log:
> A> >   Replace splhigh() with critical_enter()/leave() to ensure we write the
> A> >   config mode unlock sequence quickly enough. This likely isn't too 
> critical,
> A> >   since splhigh() has been a noop for a decade...
> A> > 
> A> > Modified:
> A> >   head/sys/dev/ppc/ppc.c
> A> > 
> A> > Modified: head/sys/dev/ppc/ppc.c
> A> > 
> ==============================================================================
> A> > --- head/sys/dev/ppc/ppc.c       Thu Feb 21 00:26:31 2013        
> (r247065)
> A> > +++ head/sys/dev/ppc/ppc.c       Thu Feb 21 00:27:51 2013        
> (r247066)
> A> > @@ -74,6 +74,22 @@ static void ppcintr(void *arg);
> A> >  
> A> >  #define DEVTOSOFTC(dev) ((struct ppc_data *)device_get_softc(dev))
> A> >  
> A> > +/*
> A> > + * We use critical enter/leave for the simple config locking needed to
> A> > + * detect the devices. We just want to make sure that both of our writes
> A> > + * happen without someone else also writing to those config registers. 
> Since
> A> > + * we just do this at startup, Giant keeps multiple threads from 
> executing,
> A> > + * and critical_enter() then is all that's needed to keep us from being 
> preempted
> A> > + * during the critical sequences with the hardware.
> A> > + *
> A> > + * Note: this doesn't prevent multiple threads from putting the chips 
> into
> A> > + * config mode, but since we only do that to detect the type at startup 
> the
> A> > + * extra overhead isn't needed since Giant protects us from multiple 
> entry
> A> > + * and no other code changes these registers.
> A> > + */
> A> > +#define PPC_CONFIG_LOCK(ppc)            critical_enter()
> A> > +#define PPC_CONFIG_UNLOCK(ppc)          critical_leave()
> A> > +
> A> 
> A> s/critical_leave/critical_exit/?
> 
> Already fixed.
> 
> However, question to Warner.
> 
> Since code already executes under Giant, what is the reason for 
> critical_section?
> What's the problem if couple of outb instructions are split (assuming they are
> properly serialized) across two CPUs?

There's a timing window that needs to be hit with these chips.

Warner
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to