Skip to contentSkip to conversation

perf/x86/intel/uncore: Revert "Remove SBOX support for Broadwell server"

PATCH RESEND In review Kan Liang 4 participants 7 messages Mar 23, 2018 – Apr 20, 2018

Subsystems (2)
x86 perf events
lore
Share & tools
lore

Kan Liang

author
#0 lore raw

PATCH RESEND 1/2 perf/x86/intel/uncore: Revert "Remove SBOX support for Broadwell server"

From: Stephane Eranian <[email protected]>

This reverts commit 3b94a891667c ("perf/x86/intel/uncore: Remove SBOX support for Broadwell server")

Rvert because we have a workaround for Broadwell-EP servers without SBOX (subsequent patch). Note that BDX-DE does not have a SBOX.

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

---

Here is the original patch set.
https://lkml.org/lkml/2016/11/15/611
Only rebase with lastest code.

 arch/x86/events/intel/uncore_snbep.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
arch/x86/events/intel/uncore_snbep.c +21 -0
index 22ec65b..47c6910 100644
@@ -3028,10 +3028,27 @@ static struct intel_uncore_type bdx_uncore_cbox = {
 	.format_group		= &hswep_uncore_cbox_format_group,
 };
 
+static struct intel_uncore_type bdx_uncore_sbox = {
+	.name			= "sbox",
+	.num_counters		= 4,
+	.num_boxes		= 4,
+	.perf_ctr_bits		= 48,
+	.event_ctl		= HSWEP_S0_MSR_PMON_CTL0,
+	.perf_ctr		= HSWEP_S0_MSR_PMON_CTR0,
+	.event_mask		= HSWEP_S_MSR_PMON_RAW_EVENT_MASK,
+	.box_ctl		= HSWEP_S0_MSR_PMON_BOX_CTL,
+	.msr_offset		= HSWEP_SBOX_MSR_OFFSET,
+	.ops			= &hswep_uncore_sbox_msr_ops,
+	.format_group		= &hswep_uncore_sbox_format_group,
+};
+
+#define BDX_MSR_UNCORE_SBOX	3
+
 static struct intel_uncore_type *bdx_msr_uncores[] = {
 	&bdx_uncore_ubox,
 	&bdx_uncore_cbox,
 	&hswep_uncore_pcu,
+	&bdx_uncore_sbox,
 	NULL,
 };
 

@@ -3047,6 +3064,10 @@ void bdx_uncore_cpu_init(void)
 		bdx_uncore_cbox.num_boxes = boot_cpu_data.x86_max_cores;
 	uncore_msr_uncores = bdx_msr_uncores;
 
+	/* BDX-DE doesn't have SBOX */
+	if (boot_cpu_data.x86_model == 86)
+		uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+
 	hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
 }
Signature · 1 line
2.4.11
author
#1 lore raw

PATCH RESEND 2/2 perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

From: Oskar Senft <[email protected]>

This fixes SBOX support for Broadwell CPUs by checking the Power Control Unit CAPID4 register to determine the number of available SBOXes on the particular CPU before trying to enable them.

Signed-off-by Oskar Senft <[email protected]>
Tested-by Mark van Dijk <[email protected]>
Reviewed-by Kan Liang <[email protected]>
---
 arch/x86/events/intel/uncore_snbep.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
arch/x86/events/intel/uncore_snbep.c +16 -1
index 47c6910..715eb14 100644
@@ -3060,6 +3060,8 @@ static struct event_constraint bdx_uncore_pcu_constraints[] = {
 
 void bdx_uncore_cpu_init(void)
 {
+	int pkg = topology_phys_to_logical_pkg(0);
+
 	if (bdx_uncore_cbox.num_boxes > boot_cpu_data.x86_max_cores)
 		bdx_uncore_cbox.num_boxes = boot_cpu_data.x86_max_cores;
 	uncore_msr_uncores = bdx_msr_uncores;

@@ -3067,7 +3069,15 @@ void bdx_uncore_cpu_init(void)
 	/* BDX-DE doesn't have SBOX */
 	if (boot_cpu_data.x86_model == 86)
 		uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
-
+	/* Detect systems with no SBOXes */
+	else if (uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]) {
+		u32 capid4;
+		pci_read_config_dword(
+			uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3],
+			0x94, &capid4);
+		if (((capid4 >> 6) & 0x3) == 0)
+			bdx_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+	}
 	hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
 }
 

