[PATCH Xenial SRU] xen/qspinlock: Don't kick CPU if IRQ is not initialized

[Tim Gardner]

On 12/14/2016 08:13 AM, Colin Ian King wrote:
> On 14/12/16 15:07, Tim Gardner wrote:
>> From: Ross Lagerwall <ross.lagerwall at citrix.com>
>>
>> BugLink: http://bugs.launchpad.net/bugs/1649821
>>
>> The following commit:
>>
>>   1fb3a8b2cfb2 ("xen/spinlock: Fix locking path engaging too soon under PVHVM.")
>>
>> ... moved the initalization of the kicker interrupt until after
>> native_cpu_up() is called.
>>
>> However, when using qspinlocks, a CPU may try to kick another CPU that is
>> spinning (because it has not yet initialized its kicker interrupt), resulting
>> in the following crash during boot:
>>
>>   kernel BUG at /build/linux-Ay7j_C/linux-4.4.0/drivers/xen/events/events_base.c:1210!
>>   invalid opcode: 0000 [#1] SMP
>>   ...
>>   RIP: 0010:[<ffffffff814c97c9>]  [<ffffffff814c97c9>] xen_send_IPI_one+0x59/0x60
>>   ...
>>   Call Trace:
>>    [<ffffffff8102be9e>] xen_qlock_kick+0xe/0x10
>>    [<ffffffff810cabc2>] __pv_queued_spin_unlock+0xb2/0xf0
>>    [<ffffffff810ca6d1>] ? __raw_callee_save___pv_queued_spin_unlock+0x11/0x20
>>    [<ffffffff81052936>] ? check_tsc_warp+0x76/0x150
>>    [<ffffffff81052aa6>] check_tsc_sync_source+0x96/0x160
>>    [<ffffffff81051e28>] native_cpu_up+0x3d8/0x9f0
>>    [<ffffffff8102b315>] xen_hvm_cpu_up+0x35/0x80
>>    [<ffffffff8108198c>] _cpu_up+0x13c/0x180
>>    [<ffffffff81081a4a>] cpu_up+0x7a/0xa0
>>    [<ffffffff81f80dfc>] smp_init+0x7f/0x81
>>    [<ffffffff81f5a121>] kernel_init_freeable+0xef/0x212
>>    [<ffffffff81817f30>] ? rest_init+0x80/0x80
>>    [<ffffffff81817f3e>] kernel_init+0xe/0xe0
>>    [<ffffffff8182488f>] ret_from_fork+0x3f/0x70
>>    [<ffffffff81817f30>] ? rest_init+0x80/0x80
>>
>> To fix this, only send the kick if the target CPU's interrupt has been
>> initialized. This check isn't racy, because the target is waiting for
>> the spinlock, so it won't have initialized the interrupt in the
>> meantime.
>>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall at citrix.com>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky at oracle.com>
>> Cc: David Vrabel <david.vrabel at citrix.com>
>> Cc: Juergen Gross <jgross at suse.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
>> Cc: Linus Torvalds <torvalds at linux-foundation.org>
>> Cc: Peter Zijlstra <peterz at infradead.org>
>> Cc: Thomas Gleixner <tglx at linutronix.de>
>> Cc: linux-kernel at vger.kernel.org
>> Cc: xen-devel at lists.xenproject.org
>> Signed-off-by: Ingo Molnar <mingo at kernel.org>
>> (cherry picked from commit 707e59ba494372a90d245f18b0c78982caa88e48)
>> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
>> ---
>>  arch/x86/xen/spinlock.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
>> index 9e2ba5c..f42e78d 100644
>> --- a/arch/x86/xen/spinlock.c
>> +++ b/arch/x86/xen/spinlock.c
>> @@ -27,6 +27,12 @@ static bool xen_pvspin = true;
>>  
>>  static void xen_qlock_kick(int cpu)
>>  {
>> +	int irq = per_cpu(lock_kicker_irq, cpu);
>> +
>> +	/* Don't kick if the target's kicker interrupt is not initialized. */
>> +	if (irq == -1)
>> +		return;
>> +
>>  	xen_send_IPI_one(cpu, XEN_SPIN_UNLOCK_VECTOR);
>>  }
>>  
>>
> Seems OK to me, however has this been tested for this specific bug?
> 
> Colin
> 

It seems like an obvious fix. Both call traces look identical. I can
certainly produce a test kernel and get the reporter to produce some
results.

rtg
-- 
Tim Gardner tim.gardner at canonical.com