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

["Kang, Luwei" <luwei.kang@xxxxxxxxx>]

> > >>>>>>>>>> First of all - please don't top post.
> > >>>>>>>>>>
> > >>>>>>>>>>> What about remove the dependency between AVX2 and AVX512F
> > >>>>>>>> ( AVX2:
> > >>>>>>>>>> [AVX512F], ) ?
> > >>>>>>>>>>
> > >>>>>>>>>> Yes, that's what I think we want, but we need Andrew's agreement 
> > >>>>>>>>>> here.
> > >>>>>>>>>>
> > >>>>>>>>> Hi Andrew,  what is your opinion ?
> > >>>>>>>> You are in a better position to answer than me.
> > >>>>>>>>
> > >>>>>>>> For a specific instruction which may be VEX and EVEX encoded,
> > >>>>>>>> is the circuitry for a specific instruction shared, or
> > >>>>>>>> discrete?  Is there a plausible future where you might
> > >>>>>>>> support only the EVEX variant of an instruction, and not the VEX 
> > >>>>>>>> variant?
> > >>>>>>>>
> > >>>>>>>> These dependences are about what may be reasonably assumed
> > >>>>>>>> about the way the environment is structured.  It doesn't look
> > >>>>>>>> reasonable to advertise an AVX512 environment to guests while
> > >>>>>>>> at the same time stating that AVX2 is not present.  If this
> > >>>>>>>> is correct, then the dependency
> > >>>>> should stay.
> > >>>>>>>> If Intel plausibly things it might release hardware with
> > >>>>>>>> !AVX2 but AVX512, then the dependency should be dropped.
> > >>>>>>> Regarding the dependency between AVX2 and AVX512F, I have ask
> > >>>>>>> some hardware
> > >>>>> architecture engineer.
> > >>>>>>> AVX512 is a superset of AVX2, the most important item being
> > >>>>>>> the state. If
> > >>>>> the state for AVX2 isn't enabled (in XCR0), then AVX512
> > >>>>>> also can't function.
> > >>>>>>> So if we want to use AVX512, AVX2 must be supported and enabled.
> > >>>>>>> The
> > >>>>> dependence between AVX512F and AVX2 may need be
> > >>>>>> reserved.
> > >>>>>>
> > >>>>>> Ok, so AVX512 strictly depends on AVX2.
> > >>>>>>
> > >>>>>> In which case, the python code was correct.  The meaning of the
> > >>>>>> key/value
> > >>>>> relationship is "if the feature in the key is not present, all
> > >>>>>> features in the value must also be disabled".
> > >>>>> Hi jan, what is your opinion?
> > >>>> There's no opinion to be had here, according to your earlier reply.
> > >>>> I do,
> > >>> however, continue to question that model: State and
> > >>>> instruction set are independent items. Of course YMM state is a
> > >>>> prereq to
> > >>> ZMM state, but I do not buy AVX2 insn support being a
> > >>>> prereq to AVX-512 insns. Yet to me the AVX2 and AVX-512F feature
> > >>>> flags solely
> > >>> represent the instructions, while the XSTATE leaf bits
> > >>>> represent the states.
> > >>>>
> > >>> Hi jan,
> > >>>     I get some information from hardware colleague.  There is no
> > >>> dependence about AVX512 instructions and AVX2 instructions. It is
> > >>> also not states in
> > > the
> > >>> official document.
> > >> As I had assumed.
> > >>
> > >>>    But I want to know the meaning of the dependence "AVX2:
> > >>> [AVX512F]"  in "gen-cpuid.py" file.
> > >>>    If it is the feature dependence, I think the dependence between
> > >>> AVX512 and AVX2  may need to add. As we talk before, Zmm is part of 
> > >>> AVX512 feature.
> > >
> > >>> If the state for AVX2 isn't enabled (in XCR0), then AVX512 also
> > >>> can't function. Apart from that, there is no processors with
> > >>> AVX512 without AVX2 currently and it is safe to treat AVX2 as part
> > >>> of the thermometer leading to
> > >
> > >>> AVX512. Regarding if will have a cpu support avx512 without avx2
> > >>> in future, it is really hard to say.
> > >>>     If it is the instructions dependence, I have no idea to delete
> > >>> this dependence in "gen-cpuid.py" file.
> > >>>     So, I want to know your advice.
> > >> Well, the main issue here is that we have no feature flag
> > >> representation for the xstate bits. If we had, the relevant parts
> > >> of the dependencies should look like
> > >>
> > >>  XSTATE_YMM: [AVX, XSTATE_ZMM]
> > >>  AVX: [AVX2]
> > >>  XSTATE_ZMM: [AVX512F]
> > >>  AVX512F: [AVX512CD, ...]
> > >>
> > >> But in their absence I guess keeping the AVX2 dependency as you
> > >> have it is the only reasonable approach. Just that I'd like it to
> > >> be accompanied by a comment explaining that this isn't the actual
> > >> dependency (so that if XSTATE feature flags got introduced later,
> > >> it would be clear how to adjust those parts).
> > >>
> > >> Andrew - I keep forgetting why you think having such XSTATE_*
> > >> feature flags is not appropriate.
> > >
> > > This is all about providing a plausible cpuid layout to a guest.
> > >
> > > It is not plausible that we will ever see a processor with e.g.
> > > AVX512F but not XSTATE_ZMM.
> >
> > This is a somewhat bogus argument considering we have
> >
> >         FXSR: [FFXSR, SSE],
> >
> > which, as you certainly realize, is slightly wrong nowadays:
> > XSTATE_XMM would suffice as a prereq, without any FXSR. (But I
> > certainly realize that lack of FXSR is unlikely, as that's considered
> > part of the base x86-64 architecture, and I also realize that
> > expressing alternative prereqs would make the deep dependency
> > generation logic more complicated.)
> >
> 
> According your advice, I change the comment of "AVX2: [AVX512F]," as below.
> If the description is not accurately enough, please help do some corrected, 
> thank you.
> 
> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py index 
> 7c45eca..e8b64ce 100755
> --- 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],
> +
> +        # This is just the dependency between AVX512 and AVX2 of XSTATE 
> feature flag.
> +        # If want to use AVX512, AVX2 must be supported and enabled.
> +        AVX2: [AVX512F],
> +
> +        # AVX512F is taken to mean hardware support for EVEX encoded 
> instructions,
> +        # 512bit registers, and the instructions themselves. All further 
> AVX512 features
> +        # are built on top of AVX512F
> +        AVX512F: [AVX512DQ, AVX512IFMA, AVX512PF, AVX512ER, AVX512CD,
> +                    AVX512BW, AVX512VL, AVX512VBMI],
>      }
> 
>      deep_features = tuple(sorted(deps.keys()))
> 

Hi jan,
    What do you think about modify as above.  If need any corrected?

Thanks,
Luwei Kang


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