Skip to contentSkip to conversation

perf/x86: fix Haswell precise store data source encoding

PATCH Superseded Stephane Eranian 2 participants 3 messages May 15, 2014

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf/x86: fix Haswell precise store data source encoding

This patches 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.

arch/x86/kernel/cpu/perf_event_intel_ds.c +6 -2
index ae96cfa..81424f6 100644
@@ -114,9 +114,13 @@ static u64 precise_store_data_hsw(u64 status)
 
 	dse.val = 0;
 	dse.mem_op = PERF_MEM_OP_STORE;
-	dse.mem_lvl = PERF_MEM_LVL_NA;
+	dse.mem_lvl = PERF_MEM_LVL_L1;
+
 	if (status & 1)
-		dse.mem_lvl = PERF_MEM_LVL_L1;
+		dse.mem_lvl |= PERF_MEM_LVL_HIT;
+	else
+		dse.mem_lvl |= PERF_MEM_LVL_MISS;
+
 	/* Nothing else supported. Sorry. */
 	return dse.val;
 }
#1 lore raw
Quoted 9 lines · Stephane Eranian

This patches 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.

It's not enough. as I said it needs more fixes.

For most events it should be _NA, _STORE can be only specified for the explicit _STORE events.

-Andi

Quoted 23 lines
arch/x86/kernel/cpu/perf_event_intel_ds.c +6 -2
index ae96cfa..81424f6 100644
@@ -114,9 +114,13 @@ static u64 precise_store_data_hsw(u64 status)
 
 	dse.val = 0;
 	dse.mem_op = PERF_MEM_OP_STORE;
-	dse.mem_lvl = PERF_MEM_LVL_NA;
+	dse.mem_lvl = PERF_MEM_LVL_L1;
+
 	if (status & 1)
-		dse.mem_lvl = PERF_MEM_LVL_L1;
+		dse.mem_lvl |= PERF_MEM_LVL_HIT;
+	else
+		dse.mem_lvl |= PERF_MEM_LVL_MISS;
+
 	/* Nothing else supported. Sorry. */
 	return dse.val;
 }
Signature · 1 line
[email protected] -- Speaking for myself only
#2 lore raw
Quoted 9 lines · Andi Kleen
Quoted 8 lines · Stephane Eranian

This patches 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 7 lines
Quoted 1 line

This patch encodes the memory level according to the specification.

It's not enough. as I said it needs more fixes.

For most events it should be _NA, _STORE can be only specified for the explicit _STORE events.

Yes, I forgot about that. Will re-spin the patch.

Quoted 28 lines

-Andi

Quoted 13 lines
arch/x86/kernel/cpu/perf_event_intel_ds.c +1 -1
index ae96cfa..81424f6 100644
@@ -114,9 +114,13 @@ static u64 precise_store_data_hsw(u64 status)
 
      dse.val = 0;
      dse.mem_op = PERF_MEM_OP_STORE;
-     dse.mem_lvl = PERF_MEM_LVL_NA;
+     dse.mem_lvl = PERF_MEM_LVL_L1;

arch/x86/kernel/cpu/perf_event_intel_ds.c static u64 precise_store_data_hsw(u64 status) · line 126

Quoted 10 lines
arch/x86/kernel/cpu/perf_event_intel_ds.c +5 -1
+
      if (status & 1)
-             dse.mem_lvl = PERF_MEM_LVL_L1;
+             dse.mem_lvl |= PERF_MEM_LVL_HIT;
+     else
+             dse.mem_lvl |= PERF_MEM_LVL_MISS;
+
      /* Nothing else supported. Sorry. */
      return dse.val;
 }

--
[email protected] -- Speaking for myself only