@@ -3285,6 +3295,11 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46),
 		.driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2),
 	},
+	{ /* PCU.3 (for Capability registers) */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0),
+		.driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV,
+						   HSWEP_PCI_PCU_3),
+	},
 	{ /* end: all zeroes */ }
 };
Signature · 1 line
2.4.11
#2 lore raw

Re: [RESEND PATCH 2/2] perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

Quoted 9 lines · Kan Liang

From: Oskar Senft <[email protected]>

This fixes SBOX support for Broadwell CPUs by checking the Power Control Unit CAPID4 register to determine the number of available SBOXes on the particular CPU before trying to enable them.

Signed-off-by Oskar Senft <[email protected]>
Tested-by Mark van Dijk <[email protected]>
Reviewed-by Kan Liang <[email protected]>

Could you please merge this change?
Thanks.

Review

Quoted 5 lines
---
  arch/x86/events/intel/uncore_snbep.c | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)
(quoted diff) +0 -0

b/arch/x86/events/intel/uncore_snbep.c

arch/x86/events/intel/uncore_snbep.c static struct event_constraint

Quoted 4 lines

index 47c6910..715eb14 100644

arch/x86/events/intel/uncore_snbep.c +0 -0
@@ -3060,6 +3060,8 @@ static struct event_constraint

bdx_uncore_pcu_constraints[] = {

arch/x86/events/intel/uncore_snbep.c static const struct pci_device_id

Quoted 25 lines
arch/x86/events/intel/uncore_snbep.c +11 -1
  void bdx_uncore_cpu_init(void)
  {
+       int pkg = topology_phys_to_logical_pkg(0);
+
         if (bdx_uncore_cbox.num_boxes > boot_cpu_data.x86_max_cores)
                 bdx_uncore_cbox.num_boxes = boot_cpu_data.x86_max_cores;
         uncore_msr_uncores = bdx_msr_uncores;

@@ -3067,7 +3069,15 @@ void bdx_uncore_cpu_init(void)
         /* BDX-DE doesn't have SBOX */
         if (boot_cpu_data.x86_model == 86)
                 uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
-
+       /* Detect systems with no SBOXes */
+       else if (uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]) {
+               u32 capid4;
+               pci_read_config_dword(
+                       uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3],
+                       0x94, &capid4);
+               if (((capid4 >> 6) & 0x3) == 0)
+                       bdx_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+       }
         hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
  }
 

@@ -3285,6 +3295,11 @@ static const struct pci_device_id

bdx_uncore_pci_ids[] = {

arch/x86/events/intel/uncore_snbep.c static const struct pci_device_id · line 3296

Quoted 2 lines
arch/x86/events/intel/uncore_snbep.c +0 -0
                 PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46),
                 .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV,

2),

Quoted 11 lines
arch/x86/events/intel/uncore_snbep.c +5 -1
         },
+       { /* PCU.3 (for Capability registers) */
+               PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0),
+               .driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV,
+                                                  HSWEP_PCI_PCU_3),
+       },
         { /* end: all zeroes */ }
  };
 
--

2.4.11

maintainer Acked-by on v1 2/2
#3 lore raw

Re: [RESEND PATCH 2/2] perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

In reply to Kan Liang

arch/x86/events/intel/uncore_snbep.c void bdx_uncore_cpu_init(void) · line 3072

Quoted 6 lines · Kan Liang
arch/x86/events/intel/uncore_snbep.c +1 -1
@@ -3067,7 +3069,15 @@ void bdx_uncore_cpu_init(void)
 	/* BDX-DE doesn't have SBOX */
 	if (boot_cpu_data.x86_model == 86)
 		uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
-
+	/* Detect systems with no SBOXes */

That really wants { }.

arch/x86/events/intel/uncore_snbep.c void bdx_uncore_cpu_init(void) · line 3082

Quoted 10 lines
arch/x86/events/intel/uncore_snbep.c +8 -0
+	else if (uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]) {
+		u32 capid4;
+		pci_read_config_dword(
+			uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3],
+			0x94, &capid4);
+		if (((capid4 >> 6) & 0x3) == 0)
+			bdx_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+	}
 	hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
 }

Otherwise these look fine to me;

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

