: fix Haswell precise store data source encoding

PATCH In review Stephane Eranian 4 participants 6 messages May 15, 2014 – May 19, 2014

Subsystems (2)

x86 perf events

lore

Share & tools

lore

Stephane Eranian

author

2014-05-15 15:56 UTC #0 lore raw

PATCH v2 1/1 : fix Haswell precise store data source encoding

This patch fixes a bug in precise_store_data_hsw() whereby it would set the data source memory level to the wrong value.

As per the the SDM Vol 3b Table 18-41 (Layout of Data Linear Address Information in PEBS Record), when status bit 0 is set this is a L1 hit, otherwise this is a L1 miss.

This patch encodes the memory level according to the specification.

In V2, we added the filtering on the store events. Only the following events produce L1 information:
* MEM_UOPS_RETIRED.STLB_MISS_STORES
* MEM_UOPS_RETIRED.LOCK_STORES
* MEM_UOPS_RETIRED.SPLIT_STORES
* MEM_UOPS_RETIRED.ALL_STORES

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

arch/x86/kernel/cpu/perf_event_intel_ds.c +19 -3

index ae96cfa..980970c 100644
@@ -108,15 +108,31 @@ static u64 precise_store_data(u64 status)
 	return val;
 }
 
-static u64 precise_store_data_hsw(u64 status)
+static u64 precise_store_data_hsw(struct perf_event *event, u64 status)
 {
 	union perf_mem_data_src dse;
+	u64 cfg = event->hw.config & INTEL_ARCH_EVENT_MASK;
 
 	dse.val = 0;
 	dse.mem_op = PERF_MEM_OP_STORE;
 	dse.mem_lvl = PERF_MEM_LVL_NA;
+
+	/*
+	 * L1 info only valid for following events:
+	 *
+	 * MEM_UOPS_RETIRED.STLB_MISS_STORES
+	 * MEM_UOPS_RETIRED.LOCK_STORES
+	 * MEM_UOPS_RETIRED.SPLIT_STORES
+	 * MEM_UOPS_RETIRED.ALL_STORES
+	 */
+	if (cfg != 0x12d0 && cfg != 0x22d0 && cfg != 0x42d0 && cfg != 0x82d0)
+		return dse.mem_lvl;
+
 	if (status & 1)
-		dse.mem_lvl = PERF_MEM_LVL_L1;
+		dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_HIT;
+	else
+		dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_MISS;
+
 	/* Nothing else supported. Sorry. */
 	return dse.val;
 }

@@ -887,7 +903,7 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
 				data.data_src.val = load_latency_data(pebs->dse);
 			else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST_HSW)
 				data.data_src.val =
-					precise_store_data_hsw(pebs->dse);
+					precise_store_data_hsw(event, pebs->dse);
 			else
 				data.data_src.val = precise_store_data(pebs->dse);
 		}

Don Zickus

2014-05-15 19:57 UTC #1 lore raw

Quoted 8 lines · Stephane Eranian

This patch fixes a bug in precise_store_data_hsw() whereby it would set the data source memory level to the wrong value.
As per the the SDM Vol 3b Table 18-41 (Layout of Data Linear Address Information in PEBS Record), when status bit 0 is set this is a L1 hit, otherwise this is a L1 miss.

Quoted 8 lines

This patch encodes the memory level according to the specification.
In V2, we added the filtering on the store events. Only the following events produce L1 information:
* MEM_UOPS_RETIRED.STLB_MISS_STORES
* MEM_UOPS_RETIRED.LOCK_STORES
* MEM_UOPS_RETIRED.SPLIT_STORES
* MEM_UOPS_RETIRED.ALL_STORES

This worked great on our Haswell-EX box. I was a little surprised to find out it did until I realized on Ivy Bridge 'mem-store' was a 0x02cd but on Haswell it is now a 0x82d0. Go generic event types! :-)

Looking at the SDM documentation it does say something about 'UOPS_RETIRED.ALL' supporting stores too but can't find that event. Is that a typo, much like the 0x02 umask for stores on the D0 event is missing from the documentation? Just wanted to make sure we are not missing one more case.

Thanks for the quick patch Stephane!

Tested-and-Reviewed-by Don Zickus <[email protected]>

Quoted 50 lines

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

arch/x86/kernel/cpu/perf_event_intel_ds.c +19 -3

index ae96cfa..980970c 100644
@@ -108,15 +108,31 @@ static u64 precise_store_data(u64 status)
 	return val;
 }
 
-static u64 precise_store_data_hsw(u64 status)
+static u64 precise_store_data_hsw(struct perf_event *event, u64 status)
 {
 	union perf_mem_data_src dse;
+	u64 cfg = event->hw.config & INTEL_ARCH_EVENT_MASK;
 
 	dse.val = 0;
 	dse.mem_op = PERF_MEM_OP_STORE;
 	dse.mem_lvl = PERF_MEM_LVL_NA;
+
+	/*
+	 * L1 info only valid for following events:
+	 *
+	 * MEM_UOPS_RETIRED.STLB_MISS_STORES
+	 * MEM_UOPS_RETIRED.LOCK_STORES
+	 * MEM_UOPS_RETIRED.SPLIT_STORES
+	 * MEM_UOPS_RETIRED.ALL_STORES
+	 */
+	if (cfg != 0x12d0 && cfg != 0x22d0 && cfg != 0x42d0 && cfg != 0x82d0)
+		return dse.mem_lvl;
+
 	if (status & 1)
-		dse.mem_lvl = PERF_MEM_LVL_L1;
+		dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_HIT;
+	else
+		dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_MISS;
+
 	/* Nothing else supported. Sorry. */
 	return dse.val;
 }

@@ -887,7 +903,7 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
 				data.data_src.val = load_latency_data(pebs->dse);
 			else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST_HSW)
 				data.data_src.val =
-					precise_store_data_hsw(pebs->dse);
+					precise_store_data_hsw(event, pebs->dse);
 			else
 				data.data_src.val = precise_store_data(pebs->dse);
 		}

Stephane Eranian

author

2014-05-15 20:08 UTC #2 lore raw

Quoted 22 lines · Don Zickus

Quoted 8 lines · Stephane Eranian
This patch fixes a bug in precise_store_data_hsw() whereby it would set the data source memory level to the wrong value.
As per the the SDM Vol 3b Table 18-41 (Layout of Data Linear Address Information in PEBS Record), when status bit 0 is set this is a L1 hit, otherwise this is a L1 miss.
Quoted 8 lines
This patch encodes the memory level according to the specification.
In V2, we added the filtering on the store events. Only the following events produce L1 information:
* MEM_UOPS_RETIRED.STLB_MISS_STORES
* MEM_UOPS_RETIRED.LOCK_STORES
* MEM_UOPS_RETIRED.SPLIT_STORES
* MEM_UOPS_RETIRED.ALL_STORES
This worked great on our Haswell-EX box. I was a little surprised to find out it did until I realized on Ivy Bridge 'mem-store' was a 0x02cd but on Haswell it is now a 0x82d0. Go generic event types! :-)

Quoted 6 lines

Looking at the SDM documentation it does say something about 'UOPS_RETIRED.ALL' supporting stores too but can't find that event. Is that a typo, much like the 0x02 umask for stores on the D0 event is missing from the documentation? Just wanted to make sure we are not missing one more case.

But uops_retired.all does not generate the L1 info. That is why it is not there in my patch.

Quoted 54 lines

Thanks for the quick patch Stephane!

Tested-and-Reviewed-by Don Zickus <[email protected]>

Quoted 50 lines

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

arch/x86/kernel/cpu/perf_event_intel_ds.c +19 -3

index ae96cfa..980970c 100644
@@ -108,15 +108,31 @@ static u64 precise_store_data(u64 status)
      return val;
 }
 
-static u64 precise_store_data_hsw(u64 status)
+static u64 precise_store_data_hsw(struct perf_event *event, u64 status)
 {
      union perf_mem_data_src dse;
+     u64 cfg = event->hw.config & INTEL_ARCH_EVENT_MASK;
 
      dse.val = 0;
      dse.mem_op = PERF_MEM_OP_STORE;
      dse.mem_lvl = PERF_MEM_LVL_NA;
+
+     /*
+      * L1 info only valid for following events:
+      *
+      * MEM_UOPS_RETIRED.STLB_MISS_STORES
+      * MEM_UOPS_RETIRED.LOCK_STORES
+      * MEM_UOPS_RETIRED.SPLIT_STORES
+      * MEM_UOPS_RETIRED.ALL_STORES
+      */
+     if (cfg != 0x12d0 && cfg != 0x22d0 && cfg != 0x42d0 && cfg != 0x82d0)
+             return dse.mem_lvl;
+
      if (status & 1)
-             dse.mem_lvl = PERF_MEM_LVL_L1;
+             dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_HIT;
+     else
+             dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_MISS;
+
      /* Nothing else supported. Sorry. */
      return dse.val;
 }

@@ -887,7 +903,7 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
                              data.data_src.val = load_latency_data(pebs->dse);
                      else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST_HSW)
                              data.data_src.val =
-                                     precise_store_data_hsw(pebs->dse);
+                                     precise_store_data_hsw(event, pebs->dse);
                      else
                              data.data_src.val = precise_store_data(pebs->dse);
              }

Don Zickus

2014-05-15 20:27 UTC #3 lore raw

Quoted 23 lines · Stephane Eranian

