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

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Aug 29, 2018 at 8:52 AM, Florian Weimer <fweimer@redhat.com> wrote:
> 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.

Do you have a testcase to show that this claim is true?

>>> 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.

That is why I added:

@defmac HAS_ARCH_FEATURE(name)

Evaluate to true if the architecture feature @code{name} is supported.
The available features are:

@itemize @bullet

@item
@code{AVX_Usable} -- AVX is usable.

@item
@code{AVX2_Usabl} --  AVX2 is usable.

@item
@code{AVX512F_Usable} -- AVX512F is usable.

@item
@code{AVX512DQ_Usable} -- AVX512DQ is usable.

@item
@code{FMA_Usable} -- FMA is usable.

@item
@code{FMA4_Usable} -- FMA4 is usable.

@item
@code{XSAVEC_Usable} -- XSAVEC is usable.

@end itemize

@end defmac

>>> 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.

Do you have a testcase to show that this claim is true?

-- 
H.J.