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

[Tim Deegan <tim@xxxxxxx>]

At 15:41 +0000 on 28 Feb (1362066080), Jan Beulich wrote:
> --- 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. */

Language nit: 'straight', meaning linear or directly, is suitable here. 
'strait' means narrow or constrained (as in strait-jacket).

> +        testb $3,UREGS_cs(%rsp)
> +        GET_CURRENT(%rbx)
> +        jz    restore_all_xen

GET_CURRENT() contains an addq, which updates ZF.  Swap it with the
testb?

Otherwise, looks good to me. 

Reviewed-by: Tim Deegan <tim@xxxxxxx>

> +        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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel