Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp40144imu; Wed, 2 Jan 2019 13:37:48 -0800 (PST) X-Google-Smtp-Source: ALg8bN4xIGcTokRxVas3ZXlb5HlT+dAlOo3CVVUCsHXvvlqvSlxQIxIVfHIb8dIwEk5jkyR0E/yZ X-Received: by 2002:a17:902:5a0b:: with SMTP id q11mr2568147pli.186.1546465068116; Wed, 02 Jan 2019 13:37:48 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1546465068; cv=none; d=google.com; s=arc-20160816; b=X+VxvZKLNquY8hiY8JoTKJencDid1xLQGkE8c6hL4WiioJMS9X5byBfQkwkToUVTeB tqhqvm/jznLaJ7wFObQy6+Ox/3tRBj9lL/JfIym7Aw7ZMOiODH271Y+JajAk0Zw8t2gn 6JM8XTRRk6S9uA/R0+EuUrtxwrgzkY9kvx9ZnexjyII/SnFKTojjaBqJqiRr2Zir/QqA lbO5TNd7OTH0nVy30LrxrreqWkUIOGa4bbhEbVx48g64QWbERDinUOeFsi8kSiBFvoyh hbodGomtoPVzb+IhBfpeWyXzGGjff3rLTRcc5W1PgkMndyruRp+ckLuifZRpvo5h1bOo lK3Q== 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=EWYEyxHRYb4mYI8hmGhzVMTjXz+2MLXEF9uCKQssQyM=; b=JfCixDQ1CgUAf0ykalUw2xTpg7w+/QOTkdIBFOGDXHVjkXY7jk5f++D+fzIK/Eujnx i8Bn5F2MpWaelkzP4W+Fr9t7MV8bIk3a+x1Zlh1vq+kujRksrItKjTQVuxjToLW4AgDF R6XDLYrKI47PdpvZaujIH26bD7jgkUXiUVXjHZ9eUvk+vyFekd5VdZXlCJDviArAo5eM 9+eCs9V8XrSnJyhejDWMOVAmfEQQNXJmNF24guUA44KgDKg61tHNeRDm4Yve10aBPFwO ZpGdooj6kK847dmeuFNQEly3z4A+OD9AseWVLb0lOqsXe3FzSTuUEi5DOMBdu5j3lhjY a94g== 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 n1si49938365pgh.172.2019.01.02.13.37.11; Wed, 02 Jan 2019 13:37:48 -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 S1728351AbfABStb (ORCPT + 99 others); Wed, 2 Jan 2019 13:49:31 -0500 Received: from mga03.intel.com ([134.134.136.65]:50811 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728196AbfABStb (ORCPT ); Wed, 2 Jan 2019 13:49:31 -0500 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Jan 2019 10:49:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,432,1539673200"; d="scan'208";a="134791813" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.154]) by fmsmga001.fm.intel.com with ESMTP; 02 Jan 2019 10:49:30 -0800 Date: Wed, 2 Jan 2019 10:49:30 -0800 From: Sean Christopherson To: Yang Weijiang Cc: pbonzini@redhat.com, rkrcmar@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, mst@redhat.com, yu-cheng.yu@intel.com, yi.z.zhang@intel.com, hjl.tools@gmail.com, Zhang Yi Z Subject: Re: [PATCH v1 6/8] kvm:cpuid Add CPUID support for CET xsaves component query. Message-ID: <20190102184930.GD7460@linux.intel.com> References: <20181226081532.30698-1-weijiang.yang@intel.com> <20181226081532.30698-7-weijiang.yang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181226081532.30698-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 Wed, Dec 26, 2018 at 04:15:30PM +0800, Yang Weijiang wrote: > CET xsaves component size is queried through CPUID.(EAX=0xD, ECX=11) > and CPUID.(EAX=0xD, ECX=12). > > Signed-off-by: Zhang Yi Z > Signed-off-by: Yang Weijiang > --- > arch/x86/kvm/cpuid.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 7bcfa61375c0..5bac31e58955 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -565,6 +565,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > case 0xd: { > int idx, i; > u64 supported = kvm_supported_xcr0(); > + u64 sv_supported; What about u_supported and s_supported? sv_supported doesn't make me think "supervisor"e. > entry->eax &= supported; > entry->ebx = xstate_required_size(supported, false); > @@ -584,18 +585,23 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > entry[i].eax &= kvm_cpuid_D_1_eax_x86_features; > cpuid_mask(&entry[i].eax, CPUID_D_1_EAX); > entry[i].ebx = 0; > + sv_supported = entry[i].ecx + > + ((u64)entry[i].edx << 32); Use '|' instead of '+' to smush ECX and EDX together, as is it looks like the code is calculating a size or something. > if (entry[i].eax & (F(XSAVES)|F(XSAVEC))) > entry[i].ebx = > xstate_required_size(supported, > true); > - } else { > + } else if (!(entry[i].ecx & 1)) { Now that we're actually consuming bit 0, it'd be nice to formally define it as referring to supervisor state. What about styling the code like this? Might make it more obvious that the logic for user vs. supervisor is identical, i.e. ECX bit 0 only determines which mask is applied. if (idx == 1) { ... } else { supported = (entry[i].ecx & 1) ? s_supported : u_supported; if (entry[i].eax == 0 || !(supported & mask)) continue; entry[i].ecx &= 1; entry[i].edx = 0; } > if (entry[i].eax == 0 || !(supported & mask)) > continue; > - if (WARN_ON_ONCE(entry[i].ecx & 1)) > + entry[i].ecx = 0; > + entry[i].edx = 0; > + } else { > + if (entry[i].eax == 0 || !(sv_supported & mask)) > continue; > + entry[i].ecx = 1; > + entry[i].edx = 0; > } > - entry[i].ecx = 0; > - entry[i].edx = 0; Removing this entirely isn't correct for CPUID.0xD.0x1, KVM should still zero out the bits it doesn't handle, e.g. KVM will signal a fault if the guest attempts to set any bits in IA32_XSS other than the CET bits. > entry[i].flags |= > KVM_CPUID_FLAG_SIGNIFCANT_INDEX; > ++*nent; > -- > 2.17.1 >