perf tools: fix guest mode monitoring on AMD
Subsystems (4)
PATCH 1/1 perf tools: fix guest mode monitoring on AMD
Commit:
1aed267 perf kvm: Do guest-only counting by default
introduced a bug on AMD systems whereby simple commands:
$ perf stat ls
Performance counter stats for 'ls':
0 cycles # 0.000 GHz
0.003704596 seconds time elapsedwould not count anything anymore. Same results for perf record.
I tracked it down to guest mode exclusion being enabled by default leading to attr->exclude_guest = 1. When not operating under any sort of virtualization, this causes the PMU not to count anything.
The fix disables guest exclusion by default.
---
tools/perf/util/util.c +1 -1
@@ -6,7 +6,7 @@* XXX We need to find a better place for these things... */ bool perf_host = true; -bool perf_guest = false; +bool perf_guest = true;void event_attr_init (struct perf_event_attr *attr) {
Quoted 11 lines · Stephane Eranian
Commit:
1aed267 perf kvm: Do guest-only counting by default
introduced a bug on AMD systems whereby simple commands:
$ perf stat ls Performance counter stats for 'ls': 0 cycles # 0.000 GHz 0.003704596 seconds time elapsed
Quoted 6 lines
would not count anything anymore. Same results for perf record.
I tracked it down to guest mode exclusion being enabled by default leading to
attr->exclude_guest= 1. When not operating under any sort of virtualization, this causes the PMU not to count anything.
CCing Joerg. I think he fixed this or similar problem recently.
Quoted 11 lines
The fix disables guest exclusion by default.
Signed-off-by Stephane Eranian <[email protected]>---
tools/perf/util/util.c +0
-0
tools/perf/util/util.c line 12
Quoted 8 lines
tools/perf/util/util.c +1
-1@@ -6,7 +6,7 @@* XXX We need to find a better place for these things... */ bool perf_host = true; -bool perf_guest = false; +bool perf_guest = true;void event_attr_init (struct perf_event_attr *attr) {
--
Gleb.
Quoted 28 lines · Stephane Eranian
Commit:
1aed267 perf kvm: Do guest-only counting by default
introduced a bug on AMD systems whereby simple commands:
$ perf stat ls Performance counter stats for 'ls': 0 cycles # 0.000 GHz 0.003704596 seconds time elapsedwould not count anything anymore. Same results for perf record.
I tracked it down to guest mode exclusion being enabled by default leading to
attr->exclude_guest= 1. When not operating under any sort of virtualization, this causes the PMU not to count anything.The fix disables guest exclusion by default.
Signed-off-by Stephane Eranian<[email protected]>---
tools/perf/util/util.c +0
-0
tools/perf/util/util.c line 9
Quoted 5 lines
tools/perf/util/util.c +1
-1@@ -6,7 +6,7 @@* XXX We need to find a better place for these things... */ bool perf_host = true; -bool perf_guest = false; +bool perf_guest = true;
This was recently reverted to false by c4a7dca92bbb9881a5d678720f1d0c2153499749
See: https://lkml.org/lkml/2012/2/8/234
David
Quoted 8 lines
tools/perf/util/util.c +0
-1void event_attr_init (struct perf_event_attr *attr) { --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 40 lines · David Ahern
Quoted 29 lines · Stephane Eranian
Commit:
1aed267 perf kvm: Do guest-only counting by default
introduced a bug on AMD systems whereby simple commands:
$ perf stat ls Performance counter stats for 'ls': 0 cycles # 0.000 GHz 0.003704596 seconds time elapsedwould not count anything anymore. Same results for perf record.
I tracked it down to guest mode exclusion being enabled by default leading to
attr->exclude_guest= 1. When not operating under any sort of virtualization, this causes the PMU not to count anything.The fix disables guest exclusion by default.
Signed-off-by Stephane Eranian<[email protected]>---
tools/perf/util/util.c +0
-0tools/perf/util/util.c line 9
Quoted 5 lines
tools/perf/util/util.c +1
-1@@ -6,7 +6,7 @@* XXX We need to find a better place for these things... */ bool perf_host = true; -bool perf_guest = false; +bool perf_guest = true;This was recently reverted to false by
c4a7dca92bbb9881a5d678720f1d0c2153499749
Quoted 2 lines
Yeah, but that causes simple commands such as "perf stat -e cycles ls" to return 0 count.
So either you get a segfault or you get zero count. There is something else going on here...
Quoted 12 lines
David
Quoted 8 lines
tools/perf/util/util.c +0
-1void event_attr_init(struct perf_event_attr *attr) { --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 41 lines · Stephane Eranian
Quoted 40 lines · David Ahern
Quoted 29 lines · Stephane Eranian
Commit:
1aed267 perf kvm: Do guest-only counting by default
introduced a bug on AMD systems whereby simple commands:
$ perf stat ls Performance counter stats for 'ls': 0 cycles # 0.000 GHz 0.003704596 seconds time elapsedwould not count anything anymore. Same results for perf record.
I tracked it down to guest mode exclusion being enabled by default leading to
attr->exclude_guest= 1. When not operating under any sort of virtualization, this causes the PMU not to count anything.The fix disables guest exclusion by default.
Signed-off-by Stephane Eranian<[email protected]>---
tools/perf/util/util.c +0
-0tools/perf/util/util.c line 9
Quoted 5 lines
tools/perf/util/util.c +1
-1@@ -6,7 +6,7 @@* XXX We need to find a better place for these things... */ bool perf_host = true; -bool perf_guest = false; +bool perf_guest = true;This was recently reverted to false by
c4a7dca92bbb9881a5d678720f1d0c2153499749
Quoted 7 lines
Quoted 2 lines
Yeah, but that causes simple commands such as "perf stat -e cycles ls" to return 0 count.
So either you get a segfault or you get zero count. There is something else going on here...
agreed. Did you try reverting exclude_guest by default?
Quoted 5 lines · Stephane Eranian
Yeah, but that causes simple commands such as "perf stat -e cycles ls" to return 0 count.
So either you get a segfault or you get zero count. There is something else going on here...
Hmm, looks like the counters will not do any counting when EFER.SVME==0 and HO==1 or GO==1 is set in the counters :(
A solution would be to mask out these bits in the perf-ctrl register when EFER.SVME=0
I'll try to come up with a fix soon.
Joerg
PATCH 1/1 perf/x86: Fix HO/GO counting with SVM disabled
It turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
--- arch/x86/include/asm/perf_event.h | 5 +++++ arch/x86/kernel/cpu/perf_event.c | 30 ++++++++++++++++++++++++++++++ arch/x86/kernel/cpu/perf_event.h | 6 +++++- arch/x86/kernel/cpu/perf_event_amd.c | 6 ++++-- arch/x86/kvm/svm.c | 5 +++++ 5 files changed, 49 insertions(+), 3 deletions(-)
arch/x86/include/asm/perf_event.h +5 -0
@@ -227,6 +227,8 @@ struct perf_guest_switch_msr {extern struct perf_guest_switch_msr *perf_guest_get_msrs (int *nr);extern void perf_get_x86_pmu_capability (struct x86_pmu_capability *cap); +extern void x86_pmu_enable_virt (void ); +extern void x86_pmu_disable_virt (void );#else static inline perf_guest_switch_msr *perf_guest_get_msrs (int *nr) {
@@ -240,6 +242,9 @@ static inline void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap) }static inline void perf_events_lapic_init (void ) { } + +static inline void x86_pmu_enable_virt (void ) { } +static inline void x86_pmu_disable_virt (void ) { }#endif #endif /* _ASM_X86_PERF_EVENT_H */
arch/x86/kernel/cpu/perf_event.c +30 -0
@@ -477,6 +477,36 @@ void x86_pmu_enable_all(int added) } } +void x86_pmu_enable_virt (void ) +{ +struct cpu_hw_events *cpuc = &__get_cpu_var (cpu_hw_events); + + cpuc->perf_ctr_virt_mask =0 ; + +/* Reload all events */ +x86_pmu_disable_all (); +x86_pmu_enable_all (0 ); +} +EXPORT_SYMBOL_GPL (x86_pmu_enable_virt); + +void x86_pmu_disable_virt (void ) +{ +struct cpu_hw_events *cpuc = &__get_cpu_var (cpu_hw_events); + +/* +* We only mask out the Host-only bit so that host-only counting works +* when SVM is disabled. If someone sets up a guest-only counter when +* SVM is disabled the Guest-only bits still gets set and the counter +* will not count anything. +*/ + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +/* Reload all events */ +x86_pmu_disable_all (); +x86_pmu_enable_all (0 ); +} +EXPORT_SYMBOL_GPL (x86_pmu_disable_virt); +static struct pmu pmu;static inline int is_x86_event (struct perf_event *event)
arch/x86/kernel/cpu/perf_event.h +5 -1
@@ -148,6 +148,8 @@ struct cpu_hw_events {* AMD specific bits */ struct amd_nb *amd_nb; +/* Inverted mask of bits to clear in the perf_ctr ctrl registers */ +u64 perf_ctr_virt_mask;void *kfree_on_online; };
@@ -417,9 +419,11 @@ void x86_pmu_disable_all(void);static inline void __x86_pmu_enable_event (struct hw_perf_event *hwc,u64 enable_mask) { +u64 disable_mask =__this_cpu_read (cpu_hw_events.perf_ctr_virt_mask); +if (hwc->extra_reg.reg)wrmsrl (hwc->extra_reg.reg, hwc->extra_reg.config); -wrmsrl (hwc->config_base, hwc->config | enable_mask); +wrmsrl (hwc->config_base, (hwc->config | enable_mask) & ~disable_mask); }void x86_pmu_enable_all (int added);
arch/x86/kernel/cpu/perf_event_amd.c +4 -2
@@ -357,7 +357,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id; -if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ; nb_id =amd_get_nb_id (cpu);
@@ -587,9 +589,9 @@ static __initconst const struct x86_pmu amd_pmu_f15h = { .put_event_constraints = amd_put_event_constraints, .cpu_prepare = amd_pmu_cpu_prepare, - .cpu_starting = amd_pmu_cpu_starting, .cpu_dead = amd_pmu_cpu_dead, #endif + .cpu_starting = amd_pmu_cpu_starting, };__init int amd_pmu_init (void )
arch/x86/kvm/svm.c +5 -0
@@ -29,6 +29,7 @@#include <linux/ftrace_event.h> #include <linux/slab.h> +#include <asm/perf_event.h> #include <asm/tlbflush.h> #include <asm/desc.h> #include <asm/kvm_para.h>
@@ -575,6 +576,8 @@ static void svm_hardware_disable(void *garbage)wrmsrl (MSR_AMD64_TSC_RATIO ,TSC_RATIO_DEFAULT );cpu_svm_disable (); + +x86_pmu_disable_virt (); }static int svm_hardware_enable (void *garbage)
@@ -622,6 +625,8 @@ static int svm_hardware_enable(void *garbage)svm_init_erratum_383 (); +x86_pmu_enable_virt (); +return 0 ; }
Signature · 1 line
1.7.5.4
Re: [PATCH] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 24 lines · Joerg Roedel
__initintamd_pmu_init(void)arch/x86/kvm/svm.c +3
-0@@ -29,6 +29,7 @@#include <linux/ftrace_event.h> #include <linux/slab.h> +#include <asm/perf_event.h> #include <asm/tlbflush.h> #include <asm/desc.h> #include <asm/kvm_para.h> @@ -575,6 +576,8 @@ static void svm_hardware_disable(void *garbage)wrmsrl (MSR_AMD64_TSC_RATIO ,TSC_RATIO_DEFAULT );cpu_svm_disable (); + +x86_pmu_disable_virt (); }static int svm_hardware_enable (void *garbage)
arch/x86/kvm/svm.c static int svm_hardware_enable(void *garbage) · line 632
Quoted 8 lines
arch/x86/kvm/svm.c +2
-0@@ -622,6 +625,8 @@ static int svm_hardware_enable(void *garbage)svm_init_erratum_383 (); +x86_pmu_enable_virt (); +return 0 ; }
These should go into x86.c. If the functions later gain meaning on Intel, we want them to be called (and nothing in the name suggests they're AMD specific).
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 28 lines · Avi Kivity
Quoted 27 lines · Joerg Roedel
__initintamd_pmu_init(void)arch/x86/kvm/svm.c +3
-0@@ -29,6 +29,7 @@#include <linux/ftrace_event.h> #include <linux/slab.h> +#include <asm/perf_event.h> #include <asm/tlbflush.h> #include <asm/desc.h> #include <asm/kvm_para.h> @@ -575,6 +576,8 @@ static void svm_hardware_disable(void *garbage)wrmsrl (MSR_AMD64_TSC_RATIO ,TSC_RATIO_DEFAULT );cpu_svm_disable (); + +x86_pmu_disable_virt (); }static int svm_hardware_enable (void *garbage)@@ -622,6 +625,8 @@ static int svm_hardware_enable(void *garbage)svm_init_erratum_383 ();
Quoted 10 lines
arch/x86/kvm/svm.c static int svm_hardware_enable(void *garbage) · line 632
Quoted 5 lines
arch/x86/kvm/svm.c +2
-0+x86_pmu_enable_virt (); +return 0 ; }These should go into
x86.c. If the functions later gain meaning on Intel, we want them to be called (and nothing in the name suggests they're AMD specific).
I was to suggest the reverse: since this patch addesses an AMD bug, why not push those functions into perf_event_amd.c and make them dependent on CONFIG_CPU_SUP_AMD as well.
David
Re: [PATCH] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 40 lines · David Ahern
Quoted 28 lines · Avi Kivity
Quoted 27 lines · Joerg Roedel
__initintamd_pmu_init(void)arch/x86/kvm/svm.c +3
-0@@ -29,6 +29,7 @@#include <linux/ftrace_event.h> #include <linux/slab.h> +#include <asm/perf_event.h> #include <asm/tlbflush.h> #include <asm/desc.h> #include <asm/kvm_para.h> @@ -575,6 +576,8 @@ static void svm_hardware_disable(void *garbage)wrmsrl (MSR_AMD64_TSC_RATIO ,TSC_RATIO_DEFAULT );cpu_svm_disable (); + +x86_pmu_disable_virt (); }static int svm_hardware_enable (void *garbage)@@ -622,6 +625,8 @@ static int svm_hardware_enable(void *garbage)svm_init_erratum_383 ();Quoted 10 lines
arch/x86/kvm/svm.c static int svm_hardware_enable(void *garbage) · line 632
Quoted 5 lines
arch/x86/kvm/svm.c +2
-0+x86_pmu_enable_virt (); +return 0 ; }These should go into
x86.c. If the functions later gain meaning on Intel, we want them to be called (and nothing in the name suggests they're AMD specific).
Quoted 3 lines
I was to suggest the reverse: since this patch addesses an AMD bug, why not push those functions into
perf_event_amd.cand make them dependent onCONFIG_CPU_SUP_AMDas well.
It depends on which direction you expect the code to grow. These hooks seem reasonable, so I think they should be generic.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 8 lines · Avi Kivity
Quoted 3 lines · David Ahern
I was to suggest the reverse: since this patch addesses an AMD bug, why not push those functions into
perf_event_amd.cand make them dependent onCONFIG_CPU_SUP_AMDas well.It depends on which direction you expect the code to grow. These hooks seem reasonable, so I think they should be generic.
Okay, I am fine with both directions. But since this patch is a fix in the first place I make it more AMD specific for now. If we need this as a generic feature it can easily be changed back.
Joerg
Signature · 5 lines
AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
PATCH v2 1/1 perf/x86: Fix HO/GO counting with SVM disabled
In reply to Joerg RoedelIt turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
--- arch/x86/include/asm/perf_event.h | 8 +++++++ arch/x86/kernel/cpu/perf_event.h | 6 ++++- arch/x86/kernel/cpu/perf_event_amd.c | 37 ++++++++++++++++++++++++++++++++- arch/x86/kvm/svm.c | 5 ++++ 4 files changed, 53 insertions(+), 3 deletions(-)
arch/x86/include/asm/perf_event.h +8 -0
@@ -242,4 +242,12 @@ static inline void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap)static inline void perf_events_lapic_init (void ) { }#endif +#if defined(CONFIG_PERF_EVENTS ) && defined(CONFIG_CPU_SUP_AMD ) +extern void amd_pmu_enable_virt (void ); +extern void amd_pmu_disable_virt (void ); +#else /* CONFIG_PERF_EVENTS && CONFIG_CPU_SUP_AMD */ +static inline void amd_pmu_enable_virt (void ) { } +static inline void amd_pmu_disable_virt (void ) { } +#endif /* CONFIG_PERF_EVENTS && CONFIG_CPU_SUP_AMD */ +#endif /* _ASM_X86_PERF_EVENT_H */
arch/x86/kernel/cpu/perf_event.h +5 -1
@@ -148,6 +148,8 @@ struct cpu_hw_events {* AMD specific bits */ struct amd_nb *amd_nb; +/* Inverted mask of bits to clear in the perf_ctr ctrl registers */ +u64 perf_ctr_virt_mask;void *kfree_on_online; };
@@ -417,9 +419,11 @@ void x86_pmu_disable_all(void);static inline void __x86_pmu_enable_event (struct hw_perf_event *hwc,u64 enable_mask) { +u64 disable_mask =__this_cpu_read (cpu_hw_events.perf_ctr_virt_mask); +if (hwc->extra_reg.reg)wrmsrl (hwc->extra_reg.reg, hwc->extra_reg.config); -wrmsrl (hwc->config_base, hwc->config | enable_mask); +wrmsrl (hwc->config_base, (hwc->config | enable_mask) & ~disable_mask); }void x86_pmu_enable_all (int added);
arch/x86/kernel/cpu/perf_event_amd.c +35 -2
@@ -1,4 +1,5 @@#include <linux/perf_event.h> +#include <linux/export.h> #include <linux/types.h> #include <linux/init.h> #include <linux/slab.h>
@@ -357,7 +358,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id; -if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ; nb_id =amd_get_nb_id (cpu);
@@ -587,9 +590,9 @@ static __initconst const struct x86_pmu amd_pmu_f15h = { .put_event_constraints = amd_put_event_constraints, .cpu_prepare = amd_pmu_cpu_prepare, - .cpu_starting = amd_pmu_cpu_starting, .cpu_dead = amd_pmu_cpu_dead, #endif + .cpu_starting = amd_pmu_cpu_starting, };__init int amd_pmu_init (void )
@@ -621,3 +624,33 @@ __init int amd_pmu_init(void)return 0 ; } + +void amd_pmu_enable_virt (void ) +{ +struct cpu_hw_events *cpuc = &__get_cpu_var (cpu_hw_events); + + cpuc->perf_ctr_virt_mask =0 ; + +/* Reload all events */ +x86_pmu_disable_all (); +x86_pmu_enable_all (0 ); +} +EXPORT_SYMBOL_GPL (amd_pmu_enable_virt); + +void amd_pmu_disable_virt (void ) +{ +struct cpu_hw_events *cpuc = &__get_cpu_var (cpu_hw_events); + +/* +* We only mask out the Host-only bit so that host-only counting works +* when SVM is disabled. If someone sets up a guest-only counter when +* SVM is disabled the Guest-only bits still gets set and the counter +* will not count anything. +*/ + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +/* Reload all events */ +x86_pmu_disable_all (); +x86_pmu_enable_all (0 ); +} +EXPORT_SYMBOL_GPL (amd_pmu_disable_virt);
arch/x86/kvm/svm.c +5 -0
@@ -29,6 +29,7 @@#include <linux/ftrace_event.h> #include <linux/slab.h> +#include <asm/perf_event.h> #include <asm/tlbflush.h> #include <asm/desc.h> #include <asm/kvm_para.h>
@@ -575,6 +576,8 @@ static void svm_hardware_disable(void *garbage)wrmsrl (MSR_AMD64_TSC_RATIO ,TSC_RATIO_DEFAULT );cpu_svm_disable (); + +amd_pmu_disable_virt (); }static int svm_hardware_enable (void *garbage)
@@ -622,6 +625,8 @@ static int svm_hardware_enable(void *garbage)svm_init_erratum_383 (); +amd_pmu_enable_virt (); +return 0 ; }
Signature · 1 line
1.7.5.4
Re: [PATCH v2] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 24 lines · Joerg Roedel
It turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
arch/x86/kernel/cpu/perf_event_amd.c +1
-0@@ -1,4 +1,5 @@#include <linux/perf_event.h> +#include <linux/export.h> #include <linux/types.h> #include <linux/init.h> #include <linux/slab.h>
arch/x86/kernel/cpu/perf_event_amd.c static void amd_pmu_cpu_starting(int cpu) · line 364
Quoted 8 lines
arch/x86/kernel/cpu/perf_event_amd.c +3
-1@@ -357,7 +358,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id; -if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ;
Why this (boot_cpu_data.x86 == 0x15) change?
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH v2] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 28 lines · Avi Kivity
Quoted 27 lines · Joerg Roedel
It turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
arch/x86/kernel/cpu/perf_event_amd.c +1
-0@@ -1,4 +1,5 @@#include <linux/perf_event.h> +#include <linux/export.h> #include <linux/types.h> #include <linux/init.h> #include <linux/slab.h> @@ -357,7 +358,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id;
Quoted 8 lines
arch/x86/kernel/cpu/perf_event_amd.c static void amd_pmu_cpu_starting(int cpu) · line 364
Quoted 5 lines
arch/x86/kernel/cpu/perf_event_amd.c +3
-1-if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ;Why this (boot_cpu_data.x86 ==
0x15) change?
I think it is related to .cpu_starting callback now available in amd_pmu_f15h where previously it wasn't.
--
Gleb.
Re: [PATCH v2] perf/x86: Fix HO/GO counting with SVM disabled
In reply to Avi KivityQuoted 28 lines · Avi Kivity
Quoted 27 lines · Joerg Roedel
It turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
arch/x86/kernel/cpu/perf_event_amd.c +1
-0@@ -1,4 +1,5 @@#include <linux/perf_event.h> +#include <linux/export.h> #include <linux/types.h> #include <linux/init.h> #include <linux/slab.h> @@ -357,7 +358,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id;
Quoted 7 lines
arch/x86/kernel/cpu/perf_event_amd.c static void amd_pmu_cpu_starting(int cpu) · line 364
Quoted 5 lines
arch/x86/kernel/cpu/perf_event_amd.c +3
-1-if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ;Why this (boot_cpu_data.x86 ==
0x15) change?
This is because this function did not run on Fam15h before but now it has to so that cpuc->perf_ctr_virt_mask is initialized. The other stuff done in this function is setup for northbridge counter. These are not yet implemented for Fam15h CPUs so this setup must not run on those CPUs. Therefore the check was added.
Once northbridge counters are implemented for Fam15h this check can go away again.
Joerg
Signature · 5 lines
AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [PATCH v2] perf/x86: Fix HO/GO counting with SVM disabled
Quoted 37 lines · Joerg Roedel
Quoted 28 lines · Avi Kivity
Quoted 27 lines · Joerg Roedel
It turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
arch/x86/kernel/cpu/perf_event_amd.c +1
-0@@ -1,4 +1,5 @@#include <linux/perf_event.h> +#include <linux/export.h> #include <linux/types.h> #include <linux/init.h> #include <linux/slab.h> @@ -357,7 +358,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id;Quoted 7 lines
arch/x86/kernel/cpu/perf_event_amd.c static void amd_pmu_cpu_starting(int cpu) · line 364
Quoted 5 lines
arch/x86/kernel/cpu/perf_event_amd.c +3
-1-if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ;Why this (boot_cpu_data.x86 ==
0x15) change?
Quoted 8 lines
This is because this function did not run on Fam15h before but now it has to so that
cpuc->perf_ctr_virt_maskis initialized. The other stuff done in this function is setup for northbridge counter. These are not yet implemented for Fam15h CPUs so this setup must not run on those CPUs. Therefore the check was added.
Once northbridge counters are implemented for Fam15h this check can go away again.
Thanks. Ingo, this had better go through tip.git since most of the changes are perf related.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH v2] perf/x86: Fix HO/GO counting with SVM disabled
In reply to Joerg RoedelQuoted 2 lines · Joerg Roedel
Once northbridge counters are implemented for Fam15h this check can go away again.
Nope, northbridge on fam15 is completely disjoint from the regular pmu and should thus be a separate driver.
The only reason the old amd driver has them both is because how they shared the registers so there is a shared resource to manage.
Re: [PATCH v2] perf/x86: Fix HO/GO counting with SVM disabled
In reply to Joerg RoedelQuoted 14 lines · Joerg Roedel
arch/x86/kernel/cpu/perf_event.h +2
-0@@ -148,6 +148,8 @@ struct cpu_hw_events {* AMD specific bits */ struct amd_nb *amd_nb; +/* Inverted mask of bits to clear in the perf_ctr ctrl registers */ +u64 perf_ctr_virt_mask;void *kfree_on_online; };
arch/x86/kernel/cpu/perf_event.h void x86_pmu_disable_all(void); · line 427
Quoted 10 lines
arch/x86/kernel/cpu/perf_event.h +3
-1@@ -417,9 +419,11 @@ void x86_pmu_disable_all(void);static inline void __x86_pmu_enable_event (struct hw_perf_event *hwc,u64 enable_mask) { +u64 disable_mask =__this_cpu_read (cpu_hw_events.perf_ctr_virt_mask); +if (hwc->extra_reg.reg)wrmsrl (hwc->extra_reg.reg, hwc->extra_reg.config); -wrmsrl (hwc->config_base, hwc->config | enable_mask); +wrmsrl (hwc->config_base, (hwc->config | enable_mask) & ~disable_mask); }
Its starting to look like we should kill this helper, the extra_reg muck is Intel only and the disable_mask is AMD.
Queued it for now though..
[tip:perf/urgent] perf/x86/kvm: Fix Host-Only/ Guest-Only counting with SVM disabled
In reply to Joerg RoedelCommit-ID: 1018faa6cf23b256bf25919ef203cd7c129f06f2 Gitweb: http://git.kernel.org/tip/1018faa6cf23b256bf25919ef203cd7c129f06f2 Author: Joerg Roedel <[email protected]> AuthorDate: Wed, 29 Feb 2012 14:57:32 +0100 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 2 Mar 2012 12:16:39 +0100
perf/x86/kvm: Fix Host-Only/Guest-Only counting with SVM disabled
It turned out that a performance counter on AMD does not count at all when the GO or HO bit is set in the control register and SVM is disabled in EFER.
This patch works around this issue by masking out the HO bit in the performance counter control register when SVM is not enabled.
The GO bit is not touched because it is only set when the user wants to count in guest-mode only. So when SVM is disabled the counter should not run at all and the not-counting is the intended behaviour.
--- arch/x86/include/asm/perf_event.h | 8 +++++++ arch/x86/kernel/cpu/perf_event.h | 8 +++++- arch/x86/kernel/cpu/perf_event_amd.c | 37 ++++++++++++++++++++++++++++++++- arch/x86/kvm/svm.c | 5 ++++ 4 files changed, 54 insertions(+), 4 deletions(-)
arch/x86/include/asm/perf_event.h +8 -0
@@ -242,4 +242,12 @@ static inline void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap)static inline void perf_events_lapic_init (void ) { }#endif +#if defined(CONFIG_PERF_EVENTS ) && defined(CONFIG_CPU_SUP_AMD ) +extern void amd_pmu_enable_virt (void ); +extern void amd_pmu_disable_virt (void ); +#else +static inline void amd_pmu_enable_virt (void ) { } +static inline void amd_pmu_disable_virt (void ) { } +#endif +#endif /* _ASM_X86_PERF_EVENT_H */
arch/x86/kernel/cpu/perf_event.h +6 -2
@@ -147,7 +147,9 @@ struct cpu_hw_events {/* * AMD specific bits */ -struct amd_nb *amd_nb; +struct amd_nb *amd_nb; +/* Inverted mask of bits to clear in the perf_ctr ctrl registers */ +u64 perf_ctr_virt_mask;void *kfree_on_online; };
@@ -417,9 +419,11 @@ void x86_pmu_disable_all(void);static inline void __x86_pmu_enable_event (struct hw_perf_event *hwc,u64 enable_mask) { +u64 disable_mask =__this_cpu_read (cpu_hw_events.perf_ctr_virt_mask); +if (hwc->extra_reg.reg)wrmsrl (hwc->extra_reg.reg, hwc->extra_reg.config); -wrmsrl (hwc->config_base, hwc->config | enable_mask); +wrmsrl (hwc->config_base, (hwc->config | enable_mask) & ~disable_mask); }void x86_pmu_enable_all (int added);
arch/x86/kernel/cpu/perf_event_amd.c +35 -2
@@ -1,4 +1,5 @@#include <linux/perf_event.h> +#include <linux/export.h> #include <linux/types.h> #include <linux/init.h> #include <linux/slab.h>
@@ -357,7 +358,9 @@ static void amd_pmu_cpu_starting(int cpu)struct amd_nb *nb;int i, nb_id; -if (boot_cpu_data.x86_max_cores <2 ) + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +if (boot_cpu_data.x86_max_cores <2 || boot_cpu_data.x86 ==0x15 )return ; nb_id =amd_get_nb_id (cpu);
@@ -587,9 +590,9 @@ static __initconst const struct x86_pmu amd_pmu_f15h = { .put_event_constraints = amd_put_event_constraints, .cpu_prepare = amd_pmu_cpu_prepare, - .cpu_starting = amd_pmu_cpu_starting, .cpu_dead = amd_pmu_cpu_dead, #endif + .cpu_starting = amd_pmu_cpu_starting, };__init int amd_pmu_init (void )
@@ -621,3 +624,33 @@ __init int amd_pmu_init(void)return 0 ; } + +void amd_pmu_enable_virt (void ) +{ +struct cpu_hw_events *cpuc = &__get_cpu_var (cpu_hw_events); + + cpuc->perf_ctr_virt_mask =0 ; + +/* Reload all events */ +x86_pmu_disable_all (); +x86_pmu_enable_all (0 ); +} +EXPORT_SYMBOL_GPL (amd_pmu_enable_virt); + +void amd_pmu_disable_virt (void ) +{ +struct cpu_hw_events *cpuc = &__get_cpu_var (cpu_hw_events); + +/* +* We only mask out the Host-only bit so that host-only counting works +* when SVM is disabled. If someone sets up a guest-only counter when +* SVM is disabled the Guest-only bits still gets set and the counter +* will not count anything. +*/ + cpuc->perf_ctr_virt_mask =AMD_PERFMON_EVENTSEL_HOSTONLY ; + +/* Reload all events */ +x86_pmu_disable_all (); +x86_pmu_enable_all (0 ); +} +EXPORT_SYMBOL_GPL (amd_pmu_disable_virt);
arch/x86/kvm/svm.c +5 -0
@@ -29,6 +29,7 @@#include <linux/ftrace_event.h> #include <linux/slab.h> +#include <asm/perf_event.h> #include <asm/tlbflush.h> #include <asm/desc.h> #include <asm/kvm_para.h>
@@ -575,6 +576,8 @@ static void svm_hardware_disable(void *garbage)wrmsrl (MSR_AMD64_TSC_RATIO ,TSC_RATIO_DEFAULT );cpu_svm_disable (); + +amd_pmu_disable_virt (); }static int svm_hardware_enable (void *garbage)
@@ -622,6 +625,8 @@ static int svm_hardware_enable(void *garbage)svm_init_erratum_383 (); +amd_pmu_enable_virt (); +return 0 ; }