Message ID | 20230914063325.85503-13-weijiang.yang@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp226728vqi; Thu, 14 Sep 2023 02:42:59 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHSt3moY26s0sQwKxpYVT9M6fkiLF5IHVJgysjmqkG9jsPemkrQlPv4lXXHWXkDn0/AHz6c X-Received: by 2002:a05:6358:15cf:b0:142:eb11:b0b8 with SMTP id t15-20020a05635815cf00b00142eb11b0b8mr2482890rwh.1.1694684579312; Thu, 14 Sep 2023 02:42:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694684579; cv=none; d=google.com; s=arc-20160816; b=cXFh+Nd2EWApqyOD5lQILYd9r+f7oZYfebD7xpyOAcvmvOmgdMERZYtvgHEbISjIab T+hlBLuAdy2Qv1aWnXq7MG09YmTaRrQjuS29TiLEIcIOF0VgoIc0gnJtO9y4eUyJdChO E5Lx5lRdiSvgM8pazOkepN30rMBh2Te7hQCSDS6zhBj0OMO7s9v6tAW1ZVc+ppHZf9Bc l+dyT9eImwf6aZN1vu/Yrz0wj43NLgBLy5uB+6+WJE0uw36mOz0P4wSR3HzQGv2SmrMX fGiigfu2WI6or+fy2laBXp2aiaTNy2bC7rr2rYYafif2PXj2i8bIEHL1qXpdAhpIrzzG /nGA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=2BGwlPxhseDDaNa8EciDLIxuH/vveOfEmcLvoMvgh4E=; fh=6178OKG2HPQ9OtiEe+sL0+amgAIGC3finCMsQ4RI52c=; b=0AwrMWex5yqwnnk3AfbSPd0B+UZy5I8B4K2mAvBBczrkxVL92w4idhJ/4GVooQ+sIm tXcllUWi5w1MoJAEoJixdU99yBOnewMLiy+Dsm0BnZn2bDsNXFiMlsc5Ybr0SoiXfbMe aBCn4AqcbzmdIe1c8tLRLumG/gyHDH6+zMVjcACPAceN5Oxs2WSyM4S4N8sVB1xYRro2 ydqJhbH+HbRJ0TdBQ4q0actgMy6aVZESRRSSqmDUH8P3JtGuFP+DiQb9kdm7Df0Yo0CM Uqza3VoDyB87Roay5ASopOyKGg8l6ElqRUlyTusByoiWkTOBzeHqC0rn+DNHLB5LysPV MawQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=R0jpc+kk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id z16-20020a656650000000b00565d05b2211si1112759pgv.819.2023.09.14.02.42.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Sep 2023 02:42:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=R0jpc+kk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 5014583328F2; Thu, 14 Sep 2023 02:39:13 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237521AbjINJiz (ORCPT <rfc822;chrisfriedt@gmail.com> + 35 others); Thu, 14 Sep 2023 05:38:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49870 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237307AbjINJiY (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 14 Sep 2023 05:38:24 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.88]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 42BA81FC0; Thu, 14 Sep 2023 02:38:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1694684300; x=1726220300; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=SgNlcd5ktSWMn2hdoXAHgtutH/xRPayw7k0UwTE2UoM=; b=R0jpc+kkwyUnD4vj0n5qK1M8z37Rw40Dnf/udaVAiWOsWK2eCysg7rtK vUIi4OqMn97Ilru9fx4+2O1OslChFVM6/KWyU9MDo3TzRyl/lw6/GQiAs Ey0T9JZ8+EzRuXIg93xvOSk4d3BsA8Jhp/Z8PACdNcrqoC65c5oIeFxtA KgUuvw7FfrMEimuF1I0Di3HOGQkruxeLtIdjBgSAOSWcfJPjpK/KhuB8F cMz213jdlLgC3sFBP7nSlx2Lg5EEpTX1IczxGvu09MmeqIuE3xQOZiM9t z343GFk2jLxEhAV0kmYynjlFSmQiOwOMiqZZOhbBR5nQMTBcUAEkh0ALZ Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10832"; a="409857365" X-IronPort-AV: E=Sophos;i="6.02,145,1688454000"; d="scan'208";a="409857365" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Sep 2023 02:38:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10832"; a="747656249" X-IronPort-AV: E=Sophos;i="6.02,145,1688454000"; d="scan'208";a="747656249" Received: from embargo.jf.intel.com ([10.165.9.183]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Sep 2023 02:38:19 -0700 From: Yang Weijiang <weijiang.yang@intel.com> To: seanjc@google.com, pbonzini@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Cc: dave.hansen@intel.com, peterz@infradead.org, chao.gao@intel.com, rick.p.edgecombe@intel.com, weijiang.yang@intel.com, john.allen@amd.com, Zhang Yi Z <yi.z.zhang@linux.intel.com> Subject: [PATCH v6 12/25] KVM: x86: Refresh CPUID on write to guest MSR_IA32_XSS Date: Thu, 14 Sep 2023 02:33:12 -0400 Message-Id: <20230914063325.85503-13-weijiang.yang@intel.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20230914063325.85503-1-weijiang.yang@intel.com> References: <20230914063325.85503-1-weijiang.yang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 14 Sep 2023 02:39:13 -0700 (PDT) X-Spam-Status: No, score=0.2 required=5.0 tests=DATE_IN_PAST_03_06, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777005577133653762 X-GMAIL-MSGID: 1777005577133653762 |
Series |
Enable CET Virtualization
|
|
Commit Message
Yang, Weijiang
Sept. 14, 2023, 6:33 a.m. UTC
Update CPUID.(EAX=0DH,ECX=1).EBX to reflect current required xstate size due to XSS MSR modification. CPUID(EAX=0DH,ECX=1).EBX reports the required storage size of all enabled xstate features in (XCR0 | IA32_XSS). The CPUID value can be used by guest before allocate sufficient xsave buffer. Note, KVM does not yet support any XSS based features, i.e. supported_xss is guaranteed to be zero at this time. Opportunistically modify XSS write access logic as: if !guest_cpuid_has(), write initiated from host is allowed iff the write is reset operaiton, i.e., data == 0, reject host_initiated non-reset write and any guest write. Suggested-by: Sean Christopherson <seanjc@google.com> Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/cpuid.c | 15 ++++++++++++++- arch/x86/kvm/x86.c | 13 +++++++++---- 3 files changed, 24 insertions(+), 5 deletions(-)
Comments
>diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >index 0fc5e6312e93..d77b030e996c 100644 >--- a/arch/x86/include/asm/kvm_host.h >+++ b/arch/x86/include/asm/kvm_host.h >@@ -803,6 +803,7 @@ struct kvm_vcpu_arch { > > u64 xcr0; > u64 guest_supported_xcr0; >+ u64 guest_supported_xss; This structure has the ia32_xss field. how about moving it here for symmetry? > > struct kvm_pio_request pio; > void *pio_data; >diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >index 1f206caec559..4e7a820cba62 100644 >--- a/arch/x86/kvm/cpuid.c >+++ b/arch/x86/kvm/cpuid.c >@@ -275,7 +275,8 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e > best = cpuid_entry2_find(entries, nent, 0xD, 1); > if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) || > cpuid_entry_has(best, X86_FEATURE_XSAVEC))) >- best->ebx = xstate_required_size(vcpu->arch.xcr0, true); >+ best->ebx = xstate_required_size(vcpu->arch.xcr0 | >+ vcpu->arch.ia32_xss, true); > > best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent); > if (kvm_hlt_in_guest(vcpu->kvm) && best && >@@ -312,6 +313,17 @@ static u64 vcpu_get_supported_xcr0(struct kvm_vcpu *vcpu) > return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; > } > >+static u64 vcpu_get_supported_xss(struct kvm_vcpu *vcpu) >+{ >+ struct kvm_cpuid_entry2 *best; >+ >+ best = kvm_find_cpuid_entry_index(vcpu, 0xd, 1); >+ if (!best) >+ return 0; >+ >+ return (best->ecx | ((u64)best->edx << 32)) & kvm_caps.supported_xss; >+} >+ > static bool kvm_cpuid_has_hyperv(struct kvm_cpuid_entry2 *entries, int nent) > { > struct kvm_cpuid_entry2 *entry; >@@ -358,6 +370,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > } > > vcpu->arch.guest_supported_xcr0 = vcpu_get_supported_xcr0(vcpu); >+ vcpu->arch.guest_supported_xss = vcpu_get_supported_xss(vcpu); > > /* > * FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >index 1258d1d6dd52..9a616d84bd39 100644 >--- a/arch/x86/kvm/x86.c >+++ b/arch/x86/kvm/x86.c >@@ -3795,20 +3795,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > vcpu->arch.ia32_tsc_adjust_msr += adj; > } > break; >- case MSR_IA32_XSS: >- if (!msr_info->host_initiated && >- !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) >+ case MSR_IA32_XSS: { >+ bool host_msr_reset = msr_info->host_initiated && data == 0; >+ >+ if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES) && >+ (!host_msr_reset || !msr_info->host_initiated)) !msr_info->host_initiated can be dropped here.
On 10/8/2023 1:54 PM, Chao Gao wrote: >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index 0fc5e6312e93..d77b030e996c 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -803,6 +803,7 @@ struct kvm_vcpu_arch { >> >> u64 xcr0; >> u64 guest_supported_xcr0; >> + u64 guest_supported_xss; > This structure has the ia32_xss field. how about moving it here for symmetry? OK, will do it, thanks! >> struct kvm_pio_request pio; >> void *pio_data; >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >> index 1f206caec559..4e7a820cba62 100644 >> --- a/arch/x86/kvm/cpuid.c >> +++ b/arch/x86/kvm/cpuid.c >> @@ -275,7 +275,8 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e >> best = cpuid_entry2_find(entries, nent, 0xD, 1); >> if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) || >> cpuid_entry_has(best, X86_FEATURE_XSAVEC))) >> - best->ebx = xstate_required_size(vcpu->arch.xcr0, true); >> + best->ebx = xstate_required_size(vcpu->arch.xcr0 | >> + vcpu->arch.ia32_xss, true); >> >> best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent); >> if (kvm_hlt_in_guest(vcpu->kvm) && best && >> @@ -312,6 +313,17 @@ static u64 vcpu_get_supported_xcr0(struct kvm_vcpu *vcpu) >> return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; >> } >> >> +static u64 vcpu_get_supported_xss(struct kvm_vcpu *vcpu) >> +{ >> + struct kvm_cpuid_entry2 *best; >> + >> + best = kvm_find_cpuid_entry_index(vcpu, 0xd, 1); >> + if (!best) >> + return 0; >> + >> + return (best->ecx | ((u64)best->edx << 32)) & kvm_caps.supported_xss; >> +} >> + >> static bool kvm_cpuid_has_hyperv(struct kvm_cpuid_entry2 *entries, int nent) >> { >> struct kvm_cpuid_entry2 *entry; >> @@ -358,6 +370,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) >> } >> >> vcpu->arch.guest_supported_xcr0 = vcpu_get_supported_xcr0(vcpu); >> + vcpu->arch.guest_supported_xss = vcpu_get_supported_xss(vcpu); >> >> /* >> * FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 1258d1d6dd52..9a616d84bd39 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -3795,20 +3795,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> vcpu->arch.ia32_tsc_adjust_msr += adj; >> } >> break; >> - case MSR_IA32_XSS: >> - if (!msr_info->host_initiated && >> - !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) >> + case MSR_IA32_XSS: { >> + bool host_msr_reset = msr_info->host_initiated && data == 0; >> + >> + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES) && >> + (!host_msr_reset || !msr_info->host_initiated)) > !msr_info->host_initiated can be dropped here. Yes, it's not necessary, will remove it, thanks!
On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote: > Update CPUID.(EAX=0DH,ECX=1).EBX to reflect current required xstate size > due to XSS MSR modification. > CPUID(EAX=0DH,ECX=1).EBX reports the required storage size of all enabled > xstate features in (XCR0 | IA32_XSS). The CPUID value can be used by guest > before allocate sufficient xsave buffer. > > Note, KVM does not yet support any XSS based features, i.e. supported_xss > is guaranteed to be zero at this time. > > Opportunistically modify XSS write access logic as: if !guest_cpuid_has(), > write initiated from host is allowed iff the write is reset operaiton, > i.e., data == 0, reject host_initiated non-reset write and any guest write. The commit message is not clear and somewhat misleading because it forces the reader to parse the whole patch before one could understand what '!guest_cpuid_has()' means. Also I don't think that the term 'reset operation' is a good choice, because it is too closely related to vCPU reset IMHO. Let's at least call it 'reset to a default value' or something like that. Also note that 0 is not always the default/reset value of an MSR. I suggest this instead: "If XSAVES is not enabled in the guest CPUID, forbid setting IA32_XSS msr to anything but 0, even if the write is host initiated." Also, isn't this change is at least in theory not backward compatible? While KVM didn't report IA32_XSS as one needing save/restore, the userspace could before this patch set the IA32_XSS to any value, now it can't. Maybe it's safer to allow to set any value, ignore the set value and issue a WARN_ON_ONCE or something? Finally, I think that this change is better to be done in a separate patch because it is unrelated and might not even be backward compatible. Best regards, Maxim Levitsky > > Suggested-by: Sean Christopherson <seanjc@google.com> > Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/cpuid.c | 15 ++++++++++++++- > arch/x86/kvm/x86.c | 13 +++++++++---- > 3 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 0fc5e6312e93..d77b030e996c 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -803,6 +803,7 @@ struct kvm_vcpu_arch { > > u64 xcr0; > u64 guest_supported_xcr0; > + u64 guest_supported_xss; > > struct kvm_pio_request pio; > void *pio_data; > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 1f206caec559..4e7a820cba62 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -275,7 +275,8 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e > best = cpuid_entry2_find(entries, nent, 0xD, 1); > if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) || > cpuid_entry_has(best, X86_FEATURE_XSAVEC))) > - best->ebx = xstate_required_size(vcpu->arch.xcr0, true); > + best->ebx = xstate_required_size(vcpu->arch.xcr0 | > + vcpu->arch.ia32_xss, true); > > best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent); > if (kvm_hlt_in_guest(vcpu->kvm) && best && > @@ -312,6 +313,17 @@ static u64 vcpu_get_supported_xcr0(struct kvm_vcpu *vcpu) > return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; > } > > +static u64 vcpu_get_supported_xss(struct kvm_vcpu *vcpu) > +{ > + struct kvm_cpuid_entry2 *best; > + > + best = kvm_find_cpuid_entry_index(vcpu, 0xd, 1); > + if (!best) > + return 0; > + > + return (best->ecx | ((u64)best->edx << 32)) & kvm_caps.supported_xss; > +} Same question as one for patch that added vcpu_get_supported_xcr0() Why to have per vCPU supported XSS if we assume that all CPUs have the same CPUID? I mean I am not against supporting hybrid CPU models, but KVM currently doesn't support this and this creates illusion that it does. > + > static bool kvm_cpuid_has_hyperv(struct kvm_cpuid_entry2 *entries, int nent) > { > struct kvm_cpuid_entry2 *entry; > @@ -358,6 +370,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > } > > vcpu->arch.guest_supported_xcr0 = vcpu_get_supported_xcr0(vcpu); > + vcpu->arch.guest_supported_xss = vcpu_get_supported_xss(vcpu); > > /* > * FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 1258d1d6dd52..9a616d84bd39 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3795,20 +3795,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > vcpu->arch.ia32_tsc_adjust_msr += adj; > } > break; > - case MSR_IA32_XSS: > - if (!msr_info->host_initiated && > - !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) > + case MSR_IA32_XSS: { > + bool host_msr_reset = msr_info->host_initiated && data == 0; > + > + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES) && > + (!host_msr_reset || !msr_info->host_initiated)) > return 1; > /* > * KVM supports exposing PT to the guest, but does not support > * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than > * XSAVES/XRSTORS to save/restore PT MSRs. > */ Just in case.... TODO > - if (data & ~kvm_caps.supported_xss) > + if (data & ~vcpu->arch.guest_supported_xss) > return 1; > + if (vcpu->arch.ia32_xss == data) > + break; > vcpu->arch.ia32_xss = data; > kvm_update_cpuid_runtime(vcpu); > break; > + } > case MSR_SMI_COUNT: > if (!msr_info->host_initiated) > return 1;
On Tue, Oct 31, 2023, Maxim Levitsky wrote: > On Thu, 2023-09-14 at 02:33 -0400, Yang Weijiang wrote: > > @@ -312,6 +313,17 @@ static u64 vcpu_get_supported_xcr0(struct kvm_vcpu *vcpu) > > return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; > > } > > > > +static u64 vcpu_get_supported_xss(struct kvm_vcpu *vcpu) > > +{ > > + struct kvm_cpuid_entry2 *best; > > + > > + best = kvm_find_cpuid_entry_index(vcpu, 0xd, 1); > > + if (!best) > > + return 0; > > + > > + return (best->ecx | ((u64)best->edx << 32)) & kvm_caps.supported_xss; > > +} > > Same question as one for patch that added vcpu_get_supported_xcr0() > Why to have per vCPU supported XSS if we assume that all CPUs have the same > CPUID? > > I mean I am not against supporting hybrid CPU models, but KVM currently doesn't > support this and this creates illusion that it does. KVM does "support" hybrid vCPU models in the sense that KVM has allow hybrid models since forever. There are definite things that won't work, e.g. not all relevant CPUID bits are captured in kvm_mmu_page_role, and so KVM will incorrectly share page tables across vCPUs that are technically incompatible. But for many features, heterogenous vCPU models do Just Work as far as KVM is concerned. There likely isn't a real world kernel that supports heterogenous feature sets for things like XSS and XCR0, but that's a guest software limitation, not a limitation of KVM's CPU virtualization. As with many things, KVM's ABI is to let userspace shoot themselves in the foot.
On 9/14/2023 2:33 PM, Yang Weijiang wrote: > Update CPUID.(EAX=0DH,ECX=1).EBX to reflect current required xstate size > due to XSS MSR modification. > CPUID(EAX=0DH,ECX=1).EBX reports the required storage size of all enabled > xstate features in (XCR0 | IA32_XSS). The CPUID value can be used by guest > before allocate sufficient xsave buffer. > > Note, KVM does not yet support any XSS based features, i.e. supported_xss > is guaranteed to be zero at this time. > > Opportunistically modify XSS write access logic as: if !guest_cpuid_has(), > write initiated from host is allowed iff the write is reset operaiton, > i.e., data == 0, reject host_initiated non-reset write and any guest write. Hi Sean & Polo, During code review of Enable CET Virtualization v5 patchset, there were discussions about "do a wholesale cleanup of all the cases that essentially allow userspace to do KVM_SET_MSR before KVM_SET_CPUID2", i.e. force the order between KVM_SET_CPUID2 and KVM_SET_MSR, but allow the host_initiated path with default (generally 0) value. https://lore.kernel.org/kvm/ZM1C+ILRMCfzJxx7@google.com/ https://lore.kernel.org/kvm/CABgObfbvr8F8g5hJN6jn95m7u7m2+8ACkqO25KAZwRmJ9AncZg@mail.gmail.com/ I can take the task to do the code cleanup. Before going any further, I want to confirm it is still the direction intended, right? > > Suggested-by: Sean Christopherson <seanjc@google.com> > Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/cpuid.c | 15 ++++++++++++++- > arch/x86/kvm/x86.c | 13 +++++++++---- > 3 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 0fc5e6312e93..d77b030e996c 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -803,6 +803,7 @@ struct kvm_vcpu_arch { > > u64 xcr0; > u64 guest_supported_xcr0; > + u64 guest_supported_xss; > > struct kvm_pio_request pio; > void *pio_data; > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 1f206caec559..4e7a820cba62 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -275,7 +275,8 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e > best = cpuid_entry2_find(entries, nent, 0xD, 1); > if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) || > cpuid_entry_has(best, X86_FEATURE_XSAVEC))) > - best->ebx = xstate_required_size(vcpu->arch.xcr0, true); > + best->ebx = xstate_required_size(vcpu->arch.xcr0 | > + vcpu->arch.ia32_xss, true); > > best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent); > if (kvm_hlt_in_guest(vcpu->kvm) && best && > @@ -312,6 +313,17 @@ static u64 vcpu_get_supported_xcr0(struct kvm_vcpu *vcpu) > return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; > } > > +static u64 vcpu_get_supported_xss(struct kvm_vcpu *vcpu) > +{ > + struct kvm_cpuid_entry2 *best; > + > + best = kvm_find_cpuid_entry_index(vcpu, 0xd, 1); > + if (!best) > + return 0; > + > + return (best->ecx | ((u64)best->edx << 32)) & kvm_caps.supported_xss; > +} > + > static bool kvm_cpuid_has_hyperv(struct kvm_cpuid_entry2 *entries, int nent) > { > struct kvm_cpuid_entry2 *entry; > @@ -358,6 +370,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > } > > vcpu->arch.guest_supported_xcr0 = vcpu_get_supported_xcr0(vcpu); > + vcpu->arch.guest_supported_xss = vcpu_get_supported_xss(vcpu); > > /* > * FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 1258d1d6dd52..9a616d84bd39 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3795,20 +3795,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > vcpu->arch.ia32_tsc_adjust_msr += adj; > } > break; > - case MSR_IA32_XSS: > - if (!msr_info->host_initiated && > - !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) > + case MSR_IA32_XSS: { > + bool host_msr_reset = msr_info->host_initiated && data == 0; > + > + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES) && > + (!host_msr_reset || !msr_info->host_initiated)) > return 1; > /* > * KVM supports exposing PT to the guest, but does not support > * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than > * XSAVES/XRSTORS to save/restore PT MSRs. > */ > - if (data & ~kvm_caps.supported_xss) > + if (data & ~vcpu->arch.guest_supported_xss) > return 1; > + if (vcpu->arch.ia32_xss == data) > + break; > vcpu->arch.ia32_xss = data; > kvm_update_cpuid_runtime(vcpu); > break; > + } > case MSR_SMI_COUNT: > if (!msr_info->host_initiated) > return 1;
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 0fc5e6312e93..d77b030e996c 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -803,6 +803,7 @@ struct kvm_vcpu_arch { u64 xcr0; u64 guest_supported_xcr0; + u64 guest_supported_xss; struct kvm_pio_request pio; void *pio_data; diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 1f206caec559..4e7a820cba62 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -275,7 +275,8 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e best = cpuid_entry2_find(entries, nent, 0xD, 1); if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) || cpuid_entry_has(best, X86_FEATURE_XSAVEC))) - best->ebx = xstate_required_size(vcpu->arch.xcr0, true); + best->ebx = xstate_required_size(vcpu->arch.xcr0 | + vcpu->arch.ia32_xss, true); best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent); if (kvm_hlt_in_guest(vcpu->kvm) && best && @@ -312,6 +313,17 @@ static u64 vcpu_get_supported_xcr0(struct kvm_vcpu *vcpu) return (best->eax | ((u64)best->edx << 32)) & kvm_caps.supported_xcr0; } +static u64 vcpu_get_supported_xss(struct kvm_vcpu *vcpu) +{ + struct kvm_cpuid_entry2 *best; + + best = kvm_find_cpuid_entry_index(vcpu, 0xd, 1); + if (!best) + return 0; + + return (best->ecx | ((u64)best->edx << 32)) & kvm_caps.supported_xss; +} + static bool kvm_cpuid_has_hyperv(struct kvm_cpuid_entry2 *entries, int nent) { struct kvm_cpuid_entry2 *entry; @@ -358,6 +370,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) } vcpu->arch.guest_supported_xcr0 = vcpu_get_supported_xcr0(vcpu); + vcpu->arch.guest_supported_xss = vcpu_get_supported_xss(vcpu); /* * FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 1258d1d6dd52..9a616d84bd39 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3795,20 +3795,25 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) vcpu->arch.ia32_tsc_adjust_msr += adj; } break; - case MSR_IA32_XSS: - if (!msr_info->host_initiated && - !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) + case MSR_IA32_XSS: { + bool host_msr_reset = msr_info->host_initiated && data == 0; + + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVES) && + (!host_msr_reset || !msr_info->host_initiated)) return 1; /* * KVM supports exposing PT to the guest, but does not support * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than * XSAVES/XRSTORS to save/restore PT MSRs. */ - if (data & ~kvm_caps.supported_xss) + if (data & ~vcpu->arch.guest_supported_xss) return 1; + if (vcpu->arch.ia32_xss == data) + break; vcpu->arch.ia32_xss = data; kvm_update_cpuid_runtime(vcpu); break; + } case MSR_SMI_COUNT: if (!msr_info->host_initiated) return 1;