V3: [PATCH] x86: Install <sys/platform/x86.h> [BZ #26124]

H.J. Lu hjl.tools@gmail.com
Mon Jun 22 20:25:57 GMT 2020


On Mon, Jun 22, 2020 at 2:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > On Thu, Jun 18, 2020 at 1:45 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu via Libc-alpha:
> >>
> >> > <sys/platform/x86.h> exports only:
> >> >
> >> > struct cpu_features
> >> > {
> >> >   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
> >> >   unsigned int feature[FEATURE_INDEX_MAX];
> >> >   struct cpu_features_basic basic;
> >> > };
> >>
> >> The struct cpu_features ABI does not appear to be stable, as I wrote on
> >> the other thread:
> >>
> >>   <https://sourceware.org/pipermail/libc-alpha/2020-June/115161.html>
> >>
> >
> > Here is the updated patch to use
> >
> > struct cpu_features
> > {
> >   struct cpu_features_basic basic;
> >   unsigned int usable[USABLE_FEATURE_INDEX_MAX];
> >   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
> > };
> >
> > This should be backward compatible for both .o and .so files.
>
> This is an improvement.  I still see issues with this interface, though.
>
> Programmers will need something like this for IFUNC resolvers, but this
> is not usable there if the relocation order does not match the explicit
> ELF dependency order (e.g., due to symbol interposition).  This is a
> generic problem with IFUNC resolvers.  Other architectures solve this by
> adding arguments to IFUNC resolvers, so that the required can be

This issue is orthogonal to <sys/platform/x86.h>.

> accessed without relocations.  __builtin_cpu_supports does not have this
> issue.
>
> The manual does not explain the difference between “available” and
> “usable”.  I do not think we should expose both.  I don't see any
> circumstances under which “available” would provide useful information.

Fixed.  "available" means supported by the operating system, which is
very useful information.

> struct cpu_features (even in its reduced form) is fairly large.  We will
> never be able to reduce its size again if it becomes public ABI.

Fixed by

struct cpu_features
{
  struct cpu_features_basic basic;
  unsigned int *usable_p;
  struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
};


> I'm not convinced that this interface conforms to the API contract for
> the CPUID instruction.  Aren't the raw bits in the cpuid field subject
> to interpretation based on family and model?  HAS_CPU_FEATURE does not
> reflect this at all.

Yes, it does.  The cpuid array is populated based on family and model.
It has accurate information about processor features.

> My preferred interface would look like this:
>
> * The low-level function call ABI is:
>
>   unsigned long long long int __x86_get_cpu_features (unsigned int word)
>     __attribute__ ((const));
>
> * IFUNC resolvers receive two arguments,
>
>   unsigned long long long int *feature_words, size_t feature_words_length
>
> * CPU_FEATURE_USABLE (NAME) would expand to something like
>
>     ((__x86_get_cpu_features (NAME_word) & NAME_bitmask) != 0)
>
>   The implementation of __x86_get_cpu_features simply returns 0 if the
>   index is too large, which solves the extension problem.
>
> * CPU_FEATURE_USABLE_ARRAY (WORDS, WORDS_LENGTH, NAME) would expand to this:
>
>   (((NAME_word < WORDS_LENGTH ? WORDS[NAME_word] ? : 0) & NAME_bitmask) != 0)
>
>   This version is expected to be used in IFUNC resolvers.  The macro
>   name is just a strawman.
>
> Only the “usable” bits are exposed, so the interface makes it clear that
> the implementation performs some normalization.

My new interface is expandable and backward compatible.  It is also
forward compatible
since the older libc.so works with binaries compiled against the newer
<sys/platform/x86.h>
as long as the cpuid and usable array sizes are the same.

> Alternatively, the __builtin_cpu_supports interface could be enhanced to
> cover all the features you need.

__builtin_cpu_supports is equivalent to CPU_FEATURE_USABLE and it
doesn't support HAS_CPU_FEATURE which does provide useful information.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Install-sys-platform-x86.h-BZ-26124.patch
Type: text/x-patch
Size: 26541 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20200622/c1df37ba/attachment-0001.bin>


More information about the Libc-alpha mailing list