Re: [PATCH 22/24] Correct handling of idt vectoring info

[Gleb Natapov <gleb@redhat.com>]

On Mon, Sep 20, 2010 at 08:37:04AM +0200, Nadav Har'El wrote:
> > Why can't you do that using existing exception/nmi/interrupt queues that
> > we have, but instead you effectively disable vmx_complete_interrupts()
> > by patch 18 when in nested mode and add logically same code in this
> > patch. I.e after exit you save info about idt event into nested_vmx
> > and reinject it on vm entry.
> 
> This is an interesting point.
> 
> The basic problem is that (as I explained in the patch's description) when
> L2 exits to L1 with idt vectoring info, L0 should *not* do its normal
> thing of injecting the event - it should basically do nothing, and just
> leave the IDT_VECTORING_INFO_FIELD in vmcs12 for L1 to find and act upon.
> So in this case we must eliminate the normal decision that KVM would make
> to inject the event.
> 
But your code disables normal path even if L0 is the one who should
handle exit and re-inject event into L2. Look at what nested SVM is doing.
It is checking in handle_exit() if vmexit should cause vmexit into L1
and if so they bypass regular code path by emulating exit instead, but if
L0 should handle the vmexit it uses regular code path.

> Perhaps it would have been possible to leave the decision as-is (i.e.,
> not change vmx_complete_interrupts()), but instead disable the injection
> itself in inject_pending_event() (in x86.c, not vmx.c) or in all of
> vmx_queue_exception, vmx_set_nmi and vmx_set_irq. But I'm not sure this will
> be a cleaner patch (and I'd especially like to avoid nested-specific changes
> in x86.c), and I'm pretty sure that however I change this code, it's bound
> to break in subtle ways. The current patch took some blood, toil, tears
> and sweat (well, maybe everything except the blood...) of my coworkers 
> to get right :-)
> 
Look at how SVM did it. VMX shouldn't be different.

--
			Gleb.