Quoted 22 lines · Don Zickus
Quoted 8 lines · Stephane Eranian
This patch fixes a bug in precise_store_data_hsw() whereby it would set the data source memory level to the wrong value.
As per the the SDM Vol 3b Table 18-41 (Layout of Data Linear Address Information in PEBS Record), when status bit 0 is set this is a L1 hit, otherwise this is a L1 miss.
Quoted 8 lines
This patch encodes the memory level according to the specification.
In V2, we added the filtering on the store events. Only the following events produce L1 information:
* MEM_UOPS_RETIRED.STLB_MISS_STORES
* MEM_UOPS_RETIRED.LOCK_STORES
* MEM_UOPS_RETIRED.SPLIT_STORES
* MEM_UOPS_RETIRED.ALL_STORES
This worked great on our Haswell-EX box. I was a little surprised to find out it did until I realized on Ivy Bridge 'mem-store' was a 0x02cd but on Haswell it is now a 0x82d0. Go generic event types! :-)

Quoted 8 lines

Quoted 6 lines
Looking at the SDM documentation it does say something about 'UOPS_RETIRED.ALL' supporting stores too but can't find that event. Is that a typo, much like the 0x02 umask for stores on the D0 event is missing from the documentation? Just wanted to make sure we are not missing one more case.
But uops_retired.all does not generate the L1 info. That is why it is not there in my patch.

Ok.

Cheers,
Don

Andi Kleen

2014-05-15 21:22 UTC #4 lore raw

In reply to Don Zickus

Quoted 5 lines

Looking at the SDM documentation it does say something about 'UOPS_RETIRED.ALL' supporting stores too but can't find that event. Is that a typo, much like the 0x02 umask for stores on the D0 event is missing from the documentation? Just wanted to make sure we are not missing one more case.

There are some problems in the HSW perf PEBS list. I posted a patch for this, but so far it's stuck in the usual perf patch limbo:

http://lkml.iu.edu/hypermail/linux/kernel/1404.2/01509.html

-Andi

tip-bot for Stephane Eranian

merge bot

2014-05-19 12:56 UTC #5 lore raw

[tip:perf/core] fix Haswell precise store data source encoding

In reply to Stephane Eranian

fix Haswell precise store data source encoding

Commit-ID:  722e76e60f2775c21b087ff12c5e678cf0ebcaaf
Gitweb:     http://git.kernel.org/tip/722e76e60f2775c21b087ff12c5e678cf0ebcaaf
Author:     Stephane Eranian <[email protected]>
AuthorDate: Thu, 15 May 2014 17:56:44 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 19 May 2014 21:52:59 +0900

fix Haswell precise store data source encoding

This patch fixes a bug in precise_store_data_hsw() whereby it would set the data source memory level to the wrong value.

As per the the SDM Vol 3b Table 18-41 (Layout of Data Linear Address Information in PEBS Record), when status bit 0 is set this is a L1 hit, otherwise this is a L1 miss.

This patch encodes the memory level according to the specification.

Cc [email protected]

Tested-and-Reviewed-by Don Zickus <[email protected]>

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

Signed-off-by Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/20140515155644.GA3884@quad open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

arch/x86/kernel/cpu/perf_event_intel_ds.c +19 -3

index ae96cfa..980970c 100644
@@ -108,15 +108,31 @@ static u64 precise_store_data(u64 status)
 	return val;
 }
 
-static u64 precise_store_data_hsw(u64 status)
+static u64 precise_store_data_hsw(struct perf_event *event, u64 status)
 {
 	union perf_mem_data_src dse;
+	u64 cfg = event->hw.config & INTEL_ARCH_EVENT_MASK;
 
 	dse.val = 0;
 	dse.mem_op = PERF_MEM_OP_STORE;
 	dse.mem_lvl = PERF_MEM_LVL_NA;
+
+	/*
+	 * L1 info only valid for following events:
+	 *
+	 * MEM_UOPS_RETIRED.STLB_MISS_STORES
+	 * MEM_UOPS_RETIRED.LOCK_STORES
+	 * MEM_UOPS_RETIRED.SPLIT_STORES
+	 * MEM_UOPS_RETIRED.ALL_STORES
+	 */
+	if (cfg != 0x12d0 && cfg != 0x22d0 && cfg != 0x42d0 && cfg != 0x82d0)
+		return dse.mem_lvl;
+
 	if (status & 1)
-		dse.mem_lvl = PERF_MEM_LVL_L1;
+		dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_HIT;
+	else
+		dse.mem_lvl = PERF_MEM_LVL_L1 | PERF_MEM_LVL_MISS;
+
 	/* Nothing else supported. Sorry. */
 	return dse.val;
 }

@@ -887,7 +903,7 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
 				data.data_src.val = load_latency_data(pebs->dse);
 			else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST_HSW)
 				data.data_src.val =
-					precise_store_data_hsw(pebs->dse);
+					precise_store_data_hsw(event, pebs->dse);
 			else
 				data.data_src.val = precise_store_data(pebs->dse);
 		}