linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support
@ 2016-06-21 18:31 David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 1/5] perf/x86/intel: output LBR support statement after validation David Carrillo-Cisneros
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: David Carrillo-Cisneros @ 2016-06-21 18:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: x86, Ingo Molnar, Andi Kleen, Kan Liang, Peter Zijlstra,
	David Carrillo-Cisneros

commit 338b522ca43c ("perf/x86/intel: Protect LBR and extra_regs against
KVM lying")
introduced an extra test for LBR support but did not move the dmesg
accordingly. This problem is fixed in first patch in this series.

When a machine that used LBR is rebooted using kexec, the extra test
for LBR support may fail due to a hw bug/quirk in Haswell that generates
a #GPF when restoring a value of MSR_LAST_BRANCH_FROM_* msrs that
has sign extension (e.g. kernel addresses). This hw bug/quirk currently
does not manifest in the context switch of LBR callstack mode because of
a workaround for another LBR bug (bug in FREEZE_LBRS_ON_PMI,
more details in second patch of this series). The workaround deactivates
LBR callstack in kernel mode.

The second and fourth patches in this series contain workarounds for the
MSR_LAST_BRANCH_FROM_* hw bug/quirk.

The third patch contains a trivial format fix for aesthetic uniformity.

The last patch is not to be committed, but to test the fourth patch. It
removes the effect of the FREEZE_LBRS_ON_PMI work-around by allowing
LBR callstack for kernel addresses.

This series is rebased at torvalds/linux/master .


Changes in 2nd version:
  - Remove branch from quirk (as pointed by Peter Z.).
  - Format fixes.

David Carrillo-Cisneros (5):
  perf/x86/intel: output LBR support statement after validation
  perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX
  perf/x86/intel: trivial format and style fix
  perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch
  not required, used to test ctxsw, do not merge

 arch/x86/events/intel/core.c | 20 ++++++++++
 arch/x86/events/intel/lbr.c  | 90 ++++++++++++++++++++++++++++++++++++--------
 arch/x86/events/perf_event.h |  2 +
 tools/perf/util/evsel.c      | 17 +++++++--
 4 files changed, 111 insertions(+), 18 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v02 1/5] perf/x86/intel: output LBR support statement after validation
  2016-06-21 18:31 [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support David Carrillo-Cisneros
@ 2016-06-21 18:31 ` David Carrillo-Cisneros
  2016-06-27 12:54   ` [tip:perf/core] perf/x86/intel: Print " tip-bot for David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX David Carrillo-Cisneros
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: David Carrillo-Cisneros @ 2016-06-21 18:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: x86, Ingo Molnar, Andi Kleen, Kan Liang, Peter Zijlstra,
	David Carrillo-Cisneros

commit 338b522ca43c ("perf/x86/intel: Protect LBR and extra_regs against KVM lying")
added an additional test to LBR support detection that is performed after
printing the LBR support statement to dmesg.

Move the LBR support output after the very last test.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/events/intel/core.c | 2 ++
 arch/x86/events/intel/lbr.c  | 9 ---------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 7c66695..a5e52ad4 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3885,6 +3885,8 @@ __init int intel_pmu_init(void)
 			x86_pmu.lbr_nr = 0;
 	}
 
+	if (x86_pmu.lbr_nr)
+		pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
 	/*
 	 * Access extra MSR may cause #GP under certain circumstances.
 	 * E.g. KVM doesn't support offcore event
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 9e2b40c..2dca66c 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -956,7 +956,6 @@ void __init intel_pmu_lbr_init_core(void)
 	 * SW branch filter usage:
 	 * - compensate for lack of HW filter
 	 */
-	pr_cont("4-deep LBR, ");
 }
 
 /* nehalem/westmere */
@@ -977,7 +976,6 @@ void __init intel_pmu_lbr_init_nhm(void)
 	 *   That requires LBR_FAR but that means far
 	 *   jmp need to be filtered out
 	 */
-	pr_cont("16-deep LBR, ");
 }
 
 /* sandy bridge */
@@ -997,7 +995,6 @@ void __init intel_pmu_lbr_init_snb(void)
 	 *   That requires LBR_FAR but that means far
 	 *   jmp need to be filtered out
 	 */
-	pr_cont("16-deep LBR, ");
 }
 
 /* haswell */
@@ -1010,8 +1007,6 @@ void intel_pmu_lbr_init_hsw(void)
 
 	x86_pmu.lbr_sel_mask = LBR_SEL_MASK;
 	x86_pmu.lbr_sel_map  = hsw_lbr_sel_map;
-
-	pr_cont("16-deep LBR, ");
 }
 
 /* skylake */
@@ -1031,7 +1026,6 @@ __init void intel_pmu_lbr_init_skl(void)
 	 *   That requires LBR_FAR but that means far
 	 *   jmp need to be filtered out
 	 */
-	pr_cont("32-deep LBR, ");
 }
 
 /* atom */
@@ -1057,7 +1051,6 @@ void __init intel_pmu_lbr_init_atom(void)
 	 * SW branch filter usage:
 	 * - compensate for lack of HW filter
 	 */
-	pr_cont("8-deep LBR, ");
 }
 
 /* slm */
@@ -1088,6 +1081,4 @@ void intel_pmu_lbr_init_knl(void)
 
 	x86_pmu.lbr_sel_mask = LBR_SEL_MASK;
 	x86_pmu.lbr_sel_map  = snb_lbr_sel_map;
-
-	pr_cont("8-deep LBR, ");
 }
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX
  2016-06-21 18:31 [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 1/5] perf/x86/intel: output LBR support statement after validation David Carrillo-Cisneros
