perf: add support for capturing skid IP

This patch adds support for SKID_IP to Intel x86 processors in PEBS mode. In that case, the off-by-1 IP from PEBS is returned in the SKID_IP field.

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/events/intel/ds.c | 7 +++++++
 1 file changed, 7 insertions(+)

arch/x86/events/intel/ds.c index renamed from arch/x86/events/intel/ds.c +0 -0

c6d23ffe422d..ee17de5d6b8d 100644

arch/x86/events/intel/ds.c +7 -0

@@ -1169,6 +1169,13 @@ static void setup_pebs_sample_data(struct
 perf_event *event,
 	    x86_pmu.intel_cap.pebs_format >= 1)
 		data->addr = pebs->dla;
 
+	/*
+	 * unmodified, skid IP which is guaranteed to be the next
+	 * dyanmic instruction
+	 */
+	if (sample_type & PERF_SAMPLE_SKID_IP)
+		data->skid_ip = pebs->ip;
+
 	if (x86_pmu.intel_cap.pebs_format >= 2) {
 		/* Only set the TSX weight when no memory weight. */

if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)
--
2.13.1.518.g3df882009-goog

Quoted 4 lines

This patch adds support for SKID_IP to Intel x86 processors in PEBS mode. In that case, the off-by-1 IP from PEBS is returned in the SKID_IP field.

It looks we can only get different skid_ip and ip with :pp event (attr.precise = 2). With the :p event (attr.precise = 1), the skid_ip and ip are the same. Right?

Thanks,
Kan

Quoted 27 lines

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/events/intel/ds.c | 7 +++++++
 1 file changed, 7 insertions(+)

arch/x86/events/intel/ds.c index renamed from arch/x86/events/intel/ds.c +0 -0

c6d23ffe422d..ee17de5d6b8d 100644

arch/x86/events/intel/ds.c +7 -0

@@ -1169,6 +1169,13 @@ static void setup_pebs_sample_data(struct
 perf_event *event,
          x86_pmu.intel_cap.pebs_format >= 1)
              data->addr = pebs->dla;
 
+     /*
+      * unmodified, skid IP which is guaranteed to be the next
+      * dyanmic instruction
+      */
+     if (sample_type & PERF_SAMPLE_SKID_IP)
+             data->skid_ip = pebs->ip;
+
      if (x86_pmu.intel_cap.pebs_format >= 2) {
              /* Only set the TSX weight when no memory weight. */

              if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)

--
2.13.1.518.g3df882009-goog

Quoted 4 lines

Quoted 4 lines

This patch adds support for SKID_IP to Intel x86 processors in PEBS mode. In that case, the off-by-1 IP from PEBS is returned in the SKID_IP

field.

Quoted 2 lines

It looks we can only get different skid_ip and ip with :pp event (attr.precise

= 2).

Quoted 2 lines

With the :p event (attr.precise = 1), the skid_ip and ip are the same. Right?

Correct, because skid_ip would be equal to the pebs->ip.

Quoted 29 lines

Quoted 28 lines

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/events/intel/ds.c | 7 +++++++
 1 file changed, 7 insertions(+)

arch/x86/events/intel/ds.c +7 -0

index c6d23ffe422d..ee17de5d6b8d 100644
@@ -1169,6 +1169,13 @@ static void setup_pebs_sample_data(struct
 perf_event *event,
          x86_pmu.intel_cap.pebs_format >= 1)
              data->addr = pebs->dla;
 
+     /*
+      * unmodified, skid IP which is guaranteed to be the next
+      * dyanmic instruction
+      */
+     if (sample_type & PERF_SAMPLE_SKID_IP)
+             data->skid_ip = pebs->ip;
+
      if (x86_pmu.intel_cap.pebs_format >= 2) {
              /* Only set the TSX weight when no memory weight. */

              if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)

--
2.13.1.518.g3df882009-goog

Quoted 8 lines · Kan

Quoted 4 lines

This patch adds support for SKID_IP to Intel x86 processors in PEBS mode. In that case, the off-by-1 IP from PEBS is returned in the SKID_IP field.

It looks we can only get different skid_ip and ip with :pp event (attr.precise = 2). With the :p event (attr.precise = 1), the skid_ip and ip are the same. Right?

Correct, because skid_ip would be equal to the pebs->ip.

Quoted 27 lines

Quoted 26 lines

Signed-off-by Stephane Eranian <[email protected]>

