[RFC PATCH v2] perf: honouring the cpuid for number of fixed counters in hypervisors

[RFC PATCH v2] perf: honouring the cpuid for number of fixed counters in hypervisors

[Imre Palik]

From: Imre Palik <imrep@amazon.de>

perf doesn't seem to honour the number of fixed counters specified by cpuid
leaf 0xa.  It always assume that Intel CPUs have at least 3 fixed counters.

So if some of the fixed counters are masked out by the hypervisor, it still
tries to check/set them.

This patch makes perf behave nicer when the kernel is running under a
hypervisor that doesn't expose all the counters.

This patch contains some ideas from Matt Wilson.

Signed-off-by: Imre Palik <imrep@amazon.de>
Cc: Matt Wilson <msw@amazon.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/events/intel/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index a3a9eb8..e06d071 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3607,10 +3607,14 @@ __init int intel_pmu_init(void)
 
 	/*
 	 * Quirk: v2 perfmon does not report fixed-purpose events, so
-	 * assume at least 3 events:
+	 * assume at least 3 events, when not running in a hypervisor:
 	 */
-	if (version > 1)
-		x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
+	if (version > 1) {
+		if (static_cpu_has(X86_FEATURE_HYPERVISOR))
+			x86_pmu.num_counters_fixed = edx.split.num_counters_fixed;
+		else
+			x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
+	}
 
 	if (boot_cpu_has(X86_FEATURE_PDCM)) {
 		u64 capabilities;
-- 
2.10.1

Re: [RFC PATCH v2] perf: honouring the cpuid for number of fixed counters in hypervisors

[Andi Kleen]

On Fri, Oct 21, 2016 at 01:18:59AM -0700, Imre Palik wrote:
> From: Imre Palik <imrep@amazon.de>
> 
> perf doesn't seem to honour the number of fixed counters specified by cpuid
> leaf 0xa.  It always assume that Intel CPUs have at least 3 fixed counters.
> 
> So if some of the fixed counters are masked out by the hypervisor, it still
> tries to check/set them.
> 
> This patch makes perf behave nicer when the kernel is running under a
> hypervisor that doesn't expose all the counters.
> 
> This patch contains some ideas from Matt Wilson.

Patch looks good to me.

Reviewed-by: Andi Kleen <ak@linux.intel.com>

-Andi

Re: [RFC PATCH v2] perf: honouring the cpuid for number of fixed counters in hypervisors

[Peter Zijlstra]

On Fri, Oct 21, 2016 at 01:18:59AM -0700, Imre Palik wrote:
> +++ b/arch/x86/events/intel/core.c
> @@ -3607,10 +3607,14 @@ __init int intel_pmu_init(void)
>  
>  	/*
>  	 * Quirk: v2 perfmon does not report fixed-purpose events, so
> +	 * assume at least 3 events, when not running in a hypervisor:
>  	 */
> +	if (version > 1) {
> +		if (static_cpu_has(X86_FEATURE_HYPERVISOR))
> +			x86_pmu.num_counters_fixed = edx.split.num_counters_fixed;
> +		else
> +			x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
> +	}


I made that:

 	/*
 	 * Quirk: v2 perfmon does not report fixed-purpose events, so
+	 * assume at least 3 events, when not running in a hypervisor:
 	 */
+	if (version > 1) {
+		int assume = 3 * !boot_cpu_has(X86_FEATURE_HYPERVISOR);
+
+		x86_pmu.num_counters_fixed =
+			max((int)edx.split.num_counters_fixed, assume);
+	}
 

Thanks!

[tip:perf/urgent] perf/x86/intel: Honour the CPUID for number of fixed counters in hypervisors

[tip-bot for Imre Palik]

Commit-ID:  f92b7604149a55cb601fc0b52911b1e11f0f2514
Gitweb:     http://git.kernel.org/tip/f92b7604149a55cb601fc0b52911b1e11f0f2514
Author:     Imre Palik <imrep@amazon.de>
AuthorDate: Fri, 21 Oct 2016 01:18:59 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 28 Oct 2016 11:06:25 +0200

perf/x86/intel: Honour the CPUID for number of fixed counters in hypervisors

perf doesn't seem to honour the number of fixed counters specified by CPUID
leaf 0xa. It always assumes that Intel CPUs have at least 3 fixed counters.

So if some of the fixed counters are masked out by the hypervisor, it still
tries to check/set them.

This patch makes perf behave nicer when the kernel is running under a
hypervisor that doesn't expose all the counters.

This patch contains some ideas from Matt Wilson.

Signed-off-by: Imre Palik <imrep@amazon.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Cc: Alexander Kozyrev <alexander.kozyrev@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Artyom Kuanbekov <artyom.kuanbekov@intel.com>
Cc: David Carrillo-Cisneros <davidcc@google.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Wilson <msw@amazon.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1477037939-15605-1-git-send-email-imrep.amz@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index eab0915..a74a2db 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3607,10 +3607,14 @@ __init int intel_pmu_init(void)
 
 	/*
 	 * Quirk: v2 perfmon does not report fixed-purpose events, so
-	 * assume at least 3 events:
+	 * assume at least 3 events, when not running in a hypervisor:
 	 */
-	if (version > 1)
-		x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
+	if (version > 1) {
+		int assume = 3 * !boot_cpu_has(X86_FEATURE_HYPERVISOR);
+
+		x86_pmu.num_counters_fixed =
+			max((int)edx.split.num_counters_fixed, assume);
+	}
 
 	if (boot_cpu_has(X86_FEATURE_PDCM)) {
 		u64 capabilities;