Re: [Xen-devel] [PATCH RFC 01/31] xen/public: Export featureset information in the public API

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.12.15 at 17:42, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/12/15 16:28, Jan Beulich wrote:
>>>>> On 16.12.15 at 22:24, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> +/* VIA/Cyrix/Centaur-defined CPU features, CPUID level 0xC0000001, FSMAX+2 
>>> */
>>> +#define X86_FEATURE_XSTORE ((FSMAX+1)*32+ 2) /* on-CPU RNG present (xstore 
>>> insn) */
>>> +#define X86_FEATURE_XSTORE_EN      ((FSMAX+1)*32+ 3) /* on-CPU RNG enabled 
>>> */
>>> +#define X86_FEATURE_XCRYPT ((FSMAX+1)*32+ 6) /* on-CPU crypto (xcrypt 
>>> insn) */
>>> +#define X86_FEATURE_XCRYPT_EN      ((FSMAX+1)*32+ 7) /* on-CPU crypto 
>>> enabled */
>>> +#define X86_FEATURE_ACE2   ((FSMAX+1)*32+ 8) /* Advanced Cryptography 
>>> Engine v2 */
>>> +#define X86_FEATURE_ACE2_EN        ((FSMAX+1)*32+ 9) /* ACE v2 enabled */
>>> +#define X86_FEATURE_PHE            ((FSMAX+1)*32+10) /* PadLock Hash 
>>> Engine */
>>> +#define X86_FEATURE_PHE_EN ((FSMAX+1)*32+11) /* PHE enabled */
>>> +#define X86_FEATURE_PMM            ((FSMAX+1)*32+12) /* PadLock Montgomery 
>>> Multiplier */
>>> +#define X86_FEATURE_PMM_EN ((FSMAX+1)*32+13) /* PMM enabled */
>> None of these get consumed anywhere - if you already touch this
>> code, just drop all of them.
> 
> X86_FEATURE_XSTORE gets used in xen/arch/x86/cpu/centaur.c to stash word
> 0xC0000001, but nothing actually reads the stashed values.
> 
> I could do a precursor patch which drops the stashing of this
> information, which will result in NCAPINTS getting shorter by the end of
> the series.

Yes, that's what I meant to imply.

>>> +/*
>>> + * CPUID leaf shorthand:
>>> + * - optional 'e', Extended (0x8xxxxxxx)
>>> + * - leaf, uppercase hex
>>> + * - register, lowercase
>>> + * - optional subleaf, uppercase hex
>>> + */
>>> +#define XEN_FEATURESET_1d     0 /* 0x00000001.edx      */
>>> +#define XEN_FEATURESET_1c     1 /* 0x00000001.ecx      */
>>> +#define XEN_FEATURESET_e1d    2 /* 0x80000001.edx      */
>>> +#define XEN_FEATURESET_e1c    3 /* 0x80000001.ecx      */
>>> +#define XEN_FEATURESET_Da1    4 /* 0x0000000d:1.eax    */
>>> +#define XEN_FEATURESET_7b0    5 /* 0x00000007:0.ebx    */
>> This ends up being pretty cryptic.
> 
> Perhaps at a first glance, but there are so many uses that a shorthand
> really is needed.  See especially the MSR masking patches towards the
> end of the series.

I understand that something short is needed. But as the main
identifier in the ABI?

>>> +#define XEN_NR_FEATURESET_ENTRIES (XEN_FEATURESET_7b0 + 1)
>> This shouldn't be exposed, as it will necessarily change sooner or later.
> 
> Hmm yes - I think I can alter where this lives, although libxc does need
> to be able to get this value.

Perhaps keep it where it is, but put it inside #ifdef __XEN__?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel