Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1411784 > unrolled thread

[PATCH 1/3] perf/x86/intel: output LBR support statement after validation

Started byDavid Carrillo-Cisneros <davidcc@google.com>
First post2016-06-02 04:50 +0200
Last post2016-06-08 11:10 +0200
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 1/3] perf/x86/intel: output LBR support statement after validation David Carrillo-Cisneros <davidcc@google.com> - 2016-06-02 04:50 +0200
    Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after  validation Peter Zijlstra <peterz@infradead.org> - 2016-06-02 10:30 +0200
    Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after  validation Andi Kleen <ak@linux.intel.com> - 2016-06-02 18:10 +0200
      Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation Stephane Eranian <eranian@google.com> - 2016-06-02 19:30 +0200
        Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after  validation Andi Kleen <ak@linux.intel.com> - 2016-06-06 04:00 +0200
          Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation Stephane Eranian <eranian@google.com> - 2016-06-08 09:30 +0200
            Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after  validation Andi Kleen <ak@linux.intel.com> - 2016-06-08 11:10 +0200

#1411784 — [PATCH 1/3] perf/x86/intel: output LBR support statement after validation

FromDavid Carrillo-Cisneros <davidcc@google.com>
Date2016-06-02 04:50 +0200
Subject[PATCH 1/3] perf/x86/intel: output LBR support statement after validation
Message-ID<rFqRP-8y-11@gated-at.bofh.it>
Commit "perf/x86/intel: Protect LBR and extra_regs against KVM lying"
added an additional test to LBR support detection is performed after
printing the LBR support statement to dmesg.

Move the LRB support output after very last test.

Reviewed-by: Stephane Eranian <eranian@google.com>
Signed-off-by: David Carrillo-Cisneros <davidcc@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

[toc] | [next] | [standalone]


#1411993 — Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation

FromPeter Zijlstra <peterz@infradead.org>
Date2016-06-02 10:30 +0200
SubjectRe: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation
Message-ID<rFwaR-3wC-17@gated-at.bofh.it>
In reply to#1411784
On Wed, Jun 01, 2016 at 07:42:01PM -0700, David Carrillo-Cisneros wrote:
> Commit "perf/x86/intel: Protect LBR and extra_regs against KVM lying"

Commit references are done like:

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

Please use that, looking them up on names is a royal pain.

[toc] | [prev] | [next] | [standalone]


#1412387 — Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation

FromAndi Kleen <ak@linux.intel.com>
Date2016-06-02 18:10 +0200
SubjectRe: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation
Message-ID<rFDm1-82K-9@gated-at.bofh.it>
In reply to#1411784
I don't think the context switch support is really needed. It's only
needed for saving/restoring LBRs, and we only do that with LBR callstacks.
In any other LBR mode that LBRs are only flushed on context switch
But LBR callstacks will never put kernel addresses into the LBRs
because they are forced to set a ring 3 filter. So you can't have
kernel addresses in the LBR when saving/restoring them
(unless I missed some case)

Dropping that will likely simplify the patch somewhat.

-Andi

[toc] | [prev] | [next] | [standalone]


#1412448

FromStephane Eranian <eranian@google.com>
Date2016-06-02 19:30 +0200
Message-ID<rFEBs-wH-27@gated-at.bofh.it>
In reply to#1412387
Andi,

On Thu, Jun 2, 2016 at 9:04 AM, Andi Kleen <ak@linux.intel.com> wrote:
>
>
> I don't think the context switch support is really needed. It's only
> needed for saving/restoring LBRs, and we only do that with LBR callstacks.
> In any other LBR mode that LBRs are only flushed on context switch
> But LBR callstacks will never put kernel addresses into the LBRs
> because they are forced to set a ring 3 filter. So you can't have
> kernel addresses in the LBR when saving/restoring them
> (unless I missed some case)
>
It is not because you force LBR to ring3 only that you do not capture
kernel addresses in the FROM field.
Keep in mind that LBR priv level filtering applies to the target of
the branch and not the source. You might
still get a kernel address if returning from kernel. Now, in callstack
mode, I think the return branch is never
actually recorded in the LBR, it just causes a pop, so theoretically
this should not happen. I'd like to be
100% sure of that, though.

> Dropping that will likely simplify the patch somewhat.
>
> -Andi

[toc] | [prev] | [next] | [standalone]


#1414557 — Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation

FromAndi Kleen <ak@linux.intel.com>
Date2016-06-06 04:00 +0200
SubjectRe: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation
Message-ID<rGRZD-67c-7@gated-at.bofh.it>
In reply to#1412448
> It is not because you force LBR to ring3 only that you do not capture
> kernel addresses in the FROM field.
> Keep in mind that LBR priv level filtering applies to the target of
> the branch and not the source. You might
> still get a kernel address if returning from kernel. Now, in callstack
> mode, I think the return branch is never
> actually recorded in the LBR, it just causes a pop, so theoretically
> this should not happen. I'd like to be
> 100% sure of that, though.

Far branches shouldn't be included in call stack LBR. Don't think
there is any other situation where the ring 0 address could leak either.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only

[toc] | [prev] | [next] | [standalone]


#1416973

FromStephane Eranian <eranian@google.com>
Date2016-06-08 09:30 +0200
Message-ID<rHG67-5xf-63@gated-at.bofh.it>
In reply to#1414557
Andi,

On Sun, Jun 5, 2016 at 6:59 PM, Andi Kleen <ak@linux.intel.com> wrote:
>
> > It is not because you force LBR to ring3 only that you do not capture
> > kernel addresses in the FROM field.
> > Keep in mind that LBR priv level filtering applies to the target of
> > the branch and not the source. You might
> > still get a kernel address if returning from kernel. Now, in callstack
> > mode, I think the return branch is never
> > actually recorded in the LBR, it just causes a pop, so theoretically
> > this should not happen. I'd like to be
> > 100% sure of that, though.
>
> Far branches shouldn't be included in call stack LBR. Don't think
> there is any other situation where the ring 0 address could leak either.
>
Ok, so you're saying that syscall and int are not causing LBR callstack to
record an entry. If that is the case, then a rfi should not cause a pop of the
last LBR entry. Is that right?

>
> -Andi
> --
> ak@linux.intel.com -- Speaking for myself only

[toc] | [prev] | [next] | [standalone]


#1417114 — Re: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation

FromAndi Kleen <ak@linux.intel.com>
Date2016-06-08 11:10 +0200
SubjectRe: [PATCH 1/3] perf/x86/intel: output LBR support statement after validation
Message-ID<rHHER-6Fd-23@gated-at.bofh.it>
In reply to#1416973
> Ok, so you're saying that syscall and int are not causing LBR callstack to
> record an entry. If that is the case, then a rfi should not cause a pop of the
> last LBR entry. Is that right?

rfi = ?

But likely yes. There shouldn't be any ring 0 branches in the callstack LBR,
as long as the patch forcing it to ring 3 is still there.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web