From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH RFC 05/31] xen/x86: Collect more CPUID feature words Date: Tue, 22 Dec 2015 17:17:44 +0000 Message-ID: <567985B8.8040402@citrix.com> References: <1450301073-28191-1-git-send-email-andrew.cooper3@citrix.com> <1450301073-28191-6-git-send-email-andrew.cooper3@citrix.com> <56798C8402000078000C260B@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56798C8402000078000C260B@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Xen-devel List-Id: xen-devel@lists.xenproject.org On 22/12/15 16:46, Jan Beulich wrote: >>>> On 16.12.15 at 22:24, wrote: >> --- a/xen/arch/x86/cpu/amd.c >> +++ b/xen/arch/x86/cpu/amd.c >> @@ -481,7 +481,7 @@ static void __devinit init_amd(struct cpuinfo_x86 *c) >> >> if (c->extended_cpuid_level >= 0x80000007) { >> c->x86_power = cpuid_edx(0x80000007); >> - if (c->x86_power & (1<<8)) { >> + if (c->x86_power & cpufeat_mask(X86_FEATURE_ITSC)) { > generic_identify() has already run by the time we get here, so no > need to re-read cpuid_edx() (interestingly enough you leverage > this in init_intel()). I didn't change either in this regard (observe that it is just in context), although it is now obvious that x86_power can be dropped completely, given that isn't used anywhere else. andrewcoop@andrewcoop:/local/xen.git/xen$ git grep x86_power arch/x86/cpu/amd.c:541: c->x86_power = cpuid_edx(0x80000007); arch/x86/cpu/amd.c:542: if (c->x86_power & cpufeat_mask(X86_FEATURE_ITSC)) { include/asm-x86/processor.h:194: int x86_power; > >> --- a/xen/arch/x86/cpuid/cpuid.c >> +++ b/xen/arch/x86/cpuid/cpuid.c >> @@ -103,6 +103,12 @@ const uint32_t known_features[XEN_NR_FEATURESET_ENTRIES] = >> cpufeat_mask(X86_FEATURE_RDSEED) | >> cpufeat_mask(X86_FEATURE_ADX) | >> cpufeat_mask(X86_FEATURE_SMAP)), >> + >> + [cpufeat_word(X86_FEATURE_PREFETCHWT1)] = (cpufeat_mask(X86_FEATURE_PREFETCHWT1)), >> + >> + [cpufeat_word(X86_FEATURE_ITSC)] = (cpufeat_mask(X86_FEATURE_ITSC)), >> + >> + [cpufeat_word(X86_FEATURE_CLZERO)] = (cpufeat_mask(X86_FEATURE_CLZERO)), >> }; > Are these indeed the only ones in their groups that can be safely > marked "known"? I am going to go out on a limb and say yes here. The point of "known" is "someone has actively thought about what the hyperivsor implications of using this feature is". These are the only ones which I feel safe declaring to be such at the time of writing. Rebasing onto staging has introduced the PKU bits, which are adjacent to PREFETCHWT1, and altered this patch quite a lot. ~Andrew