From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bandan Das Subject: Re: [PATCH v2 1/3] KVM: nVMX: Don't advertise single context invalidation for invept Date: Fri, 11 Apr 2014 14:35:19 -0400 Message-ID: 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> <53482DF4.3030808@siemens.com> Mime-Version: 1.0 Content-Type: text/plain Cc: Marcelo Tosatti , kvm@vger.kernel.org, Paolo Bonzini , Gleb Natapov To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42973 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754504AbaDKSf0 (ORCPT ); Fri, 11 Apr 2014 14:35:26 -0400 In-Reply-To: <53482DF4.3030808@siemens.com> (Jan Kiszka's message of "Fri, 11 Apr 2014 20:01:24 +0200") Sender: kvm-owner@vger.kernel.org List-ID: Jan Kiszka writes: > On 2014-04-11 19:26, Bandan Das wrote: >> Jan Kiszka writes: >> >>> 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, >>>> 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. >> >> Well, but we could make life a little bit easier for the unfortunate user >> using the broken hypervisor :) And advertising single context inavalidation >> doesn't really seem to have any downsides. > > Ok, let's try it this way: single-context invalidation is inherently > tied to VPID support (that's how you address a context). However, KVM > does not expose VPID to its guest. So this discussion is mood: no > hypervisor will make use of this feature as it has no means to fill in > the required parameter. I thought (from the spec) invept single context invalidation takes the EP4TA as the second argument. invvpid single context however takes the VPID as its descriptor. The Xen L1 hypervisor was actually calling single context invept multiple times. That's how I hit this bug. > Once we start supporting VPID, we can also think about how to address > single-context invalidation reasonably. > > Jan