perfmon: Intel architectural PMU support (x86)
PATCH 21/24 perfmon: Intel architectural PMU support (x86)
This patch adds Intel architectural PMU support (version 1, 2, and 3).
--
arch/x86/perfmon/Makefile +1 -0
@@ -3,3 +3,4 @@ # Contributed by Stephane Eranian <[email protected]> # obj-$(CONFIG_PERFMON) += perfmon.o +obj-$(CONFIG_X86_PERFMON_INTEL_ARCH) += perfmon_intel_arch.o
arch/x86/perfmon/perfmon_intel_arch.c new file +628 -0
628 lines not on this page · raw message
arch/x86/perfmon/Kconfig +7 -0
@@ -15,4 +15,11 @@ help Enables perfmon debugging support +config X86_PERFMON_INTEL_ARCH + bool "Support for Intel architectural perfmon v1/v2/v3" + depends on PERFMON + default n + help + Enables support for Intel architectural performance counters. + This feature was introduced with Intel Core Solo/Core Duo processors. endmenu
Quoted 1 line · On Wed, 26 Nov 2008, [email protected] wrote:
+static u64 enable_mask[PFM_MAX_PMCS];
Why do we need enable_mask twice for AMD and Intel ?
arch/x86/perfmon/perfmon_intel_arch.c line 34
Quoted 4 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +4
-0+static u16 max_enable; +static int pfm_intel_arch_version; + +DEFINE_PER_CPU (u64, saved_global_ctrl);
static
arch/x86/perfmon/perfmon_intel_arch.c line 72
Quoted 9 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +9
-0+/* +* layout of EAX for CPUID.0xa leaf function +*/ +struct pmu_eax { +unsigned int version:8 ;/* architectural perfmon version */ +unsigned int num_cnt:8 ;/* number of generic counters */ +unsigned int cnt_width:8 ;/* width of generic counters */ +unsigned int ebx_length:8 ;/* number of architected events */ +};
in arch/x86/include/asm/intel_arch_perfmon.h we have already:
union cpuid10_eax {
struct {
unsigned int version_id:8;
unsigned int num_counters:8;
unsigned int bit_width:8;
unsigned int mask_length:8;
} split;
unsigned int full;
};Can we either use this or remove it ?
Quoted 16 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +15
-0+/* +* layout of EDX for CPUID.0xa leaf function when perfmon v2 is detected +*/ +struct pmu_edx { +unsigned int num_cnt:5 ;/* number of fixed counters */ +unsigned int cnt_width:8 ;/* width of fixed counters */ +unsigned int reserved:19 ; +}; +static void pfm_intel_arch_acquire_pmu_percpu (void ); +static void pfm_intel_arch_release_pmu_percpu (void ); +static int pfm_intel_arch_stop_save (struct pfm_context *ctx, +struct pfm_event_set *set); +static int pfm_intel_arch_has_ovfls (struct pfm_context *ctx); +static void __kprobes pfm_intel_arch_quiesce (void ); +
arch/x86/perfmon/perfmon_intel_arch.c line 99
Quoted 10 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +10
-0+/* +* physical addresses of MSR controlling the perfevtsel and counter registers +*/ +struct pfm_arch_pmu_info pfm_intel_arch_pmu_info = { + .stop_save = pfm_intel_arch_stop_save, + .has_ovfls = pfm_intel_arch_has_ovfls, + .quiesce = pfm_intel_arch_quiesce, + .acquire_pmu_percpu = pfm_intel_arch_acquire_pmu_percpu, + .release_pmu_percpu = pfm_intel_arch_release_pmu_percpu +};
A) static
B) Please move it to the bottom to avoid all the forward declarations.
Quoted 1 line
+static void
pfm_intel_arch_check_errata(void)
__init ?
Quoted 1 line
+static void pfm_intel_arch_setup_generic(unsigned int version,
Ditto.
Quoted 1 line
+static void pfm_intel_arch_setup_fixed(unsigned int version,
Ditto.
Quoted 1 line
+static int
pfm_intel_arch_probe_pmu(void)
Ditto.
arch/x86/perfmon/perfmon_intel_arch.c line 263
Quoted 3 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +3
-0+/* +* handle version new anythread bit (bit 2) +*/
-ENOPARSE
arch/x86/perfmon/perfmon_intel_arch.c line 265
Quoted 2 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +2
-0+if (version ==3 ) + rsvd =1ULL <<3 ;
This sets bit 3
arch/x86/perfmon/perfmon_intel_arch.c line 267
Quoted 2 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +2
-0+else + rsvd =3ULL <<2 ;
And this sets bit 2 and 3.
Quoted 5 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +5
-0+static int pfm_intel_arch_stop_save (struct pfm_context *ctx, +struct pfm_event_set *set) +{ +u64 used_mask[PFM_PMC_BV ]; +u64 val, wmask, ovfl_mask;
arch/x86/perfmon/perfmon_intel_arch.c line 445
Quoted 10 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +10
-0+u32 i, count; + + wmask =1ULL << pfm_pmu_conf->counter_width; + +pfm_arch_bv_and (used_mask, + set->used_pmcs, + enable_mask, + max_enable); + + count =pfm_arch_bv_weight (used_mask, max_enable);
So we have:
set->used_pmcs and enable_mask and max_enable.
Why can set->used_pmcs contain bits which are not in the enable_mask in the first place ? Why does the arch code not tell the generic code which pmcs are available so we can avoid all this mask, weight whatever magic ?
We store the same information in slightly different incarnations in various places and then we need to mingle them all together to get to the real data. That makes no sense at all.
Quoted 8 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +8
-0+/* +* stop monitoring +* Unfortunately, this is very expensive! +* wrmsrl() is serializing. +*/ +for (i =0 ; count; i++) { +if (pfm_arch_bv_test_bit (i, used_mask)) { +wrmsrl (pfm_pmu_conf->pmc_desc[i].hw_addr,0 );
arch/x86/perfmon/perfmon_intel_arch.c line 464
Quoted 10 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +10
-0+ count--; + } + } + +/* +* if we already having a pending overflow condition, we simply +* return to take care of this first. +*/ +if (set->npend_ovfls) +return 1 ;
Why are the counters enabled at all when an overflow is pending, which stopped the counters anyway ?
Thanks,
tglx
Thomas,
Quoted 6 lines · Thomas Gleixner
Quoted 1 line · On Wed, 26 Nov 2008, [email protected] wrote:
+static u64 enable_mask[PFM_MAX_PMCS];
Why do we need
enable_masktwice for AMD and Intel ?
Ok, some explanations are needed here.
Both perfmon_intel_arch.c and perfmon_amd64.c are supposed to be kernel modules. They are hardcoded right now to make the patch simpler. Have PMU description be modules is a key features because it allows adding new processor support without necessarily patch the core kernel or rebooting. This has been working nicely on Itanium.
With the introduction of Intel architectural Perfmon (IA32 SDM chapter 18), this becomes
possible on Intel X86 as well. on AMD64 processor, this is not really an issue as they
all use the same architected PMU, except for family 10h which has nice extensions.
In anycase, the idea is to encapsulate as much as possible code related into a PMU model
into each module. That is why you are seing some redundancy.
There is a difference between enable_mask and used_pmcs. The used_pmcs bitmasks shows
all the config registers in use. Whereas enable_mask shows the all config registers which have
start/stop capabilities. For the basic AMD64 PMU (4 counters) used_pmcs and enable_mask
are equivalent, but that is not the case on Barcelona once we support IBS and sampling. So
for now, I could clean this up and drop enable_mask to use plain used_pmcs.
Quoted 111 lines
arch/x86/perfmon/perfmon_intel_arch.c line 34
Quoted 4 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +4
-0+static u16 max_enable; +static int pfm_intel_arch_version; + +DEFINE_PER_CPU (u64, saved_global_ctrl);static
arch/x86/perfmon/perfmon_intel_arch.c line 72
Quoted 9 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +9
-0+/* +* layout of EAX for CPUID.0xa leaf function +*/ +struct pmu_eax { +unsigned int version:8 ;/* architectural perfmon version */ +unsigned int num_cnt:8 ;/* number of generic counters */ +unsigned int cnt_width:8 ;/* width of generic counters */ +unsigned int ebx_length:8 ;/* number of architected events */ +};in
arch/x86/include/asm/intel_arch_perfmon.hwe have already:union cpuid10_eax { struct { unsigned int version_id:8; unsigned int num_counters:8; unsigned int bit_width:8; unsigned int mask_length:8; } split; unsigned int full; };Can we either use this or remove it ?
Quoted 16 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +15
-0+/* +* layout of EDX for CPUID.0xa leaf function when perfmon v2 is detected +*/ +struct pmu_edx { +unsigned int num_cnt:5 ;/* number of fixed counters */ +unsigned int cnt_width:8 ;/* width of fixed counters */ +unsigned int reserved:19 ; +}; +static void pfm_intel_arch_acquire_pmu_percpu (void ); +static void pfm_intel_arch_release_pmu_percpu (void ); +static int pfm_intel_arch_stop_save (struct pfm_context *ctx, +struct pfm_event_set *set); +static int pfm_intel_arch_has_ovfls (struct pfm_context *ctx); +static void __kprobes pfm_intel_arch_quiesce (void ); +arch/x86/perfmon/perfmon_intel_arch.c line 99
Quoted 10 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +10
-0+/* +* physical addresses of MSR controlling the perfevtsel and counter registers +*/ +struct pfm_arch_pmu_info pfm_intel_arch_pmu_info = { + .stop_save = pfm_intel_arch_stop_save, + .has_ovfls = pfm_intel_arch_has_ovfls, + .quiesce = pfm_intel_arch_quiesce, + .acquire_pmu_percpu = pfm_intel_arch_acquire_pmu_percpu, + .release_pmu_percpu = pfm_intel_arch_release_pmu_percpu +};A) static
B) Please move it to the bottom to avoid all the forward declarations.Quoted 1 line
+static void
pfm_intel_arch_check_errata(void)
__init?Quoted 1 line
+static void pfm_intel_arch_setup_generic(unsigned int version,
Ditto.
Quoted 1 line
+static void pfm_intel_arch_setup_fixed(unsigned int version,
Ditto.
Quoted 1 line
+static int
pfm_intel_arch_probe_pmu(void)Ditto.
arch/x86/perfmon/perfmon_intel_arch.c line 263
Quoted 3 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +3
-0+/* +* handle version new anythread bit (bit 2) +*/
-ENOPARSEarch/x86/perfmon/perfmon_intel_arch.c line 265
Quoted 2 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +2
-0+if (version ==3 ) + rsvd =1ULL <<3 ;This sets bit 3
arch/x86/perfmon/perfmon_intel_arch.c line 267
Quoted 2 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +2
-0+else + rsvd =3ULL <<2 ;And this sets bit 2 and 3.
Quoted 5 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +5
-0+static int pfm_intel_arch_stop_save (struct pfm_context *ctx, +struct pfm_event_set *set) +{ +u64 used_mask[PFM_PMC_BV ]; +u64 val, wmask, ovfl_mask;arch/x86/perfmon/perfmon_intel_arch.c line 445
Quoted 10 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +10
-0+u32 i, count; + + wmask =1ULL << pfm_pmu_conf->counter_width; + +pfm_arch_bv_and (used_mask, + set->used_pmcs, + enable_mask, + max_enable); + + count =pfm_arch_bv_weight (used_mask, max_enable);So we have:
Quoted 7 lines
set->used_pmcsandenable_maskandmax_enable.Why can
set->used_pmcscontain bits which are not in theenable_maskin the first place ? Why does the arch code not tell the generic code which pmcs are available so we can avoid all this mask, weight whatever magic ?
Because used_pmcs is part of generic code and enable_mask is a x86 construct. As I said above, for now, I could drop enable_mask.
The arch code already export the list of available pmcs and pmds in impl_pmcs and impl_pmds.
There is a difference between impl_pmcs and used_pmcs. The former list everything that is
available. The latter shows what we are currently using. We may be using fewer registers than
what is available, and we use this information to avoid saving/restoring MSR on context switch
for instance.
Quoted 23 lines
We store the same information in slightly different incarnations in various places and then we need to mingle them all together to get to the real data. That makes no sense at all.
Quoted 8 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +8
-0+/* +* stop monitoring +* Unfortunately, this is very expensive! +* wrmsrl() is serializing. +*/ +for (i =0 ; count; i++) { +if (pfm_arch_bv_test_bit (i, used_mask)) { +wrmsrl (pfm_pmu_conf->pmc_desc[i].hw_addr,0 );arch/x86/perfmon/perfmon_intel_arch.c line 464
Quoted 10 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +10
-0+ count--; + } + } + +/* +* if we already having a pending overflow condition, we simply +* return to take care of this first. +*/ +if (set->npend_ovfls) +return 1 ;
Quoted 3 lines
Why are the counters enabled at all when an overflow is pending, which stopped the counters anyway ?
Because on Intel and AMD64, counters are not automatically frozen on interrupt. On Intel X86, they can be configured to do so, but it is an all or nothing setting.
I am not using this option because we would then have a problem with the NMI watchdog given that it is also using a counter.
Quoted 3 lines · stephane eranian
In anycase, the idea is to encapsulate as much as possible code related into a PMU model
into each module. That is why you are seing some redundancy.
Makes sense.
Quoted 9 lines
There is a difference between
enable_maskandused_pmcs. Theused_pmcsbitmasks shows
all the config registers in use. Whereasenable_maskshows the all config registers which have
start/stop capabilities. For the basic AMD64 PMU (4 counters)used_pmcsandenable_mask
are equivalent, but that is not the case on Barcelona once we support IBS and sampling. So
for now, I could clean this up and dropenable_maskto use plainused_pmcs.
Understood. If we need that in the near future then it's ok to keep it, it just did not make any sense from the current code.
But I think you should do this once when you set up the context and keep that as a separate mask. Right now you evaluate enable_mask and used_pmcs over and over again.
Quoted 12 lines
Quoted 11 lines
Quoted 1 line
+ count =
pfm_arch_bv_weight(used_mask, max_enable);So we have:
set->used_pmcsandenable_maskandmax_enable.Why can
set->used_pmcscontain bits which are not in theenable_maskin the first place ? Why does the arch code not tell the generic code which pmcs are available so we can avoid all this mask, weight whatever magic ?
Quoted 4 lines
Because
used_pmcsis part of generic code andenable_maskis a x86 construct. As I said above, for now, I could dropenable_mask.
The arch code already export the list of available pmcs and pmds inimpl_pmcsandimpl_pmds.
See above.
Quoted 8 lines
Quoted 3 lines
Why are the counters enabled at all when an overflow is pending, which stopped the counters anyway ?
Because on Intel and AMD64, counters are not automatically frozen on interrupt. On Intel X86, they can be configured to do so, but it is an all or nothing setting.
I am not using this option because we would then have a problem with the NMI watchdog given that it is also using a counter.
Well, my question was: why do we have to stop the counters when an overflow is pending already ?
The overflow pending is set inside of stop_save() and cleared somewhere else.
stop_save() is called from pfm_arch_stop() and pfm_arch_ctxswout_thread(). The first thing it does is to disable the counters.
Now at some points the counters are obviously reenabled for this context, but why are they reenabled _before_ the pending overflow has been resolved ? For N counters that N * 2 wrmsrl() overhead.
Thanks,
tglx
Quoted 4 lines
Both
perfmon_intel_arch.candperfmon_amd64.care supposed to be kernel modules. They are hardcoded right now to make the patch simpler. Have PMU description be modules is a key features because it allows adding new processor support without necessarily patch the core kernel or rebooting. This has been working
To install the new processor in the first place you have to reboot anyways.
Also typically at least for new families (which tend to be the only ones with radically new performance counters) it's typically needed to change some things in the core kernel anyways. So this doesn't seem like a really useful feature.
Quoted 4 lines
nicely on Itanium.
With the introduction of Intel architectural Perfmon (IA32 SDM chapter 18), this becomes
possible on Intel X86 as well.
It becomes possible, but without having to use any modules. It should just work.
Probably even without it worked -- at least if you limit yourself to family 6 -- because the register layout all stayed the same too.
That said having modular PMUs is probably a good thing for distribution kernels, but there is really no need for any code compromises just to avoid a core kernel patch now and then.
-Andi
Quoted 4 lines · Thomas Gleixner
Quoted 1 line · On Wed, 26 Nov 2008, [email protected] wrote:
+static u64 enable_mask[PFM_MAX_PMCS];
Quoted 9 lines
Why do we need
enable_masktwice for AMD and Intel ?arch/x86/perfmon/perfmon_intel_arch.c line 34
Quoted 4 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +4
-0+static u16 max_enable; +static int pfm_intel_arch_version; + +DEFINE_PER_CPU (u64, saved_global_ctrl);static
Why you want this static instead of per-cpu?
Quoted 22 lines
arch/x86/perfmon/perfmon_intel_arch.c line 72
Quoted 9 lines
arch/x86/perfmon/perfmon_intel_arch.c new file +9
-0+/* +* layout of EAX for CPUID.0xa leaf function +*/ +struct pmu_eax { +unsigned int version:8 ;/* architectural perfmon version */ +unsigned int num_cnt:8 ;/* number of generic counters */ +unsigned int cnt_width:8 ;/* width of generic counters */ +unsigned int ebx_length:8 ;/* number of architected events */ +};in
arch/x86/include/asm/intel_arch_perfmon.hwe have already:union cpuid10_eax { struct { unsigned int version_id:8; unsigned int num_counters:8; unsigned int bit_width:8; unsigned int mask_length:8; } split; unsigned int full; };
Quoted 2 lines
Can we either use this or remove it ?
Well, I need more than eax. We could rewrite this union to include eax, edx, So I propose we call it union cpuid10 and define it as:
union cpuid_eax {
struct {
unsigned int version_id:8;
unsigned int num_counters:8;
unsigned int bit_width:8;
unsigned int mask_length:8;
} split_eax;
struct {
unsigned int num_counters:5;
unsigned int bit_width:8;
unsigned int reserved:19;
} split_edx;
unsigned int full;
}