Re: [Xen-devel] [PATCH V2] xen: vmx: Use an INT 2 call to process real NMI's instead of self_nmi() in VMEXIT handler

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.11.12 at 21:08, Malcolm Crossley <malcolm.crossley@xxxxxxxxxx> wrote:
> The self_nmi() code cause's an NMI to be triggered by sending an APIC message
> to the local processor. Unfortunately there is a delay in the delivery of 
> the
> APIC message and we may already have re-entered the HVM guest by the time the
> NMI is taken. This results in the VMEXIT code calling the self_nmi() 
> function
> again. We have seen hundreds of iterations of this VMEXIT/VMENTER loop 
> before
> the HVM guest resumes normal operation.
> 
> Volume 3 Chapter 27 Section 1 of the Intel SDM states:
> 
> An NMI causes subsequent NMIs to be blocked, but only after the VM exit
> completes.
> 
> So we believe it is safe to directly invoke the INT call as NMI's should
> already be blocked.
> 
> The INT 2 call will perform an IRET which will unblock later calls to the 
> NMI
> handler, according to Intel SDM Volume 3 Chapter 6.7.1. We must ensure that 
> the
> IRET from the INT 2 IRET is the first IRET issued to prevent losing an NMI.
> Moving the INT 2 call to before the interrupts are enabled should ensure we
> don't lose the NMI.
> 
> Signed-off-by: Malcolm Crossley <malcolm.crossley@xxxxxxxxxx>
> Acked-by: Tim Deegan <tim@xxxxxxx>
> 
> diff -r 62885b3c34c8 -r 7d6fd0219dd7 xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2269,6 +2269,14 @@ void vmx_vmexit_handler(struct cpu_user_
>          vector = intr_info & INTR_INFO_VECTOR_MASK;
>          if ( vector == TRAP_machine_check )
>              do_machine_check(regs);
> +        else if ( vector == TRAP_nmi &&
> +                ( (intr_info & INTR_INFO_INTR_TYPE_MASK) ==
> +                  (X86_EVENTTYPE_NMI << 8) ) )
> +            /* Must be called before interrupts are enabled to ensure
> +             * the NMI handler code is run before the first IRET. The
> +             * IRET unblocks subsequent NMI's (Intel SDM Vol 3, 6.7.1)
> +             */
> +            asm volatile("int $2"); /* Real NMI, vector 2: normal processing 
> */

And I still don't like this use of "int $2" here: An aspect we didn't
consider so far is that a nested MCE would break things again
(yes, that also is the case for NMIs arriving when in VMX root
context, but this wouldn't be the case here if we called do_nmi()
and took care of the missing IRET in a place also covering the PV
path, e.g. in continue_idle_domain() or reset_stack_and_jump();
we should at least keep the window where this can happen as
small as possible).

Jan

>          break;
>      case EXIT_REASON_MCE_DURING_VMENTRY:
>          do_machine_check(regs);
> @@ -2442,7 +2450,6 @@ void vmx_vmexit_handler(struct cpu_user_
>                   (X86_EVENTTYPE_NMI << 8) )
>                  goto exit_and_crash;
>              HVMTRACE_0D(NMI);
> -            self_nmi(); /* Real NMI, vector 2: normal processing. */
>              break;
>          case TRAP_machine_check:
>              HVMTRACE_0D(MCE);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel