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

[George Dunlap <george.dunlap@eu.citrix.com>]

On 22/11/12 17:34, Tim Deegan wrote:
> At 15:00 +0000 on 22 Nov (1353596446), Andrew Cooper wrote:
>> diff -r 2489c2926698 -r d7ea938044ac 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)
>> +             */
>> +            do_nmi();
>>           break;
>>       case EXIT_REASON_MCE_DURING_VMENTRY:
>>           do_machine_check(regs);
> I think it might also make sense to move the HVMTARCE invocations that
> are just above here down to after this block.  There's quite a lot of
> code behind there and though I don't see any potential faults they might
> well get added later (including in places like printk() and
> tasklet_schedule()).
>
> I had a look at the rest of the code that runs between the vmexit and
> here, and it looks OK to me.
>
> George, would that make the tracing more confusing?

Well, it would mildly, because you'd get the trace in vmx_do_extint() 
before the trace for the VMEXIT that caused it. :-) If it's important 
for correctness, then xenalyze will just have to deal.  But it would be 
a lot nicer not to have to deal.

  -George