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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

> --- a/xen/include/asm-x86/cpuid.h
> +++ b/xen/include/asm-x86/cpuid.h
> @@ -103,7 +103,25 @@ struct cpuid_policy
>  
>              /* Leaf 0x1 - family/model/stepping and features. */
>              struct {
> -                uint32_t :32, :32, _1c, _1d;
> +                uint32_t :32, :32;
> +                union {
> +                    uint32_t _1c;
> +                    struct {
> +                        bool sse3:1, pclmulqdq:1, dtes64:1, monitor:1, 
> dscpl:1, vmx:1, smx:1, eist:1,
> +                             tm2:1, ssse3:1, /* cid */:1, /* sdbg */:1, 
> fma:1, cx16:1, xtpr:1, pcdm:1,
> +                             :1, pcid:1, dca:1, sse4_1:1, sse4_2:1, 
> x2apic:1, movebe:1, popcnt:1,

movbe

> +                             tsc_deadline:1, aesni:1, xsave:1, /* osxsave 
> */:1, avx:1, f16c:1, rdrand:1, hv:1;

hypervisor (please make sure the name match up with X86_FEATURE_*).

> @@ -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.

> +                    };
> +                };
> +                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 ...

> @@ -126,7 +171,16 @@ struct cpuid_policy
>                  uint32_t xcr0_low, :32, :32, xcr0_high;
>              };
>              struct {
> -                uint32_t Da1, :32, xss_low, xss_high;
> +                union {
> +                    uint32_t Da1;
> +                    struct {
> +                        bool xsaveopt: 1, xsavec: 1, xgetbv1: 1, xsaves: 1, 
> :1, :1, :1, :1,

Why the blanks after the colons?

Jan


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