From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1895DC04AAE for ; Wed, 8 May 2019 19:55:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E7CC620675 for ; Wed, 8 May 2019 19:55:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728564AbfEHTzE (ORCPT ); Wed, 8 May 2019 15:55:04 -0400 Received: from mail-qt1-f196.google.com ([209.85.160.196]:36183 "EHLO mail-qt1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727405AbfEHTzB (ORCPT ); Wed, 8 May 2019 15:55:01 -0400 Received: by mail-qt1-f196.google.com with SMTP id a17so3736666qth.3 for ; Wed, 08 May 2019 12:55:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=RvSKii+KhiFkuVDqASPLy62yyUh/oPO0cxoeTudN2mQ=; b=NRa6i07WsL3G7xVikWi5bR84KhoSnDk9OxL0zNogFPtgMnqNUt22dl0CSRaKyPtARQ TWm0aIYZ1QwjkiOom5tTI3nFZq6hByuQj5crVt877jWJflA15NkZ1rBQcu1gVFUJMke5 i0QmjHkAvfQZKLECOyYdpyus38XuoBOz1TY+ZQ+Si2RULMha+Uh6Sr4ZeaccpbSa4DqL f7VztT2V1/A6qhxTqffXTr73idi5QcuLdZQdESb2yVEQVO3EvbczcyAI+cIS/Eir5eb8 J7vFXjw0Ou5gxn5iBboc5A+0yOfmgzJZuLA8IpIgjSA77kJsKRfrBlsQLH0EkW2AdiMn wTUQ== X-Gm-Message-State: APjAAAVzibJGnokHONutc2IxL51u9ufj7VM6X8KUbo7FwqGMNj+F8f7u AmLvox06FtAmo3bvvO7iVk7CYPABlqo= X-Google-Smtp-Source: APXvYqwraZ8O3T+rgFDnempnH00gUC5HuV1261EekiO4ePiMgW7o47XJjcLuIfvuXPZmNBSM6uEwNw== X-Received: by 2002:a0c:e705:: with SMTP id d5mr35526qvn.218.1557345299906; Wed, 08 May 2019 12:54:59 -0700 (PDT) Received: from vitty.brq.redhat.com ([64.251.121.244]) by smtp.gmail.com with ESMTPSA id 46sm11168412qto.57.2019.05.08.12.54.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 08 May 2019 12:54:59 -0700 (PDT) From: Vitaly Kuznetsov To: Aaron Lewis Cc: Peter Shier , Paolo Bonzini , rkrcmar@redhat.com, Jim Mattson , Marc Orr , kvm@vger.kernel.org Subject: Re: [PATCH 2/3] KVM: nVMX: KVM_SET_NESTED_STATE - Tear down old EVMCS state before setting new state In-Reply-To: References: <20190502183133.258026-1-aaronlewis@google.com> <87zho37s2h.fsf@vitty.brq.redhat.com> Date: Wed, 08 May 2019 15:54:59 -0400 Message-ID: <878svgsovg.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Aaron Lewis writes: > From: Vitaly Kuznetsov > Date: Fri, May 3, 2019 at 3:25 AM > To: Aaron Lewis > Cc: Peter Shier, , , > , , > >> Aaron Lewis writes: >> >> > Move call to nested_enable_evmcs until after free_nested() is complete. >> > >> > Signed-off-by: Aaron Lewis >> > Reviewed-by: Marc Orr >> > Reviewed-by: Peter Shier >> > --- >> > arch/x86/kvm/vmx/nested.c | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >> > index 081dea6e211a..3b39c60951ac 100644 >> > --- a/arch/x86/kvm/vmx/nested.c >> > +++ b/arch/x86/kvm/vmx/nested.c >> > @@ -5373,9 +5373,6 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >> > if (kvm_state->format != 0) >> > return -EINVAL; >> > >> > - if (kvm_state->flags & KVM_STATE_NESTED_EVMCS) >> > - nested_enable_evmcs(vcpu, NULL); >> > - >> > if (!nested_vmx_allowed(vcpu)) >> > return kvm_state->vmx.vmxon_pa == -1ull ? 0 : -EINVAL; >> > >> > @@ -5417,6 +5414,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >> > if (kvm_state->vmx.vmxon_pa == -1ull) >> > return 0; >> > >> > + if (kvm_state->flags & KVM_STATE_NESTED_EVMCS) >> > + nested_enable_evmcs(vcpu, NULL); >> > + >> > vmx->nested.vmxon_ptr = kvm_state->vmx.vmxon_pa; >> > ret = enter_vmx_operation(vcpu); >> > if (ret) >> >> nested_enable_evmcs() doesn't do much, actually, in case it was >> previously enabled it doesn't do anything and in case it wasn't ordering >> with free_nested() (where you're aiming at nested_release_evmcs() I >> would guess) shouldn't matter. So could you please elaborate (better in >> the commit message) why do we need this re-ordered? My guess is that >> you'd like to perform checks for e.g. 'vmx.vmxon_pa == -1ull' before >> we actually start doing any changes but let's clarify that. >> >> Thanks! >> >> -- >> Vitaly > > There are two reasons for doing this: > 1. We don't want to set new state if we are going to leave nesting and > exit the function (ie: vmx.vmxon_pa = -1), like you pointed out. > 2. To be more future proof, we don't want to set new state before > tearing down state. This could cause conflicts down the road. > > I can add this to the commit message if there are no objections to > these points. Sounds good to me, please do. Thanks! -- Vitaly