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

[H.J. Lu]

On Thu, Aug 30, 2018 at 12:47 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/29/2018 05:57 PM, H.J. Lu wrote:
>
>>> 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?
>
>
> I expect the effect to be similar to this bug:
>
>   <https://bugzilla.redhat.com/show_bug.cgi?id=1377895>
>
> And this one, which has more analysis:
>
>   <https://sourceware.org/bugzilla/show_bug.cgi?id=20019>
>
> This shows that the IFUNC resolvers inside libc itself (which use the
> proposed mechanism) are unsafe.

This particular issue involves a shared library which references libc.so,
but is not linked against libc.so.6.

x86_get_arch_feature,  x86_get_cpu_kind and x86_get_cpuid_registers
are different since they are provided by ld.so.  They don't have any dependency
issues.  I added 2 new testcases to verify that they can be used in shared
libraries with or without lazy binding.

>>> 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)
>
>
> There should only be a single feature to query, to avoid confusion.

Good point.  I added more XXX_Usable definitions and updated
document.

>>> 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?
>
>
> glibc on Red Hat Enterprise Linux 6 before glibc-2.12-1.207.el6 did not have
> correct FMA3 detection.  It did not consider XOSSAVE support.  As a result,
> the fma function would crash with SIGILL when running on a FMA3-capable CPU
> on a Red Hat Enterprise Linux 5 kernel.
>
> The key notes from the private bug (#1384281) are:
>
> “
> The crash happens at an FMA3 instruction.
>
> Program received signal SIGILL, Illegal instruction.
> __fma_fma (x=1, y=2, z=3) at ../sysdeps/x86_64/multiarch/s_fma.c:33
> 33        asm ("vfmadd213sd %3, %2, %0" : "=x" (x) : "0" (x), "x" (y), "xm"
> (z));
>
> #0  __fma_fma (x=1, y=2, z=3) at ../sysdeps/x86_64/multiarch/s_fma.c:33
> #1  0x000000000041993c in fma_test ()
>     at
> /builddir/build/BUILD/glibc-2.12-2-gc4ccff1/build-x86_64-linuxnptl/math/libm-test.c:3125
> #2  0x000000000043ba4a in main (argc=1, argv=<value optimized out>)
>     at
> /builddir/build/BUILD/glibc-2.12-2-gc4ccff1/build-x86_64-linuxnptl/math/libm-test.c:7198
>
> The IFUNC selector is gated on:
>
> # define HAS_FMA        HAS_CPU_FEATURE (COMMON_CPUID_INDEX_1, ecx, 12)
>
> This is a glibc bug because it does not probe operating system support for
> AVX (using XGETBV), only FMA CPU support (using CPUID).  The Intel
> guidelines are extremely clear that you need to check both.
>
> It probably was fixed upstream in this commit:
>
> commit afc5ed09cbce5d6fd48b3a8c5ec427b31f996880
> Author: Ulrich Drepper <drepper@gmail.com>
> Date:   Thu Jan 26 07:45:14 2012 -0500
>
>     Reset bit_AVX in __cpu_features is OS support is missing
>
> More fixes may be needed (possibly
> 08cf777f9e7f6d826658a99c7d77a359f73a45bf).  I have not yet checked if commit
> afc5ed09cbce5d6fd48b3a8c5ec427b31f996880 is sufficient to make the HAS_FMA
> check fail (so that the SSE2 implementation is selected, as required by the
> RHEL 5 kernel due to its lack of AVX support).
>
> Current upstream code is very different in this area.
> ”
>
> Carlos and myself spend quite some time chasing this down, and I think with
> a proper interface design, we can spare other programmers that.
>

That is what HAS_ARCH_FEATURE is for.

Here is the updated patch.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Add-sys-platform-x86.h.patch
Type: text/x-patch
Size: 76522 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20180830/ed2fcaa7/attachment.bin>