From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Lin Ming <ming.m.lin@intel.com>
Cc: Ingo Molnar <mingo@elte.hu>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Robert Richter <robert.richter@amd.com>,
lkml <linux-kernel@vger.kernel.org>
Subject: [RFC -tip] perf, x86: P4 PMU -- Address erratum 15 and 17
Date: Fri, 04 Mar 2011 22:48:12 +0300 [thread overview]
Message-ID: <4D7141FC.8000504@gmail.com> (raw)
Errata N15 and 17 of 249199-071 should be taken
into account. They are mostly hard to hit at moment
i believe but still better to be fixed.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
Ming, mind to give it a shot? I seems will be able to test myself
at Wednesday only. And I would like to get a review first to eliminate
some silly mistakes ;)
arch/x86/kernel/cpu/perf_event_p4.c | 54 ++++++++++++++++++++++++++++++++++--
1 file changed, 51 insertions(+), 3 deletions(-)
Index: linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/perf_event_p4.c
+++ linux-2.6.git/arch/x86/kernel/cpu/perf_event_p4.c
@@ -809,15 +809,51 @@ static void p4_pmu_disable_pebs(void)
static inline void p4_pmu_disable_event(struct perf_event *event)
{
struct hw_perf_event *hwc = &event->hw;
+ unsigned int cntr_msr;
+ u32 prev_lo, prev_hi, new_lo, new_hi;
+
+ cntr_msr = hwc->event_base + hwc->idx;
+
+ /*
+ * Erratum N17 of 249199-071 says if a performance counter is
+ * stopped on the precise internal clock cycle where the intermediate
+ * carry from the lower 32 bits of the counter to the upper eight
+ * bits occurs, the intermediate carry is lost.
+ *
+ * As a workaround we read before stop and check for lost carry
+ * bit, if it get lost simply write previous value back, this is
+ * of course might introduce a delta in precise counting but still
+ * it's a way better than 2^32 magnitude lost.
+ */
+ rdmsr(cntr_msr, prev_lo, prev_hi);
/*
* If event gets disabled while counter is in overflowed
* state we need to clear P4_CCCR_OVF, otherwise interrupt get
* asserted again and again
+ *
+ * Erratum N15 of 249199-071 says we are to clear P4_CCCR_COMPARE
+ * otherwise writing a performance counter may result in incorrect
+ * value (to be sure we do a double write later) but since
+ * we're modifying CCCR anyway better to take this bit into account
+ * just to be double safe. Note we don't touch the former
+ * config so no affects on user supplied data.
*/
(void)checking_wrmsrl(hwc->config_base,
(u64)(p4_config_unpack_cccr(hwc->config)) &
- ~P4_CCCR_ENABLE & ~P4_CCCR_OVF & ~P4_CCCR_RESERVED);
+ ~P4_CCCR_COMPARE & ~P4_CCCR_ENABLE &
+ ~P4_CCCR_OVF & ~P4_CCCR_RESERVED);
+
+ /*
+ * Lets try to recover from error if happened
+ */
+ if (prev_lo == -1U) {
+ rdmsr(cntr_msr, new_lo, new_hi);
+ if (new_lo == 0 && (new_hi - prev_hi) == 0) {
+ wrmsr_safe(cntr_msr, prev_lo, prev_hi);
+ printk_once("P4 PMU: Recover lost carry bit\n");
+ }
+ }
}
static void p4_pmu_disable_all(void)
@@ -841,6 +877,16 @@ static void p4_pmu_enable_pebs(u64 confi
struct p4_pebs_bind *bind;
unsigned int idx;
+ /*
+ * NOTE: There is an errata says the full PEBS support
+ * requires to check if associated counting logic if properly
+ * configured, in short -- if an event requires some
+ * additional uops tagging and friends it *must* be guaranted
+ * the tagging is done properly otherwise the results are
+ * unknown, for while there is no classic PEBS support but better
+ * to keep this (potential) problem explicitly marked
+ */
+
BUILD_BUG_ON(P4_PEBS_METRIC__max > P4_PEBS_CONFIG_METRIC_MASK);
idx = p4_config_unpack_metric(config);
@@ -866,8 +912,10 @@ static void p4_pmu_enable_event(struct p
escr_addr = (u64)bind->escr_msr[thread];
/*
- * - we dont support cascaded counters yet
- * - and counter 1 is broken (erratum)
+ * In a sake of erratum:
+ * - cascaded counters do not work properly with
+ * force overflow flag set but take it wider
+ * - counter 1 is broken
*/
WARN_ON_ONCE(p4_is_event_cascaded(hwc->config));
WARN_ON_ONCE(hwc->idx == 1);
next reply other threads:[~2011-03-04 19:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-04 19:48 Cyrill Gorcunov [this message]
2011-03-04 20:05 ` [RFC -tip] perf, x86: P4 PMU -- Address erratum 15 and 17 Joe Perches
2011-03-04 20:34 ` Cyrill Gorcunov
2011-03-04 20:35 ` Cyrill Gorcunov
2011-03-05 14:14 ` Lin Ming
2011-03-05 14:49 ` Cyrill Gorcunov
2011-03-07 8:27 ` Lin Ming
2011-03-07 8:40 ` Cyrill Gorcunov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D7141FC.8000504@gmail.com \
--to=gorcunov@gmail.com \
--cc=a.p.zijlstra@chello.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.m.lin@intel.com \
--cc=mingo@elte.hu \
--cc=robert.richter@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox