From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by sourceware.org (Postfix) with ESMTPS id B58D93AAA0CA for ; Thu, 24 Jun 2021 12:28:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B58D93AAA0CA Received: by mail-pj1-x102e.google.com with SMTP id 13-20020a17090a08cdb029016eed209ca4so3386200pjn.1 for ; Thu, 24 Jun 2021 05:28:25 -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=IGYq6I5NJVtJ8dTO0S4gsALNajQUzsG1+aptLovX5f8=; b=GYKJEpKxKW6y1qhfFix8DIbMCDiY66cptDwv4kwx3RKjU8n71+tr1Gcq+2OIXyP+iI RJuHRqhOTAoJr7VmpY2pGnqhyACNFupTSUwX7Mgu7EmFOuWwCva+RuDROueJM8MNxUg4 QnVfQL7mGuhMCmb1tTbdlGSJ5LsrebnI2mPyKtTLs8TDe9enzA3+K9KRfI0vDiYL0veW kyYcpWaV5JwvvP/vnAdi9b6y1ZQJ7og+UqFBv7trUfWxopK3q8RmUlHCFUs59YJmTVpR UQ3disyUKS2nv/1MEnN0czQ08SMkZ4r7DHVuQfViTwYvb+SOQqf20JpusIciiBzPcr5P uUzg== X-Gm-Message-State: AOAM530gVwwJB+H1Yp7XAIrVvfYvdc8oqNrOfuiRgiWztoySNcpXdCAY 63AQMf9X2sdxSi2eDFQWoBkT6vhi8FaaGYu/5LY= X-Google-Smtp-Source: ABdhPJz23ucjwIznJ8A38d7c+/cDeHnscKKzyp1rXUTI+Ft6l4NFfpcSarx0vw67SrRfDrs3Wh5Sxvml6MAIxo3xrgQ= X-Received: by 2002:a17:90b:10e:: with SMTP id p14mr15167565pjz.153.1624537704552; Thu, 24 Jun 2021 05:28:24 -0700 (PDT) MIME-Version: 1.0 References: <20210623145419.3025540-1-adhemerval.zanella@linaro.org> <7b2557a7-9b86-7121-a366-5fee330f8364@linaro.org> <89312fbc-0fb1-8523-2c1c-c99025bb787e@linaro.org> <8a0679a6-28c8-fc64-113c-2b883f754756@linaro.org> In-Reply-To: <8a0679a6-28c8-fc64-113c-2b883f754756@linaro.org> From: "H.J. Lu" Date: Thu, 24 Jun 2021 05:27:48 -0700 Message-ID: Subject: Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled To: Adhemerval Zanella Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3032.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 24 Jun 2021 12:28:27 -0000 On Thu, Jun 24, 2021 at 5:17 AM Adhemerval Zanella wrote: > > > > On 23/06/2021 21:40, H.J. Lu wrote: > > On Wed, Jun 23, 2021 at 5:30 PM Adhemerval Zanella > > wrote: > >> > >> > >> > >> On 23/06/2021 18:36, H.J. Lu wrote: > >>> On Wed, Jun 23, 2021 at 2:15 PM Adhemerval Zanella > >>> wrote: > >>>> > >>>> > >>>> > >>>> On 23/06/2021 17:41, H.J. Lu wrote: > >>>>>> @@ -216,12 +233,15 @@ do_test (int argc, char **argv) > >>>>>> fails += CHECK_PROC (sgx, SGX); > >>>>>> fails += CHECK_PROC (sgx_lc, SGX_LC); > >>>>>> fails += CHECK_PROC (sha_ni, SHA); > >>>>>> - fails += CHECK_PROC (shstk, SHSTK); > >>>>>> + fails += CHECK_PROC_OPTIN (shstk, SHSTK); > >>>>> > >>>>> Why do you need this? If kernel doesn't support SHSTK, it will be > >>>>> turned off: > >>>>> > >>>>> /* Check CET status. */ > >>>>> unsigned int cet_status = get_cet_status (); > >>>>> > >>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0) > >>>>> CPU_FEATURE_UNSET (cpu_features, IBT) > >>>>> if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0) > >>>>> CPU_FEATURE_UNSET (cpu_features, SHSTK) > >>>> > >>>> The problem is this is only enabled for CET_ENABLED, the configuration I am using > >>>> does not define __CET__. So the CPU I am using does support SHSTK, but the bit > >>>> ended up not being cleared by glibc. > >>> > >>> IBT and SHSTK usable bits are copied from CPUID feature bits and later > >>> cleared if kernel doesn't support CET. Copy IBT and SHSTK usable only > >>> if CET is enabled so that they aren't set on CET capable processors > >>> with non-CET enabled glibc. > >>> > >>> Can you try this? > >> > >> It fixes the SHSTK issue, but it still shows the following failures on > >> the Ryzen 9 cpu: > >> > >> Checking HAS_CPU_FEATURE (IBRS_IBPB): > >> HAS_CPU_FEATURE (IBRS_IBPB): 0 > >> cpuinfo (ibrs): 1 > >> *** failure *** > >> > >> Checking HAS_CPU_FEATURE (SSBD): > >> HAS_CPU_FEATURE (SSBD): 0 > >> cpuinfo (ssbd): 1 > >> *** failure *** > >> > >> Checking HAS_CPU_FEATURE (STIBP): > >> HAS_CPU_FEATURE (STIBP): 0 > >> cpuinfo (stibp): 1 > >> *** failure *** > >> > >> Below I update my patch, your look ok thanks! > >> > >> --- > >> > >> [PATCH v2] x86: Fix tst-cpu-features-cpuinfo on Ryzen 9 (BZ #27873) > >> > >> AMD define different flags for IRPB, IBRS, and STIPBP [1], so new > >> x86_64_cpu are added and IBRS_IBPB is only tested for Intel. > >> > >> The SSDB is also defined and implemented different on AMD [2], > >> and also a new AMD_SSDB flag is added. It should map to the > >> cpuinfo 'ssdb' on recent AMD cpus. > >> > >> It fixes tst-cpu-features-cpuinfo and tst-cpu-features-cpuinfo-static > >> on recent AMD cpus. > >> > >> Checked on x86_64-linux-gnu on AMD Ryzen 9 5900X. > >> > >> [1] https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf > >> [2] https://bugzilla.kernel.org/show_bug.cgi?id=199889 > >> --- > >> sysdeps/x86/bits/platform/x86.h | 4 ++++ > >> sysdeps/x86/include/cpu-features.h | 12 ++++++++++++ > >> sysdeps/x86/tst-cpu-features-cpuinfo.c | 22 ++++++++++++++++++---- > >> 3 files changed, 34 insertions(+), 4 deletions(-) > >> > >> diff --git a/sysdeps/x86/bits/platform/x86.h b/sysdeps/x86/bits/platform/x86.h > >> index fe08d8a1b6..26e3b67ede 100644 > >> --- a/sysdeps/x86/bits/platform/x86.h > >> +++ b/sysdeps/x86/bits/platform/x86.h > >> @@ -278,6 +278,10 @@ enum > >> + cpuid_register_index_ebx * 8 * sizeof (unsigned int)), > >> > >> x86_cpu_WBNOINVD = x86_cpu_index_80000008_ebx + 9, > >> + x86_cpu_AMD_IBPB = x86_cpu_index_80000008_ebx + 12, > >> + x86_cpu_AMD_IBRS = x86_cpu_index_80000008_ebx + 14, > >> + x86_cpu_AMD_STIBP = x86_cpu_index_80000008_ebx + 15, > >> + x86_cpu_AMD_SSBD = x86_cpu_index_80000008_ebx + 24, > >> > >> x86_cpu_index_7_ecx_1_eax > >> = (CPUID_INDEX_7_ECX_1 * 8 * 4 * sizeof (unsigned int) > >> diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h > >> index d042a2ebef..4f1c4ee402 100644 > >> --- a/sysdeps/x86/include/cpu-features.h > >> +++ b/sysdeps/x86/include/cpu-features.h > >> @@ -289,6 +289,10 @@ enum > >> > >> /* EBX. */ > >> #define bit_cpu_WBNOINVD (1u << 9) > >> +#define bit_cpu_AMD_IBPB (1u << 12) > >> +#define bit_cpu_AMD_IBRS (1u << 14) > >> +#define bit_cpu_AMD_STIBP (1u << 15) > >> +#define bit_cpu_AMD_SSBD (1u << 24) > >> > >> /* CPUID_INDEX_7_ECX_1. */ > >> > >> @@ -519,6 +523,10 @@ enum > >> > >> /* EBX. */ > >> #define index_cpu_WBNOINVD CPUID_INDEX_80000008 > >> +#define index_cpu_AMD_IBPB CPUID_INDEX_80000008 > >> +#define index_cpu_AMD_IBRS CPUID_INDEX_80000008 > >> +#define index_cpu_AMD_STIBP CPUID_INDEX_80000008 > >> +#define index_cpu_AMD_SSBD CPUID_INDEX_80000008 > >> > >> /* CPUID_INDEX_7_ECX_1. */ > >> > >> @@ -749,6 +757,10 @@ enum > >> > >> /* EBX. */ > >> #define reg_WBNOINVD ebx > >> +#define reg_AMD_IBPB ebx > >> +#define reg_AMD_IBRS ebx > >> +#define reg_AMD_STIBP ebx > >> +#define reg_AMD_SSBD ebx > >> > >> /* CPUID_INDEX_7_ECX_1. */ > >> > >> diff --git a/sysdeps/x86/tst-cpu-features-cpuinfo.c b/sysdeps/x86/tst-cpu-features-cpuinfo.c > >> index 75e7eb9352..f457e8677b 100644 > >> --- a/sysdeps/x86/tst-cpu-features-cpuinfo.c > >> +++ b/sysdeps/x86/tst-cpu-features-cpuinfo.c > >> @@ -16,10 +16,11 @@ > >> License along with the GNU C Library; if not, see > >> . */ > >> > >> -#include > >> +#include > >> #include > >> #include > >> #include > >> +#include > >> > >> static char *cpu_flags; > >> > >> @@ -99,6 +100,7 @@ static int > >> do_test (int argc, char **argv) > >> { > >> int fails = 0; > >> + const struct cpu_features *cpu_features = __get_cpu_features (); > >> > >> get_cpuinfo (); > >> fails += CHECK_PROC (acpi, ACPI); > >> @@ -159,7 +161,17 @@ do_test (int argc, char **argv) > >> fails += CHECK_PROC (hle, HLE); > >> fails += CHECK_PROC (ht, HTT); > >> fails += CHECK_PROC (hybrid, HYBRID); > >> - fails += CHECK_PROC (ibrs, IBRS_IBPB); > >> + if (cpu_features->basic.kind == arch_kind_intel) > >> + { > >> + fails += CHECK_PROC (ibrs, IBRS_IBPB); > >> + fails += CHECK_PROC (stibp, STIBP); > >> + } > >> + else if (cpu_features->basic.kind == arch_kind_amd) > >> + { > >> + fails += CHECK_PROC (ibpb, AMD_IBPB); > >> + fails += CHECK_PROC (ibrs, AMD_IBRS); > >> + fails += CHECK_PROC (stibp, AMD_STIBP); > >> + } > >> fails += CHECK_PROC (ibt, IBT); > >> fails += CHECK_PROC (invariant_tsc, INVARIANT_TSC); > >> fails += CHECK_PROC (invpcid, INVPCID); > >> @@ -221,7 +233,10 @@ do_test (int argc, char **argv) > >> fails += CHECK_PROC (smep, SMEP); > >> fails += CHECK_PROC (smx, SMX); > >> fails += CHECK_PROC (ss, SS); > >> - fails += CHECK_PROC (ssbd, SSBD); > >> + if (cpu_features->basic.kind == arch_kind_intel) > >> + fails += CHECK_PROC (ssbd, SSBD); > >> + else if (cpu_features->basic.kind == arch_kind_amd) > >> + fails += CHECK_PROC (ssbd, AMD_SSBD); > >> fails += CHECK_PROC (sse, SSE); > >> fails += CHECK_PROC (sse2, SSE2); > >> fails += CHECK_PROC (pni, SSE3); > >> @@ -229,7 +244,6 @@ do_test (int argc, char **argv) > >> fails += CHECK_PROC (sse4_2, SSE4_2); > >> fails += CHECK_PROC (sse4a, SSE4A); > >> fails += CHECK_PROC (ssse3, SSSE3); > >> - fails += CHECK_PROC (stibp, STIBP); > >> fails += CHECK_PROC (svm, SVM); > >> #ifdef __x86_64__ > >> /* NB: SYSCALL_SYSRET is 64-bit only. */ > >> -- > >> 2.30.2 > > > > LGTM. > > > > Reviewed-by: H.J. Lu > > I will also add the updated documentation: > > diff --git a/manual/platform.texi b/manual/platform.texi > index a0b204b099..bf1483fe82 100644 > --- a/manual/platform.texi > +++ b/manual/platform.texi > @@ -659,6 +659,18 @@ XSETBV/XGETBV instructions, and XCR0. > @item > @code{XTPRUPDCTRL} -- xTPR Update Control. > > +@item > +@code{AMD_IBPB} -- Indirect branch predictor barrier (IBPB) for AMD cpus. > + > +@item > +@code{AMD_IBRS} -- Indirect branch restricted speculation (IBPB) for AMD cpus. > + > +@item > +@code{AMD_STIBP} -- Single thread indirect branch predictors (STIBP) for AMD cpus. > + > +@item > +@code{AMD_SSBD} -- Speculative Store Bypass Disable (SSBD) for AMD cpus. > + > @end itemize > > You could query if a processor supports @code{AVX} with: > Please sort new features by name. Thanks. -- H.J.