Re: Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-07-30 at 11:54 +0100, Stefano Stabellini wrote:
> On Thu, 30 Jul 2015, Ian Campbell wrote:
> > On Wed, 2015-07-29 at 19:18 +0100, Julien Grall wrote:
> > 
> > As an aside from the issue you are seeing:
> > 
> > > The old implementation of spinlock is sending an event (via the 
> > > assembly
> > > instruction SEV) to the other physical CPUs. This will wake up the
> > > others CPUs waiting on the assembly instruction WFE (Wait For Event).
> > 
> > Uh, I didn't notice this about the new implementation, sorry I should 
> > have
> > done.
> > 
> > IMHO we should investigate (probably with some urgency) inserting a WFE 
> > and
> > SEV pair into the lock/unlock paths, else power consumption will suck.
> > 
> > I think that probably means using something new to replace the 
> > cpu_relax()
> > calls in the spinlocks with a WFE on ARM (we don't just want to change
> > relax) and to add a arch specific hook for the SEV on the release path.
> 
> I agree: adding a WFE in cpu_relax() is too risky at this point.
> 
> 
> > If it is too late for 4.6 (which would depend on the eventual 
> > complexity of
> > the actual fix) then we should fix this ASAP in 4.7 and backport for 
> > 4.6.1.
> 
> I don't think we can release 4.6 without a WFE in the locks. We might
> want to consider reverting to spin_locks on ARM (although I am aware
> that the code is common at the moment).

It turns out we were missing the WFE even in the old code (I vaguely recall
having to refactor the Linux original to fit in with our arch/common split
and leaving myself a TODO item).

So I don't think we can justify changing this was 4.6. Investigating for
4.7 would be nice. Needs some careful though about races of the evt bit vs
the tickets changing though.

Ian.