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=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 E506CC4338F for ; Mon, 2 Aug 2021 08:22:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BC42161057 for ; Mon, 2 Aug 2021 08:22:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232688AbhHBIW7 (ORCPT ); Mon, 2 Aug 2021 04:22:59 -0400 Received: from mga18.intel.com ([134.134.136.126]:50020 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232562AbhHBIW7 (ORCPT ); Mon, 2 Aug 2021 04:22:59 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10063"; a="200614779" X-IronPort-AV: E=Sophos;i="5.84,288,1620716400"; d="scan'208";a="200614779" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Aug 2021 01:22:49 -0700 X-IronPort-AV: E=Sophos;i="5.84,288,1620716400"; d="scan'208";a="509975082" Received: from zengguan-mobl.ccr.corp.intel.com (HELO [10.238.0.133]) ([10.238.0.133]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Aug 2021 01:22:44 -0700 Subject: Re: [PATCH 1/6] x86/feat_ctl: Add new VMX feature, Tertiary VM-Execution control To: Sean Christopherson Cc: Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , "kvm@vger.kernel.org" , Dave Hansen , "Luck, Tony" , Kan Liang , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , Kim Phillips , Jarkko Sakkinen , Jethro Beekman , "Huang, Kai" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "Hu, Robert" , "Gao, Chao" , Robert Hoo References: <20210716064808.14757-1-guang.zeng@intel.com> <20210716064808.14757-2-guang.zeng@intel.com> From: Zeng Guang Message-ID: Date: Mon, 2 Aug 2021 16:22:32 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/29/2021 7:44 AM, Sean Christopherson wrote: > On Fri, Jul 16, 2021, Zeng Guang wrote: >> From: Robert Hoo >> >> New VMX capability MSR IA32_VMX_PROCBASED_CTLS3 conresponse to this new >> VM-Execution control field. And it is 64bit allow-1 semantics, not like >> previous capability MSRs 32bit allow-0 and 32bit allow-1. So with Tertiary >> VM-Execution control field introduced, 2 vmx_feature leaves are introduced, >> TERTIARY_CTLS_LOW and TERTIARY_CTLS_HIGH. > ... > >> /* >> * Note: If the comment begins with a quoted string, that string is used >> @@ -43,6 +43,7 @@ >> #define VMX_FEATURE_RDTSC_EXITING ( 1*32+ 12) /* "" VM-Exit on RDTSC */ >> #define VMX_FEATURE_CR3_LOAD_EXITING ( 1*32+ 15) /* "" VM-Exit on writes to CR3 */ >> #define VMX_FEATURE_CR3_STORE_EXITING ( 1*32+ 16) /* "" VM-Exit on reads from CR3 */ >> +#define VMX_FEATURE_TER_CONTROLS (1*32 + 17) /* "" Enable Tertiary VM-Execution Controls */ > Maybe spell out TERTIARY? SEC_CONTROLS is at least somewhat guessable, I doubt > TERTIARY is the first thing that comes to mind for most people when seeing "TER" :-) Agree. TERTIARY could be readable without any confusion. >> #define VMX_FEATURE_CR8_LOAD_EXITING ( 1*32+ 19) /* "" VM-Exit on writes to CR8 */ >> #define VMX_FEATURE_CR8_STORE_EXITING ( 1*32+ 20) /* "" VM-Exit on reads from CR8 */ >> #define VMX_FEATURE_VIRTUAL_TPR ( 1*32+ 21) /* "vtpr" TPR virtualization, a.k.a. TPR shadow */ >> diff --git a/arch/x86/kernel/cpu/feat_ctl.c b/arch/x86/kernel/cpu/feat_ctl.c >> index da696eb4821a..2e0272d127e4 100644 >> --- a/arch/x86/kernel/cpu/feat_ctl.c >> +++ b/arch/x86/kernel/cpu/feat_ctl.c >> @@ -15,6 +15,8 @@ enum vmx_feature_leafs { >> MISC_FEATURES = 0, >> PRIMARY_CTLS, >> SECONDARY_CTLS, >> + TERTIARY_CTLS_LOW, >> + TERTIARY_CTLS_HIGH, >> NR_VMX_FEATURE_WORDS, >> }; >> >> @@ -42,6 +44,13 @@ static void init_vmx_capabilities(struct cpuinfo_x86 *c) >> rdmsr_safe(MSR_IA32_VMX_PROCBASED_CTLS2, &ign, &supported); >> c->vmx_capability[SECONDARY_CTLS] = supported; >> >> + /* >> + * For tertiary execution controls MSR, it's actually a 64bit allowed-1. >> + */ >> + rdmsr_safe(MSR_IA32_VMX_PROCBASED_CTLS3, &ign, &supported); >> + c->vmx_capability[TERTIARY_CTLS_LOW] = ign; >> + c->vmx_capability[TERTIARY_CTLS_HIGH] = supported; > Assuming only the lower 32 bits are going to be used for the near future (next > few years), what about defining just TERTIARY_CTLS_LOW and then doing: > > /* > * Tertiary controls are 64-bit allowed-1, so unlikely other MSRs, the > * upper bits are ignored (because they're not used, yet...). > */ > rdmsr_safe(MSR_IA32_VMX_PROCBASED_CTLS3, &supported, &ign); > c->vmx_capability[TERTIARY_CTLS_LOW] = supported; > > I.e. punt the ugliness issue down the road a few years. Prefer to keep it complete, and use new variables like low/high consistent with its function meaning. Ok for that ? >> + >> rdmsr(MSR_IA32_VMX_PINBASED_CTLS, ign, supported); >> rdmsr_safe(MSR_IA32_VMX_VMFUNC, &ign, &funcs); >> >> -- >> 2.25.1 >>