From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id 6CAAA385C40F for ; Sat, 10 Jul 2021 17:21:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6CAAA385C40F Received: by mail-pf1-x436.google.com with SMTP id a127so11851145pfa.10 for ; Sat, 10 Jul 2021 10:21:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=v2hNSY6xohQ3V4VViJpC+cYsX53U1hgzHoMGPgeHgzc=; b=Uh4KOlfQwhFX4hIYez9yNVN+mX7Vhw+Wu8/P4HW/k9qRVyJiDKT6s0XUwZPgPmPGvT AnIIhGChBC3ltUJjThcLtV8eZYj/veBG3CFRts0Vu8NAj1nVVzzmga8R1MBqSARtHTIE SNcH0n0YE166/w2XJdsm00e8S7xmEt/kOnj4obiaGoWsWtmQWPygBNnYkHnlmOZNl/X7 y2MoLkfYUixTg/EmZlkFaw3dHb9ugzBhChfrtMyhfTfpHqslyO+zx4Hw0NDoKfi2sifC hDdyHQmfrOXCqqWxnJZyY0zBxsRkuHRyQyj7P3iqmgj+CjyQKBjdc6Ede4Hi79ksQNwg EgKA== X-Gm-Message-State: AOAM533dfLKSujNFu+5A9KOpLAyHVD8kzuQOfLlqByLZrRKtdOtPwOd1 3bL566VldU5KH2TEFRMTTqYH31uYRfiTWaG3AR4= X-Google-Smtp-Source: ABdhPJwf8njoARm0vyh1XxeTFt5vasLTfeOo5/Qv71jQiMWz2GEmcUrQzNUc4vSz57A0mn1LRRfCywlK4mv3ZtNi9v0= X-Received: by 2002:a63:5f11:: with SMTP id t17mr15628198pgb.37.1625937717659; Sat, 10 Jul 2021 10:21:57 -0700 (PDT) MIME-Version: 1.0 References: <20210605135947.469959-1-hjl.tools@gmail.com> <83077104-ea01-0afc-5636-87e1039d463a@redhat.com> In-Reply-To: <83077104-ea01-0afc-5636-87e1039d463a@redhat.com> From: "H.J. Lu" Date: Sat, 10 Jul 2021 10:21:21 -0700 Message-ID: Subject: Re: [PATCH] x86: Install [BZ #27958] To: "Carlos O'Donell" Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3031.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 10 Jul 2021 17:21:59 -0000 On Fri, Jul 9, 2021 at 1:01 PM Carlos O'Donell wrote: > > On 6/5/21 9:59 AM, H.J. Lu via Libc-alpha wrote: > > Install for which includes > > . > > > > Fixes BZ #27958. > > The constants in bits/platform/x86.h are largely ABI given the behaviour > of the cpuid instruction. Likewise we do a consistent mapping between > the cpuid_array <-> usable_array without exposing internal details. > > The API in sys/platform/x86.h has already been reviewed, discussed, and > exposes HAS_CPU_FEATURE(name) and CPU_FEATURE_USABLE(name). > > Given that we get one more chance at review let me ask a few final questions. > > (1) API prefixes in macros help developers remember names. > > Consistent prefix for APIs help developers remember. > > We use HAS_* but also CPU_* which requires the programmer remember > two distinct naming strategies. > > Suggestion: CPU_FEATURE_PRESENT(), CPU_FEATURE_USABLE()? I will rename them to CPU_FEATURE_PRESENT and CPU_FEATURE_POSSIBLE. Given the current AMX support discussion, additional information may be needed from the Linux kernel to determine if a feature can be used. > Note: We do this in the underlying name e.g. x86_cpu_* > has_feature (could be is_present) vs. is_usable. I will change them to x86_cpu_present and x86_cpu_possible. I will also rename the "usable" field to "possible". > (2) ABI testing? > > - How are we making sure we don't accidentally break ABI? > > - Do we need any further testing? We only export struct cpuid_feature { unsigned int cpuid_array[4]; unsigned int possible_array[4]; }; extern const struct cpuid_feature *__x86_get_cpuid_feature_leaf (unsigned int) __attribute__ ((pure)); const struct cpuid_feature * __x86_get_cpuid_feature_leaf (unsigned int leaf) { static const struct cpuid_feature feature = {}; if (leaf < CPUID_INDEX_MAX) return ((const struct cpuid_feature *) &GLRO(dl_x86_cpu_features).features[leaf]); else return &feature; } As long as all new features are appended to the end, there should be no ABI issues. > - Do we have a decoupled test to ensure a refactor doesn't break > things? This shouldn't be a problem. > - We have tst-cpu-features-cpuinfo.c, which should cover > comparison to the decoupled cpuinfo. > > Notes: > > - We will not be able to avoid in-place-update failures, in that rpm > will do an atomic rename that unlinks the old libc.so.6 with the > new libc.so.6 and if ld.so is not yet updated or updated first then > a process that starts will crash. This makes it error prone to update > the ABI in downstream minor updates. > > > --- > > sysdeps/x86/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > > index 346ec491b3..567ea54243 100644 > > --- a/sysdeps/x86/Makefile > > +++ b/sysdeps/x86/Makefile > > @@ -5,7 +5,7 @@ endif > > ifeq ($(subdir),elf) > > sysdep_routines += get-cpuid-feature-leaf > > sysdep-dl-routines += dl-get-cpu-features > > -sysdep_headers += sys/platform/x86.h > > +sysdep_headers += sys/platform/x86.h bits/platform/x86.h > > > > CFLAGS-get-cpuid-feature-leaf.o += $(no-stack-protector) > > > > -- > Cheers, > Carlos. > Thanks. -- H.J.