Re: [Xen-devel] [PATCH v3 10/28] xen/x86: Generate deep dependencies of features

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.16 at 16:35, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -18,12 +19,34 @@ uint32_t __read_mostly hvm_featureset[FSCAPINTS];
>  
>  static void __init sanitise_featureset(uint32_t *fs)
>  {
> +    uint32_t disabled_features[FSCAPINTS];
>      unsigned int i;
>  
>      for ( i = 0; i < FSCAPINTS; ++i )
>      {
>          /* Clamp to known mask. */
>          fs[i] &= known_features[i];
> +
> +        /*
> +         * Identify which features with deep dependencies have been
> +         * disabled.
> +         */
> +        disabled_features[i] = ~fs[i] & deep_features[i];
> +    }
> +
> +    for_each_set_bit(i, (void *)disabled_features,

I'm afraid the cast here is not really valid: If FSCAPINTS is odd,
there would be an (admittedly benign) out of bounds access as
a result. For fully defined behavior I think you need to kind of
open code for_each_set_bit() here (and, if there are any, in
similar constructs).

> +    deps = {
> +        # FPU is taken to mean support for the x87 regisers as well as the
> +        # instructions.  MMX is documented to alias the %MM registers over 
> the
> +        # x87 %ST registers in hardware.
> +        FPU: [MMX],
> +
> +        # The PSE36 feature indicates that reserved bits in a PSE superpage
> +        # may be used as extra physical address bits.
> +        PSE: [PSE36],
> +
> +        # Entering Long Mode requires that %CR4.PAE is set.  The NX pagetable
> +        # bit is only representable in the 64bit PTE format offered by PAE.
> +        PAE: [LM, NX],

Also PKU?

> +        # APIC is special, but X2APIC does depend on APIC being available in
> +        # the first place.
> +        APIC: [X2APIC],
> +
> +        # AMD built MMXExtentions and 3DNow as extentions to MMX.
> +        MMX: [MMXEXT, _3DNOW],
> +
> +        # The FXSAVE/FXRSTOR instructions were introduced into hardware 
> before
> +        # SSE, which is why they behave differently based on %CR4.OSFXSAVE 
> and
> +        # have their own feature bit.  AMD however introduce the Fast FXSR
> +        # feature as an optimisation.
> +        FXSR: [FFXSR],

Also SSE.

> +        # SSE is taken to mean support for the %XMM registers as well as the
> +        # instructions.  The SSE extentions were re-specified as core for
> +        # 64bit support.
> +        SSE: [SSE2, LM],

I think listing LM here is pointless when it's also listed with SSE2.

> +        # SSE2 was also re-specified as core for 64bit.  The AESNI and SHA
> +        # instruction groups are documented to require checking for SSE2
> +        # support as a prerequisite.
> +        SSE2: [SSE3, LM, AESNI, SHA],
> +
> +        # AMD K10 processors has SSE3 and SSE4A.  Bobcat/Barcelona processors
> +        # subsequently included SSSE3, and Bulldozer subsequently included
> +        # SSE4_1.  Intel have never shipped SSE4A.
> +        SSE3: [SSSE3, SSE4_1, SSE4A],
> +        SSE4_1: [SSE4_2],

Numerically these dependencies make sense, but do they really build
on top of one another? The last ones (SSE4.1 and SSE4.2) are
particularly examples of things that look more like siblings than
ancestors, as do AESNI and SHA wrt SSE2. Otherwise BMI2 would
likely need to be considered dependent on BMI1...

> +        # XSAVE is an extra set of instructions for state management, but
> +        # doesn't constitue new state itself.  Some of the dependent features
> +        # are instructions built on top of base XSAVE, while others are new
> +        # instruction groups which are specified to require XSAVE for state
> +        # management.
> +        XSAVE: [XSAVEOPT, XSAVEC, XGETBV1, XSAVES, AVX, MPX],

Also PKU again? And LWP?

Jan


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