---
 arch/x86/events/intel/ds.c | 7 +++++++
 1 file changed, 7 insertions(+)

arch/x86/events/intel/ds.c index renamed from arch/x86/events/intel/ds.c +0 -0

c6d23ffe422d..ee17de5d6b8d 100644

arch/x86/events/intel/ds.c +7 -0

@@ -1169,6 +1169,13 @@ static void setup_pebs_sample_data(struct
 perf_event *event,
          x86_pmu.intel_cap.pebs_format >= 1)
              data->addr = pebs->dla;
 
+     /*
+      * unmodified, skid IP which is guaranteed to be the next
+      * dyanmic instruction
+      */
+     if (sample_type & PERF_SAMPLE_SKID_IP)
+             data->skid_ip = pebs->ip;
+
      if (x86_pmu.intel_cap.pebs_format >= 2) {
              /* Only set the TSX weight when no memory weight. */

              if ((sample_type & PERF_SAMPLE_WEIGHT) && !fll)

--
2.13.1.518.g3df882009-goog

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Quoted 4 lines · Stephane Eranian

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Patches look reasonable for me.

If you only cared about branches it would be more natural to model it like a 1 entry LBR. That would make a lot more tooling work automatically.

Quoted 6 lines · Andi Kleen

Quoted 4 lines · Stephane Eranian

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Quoted 6 lines

Patches look reasonable for me.

If you only cared about branches it would be more natural to model it like a 1 entry LBR. That would make a lot more tooling work automatically.

You'd still have to modify tooling to present correct column headers.

Quoted 9 lines · Stephane Eranian

Quoted 8 lines · Andi Kleen

Quoted 4 lines · Stephane Eranian

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Patches look reasonable for me.

Quoted 5 lines

Quoted 4 lines

If you only cared about branches it would be more natural to model it like a 1 entry LBR. That would make a lot more tooling work automatically.

You'd still have to modify tooling to present correct column headers.

Why? It's from/to?

-Andi

Quoted 14 lines · Andi Kleen

Quoted 13 lines · Stephane Eranian

Quoted 12 lines · Andi Kleen

Quoted 4 lines · Stephane Eranian

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Patches look reasonable for me.

If you only cared about branches it would be more natural to model it like a 1 entry LBR. That would make a lot more tooling work automatically.

Quoted 4 lines

Quoted 1 line

You'd still have to modify tooling to present correct column headers.

Why? It's from/to?

Ah, yes you are right, but it is not clear to me how you would specify this cleanly with the interface.
Especially in the case where this could be used for non-branch instructions.

Quoted 17 lines · Stephane Eranian

Quoted 16 lines · Andi Kleen

Quoted 14 lines · Stephane Eranian

Quoted 6 lines · Andi Kleen

Quoted 4 lines · Stephane Eranian

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Quoted 6 lines

Patches look reasonable for me.

If you only cared about branches it would be more natural to model it like a 1 entry LBR. That would make a lot more tooling work automatically.

You'd still have to modify tooling to present correct column headers.

Quoted 5 lines

Quoted 2 lines

Why? It's from/to?

Ah, yes you are right, but it is not clear to me how you would specify this cleanly with the interface.
Especially in the case where this could be used for non-branch instructions.

Generally the skid ip is only interesting for instructions that have some kind of control flow change, or an exception/interrupt.

So if it's interesting the headers would be correct.

Actually it's even correct for non control flow change, because the IP moves from "FROM" to "TO" ...

Quoted 18 lines · Andi Kleen

Quoted 17 lines · Stephane Eranian

Quoted 16 lines · Andi Kleen

Quoted 14 lines · Stephane Eranian

Quoted 6 lines · Andi Kleen

Quoted 4 lines · Stephane Eranian

This patchs adds a new sample record type called PERF_SAMPLE_SKID_IP. The goal is to record
the unmodified interrupted instruction pointer (IP) as seen by the kernel and reflected in the machine state.

Quoted 6 lines

Patches look reasonable for me.

If you only cared about branches it would be more natural to model it like a 1 entry LBR. That would make a lot more tooling work automatically.

You'd still have to modify tooling to present correct column headers.

Quoted 9 lines

Quoted 5 lines

Quoted 2 lines

Why? It's from/to?

Ah, yes you are right, but it is not clear to me how you would specify this cleanly with the interface.
Especially in the case where this could be used for non-branch instructions.

Generally the skid ip is only interesting for instructions that have some kind of control flow change, or an exception/interrupt.

True, right now I found use for the control flow changes, but any PEBS enabled event + precise=2 could potentially be useful.

Quoted 5 lines

So if it's interesting the headers would be correct.

Actually it's even correct for non control flow change, because the IP moves from "FROM" to "TO" ...

Sure.
But I am not too fond of tying this to a branch abstraction. You'd have to express
this with the perf_events interface.To trigger LBR-style hardware, you'd have to define a new PERF_SAMPLE_BRANCH_SKID_IP of some sort and then you tie this to a branch kind of sampling feature:

$   perf record -j skid_ip -e BR_INST_RETIRED.CONDITIONAL:pp

Is what I think you are thinking about.

Looking at this approach, the user interface is straightforward, implementation in the x86 code is a bit more hairy because of the way the branch_stack is captured, via the cpuc->lbr_entries. If you assume that SKID_IP cannot be used with any other branch stack mode, then it is easy. It becomes messy if you don't.

Quoted 5 lines

Looking at this approach, the user interface is straightforward, implementation in the x86 code is a bit more hairy because of the way the branch_stack is captured, via the cpuc->lbr_entries. If you assume that SKID_IP cannot be used with any other branch stack mode, then it is easy. It becomes messy if you don't.

That should be fine. After all if you have real LBRs you don't need the skid IP.

Andi,

Quoted 6 lines · Andi Kleen

Quoted 5 lines

Looking at this approach, the user interface is straightforward, implementation in the x86 code is a bit more hairy because of the way the branch_stack is captured, via the cpuc->lbr_entries. If you assume that SKID_IP cannot be used with any other branch stack mode, then it is easy. It becomes messy if you don't.

Quoted 3 lines

That should be fine. After all if you have real LBRs you don't need the skid IP.

Yes, you still do. This is not the same thing. LBR captures only taken branches. I care about taken AND non-taken branches and I don't want to sample on a non-taken event, assuming it is available.

Quoted 12 lines · Stephane Eranian

Andi,

Quoted 9 lines · Andi Kleen

Quoted 5 lines

Looking at this approach, the user interface is straightforward, implementation in the x86 code is a bit more hairy because of the way the branch_stack is captured, via the cpuc->lbr_entries. If you assume that SKID_IP cannot be used with any other branch stack mode, then it is easy. It becomes messy if you don't.

That should be fine. After all if you have real LBRs you don't need the skid IP.

Quoted 3 lines

Yes, you still do. This is not the same thing. LBR captures only taken branches. I care about taken AND non-taken branches and I don't want to sample on a non-taken event, assuming it is available.

Ok that's a reasonable argument for reporting it separately, like in your original patch.

-Andi

Quoted 17 lines · Andi Kleen

Quoted 15 lines · Stephane Eranian

Andi,

Quoted 9 lines · Andi Kleen

Quoted 5 lines

Looking at this approach, the user interface is straightforward, implementation in the x86 code is a bit more hairy because of the way the branch_stack is captured, via the cpuc->lbr_entries. If you assume that SKID_IP cannot be used with any other branch stack mode, then it is easy. It becomes messy if you don't.

That should be fine. After all if you have real LBRs you don't need the skid IP.

Yes, you still do. This is not the same thing. LBR captures only taken branches. I care about taken AND non-taken branches and I don't want to sample on a non-taken event, assuming it is available.

Quoted 3 lines

Ok that's a reasonable argument for reporting it separately, like in your original patch.

Yeah, I think it is easier and more portable, especially on hardware with a PEBS-like mechanism but no branch buffer (like LBR). FYI, I did do a test implementation yesterday to evaluate the difficulty.

Quoted 1 line

-Andi

Quoted 4 lines

Yeah, I think it is easier and more portable, especially on hardware with a PEBS-like mechanism but no branch buffer (like LBR). FYI, I did do a test implementation yesterday to evaluate the difficulty.

A more generalized usage of the feature is to evaluate the amount of skid for any precise event.

Quoted 6 lines

Quoted 4 lines

Yeah, I think it is easier and more portable, especially on hardware with a PEBS-like mechanism but no branch buffer (like LBR). FYI, I did do a test implementation yesterday to evaluate the difficulty.

A more generalized usage of the feature is to evaluate the amount of skid for any precise event.

It should be always the same (one instruction), except for the control flow change case.

-Andi