@ 2016-06-21 18:31 ` David Carrillo-Cisneros
  2016-06-21 22:54   ` Andi Kleen
  2016-06-27 12:54   ` [tip:perf/core] perf/x86/intel: Fix " tip-bot for David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 3/5] perf/x86/intel: trivial format and style fix David Carrillo-Cisneros
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: David Carrillo-Cisneros @ 2016-06-21 18:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: x86, Ingo Molnar, Andi Kleen, Kan Liang, Peter Zijlstra,
	David Carrillo-Cisneros

Intel's SDM states that bits 61:62 in MSR_LAST_BRANCH_FROM_x are the
TSX flags for formats with LBR_TSX flags (i.e. LBR_FORMAT_EIP_EFLAGS2).

However, when the CPU has TSX support deactivated, bits 61:62 actually
behave as follows:
  - For wrmsr, bits 61:62 are considered part of the sign extension.
  - When capturing branches, the LBR hw will always clear bits 61:62.
  regardless of the sign extension.

Therefore, if:
  1) LBR has TSX format.
  2) CPU has no TSX support enabled.
then any value passed to wrmsr must be sign extended to 63 bits and any
value from rdmsr must be converted to have a sign extension of 61 bits,
ignoring the values at TSX flags.

This bug was masked by the work-around to the Intel's CPU bug:
BJ94. "LBR May Contain Incorrect Information When Using FREEZE_LBRS_ON_PMI"
in Document Number: 324643-037US.

The aforementioned work-around uses hw flags to filter out all kernel
branches, limiting LBR callstack to user level execution only.
Since user addresses are not sign extended, they do not trigger the wrmsr
bug in MSR_LAST_BRANCH_FROM_x when saved/restored at context switch.

To verify the hw bug:

  $ perf record -b -e cycles sleep 1
  $ rdmsr -p 0 0x680
  0x1fffffffb0b9b0cc
  $ wrmsr -p 0 0x680 0x1fffffffb0b9b0cc
  write(): Input/output error

The quirk for LBR_FROM_ msrs is required before calls to wrmsrl and
after rdmsrl.

This patch introduces it for wrmsrl's done for testing LBR support.
Future patch in series adds the quirk for context switch, that would
be required if LBR callstack is to be enabled for ring 0.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/events/intel/core.c | 18 ++++++++++++++++
 arch/x86/events/intel/lbr.c  | 49 ++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/events/perf_event.h |  2 ++
 3 files changed, 69 insertions(+)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index a5e52ad4..eb3a26a 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3302,6 +3302,13 @@ static void intel_snb_check_microcode(void)
 	}
 }
 
+static bool is_lbr_from(unsigned long msr)
+{
+	unsigned long lbr_from_nr = x86_pmu.lbr_from + x86_pmu.lbr_nr;
+
+	return x86_pmu.lbr_from <= msr && msr < lbr_from_nr;
+}
+
 /*
  * Under certain circumstances, access certain MSR may cause #GP.
  * The function tests if the input MSR can be safely accessed.
@@ -3322,13 +3329,24 @@ static bool check_msr(unsigned long msr, u64 mask)
 	 * Only change the bits which can be updated by wrmsrl.
 	 */
 	val_tmp = val_old ^ mask;
+
+	if (is_lbr_from(msr))
+		val_tmp = lbr_from_signext_quirk_wr(val_tmp);
+
 	if (wrmsrl_safe(msr, val_tmp) ||
 	    rdmsrl_safe(msr, &val_new))
 		return false;
 
+	/*
+	 * quirk only affects validation in wrmsr, so wrmsrl's value
+	 * should equal rdmsrl's even with the quirk.
+	 */
 	if (val_new != val_tmp)
 		return false;
 
+	if (is_lbr_from(msr))
+		val_old = lbr_from_signext_quirk_wr(val_old);
+
 	/* Here it's sure that the MSR can be safely accessed.
 	 * Restore the old value and return.
 	 */
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 2dca66c..3a2c67d 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -81,6 +81,8 @@ static enum {
 #define LBR_FROM_FLAG_IN_TX    (1ULL << 62)
 #define LBR_FROM_FLAG_ABORT    (1ULL << 61)
 
+#define LBR_FROM_SIGNEXT_2MSB	(BIT_ULL(60) | BIT_ULL(59))
+
 /*
  * x86control flow change classification
  * x86control flow changes include branches, interrupts, traps, faults
@@ -235,6 +237,50 @@ enum {
 	LBR_VALID,
 };
 
+/*
+ * For formats with LBR_TSX flags (e.g. LBR_FORMAT_EIP_FLAGS2), bits 61:62 in
+ * MSR_LAST_BRANCH_FROM_x are the TSX flags when TSX is supported, but when
+ * TSX is not supported they have no consistent behavior:
+ *   - For wrmsr, bits 61:62 are considered part of the sign extension.
+ *   - For HW updates (branch captures) bits 61:62 are always OFF and are not
+ *  part of the sign extension.
+ *
+ * Therefore, if:
+ *   1) LBR has TSX format
+ *   2) CPU has no TSX support enabled
+ * then any value passed to wrmsr must be sign extended to 63 bits and any
+ * value from rdmsr must be converted to have a 61 bits sign extension,
+ * ignoring the TSX flags.
+ */
+
+static inline bool lbr_from_signext_quirk_needed(void)
+{
+	int lbr_format = x86_pmu.intel_cap.lbr_format;
+	bool tsx_support = boot_cpu_has(X86_FEATURE_HLE) ||
+			   boot_cpu_has(X86_FEATURE_RTM);
+
+	return !tsx_support && (lbr_desc[lbr_format] & LBR_TSX);
+}
+
+DEFINE_STATIC_KEY_FALSE(lbr_from_quirk_key);
+
+/*If quirk is enabled, ensure sign extension is 63 bits. */
+inline u64 lbr_from_signext_quirk_wr(u64 val)
+{
+	if (static_branch_unlikely(&lbr_from_quirk_key))
+		/*
+		 * Sign extend into bits 61:62 while preserving bit 63.
+		 *
+		 * Quirk is enabled when TSX is disabled. Therefore TSX bits
+		 * in val are always OFF and must be changed to be sign
+		 * extension bits. Since bits 59:60 are guaranteed to be
+		 * part of the sign extension bits, we can just copy them
+		 * to 61:62.
+		 */
+		val |= (LBR_FROM_SIGNEXT_2MSB & val) << 2;
+	return val;
+}
+
 static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 {
 	int i;
@@ -1007,6 +1053,9 @@ void intel_pmu_lbr_init_hsw(void)
 
 	x86_pmu.lbr_sel_mask = LBR_SEL_MASK;
 	x86_pmu.lbr_sel_map  = hsw_lbr_sel_map;
+
+	if (lbr_from_signext_quirk_needed())
+		static_branch_enable(&lbr_from_quirk_key);
 }
 
 /* skylake */
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 8bd764d..c934931 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -892,6 +892,8 @@ void intel_ds_init(void);
 
 void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in);
 
+u64 lbr_from_signext_quirk_wr(u64 val);
+
 void intel_pmu_lbr_reset(void);
 
 void intel_pmu_lbr_enable(struct perf_event *event);
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v02 3/5] perf/x86/intel: trivial format and style fix
  2016-06-21 18:31 [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 1/5] perf/x86/intel: output LBR support statement after validation David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX David Carrillo-Cisneros
@ 2016-06-21 18:31 ` David Carrillo-Cisneros
  2016-06-27 12:55   ` [tip:perf/core] perf/x86/intel: Fix trivial formatting and style bug tip-bot for David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 5/5] not required, used to test ctxsw, do not merge David Carrillo-Cisneros
  4 siblings, 1 reply; 12+ messages in thread
From: David Carrillo-Cisneros @ 2016-06-21 18:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: x86, Ingo Molnar, Andi Kleen, Kan Liang, Peter Zijlstra,
	David Carrillo-Cisneros

Replace spaces by tabs in LBR_FROM_* constants to align with newly
defined constant. Use BIT_ULL.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/events/intel/lbr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 3a2c67d..2ee5dde 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -77,9 +77,9 @@ static enum {
 	 LBR_IND_JMP	|\
 	 LBR_FAR)
 
-#define LBR_FROM_FLAG_MISPRED  (1ULL << 63)
-#define LBR_FROM_FLAG_IN_TX    (1ULL << 62)
-#define LBR_FROM_FLAG_ABORT    (1ULL << 61)
+#define LBR_FROM_FLAG_MISPRED	BIT_ULL(63)
+#define LBR_FROM_FLAG_IN_TX	BIT_ULL(62)
+#define LBR_FROM_FLAG_ABORT	BIT_ULL(61)
 
 #define LBR_FROM_SIGNEXT_2MSB	(BIT_ULL(60) | BIT_ULL(59))
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch
  2016-06-21 18:31 [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support David Carrillo-Cisneros
                   ` (2 preceding siblings ...)
  2016-06-21 18:31 ` [PATCH v02 3/5] perf/x86/intel: trivial format and style fix David Carrillo-Cisneros
@ 2016-06-21 18:31 ` David Carrillo-Cisneros
  2016-06-23  8:43   ` Peter Zijlstra
  2016-06-27 12:55   ` [tip:perf/core] perf/x86/intel: Add " tip-bot for David Carrillo-Cisneros
  2016-06-21 18:31 ` [PATCH v02 5/5] not required, used to test ctxsw, do not merge David Carrillo-Cisneros
  4 siblings, 2 replies; 12+ messages in thread
From: David Carrillo-Cisneros @ 2016-06-21 18:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: x86, Ingo Molnar, Andi Kleen, Kan Liang, Peter Zijlstra,
	David Carrillo-Cisneros

Add quirk for context switch to save/restore the value of
MSR_LAST_BRANCH_FROM_x when LBR is enabled and there is potential for
kernel addresses to be in the lbr_from register.

To test this patch, use a perf tool and kernel with the patch
next in this series. That patch removes the work around that masked
the hw bug:

  $ ./lbr_perf record --call-graph lbr -e cycles:k sleep 1

where lbr_perf is the patched perf tool, that allows to specify :k
on lbr mode. The above command will trigger a #GPF :

[  411.191445] ------------[ cut here ]------------
[  411.196015] WARNING: CPU: 28 PID: 14096 at arch/x86/mm/extable.c:65 ex_handler_wrmsr_unsafe+0x70/0x80
[  411.205123] unchecked MSR access error: WRMSR to 0x681 (tried to write 0x1fffffff81010794)
...
[  411.265962] Call Trace:
[  411.268384]  [<ffffffff8167af49>] dump_stack+0x4d/0x63
[  411.273462]  [<ffffffff810b9b15>] __warn+0xe5/0x100
[  411.278278]  [<ffffffff810b9be9>] warn_slowpath_fmt+0x49/0x50
[  411.283955]  [<ffffffff810abb40>] ex_handler_wrmsr_unsafe+0x70/0x80
[  411.290144]  [<ffffffff810abc42>] fixup_exception+0x42/0x50
[  411.295658]  [<ffffffff81079d1a>] do_general_protection+0x8a/0x160
[  411.301764]  [<ffffffff81684ec2>] general_protection+0x22/0x30
[  411.307527]  [<ffffffff810101b9>] ? intel_pmu_lbr_sched_task+0xc9/0x380
[  411.314063]  [<ffffffff81009d7c>] intel_pmu_sched_task+0x3c/0x60
[  411.319996]  [<ffffffff81003a2b>] x86_pmu_sched_task+0x1b/0x20
[  411.325762]  [<ffffffff81192a5b>] perf_pmu_sched_task+0x6b/0xb0
[  411.331610]  [<ffffffff8119746d>] __perf_event_task_sched_in+0x7d/0x150
[  411.338145]  [<ffffffff810dd9dc>] finish_task_switch+0x15c/0x200
[  411.344078]  [<ffffffff8167f894>] __schedule+0x274/0x6cc
[  411.349325]  [<ffffffff8167fdd9>] schedule+0x39/0x90
[  411.354229]  [<ffffffff81675398>] exit_to_usermode_loop+0x39/0x89
[  411.360246]  [<ffffffff810028ce>] prepare_exit_to_usermode+0x2e/0x30
[  411.366524]  [<ffffffff81683c1b>] retint_user+0x8/0x10
[  411.371599] ---[ end trace 1ed61b8a551e95d3 ]---

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/events/intel/lbr.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 2ee5dde..6cd7cc0 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -281,6 +281,21 @@ inline u64 lbr_from_signext_quirk_wr(u64 val)
 	return val;
 }
 
+/*
+ * If quirk is needed, ensure sign extension is 61 bits.
+ */
+
+u64 lbr_from_signext_quirk_rd(u64 val)
+{
+	if (static_branch_unlikely(&lbr_from_quirk_key))
+		/*
+		 * Quirk is on when TSX is not enabled. Therefore TSX
+		 * flags must be read as OFF.
+		 */
+		val &= ~(LBR_FROM_FLAG_IN_TX | LBR_FROM_FLAG_ABORT);
+	return val;
+}
+
 static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 {
 	int i;
@@ -297,7 +312,8 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
-		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
+		wrmsrl(x86_pmu.lbr_from + lbr_idx,
+			lbr_from_signext_quirk_wr(task_ctx->lbr_from[i]));
 		wrmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
@@ -310,7 +326,7 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 {
 	int i;
 	unsigned lbr_idx, mask;
-	u64 tos;
+	u64 tos, val;
 
 	if (task_ctx->lbr_callstack_users == 0) {
 		task_ctx->lbr_stack_state = LBR_NONE;
@@ -321,7 +337,8 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 	tos = intel_pmu_lbr_tos();
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
-		rdmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
+		rdmsrl(x86_pmu.lbr_from + lbr_idx, val);
+		task_ctx->lbr_from[i] = lbr_from_signext_quirk_rd(val);
 		rdmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
@@ -499,6 +516,8 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 		int lbr_flags = lbr_desc[lbr_format];
 
 		rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
+		from = lbr_from_signext_quirk_rd(from);
+
 		rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
 
 		if (lbr_format == LBR_FORMAT_INFO && need_info) {
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v02 5/5] not required, used to test ctxsw, do not merge
  2016-06-21 18:31 [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support David Carrillo-Cisneros
                   ` (3 preceding siblings ...)
  2016-06-21 18:31 ` [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch David Carrillo-Cisneros
@ 2016-06-21 18:31 ` David Carrillo-Cisneros
  4 siblings, 0 replies; 12+ messages in thread
