Re: [Xen-devel] [PATCH 10/27] x86/cpuid: Introduce named feature bitmaps

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.17 at 18:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/01/17 15:44, Jan Beulich wrote:
>>>>> On 04.01.17 at 13:39, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> Use anonymous unions to access the feature leaves as complete words, and by
>>> named individual feature.
>>>
>>> A feature name is introduced for every architectural X86_FEATURE_*, other 
>>> than
>>> the dynamically calculated values such as APIC, OSXSAVE and OSPKE.
>> A rationale for this change would be nice to have here, as the
>> redundancy with public/arch-x86/cpufeatureset.h means any
>> addition will now need to change two places. Would it be possible
>> for gen-cpuid.py to generate these bitfield declarations?
> 
> Hmm.  I hadn't considered that as an option.
> 
> Thinking about it however, I'd ideally prefer not to hide the
> declarations behind a macro.

What's wrong with that? It's surely better than having to keep two
pieces of code in sync manually.

>>> @@ -113,7 +131,34 @@ struct cpuid_policy
>>>          struct cpuid_leaf raw[CPUID_GUEST_NR_FEAT];
>>>          struct {
>>>              struct {
>>> -                uint32_t max_subleaf, _7b0, _7c0, _7d0;
>>> +                uint32_t max_subleaf;
>>> +                union {
>>> +                    uint32_t _7b0;
>>> +                    struct {
>>> +                        bool fsgsbase:1, tsc_adjust:1, sgx:1, bmi1:1, 
>>> hle:1, avx2:1, fdp_excp_only:1, smep:1,
>>> +                             bmi2:1, erms:1, invpcid:1, rtm:1, pqm:1, 
>>> no_fpu_sel:1, mpx:1, pqe:1,
>>> +                             avx512f:1, avx512dq:1, rdseed:1, adx:1, 
>>> smap:1, avx512ifma:1, /* pcommit */:1, clflushopt:1,
>>> +                             clwb:1, /* pt */:1, avx512pf:1, avx512er:1, 
>>> avx512cd:1, sha:1, avx512bw:1, avx512vl:1;
>> The commented out entries here don't match the commit message.
> 
> Well - they are not yet implemented.

Or have been removed, in the case of pcommit. But my point really
was that the commit message might better be relaxed/extended a
little in this regard.

>>> +                    };
>>> +                };
>>> +                union {
>>> +                    uint32_t _7c0;
>>> +                    struct {
>>> +                        bool prefetchwt1:1, avx512vbmi:1, :1, pku: 1, :1, 
>>> :1, :1, :1,
>>> +                             :1, :1, :1, :1, :1, :1, :1, :1,
>>> +                             :1, :1, :1, :1, :1, :1, :1, :1,
>>> +                             :1, :1, :1, :1, :1, :1, :1, :1;
>> This is ugly, but I remember you saying (on irc?) the compiler
>> doesn't allow bitfields wider than one bit for bool ...
> 
> Correct.  I was quite surprised by this, but I can understand that bool
> foo:2 is quite meaningless when foo can strictly only take a binary value.

Thinking about it another time - what's wrong with using uint32_t
instead of bool here, allowing consecutive unknown fields to be
folded?

Jan


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