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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.