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=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 41A9CC2B9F7 for ; Mon, 24 May 2021 12:35:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 26317613AD for ; Mon, 24 May 2021 12:35:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232848AbhEXMhA (ORCPT ); Mon, 24 May 2021 08:37:00 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:20743 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232830AbhEXMgw (ORCPT ); Mon, 24 May 2021 08:36:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1621859723; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=PpgAUyiKcsxyyDpd43AlLz5pN+mpE43cHu+cRRsUC+8=; b=TXBsywCfggwQyUN+l1FtiIFDLLachpFRL4PnAoBz7HeuE3I7Kn4ShtnB9WnkPMaewwWNVX SFPTJ18bntA+OuiG2eMH+GHCAClpafjX5dhFLcDfoGVo3hURPGVLa+i1iffpqfpQJNGLG4 fGNx1ReUydQN6CTTT1XWq9NG8hALwl4= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-480-yZAeC1ItOF6lWx6yrnq84Q-1; Mon, 24 May 2021 08:35:17 -0400 X-MC-Unique: yZAeC1ItOF6lWx6yrnq84Q-1 Received: by mail-wm1-f71.google.com with SMTP id h9-20020a1cb7090000b029016d3f0b6ce4so3924605wmf.9 for ; Mon, 24 May 2021 05:35:16 -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=PpgAUyiKcsxyyDpd43AlLz5pN+mpE43cHu+cRRsUC+8=; b=mBJOwhhU2aoLR9GkDxMjlnZkzWNyAVsm9i57fLfc1arE0qmXjuiUOYy2UMMhY+2KyU HnrUoh8G6fU+NM/EVJrlGDnkpn2Gx4rp97VOrXokmz17KfWEUsZyAPGaiEq4KTn6nbrh oTyZAch4SeMunp8C3yF9AWCjt2AFmW9jmWRAEhTFEqf/H5FlEzgQvt/URUYJvorQUyjc W41fkx6WBgSpOo1Lhs6Z+z5LtWxGF/YtTPYnU+feabAYMlST8uLiah8CDqKy1VIhLQiR uf+SmelxEeIq5bEiAiZ/cdb6uAB0azJXpcUvlYjVYMMK6rcH7I93tgZO15Gem04TC2ym +C2Q== X-Gm-Message-State: AOAM530kkEgSvRAygFzROmgHW2FTbO5taPIUAOLnZ4pYccGzEry5sqKs skZL/CifQkJLtiIUxiXExtr3ZwAkKb3BHaMjj7nQI8+RwG4njClmp4c5A2/C3iwFLJ0kwLpa9De gnqzOrwQwJwxu55afDAQehjRypH6tHwH69UxRu5eILWJ4mRxfaFjHWkCfUBJGNIXSAg9exdoxwA nz X-Received: by 2002:a5d:4fce:: with SMTP id h14mr21536335wrw.239.1621859715828; Mon, 24 May 2021 05:35:15 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwJaSdc2G2JIlQi2ysSOdWJOO94g1Ez7aGhHWhFsXXKj8SOWwzwrCkMWWBxRFqpEPku9GxM5Q== X-Received: by 2002:a5d:4fce:: with SMTP id h14mr21536302wrw.239.1621859715485; Mon, 24 May 2021 05:35:15 -0700 (PDT) Received: from vitty.brq.redhat.com (g-server-2.ign.cz. [91.219.240.2]) by smtp.gmail.com with ESMTPSA id t11sm12357620wrz.57.2021.05.24.05.35.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 May 2021 05:35:15 -0700 (PDT) From: Vitaly Kuznetsov To: Maxim Levitsky , kvm@vger.kernel.org, Paolo Bonzini Cc: Sean Christopherson , Wanpeng Li , Jim Mattson , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/7] KVM: nVMX: Introduce nested_evmcs_is_used() In-Reply-To: <80892ca2e3d7122b5b92f696ecf4c1943b0245b9.camel@redhat.com> References: <20210517135054.1914802-1-vkuznets@redhat.com> <20210517135054.1914802-2-vkuznets@redhat.com> <80892ca2e3d7122b5b92f696ecf4c1943b0245b9.camel@redhat.com> Date: Mon, 24 May 2021 14:35:14 +0200 Message-ID: <875yz871j1.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Maxim Levitsky writes: > On Mon, 2021-05-17 at 15:50 +0200, Vitaly Kuznetsov wrote: >> Unlike regular set_current_vmptr(), nested_vmx_handle_enlightened_vmptrld() >> can not be called directly from vmx_set_nested_state() as KVM may not have >> all the information yet (e.g. HV_X64_MSR_VP_ASSIST_PAGE MSR may not be >> restored yet). Enlightened VMCS is mapped later while getting nested state >> pages. In the meantime, vmx->nested.hv_evmcs remains NULL and using it >> for various checks is incorrect. In particular, if KVM_GET_NESTED_STATE is >> called right after KVM_SET_NESTED_STATE, KVM_STATE_NESTED_EVMCS flag in the >> resulting state will be unset (and such state will later fail to load). >> >> Introduce nested_evmcs_is_used() and use 'is_guest_mode(vcpu) && >> vmx->nested.current_vmptr == -1ull' check to detect not-yet-mapped eVMCS >> after restore. >> >> Signed-off-by: Vitaly Kuznetsov >> --- >> arch/x86/kvm/vmx/nested.c | 31 ++++++++++++++++++++++++++----- >> 1 file changed, 26 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >> index 6058a65a6ede..3080e00c8f90 100644 >> --- a/arch/x86/kvm/vmx/nested.c >> +++ b/arch/x86/kvm/vmx/nested.c >> @@ -141,6 +141,27 @@ static void init_vmcs_shadow_fields(void) >> max_shadow_read_write_fields = j; >> } >> >> +static inline bool nested_evmcs_is_used(struct vcpu_vmx *vmx) >> +{ >> + struct kvm_vcpu *vcpu = &vmx->vcpu; >> + >> + if (vmx->nested.hv_evmcs) >> + return true; >> + >> + /* >> + * After KVM_SET_NESTED_STATE, enlightened VMCS is mapped during >> + * KVM_REQ_GET_NESTED_STATE_PAGES handling and until the request is >> + * processed vmx->nested.hv_evmcs is NULL. It is, however, possible to >> + * detect such state by checking 'nested.current_vmptr == -1ull' when >> + * vCPU is in guest mode, it is only possible with eVMCS. >> + */ >> + if (unlikely(vmx->nested.enlightened_vmcs_enabled && is_guest_mode(vcpu) && >> + (vmx->nested.current_vmptr == -1ull))) >> + return true; >> + >> + return false; >> +} > > > I think that this is a valid way to solve the issue, > but it feels like there might be a better way. > I don't mind though to accept this patch as is. > > So here are my 2 cents about this: > > First of all after studying how evmcs works I take my words back > about needing to migrate its contents. > > It is indeed enough to migrate its physical address, > or maybe even just a flag that evmcs is loaded > (and to my surprise we already do this - KVM_STATE_NESTED_EVMCS) > > So how about just having a boolean flag that indicates that evmcs is in use, > but doesn't imply that we know its address or that it is mapped > to host address space, something like 'vmx->nested.enlightened_vmcs_loaded' > > On migration that flag saved and restored as the KVM_STATE_NESTED_EVMCS, > otherwise it set when we load an evmcs and cleared when it is released. > > Then as far as I can see we can use this flag in nested_evmcs_is_used > since all its callers don't touch evmcs, thus don't need it to be > mapped. > > What do you think? > First, we need to be compatible with older KVMs which don't have the flag and this is problematic: currently, we always expect vmcs12 to carry valid contents. This is challenging. Second, vCPU can be migrated in three different states: 1) While L2 was running ('true' nested state is in VMCS02) 2) While L1 was running ('true' nested state is in eVMCS) 3) Right after an exit from L2 to L1 was forced ('need_vmcs12_to_shadow_sync = true') ('true' nested state is in VMCS12). The current solution is to always use VMCS12 as a container to transfer the state and conceptually, it is at least easier to understand. We can, indeed, transfer eVMCS (or VMCS12) in case 2) through guest memory and I even tried that but that was making the code more complex so eventually I gave up and decided to preserve the 'always use VMCS12 as a container' status quo. -- Vitaly