svn commit: r283331 - head/sys/arm/arm

[John Baldwin]

On Monday, May 25, 2015 01:21:48 PM Andrew Turner wrote:
> On Mon, 25 May 2015 07:23:28 -0400
> John Baldwin <jhb at freebsd.org> wrote:
> 
> > On Saturday, May 23, 2015 10:28:59 PM Andrew Turner wrote:
> > > Author: andrew
> > > Date: Sat May 23 22:28:59 2015
> > > New Revision: 283331
> > > URL: https://svnweb.freebsd.org/changeset/base/283331
> > > 
> > > Log:
> > >   Use the wait-for-event instruction to put the core we have just
> > > enabled to sleep while it waits to start scheduling. The boot core
> > > can then use the send-event instruction to wake the cores when they
> > > should enter the scheduler.
> > >   
> > >   MFC after:	1 week
> > > 
> > > Modified:
> > >   head/sys/arm/arm/mp_machdep.c
> > > 
> > > Modified: head/sys/arm/arm/mp_machdep.c
> > > ==============================================================================
> > > --- head/sys/arm/arm/mp_machdep.c	Sat May 23 21:58:41
> > > 2015	(r283330) +++ head/sys/arm/arm/mp_machdep.c	Sat
> > > May 23 22:28:59 2015	(r283331) @@ -185,8 +185,11 @@
> > > init_secondary(int cpu) atomic_add_rel_32(&mp_naps, 1);
> > >  
> > >  	/* Spin until the BSP releases the APs */
> > > -	while (!aps_ready)
> > > -		;
> > > +	while (!atomic_load_acq_int(&aps_ready)) {
> > > +#if __ARM_ARCH >= 7
> > > +		__asm __volatile("wfe");
> > > +#endif
> > > +	}
> > 
> > I don't know that this atomic load acquire is really changing
> > anything here?  Since aps_ready is volatile reading it should
> > already be "atomic" on each check around the loop.
> 
> It's also adding acquire semantics to ensure we don't
> incorrectly reorder memory operations across the call.

I think the _rel barrier on the update to mp_naps above probably
already does that, but ok.
 
> > >  	/* Initialize curthread */
> > >  	KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
> > > @@ -353,6 +356,10 @@ release_aps(void *dummy __unused)
> > >  		arm_unmask_irq(i);
> > >  	}
> > >  	atomic_store_rel_int(&aps_ready, 1);
> > > +	/* Wake the other threads up */
> > > +#if __ARM_ARCH >= 7
> > > +	armv7_sev();
> > > +#endif
> > 
> > So I'm not at all familiar with these instructions or what they do,
> > but are the events level triggered?  In particular, is there any
> > sort of race where the sev might arrive in between the check of
> > aps_ready and the wfe on an AP?  (For example, if wfe/sev were
> > similar to using mwait on x86 for wfe and a memory write for sev,
> > x86 would require a call to monitor before doing a check of
> > aps_ready to handle the race like so:
> > 
> > 	while (!aps_ready) {
> > 		monitor(&aps_ready);
> > 		if (!aps_ready)
> > 			mwait();
> > 	}
> > 
> 
> The armv7_sev function includes a barrier to ensure any previous memory
> operations have been flushed to cache before we send the event. The sev
> instruction then sets the event register in every processor.
> 
> The wfe instruction will check this event register and, if it is unset,
> it the processor can then enter a low power mode. From my reading of
> the documentation, if an event has been signalled before executing the
> wfe then the instruction is a nop so will exit the loop as the new
> value of aps_ready will be visible on all processors.

Mmmm, does that mean then that you can (conceivably) lose the race the other
way where it "sees" ap_ready's update before it calls wfe and never calls
wfe to "harvest" the event from sev?  (In practice I think this is not
possible during boot as AP's can't get preempted and there is typically
a "long" time between AP's being signalled to start and start_aps being
set.  However, this would be a concern for use of wfe/sev for other use
cases such as for the cpu_idle hook perhaps?)

-- 
John Baldwin