Thomas will you magic them into tip?

merge bot
#4 lore raw

[tip:perf/urgent] perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

In reply to Kan Liang
perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs
Commit-ID:  ca2f6779608e364443cd82d31b1efd8b76f23bf6
Gitweb:     https://git.kernel.org/tip/ca2f6779608e364443cd82d31b1efd8b76f23bf6
Author:     Oskar Senft <[email protected]>
AuthorDate: Fri, 23 Mar 2018 09:11:30 -0400
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 20 Apr 2018 12:41:17 +0200

perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

SBOX on some Broadwell CPUs is broken because it's enabled unconditionally despite the fact that there are no SBOXes available.

Check the Power Control Unit CAPID4 register to determine the number of available SBOXes on the particular CPU before trying to enable them. If there are none, nullify the SBOX descriptor so it isn't tried to be initialized.

Signed-off-by Oskar Senft <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Tested-by Mark van Dijk <[email protected]>
Reviewed-by Kan Liang <[email protected]>
Acked-by Peter Zijlstra <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Link https://lkml.kernel.org/r/[email protected] open here
---
 arch/x86/events/intel/uncore_snbep.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/x86/events/intel/uncore_snbep.c +16 -0
index 5bbbbee11879..37e809d457c6 100644
@@ -3060,6 +3060,8 @@ static struct event_constraint bdx_uncore_pcu_constraints[] = {
 
 void bdx_uncore_cpu_init(void)
 {
+	int pkg = topology_phys_to_logical_pkg(0);
+
 	if (bdx_uncore_cbox.num_boxes > boot_cpu_data.x86_max_cores)
 		bdx_uncore_cbox.num_boxes = boot_cpu_data.x86_max_cores;
 	uncore_msr_uncores = bdx_msr_uncores;

@@ -3067,7 +3069,16 @@ void bdx_uncore_cpu_init(void)
 	/* BDX-DE doesn't have SBOX */
 	if (boot_cpu_data.x86_model == 86)
 		uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+	/* Detect systems with no SBOXes */
+	else if (uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]) {
+		struct pci_dev *pdev;
+		u32 capid4;
 
+		pdev = uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3];
+		pci_read_config_dword(pdev, 0x94, &capid4);
+		if (((capid4 >> 6) & 0x3) == 0)
+			bdx_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+	}
 	hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
 }
 

@@ -3285,6 +3296,11 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46),
 		.driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2),
 	},
+	{ /* PCU.3 (for Capability registers) */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0),
+		.driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV,
+						   HSWEP_PCI_PCU_3),
+	},
 	{ /* end: all zeroes */ }
 };
merge bot
#5 lore raw

[tip:perf/urgent] perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

In reply to Kan Liang
perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs
Commit-ID:  15a3e845b01ce2342cf187dc123c92c44c3c8170
Gitweb:     https://git.kernel.org/tip/15a3e845b01ce2342cf187dc123c92c44c3c8170
Author:     Oskar Senft <[email protected]>
AuthorDate: Fri, 23 Mar 2018 09:11:30 -0400
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 20 Apr 2018 13:17:50 +0200

perf/x86/intel/uncore: Fix SBOX support for Broadwell CPUs

SBOX on some Broadwell CPUs is broken because it's enabled unconditionally despite the fact that there are no SBOXes available.

Check the Power Control Unit CAPID4 register to determine the number of available SBOXes on the particular CPU before trying to enable them. If there are none, nullify the SBOX descriptor so it isn't tried to be initialized.

Signed-off-by Oskar Senft <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Tested-by Mark van Dijk <[email protected]>
Reviewed-by Kan Liang <[email protected]>
Acked-by Peter Zijlstra <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Link https://lkml.kernel.org/r/[email protected] open here
---
 arch/x86/events/intel/uncore_snbep.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