From: David Carrillo-Cisneros @ 2016-06-21 18:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: x86, Ingo Molnar, Andi Kleen, Kan Liang, Peter Zijlstra,
	David Carrillo-Cisneros

DO NOT MERGE. Provided only to verify bug fix.

Change kernel and perf tool to activate tracking and context
switch for kernel branches.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 arch/x86/events/intel/lbr.c |  3 ++-
 tools/perf/util/evsel.c     | 17 ++++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 6cd7cc0..ef3e275a39 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -388,7 +388,8 @@ void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in)
 
 static inline bool branch_user_callstack(unsigned br_sel)
 {
-	return (br_sel & X86_BR_USER) && (br_sel & X86_BR_CALL_STACK);
+	return (br_sel & (X86_BR_USER | X86_BR_KERNEL)) &&
+	       (br_sel & X86_BR_CALL_STACK);
 }
 
 void intel_pmu_lbr_enable(struct perf_event *event)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5d7037e..b39aafc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -575,9 +575,20 @@ void perf_evsel__config_callchain(struct perf_evsel *evsel,
 	if (param->record_mode == CALLCHAIN_LBR) {
 		if (!opts->branch_stack) {
 			if (attr->exclude_user) {
-				pr_warning("LBR callstack option is only available "
-					   "to get user callchain information. "
-					   "Falling back to framepointers.\n");
+				/*
+				 * Sample kernel AND user branches. Do not exclude user
+				 * branches because task_ctx->lbr_callstack_users in
+				 * arch/x86/events/intel/lbr.c only count users for
+				 * user branches.
+				 */
+				pr_warning("BRANCH_STACK with kernel and user\n");
+				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
+				attr->branch_sample_type = PERF_SAMPLE_BRANCH_KERNEL |
+							PERF_SAMPLE_BRANCH_USER |
+							PERF_SAMPLE_BRANCH_CALL_STACK |
+							PERF_SAMPLE_BRANCH_NO_CYCLES |
+							PERF_SAMPLE_BRANCH_NO_FLAGS;
+
 			} else {
 				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
 				attr->branch_sample_type = PERF_SAMPLE_BRANCH_USER |
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX
  2016-06-21 18:31 ` [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX David Carrillo-Cisneros
@ 2016-06-21 22:54   ` Andi Kleen
  2016-06-27 12:54   ` [tip:perf/core] perf/x86/intel: Fix " tip-bot for David Carrillo-Cisneros
  1 sibling, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2016-06-21 22:54 UTC (permalink / raw)
  To: David Carrillo-Cisneros
  Cc: linux-kernel, x86, Ingo Molnar, Kan Liang, Peter Zijlstra

> This patch introduces it for wrmsrl's done for testing LBR support.
> Future patch in series adds the quirk for context switch, that would
> be required if LBR callstack is to be enabled for ring 0.

Patches are fine for me.

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

-Andi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch
  2016-06-21 18:31 ` [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch David Carrillo-Cisneros
@ 2016-06-23  8:43   ` Peter Zijlstra
  2016-06-27 12:55   ` [tip:perf/core] perf/x86/intel: Add " tip-bot for David Carrillo-Cisneros
  1 sibling, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2016-06-23  8:43 UTC (permalink / raw)
  To: David Carrillo-Cisneros
  Cc: linux-kernel, x86, Ingo Molnar, Andi Kleen, Kan Liang

On Tue, Jun 21, 2016 at 11:31:13AM -0700, David Carrillo-Cisneros wrote:
> @@ -297,7 +312,8 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
>  	tos = task_ctx->tos;
>  	for (i = 0; i < tos; i++) {
>  		lbr_idx = (tos - i) & mask;
> -		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
> +		wrmsrl(x86_pmu.lbr_from + lbr_idx,
> +			lbr_from_signext_quirk_wr(task_ctx->lbr_from[i]));
>  		wrmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
>  		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
>  			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
> @@ -321,7 +337,8 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
>  	tos = intel_pmu_lbr_tos();
>  	for (i = 0; i < tos; i++) {
>  		lbr_idx = (tos - i) & mask;
> -		rdmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
> +		rdmsrl(x86_pmu.lbr_from + lbr_idx, val);
> +		task_ctx->lbr_from[i] = lbr_from_signext_quirk_rd(val);
>  		rdmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
>  		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
>  			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
> @@ -499,6 +516,8 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
>  		int lbr_flags = lbr_desc[lbr_format];
>  
>  		rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
> +		from = lbr_from_signext_quirk_rd(from);
> +
>  		rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
>  
>  		if (lbr_format == LBR_FORMAT_INFO && need_info) {


I did this on top... 


--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -288,12 +288,42 @@ inline u64 lbr_from_signext_quirk_wr(u64
 
 u64 lbr_from_signext_quirk_rd(u64 val)
 {
-	if (static_branch_unlikely(&lbr_from_quirk_key))
+	if (static_branch_unlikely(&lbr_from_quirk_key)) {
 		/*
 		 * Quirk is on when TSX is not enabled. Therefore TSX
 		 * flags must be read as OFF.
 		 */
 		val &= ~(LBR_FROM_FLAG_IN_TX | LBR_FROM_FLAG_ABORT);
+	}
+	return val;
+}
+
+static inline void wrlbr_from(unsigned int idx, u64 val)
+{
+	val = lbr_from_signext_quirk_wr(val);
+	wrmsrl(x86_pmu.lbr_from + idx, val);
+}
+
+static inline void wrlbr_to(unsigned int idx, u64 val)
+{
+	wrmsrl(x86_pmu.lbr_to + idx, val);
+}
+
+static inline u64 rdlbr_from(unsigned int idx)
+{
+	u64 val;
+
+	rdmsrl(x86_pmu.lbr_from + idx, val);
+
+	return lbr_from_signext_quirk_rd(val);
+}
+
+static inline u64 rdlbr_to(unsigned int idx)
+{
+	u64 val;
+
+	rdmsrl(x86_pmu.lbr_from + idx, val);
+
 	return val;
 }
 
@@ -313,9 +343,9 @@ static void __intel_pmu_lbr_restore(stru
 	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
-		wrmsrl(x86_pmu.lbr_from + lbr_idx,
-			lbr_from_signext_quirk_wr(task_ctx->lbr_from[i]));
-		wrmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
+		wrlbr_from(lbr_idx, task_ctx->lbr_from[i]);
+		wrlbr_to  (lbr_idx, task_ctx->lbr_to[i]);
+
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
@@ -325,9 +355,9 @@ static void __intel_pmu_lbr_restore(stru
 
 static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 {
-	int i;
 	unsigned lbr_idx, mask;
-	u64 tos, val;
+	u64 tos;
+	int i;
 
 	if (task_ctx->lbr_callstack_users == 0) {
 		task_ctx->lbr_stack_state = LBR_NONE;
@@ -338,9 +368,8 @@ static void __intel_pmu_lbr_save(struct
 	tos = intel_pmu_lbr_tos();
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
-		rdmsrl(x86_pmu.lbr_from + lbr_idx, val);
-		task_ctx->lbr_from[i] = lbr_from_signext_quirk_rd(val);
-		rdmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
+		task_ctx->lbr_from[i] = rdlbr_from(lbr_idx);
+		task_ctx->lbr_to[i]   = rdlbr_to(lbr_idx);
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
@@ -516,10 +545,8 @@ static void intel_pmu_lbr_read_64(struct
 		u16 cycles = 0;
 		int lbr_flags = lbr_desc[lbr_format];
 
-		rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
-		from = lbr_from_signext_quirk_rd(from);
-
-		rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
+		from = rdlbr_from(lbr_idx);
+		to   = rdlbr_to(lbr_idx);
 
 		if (lbr_format == LBR_FORMAT_INFO && need_info) {
 			u64 info;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf/x86/intel: Print LBR support statement after validation
  2016-06-21 18:31 ` [PATCH v02 1/5] perf/x86/intel: output LBR support statement after validation David Carrillo-Cisneros
@ 2016-06-27 12:54   ` tip-bot for David Carrillo-Cisneros
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for David Carrillo-Cisneros @ 2016-06-27 12:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, vincent.weaver, linux-kernel, kan.liang, acme, eranian, ak,
	mingo, alexander.shishkin, tglx, davidcc, torvalds, jolsa,
	peterz

Commit-ID:  f09509b9398b23ca53360ca57106555698ec2e93
Gitweb:     http://git.kernel.org/tip/f09509b9398b23ca53360ca57106555698ec2e93
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Tue, 21 Jun 2016 11:31:10 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 27 Jun 2016 11:34:18 +0200

perf/x86/intel: Print LBR support statement after validation

The following commit:

  338b522ca43c ("perf/x86/intel: Protect LBR and extra_regs against KVM lying")

added an additional test to LBR support detection that is performed after
printing the LBR support statement to dmesg.

Move the LBR support output after the very last test, to make sure we
print the true status of LBR support.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Stephane Eranian <eranian@google.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1466533874-52003-2-git-send-email-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/core.c | 2 ++
 arch/x86/events/intel/lbr.c  | 9 ---------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 3ed528c..61a027b 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3958,6 +3958,8 @@ __init int intel_pmu_init(void)
 			x86_pmu.lbr_nr = 0;
 	}
 
+	if (x86_pmu.lbr_nr)
+		pr_cont("%d-deep LBR, ", x86_pmu.lbr_nr);
 	/*
 	 * Access extra MSR may cause #GP under certain circumstances.
 	 * E.g. KVM doesn't support offcore event
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 9e2b40c..2dca66c 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -956,7 +956,6 @@ void __init intel_pmu_lbr_init_core(void)
 	 * SW branch filter usage:
 	 * - compensate for lack of HW filter
 	 */
-	pr_cont("4-deep LBR, ");
 }
 
 /* nehalem/westmere */
@@ -977,7 +976,6 @@ void __init intel_pmu_lbr_init_nhm(void)
 	 *   That requires LBR_FAR but that means far
 	 *   jmp need to be filtered out
 	 */
-	pr_cont("16-deep LBR, ");
 }
 
 /* sandy bridge */
@@ -997,7 +995,6 @@ void __init intel_pmu_lbr_init_snb(void)
 	 *   That requires LBR_FAR but that means far
 	 *   jmp need to be filtered out
 	 */
-	pr_cont("16-deep LBR, ");
 }
 
 /* haswell */
@@ -1010,8 +1007,6 @@ void intel_pmu_lbr_init_hsw(void)
 
 	x86_pmu.lbr_sel_mask = LBR_SEL_MASK;
 	x86_pmu.lbr_sel_map  = hsw_lbr_sel_map;
-
-	pr_cont("16-deep LBR, ");
 }
 
 /* skylake */
@@ -1031,7 +1026,6 @@ __init void intel_pmu_lbr_init_skl(void)
 	 *   That requires LBR_FAR but that means far
 	 *   jmp need to be filtered out
 	 */
-	pr_cont("32-deep LBR, ");
 }
 
 /* atom */
@@ -1057,7 +1051,6 @@ void __init intel_pmu_lbr_init_atom(void)
 	 * SW branch filter usage:
 	 * - compensate for lack of HW filter
 	 */
-	pr_cont("8-deep LBR, ");
 }
 
 /* slm */
@@ -1088,6 +1081,4 @@ void intel_pmu_lbr_init_knl(void)
 
 	x86_pmu.lbr_sel_mask = LBR_SEL_MASK;
 	x86_pmu.lbr_sel_map  = snb_lbr_sel_map;
-
-	pr_cont("8-deep LBR, ");
 }

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf/x86/intel: Fix MSR_LAST_BRANCH_FROM_x bug when no TSX
  2016-06-21 18:31 ` [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX David Carrillo-Cisneros
  2016-06-21 22:54   ` Andi Kleen
@ 2016-06-27 12:54   ` tip-bot for David Carrillo-Cisneros
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for David Carrillo-Cisneros @ 2016-06-27 12:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, mingo, torvalds, kan.liang, linux-kernel, vincent.weaver,
	eranian, acme, alexander.shishkin, peterz, tglx, davidcc, hpa,
	ak

Commit-ID:  19fc9ddd61e059cc45464bdf6e8fa304bb94080f
Gitweb:     http://git.kernel.org/tip/19fc9ddd61e059cc45464bdf6e8fa304bb94080f
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Tue, 21 Jun 2016 11:31:11 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 27 Jun 2016 11:34:19 +0200

perf/x86/intel: Fix MSR_LAST_BRANCH_FROM_x bug when no TSX

Intel's SDM states that bits 61:62 in MSR_LAST_BRANCH_FROM_x are the
TSX flags for formats with LBR_TSX flags (i.e. LBR_FORMAT_EIP_EFLAGS2).

However, when the CPU has TSX support deactivated, bits 61:62 actually
behave as follows:

  - For wrmsr(), bits 61:62 are considered part of the sign extension.
  - When capturing branches, the LBR hw will always clear bits 61:62.
    regardless of the sign extension.

Therefore, if:

  1) LBR has TSX format.
  2) CPU has no TSX support enabled.

... then any value passed to wrmsr() must be sign extended to 63 bits
and any value from rdmsr() must be converted to have a sign extension
of 61 bits, ignoring the values at TSX flags.

This bug was masked by the work-around to the Intel's CPU bug:
BJ94. "LBR May Contain Incorrect Information When Using FREEZE_LBRS_ON_PMI"
in Document Number: 324643-037US.

The aforementioned work-around uses hw flags to filter out all kernel
branches, limiting LBR callstack to user level execution only.

Since user addresses are not sign extended, they do not trigger the wrmsr()
bug in MSR_LAST_BRANCH_FROM_x when saved/restored at context switch.

To verify the hw bug:

  $ perf record -b -e cycles sleep 1
  $ rdmsr -p 0 0x680
  0x1fffffffb0b9b0cc
  $ wrmsr -p 0 0x680 0x1fffffffb0b9b0cc
  write(): Input/output error

The quirk for LBR_FROM_ MSRs is required before calls to wrmsrl() and
after rdmsrl().

This patch introduces it for wrmsrl()'s done for testing LBR support.

Future patch in series adds the quirk for context switch, that would
be required if LBR callstack is to be enabled for ring 0.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Stephane Eranian <eranian@google.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1466533874-52003-3-git-send-email-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/core.c | 18 +++++++++++++++
 arch/x86/events/intel/lbr.c  | 52 ++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/events/perf_event.h |  2 ++
 3 files changed, 72 insertions(+)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 61a027b..3eccc42 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3361,6 +3361,13 @@ static void intel_snb_check_microcode(void)
 	}
 }
 
+static bool is_lbr_from(unsigned long msr)
+{
+	unsigned long lbr_from_nr = x86_pmu.lbr_from + x86_pmu.lbr_nr;
+
+	return x86_pmu.lbr_from <= msr && msr < lbr_from_nr;
+}
+
 /*
  * Under certain circumstances, access certain MSR may cause #GP.
  * The function tests if the input MSR can be safely accessed.
@@ -3381,13 +3388,24 @@ static bool check_msr(unsigned long msr, u64 mask)
 	 * Only change the bits which can be updated by wrmsrl.
 	 */
 	val_tmp = val_old ^ mask;
+
+	if (is_lbr_from(msr))
+		val_tmp = lbr_from_signext_quirk_wr(val_tmp);
+
 	if (wrmsrl_safe(msr, val_tmp) ||
 	    rdmsrl_safe(msr, &val_new))
 		return false;
 
+	/*
+	 * Quirk only affects validation in wrmsr(), so wrmsrl()'s value
+	 * should equal rdmsrl()'s even with the quirk.
+	 */
 	if (val_new != val_tmp)
 		return false;
 
+	if (is_lbr_from(msr))
+		val_old = lbr_from_signext_quirk_wr(val_old);
+
 	/* Here it's sure that the MSR can be safely accessed.
 	 * Restore the old value and return.
 	 */
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 2dca66c..88093e0 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -81,6 +81,8 @@ static enum {
 #define LBR_FROM_FLAG_IN_TX    (1ULL << 62)
 #define LBR_FROM_FLAG_ABORT    (1ULL << 61)
 
+#define LBR_FROM_SIGNEXT_2MSB	(BIT_ULL(60) | BIT_ULL(59))
+
 /*
  * x86control flow change classification
  * x86control flow changes include branches, interrupts, traps, faults
@@ -235,6 +237,53 @@ enum {
 	LBR_VALID,
 };
 
+/*
+ * For formats with LBR_TSX flags (e.g. LBR_FORMAT_EIP_FLAGS2), bits 61:62 in
+ * MSR_LAST_BRANCH_FROM_x are the TSX flags when TSX is supported, but when
+ * TSX is not supported they have no consistent behavior:
+ *
+ *   - For wrmsr(), bits 61:62 are considered part of the sign extension.
+ *   - For HW updates (branch captures) bits 61:62 are always OFF and are not
+ *     part of the sign extension.
+ *
+ * Therefore, if:
+ *
+ *   1) LBR has TSX format
+ *   2) CPU has no TSX support enabled
+ *
+ * ... then any value passed to wrmsr() must be sign extended to 63 bits and any
+ * value from rdmsr() must be converted to have a 61 bits sign extension,
+ * ignoring the TSX flags.
+ */
+static inline bool lbr_from_signext_quirk_needed(void)
+{
+	int lbr_format = x86_pmu.intel_cap.lbr_format;
+	bool tsx_support = boot_cpu_has(X86_FEATURE_HLE) ||
+			   boot_cpu_has(X86_FEATURE_RTM);
+
+	return !tsx_support && (lbr_desc[lbr_format] & LBR_TSX);
+}
+
+DEFINE_STATIC_KEY_FALSE(lbr_from_quirk_key);
+
+/* If quirk is enabled, ensure sign extension is 63 bits: */
+inline u64 lbr_from_signext_quirk_wr(u64 val)
+{
+	if (static_branch_unlikely(&lbr_from_quirk_key)) {
+		/*
+		 * Sign extend into bits 61:62 while preserving bit 63.
+		 *
+		 * Quirk is enabled when TSX is disabled. Therefore TSX bits
+		 * in val are always OFF and must be changed to be sign
+		 * extension bits. Since bits 59:60 are guaranteed to be
+		 * part of the sign extension bits, we can just copy them
+		 * to 61:62.
+		 */
+		val |= (LBR_FROM_SIGNEXT_2MSB & val) << 2;
+	}
+	return val;
+}
+
 static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 {
 	int i;
@@ -1007,6 +1056,9 @@ void intel_pmu_lbr_init_hsw(void)
 
 	x86_pmu.lbr_sel_mask = LBR_SEL_MASK;
 	x86_pmu.lbr_sel_map  = hsw_lbr_sel_map;
+
+	if (lbr_from_signext_quirk_needed())
+		static_branch_enable(&lbr_from_quirk_key);
 }
 
 /* skylake */
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index e2d7285..8c4a477 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -902,6 +902,8 @@ void intel_ds_init(void);
 
 void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in);
 
+u64 lbr_from_signext_quirk_wr(u64 val);
+
 void intel_pmu_lbr_reset(void);
 
 void intel_pmu_lbr_enable(struct perf_event *event);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf/x86/intel: Fix trivial formatting and style bug
  2016-06-21 18:31 ` [PATCH v02 3/5] perf/x86/intel: trivial format and style fix David Carrillo-Cisneros
@ 2016-06-27 12:55   ` tip-bot for David Carrillo-Cisneros
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for David Carrillo-Cisneros @ 2016-06-27 12:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: davidcc, kan.liang, acme, vincent.weaver, mingo, torvalds,
	alexander.shishkin, peterz, linux-kernel, jolsa, hpa, ak, tglx,
	eranian

Commit-ID:  3812bba84f3d721ff7dc3bb90360bc5ed6771994
Gitweb:     http://git.kernel.org/tip/3812bba84f3d721ff7dc3bb90360bc5ed6771994
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Tue, 21 Jun 2016 11:31:12 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 27 Jun 2016 11:34:19 +0200

perf/x86/intel: Fix trivial formatting and style bug

Replace spaces by tabs in LBR_FROM_* constants to align with newly
defined constant. Use BIT_ULL.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Stephane Eranian <eranian@google.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1466533874-52003-4-git-send-email-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/lbr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 88093e0..0da0eb0 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -77,9 +77,9 @@ static enum {
 	 LBR_IND_JMP	|\
 	 LBR_FAR)
 
-#define LBR_FROM_FLAG_MISPRED  (1ULL << 63)
-#define LBR_FROM_FLAG_IN_TX    (1ULL << 62)
-#define LBR_FROM_FLAG_ABORT    (1ULL << 61)
+#define LBR_FROM_FLAG_MISPRED	BIT_ULL(63)
+#define LBR_FROM_FLAG_IN_TX	BIT_ULL(62)
+#define LBR_FROM_FLAG_ABORT	BIT_ULL(61)
 
 #define LBR_FROM_SIGNEXT_2MSB	(BIT_ULL(60) | BIT_ULL(59))
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf/x86/intel: Add MSR_LAST_BRANCH_FROM_x quirk for ctx switch
  2016-06-21 18:31 ` [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch David Carrillo-Cisneros
  2016-06-23  8:43   ` Peter Zijlstra
@ 2016-06-27 12:55   ` tip-bot for David Carrillo-Cisneros
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for David Carrillo-Cisneros @ 2016-06-27 12:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, acme, kan.liang, peterz, eranian, ak, torvalds,
	vincent.weaver, davidcc, tglx, jolsa, mingo, hpa,
	alexander.shishkin

Commit-ID:  71adae99ed187de9fcf988cc8873ee2c3af3385f
Gitweb:     http://git.kernel.org/tip/71adae99ed187de9fcf988cc8873ee2c3af3385f
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Tue, 21 Jun 2016 11:31:13 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 27 Jun 2016 11:34:20 +0200

perf/x86/intel: Add MSR_LAST_BRANCH_FROM_x quirk for ctx switch

Add quirk for context switch to save/restore the value of
MSR_LAST_BRANCH_FROM_x when LBR is enabled and there is potential for
kernel addresses to be in the lbr_from register.

To test this patch, use a perf tool and kernel with the patch
next in this series. That patch removes the work around that masked
the hw bug:

  $ ./lbr_perf record --call-graph lbr -e cycles:k sleep 1

where lbr_perf is the patched perf tool, that allows to specify :k
on lbr mode. The above command will trigger a #GPF :

 WARNING: CPU: 28 PID: 14096 at arch/x86/mm/extable.c:65 ex_handler_wrmsr_unsafe+0x70/0x80
 unchecked MSR access error: WRMSR to 0x681 (tried to write 0x1fffffff81010794)
 ...
 Call Trace:
  [<ffffffff8167af49>] dump_stack+0x4d/0x63
  [<ffffffff810b9b15>] __warn+0xe5/0x100
  [<ffffffff810b9be9>] warn_slowpath_fmt+0x49/0x50
  [<ffffffff810abb40>] ex_handler_wrmsr_unsafe+0x70/0x80
  [<ffffffff810abc42>] fixup_exception+0x42/0x50
  [<ffffffff81079d1a>] do_general_protection+0x8a/0x160
  [<ffffffff81684ec2>] general_protection+0x22/0x30
  [<ffffffff810101b9>] ? intel_pmu_lbr_sched_task+0xc9/0x380
  [<ffffffff81009d7c>] intel_pmu_sched_task+0x3c/0x60
  [<ffffffff81003a2b>] x86_pmu_sched_task+0x1b/0x20
  [<ffffffff81192a5b>] perf_pmu_sched_task+0x6b/0xb0
  [<ffffffff8119746d>] __perf_event_task_sched_in+0x7d/0x150
  [<ffffffff810dd9dc>] finish_task_switch+0x15c/0x200
  [<ffffffff8167f894>] __schedule+0x274/0x6cc
  [<ffffffff8167fdd9>] schedule+0x39/0x90
  [<ffffffff81675398>] exit_to_usermode_loop+0x39/0x89
  [<ffffffff810028ce>] prepare_exit_to_usermode+0x2e/0x30
  [<ffffffff81683c1b>] retint_user+0x8/0x10

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Stephane Eranian <eranian@google.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1466533874-52003-5-git-send-email-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/lbr.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 0da0eb0..52bef15 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -284,6 +284,20 @@ inline u64 lbr_from_signext_quirk_wr(u64 val)
 	return val;
 }
 
+/*
+ * If quirk is needed, ensure sign extension is 61 bits:
+ */
+u64 lbr_from_signext_quirk_rd(u64 val)
+{
+	if (static_branch_unlikely(&lbr_from_quirk_key))
+		/*
+		 * Quirk is on when TSX is not enabled. Therefore TSX
+		 * flags must be read as OFF.
+		 */
+		val &= ~(LBR_FROM_FLAG_IN_TX | LBR_FROM_FLAG_ABORT);
+	return val;
+}
+
 static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 {
 	int i;
@@ -300,7 +314,8 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
-		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
+		wrmsrl(x86_pmu.lbr_from + lbr_idx,
+			lbr_from_signext_quirk_wr(task_ctx->lbr_from[i]));
 		wrmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
@@ -313,7 +328,7 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 {
 	int i;
 	unsigned lbr_idx, mask;
-	u64 tos;
+	u64 tos, val;
 
 	if (task_ctx->lbr_callstack_users == 0) {
 		task_ctx->lbr_stack_state = LBR_NONE;
@@ -324,7 +339,8 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 	tos = intel_pmu_lbr_tos();
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
-		rdmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
+		rdmsrl(x86_pmu.lbr_from + lbr_idx, val);
+		task_ctx->lbr_from[i] = lbr_from_signext_quirk_rd(val);
 		rdmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
@@ -502,6 +518,8 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 		int lbr_flags = lbr_desc[lbr_format];
 
 		rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
+		from = lbr_from_signext_quirk_rd(from);
+
 		rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
 
 		if (lbr_format == LBR_FORMAT_INFO && need_info) {

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-06-27 12:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-21 18:31 [PATCH v02 0/5] fix MSR_LAST_BRANCH_FROM Haswell support David Carrillo-Cisneros
2016-06-21 18:31 ` [PATCH v02 1/5] perf/x86/intel: output LBR support statement after validation David Carrillo-Cisneros
2016-06-27 12:54   ` [tip:perf/core] perf/x86/intel: Print " tip-bot for David Carrillo-Cisneros
2016-06-21 18:31 ` [PATCH v02 2/5] perf/x86/intel: fix for MSR_LAST_BRANCH_FROM_x bug when no TSX David Carrillo-Cisneros
2016-06-21 22:54   ` Andi Kleen
2016-06-27 12:54   ` [tip:perf/core] perf/x86/intel: Fix " tip-bot for David Carrillo-Cisneros
2016-06-21 18:31 ` [PATCH v02 3/5] perf/x86/intel: trivial format and style fix David Carrillo-Cisneros
2016-06-27 12:55   ` [tip:perf/core] perf/x86/intel: Fix trivial formatting and style bug tip-bot for David Carrillo-Cisneros
2016-06-21 18:31 ` [PATCH v02 4/5] perf/x86/intel: MSR_LAST_BRANCH_FROM_x quirk for ctx switch David Carrillo-Cisneros
2016-06-23  8:43   ` Peter Zijlstra
2016-06-27 12:55   ` [tip:perf/core] perf/x86/intel: Add " tip-bot for David Carrillo-Cisneros
2016-06-21 18:31 ` [PATCH v02 5/5] not required, used to test ctxsw, do not merge David Carrillo-Cisneros

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).