From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH v2 1/3] KVM: nVMX: Don't advertise single context invalidation for invept Date: Fri, 11 Apr 2014 15:48:23 -0300 Message-ID: <20140411184823.GA17124@amt.cnet> References: <1396299625-8285-1-git-send-email-bsd@redhat.com> <1396299625-8285-2-git-send-email-bsd@redhat.com> <20140410204738.GA28576@amt.cnet> <53478A15.9080903@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Bandan Das , kvm@vger.kernel.org, Paolo Bonzini , Gleb Natapov To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:21904 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754952AbaDKTCk (ORCPT ); Fri, 11 Apr 2014 15:02:40 -0400 Content-Disposition: inline In-Reply-To: <53478A15.9080903@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Apr 11, 2014 at 08:22:13AM +0200, Jan Kiszka wrote: > On 2014-04-11 02:27, Bandan Das wrote: > > Marcelo Tosatti writes: > > > >> On Mon, Mar 31, 2014 at 05:00:23PM -0400, Bandan Das wrote: > >>> For single context invalidation, we fall through to global > >>> invalidation in handle_invept() except for one case - when > >>> the operand supplied by L1 is different from what we have in > >>> vmcs12. However, typically hypervisors will only call invept > >>> for the currently loaded eptp, so the condition will > >>> never be true. > >>> > >>> Signed-off-by: Bandan Das > >> >> Bandan, > >> > >> Why not fix INVEPT single-context rather than removing it entirely? > >> > >> "Single-context. If the INVEPT type is 1, the logical processor > >> invalidates all guest-physical mappings and combined mappings associated > >> with the EP4TA specified in the INVEPT descriptor. Combined mappings for > >> that EP4TA are invalidated for all VPIDs and all PCIDs. (The instruction > >> may invalidate mappings associated with other EP4TAs.)" > >> > >> So just removing the "if (EPTP != CURRENT.EPTP) BREAK" should be enough. > > > > The single context invalidation in handle_invept() doesn't do > > anything different. It just falls down to the global case. > > And the invept code in Xen and KVM both seemed to fall back > > to global invalidation if support for single context wasn't found. > > So, it was proposed not to advertise it at all. > > > > But rethinking this again, I agree with you. If there's a hypervisor > > with a single context invept implmentation that does not fallback, What do you mean "does not fallback" ? The hypervisor cannot detect fallback because: "(The instruction may invalidate mappings associated with other EP4TAs.)" So the spec says single context can behave as global context (similar with TLB entries and INVLPG). So it is valid to implement single context as global context. > > this will unfortunately not work. Jan, do you agree with this ? > > A hypervisor that doesn't properly check the HW caps is just broken. And > one that mandates single context invalidation support is silly. > > Jan I imagined Xen broke because broken KVM's implementation of INVEPT single context (so that should be fixed). If with the proper implementation of INVEPT single context in KVM Xen still fails for some reason, would have to understand why it is failing.