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=-11.6 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FSL_HELO_FAKE, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=no 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 BE934C12002 for ; Wed, 21 Jul 2021 16:19:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A0CBF60FF3 for ; Wed, 21 Jul 2021 16:19:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234280AbhGUPia (ORCPT ); Wed, 21 Jul 2021 11:38:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53410 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234359AbhGUPiT (ORCPT ); Wed, 21 Jul 2021 11:38:19 -0400 Received: from mail-pg1-x52d.google.com (mail-pg1-x52d.google.com [IPv6:2607:f8b0:4864:20::52d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CACADC061757 for ; Wed, 21 Jul 2021 09:18:54 -0700 (PDT) Received: by mail-pg1-x52d.google.com with SMTP id t9so2373868pgn.4 for ; Wed, 21 Jul 2021 09:18:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=jlmR2FOonmL/63pitZehwQRYMbA6yhFj8rYQ3kGrZEU=; b=eERMJWoVCwQq7vzCp2h2L4u/l2Dbh92XhsPYNxRqSp+TANc2GKl/AT6hkZ7ojAEPkT tawMnFE6vYyiQ/KJJHuCBiwK4gubkcZmtjwEIx1oUjyEk2dakeOR7ObH0ozBjK4usQBH /zFi2nzhqH4JSAAIUyWeFSwR/y7RPZXwQvW7NwMvhuPEkRczlP2zsxB4ivRpxe+davpW mzmDUVAVuhL+8735q+I8RQ4KIkyW0wOZg5ZpwQxrhtUSVkHmXr6nwG37bwtGUjDWbsvY xiIPWi4rHBryhy97/DNZXGiS62bVpZLbz3nICb5/dtMnPB5kRGkpJ02WG02K8bnyIILL i9VA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=jlmR2FOonmL/63pitZehwQRYMbA6yhFj8rYQ3kGrZEU=; b=pD0h6raa/dxoNAAa7uV8jAnErKMifVEJjchqiWuKptl5bJfXU7QGAdZVNeVA79JWfL kBzhboNSiROTHWEl3eBYS1gJXvE6vlcOL4rAg7Tllp9W5hU8j/3ZCEVeZa4KAmQvY2g1 Fm5VWpazJVDWc9Dbnc0HkYp4gJ1I2BLFGjOZZb2Ra0WiYm9ixZGACbagUe3+szZzAtg4 3wftAiNoDJ5IJKtFYaFagzQU/VUWUd6SXvYwga4vjO8uqLQZ9lzTRBIe/1wMB4qHUJeK W8O03wL4aHR+rtXcm9YBoJr6gu0luTXaTk61nRLV8wTtY9DH3IwSYXM+nfTFu+OXACz4 in3Q== X-Gm-Message-State: AOAM531/e54IlSJLZU9GPUbYkfwJT5HPRT83IxFI+1R33cVw7KgfxAZi P5t6JGqr+q0n09rCBHUb4fAR0A== X-Google-Smtp-Source: ABdhPJwu8EMHORLpQ830hmtLewIMD4KzJnBIqmBF2yvCCsegIA/zYboFShntBB7PCCxumgWRDD4QNQ== X-Received: by 2002:a65:5186:: with SMTP id h6mr36710306pgq.62.1626884333648; Wed, 21 Jul 2021 09:18:53 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id z12sm22075927pjd.39.2021.07.21.09.18.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Jul 2021 09:18:53 -0700 (PDT) Date: Wed, 21 Jul 2021 16:18:49 +0000 From: Sean Christopherson To: "Hu, Robert" Cc: Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Robert Hoo Subject: Re: [PATCH] KVM: nVMX: Dynamically compute max VMCS index for vmcs12 Message-ID: References: <20210618214658.2700765-1-seanjc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 21, 2021, Hu, Robert wrote: > > > Queued, thanks. Without having checked the kvm-unit-tests sources > > > very thoroughly, this might be a configuration issue in > > > kvm-unit-tests; in theory "-cpu host" (unlike "-cpu > > > host,migratable=no") should not enable TSC scaling. > > > > As noted in the code comments, KVM allows VMREAD/VMWRITE to all defined > > fields, whether or not the field should actually exist for the vCPU model doesn't > > enter into the equation. That's technically wrong as there are a number of > > fields that the SDM explicitly states exist iff a certain feature is supported. > > It's right that a number of fields' existence depends on some certain feature. > Meanwhile, "IA32_VMX_VMCS_ENUM indicates to software the highest index > value used in the encoding of any field *supported* by the processor", rather than > *existed*. Yes. > So my understanding is no matter what VMCS exec control field's value is set, > value of IA32_VMX_VMCS_ENUM shall not be affected, as it reports the physical > CPU's capability rather than runtime VMCS configuration. Yes. > Back to nested case, L1's VMCS configuration lays the "physical" capability > for L2, right? Yes. > nested_vmx_msrs or at least nested_vmx_msrs.vmcs_enum shall be put to vcpu > scope, rather than current kvm global. > > Current nested_vmx_calc_vmcs_enum_msr() is invoked at early stage, before > vcpu features are settled. I think should be moved to later stage as well. Just moving the helper (or adding another call) wouldn't fix the underlying problem that KVM doesn't correctly model the interplay between VMX features and VMCS fields. It's arguably less wrong than letting userspace stuff an incorrect value, but it's not 100% correct and ignoring/overriding userspace is sketchy at best. As suggested below, the full fix is to fail VMREAD/VMWRITE to fields that shouldn't exist given the vCPU model. > > To fix that we'd need to add a "feature flag" to vmcs_field_to_offset_table > > that is checked against the vCPU model, though updating the MSR would > > probably fall onto userspace's shoulders? > > > > And FWIW, this is the QEMU code: > > > > #define VMCS12_MAX_FIELD_INDEX (0x17) > > > > static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f) > > { > > ... > > > > /* > > * Just to be safe, write these with constant values. The CRn_FIXED1 > > * MSRs are generated by KVM based on the vCPU's CPUID. > > */ > > kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR0_FIXED0, > > CR0_PE_MASK | CR0_PG_MASK | CR0_NE_MASK); > > kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR4_FIXED0, > > CR4_VMXE_MASK); > > kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM, > > VMCS12_MAX_FIELD_INDEX << 1); > > } >