arch/x86/events/intel/uncore_snbep.c +17 -1
index 5bbbbee11879..77076a102e34 100644
@@ -3060,14 +3060,25 @@ static struct event_constraint bdx_uncore_pcu_constraints[] = {
 
 void bdx_uncore_cpu_init(void)
 {
+	int pkg = topology_phys_to_logical_pkg(0);
+
 	if (bdx_uncore_cbox.num_boxes > boot_cpu_data.x86_max_cores)
 		bdx_uncore_cbox.num_boxes = boot_cpu_data.x86_max_cores;
 	uncore_msr_uncores = bdx_msr_uncores;
 
 	/* BDX-DE doesn't have SBOX */
-	if (boot_cpu_data.x86_model == 86)
+	if (boot_cpu_data.x86_model == 86) {
 		uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+	/* Detect systems with no SBOXes */
+	} else if (uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3]) {
+		struct pci_dev *pdev;
+		u32 capid4;
 
+		pdev = uncore_extra_pci_dev[pkg].dev[HSWEP_PCI_PCU_3];
+		pci_read_config_dword(pdev, 0x94, &capid4);
+		if (((capid4 >> 6) & 0x3) == 0)
+			bdx_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+	}
 	hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
 }
 

@@ -3285,6 +3296,11 @@ static const struct pci_device_id bdx_uncore_pci_ids[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6f46),
 		.driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV, 2),
 	},
+	{ /* PCU.3 (for Capability registers) */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x6fc0),
+		.driver_data = UNCORE_PCI_DEV_DATA(UNCORE_EXTRA_PCI_DEV,
+						   HSWEP_PCI_PCU_3),
+	},
 	{ /* end: all zeroes */ }
 };
merge bot
#6 lore raw
In reply to Kan Liang
perf/x86/intel/uncore: Revert "Remove SBOX support for Broadwell server"
Commit-ID:  d7717587ac6deae00e0b66c0113a046be2c6fb1c
Gitweb:     https://git.kernel.org/tip/d7717587ac6deae00e0b66c0113a046be2c6fb1c
Author:     Stephane Eranian <[email protected]>
AuthorDate: Fri, 23 Mar 2018 09:11:29 -0400
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Fri, 20 Apr 2018 12:41:17 +0200

perf/x86/intel/uncore: Revert "Remove SBOX support for Broadwell server"

This reverts commit 3b94a891667c ("perf/x86/intel/uncore: Remove SBOX support for Broadwell server")

Revert because there exists a proper workaround for Broadwell-EP servers without SBOX now. Note that BDX-DE does not have a SBOX.

Signed-off-by Stephane Eranian <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Kan Liang <[email protected]>
Acked-by Peter Zijlstra <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
Link https://lkml.kernel.org/r/[email protected] open here
---
 arch/x86/events/intel/uncore_snbep.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
arch/x86/events/intel/uncore_snbep.c +21 -0
index c98b943e58b4..5bbbbee11879 100644
@@ -3028,10 +3028,27 @@ static struct intel_uncore_type bdx_uncore_cbox = {
 	.format_group		= &hswep_uncore_cbox_format_group,
 };
 
+static struct intel_uncore_type bdx_uncore_sbox = {
+	.name			= "sbox",
+	.num_counters		= 4,
+	.num_boxes		= 4,
+	.perf_ctr_bits		= 48,
+	.event_ctl		= HSWEP_S0_MSR_PMON_CTL0,
+	.perf_ctr		= HSWEP_S0_MSR_PMON_CTR0,
+	.event_mask		= HSWEP_S_MSR_PMON_RAW_EVENT_MASK,
+	.box_ctl		= HSWEP_S0_MSR_PMON_BOX_CTL,
+	.msr_offset		= HSWEP_SBOX_MSR_OFFSET,
+	.ops			= &hswep_uncore_sbox_msr_ops,
+	.format_group		= &hswep_uncore_sbox_format_group,
+};
+
+#define BDX_MSR_UNCORE_SBOX	3
+
 static struct intel_uncore_type *bdx_msr_uncores[] = {
 	&bdx_uncore_ubox,
 	&bdx_uncore_cbox,
 	&hswep_uncore_pcu,
+	&bdx_uncore_sbox,
 	NULL,
 };
 

@@ -3047,6 +3064,10 @@ void bdx_uncore_cpu_init(void)
 		bdx_uncore_cbox.num_boxes = boot_cpu_data.x86_max_cores;
 	uncore_msr_uncores = bdx_msr_uncores;
 
+	/* BDX-DE doesn't have SBOX */
+	if (boot_cpu_data.x86_model == 86)
+		uncore_msr_uncores[BDX_MSR_UNCORE_SBOX] = NULL;
+
 	hswep_uncore_pcu.constraints = bdx_uncore_pcu_constraints;
 }