Re: [Xen-devel] [V3] x86/cpuid: AVX-512 Feature Detection

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.06.16 at 13:27, <luwei.kang@xxxxxxxxx> wrote:
> AVX-512 is an extention of AVX2. Its spec can be found at:
> https://software.intel.com/sites/default/files/managed/b4/3a/319433-024.pdf 
> This patch detects AVX-512 features by CPUID.
> 
> Signed-off-by: Luwei Kang <luwei.kang@xxxxxxxxx>
> ---
> [V3]
> 1.adjust dependencies between features.
> [V2]
> 1.one per bit, change from
>     > + xstate_size = max(xstate_size,
>       > +                 xstate_offsets[_XSTATE_HI_ZMM] +
>       > +                 xstate_sizes[_XSTATE_HI_ZMM]);
>   to
>     xstate_size = max(xstate_size,
>                         xstate_offsets[_XSTATE_OPMASK] +
>                       xstate_sizes[_XSTATE_OPMASK]);
>     xstate_size = max(xstate_size,
>                       xstate_offsets[_XSTATE_ZMM] +
>                       xstate_sizes[_XSTATE_ZMM]);
>     xstate_size = max(xstate_size,
>                       xstate_offsets[_XSTATE_HI_ZMM] +
>                       xstate_sizes[_XSTATE_HI_ZMM]);
> 2.change form
>     domain_cpuid(currd, 0x07, 0, &tmp, &_ebx, &tmp, &tmp);
>   to
>     domain_cpuid(currd, 7, 0, &tmp, &_ebx, &tmp, &tmp);
> 3.add dependencies between features in xen/tools/gen-cpuid.py
> 4.split the cpuid call just like the way the hvm_cpuid() side works.

Especially item 1 is clearly to verbose; note how I said "brief" when
I asked for the revision log.

> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -243,6 +243,17 @@ def crunch_numbers(state):
>          # AMD K6-2+ and K6-III processors shipped with 3DNow+, beyond the
>          # standard 3DNow in the earlier K6 processors.
>          _3DNOW: [_3DNOWEXT],
> +
> +        # AVX2 is an extension to AVX, providing mainly new integer 
> instructions.
> +        # In principle, AVX512 only depends on YMM register state, but many 
> AVX2
> +        # instructions are extended by AVX512F to 512-bit forms.

I realize you used the wording as suggested by Andrew, and while
his reply to my question about it meanwhile clarified what is meant,
I continue to think that the mentioning of YMM registers above is
misleading. May I suggest something like "AVX512 only takes YMM
register state as a prerequisite", subject to further improvement by
Andrew or another native speaker?

Apart from that the patch looks fine to me now.

Jan


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