Received: by 2002:ac0:8845:0:0:0:0:0 with SMTP id g63csp1017648img; Thu, 28 Feb 2019 11:34:13 -0800 (PST) X-Google-Smtp-Source: APXvYqxJCrDRkUCTW7Te15cLBOChRz54WGMAzG1fbvplVx2MipoIq6TTBzZTwoCZuHiRyNWhfe8t X-Received: by 2002:a63:3fc8:: with SMTP id m191mr811462pga.240.1551382453017; Thu, 28 Feb 2019 11:34:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1551382453; cv=none; d=google.com; s=arc-20160816; b=IX0788PTrgL3nShpZLC2dnIaglUyD/Pt6UTNyJ16NaZnVwIIkkESRVRhl0yEcfyPlK xB2oawyRfFvOeGRE5IurTTetCx1H9Ojk6SQ5hBRt+tf91U0WQMq/6xWeuByNocWCGTy4 KCidiWzIbXT54Wc180zBVW38gPFk6FN+VR9cnGFzmiOOi2+BfF8izg95WdOJYEz/h0xL jtl92a5e9t4tkgOS8rx7iP+4SYKq09m88n6q+0Rd5gzgzZIhLSlbq9yV5eBWuUClBc2j 9GRtLHNR8PJ55tsVElmkAi/SMC7POnAnUFrCwM7XraWMmjOAHK2ra74T1YA6MjpCI6SI JagQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=3m5YUfs1qcT4SzSfvdDEZxifR8KQPPix6NEU7oFqtvQ=; b=fzTTR5LnFH0lassaAP6aNaXVZNDRKX40ye8NVIA/QqVjXJRiRwGTpSHuHBSmXB0Co6 W8vf8hjnVzfChelFDZ2DCIXMMwkm9Z0M84j+FPeEFt+FiA3jv94TDZUKWNcE/GS6+ntO v5sx+0yIerU9D3OQXBJbWDcuilZ8m+7S9dG8B6dYGH3jPf5ApWd0NBJMuc+nVPpdVm9d 4TGDCq2rcLJ5WdSPf0XTs6vtGQrjYkk6Xe2dgAmR4FI+U4LY5SGWwTJwUa9hhoN1Ae4Z U0exHQ9tmMRalaTfHaZYwskDQS5mA/AnKoz8zWK+yKLhu5xiXdFXxqyrF9Wgi0s8B+/8 OVvQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id x17si18276860plr.435.2019.02.28.11.33.57; Thu, 28 Feb 2019 11:34:13 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731103AbfB1QRR (ORCPT + 99 others); Thu, 28 Feb 2019 11:17:17 -0500 Received: from mga07.intel.com ([134.134.136.100]:53979 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726403AbfB1QRQ (ORCPT ); Thu, 28 Feb 2019 11:17:16 -0500 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Feb 2019 08:17:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,423,1544515200"; d="scan'208";a="137985056" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.181]) by orsmga002.jf.intel.com with ESMTP; 28 Feb 2019 08:17:15 -0800 Date: Thu, 28 Feb 2019 08:17:15 -0800 From: Sean Christopherson To: Yang Weijiang Cc: pbonzini@redhat.com, rkrcmar@redhat.com, jmattson@google.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, mst@redhat.com, yu-cheng.yu@intel.com, Zhang Yi Z Subject: Re: [PATCH v3 6/8] KVM:VMX: Load Guest CET via VMCS when CET is enabled in Guest Message-ID: <20190228161715.GF6166@linux.intel.com> References: <20190225132716.6982-1-weijiang.yang@intel.com> <20190225132716.6982-7-weijiang.yang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190225132716.6982-7-weijiang.yang@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 25, 2019 at 09:27:14PM +0800, Yang Weijiang wrote: > "Load Guest CET state" bit controls whether guest CET states > will be loaded at Guest entry. Before doing that, KVM needs > to check if CPU CET feature is available. > > Signed-off-by: Zhang Yi Z > Signed-off-by: Yang Weijiang > --- > arch/x86/kvm/vmx.c | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 89ee086e1729..d32cee9ee079 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -55,6 +55,7 @@ > #include > #include > #include > +#include > > #include "trace.h" > #include "pmu.h" > @@ -4065,6 +4066,20 @@ static inline bool vmx_feature_control_msr_valid(struct kvm_vcpu *vcpu, > return !(val & ~valid_bits); > } > > +static int vmx_guest_cet_cap(struct kvm_vcpu *vcpu) > +{ > + u32 eax, ebx, ecx, edx; > + > + /* > + * Guest CET can work as long as HW supports the feature, independent > + * to Host SW enabling status. > + */ > + cpuid_count(7, 0, &eax, &ebx, &ecx, &edx); > + > + return ((ecx & bit(X86_FEATURE_SHSTK)) | > + (edx & bit(X86_FEATURE_IBT))) ? 1 : 0; Given the holes in the (current) architecture/spec, I think KVM has to require both features to be supported in the guest to allow CR4.CET to be enabled. Technically SHSTK and IBT can be enabled independently, but unless I'm missing something, supporting that in KVM (or any VMM) would be nasty and would likely degrade guest performance significantly. MSRs IA32_U_CET and IA32_S_CET have enable bits for each CET feature. Presumably the bits for each feature are reserved if the feature is not supported, e.g. SH_STK_EN is reserved to zero if SHSTK isn't supported. This wouldn't be a problem except that IA32_U_CET and the shadow stack MSRs, e.g. IA32_PL*_SSP, can be saved/restored via XSAVES/XRSTORS. The behavior is restricted by IA32_XSS, but again it's all or nothing, e.g. if any CET feature is supported then XSS_CET_{S,U} can be set. For example, if a guest supported IBT and !SHSTK, and the guest enabled XSS_CET_{S,I}, KVM would need to trap XSAVES/XRSTORS to enforce that the SHSTK bits in XSS_CET_U aren't set. And that doesn't even address the fact that the architecture defines the effects on the size of the XSAVE state area as being a bundled deal, e.g. IA32_XSS.CET_U=1 increases the size by 16 bytes (for IA32_U_CET and IA32_PL3_SSP) regardless of whether or not SHSTK is supported. One would assume that IA32_PL3_SSP doesn't exist if shadow stacks are not supported by the CPU. TL;DR: the architecture enumerates SHSTK and IBT independently, but the architecture effectively assumes they are bundled together. > +} > + > static int vmx_get_msr_feature(struct kvm_msr_entry *msr) > { > switch (msr->index) { > @@ -5409,6 +5424,23 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) > return 1; > } > > + /* > + * To enable Guest CET, check whether CPU CET feature is > + * available, if it's there, set Guest CET state loading bit > + * per CR4.CET status, otherwise, return a fault to Guest. > + */ > + if (vmx_guest_cet_cap(vcpu)) { This is wrong, it's checking the host capabilities. Use guest_cpuid_has() to query the guest capabilities. E.g. CET can be supported in the host but not exposed to guest, in which case the CPUID bits will not be "set" for the guest. > + if (cr4 & X86_CR4_CET) { No need for curly braces here, both the 'if' and 'else' contain a single statement. > + vmcs_set_bits(VM_ENTRY_CONTROLS, > + VM_ENTRY_LOAD_GUEST_CET_STATE); > + } else { > + vmcs_clear_bits(VM_ENTRY_CONTROLS, > + VM_ENTRY_LOAD_GUEST_CET_STATE); > + } > + } else if (cr4 & X86_CR4_CET) { > + return 1; > + } > + > if (to_vmx(vcpu)->nested.vmxon && !nested_cr4_valid(vcpu, cr4)) > return 1; > > -- > 2.17.1 >