linux-kernel - Re: [PATCH] perf/x86/intel/uncore: correct uncore num

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]

Message-ID: <20160810105231.GA14283@gmail.com>
Date:	Wed, 10 Aug 2016 12:52:31 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	kan.liang@...el.com
Cc:	peterz@...radead.org, linux-kernel@...r.kernel.org,
	tglx@...utronix.de, mingo@...hat.com, eranian@...gle.com,
	andi@...stfloor.org, Lukasz Odzioba <lukasz.odzioba@...el.com>
Subject: Re: [PATCH] perf/x86/intel/uncore: correct uncore num_counters


* kan.liang@...el.com <kan.liang@...el.com> wrote:

> From: Kan Liang <kan.liang@...el.com>
> 
> Some uncore boxes' num_counters for Haswell server and Broadwell server
> are not correct. This patch make them consistent with the uncore
> document.
> 
> Reported-by: Lukasz Odzioba <lukasz.odzioba@...el.com>
> Signed-off-by: Kan Liang <kan.liang@...el.com>
> ---
>  arch/x86/events/intel/uncore_snbep.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
> index 824e540..8aee83b 100644
> --- a/arch/x86/events/intel/uncore_snbep.c
> +++ b/arch/x86/events/intel/uncore_snbep.c
> @@ -2626,7 +2626,7 @@ void hswep_uncore_cpu_init(void)
>  
>  static struct intel_uncore_type hswep_uncore_ha = {
>  	.name		= "ha",
> -	.num_counters   = 5,
> +	.num_counters   = 4,
>  	.num_boxes	= 2,
>  	.perf_ctr_bits	= 48,
>  	SNBEP_UNCORE_PCI_COMMON_INIT(),
> @@ -2645,7 +2645,7 @@ static struct uncore_event_desc hswep_uncore_imc_events[] = {
>  
>  static struct intel_uncore_type hswep_uncore_imc = {
>  	.name		= "imc",
> -	.num_counters   = 5,
> +	.num_counters   = 4,
>  	.num_boxes	= 8,
>  	.perf_ctr_bits	= 48,
>  	.fixed_ctr_bits	= 48,
> @@ -2691,7 +2691,7 @@ static struct intel_uncore_type hswep_uncore_irp = {
>  
>  static struct intel_uncore_type hswep_uncore_qpi = {
>  	.name			= "qpi",
> -	.num_counters		= 5,
> +	.num_counters		= 4,
>  	.num_boxes		= 3,
>  	.perf_ctr_bits		= 48,
>  	.perf_ctr		= SNBEP_PCI_PMON_CTR0,
> @@ -2773,7 +2773,7 @@ static struct event_constraint hswep_uncore_r3qpi_constraints[] = {
>  
>  static struct intel_uncore_type hswep_uncore_r3qpi = {
>  	.name		= "r3qpi",
> -	.num_counters   = 4,
> +	.num_counters   = 3,
>  	.num_boxes	= 3,
>  	.perf_ctr_bits	= 44,
>  	.constraints	= hswep_uncore_r3qpi_constraints,
> @@ -2972,7 +2972,7 @@ static struct intel_uncore_type bdx_uncore_ha = {
>  
>  static struct intel_uncore_type bdx_uncore_imc = {
>  	.name		= "imc",
> -	.num_counters   = 5,
> +	.num_counters   = 4,
>  	.num_boxes	= 8,
>  	.perf_ctr_bits	= 48,
>  	.fixed_ctr_bits	= 48,

So this changelog really sucks: what was the effect of the bug?

Did we report bogus (or zero) counts for those non-existent counters - or did the 
code actually crash in a visible way?

Thanks,

	Ingo