From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf33.google.com (mail-qv1-xf33.google.com [IPv6:2607:f8b0:4864:20::f33]) by sourceware.org (Postfix) with ESMTPS id 15560385DC33 for ; Thu, 24 Jun 2021 12:55:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 15560385DC33 Received: by mail-qv1-xf33.google.com with SMTP id c2so3210199qvs.6 for ; Thu, 24 Jun 2021 05:55:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Mjc/Br9zR9VbavyCZYwEnwH+5RyRqCi3OEQlgG6v+Qs=; b=OxtrBD3NSNDa1IR7uedu1IML5KizRmyYQBdvB38pa7EovVPROE0RU59HjSJB4Bag2+ YvF74Be3fNi7sTgqqfDrNv9MJtFTrb3rMFvPXd1Fze+FngwTGLpsSGQMvKYXz9ki9BLg CyIwMwBc2TzNJbUnRu/cTNOsOIxtDGcoZ2SfcAiDK41BoD5EyZEPEB90Mgugc182iZQh hi685Z0YiD7Og4MPMFJ9ApyE75FxvdGtmWTPfj1kiT16Wjl/gAsPV43Lf1RhoLUW0yyq WWkhYfhrqOxlXe0k55cmiNTKPxgAO80/JdgqRao44qaFDs0sDs9SP0YG83mLT7tff0Vm dLmw== X-Gm-Message-State: AOAM530gXp1CrxAIvu1yb6ZwFn2zOASVdysVirXolnhuPijLHJtnA2w4 V0MOMF8A29tRaE8Exvytp7qstyS42iD2GA== X-Google-Smtp-Source: ABdhPJytxcCz8cjmMCsPoVW+jzNnw96LeNH+ZZiPhsfgEW4rYZyqkZ5BWgQ4ihNoI3uI4AWqzdnbrA== X-Received: by 2002:a05:6214:14aa:: with SMTP id bo10mr5266084qvb.24.1624539343474; Thu, 24 Jun 2021 05:55:43 -0700 (PDT) Received: from [192.168.1.108] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id a77sm393300qkc.119.2021.06.24.05.55.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 24 Jun 2021 05:55:43 -0700 (PDT) Subject: Re: [PATCH] x86: Copy IBT and SHSTK usable only if CET is enabled To: "H.J. Lu" Cc: GNU C Library 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> From: Adhemerval Zanella Message-ID: <98d623e5-f205-c520-e5fa-237e99ca1072@linaro.org> Date: Thu, 24 Jun 2021 09:55:40 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, 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:55:46 -0000 On 24/06/2021 09:27, H.J. Lu wrote: > 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. Ack.