Re: [PATCH] x86/xstate: Don't special case feature collection

From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Xen-devel <xen-devel@lists.xen.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH] x86/xstate: Don't special case feature collection
Date: Mon, 25 Feb 2019 10:01:25 -0700	[thread overview]
Message-ID: <5C741F65020000780021A03F@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <1551107027-1089-1-git-send-email-andrew.cooper3@citrix.com>

>>> On 25.02.19 at 16:03, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -414,6 +414,10 @@ static void generic_identify(struct cpuinfo_x86 *c)
>  			    &c->x86_capability[cpufeat_word(X86_FEATURE_FSGSBASE)],
>  			    &c->x86_capability[cpufeat_word(X86_FEATURE_PKU)],
>  			    &c->x86_capability[cpufeat_word(X86_FEATURE_AVX512_4VNNIW)]);
> +	if ( c->cpuid_level >= 0xd )
> +		cpuid_count(0xd, 1,
> +			    &c->x86_capability[cpufeat_word(X86_FEATURE_XSAVEOPT)],
> +			    &tmp, &tmp, &tmp);

I think this file is still trying to be Linux style, with a couple of
violations. I do notice that the immediately preceding if() is
the same style as the one you add, but I think it would be
more consistent to omit the blanks immediately inside the
parentheses here. Either way,
Acked-by: Jan Beulich <jbeulich@suse.com>

I take it that you're intentionally using 0xd in favor of
XSTATE_CPUID?

As an aside I wonder why we have cpuid_count_ebx(), but
not e.g. cpuid_count_eax() as would have been useful here.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel