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

[Keir Fraser <keir@xen.org>]

On 28/02/2013 15:41, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 28.02.13 at 15:42, "Jan Beulich" <JBeulich@suse.com> wrote:
>> ... 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).
> 
> Like this:
> 
> x86: skip processing events on the NMI exit path
> 
> Otherwise, we may end up in the scheduler, keeping NMIs masked for a
> possibly unbounded time (until whenever the next IRET gets executed).

Is this alternative that we might not process events for an unbounded time?
No, I guess not -- either we would interrupt the notifying IPI and we will
be IRETing into that IPI's handler, or the notifying IPI is delayed until
the NMI handler's IRET.

What about if the NMI handler itself raises an event (eg softirq)? Perhaps
there are no very essential ones of those?

> Of course it's open for discussion whether to always use the strait
> exit path from handle_ist_exception.

s/strait/straight (and below in the code comment that you add).

> Signed-off-by: Jan Beulich <jbeulich@suse.com>

> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -171,7 +171,7 @@ compat_bad_hypercall:
>          jmp  compat_test_all_events
>  
>  /* %rbx: struct vcpu, interrupts disabled */
> -compat_restore_all_guest:
> +ENTRY(compat_restore_all_guest)
>          ASSERT_INTERRUPTS_DISABLED
>          RESTORE_ALL adj=8 compat=1
>  .Lft0:  iretq
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -635,6 +635,9 @@ ENTRY(early_page_fault)
>          jmp   restore_all_xen
>          .popsection
>  
> +ENTRY(nmi)
> +        pushq $0
> +        movl  $TRAP_nmi,4(%rsp)
>  handle_ist_exception:
>          SAVE_ALL
>          testb $3,UREGS_cs(%rsp)
> @@ -649,12 +652,17 @@ handle_ist_exception:
>          movzbl UREGS_entry_vector(%rsp),%eax
>          leaq  exception_table(%rip),%rdx
>          callq *(%rdx,%rax,8)
> -        jmp   ret_from_intr
> +        cmpb  $TRAP_nmi,UREGS_entry_vector(%rsp)
> +        jne   ret_from_intr
>  
> -ENTRY(nmi)
> -        pushq $0
> -        movl  $TRAP_nmi,4(%rsp)
> -        jmp   handle_ist_exception
> +        /* We want to get strait to the IRET in the NMI exit path. */
> +        testb $3,UREGS_cs(%rsp)
> +        GET_CURRENT(%rbx)
> +        jz    restore_all_xen
> +        movq  VCPU_domain(%rbx),%rax
> +        testb $1,DOMAIN_is_32bit_pv(%rax)
> +        jz    restore_all_guest
> +        jmp   compat_restore_all_guest
>  
>  ENTRY(nmi_crash)
>          pushq $0
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel