Re: [PATCH V3] vmx/nmi: Do not use self_nmi() in VMEXIT handler

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 28/02/13 14:42, Jan Beulich wrote:
>>>> On 28.02.13 at 15:25, Tim Deegan <tim@xen.org> wrote:
>> At 13:39 +0000 on 28 Feb (1362058773), Jan Beulich wrote:
>>> (realizing that dealing with the PV side of the issue will be left to
>>> me in the end)
>> For PV, would you be happy with something like this, or do you want to
>> avoid the extra IRET in cases where we would be returning with IRET
>> anyway?
> No, and not so much because of the redundant IRET, but
> because ...
>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -3211,7 +3211,7 @@ void do_nmi(struct cpu_user_regs *regs)
>>      ++nmi_count(cpu);
>>  
>>      if ( nmi_callback(regs, cpu) )
>> -        return;
>> +        goto out;
>>  
>>      if ( nmi_watchdog )
>>          nmi_watchdog_tick(regs);
>> @@ -3227,6 +3227,9 @@ void do_nmi(struct cpu_user_regs *regs)
>>          if ( !(reason & 0xc0) && !nmi_watchdog )
>>              unknown_nmi_error(regs, reason);
>>      }
>> +
>> +out:
>> +    enable_nmis();
> ... this must not be done when on the NMI stack (i.e. when the
> NMI was raised while in hypervisor context). Checking for this
> here would be strait forward, but I was really considering to do
> all of this in the assembly exit path, and I was still undecided
> whether we shouldn't follow Linux in skipping softirq processing
> (and hence scheduling) on the way out from an NMI (I don't
> think we'd need to do the same for MCE).
>
> Jan

It is furthermore pointless.  If we interrupted a guest with the NMI, we
would have moved onto the main stack.  We would only be on the NMI stack
at this point if we interrupted Xen with the NMI, at which point we will
be iret'ing back anyway.

~Andrew

>
>>  }
>>  
>>  void set_nmi_callback(nmi_callback_t callback)
>
>