[PATCH] x86: include OSXSAVE in x86-64-v3 level

[Fabian Vogt]

Am Freitag, 9. Dezember 2022, 23:52:37 CET schrieb H.J. Lu:
> On Fri, Dec 9, 2022 at 1:09 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > On Fri, Dec 9, 2022 at 12:26 PM H.J. Lu via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > On Fri, Dec 9, 2022 at 2:36 AM Fabian Vogt <fvogt@suse.de> wrote:
> > > >
> > > > For some reason the initial x86-64-v3 detection code was missing checks for
> > > > BMI, BMI2 and OSXSAVE, which are all required for that level to be met.
> > > > BMI and BMI2 got added recently, but OSXSAVE was still missing. Add it.
> > > >
> > > > Signed-off-by: Fabian Vogt <fvogt@suse.de>
> > > > ---
> > > >  sysdeps/x86/get-isa-level.h | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/sysdeps/x86/get-isa-level.h b/sysdeps/x86/get-isa-level.h
> > > > index 5b4dd5f062..d62bf92cde 100644
> > > > --- a/sysdeps/x86/get-isa-level.h
> > > > +++ b/sysdeps/x86/get-isa-level.h
> > > > @@ -52,7 +52,8 @@ get_isa_level (const struct cpu_features *cpu_features)
> > > >               && CPU_FEATURE_USABLE_P (cpu_features, F16C)
> > > >               && CPU_FEATURE_USABLE_P (cpu_features, FMA)
> > > >               && CPU_FEATURE_USABLE_P (cpu_features, LZCNT)
> > > > -             && CPU_FEATURE_USABLE_P (cpu_features, MOVBE))
> > > > +             && CPU_FEATURE_USABLE_P (cpu_features, MOVBE)
> > > > +             && CPU_FEATURE_USABLE_P (cpu_features, OSXSAVE))
> > >
> > > If OSXSAVE isn't usable, all AVX/AVX512 features shouldn't be usable.
> > > Am I missing something?
> >
> > Seems like better to err on side of caution here.
> 
> We will have serious issues in many places if we don't get it correctly.
> CPU_FEATURE_USABLE_P is set according to the states supported
> by OSXSAVE.  Checking OSXSAVE isn't needed here.

I see, you explicitly unset some flags if OSXSAVE is not available.
In that case the patch is indeed a noop, but also won't hurt. What about
adding a comment instead?

> > Skipping the extra check seems prone to bugs like BZ #29611.