Fixes for the Haswell EP uncore driver

PATCH New Andi Kleen 2 participants 7 messages Nov 4, 2014 – Nov 16, 2014

Subsystems (2)

x86 perf events

lore

Share & tools

lore

Andi Kleen

author

2014-11-04 01:00 UTC #0 lore raw

The first patch is the "proper" fix for the SBOX boot crash on some system. It replaces the disabling patch I submitted earlier. If we write the initialization bits in different WRMSRs the problem goes away.

Also fix another problem with wrong register offsets in the IRP unit driver, thanks to Patrick Lu.

Finally fix the units for the memory controller (third patch is optional, but harmless)

-Andi

Andi Kleen

author

2014-11-04 01:00 UTC #1 lore raw

PATCH 1/3 perf, x86: Add scaling units to the EP iMC events

From: Andi Kleen <[email protected]>

Add scaling to MB/s to the memory controller read/write events for Sandy/IvyBridge/Haswell-EP similar to how the client does. This makes the events easier to use from the standard perf tool.

Signed-off-by Andi Kleen <[email protected]>

---
 arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c | 8 ++++++++
 1 file changed, 8 insertions(+)

arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c +8 -0

index 22c6272..286c692 100644
@@ -449,7 +449,11 @@ static struct attribute *snbep_uncore_qpi_formats_attr[] = {
 static struct uncore_event_desc snbep_uncore_imc_events[] = {
 	INTEL_UNCORE_EVENT_DESC(clockticks,      "event=0xff,umask=0x00"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_read,  "event=0x04,umask=0x03"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.unit, "MiB"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_write, "event=0x04,umask=0x0c"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.unit, "MiB"),
 	{ /* end: all zeroes */ },
 };

@@ -2009,7 +2013,11 @@ static struct intel_uncore_type hswep_uncore_ha = {
 static struct uncore_event_desc hswep_uncore_imc_events[] = {
 	INTEL_UNCORE_EVENT_DESC(clockticks,      "event=0x00,umask=0x00"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_read,  "event=0x04,umask=0x03"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.unit, "MiB"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_write, "event=0x04,umask=0x0c"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.unit, "MiB"),
 	{ /* end: all zeroes */ },
 };

Signature · 1 line

1.9.3

tip-bot for Andi Kleen

merge bot

2014-11-16 12:35 UTC #2 lore raw

[tip:perf/core] perf/x86/intel/uncore: Add scaling units to the EP iMC events

perf/x86/intel/uncore: Add scaling units to the EP iMC events

Commit-ID:  c0737ce453bdccd0d09aa4f916795533a4e02fef
Gitweb:     http://git.kernel.org/tip/c0737ce453bdccd0d09aa4f916795533a4e02fef
Author:     Andi Kleen <[email protected]>
AuthorDate: Mon, 3 Nov 2014 17:00:26 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sun, 16 Nov 2014 11:41:52 +0100

perf/x86/intel/uncore: Add scaling units to the EP iMC events

Add scaling to MB/s to the memory controller read/write events for Sandy/IvyBridge/Haswell-EP similar to how the client does. This makes the events easier to use from the standard perf tool.

Signed-off-by Andi Kleen <[email protected]>

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

Cc Arnaldo Carvalho de Melo <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c | 8 ++++++++
 1 file changed, 8 insertions(+)

arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c +8 -0

index f9ed429..745b158 100644
@@ -449,7 +449,11 @@ static struct attribute *snbep_uncore_qpi_formats_attr[] = {
 static struct uncore_event_desc snbep_uncore_imc_events[] = {
 	INTEL_UNCORE_EVENT_DESC(clockticks,      "event=0xff,umask=0x00"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_read,  "event=0x04,umask=0x03"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.unit, "MiB"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_write, "event=0x04,umask=0x0c"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.unit, "MiB"),
 	{ /* end: all zeroes */ },
 };

@@ -2036,7 +2040,11 @@ static struct intel_uncore_type hswep_uncore_ha = {
 static struct uncore_event_desc hswep_uncore_imc_events[] = {
 	INTEL_UNCORE_EVENT_DESC(clockticks,      "event=0x00,umask=0x00"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_read,  "event=0x04,umask=0x03"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_read.unit, "MiB"),
 	INTEL_UNCORE_EVENT_DESC(cas_count_write, "event=0x04,umask=0x0c"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.scale, "6.103515625e-5"),
+	INTEL_UNCORE_EVENT_DESC(cas_count_write.unit, "MiB"),
 	{ /* end: all zeroes */ },
 };

Andi Kleen

author

2014-11-04 01:00 UTC #3 lore raw

PATCH 2/3 perf, x86, uncore: Fix IRP uncore register offsets on Haswell EP

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

The counter register offsets for the IRP box PMU for Haswell-EP were incorrect. The offsets actually changed over IvyBridge EP.

Fix them to the correct values. For this we need to fork the read function from the IVB and use an own counter array.

Thanks to Patrick Lu.

Tested-by [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
 arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c +15 -1

index 286c692..04e9a83 100644
@@ -2033,13 +2033,27 @@ static struct intel_uncore_type hswep_uncore_imc = {
 	SNBEP_UNCORE_PCI_COMMON_INIT(),
 };
 
+static unsigned hswep_uncore_irp_ctrs[] = {0xa0, 0xa8, 0xb0, 0xb8};
+
+static u64 hswep_uncore_irp_read_counter(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct pci_dev *pdev = box->pci_dev;
+	struct hw_perf_event *hwc = &event->hw;
+	u64 count = 0;
+
+	pci_read_config_dword(pdev, hswep_uncore_irp_ctrs[hwc->idx], (u32 *)&count);
+	pci_read_config_dword(pdev, hswep_uncore_irp_ctrs[hwc->idx] + 4, (u32 *)&count + 1);
+
+	return count;
+}
+
 static struct intel_uncore_ops hswep_uncore_irp_ops = {
 	.init_box	= snbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
 	.disable_event	= ivbep_uncore_irp_disable_event,
 	.enable_event	= ivbep_uncore_irp_enable_event,
-	.read_counter	= ivbep_uncore_irp_read_counter,
+	.read_counter	= hswep_uncore_irp_read_counter,
 };
 
 static struct intel_uncore_type hswep_uncore_irp = {

Signature · 1 line

1.9.3

tip-bot for Andi Kleen

merge bot

2014-11-16 09:50 UTC #4 lore raw

[tip:perf/urgent] perf/x86/intel/uncore: Fix IRP uncore register offsets on Haswell EP

perf/x86/intel/uncore: Fix IRP uncore register offsets on Haswell EP

Commit-ID:  41a134a5830a5e1396723ace0a63000780d6e267
Gitweb:     http://git.kernel.org/tip/41a134a5830a5e1396723ace0a63000780d6e267
Author:     Andi Kleen <[email protected]>
AuthorDate: Mon, 3 Nov 2014 17:00:27 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sun, 16 Nov 2014 09:45:47 +0100

perf/x86/intel/uncore: Fix IRP uncore register offsets on Haswell EP

The counter register offsets for the IRP box PMU for Haswell-EP were incorrect. The offsets actually changed over IvyBridge EP.

Fix them to the correct values. For this we need to fork the read function from the IVB and use an own counter array.

Tested-by [email protected]

Signed-off-by Andi Kleen <[email protected]>

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

Cc Arnaldo Carvalho de Melo <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c +15 -1

index adf138e..0af1c93 100644
@@ -2025,13 +2025,27 @@ static struct intel_uncore_type hswep_uncore_imc = {
 	SNBEP_UNCORE_PCI_COMMON_INIT(),
 };
 
+static unsigned hswep_uncore_irp_ctrs[] = {0xa0, 0xa8, 0xb0, 0xb8};
+
+static u64 hswep_uncore_irp_read_counter(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct pci_dev *pdev = box->pci_dev;
+	struct hw_perf_event *hwc = &event->hw;
+	u64 count = 0;
+
+	pci_read_config_dword(pdev, hswep_uncore_irp_ctrs[hwc->idx], (u32 *)&count);
+	pci_read_config_dword(pdev, hswep_uncore_irp_ctrs[hwc->idx] + 4, (u32 *)&count + 1);
+
+	return count;
+}
+
 static struct intel_uncore_ops hswep_uncore_irp_ops = {
 	.init_box	= snbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
 	.disable_event	= ivbep_uncore_irp_disable_event,
 	.enable_event	= ivbep_uncore_irp_enable_event,
-	.read_counter	= ivbep_uncore_irp_read_counter,
+	.read_counter	= hswep_uncore_irp_read_counter,
 };
 
 static struct intel_uncore_type hswep_uncore_irp = {

Andi Kleen

author

2014-11-04 01:00 UTC #5 lore raw

PATCH 3/3 perf, x86, uncore: Fix SBOX PMU on Haswell-EP to fix boot crash

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

There were several reports that on some systems writing the SBOX0 PMU initialization MSR would #GP at boot. This did not happen on all systems -- my two test systems booted fine.

Writing the three initialization bits bit-by-bit seems to avoid the problem. So add a special callback to do just that.

This replaces an earlier patch that disabled the SBOX.

Tested-by [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
 .../x86/kernel/cpu/perf_event_intel_uncore_snbep.c | 33 ++++++++++++++++++++--
 1 file changed, 30 insertions(+), 3 deletions(-)

arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c +30 -3

index 04e9a83..236dde2 100644
@@ -490,14 +490,17 @@ static struct attribute_group snbep_uncore_qpi_format_group = {
 	.attrs = snbep_uncore_qpi_formats_attr,
 };
 
-#define SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
-	.init_box	= snbep_uncore_msr_init_box,		\
+#define __SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
 	.disable_box	= snbep_uncore_msr_disable_box,		\
 	.enable_box	= snbep_uncore_msr_enable_box,		\
 	.disable_event	= snbep_uncore_msr_disable_event,	\
 	.enable_event	= snbep_uncore_msr_enable_event,	\
 	.read_counter	= uncore_msr_read_counter
 
+#define SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
+	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),			\
+	.init_box	= snbep_uncore_msr_init_box		\
+
 static struct intel_uncore_ops snbep_uncore_msr_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 };

@@ -1923,6 +1926,30 @@ static struct intel_uncore_type hswep_uncore_cbox = {
 	.format_group		= &hswep_uncore_cbox_format_group,
 };
 
+/*
+ * Write SBOX Initialization register bit by bit to avoid spurious #GPs
+ */
+static void hswep_uncore_sbox_msr_init_box(struct intel_uncore_box *box)
+{
+	unsigned msr = uncore_msr_box_ctl(box);
+
+	if (msr) {
+		u64 init = SNBEP_PMON_BOX_CTL_INT;
+		u64 flags = 0;
+		int i;
+
+		for_each_set_bit (i, (unsigned long *)&init, 64) {
+			flags |= (1ULL << i);
+			wrmsrl(msr, flags);
+		}
+	}
+}
+
+static struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
+	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
+	.init_box		= hswep_uncore_sbox_msr_init_box
+};
+
 static struct attribute *hswep_uncore_sbox_formats_attr[] = {
 	&format_attr_event.attr,
 	&format_attr_umask.attr,

@@ -1948,7 +1975,7 @@ static struct intel_uncore_type hswep_uncore_sbox = {
 	.event_mask		= HSWEP_S_MSR_PMON_RAW_EVENT_MASK,
 	.box_ctl		= HSWEP_S0_MSR_PMON_BOX_CTL,
 	.msr_offset		= HSWEP_SBOX_MSR_OFFSET,
-	.ops			= &snbep_uncore_msr_ops,
+	.ops			= &hswep_uncore_sbox_msr_ops,
 	.format_group		= &hswep_uncore_sbox_format_group,
 };

Signature · 1 line

1.9.3

tip-bot for Andi Kleen

merge bot

2014-11-16 09:50 UTC #6 lore raw

[tip:perf/urgent] perf/x86/intel/uncore: Fix boot crash on SBOX PMU on Haswell-EP

perf/x86/intel/uncore: Fix boot crash on SBOX PMU on Haswell-EP

Commit-ID:  68055915c1c22489f9658bd2b7391bb11b2cf4e4
Gitweb:     http://git.kernel.org/tip/68055915c1c22489f9658bd2b7391bb11b2cf4e4
Author:     Andi Kleen <[email protected]>
AuthorDate: Mon, 3 Nov 2014 17:00:28 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sun, 16 Nov 2014 09:53:36 +0100

perf/x86/intel/uncore: Fix boot crash on SBOX PMU on Haswell-EP

There were several reports that on some systems writing the SBOX0 PMU initialization MSR would #GP at boot. This did not happen on all systems -- my two test systems booted fine.

Writing the three initialization bits bit-by-bit seems to avoid the problem. So add a special callback to do just that.

This replaces an earlier patch that disabled the SBOX.

Reported-by Alexei Starovoitov <[email protected]>

Reported-and-Tested-by Patrick Lu <[email protected]>

Signed-off-by Andi Kleen <[email protected]>

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

Cc Arnaldo Carvalho de Melo <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ Fixed a whitespace error and added attribution tags that were left out inexplicably. ]

Signed-off-by Ingo Molnar <[email protected]>

---
 .../x86/kernel/cpu/perf_event_intel_uncore_snbep.c | 33 ++++++++++++++++++++--
 1 file changed, 30 insertions(+), 3 deletions(-)

arch/x86/kernel/cpu/perf_event_intel_uncore_snbep.c +30 -3

index 0af1c93..f9ed429 100644
@@ -486,14 +486,17 @@ static struct attribute_group snbep_uncore_qpi_format_group = {
 	.attrs = snbep_uncore_qpi_formats_attr,
 };
 
-#define SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
-	.init_box	= snbep_uncore_msr_init_box,		\
+#define __SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
 	.disable_box	= snbep_uncore_msr_disable_box,		\
 	.enable_box	= snbep_uncore_msr_enable_box,		\
 	.disable_event	= snbep_uncore_msr_disable_event,	\
 	.enable_event	= snbep_uncore_msr_enable_event,	\
 	.read_counter	= uncore_msr_read_counter
 
+#define SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
+	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),			\
+	.init_box	= snbep_uncore_msr_init_box		\
+
 static struct intel_uncore_ops snbep_uncore_msr_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 };

@@ -1919,6 +1922,30 @@ static struct intel_uncore_type hswep_uncore_cbox = {
 	.format_group		= &hswep_uncore_cbox_format_group,
 };
 
+/*
+ * Write SBOX Initialization register bit by bit to avoid spurious #GPs
+ */
+static void hswep_uncore_sbox_msr_init_box(struct intel_uncore_box *box)
+{
+	unsigned msr = uncore_msr_box_ctl(box);
+
+	if (msr) {
+		u64 init = SNBEP_PMON_BOX_CTL_INT;
+		u64 flags = 0;
+		int i;
+
+		for_each_set_bit(i, (unsigned long *)&init, 64) {
+			flags |= (1ULL << i);
+			wrmsrl(msr, flags);
+		}
+	}
+}
+
+static struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
+	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
+	.init_box		= hswep_uncore_sbox_msr_init_box
+};
+
 static struct attribute *hswep_uncore_sbox_formats_attr[] = {
 	&format_attr_event.attr,
 	&format_attr_umask.attr,

@@ -1944,7 +1971,7 @@ static struct intel_uncore_type hswep_uncore_sbox = {
 	.event_mask		= HSWEP_S_MSR_PMON_RAW_EVENT_MASK,
 	.box_ctl		= HSWEP_S0_MSR_PMON_BOX_CTL,
 	.msr_offset		= HSWEP_SBOX_MSR_OFFSET,
-	.ops			= &snbep_uncore_msr_ops,
+	.ops			= &hswep_uncore_sbox_msr_ops,
 	.format_group		= &hswep_uncore_sbox_format_group,
 };