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

[David Vrabel <david.vrabel@citrix.com>]

On 30/07/15 11:54, 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.

WFE in cpu_relax() would be broken.

However, adding two hooks for spin_relax() (using this instead of
cpu_relax()) and spin_signal() that do the WFE/SEV seems low risk to me.

For x86 use:

#define spin_relax() cpu_relax()
#define spin_signal()

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

You can't revert the ticket locks for one architecture only.

David