lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 11 Feb 2020 14:42:05 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Kim Phillips <kim.phillips@....com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Babu Moger <babu.moger@....com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Frank van der Linden <fllinden@...zon.com>,
        "H . Peter Anvin" <hpa@...or.com>, Huang Rui <ray.huang@....com>,
        Janakarajan Natarajan <Janakarajan.Natarajan@....com>,
        Jan Beulich <jbeulich@...e.com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Luwei Kang <luwei.kang@...el.com>,
        Martin Liška <mliska@...e.cz>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        Michael Petlan <mpetlan@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
        Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tom Lendacky <thomas.lendacky@....com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 2/2 v2 RESEND] x86/cpu/amd: Enable the fixed intructions
 retired free counter IRPERF

On Fri, Feb 07, 2020 at 05:04:27PM -0600, Kim Phillips wrote:
> commit aaf248848db50 ("perf/x86/msr: Add AMD IRPERF (Instructions
> Retired) performance counter") added support for 'perf -e msr/irperf/',
> but when exercised, we always get a 0 count:
> 
> BEFORE:
> 
> $ sudo perf stat -e instructions,msr/irperf/ true
> 
>  Performance counter stats for 'true':
> 
>            624,833      instructions
>                                                   #    0.00  stalled cycles per insn
>                  0      msr/irperf/
> 
> It turns out it simply needs its enable bit - HWCR bit 30 - set.  This patch

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

> does just that.
> 
> Enablement is restricted to all machines advertising IRPERF capability,
> except those susceptible to an erratum that makes the IRPERF return
> bad values.
> 
> That erratum occurs in Family 17h models 00-1fh [1], but not in F17h
> models 20h and above [2].
> 
> AFTER (on a family 17h model 31h machine):
> 
> $ sudo perf stat -e instructions,msr/irperf/ true
> 
>  Performance counter stats for 'true':
> 
>            621,690      instructions
>                                                   #    0.00  stalled cycles per insn
>            622,490      msr/irperf/
> 
> [1] "Revision Guide for AMD Family 17h Models 00h-0Fh Processors",
>     currently available here:
> 
>     https://www.amd.com/system/files/TechDocs/55449_Fam_17h_M_00h-0Fh_Rev_Guide.pdf
> 
> [2] "Revision Guide for AMD Family 17h Models 30h-3Fh Processors",
>     currently available here:
> 
>     https://developer.amd.com/wp-content/resources/56323-PUB_0.74.pdf

How stable are those links? Past experience shows not very.

Please upload those to a bugzilla.kernel.org entry and add that URL here
with a Link: tag.

> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Babu Moger <babu.moger@....com>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Fenghua Yu <fenghua.yu@...el.com>
> Cc: Frank van der Linden <fllinden@...zon.com>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: Huang Rui <ray.huang@....com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Janakarajan Natarajan <Janakarajan.Natarajan@....com>
> Cc: Jan Beulich <jbeulich@...e.com>
> Cc: Jiaxun Yang <jiaxun.yang@...goat.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Josh Poimboeuf <jpoimboe@...hat.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Luwei Kang <luwei.kang@...el.com>
> Cc: Martin Liška <mliska@...e.cz>
> Cc: Matt Fleming <matt@...eblueprint.co.uk>
> Cc: Michael Petlan <mpetlan@...hat.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Paolo Bonzini <pbonzini@...hat.com>
> Cc: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Tom Lendacky <thomas.lendacky@....com>
> Cc: x86@...nel.org
> Cc: linux-kernel@...r.kernel.org
> Cc: stable@...r.kernel.org
> Fixes: aaf248848db50 ("perf/x86/msr: Add AMD IRPERF (Instructions Retired) performance counter")
> Signed-off-by: Kim Phillips <kim.phillips@....com>
> ---
> RESEND, adding Michael Petlan to cc. Original v2:
> 
> https://lore.kernel.org/lkml/20200121171232.28839-2-kim.phillips@amd.com/
> 
> v2: Based on Andi Kleen's review:
> 
>     https://lore.kernel.org/lkml/20200116040324.GI302770@tassilo.jf.intel.com/
> 
>     Add an amd_erratum_1054 and use cpu_has_bug infrastructure instead of
>     open-coding the {family,model} check.
> 
>  arch/x86/include/asm/cpufeatures.h |  1 +
>  arch/x86/include/asm/msr-index.h   |  2 ++
>  arch/x86/kernel/cpu/amd.c          | 17 +++++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index f3327cb56edf..1c1600e7476b 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -404,5 +404,6 @@
>  #define X86_BUG_SWAPGS			X86_BUG(21) /* CPU is affected by speculation through SWAPGS */
>  #define X86_BUG_TAA			X86_BUG(22) /* CPU is affected by TSX Async Abort(TAA) */
>  #define X86_BUG_ITLB_MULTIHIT		X86_BUG(23) /* CPU may incur MCE during certain page attribute changes */
> +#define X86_BUG_AMD_E1054		X86_BUG(24) /* CPU is among the affected by Erratum 1054 */

That is visible in /proc/cpuinfo and the string "amd_e1054" means
nothing. Call that

X86_BUG_IRPERF

or so to at least give some hint as to what the bug is.

>  
>  #endif /* _ASM_X86_CPUFEATURES_H */
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index ebe1685e92dd..d5e517d1c3dd 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -512,6 +512,8 @@
>  #define MSR_K7_HWCR			0xc0010015
>  #define MSR_K7_HWCR_SMMLOCK_BIT		0
>  #define MSR_K7_HWCR_SMMLOCK		BIT_ULL(MSR_K7_HWCR_SMMLOCK_BIT)
> +#define MSR_K7_HWCR_IRPERF_EN_BIT	30
> +#define MSR_K7_HWCR_IRPERF_EN		BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)
>  #define MSR_K7_FID_VID_CTL		0xc0010041
>  #define MSR_K7_FID_VID_STATUS		0xc0010042
>  
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 62c30279be77..c067234a271f 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -28,6 +28,7 @@
>  
>  static const int amd_erratum_383[];
>  static const int amd_erratum_400[];
> +static const int amd_erratum_1054[];
>  static bool cpu_has_amd_erratum(struct cpuinfo_x86 *cpu, const int *erratum);
>  
>  /*
> @@ -701,6 +702,9 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>  	if (cpu_has_amd_erratum(c, amd_erratum_400))
>  		set_cpu_bug(c, X86_BUG_AMD_E400);
>  
> +	if (cpu_has_amd_erratum(c, amd_erratum_1054))
> +		set_cpu_bug(c, X86_BUG_AMD_E1054);
> +
>  	early_detect_mem_encrypt(c);
>  
>  	/* Re-enable TopologyExtensions if switched off by BIOS */
> @@ -978,6 +982,15 @@ static void init_amd(struct cpuinfo_x86 *c)
>  	/* AMD CPUs don't reset SS attributes on SYSRET, Xen does. */
>  	if (!cpu_has(c, X86_FEATURE_XENPV))
>  		set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
> +
> +	/*
> +	 * Turn on the Instructions Retired free counter on machines not
> +	 * susceptible to erratum #1054 "Instructions Retired Performance
> +	 * Counter May Be Inaccurate"
				     .
				     ^
				     |--- fullstop.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