Re: V2 [PATCH] x86: Add <sys/platform/x86.h>

[Florian Weimer <fweimer at redhat dot com>]

On 08/29/2018 05:48 PM, H.J. Lu wrote:
> On Wed, Aug 29, 2018 at 8:41 AM, Florian Weimer <fweimer@redhat.com> wrote:
> > On 08/29/2018 04:38 PM, H.J. Lu wrote:
> > > On Mon, Aug 20, 2018 at 1:54 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Mon, Aug 6, 2018 at 2:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > Add <sys/platform/x86.h> to provide an API to access x86 specific
> > > > > platform
> > > > > features:
> > > >
> > > > Here is the updated patch to rename x86  specific functions to x86_XXX.
> > >
> > > If there are no comments nor objections:
> > >
> > > https://sourceware.org/ml/libc-alpha/2018-08/msg00435.html
> > >
> > > I will check it in this Friday,
> >
> >
> > I'm still concerned that the intended use of this facility is incompatible
> > with BIND_NOW and other things that disable lazy binding.
>
> The intended usage is similar to using CPUID instruction to detect CPU features.

CPUID and XGETBV do not need relocations, so they will work where this 
approach will not.

> > The manual also needs to explain how to use these facilities for feature
> > detection.  I assume that just looking at (for example) HAS_CPU_FEATURE
> > (AVX512F) will give wrong results.  If this is true, this seriously
>
> Why should it give the wrong results?  Can you elaborate it?  Were you saying
> that the result was different from using CPUID instruction directly?

You also need to check for XSAVE support, I think.  This is easy to 
miss, and the proposed interface does not prevent this.

> > questions why we should expose these tests in a public header file.
>
> <x86.h> provides a C interface to raw CPUID instruction.

I think it's not at the right level of abstraction.  Features should 
only be marked as available if you can actually execute the CPU 
instructions, and they they will not fault due to missing CPU, kernel, 
or hypervisor support.

Thanks,
Florian