Re: [PATCH 2/4] x86/nmi: enable local irqs in wait_for_nmis()

["Jan Beulich" <JBeulich@suse.com>]

>>> On 14.05.14 at 16:38, <david.vrabel@citrix.com> wrote:
> On 14/05/14 14:57, Jan Beulich wrote:
>>>>> On 14.05.14 at 14:58, <david.vrabel@citrix.com> wrote:
>>> --- a/xen/arch/x86/nmi.c
>>> +++ b/xen/arch/x86/nmi.c
>>> @@ -111,7 +111,14 @@ int nmi_active;
>>>  
>>>  static void __init wait_for_nmis(void *p)
>>>  {
>>> +    unsigned long flags;
>>> +
>>> +    local_save_flags(flags);
>>> +    local_irq_enable();
>>> +
>>>      mdelay((10*1000)/nmi_hz); /* wait 10 ticks */
>>> +
>>> +    local_irq_restore(flags);
>>>  }
>> 
>> This being the callback for on_selected_cpus(), i.e. called out of
>> interrupt context, I don't think it is uniformly safe to enable
>> interrupts here. The current behavior anyway is for the function
>> to run with interrupts enabled on the boot CPU, and with interrupts
>> disabled on the APs. So for this change to make a difference, the
>> IRQ would need to be bound to one of the APs, and consequently
>> the fix would be to force it onto the BP until boot progressed far
>> enough.
> 
> The call function IPIs are handled with a direct apic vector so skip the
> normal interrupt enabling path for irqs.  I can't see anywhere where
> irqs are enabled in call_function_interrupt() (x86) and
> smp_call_function_interrupt() (common).

Exactly, they aren't, because it's not generally safe to do so. And if
the generic code doesn't do so, you'll need a rather good reason to do
this manually.

Jan