From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 22/24] Correct handling of idt vectoring info Date: Mon, 20 Sep 2010 11:34:40 +0200 Message-ID: <20100920093440.GH3008@redhat.com> References: <1276431753-nyh@il.ibm.com> <201006131233.o5DCXoel013156@rice.haifa.ibm.com> <20100617115803.GP523@redhat.com> <20100920063704.GA29641@fermat.math.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@redhat.com, kvm@vger.kernel.org To: "Nadav Har'El" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40158 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752864Ab0ITJes (ORCPT ); Mon, 20 Sep 2010 05:34:48 -0400 Content-Disposition: inline In-Reply-To: <20100920063704.GA29641@fermat.math.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: 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.