[RFC PATCH v9 0/7] Cavium ARM64 uncore PMU support

[RFC PATCH v9 0/7] Cavium ARM64 uncore PMU support

[Jan Glauber]

I'm posting this as RFC following this discussion:
https://marc.info/?l=linux-arm-kernel&m=150099526923838&w=2

I've implemented the wrapper for the PCI devices and put it under
drivers/soc/cavium which I found more appropriate than drivers/misc.

I was not able to find a way to build the EDAC driver and the PMU driver
with all combinations (builtin and module) so I limited the build options
to module only. The problem is that the select from EDAC or PMU
sets the wrappers build type to whatever EDAC or PMU choose.
But all parts must be either built-in or modules, having the wrapper
builtin and calling into module code will not work. If there is a better
solution please let me know.

The PMU code is the same as in v8.

Add support for various PMU counters found on the Cavium ThunderX and
OcteonTx SoC.

The PMU driver provides common "uncore" functions to avoid code duplication
and support adding more device PMUs (like L2 cache) in the future.

Changes to v8:
- Wrapper for PCI devices

Jan Glauber (7):
  edac: thunderx: Remove suspend/resume support
  edac,soc: thunderx: Add wrapper for EDAC LMC PCI device
  edac,soc: thunderx: Add wrapper for EDAC OCX PCI device
  perf: export perf_event_update_userpage()
  perf: cavium: Support memory controller PMU counters
  perf: cavium: Support transmit-link PMU counters
  perf: cavium: Add Documentation

 Documentation/perf/cavium-pmu.txt |  75 +++++
 drivers/edac/Kconfig              |   3 +
 drivers/edac/thunderx_edac.c      |  92 +-----
 drivers/perf/Kconfig              |  15 +
 drivers/perf/Makefile             |   1 +
 drivers/perf/cavium_pmu.c         | 680 ++++++++++++++++++++++++++++++++++++++
 drivers/soc/Kconfig               |   1 +
 drivers/soc/Makefile              |   1 +
 drivers/soc/cavium/Kconfig        |  14 +
 drivers/soc/cavium/Makefile       |   2 +
 drivers/soc/cavium/cavium_lmc.c   |  53 +++
 drivers/soc/cavium/cavium_ocx.c   |  49 +++
 include/linux/cpuhotplug.h        |   1 +
 include/linux/soc/cavium/lmc.h    |  12 +
 include/linux/soc/cavium/ocx.h    |  12 +
 kernel/events/core.c              |   1 +
 16 files changed, 933 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/perf/cavium-pmu.txt
 create mode 100644 drivers/perf/cavium_pmu.c
 create mode 100644 drivers/soc/cavium/Kconfig
 create mode 100644 drivers/soc/cavium/Makefile
 create mode 100644 drivers/soc/cavium/cavium_lmc.c
 create mode 100644 drivers/soc/cavium/cavium_ocx.c
 create mode 100644 include/linux/soc/cavium/lmc.h
 create mode 100644 include/linux/soc/cavium/ocx.h

-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 0/7] Cavium ARM64 uncore PMU support

[Jan Glauber]

I'm posting this as RFC following this discussion:
https://marc.info/?l=linux-arm-kernel&m=150099526923838&w=2

I've implemented the wrapper for the PCI devices and put it under
drivers/soc/cavium which I found more appropriate than drivers/misc.

I was not able to find a way to build the EDAC driver and the PMU driver
with all combinations (builtin and module) so I limited the build options
to module only. The problem is that the select from EDAC or PMU
sets the wrappers build type to whatever EDAC or PMU choose.
But all parts must be either built-in or modules, having the wrapper
builtin and calling into module code will not work. If there is a better
solution please let me know.

The PMU code is the same as in v8.

Add support for various PMU counters found on the Cavium ThunderX and
OcteonTx SoC.

The PMU driver provides common "uncore" functions to avoid code duplication
and support adding more device PMUs (like L2 cache) in the future.

Changes to v8:
- Wrapper for PCI devices

Jan Glauber (7):
  edac: thunderx: Remove suspend/resume support
  edac,soc: thunderx: Add wrapper for EDAC LMC PCI device
  edac,soc: thunderx: Add wrapper for EDAC OCX PCI device
  perf: export perf_event_update_userpage()
  perf: cavium: Support memory controller PMU counters
  perf: cavium: Support transmit-link PMU counters
  perf: cavium: Add Documentation

 Documentation/perf/cavium-pmu.txt |  75 +++++
 drivers/edac/Kconfig              |   3 +
 drivers/edac/thunderx_edac.c      |  92 +-----
 drivers/perf/Kconfig              |  15 +
 drivers/perf/Makefile             |   1 +
 drivers/perf/cavium_pmu.c         | 680 ++++++++++++++++++++++++++++++++++++++
 drivers/soc/Kconfig               |   1 +
 drivers/soc/Makefile              |   1 +
 drivers/soc/cavium/Kconfig        |  14 +
 drivers/soc/cavium/Makefile       |   2 +
 drivers/soc/cavium/cavium_lmc.c   |  53 +++
 drivers/soc/cavium/cavium_ocx.c   |  49 +++
 include/linux/cpuhotplug.h        |   1 +
 include/linux/soc/cavium/lmc.h    |  12 +
 include/linux/soc/cavium/ocx.h    |  12 +
 kernel/events/core.c              |   1 +
 16 files changed, 933 insertions(+), 79 deletions(-)
 create mode 100644 Documentation/perf/cavium-pmu.txt
 create mode 100644 drivers/perf/cavium_pmu.c
 create mode 100644 drivers/soc/cavium/Kconfig
 create mode 100644 drivers/soc/cavium/Makefile
 create mode 100644 drivers/soc/cavium/cavium_lmc.c
 create mode 100644 drivers/soc/cavium/cavium_ocx.c
 create mode 100644 include/linux/soc/cavium/lmc.h
 create mode 100644 include/linux/soc/cavium/ocx.h

-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 1/7] edac: thunderx: Remove suspend/resume support

[Jan Glauber]

The memory controller on ThunderX/OcteonTX systems does not
support power management. Therefore remove the suspend/resume
callbacks.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/edac/thunderx_edac.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index 2d352b4..d02bf3b 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -639,27 +639,6 @@ static irqreturn_t thunderx_lmc_threaded_isr(int irq, void *dev_id)
 	return ret;
 }
 
-#ifdef CONFIG_PM
-static int thunderx_lmc_suspend(struct pci_dev *pdev, pm_message_t state)
-{
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
-	return 0;
-}
-
-static int thunderx_lmc_resume(struct pci_dev *pdev)
-{
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-
-	return 0;
-}
-#endif
-
 static const struct pci_device_id thunderx_lmc_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_LMC) },
 	{ 0, },
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 1/7] edac: thunderx: Remove suspend/resume support

[Jan Glauber]

The memory controller on ThunderX/OcteonTX systems does not
support power management. Therefore remove the suspend/resume
callbacks.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/edac/thunderx_edac.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index 2d352b4..d02bf3b 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -639,27 +639,6 @@ static irqreturn_t thunderx_lmc_threaded_isr(int irq, void *dev_id)
 	return ret;
 }
 
-#ifdef CONFIG_PM
-static int thunderx_lmc_suspend(struct pci_dev *pdev, pm_message_t state)
-{
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
-	return 0;
-}
-
-static int thunderx_lmc_resume(struct pci_dev *pdev)
-{
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-
-	return 0;
-}
-#endif
-
 static const struct pci_device_id thunderx_lmc_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_LMC) },
 	{ 0, },
-- 
2.9.0.rc0.21.g7777322

Re: [RFC PATCH v9 1/7] edac: thunderx: Remove suspend/resume support

[Borislav Petkov]

On Tue, Aug 29, 2017 at 03:12:32PM +0200, Jan Glauber wrote:
> The memory controller on ThunderX/OcteonTX systems does not
> support power management. Therefore remove the suspend/resume
> callbacks.
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  drivers/edac/thunderx_edac.c | 21 ---------------------
>  1 file changed, 21 deletions(-)

Just when I thought I'd pick that one up now because it is removing
stuff:

drivers/edac/thunderx_edac.c:817:14: error: ‘thunderx_lmc_suspend’ undeclared here (not in a function)
  .suspend  = thunderx_lmc_suspend,
              ^~~~~~~~~~~~~~~~~~~~
drivers/edac/thunderx_edac.c:818:14: error: ‘thunderx_lmc_resume’ undeclared here (not in a function)
  .resume   = thunderx_lmc_resume,
              ^~~~~~~~~~~~~~~~~~~
scripts/Makefile.build:308: recipe for target 'drivers/edac/thunderx_edac.o' failed
make[1]: *** [drivers/edac/thunderx_edac.o] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1682: recipe for target 'drivers/edac/' failed
make: *** [drivers/edac/] Error 2

Please make sure you build and test every patch before submitting.
You're lucky I can at least build arm64 on my x86 workstation. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[RFC PATCH v9 1/7] edac: thunderx: Remove suspend/resume support

[Borislav Petkov]

On Tue, Aug 29, 2017 at 03:12:32PM +0200, Jan Glauber wrote:
> The memory controller on ThunderX/OcteonTX systems does not
> support power management. Therefore remove the suspend/resume
> callbacks.
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  drivers/edac/thunderx_edac.c | 21 ---------------------
>  1 file changed, 21 deletions(-)

Just when I thought I'd pick that one up now because it is removing
stuff:

drivers/edac/thunderx_edac.c:817:14: error: ?thunderx_lmc_suspend? undeclared here (not in a function)
  .suspend  = thunderx_lmc_suspend,
              ^~~~~~~~~~~~~~~~~~~~
drivers/edac/thunderx_edac.c:818:14: error: ?thunderx_lmc_resume? undeclared here (not in a function)
  .resume   = thunderx_lmc_resume,
              ^~~~~~~~~~~~~~~~~~~
scripts/Makefile.build:308: recipe for target 'drivers/edac/thunderx_edac.o' failed
make[1]: *** [drivers/edac/thunderx_edac.o] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1682: recipe for target 'drivers/edac/' failed
make: *** [drivers/edac/] Error 2

Please make sure you build and test every patch before submitting.
You're lucky I can at least build arm64 on my x86 workstation. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH v9 1/7] edac: thunderx: Remove suspend/resume support

[Jan Glauber]

On Wed, Aug 30, 2017 at 07:54:06PM +0200, Borislav Petkov wrote:
> On Tue, Aug 29, 2017 at 03:12:32PM +0200, Jan Glauber wrote:
> > The memory controller on ThunderX/OcteonTX systems does not
> > support power management. Therefore remove the suspend/resume
> > callbacks.
> > 
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  drivers/edac/thunderx_edac.c | 21 ---------------------
> >  1 file changed, 21 deletions(-)
> 
> Just when I thought I'd pick that one up now because it is removing
> stuff:
> 
> drivers/edac/thunderx_edac.c:817:14: error: ‘thunderx_lmc_suspend’ undeclared here (not in a function)
>   .suspend  = thunderx_lmc_suspend,
>               ^~~~~~~~~~~~~~~~~~~~
> drivers/edac/thunderx_edac.c:818:14: error: ‘thunderx_lmc_resume’ undeclared here (not in a function)
>   .resume   = thunderx_lmc_resume,
>               ^~~~~~~~~~~~~~~~~~~
> scripts/Makefile.build:308: recipe for target 'drivers/edac/thunderx_edac.o' failed
> make[1]: *** [drivers/edac/thunderx_edac.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1682: recipe for target 'drivers/edac/' failed
> make: *** [drivers/edac/] Error 2

Argh... forgot to build test the single patches. 

> Please make sure you build and test every patch before submitting.
> You're lucky I can at least build arm64 on my x86 workstation. :-)

Sorry for that. The whole series builds because I removed the suspend/resume
callbacks during the move to the soc driver.

--Jan

> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.

[RFC PATCH v9 1/7] edac: thunderx: Remove suspend/resume support

[Jan Glauber]

On Wed, Aug 30, 2017 at 07:54:06PM +0200, Borislav Petkov wrote:
> On Tue, Aug 29, 2017 at 03:12:32PM +0200, Jan Glauber wrote:
> > The memory controller on ThunderX/OcteonTX systems does not
> > support power management. Therefore remove the suspend/resume
> > callbacks.
> > 
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  drivers/edac/thunderx_edac.c | 21 ---------------------
> >  1 file changed, 21 deletions(-)
> 
> Just when I thought I'd pick that one up now because it is removing
> stuff:
> 
> drivers/edac/thunderx_edac.c:817:14: error: ?thunderx_lmc_suspend? undeclared here (not in a function)
>   .suspend  = thunderx_lmc_suspend,
>               ^~~~~~~~~~~~~~~~~~~~
> drivers/edac/thunderx_edac.c:818:14: error: ?thunderx_lmc_resume? undeclared here (not in a function)
>   .resume   = thunderx_lmc_resume,
>               ^~~~~~~~~~~~~~~~~~~
> scripts/Makefile.build:308: recipe for target 'drivers/edac/thunderx_edac.o' failed
> make[1]: *** [drivers/edac/thunderx_edac.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1682: recipe for target 'drivers/edac/' failed
> make: *** [drivers/edac/] Error 2

Argh... forgot to build test the single patches. 

> Please make sure you build and test every patch before submitting.
> You're lucky I can at least build arm64 on my x86 workstation. :-)

Sorry for that. The whole series builds because I removed the suspend/resume
callbacks during the move to the soc driver.

--Jan

> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.

[RFC PATCH v9 2/7] edac,soc: thunderx: Add wrapper for EDAC LMC PCI device

[Jan Glauber]

Cavium SOCs contain a memory controller that is presented as a
PCI device. This PCI device will be used by an EDAC driver and
by a PMU driver.

To allow both subsystems to access the device a small wrapper is
introduced that multi-plexes PCI probe and removal calls of the
device to the EDAC driver.

The same mechanism will be used later to call the PMU driver.

The ThunderX EDAC driver is limited to only build as module
with this patch. The reason is that with multiple users of the
multi-plexer all users must be either builtin or modules.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/edac/Kconfig            |  2 ++
 drivers/edac/thunderx_edac.c    | 31 ++++++--------------------
 drivers/soc/Kconfig             |  1 +
 drivers/soc/Makefile            |  1 +
 drivers/soc/cavium/Kconfig      |  6 +++++
 drivers/soc/cavium/Makefile     |  1 +
 drivers/soc/cavium/cavium_lmc.c | 49 +++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/cavium/lmc.h  |  9 ++++++++
 8 files changed, 76 insertions(+), 24 deletions(-)
 create mode 100644 drivers/soc/cavium/Kconfig
 create mode 100644 drivers/soc/cavium/Makefile
 create mode 100644 drivers/soc/cavium/cavium_lmc.c
 create mode 100644 include/linux/soc/cavium/lmc.h

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 96afb2a..7330447 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -371,6 +371,8 @@ config EDAC_THUNDERX
 	tristate "Cavium ThunderX EDAC"
 	depends on ARM64
 	depends on PCI
+	depends on m
+	select CAVIUM_LMC
 	help
 	  Support for error detection and correction on the
 	  Cavium ThunderX memory controllers (LMC), Cache
diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index d02bf3b..16f3d62 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/edac.h>
+#include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/string.h>
 #include <linux/stop_machine.h>
@@ -20,6 +21,7 @@
 #include <linux/atomic.h>
 #include <linux/bitfield.h>
 #include <linux/circ_buf.h>
+#include <linux/soc/cavium/lmc.h>
 
 #include <asm/page.h>
 
@@ -654,8 +656,7 @@ static inline int pci_dev_to_mc_idx(struct pci_dev *pdev)
 	return ret;
 }
 
-static int thunderx_lmc_probe(struct pci_dev *pdev,
-				const struct pci_device_id *id)
+int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct thunderx_lmc *lmc;
 	struct edac_mc_layer layer;
@@ -797,8 +798,9 @@ static int thunderx_lmc_probe(struct pci_dev *pdev,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(thunderx_edac_lmc_probe);
 
-static void thunderx_lmc_remove(struct pci_dev *pdev)
+void thunderx_edac_lmc_remove(struct pci_dev *pdev)
 {
 	struct mem_ctl_info *mci = pci_get_drvdata(pdev);
 	struct thunderx_lmc *lmc = mci->pvt_info;
@@ -808,19 +810,7 @@ static void thunderx_lmc_remove(struct pci_dev *pdev)
 	edac_mc_del_mc(&pdev->dev);
 	edac_mc_free(mci);
 }
-
-MODULE_DEVICE_TABLE(pci, thunderx_lmc_pci_tbl);
-
-static struct pci_driver thunderx_lmc_driver = {
-	.name     = "thunderx_lmc_edac",
-	.probe    = thunderx_lmc_probe,
-	.remove   = thunderx_lmc_remove,
-#ifdef CONFIG_PM
-	.suspend  = thunderx_lmc_suspend,
-	.resume   = thunderx_lmc_resume,
-#endif
-	.id_table = thunderx_lmc_pci_tbl,
-};
+EXPORT_SYMBOL_GPL(thunderx_edac_lmc_remove);
 
 /*---------------------- OCX driver ---------------------------------*/
 
@@ -2116,13 +2106,9 @@ static int __init thunderx_edac_init(void)
 {
 	int rc = 0;
 
-	rc = pci_register_driver(&thunderx_lmc_driver);
-	if (rc)
-		return rc;
-
 	rc = pci_register_driver(&thunderx_ocx_driver);
 	if (rc)
-		goto err_lmc;
+		return rc;
 
 	rc = pci_register_driver(&thunderx_l2c_driver);
 	if (rc)
@@ -2131,8 +2117,6 @@ static int __init thunderx_edac_init(void)
 	return rc;
 err_ocx:
 	pci_unregister_driver(&thunderx_ocx_driver);
-err_lmc:
-	pci_unregister_driver(&thunderx_lmc_driver);
 
 	return rc;
 }
@@ -2141,7 +2125,6 @@ static void __exit thunderx_edac_exit(void)
 {
 	pci_unregister_driver(&thunderx_l2c_driver);
 	pci_unregister_driver(&thunderx_ocx_driver);
-	pci_unregister_driver(&thunderx_lmc_driver);
 
 }
 
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 07fc0ac..6e31936 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -3,6 +3,7 @@ menu "SOC (System On Chip) specific Drivers"
 source "drivers/soc/actions/Kconfig"
 source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
+source "drivers/soc/cavium/Kconfig"
 source "drivers/soc/fsl/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 9241125..96eff43 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
 obj-$(CONFIG_ARCH_AT91)		+= atmel/
 obj-y				+= bcm/
+obj-$(CONFIG_ARCH_THUNDER)	+= cavium/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
diff --git a/drivers/soc/cavium/Kconfig b/drivers/soc/cavium/Kconfig
new file mode 100644
index 0000000..46ded89
--- /dev/null
+++ b/drivers/soc/cavium/Kconfig
@@ -0,0 +1,6 @@
+#
+# Cavium ThunderX Soc drivers
+#
+config CAVIUM_LMC
+        depends on ARCH_THUNDER
+	def_tristate m
diff --git a/drivers/soc/cavium/Makefile b/drivers/soc/cavium/Makefile
new file mode 100644
index 0000000..4ad0c7f
--- /dev/null
+++ b/drivers/soc/cavium/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_CAVIUM_LMC) += cavium_lmc.o
diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
new file mode 100644
index 0000000..87248e8
--- /dev/null
+++ b/drivers/soc/cavium/cavium_lmc.c
@@ -0,0 +1,49 @@
+/*
+ * These PCI devices contain RAS functionality and PMU counters. To allow
+ * independent RAS and PMU drivers this driver registers for the PCI devices
+ * and multi-plexes probe and removal.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright: Cavium, Inc. (C) 2017
+ *
+ */
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/soc/cavium/lmc.h>
+
+static int cvm_lmc_probe(struct pci_dev *pdev,
+			 const struct pci_device_id *ent)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_lmc_probe(pdev, ent);
+	return 0;
+}
+
+static void cvm_lmc_remove(struct pci_dev *pdev)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_lmc_remove(pdev);
+}
+
+static const struct pci_device_id cvm_lmc_pci_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa022) },
+	{ 0, },
+};
+
+MODULE_DEVICE_TABLE(pci, cvm_lmc_pci_table);
+
+static struct pci_driver cvm_lmc_pci_driver = {
+	.name     = "Cavium SOC memory controller",
+	.id_table = cvm_lmc_pci_table,
+	.probe    = cvm_lmc_probe,
+	.remove   = cvm_lmc_remove,
+};
+
+module_pci_driver(cvm_lmc_pci_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Cavium, Inc.");
+MODULE_DESCRIPTION("PCI driver for Cavium SOC memory controller");
diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
new file mode 100644
index 0000000..336f467
--- /dev/null
+++ b/include/linux/soc/cavium/lmc.h
@@ -0,0 +1,9 @@
+#ifndef _LMC_H
+#define _LMC_H
+
+#include <linux/pci.h>
+
+int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void thunderx_edac_lmc_remove(struct pci_dev *pdev);
+
+#endif
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 2/7] edac, soc: thunderx: Add wrapper for EDAC LMC PCI device

[Jan Glauber]

Cavium SOCs contain a memory controller that is presented as a
PCI device. This PCI device will be used by an EDAC driver and
by a PMU driver.

To allow both subsystems to access the device a small wrapper is
introduced that multi-plexes PCI probe and removal calls of the
device to the EDAC driver.

The same mechanism will be used later to call the PMU driver.

The ThunderX EDAC driver is limited to only build as module
with this patch. The reason is that with multiple users of the
multi-plexer all users must be either builtin or modules.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/edac/Kconfig            |  2 ++
 drivers/edac/thunderx_edac.c    | 31 ++++++--------------------
 drivers/soc/Kconfig             |  1 +
 drivers/soc/Makefile            |  1 +
 drivers/soc/cavium/Kconfig      |  6 +++++
 drivers/soc/cavium/Makefile     |  1 +
 drivers/soc/cavium/cavium_lmc.c | 49 +++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/cavium/lmc.h  |  9 ++++++++
 8 files changed, 76 insertions(+), 24 deletions(-)
 create mode 100644 drivers/soc/cavium/Kconfig
 create mode 100644 drivers/soc/cavium/Makefile
 create mode 100644 drivers/soc/cavium/cavium_lmc.c
 create mode 100644 include/linux/soc/cavium/lmc.h

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 96afb2a..7330447 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -371,6 +371,8 @@ config EDAC_THUNDERX
 	tristate "Cavium ThunderX EDAC"
 	depends on ARM64
 	depends on PCI
+	depends on m
+	select CAVIUM_LMC
 	help
 	  Support for error detection and correction on the
 	  Cavium ThunderX memory controllers (LMC), Cache
diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index d02bf3b..16f3d62 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/edac.h>
+#include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/string.h>
 #include <linux/stop_machine.h>
@@ -20,6 +21,7 @@
 #include <linux/atomic.h>
 #include <linux/bitfield.h>
 #include <linux/circ_buf.h>
+#include <linux/soc/cavium/lmc.h>
 
 #include <asm/page.h>
 
@@ -654,8 +656,7 @@ static inline int pci_dev_to_mc_idx(struct pci_dev *pdev)
 	return ret;
 }
 
-static int thunderx_lmc_probe(struct pci_dev *pdev,
-				const struct pci_device_id *id)
+int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct thunderx_lmc *lmc;
 	struct edac_mc_layer layer;
@@ -797,8 +798,9 @@ static int thunderx_lmc_probe(struct pci_dev *pdev,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(thunderx_edac_lmc_probe);
 
-static void thunderx_lmc_remove(struct pci_dev *pdev)
+void thunderx_edac_lmc_remove(struct pci_dev *pdev)
 {
 	struct mem_ctl_info *mci = pci_get_drvdata(pdev);
 	struct thunderx_lmc *lmc = mci->pvt_info;
@@ -808,19 +810,7 @@ static void thunderx_lmc_remove(struct pci_dev *pdev)
 	edac_mc_del_mc(&pdev->dev);
 	edac_mc_free(mci);
 }
-
-MODULE_DEVICE_TABLE(pci, thunderx_lmc_pci_tbl);
-
-static struct pci_driver thunderx_lmc_driver = {
-	.name     = "thunderx_lmc_edac",
-	.probe    = thunderx_lmc_probe,
-	.remove   = thunderx_lmc_remove,
-#ifdef CONFIG_PM
-	.suspend  = thunderx_lmc_suspend,
-	.resume   = thunderx_lmc_resume,
-#endif
-	.id_table = thunderx_lmc_pci_tbl,
-};
+EXPORT_SYMBOL_GPL(thunderx_edac_lmc_remove);
 
 /*---------------------- OCX driver ---------------------------------*/
 
@@ -2116,13 +2106,9 @@ static int __init thunderx_edac_init(void)
 {
 	int rc = 0;
 
-	rc = pci_register_driver(&thunderx_lmc_driver);
-	if (rc)
-		return rc;
-
 	rc = pci_register_driver(&thunderx_ocx_driver);
 	if (rc)
-		goto err_lmc;
+		return rc;
 
 	rc = pci_register_driver(&thunderx_l2c_driver);
 	if (rc)
@@ -2131,8 +2117,6 @@ static int __init thunderx_edac_init(void)
 	return rc;
 err_ocx:
 	pci_unregister_driver(&thunderx_ocx_driver);
-err_lmc:
-	pci_unregister_driver(&thunderx_lmc_driver);
 
 	return rc;
 }
@@ -2141,7 +2125,6 @@ static void __exit thunderx_edac_exit(void)
 {
 	pci_unregister_driver(&thunderx_l2c_driver);
 	pci_unregister_driver(&thunderx_ocx_driver);
-	pci_unregister_driver(&thunderx_lmc_driver);
 
 }
 
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 07fc0ac..6e31936 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -3,6 +3,7 @@ menu "SOC (System On Chip) specific Drivers"
 source "drivers/soc/actions/Kconfig"
 source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
+source "drivers/soc/cavium/Kconfig"
 source "drivers/soc/fsl/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 9241125..96eff43 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
 obj-$(CONFIG_ARCH_AT91)		+= atmel/
 obj-y				+= bcm/
+obj-$(CONFIG_ARCH_THUNDER)	+= cavium/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
diff --git a/drivers/soc/cavium/Kconfig b/drivers/soc/cavium/Kconfig
new file mode 100644
index 0000000..46ded89
--- /dev/null
+++ b/drivers/soc/cavium/Kconfig
@@ -0,0 +1,6 @@
+#
+# Cavium ThunderX Soc drivers
+#
+config CAVIUM_LMC
+        depends on ARCH_THUNDER
+	def_tristate m
diff --git a/drivers/soc/cavium/Makefile b/drivers/soc/cavium/Makefile
new file mode 100644
index 0000000..4ad0c7f
--- /dev/null
+++ b/drivers/soc/cavium/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_CAVIUM_LMC) += cavium_lmc.o
diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
new file mode 100644
index 0000000..87248e8
--- /dev/null
+++ b/drivers/soc/cavium/cavium_lmc.c
@@ -0,0 +1,49 @@
+/*
+ * These PCI devices contain RAS functionality and PMU counters. To allow
+ * independent RAS and PMU drivers this driver registers for the PCI devices
+ * and multi-plexes probe and removal.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright: Cavium, Inc. (C) 2017
+ *
+ */
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/soc/cavium/lmc.h>
+
+static int cvm_lmc_probe(struct pci_dev *pdev,
+			 const struct pci_device_id *ent)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_lmc_probe(pdev, ent);
+	return 0;
+}
+
+static void cvm_lmc_remove(struct pci_dev *pdev)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_lmc_remove(pdev);
+}
+
+static const struct pci_device_id cvm_lmc_pci_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa022) },
+	{ 0, },
+};
+
+MODULE_DEVICE_TABLE(pci, cvm_lmc_pci_table);
+
+static struct pci_driver cvm_lmc_pci_driver = {
+	.name     = "Cavium SOC memory controller",
+	.id_table = cvm_lmc_pci_table,
+	.probe    = cvm_lmc_probe,
+	.remove   = cvm_lmc_remove,
+};
+
+module_pci_driver(cvm_lmc_pci_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Cavium, Inc.");
+MODULE_DESCRIPTION("PCI driver for Cavium SOC memory controller");
diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
new file mode 100644
index 0000000..336f467
--- /dev/null
+++ b/include/linux/soc/cavium/lmc.h
@@ -0,0 +1,9 @@
+#ifndef _LMC_H
+#define _LMC_H
+
+#include <linux/pci.h>
+
+int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void thunderx_edac_lmc_remove(struct pci_dev *pdev);
+
+#endif
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 3/7] edac,soc: thunderx: Add wrapper for EDAC OCX PCI device

[Jan Glauber]

Cavium SOCs contain an processor interconnect that is presented as a
PCI device. This PCI device will be used by an EDAC driver and
by a PMU driver.

To allow both subsystems to access the device a small wrapper is
introduced that multi-plexes PCI probe and removal calls of the
device to the EDAC driver.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/edac/Kconfig            |  1 +
 drivers/edac/thunderx_edac.c    | 42 +++++++-------------------------------
 drivers/soc/cavium/Kconfig      |  4 ++++
 drivers/soc/cavium/Makefile     |  1 +
 drivers/soc/cavium/cavium_ocx.c | 45 +++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/cavium/ocx.h  |  9 +++++++++
 6 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 drivers/soc/cavium/cavium_ocx.c
 create mode 100644 include/linux/soc/cavium/ocx.h

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 7330447..8304212 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -373,6 +373,7 @@ config EDAC_THUNDERX
 	depends on PCI
 	depends on m
 	select CAVIUM_LMC
+	select CAVIUM_OCX
 	help
 	  Support for error detection and correction on the
 	  Cavium ThunderX memory controllers (LMC), Cache
diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index 16f3d62..ddbda71 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -22,6 +22,7 @@
 #include <linux/bitfield.h>
 #include <linux/circ_buf.h>
 #include <linux/soc/cavium/lmc.h>
+#include <linux/soc/cavium/ocx.h>
 
 #include <asm/page.h>
 
@@ -814,8 +815,6 @@ EXPORT_SYMBOL_GPL(thunderx_edac_lmc_remove);
 
 /*---------------------- OCX driver ---------------------------------*/
 
-#define PCI_DEVICE_ID_THUNDER_OCX 0xa013
-
 #define OCX_LINK_INTS		3
 #define OCX_INTS		(OCX_LINK_INTS + 1)
 #define OCX_RX_LANES		24
@@ -1311,11 +1310,6 @@ struct debugfs_entry *ocx_dfs_ents[] = {
 	&debugfs_com_int,
 };
 
-static const struct pci_device_id thunderx_ocx_pci_tbl[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_OCX) },
-	{ 0, },
-};
-
 static void thunderx_ocx_clearstats(struct thunderx_ocx *ocx)
 {
 	int lane, stat, cfg;
@@ -1331,8 +1325,8 @@ static void thunderx_ocx_clearstats(struct thunderx_ocx *ocx)
 	}
 }
 
-static int thunderx_ocx_probe(struct pci_dev *pdev,
-			      const struct pci_device_id *id)
+int thunderx_edac_ocx_probe(struct pci_dev *pdev,
+			    const struct pci_device_id *id)
 {
 	struct thunderx_ocx *ocx;
 	struct edac_device_ctl_info *edac_dev;
@@ -1461,8 +1455,9 @@ static int thunderx_ocx_probe(struct pci_dev *pdev,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(thunderx_edac_ocx_probe);
 
-static void thunderx_ocx_remove(struct pci_dev *pdev)
+void thunderx_edac_ocx_remove(struct pci_dev *pdev)
 {
 	struct edac_device_ctl_info *edac_dev = pci_get_drvdata(pdev);
 	struct thunderx_ocx *ocx = edac_dev->pvt_info;
@@ -1480,15 +1475,7 @@ static void thunderx_ocx_remove(struct pci_dev *pdev)
 	edac_device_del_device(&pdev->dev);
 	edac_device_free_ctl_info(edac_dev);
 }
-
-MODULE_DEVICE_TABLE(pci, thunderx_ocx_pci_tbl);
-
-static struct pci_driver thunderx_ocx_driver = {
-	.name     = "thunderx_ocx_edac",
-	.probe    = thunderx_ocx_probe,
-	.remove   = thunderx_ocx_remove,
-	.id_table = thunderx_ocx_pci_tbl,
-};
+EXPORT_SYMBOL_GPL(thunderx_edac_ocx_remove);
 
 /*---------------------- L2C driver ---------------------------------*/
 
@@ -2104,27 +2091,12 @@ static struct pci_driver thunderx_l2c_driver = {
 
 static int __init thunderx_edac_init(void)
 {
-	int rc = 0;
-
-	rc = pci_register_driver(&thunderx_ocx_driver);
-	if (rc)
-		return rc;
-
-	rc = pci_register_driver(&thunderx_l2c_driver);
-	if (rc)
-		goto err_ocx;
-
-	return rc;
-err_ocx:
-	pci_unregister_driver(&thunderx_ocx_driver);
-
-	return rc;
+	return pci_register_driver(&thunderx_l2c_driver);
 }
 
 static void __exit thunderx_edac_exit(void)
 {
 	pci_unregister_driver(&thunderx_l2c_driver);
-	pci_unregister_driver(&thunderx_ocx_driver);
 
 }
 
diff --git a/drivers/soc/cavium/Kconfig b/drivers/soc/cavium/Kconfig
index 46ded89..fe56503 100644
--- a/drivers/soc/cavium/Kconfig
+++ b/drivers/soc/cavium/Kconfig
@@ -4,3 +4,7 @@
 config CAVIUM_LMC
         depends on ARCH_THUNDER
 	def_tristate m
+
+config CAVIUM_OCX
+	depends on ARCH_THUNDER
+	def_tristate m
diff --git a/drivers/soc/cavium/Makefile b/drivers/soc/cavium/Makefile
index 4ad0c7f..bf7ba25 100644
--- a/drivers/soc/cavium/Makefile
+++ b/drivers/soc/cavium/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_CAVIUM_LMC) += cavium_lmc.o
+obj-$(CONFIG_CAVIUM_OCX) += cavium_ocx.o
diff --git a/drivers/soc/cavium/cavium_ocx.c b/drivers/soc/cavium/cavium_ocx.c
new file mode 100644
index 0000000..fa3341b
--- /dev/null
+++ b/drivers/soc/cavium/cavium_ocx.c
@@ -0,0 +1,45 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright: Cavium, Inc. (C) 2017
+ *
+ */
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/soc/cavium/ocx.h>
+
+static int cvm_ocx_probe(struct pci_dev *pdev,
+			 const struct pci_device_id *ent)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_ocx_probe(pdev, ent);
+	return 0;
+}
+
+static void cvm_ocx_remove(struct pci_dev *pdev)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_ocx_remove(pdev);
+}
+
+static const struct pci_device_id cvm_ocx_pci_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa013) },
+	{ 0, },
+};
+
+MODULE_DEVICE_TABLE(pci, cvm_ocx_pci_table);
+
+static struct pci_driver cvm_ocx_pci_driver = {
+	.name     = "Cavium ThunderX interconnect",
+	.id_table = cvm_ocx_pci_table,
+	.probe    = cvm_ocx_probe,
+	.remove   = cvm_ocx_remove,
+};
+
+module_pci_driver(cvm_ocx_pci_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Cavium, Inc.");
+MODULE_DESCRIPTION("PCI driver for Cavium ThunderX interconnect");
diff --git a/include/linux/soc/cavium/ocx.h b/include/linux/soc/cavium/ocx.h
new file mode 100644
index 0000000..29f55b3
--- /dev/null
+++ b/include/linux/soc/cavium/ocx.h
@@ -0,0 +1,9 @@
+#ifndef _OCX_H
+#define _OCX_H
+
+#include <linux/pci.h>
+
+int thunderx_edac_ocx_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void thunderx_edac_ocx_remove(struct pci_dev *pdev);
+
+#endif
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 3/7] edac, soc: thunderx: Add wrapper for EDAC OCX PCI device

[Jan Glauber]

Cavium SOCs contain an processor interconnect that is presented as a
PCI device. This PCI device will be used by an EDAC driver and
by a PMU driver.

To allow both subsystems to access the device a small wrapper is
introduced that multi-plexes PCI probe and removal calls of the
device to the EDAC driver.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/edac/Kconfig            |  1 +
 drivers/edac/thunderx_edac.c    | 42 +++++++-------------------------------
 drivers/soc/cavium/Kconfig      |  4 ++++
 drivers/soc/cavium/Makefile     |  1 +
 drivers/soc/cavium/cavium_ocx.c | 45 +++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/cavium/ocx.h  |  9 +++++++++
 6 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 drivers/soc/cavium/cavium_ocx.c
 create mode 100644 include/linux/soc/cavium/ocx.h

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 7330447..8304212 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -373,6 +373,7 @@ config EDAC_THUNDERX
 	depends on PCI
 	depends on m
 	select CAVIUM_LMC
+	select CAVIUM_OCX
 	help
 	  Support for error detection and correction on the
 	  Cavium ThunderX memory controllers (LMC), Cache
diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index 16f3d62..ddbda71 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -22,6 +22,7 @@
 #include <linux/bitfield.h>
 #include <linux/circ_buf.h>
 #include <linux/soc/cavium/lmc.h>
+#include <linux/soc/cavium/ocx.h>
 
 #include <asm/page.h>
 
@@ -814,8 +815,6 @@ EXPORT_SYMBOL_GPL(thunderx_edac_lmc_remove);
 
 /*---------------------- OCX driver ---------------------------------*/
 
-#define PCI_DEVICE_ID_THUNDER_OCX 0xa013
-
 #define OCX_LINK_INTS		3
 #define OCX_INTS		(OCX_LINK_INTS + 1)
 #define OCX_RX_LANES		24
@@ -1311,11 +1310,6 @@ struct debugfs_entry *ocx_dfs_ents[] = {
 	&debugfs_com_int,
 };
 
-static const struct pci_device_id thunderx_ocx_pci_tbl[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_OCX) },
-	{ 0, },
-};
-
 static void thunderx_ocx_clearstats(struct thunderx_ocx *ocx)
 {
 	int lane, stat, cfg;
@@ -1331,8 +1325,8 @@ static void thunderx_ocx_clearstats(struct thunderx_ocx *ocx)
 	}
 }
 
-static int thunderx_ocx_probe(struct pci_dev *pdev,
-			      const struct pci_device_id *id)
+int thunderx_edac_ocx_probe(struct pci_dev *pdev,
+			    const struct pci_device_id *id)
 {
 	struct thunderx_ocx *ocx;
 	struct edac_device_ctl_info *edac_dev;
@@ -1461,8 +1455,9 @@ static int thunderx_ocx_probe(struct pci_dev *pdev,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(thunderx_edac_ocx_probe);
 
-static void thunderx_ocx_remove(struct pci_dev *pdev)
+void thunderx_edac_ocx_remove(struct pci_dev *pdev)
 {
 	struct edac_device_ctl_info *edac_dev = pci_get_drvdata(pdev);
 	struct thunderx_ocx *ocx = edac_dev->pvt_info;
@@ -1480,15 +1475,7 @@ static void thunderx_ocx_remove(struct pci_dev *pdev)
 	edac_device_del_device(&pdev->dev);
 	edac_device_free_ctl_info(edac_dev);
 }
-
-MODULE_DEVICE_TABLE(pci, thunderx_ocx_pci_tbl);
-
-static struct pci_driver thunderx_ocx_driver = {
-	.name     = "thunderx_ocx_edac",
-	.probe    = thunderx_ocx_probe,
-	.remove   = thunderx_ocx_remove,
-	.id_table = thunderx_ocx_pci_tbl,
-};
+EXPORT_SYMBOL_GPL(thunderx_edac_ocx_remove);
 
 /*---------------------- L2C driver ---------------------------------*/
 
@@ -2104,27 +2091,12 @@ static struct pci_driver thunderx_l2c_driver = {
 
 static int __init thunderx_edac_init(void)
 {
-	int rc = 0;
-
-	rc = pci_register_driver(&thunderx_ocx_driver);
-	if (rc)
-		return rc;
-
-	rc = pci_register_driver(&thunderx_l2c_driver);
-	if (rc)
-		goto err_ocx;
-
-	return rc;
-err_ocx:
-	pci_unregister_driver(&thunderx_ocx_driver);
-
-	return rc;
+	return pci_register_driver(&thunderx_l2c_driver);
 }
 
 static void __exit thunderx_edac_exit(void)
 {
 	pci_unregister_driver(&thunderx_l2c_driver);
-	pci_unregister_driver(&thunderx_ocx_driver);
 
 }
 
diff --git a/drivers/soc/cavium/Kconfig b/drivers/soc/cavium/Kconfig
index 46ded89..fe56503 100644
--- a/drivers/soc/cavium/Kconfig
+++ b/drivers/soc/cavium/Kconfig
@@ -4,3 +4,7 @@
 config CAVIUM_LMC
         depends on ARCH_THUNDER
 	def_tristate m
+
+config CAVIUM_OCX
+	depends on ARCH_THUNDER
+	def_tristate m
diff --git a/drivers/soc/cavium/Makefile b/drivers/soc/cavium/Makefile
index 4ad0c7f..bf7ba25 100644
--- a/drivers/soc/cavium/Makefile
+++ b/drivers/soc/cavium/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_CAVIUM_LMC) += cavium_lmc.o
+obj-$(CONFIG_CAVIUM_OCX) += cavium_ocx.o
diff --git a/drivers/soc/cavium/cavium_ocx.c b/drivers/soc/cavium/cavium_ocx.c
new file mode 100644
index 0000000..fa3341b
--- /dev/null
+++ b/drivers/soc/cavium/cavium_ocx.c
@@ -0,0 +1,45 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright: Cavium, Inc. (C) 2017
+ *
+ */
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/soc/cavium/ocx.h>
+
+static int cvm_ocx_probe(struct pci_dev *pdev,
+			 const struct pci_device_id *ent)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_ocx_probe(pdev, ent);
+	return 0;
+}
+
+static void cvm_ocx_remove(struct pci_dev *pdev)
+{
+	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
+		thunderx_edac_ocx_remove(pdev);
+}
+
+static const struct pci_device_id cvm_ocx_pci_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa013) },
+	{ 0, },
+};
+
+MODULE_DEVICE_TABLE(pci, cvm_ocx_pci_table);
+
+static struct pci_driver cvm_ocx_pci_driver = {
+	.name     = "Cavium ThunderX interconnect",
+	.id_table = cvm_ocx_pci_table,
+	.probe    = cvm_ocx_probe,
+	.remove   = cvm_ocx_remove,
+};
+
+module_pci_driver(cvm_ocx_pci_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Cavium, Inc.");
+MODULE_DESCRIPTION("PCI driver for Cavium ThunderX interconnect");
diff --git a/include/linux/soc/cavium/ocx.h b/include/linux/soc/cavium/ocx.h
new file mode 100644
index 0000000..29f55b3
--- /dev/null
+++ b/include/linux/soc/cavium/ocx.h
@@ -0,0 +1,9 @@
+#ifndef _OCX_H
+#define _OCX_H
+
+#include <linux/pci.h>
+
+int thunderx_edac_ocx_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void thunderx_edac_ocx_remove(struct pci_dev *pdev);
+
+#endif
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 4/7] perf: export perf_event_update_userpage()

[Jan Glauber]

Export perf_event_update_userpage(). This change is needed to allow
building a PMU driver as a kernel module.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 kernel/events/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3504125..639bbf5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4977,6 +4977,7 @@ void perf_event_update_userpage(struct perf_event *event)
 unlock:
 	rcu_read_unlock();
 }
+EXPORT_SYMBOL_GPL(perf_event_update_userpage);
 
 static int perf_mmap_fault(struct vm_fault *vmf)
 {
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 4/7] perf: export perf_event_update_userpage()

[Jan Glauber]

Export perf_event_update_userpage(). This change is needed to allow
building a PMU driver as a kernel module.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 kernel/events/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3504125..639bbf5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4977,6 +4977,7 @@ void perf_event_update_userpage(struct perf_event *event)
 unlock:
 	rcu_read_unlock();
 }
+EXPORT_SYMBOL_GPL(perf_event_update_userpage);
 
 static int perf_mmap_fault(struct vm_fault *vmf)
 {
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

Add support for the PMU counters on Cavium SOC memory controllers.

This patch also adds generic functions to allow supporting more
devices with PMU counters.

Properties of the LMC PMU counters:
- not stoppable
- fixed purpose
- read-only
- one PCI device per memory controller

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/perf/Kconfig            |   8 +
 drivers/perf/Makefile           |   1 +
 drivers/perf/cavium_pmu.c       | 445 ++++++++++++++++++++++++++++++++++++++++
 drivers/soc/cavium/cavium_lmc.c |   4 +
 include/linux/cpuhotplug.h      |   1 +
 include/linux/soc/cavium/lmc.h  |   3 +
 6 files changed, 462 insertions(+)
 create mode 100644 drivers/perf/cavium_pmu.c

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index e5197ff..a787562 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -43,4 +43,12 @@ config XGENE_PMU
         help
           Say y if you want to use APM X-Gene SoC performance monitors.
 
+config CAVIUM_PMU_LMC
+	tristate "Cavium SOC memory controller PMU"
+	depends on ARCH_THUNDER && m
+	select CAVIUM_LMC
+	help
+	  Provides PMU counters for the memory controller on
+	  Cavium ThunderX or OcteonTX SOCs.
+
 endmenu
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index 6420bd4..077a15d 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
 obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
 obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
+obj-$(CONFIG_CAVIUM_PMU_LMC) += cavium_pmu.o
diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
new file mode 100644
index 0000000..bcdedaa
--- /dev/null
+++ b/drivers/perf/cavium_pmu.c
@@ -0,0 +1,445 @@
+/*
+ * Cavium ARM SOC "uncore" PMU counters
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright Cavium, Inc. 2017
+ * Author(s): Jan Glauber <jan.glauber@cavium.com>
+ *
+ */
+#include <linux/cpumask.h>
+#include <linux/cpuhotplug.h>
+#include <linux/io.h>
+#include <linux/export.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/perf_event.h>
+#include <linux/slab.h>
+#include <linux/soc/cavium/lmc.h>
+
+enum cvm_pmu_type {
+	CVM_PMU_LMC,
+};
+
+/* maximum number of parallel hardware counters for all pmu types */
+#define CVM_PMU_MAX_COUNTERS 64
+
+/* generic struct to cover the different pmu types */
+struct cvm_pmu_dev {
+	struct pmu pmu;
+	const char *pmu_name;
+	bool (*event_valid)(u64);
+	void __iomem *map;
+	struct pci_dev *pdev;
+	int num_counters;
+	struct perf_event *events[CVM_PMU_MAX_COUNTERS];
+	struct list_head entry;
+	struct hlist_node cpuhp_node;
+	cpumask_t active_mask;
+};
+
+static struct list_head cvm_pmu_lmcs;
+static struct list_head cvm_pmu_tlks;
+
+/*
+ * Common Cavium PMU stuff
+ *
+ * Shared properties of the different PMU types:
+ * - all counters are 64 bit long
+ * - there are no overflow interrupts
+ * - all devices with PMU counters appear as PCI devices
+ *
+ * Counter control, access and device association depends on the
+ * PMU type.
+ */
+
+#define to_pmu_dev(x) container_of((x), struct cvm_pmu_dev, pmu)
+
+static int cvm_pmu_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct cvm_pmu_dev *pmu_dev;
+	struct perf_event *sibling;
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* we do not support sampling */
+	if (is_sampling_event(event))
+		return -EINVAL;
+
+	/* PMU counters do not support any these bits */
+	if (event->attr.exclude_user	||
+	    event->attr.exclude_kernel	||
+	    event->attr.exclude_host	||
+	    event->attr.exclude_guest	||
+	    event->attr.exclude_hv	||
+	    event->attr.exclude_idle)
+		return -EINVAL;
+
+	pmu_dev = to_pmu_dev(event->pmu);
+	if (!pmu_dev->event_valid(event->attr.config))
+		return -EINVAL;
+
+	/*
+	 * Forbid groups containing mixed PMUs, software events are acceptable.
+	 */
+	if (event->group_leader->pmu != event->pmu &&
+	    !is_software_event(event->group_leader))
+		return -EINVAL;
+
+	list_for_each_entry(sibling, &event->group_leader->sibling_list,
+			    group_entry)
+		if (sibling->pmu != event->pmu &&
+		    !is_software_event(sibling))
+			return -EINVAL;
+
+	hwc->config = event->attr.config;
+	hwc->idx = -1;
+	return 0;
+}
+
+static void cvm_pmu_read(struct perf_event *event)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 prev, delta, new;
+
+again:
+	prev = local64_read(&hwc->prev_count);
+	new = readq(hwc->event_base + pmu_dev->map);
+
+	if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev)
+		goto again;
+
+	delta = new - prev;
+	local64_add(delta, &event->count);
+}
+
+static void cvm_pmu_start(struct perf_event *event, int flags)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 new;
+
+	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
+		return;
+
+	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
+	hwc->state = 0;
+
+	/* update prev_count always in order support unstoppable counters */
+	new = readq(hwc->event_base + pmu_dev->map);
+	local64_set(&hwc->prev_count, new);
+
+	perf_event_update_userpage(event);
+}
+
+static void cvm_pmu_stop(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+	hwc->state |= PERF_HES_STOPPED;
+
+	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
+		cvm_pmu_read(event);
+		hwc->state |= PERF_HES_UPTODATE;
+	}
+}
+
+static int cvm_pmu_add(struct perf_event *event, int flags, u64 config_base,
+		       u64 event_base)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (!cmpxchg(&pmu_dev->events[hwc->config], NULL, event))
+		hwc->idx = hwc->config;
+
+	if (hwc->idx == -1)
+		return -EBUSY;
+
+	hwc->config_base = config_base;
+	hwc->event_base = event_base;
+	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
+
+	if (flags & PERF_EF_START)
+		pmu_dev->pmu.start(event, PERF_EF_RELOAD);
+
+	return 0;
+}
+
+static void cvm_pmu_del(struct perf_event *event, int flags)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	int i;
+
+	event->pmu->stop(event, PERF_EF_UPDATE);
+
+	/*
+	 * For programmable counters we need to check where we installed it.
+	 * To keep this function generic always test the more complicated
+	 * case (free running counters won't need the loop).
+	 */
+	for (i = 0; i < pmu_dev->num_counters; i++)
+		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
+			break;
+
+	perf_event_update_userpage(event);
+	hwc->idx = -1;
+}
+
+static ssize_t cvm_pmu_event_sysfs_show(struct device *dev,
+					struct device_attribute *attr,
+					char *page)
+{
+	struct perf_pmu_events_attr *pmu_attr =
+		container_of(attr, struct perf_pmu_events_attr, attr);
+
+	if (pmu_attr->event_str)
+		return sprintf(page, "%s", pmu_attr->event_str);
+
+	return 0;
+}
+
+/*
+ * The pmu events are independent from CPUs. Provide a cpumask
+ * nevertheless to prevent perf from adding the event per-cpu and just
+ * set the mask to one online CPU. Use the same cpumask for all "uncore"
+ * devices.
+ *
+ * There is a performance penalty for accessing a device from a CPU on
+ * another socket, but we do not care.
+ */
+static int cvm_pmu_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
+{
+	struct cvm_pmu_dev *pmu_dev;
+	int new_cpu;
+
+	pmu_dev = hlist_entry_safe(node, struct cvm_pmu_dev, cpuhp_node);
+	if (!cpumask_test_and_clear_cpu(old_cpu, &pmu_dev->active_mask))
+		return 0;
+
+	new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
+	if (new_cpu >= nr_cpu_ids)
+		return 0;
+
+	perf_pmu_migrate_context(&pmu_dev->pmu, old_cpu, new_cpu);
+	cpumask_set_cpu(new_cpu, &pmu_dev->active_mask);
+
+	return 0;
+}
+
+static ssize_t cvm_pmu_attr_show_cpumask(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct pmu *pmu = dev_get_drvdata(dev);
+	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
+
+	return cpumap_print_to_pagebuf(true, buf, &pmu_dev->active_mask);
+}
+
+static DEVICE_ATTR(cpumask, S_IRUGO, cvm_pmu_attr_show_cpumask, NULL);
+
+static struct attribute *cvm_pmu_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_attr_group = {
+	.attrs = cvm_pmu_attrs,
+};
+
+/*
+ * LMC (memory controller) counters:
+ * - not stoppable, always on, read-only
+ * - one PCI device per memory controller
+ */
+#define LMC_CONFIG_OFFSET		0x188
+#define LMC_CONFIG_RESET_BIT		BIT(17)
+
+/* LMC events */
+#define LMC_EVENT_IFB_CNT		0x1d0
+#define LMC_EVENT_OPS_CNT		0x1d8
+#define LMC_EVENT_DCLK_CNT		0x1e0
+#define LMC_EVENT_BANK_CONFLICT1	0x360
+#define LMC_EVENT_BANK_CONFLICT2	0x368
+
+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
+	&((struct perf_pmu_events_attr[]) {						\
+		{									\
+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
+			_id,								\
+			"lmc_event=" __stringify(_id),					\
+		}									\
+	})[0].attr.attr
+
+/* map counter numbers to register offsets */
+static int lmc_events[] = {
+	LMC_EVENT_IFB_CNT,
+	LMC_EVENT_OPS_CNT,
+	LMC_EVENT_DCLK_CNT,
+	LMC_EVENT_BANK_CONFLICT1,
+	LMC_EVENT_BANK_CONFLICT2,
+};
+
+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
+			   lmc_events[hwc->config]);
+}
+
+PMU_FORMAT_ATTR(lmc_event, "config:0-2");
+
+static struct attribute *cvm_pmu_lmc_format_attr[] = {
+	&format_attr_lmc_event.attr,
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_lmc_format_group = {
+	.name = "format",
+	.attrs = cvm_pmu_lmc_format_attr,
+};
+
+static struct attribute *cvm_pmu_lmc_events_attr[] = {
+	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
+	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
+	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_lmc_events_group = {
+	.name = "events",
+	.attrs = cvm_pmu_lmc_events_attr,
+};
+
+static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
+	&cvm_pmu_attr_group,
+	&cvm_pmu_lmc_format_group,
+	&cvm_pmu_lmc_events_group,
+	NULL,
+};
+
+static bool cvm_pmu_lmc_event_valid(u64 config)
+{
+	return (config < ARRAY_SIZE(lmc_events));
+}
+
+int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	struct cvm_pmu_dev *next, *lmc;
+	int nr = 0, ret = -ENOMEM;
+
+	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);
+	if (!lmc)
+		return -ENOMEM;
+
+	lmc->map = ioremap(pci_resource_start(pdev, 0),
+			   pci_resource_len(pdev, 0));
+	if (!lmc->map)
+		goto fail_ioremap;
+
+	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
+		nr++;
+	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);
+	if (!lmc->pmu_name)
+		goto fail_kasprintf;
+
+	lmc->pdev = pdev;
+	lmc->num_counters = ARRAY_SIZE(lmc_events);
+	lmc->pmu = (struct pmu) {
+		.task_ctx_nr    = perf_invalid_context,
+		.event_init	= cvm_pmu_event_init,
+		.add		= cvm_pmu_lmc_add,
+		.del		= cvm_pmu_del,
+		.start		= cvm_pmu_start,
+		.stop		= cvm_pmu_stop,
+		.read		= cvm_pmu_read,
+		.attr_groups	= cvm_pmu_lmc_attr_groups,
+	};
+
+	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					 &lmc->cpuhp_node);
+
+	/*
+	 * perf PMU is CPU dependent so pick a random CPU and migrate away
+	 * if it goes offline.
+	 */
+	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
+
+	list_add(&lmc->entry, &cvm_pmu_lmcs);
+	lmc->event_valid = cvm_pmu_lmc_event_valid;
+
+	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
+	if (ret)
+		goto fail_pmu;
+
+	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
+		 lmc->pmu_name, lmc->num_counters);
+	return 0;
+
+fail_pmu:
+	kfree(lmc->pmu_name);
+	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+				    &lmc->cpuhp_node);
+fail_kasprintf:
+	iounmap(lmc->map);
+fail_ioremap:
+	kfree(lmc);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cvm_lmc_pmu_probe);
+
+void cvm_lmc_pmu_remove(struct pci_dev *pdev)
+{
+	struct list_head *l, *tmp;
+	struct cvm_pmu_dev *lmc;
+
+	list_for_each_safe(l, tmp, &cvm_pmu_lmcs) {
+		lmc = list_entry(l, struct cvm_pmu_dev, entry);
+		if (pdev != lmc->pdev)
+			continue;
+
+		perf_pmu_unregister(&lmc->pmu);
+		iounmap(lmc->map);
+		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					    &lmc->cpuhp_node);
+		list_del(&lmc->entry);
+		kfree(lmc->pmu_name);
+		kfree(lmc);
+	}
+}
+EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
+
+static int __init cvm_pmu_init(void)
+{
+	INIT_LIST_HEAD(&cvm_pmu_lmcs);
+	INIT_LIST_HEAD(&cvm_pmu_tlks);
+
+	return cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+				       "perf/arm/cvm:online", NULL,
+				       cvm_pmu_offline_cpu);
+}
+
+static void __exit cvm_pmu_exit(void)
+{
+	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_CVM_ONLINE);
+}
+
+module_init(cvm_pmu_init);
+module_exit(cvm_pmu_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Cavium, Inc.");
+MODULE_DESCRIPTION("PMU Driver for Cavium ThunderX SOC");
diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
index 87248e8..d21d59c 100644
--- a/drivers/soc/cavium/cavium_lmc.c
+++ b/drivers/soc/cavium/cavium_lmc.c
@@ -17,6 +17,8 @@
 static int cvm_lmc_probe(struct pci_dev *pdev,
 			 const struct pci_device_id *ent)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
+		cvm_lmc_pmu_probe(pdev, ent);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_lmc_probe(pdev, ent);
 	return 0;
@@ -24,6 +26,8 @@ static int cvm_lmc_probe(struct pci_dev *pdev,
 
 static void cvm_lmc_remove(struct pci_dev *pdev)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
+		cvm_lmc_pmu_remove(pdev);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_lmc_remove(pdev);
 }
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 82b30e6..ca84ac8 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -139,6 +139,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
 	CPUHP_AP_WORKQUEUE_ONLINE,
 	CPUHP_AP_RCUTREE_ONLINE,
+	CPUHP_AP_PERF_ARM_CVM_ONLINE,
 	CPUHP_AP_ONLINE_DYN,
 	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
 	CPUHP_AP_X86_HPET_ONLINE,
diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
index 336f467..e5ad650 100644
--- a/include/linux/soc/cavium/lmc.h
+++ b/include/linux/soc/cavium/lmc.h
@@ -3,6 +3,9 @@
 
 #include <linux/pci.h>
 
+int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void cvm_lmc_pmu_remove(struct pci_dev *pdev);
+
 int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
 void thunderx_edac_lmc_remove(struct pci_dev *pdev);
 
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

Add support for the PMU counters on Cavium SOC memory controllers.

This patch also adds generic functions to allow supporting more
devices with PMU counters.

Properties of the LMC PMU counters:
- not stoppable
- fixed purpose
- read-only
- one PCI device per memory controller

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/perf/Kconfig            |   8 +
 drivers/perf/Makefile           |   1 +
 drivers/perf/cavium_pmu.c       | 445 ++++++++++++++++++++++++++++++++++++++++
 drivers/soc/cavium/cavium_lmc.c |   4 +
 include/linux/cpuhotplug.h      |   1 +
 include/linux/soc/cavium/lmc.h  |   3 +
 6 files changed, 462 insertions(+)
 create mode 100644 drivers/perf/cavium_pmu.c

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index e5197ff..a787562 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -43,4 +43,12 @@ config XGENE_PMU
         help
           Say y if you want to use APM X-Gene SoC performance monitors.
 
+config CAVIUM_PMU_LMC
+	tristate "Cavium SOC memory controller PMU"
+	depends on ARCH_THUNDER && m
+	select CAVIUM_LMC
+	help
+	  Provides PMU counters for the memory controller on
+	  Cavium ThunderX or OcteonTX SOCs.
+
 endmenu
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index 6420bd4..077a15d 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
 obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
 obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
+obj-$(CONFIG_CAVIUM_PMU_LMC) += cavium_pmu.o
diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
new file mode 100644
index 0000000..bcdedaa
--- /dev/null
+++ b/drivers/perf/cavium_pmu.c
@@ -0,0 +1,445 @@
+/*
+ * Cavium ARM SOC "uncore" PMU counters
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright Cavium, Inc. 2017
+ * Author(s): Jan Glauber <jan.glauber@cavium.com>
+ *
+ */
+#include <linux/cpumask.h>
+#include <linux/cpuhotplug.h>
+#include <linux/io.h>
+#include <linux/export.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+#include <linux/perf_event.h>
+#include <linux/slab.h>
+#include <linux/soc/cavium/lmc.h>
+
+enum cvm_pmu_type {
+	CVM_PMU_LMC,
+};
+
+/* maximum number of parallel hardware counters for all pmu types */
+#define CVM_PMU_MAX_COUNTERS 64
+
+/* generic struct to cover the different pmu types */
+struct cvm_pmu_dev {
+	struct pmu pmu;
+	const char *pmu_name;
+	bool (*event_valid)(u64);
+	void __iomem *map;
+	struct pci_dev *pdev;
+	int num_counters;
+	struct perf_event *events[CVM_PMU_MAX_COUNTERS];
+	struct list_head entry;
+	struct hlist_node cpuhp_node;
+	cpumask_t active_mask;
+};
+
+static struct list_head cvm_pmu_lmcs;
+static struct list_head cvm_pmu_tlks;
+
+/*
+ * Common Cavium PMU stuff
+ *
+ * Shared properties of the different PMU types:
+ * - all counters are 64 bit long
+ * - there are no overflow interrupts
+ * - all devices with PMU counters appear as PCI devices
+ *
+ * Counter control, access and device association depends on the
+ * PMU type.
+ */
+
+#define to_pmu_dev(x) container_of((x), struct cvm_pmu_dev, pmu)
+
+static int cvm_pmu_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct cvm_pmu_dev *pmu_dev;
+	struct perf_event *sibling;
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/* we do not support sampling */
+	if (is_sampling_event(event))
+		return -EINVAL;
+
+	/* PMU counters do not support any these bits */
+	if (event->attr.exclude_user	||
+	    event->attr.exclude_kernel	||
+	    event->attr.exclude_host	||
+	    event->attr.exclude_guest	||
+	    event->attr.exclude_hv	||
+	    event->attr.exclude_idle)
+		return -EINVAL;
+
+	pmu_dev = to_pmu_dev(event->pmu);
+	if (!pmu_dev->event_valid(event->attr.config))
+		return -EINVAL;
+
+	/*
+	 * Forbid groups containing mixed PMUs, software events are acceptable.
+	 */
+	if (event->group_leader->pmu != event->pmu &&
+	    !is_software_event(event->group_leader))
+		return -EINVAL;
+
+	list_for_each_entry(sibling, &event->group_leader->sibling_list,
+			    group_entry)
+		if (sibling->pmu != event->pmu &&
+		    !is_software_event(sibling))
+			return -EINVAL;
+
+	hwc->config = event->attr.config;
+	hwc->idx = -1;
+	return 0;
+}
+
+static void cvm_pmu_read(struct perf_event *event)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 prev, delta, new;
+
+again:
+	prev = local64_read(&hwc->prev_count);
+	new = readq(hwc->event_base + pmu_dev->map);
+
+	if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev)
+		goto again;
+
+	delta = new - prev;
+	local64_add(delta, &event->count);
+}
+
+static void cvm_pmu_start(struct perf_event *event, int flags)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 new;
+
+	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
+		return;
+
+	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
+	hwc->state = 0;
+
+	/* update prev_count always in order support unstoppable counters */
+	new = readq(hwc->event_base + pmu_dev->map);
+	local64_set(&hwc->prev_count, new);
+
+	perf_event_update_userpage(event);
+}
+
+static void cvm_pmu_stop(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+	hwc->state |= PERF_HES_STOPPED;
+
+	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
+		cvm_pmu_read(event);
+		hwc->state |= PERF_HES_UPTODATE;
+	}
+}
+
+static int cvm_pmu_add(struct perf_event *event, int flags, u64 config_base,
+		       u64 event_base)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (!cmpxchg(&pmu_dev->events[hwc->config], NULL, event))
+		hwc->idx = hwc->config;
+
+	if (hwc->idx == -1)
+		return -EBUSY;
+
+	hwc->config_base = config_base;
+	hwc->event_base = event_base;
+	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
+
+	if (flags & PERF_EF_START)
+		pmu_dev->pmu.start(event, PERF_EF_RELOAD);
+
+	return 0;
+}
+
+static void cvm_pmu_del(struct perf_event *event, int flags)
+{
+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+	int i;
+
+	event->pmu->stop(event, PERF_EF_UPDATE);
+
+	/*
+	 * For programmable counters we need to check where we installed it.
+	 * To keep this function generic always test the more complicated
+	 * case (free running counters won't need the loop).
+	 */
+	for (i = 0; i < pmu_dev->num_counters; i++)
+		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
+			break;
+
+	perf_event_update_userpage(event);
+	hwc->idx = -1;
+}
+
+static ssize_t cvm_pmu_event_sysfs_show(struct device *dev,
+					struct device_attribute *attr,
+					char *page)
+{
+	struct perf_pmu_events_attr *pmu_attr =
+		container_of(attr, struct perf_pmu_events_attr, attr);
+
+	if (pmu_attr->event_str)
+		return sprintf(page, "%s", pmu_attr->event_str);
+
+	return 0;
+}
+
+/*
+ * The pmu events are independent from CPUs. Provide a cpumask
+ * nevertheless to prevent perf from adding the event per-cpu and just
+ * set the mask to one online CPU. Use the same cpumask for all "uncore"
+ * devices.
+ *
+ * There is a performance penalty for accessing a device from a CPU on
+ * another socket, but we do not care.
+ */
+static int cvm_pmu_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
+{
+	struct cvm_pmu_dev *pmu_dev;
+	int new_cpu;
+
+	pmu_dev = hlist_entry_safe(node, struct cvm_pmu_dev, cpuhp_node);
+	if (!cpumask_test_and_clear_cpu(old_cpu, &pmu_dev->active_mask))
+		return 0;
+
+	new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
+	if (new_cpu >= nr_cpu_ids)
+		return 0;
+
+	perf_pmu_migrate_context(&pmu_dev->pmu, old_cpu, new_cpu);
+	cpumask_set_cpu(new_cpu, &pmu_dev->active_mask);
+
+	return 0;
+}
+
+static ssize_t cvm_pmu_attr_show_cpumask(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct pmu *pmu = dev_get_drvdata(dev);
+	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
+
+	return cpumap_print_to_pagebuf(true, buf, &pmu_dev->active_mask);
+}
+
+static DEVICE_ATTR(cpumask, S_IRUGO, cvm_pmu_attr_show_cpumask, NULL);
+
+static struct attribute *cvm_pmu_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_attr_group = {
+	.attrs = cvm_pmu_attrs,
+};
+
+/*
+ * LMC (memory controller) counters:
+ * - not stoppable, always on, read-only
+ * - one PCI device per memory controller
+ */
+#define LMC_CONFIG_OFFSET		0x188
+#define LMC_CONFIG_RESET_BIT		BIT(17)
+
+/* LMC events */
+#define LMC_EVENT_IFB_CNT		0x1d0
+#define LMC_EVENT_OPS_CNT		0x1d8
+#define LMC_EVENT_DCLK_CNT		0x1e0
+#define LMC_EVENT_BANK_CONFLICT1	0x360
+#define LMC_EVENT_BANK_CONFLICT2	0x368
+
+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
+	&((struct perf_pmu_events_attr[]) {						\
+		{									\
+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
+			_id,								\
+			"lmc_event=" __stringify(_id),					\
+		}									\
+	})[0].attr.attr
+
+/* map counter numbers to register offsets */
+static int lmc_events[] = {
+	LMC_EVENT_IFB_CNT,
+	LMC_EVENT_OPS_CNT,
+	LMC_EVENT_DCLK_CNT,
+	LMC_EVENT_BANK_CONFLICT1,
+	LMC_EVENT_BANK_CONFLICT2,
+};
+
+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
+			   lmc_events[hwc->config]);
+}
+
+PMU_FORMAT_ATTR(lmc_event, "config:0-2");
+
+static struct attribute *cvm_pmu_lmc_format_attr[] = {
+	&format_attr_lmc_event.attr,
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_lmc_format_group = {
+	.name = "format",
+	.attrs = cvm_pmu_lmc_format_attr,
+};
+
+static struct attribute *cvm_pmu_lmc_events_attr[] = {
+	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
+	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
+	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_lmc_events_group = {
+	.name = "events",
+	.attrs = cvm_pmu_lmc_events_attr,
+};
+
+static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
+	&cvm_pmu_attr_group,
+	&cvm_pmu_lmc_format_group,
+	&cvm_pmu_lmc_events_group,
+	NULL,
+};
+
+static bool cvm_pmu_lmc_event_valid(u64 config)
+{
+	return (config < ARRAY_SIZE(lmc_events));
+}
+
+int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	struct cvm_pmu_dev *next, *lmc;
+	int nr = 0, ret = -ENOMEM;
+
+	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);
+	if (!lmc)
+		return -ENOMEM;
+
+	lmc->map = ioremap(pci_resource_start(pdev, 0),
+			   pci_resource_len(pdev, 0));
+	if (!lmc->map)
+		goto fail_ioremap;
+
+	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
+		nr++;
+	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);
+	if (!lmc->pmu_name)
+		goto fail_kasprintf;
+
+	lmc->pdev = pdev;
+	lmc->num_counters = ARRAY_SIZE(lmc_events);
+	lmc->pmu = (struct pmu) {
+		.task_ctx_nr    = perf_invalid_context,
+		.event_init	= cvm_pmu_event_init,
+		.add		= cvm_pmu_lmc_add,
+		.del		= cvm_pmu_del,
+		.start		= cvm_pmu_start,
+		.stop		= cvm_pmu_stop,
+		.read		= cvm_pmu_read,
+		.attr_groups	= cvm_pmu_lmc_attr_groups,
+	};
+
+	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					 &lmc->cpuhp_node);
+
+	/*
+	 * perf PMU is CPU dependent so pick a random CPU and migrate away
+	 * if it goes offline.
+	 */
+	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
+
+	list_add(&lmc->entry, &cvm_pmu_lmcs);
+	lmc->event_valid = cvm_pmu_lmc_event_valid;
+
+	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
+	if (ret)
+		goto fail_pmu;
+
+	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
+		 lmc->pmu_name, lmc->num_counters);
+	return 0;
+
+fail_pmu:
+	kfree(lmc->pmu_name);
+	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+				    &lmc->cpuhp_node);
+fail_kasprintf:
+	iounmap(lmc->map);
+fail_ioremap:
+	kfree(lmc);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cvm_lmc_pmu_probe);
+
+void cvm_lmc_pmu_remove(struct pci_dev *pdev)
+{
+	struct list_head *l, *tmp;
+	struct cvm_pmu_dev *lmc;
+
+	list_for_each_safe(l, tmp, &cvm_pmu_lmcs) {
+		lmc = list_entry(l, struct cvm_pmu_dev, entry);
+		if (pdev != lmc->pdev)
+			continue;
+
+		perf_pmu_unregister(&lmc->pmu);
+		iounmap(lmc->map);
+		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					    &lmc->cpuhp_node);
+		list_del(&lmc->entry);
+		kfree(lmc->pmu_name);
+		kfree(lmc);
+	}
+}
+EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
+
+static int __init cvm_pmu_init(void)
+{
+	INIT_LIST_HEAD(&cvm_pmu_lmcs);
+	INIT_LIST_HEAD(&cvm_pmu_tlks);
+
+	return cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+				       "perf/arm/cvm:online", NULL,
+				       cvm_pmu_offline_cpu);
+}
+
+static void __exit cvm_pmu_exit(void)
+{
+	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_CVM_ONLINE);
+}
+
+module_init(cvm_pmu_init);
+module_exit(cvm_pmu_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Cavium, Inc.");
+MODULE_DESCRIPTION("PMU Driver for Cavium ThunderX SOC");
diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
index 87248e8..d21d59c 100644
--- a/drivers/soc/cavium/cavium_lmc.c
+++ b/drivers/soc/cavium/cavium_lmc.c
@@ -17,6 +17,8 @@
 static int cvm_lmc_probe(struct pci_dev *pdev,
 			 const struct pci_device_id *ent)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
+		cvm_lmc_pmu_probe(pdev, ent);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_lmc_probe(pdev, ent);
 	return 0;
@@ -24,6 +26,8 @@ static int cvm_lmc_probe(struct pci_dev *pdev,
 
 static void cvm_lmc_remove(struct pci_dev *pdev)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
+		cvm_lmc_pmu_remove(pdev);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_lmc_remove(pdev);
 }
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 82b30e6..ca84ac8 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -139,6 +139,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
 	CPUHP_AP_WORKQUEUE_ONLINE,
 	CPUHP_AP_RCUTREE_ONLINE,
+	CPUHP_AP_PERF_ARM_CVM_ONLINE,
 	CPUHP_AP_ONLINE_DYN,
 	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
 	CPUHP_AP_X86_HPET_ONLINE,
diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
index 336f467..e5ad650 100644
--- a/include/linux/soc/cavium/lmc.h
+++ b/include/linux/soc/cavium/lmc.h
@@ -3,6 +3,9 @@
 
 #include <linux/pci.h>
 
+int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void cvm_lmc_pmu_remove(struct pci_dev *pdev);
+
 int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
 void thunderx_edac_lmc_remove(struct pci_dev *pdev);
 
-- 
2.9.0.rc0.21.g7777322

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Zhangshaokun]

Hi Jan,

Some trivial things i noticed, please consider if you are glad.

Thanks,
Shaokun

On 2017/8/29 21:12, Jan Glauber wrote:
> Add support for the PMU counters on Cavium SOC memory controllers.
> 
> This patch also adds generic functions to allow supporting more
> devices with PMU counters.
> 
> Properties of the LMC PMU counters:
> - not stoppable
> - fixed purpose
> - read-only
> - one PCI device per memory controller
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  drivers/perf/Kconfig            |   8 +
>  drivers/perf/Makefile           |   1 +
>  drivers/perf/cavium_pmu.c       | 445 ++++++++++++++++++++++++++++++++++++++++
>  drivers/soc/cavium/cavium_lmc.c |   4 +
>  include/linux/cpuhotplug.h      |   1 +
>  include/linux/soc/cavium/lmc.h  |   3 +
>  6 files changed, 462 insertions(+)
>  create mode 100644 drivers/perf/cavium_pmu.c
> 
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index e5197ff..a787562 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -43,4 +43,12 @@ config XGENE_PMU
>          help
>            Say y if you want to use APM X-Gene SoC performance monitors.
>  
> +config CAVIUM_PMU_LMC
> +	tristate "Cavium SOC memory controller PMU"
> +	depends on ARCH_THUNDER && m
> +	select CAVIUM_LMC
> +	help
> +	  Provides PMU counters for the memory controller on
> +	  Cavium ThunderX or OcteonTX SOCs.
> +
>  endmenu
> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> index 6420bd4..077a15d 100644
> --- a/drivers/perf/Makefile
> +++ b/drivers/perf/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
>  obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
>  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
>  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> +obj-$(CONFIG_CAVIUM_PMU_LMC) += cavium_pmu.o

Keep in alphabetic order?

> diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
> new file mode 100644
> index 0000000..bcdedaa
> --- /dev/null
> +++ b/drivers/perf/cavium_pmu.c
> @@ -0,0 +1,445 @@
> +/*
> + * Cavium ARM SOC "uncore" PMU counters
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright Cavium, Inc. 2017
> + * Author(s): Jan Glauber <jan.glauber@cavium.com>
> + *
> + */
> +#include <linux/cpumask.h>
> +#include <linux/cpuhotplug.h>
> +#include <linux/io.h>
> +#include <linux/export.h>

Keep the include header files in alphabetic order too?

> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/perf_event.h>
> +#include <linux/slab.h>
> +#include <linux/soc/cavium/lmc.h>
> +
> +enum cvm_pmu_type {
> +	CVM_PMU_LMC,
> +};
> +
> +/* maximum number of parallel hardware counters for all pmu types */
> +#define CVM_PMU_MAX_COUNTERS 64
> +
> +/* generic struct to cover the different pmu types */
> +struct cvm_pmu_dev {
> +	struct pmu pmu;
> +	const char *pmu_name;

It seems that pmu_name is redundant since struct pmu has a name field,
Mark has mentioned it in HiSilicon uncore PMU driver, Link:
https://patchwork.kernel.org/patch/9861821/

> +	bool (*event_valid)(u64);
> +	void __iomem *map;
> +	struct pci_dev *pdev;
> +	int num_counters;
> +	struct perf_event *events[CVM_PMU_MAX_COUNTERS];
> +	struct list_head entry;
> +	struct hlist_node cpuhp_node;
> +	cpumask_t active_mask;
> +};
> +
> +static struct list_head cvm_pmu_lmcs;
> +static struct list_head cvm_pmu_tlks;
> +
> +/*
> + * Common Cavium PMU stuff
> + *
> + * Shared properties of the different PMU types:
> + * - all counters are 64 bit long
> + * - there are no overflow interrupts
> + * - all devices with PMU counters appear as PCI devices
> + *
> + * Counter control, access and device association depends on the
> + * PMU type.
> + */
> +
> +#define to_pmu_dev(x) container_of((x), struct cvm_pmu_dev, pmu)
> +
> +static int cvm_pmu_event_init(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct cvm_pmu_dev *pmu_dev;
> +	struct perf_event *sibling;
> +
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	/* we do not support sampling */
> +	if (is_sampling_event(event))
> +		return -EINVAL;
> +
> +	/* PMU counters do not support any these bits */
> +	if (event->attr.exclude_user	||
> +	    event->attr.exclude_kernel	||
> +	    event->attr.exclude_host	||
> +	    event->attr.exclude_guest	||
> +	    event->attr.exclude_hv	||
> +	    event->attr.exclude_idle)
> +		return -EINVAL;
> +
> +	pmu_dev = to_pmu_dev(event->pmu);
> +	if (!pmu_dev->event_valid(event->attr.config))
> +		return -EINVAL;
> +
> +	/*
> +	 * Forbid groups containing mixed PMUs, software events are acceptable.
> +	 */
> +	if (event->group_leader->pmu != event->pmu &&
> +	    !is_software_event(event->group_leader))
> +		return -EINVAL;
> +
> +	list_for_each_entry(sibling, &event->group_leader->sibling_list,
> +			    group_entry)
> +		if (sibling->pmu != event->pmu &&
> +		    !is_software_event(sibling))
> +			return -EINVAL;
> +
> +	hwc->config = event->attr.config;
> +	hwc->idx = -1;

Blank line?

> +	return 0;
> +}
> +
> +static void cvm_pmu_read(struct perf_event *event)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	u64 prev, delta, new;
> +
> +again:
> +	prev = local64_read(&hwc->prev_count);
> +	new = readq(hwc->event_base + pmu_dev->map);
> +
> +	if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev)
> +		goto again;
> +
> +	delta = new - prev;
> +	local64_add(delta, &event->count);
> +}
> +
> +static void cvm_pmu_start(struct perf_event *event, int flags)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	u64 new;
> +
> +	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> +		return;
> +
> +	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> +	hwc->state = 0;
> +
> +	/* update prev_count always in order support unstoppable counters */
> +	new = readq(hwc->event_base + pmu_dev->map);
> +	local64_set(&hwc->prev_count, new);
> +
> +	perf_event_update_userpage(event);
> +}
> +
> +static void cvm_pmu_stop(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> +	hwc->state |= PERF_HES_STOPPED;
> +
> +	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
> +		cvm_pmu_read(event);
> +		hwc->state |= PERF_HES_UPTODATE;
> +	}
> +}
> +
> +static int cvm_pmu_add(struct perf_event *event, int flags, u64 config_base,
> +		       u64 event_base)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (!cmpxchg(&pmu_dev->events[hwc->config], NULL, event))
> +		hwc->idx = hwc->config;
> +
> +	if (hwc->idx == -1)
> +		return -EBUSY;
> +
> +	hwc->config_base = config_base;
> +	hwc->event_base = event_base;
> +	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> +
> +	if (flags & PERF_EF_START)
> +		pmu_dev->pmu.start(event, PERF_EF_RELOAD);
> +
> +	return 0;
> +}
> +
> +static void cvm_pmu_del(struct perf_event *event, int flags)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	int i;
> +
> +	event->pmu->stop(event, PERF_EF_UPDATE);
> +
> +	/*
> +	 * For programmable counters we need to check where we installed it.
> +	 * To keep this function generic always test the more complicated
> +	 * case (free running counters won't need the loop).
> +	 */
> +	for (i = 0; i < pmu_dev->num_counters; i++)
> +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> +			break;
> +
> +	perf_event_update_userpage(event);
> +	hwc->idx = -1;
> +}
> +
> +static ssize_t cvm_pmu_event_sysfs_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *page)
> +{
> +	struct perf_pmu_events_attr *pmu_attr =
> +		container_of(attr, struct perf_pmu_events_attr, attr);
> +
> +	if (pmu_attr->event_str)
> +		return sprintf(page, "%s", pmu_attr->event_str);
> +
> +	return 0;
> +}
> +
> +/*
> + * The pmu events are independent from CPUs. Provide a cpumask
> + * nevertheless to prevent perf from adding the event per-cpu and just
> + * set the mask to one online CPU. Use the same cpumask for all "uncore"
> + * devices.
> + *
> + * There is a performance penalty for accessing a device from a CPU on
> + * another socket, but we do not care.
> + */
> +static int cvm_pmu_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
> +{
> +	struct cvm_pmu_dev *pmu_dev;
> +	int new_cpu;

unsigned int?

> +
> +	pmu_dev = hlist_entry_safe(node, struct cvm_pmu_dev, cpuhp_node);
> +	if (!cpumask_test_and_clear_cpu(old_cpu, &pmu_dev->active_mask))
> +		return 0;
> +
> +	new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
> +	if (new_cpu >= nr_cpu_ids)
> +		return 0;
> +
> +	perf_pmu_migrate_context(&pmu_dev->pmu, old_cpu, new_cpu);
> +	cpumask_set_cpu(new_cpu, &pmu_dev->active_mask);
> +
> +	return 0;
> +}
> +
> +static ssize_t cvm_pmu_attr_show_cpumask(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct pmu *pmu = dev_get_drvdata(dev);
> +	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
> +
> +	return cpumap_print_to_pagebuf(true, buf, &pmu_dev->active_mask);
> +}
> +
> +static DEVICE_ATTR(cpumask, S_IRUGO, cvm_pmu_attr_show_cpumask, NULL);
> +
> +static struct attribute *cvm_pmu_attrs[] = {
> +	&dev_attr_cpumask.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_attr_group = {
> +	.attrs = cvm_pmu_attrs,
> +};
> +
> +/*
> + * LMC (memory controller) counters:
> + * - not stoppable, always on, read-only
> + * - one PCI device per memory controller
> + */
> +#define LMC_CONFIG_OFFSET		0x188
> +#define LMC_CONFIG_RESET_BIT		BIT(17)
> +
> +/* LMC events */
> +#define LMC_EVENT_IFB_CNT		0x1d0
> +#define LMC_EVENT_OPS_CNT		0x1d8
> +#define LMC_EVENT_DCLK_CNT		0x1e0
> +#define LMC_EVENT_BANK_CONFLICT1	0x360
> +#define LMC_EVENT_BANK_CONFLICT2	0x368
> +
> +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> +	&((struct perf_pmu_events_attr[]) {						\
> +		{									\
> +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> +			_id,								\
> +			"lmc_event=" __stringify(_id),					\
> +		}									\
> +	})[0].attr.attr
> +
> +/* map counter numbers to register offsets */
> +static int lmc_events[] = {

Add const?

> +	LMC_EVENT_IFB_CNT,
> +	LMC_EVENT_OPS_CNT,
> +	LMC_EVENT_DCLK_CNT,
> +	LMC_EVENT_BANK_CONFLICT1,
> +	LMC_EVENT_BANK_CONFLICT2,
> +};
> +
> +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> +			   lmc_events[hwc->config]);
> +}
> +
> +PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> +
> +static struct attribute *cvm_pmu_lmc_format_attr[] = {
> +	&format_attr_lmc_event.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_format_group = {
> +	.name = "format",
> +	.attrs = cvm_pmu_lmc_format_attr,
> +};
> +
> +static struct attribute *cvm_pmu_lmc_events_attr[] = {
> +	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> +	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> +	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_events_group = {
> +	.name = "events",
> +	.attrs = cvm_pmu_lmc_events_attr,
> +};
> +
> +static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
> +	&cvm_pmu_attr_group,
> +	&cvm_pmu_lmc_format_group,
> +	&cvm_pmu_lmc_events_group,
> +	NULL,
> +};
> +
> +static bool cvm_pmu_lmc_event_valid(u64 config)
> +{
> +	return (config < ARRAY_SIZE(lmc_events));
> +}
> +
> +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct cvm_pmu_dev *next, *lmc;
> +	int nr = 0, ret = -ENOMEM;
> +
> +	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);

How about use devm_kzalloc?

> +	if (!lmc)
> +		return -ENOMEM;
> +
> +	lmc->map = ioremap(pci_resource_start(pdev, 0),
> +			   pci_resource_len(pdev, 0));
> +	if (!lmc->map)
> +		goto fail_ioremap;
> +
> +	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
> +		nr++;
> +	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);

Use devm_kasprintf, simplify fail handle and memory free?

> +	if (!lmc->pmu_name)
> +		goto fail_kasprintf;
> +
> +	lmc->pdev = pdev;
> +	lmc->num_counters = ARRAY_SIZE(lmc_events);
> +	lmc->pmu = (struct pmu) {
> +		.task_ctx_nr    = perf_invalid_context,
> +		.event_init	= cvm_pmu_event_init,
> +		.add		= cvm_pmu_lmc_add,
> +		.del		= cvm_pmu_del,
> +		.start		= cvm_pmu_start,
> +		.stop		= cvm_pmu_stop,
> +		.read		= cvm_pmu_read,
> +		.attr_groups	= cvm_pmu_lmc_attr_groups,
> +	};
> +
> +	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +					 &lmc->cpuhp_node);
> +
> +	/*
> +	 * perf PMU is CPU dependent so pick a random CPU and migrate away
> +	 * if it goes offline.
> +	 */
> +	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
> +
> +	list_add(&lmc->entry, &cvm_pmu_lmcs);
> +	lmc->event_valid = cvm_pmu_lmc_event_valid;
> +
> +	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
> +	if (ret)
> +		goto fail_pmu;
> +
> +	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
> +		 lmc->pmu_name, lmc->num_counters);

Blank line?

> +	return 0;
> +
> +fail_pmu:
> +	kfree(lmc->pmu_name);
> +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +				    &lmc->cpuhp_node);
> +fail_kasprintf:
> +	iounmap(lmc->map);
> +fail_ioremap:
> +	kfree(lmc);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_probe);
> +
> +void cvm_lmc_pmu_remove(struct pci_dev *pdev)
> +{
> +	struct list_head *l, *tmp;
> +	struct cvm_pmu_dev *lmc;
> +
> +	list_for_each_safe(l, tmp, &cvm_pmu_lmcs) {
> +		lmc = list_entry(l, struct cvm_pmu_dev, entry);
> +		if (pdev != lmc->pdev)
> +			continue;
> +
> +		perf_pmu_unregister(&lmc->pmu);
> +		iounmap(lmc->map);
> +		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +					    &lmc->cpuhp_node);
> +		list_del(&lmc->entry);
> +		kfree(lmc->pmu_name);
> +		kfree(lmc);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
> +
> +static int __init cvm_pmu_init(void)
> +{
> +	INIT_LIST_HEAD(&cvm_pmu_lmcs);
> +	INIT_LIST_HEAD(&cvm_pmu_tlks);
> +
> +	return cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +				       "perf/arm/cvm:online", NULL,
> +				       cvm_pmu_offline_cpu);
> +}
> +
> +static void __exit cvm_pmu_exit(void)
> +{
> +	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_CVM_ONLINE);
> +}
> +
> +module_init(cvm_pmu_init);
> +module_exit(cvm_pmu_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Cavium, Inc.");
> +MODULE_DESCRIPTION("PMU Driver for Cavium ThunderX SOC");
> diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
> index 87248e8..d21d59c 100644
> --- a/drivers/soc/cavium/cavium_lmc.c
> +++ b/drivers/soc/cavium/cavium_lmc.c
> @@ -17,6 +17,8 @@
>  static int cvm_lmc_probe(struct pci_dev *pdev,
>  			 const struct pci_device_id *ent)
>  {
> +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> +		cvm_lmc_pmu_probe(pdev, ent);
>  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
>  		thunderx_edac_lmc_probe(pdev, ent);
>  	return 0;
> @@ -24,6 +26,8 @@ static int cvm_lmc_probe(struct pci_dev *pdev,
>  
>  static void cvm_lmc_remove(struct pci_dev *pdev)
>  {
> +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> +		cvm_lmc_pmu_remove(pdev);
>  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
>  		thunderx_edac_lmc_remove(pdev);
>  }
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 82b30e6..ca84ac8 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -139,6 +139,7 @@ enum cpuhp_state {
>  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
>  	CPUHP_AP_WORKQUEUE_ONLINE,
>  	CPUHP_AP_RCUTREE_ONLINE,
> +	CPUHP_AP_PERF_ARM_CVM_ONLINE,

Alphabetic order?

>  	CPUHP_AP_ONLINE_DYN,
>  	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
>  	CPUHP_AP_X86_HPET_ONLINE,
> diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
> index 336f467..e5ad650 100644
> --- a/include/linux/soc/cavium/lmc.h
> +++ b/include/linux/soc/cavium/lmc.h
> @@ -3,6 +3,9 @@
>  
>  #include <linux/pci.h>
>  
> +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> +void cvm_lmc_pmu_remove(struct pci_dev *pdev);
> +
>  int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
>  void thunderx_edac_lmc_remove(struct pci_dev *pdev);
>  
> 

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Zhangshaokun]

Hi Jan,

Some trivial things i noticed, please consider if you are glad.

Thanks,
Shaokun

On 2017/8/29 21:12, Jan Glauber wrote:
> Add support for the PMU counters on Cavium SOC memory controllers.
> 
> This patch also adds generic functions to allow supporting more
> devices with PMU counters.
> 
> Properties of the LMC PMU counters:
> - not stoppable
> - fixed purpose
> - read-only
> - one PCI device per memory controller
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  drivers/perf/Kconfig            |   8 +
>  drivers/perf/Makefile           |   1 +
>  drivers/perf/cavium_pmu.c       | 445 ++++++++++++++++++++++++++++++++++++++++
>  drivers/soc/cavium/cavium_lmc.c |   4 +
>  include/linux/cpuhotplug.h      |   1 +
>  include/linux/soc/cavium/lmc.h  |   3 +
>  6 files changed, 462 insertions(+)
>  create mode 100644 drivers/perf/cavium_pmu.c
> 
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index e5197ff..a787562 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -43,4 +43,12 @@ config XGENE_PMU
>          help
>            Say y if you want to use APM X-Gene SoC performance monitors.
>  
> +config CAVIUM_PMU_LMC
> +	tristate "Cavium SOC memory controller PMU"
> +	depends on ARCH_THUNDER && m
> +	select CAVIUM_LMC
> +	help
> +	  Provides PMU counters for the memory controller on
> +	  Cavium ThunderX or OcteonTX SOCs.
> +
>  endmenu
> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> index 6420bd4..077a15d 100644
> --- a/drivers/perf/Makefile
> +++ b/drivers/perf/Makefile
> @@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
>  obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
>  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
>  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> +obj-$(CONFIG_CAVIUM_PMU_LMC) += cavium_pmu.o

Keep in alphabetic order?

> diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
> new file mode 100644
> index 0000000..bcdedaa
> --- /dev/null
> +++ b/drivers/perf/cavium_pmu.c
> @@ -0,0 +1,445 @@
> +/*
> + * Cavium ARM SOC "uncore" PMU counters
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright Cavium, Inc. 2017
> + * Author(s): Jan Glauber <jan.glauber@cavium.com>
> + *
> + */
> +#include <linux/cpumask.h>
> +#include <linux/cpuhotplug.h>
> +#include <linux/io.h>
> +#include <linux/export.h>

Keep the include header files in alphabetic order too?

> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/perf_event.h>
> +#include <linux/slab.h>
> +#include <linux/soc/cavium/lmc.h>
> +
> +enum cvm_pmu_type {
> +	CVM_PMU_LMC,
> +};
> +
> +/* maximum number of parallel hardware counters for all pmu types */
> +#define CVM_PMU_MAX_COUNTERS 64
> +
> +/* generic struct to cover the different pmu types */
> +struct cvm_pmu_dev {
> +	struct pmu pmu;
> +	const char *pmu_name;

It seems that pmu_name is redundant since struct pmu has a name field,
Mark has mentioned it in HiSilicon uncore PMU driver, Link:
https://patchwork.kernel.org/patch/9861821/

> +	bool (*event_valid)(u64);
> +	void __iomem *map;
> +	struct pci_dev *pdev;
> +	int num_counters;
> +	struct perf_event *events[CVM_PMU_MAX_COUNTERS];
> +	struct list_head entry;
> +	struct hlist_node cpuhp_node;
> +	cpumask_t active_mask;
> +};
> +
> +static struct list_head cvm_pmu_lmcs;
> +static struct list_head cvm_pmu_tlks;
> +
> +/*
> + * Common Cavium PMU stuff
> + *
> + * Shared properties of the different PMU types:
> + * - all counters are 64 bit long
> + * - there are no overflow interrupts
> + * - all devices with PMU counters appear as PCI devices
> + *
> + * Counter control, access and device association depends on the
> + * PMU type.
> + */
> +
> +#define to_pmu_dev(x) container_of((x), struct cvm_pmu_dev, pmu)
> +
> +static int cvm_pmu_event_init(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct cvm_pmu_dev *pmu_dev;
> +	struct perf_event *sibling;
> +
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	/* we do not support sampling */
> +	if (is_sampling_event(event))
> +		return -EINVAL;
> +
> +	/* PMU counters do not support any these bits */
> +	if (event->attr.exclude_user	||
> +	    event->attr.exclude_kernel	||
> +	    event->attr.exclude_host	||
> +	    event->attr.exclude_guest	||
> +	    event->attr.exclude_hv	||
> +	    event->attr.exclude_idle)
> +		return -EINVAL;
> +
> +	pmu_dev = to_pmu_dev(event->pmu);
> +	if (!pmu_dev->event_valid(event->attr.config))
> +		return -EINVAL;
> +
> +	/*
> +	 * Forbid groups containing mixed PMUs, software events are acceptable.
> +	 */
> +	if (event->group_leader->pmu != event->pmu &&
> +	    !is_software_event(event->group_leader))
> +		return -EINVAL;
> +
> +	list_for_each_entry(sibling, &event->group_leader->sibling_list,
> +			    group_entry)
> +		if (sibling->pmu != event->pmu &&
> +		    !is_software_event(sibling))
> +			return -EINVAL;
> +
> +	hwc->config = event->attr.config;
> +	hwc->idx = -1;

Blank line?

> +	return 0;
> +}
> +
> +static void cvm_pmu_read(struct perf_event *event)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	u64 prev, delta, new;
> +
> +again:
> +	prev = local64_read(&hwc->prev_count);
> +	new = readq(hwc->event_base + pmu_dev->map);
> +
> +	if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev)
> +		goto again;
> +
> +	delta = new - prev;
> +	local64_add(delta, &event->count);
> +}
> +
> +static void cvm_pmu_start(struct perf_event *event, int flags)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	u64 new;
> +
> +	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> +		return;
> +
> +	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> +	hwc->state = 0;
> +
> +	/* update prev_count always in order support unstoppable counters */
> +	new = readq(hwc->event_base + pmu_dev->map);
> +	local64_set(&hwc->prev_count, new);
> +
> +	perf_event_update_userpage(event);
> +}
> +
> +static void cvm_pmu_stop(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> +	hwc->state |= PERF_HES_STOPPED;
> +
> +	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
> +		cvm_pmu_read(event);
> +		hwc->state |= PERF_HES_UPTODATE;
> +	}
> +}
> +
> +static int cvm_pmu_add(struct perf_event *event, int flags, u64 config_base,
> +		       u64 event_base)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (!cmpxchg(&pmu_dev->events[hwc->config], NULL, event))
> +		hwc->idx = hwc->config;
> +
> +	if (hwc->idx == -1)
> +		return -EBUSY;
> +
> +	hwc->config_base = config_base;
> +	hwc->event_base = event_base;
> +	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> +
> +	if (flags & PERF_EF_START)
> +		pmu_dev->pmu.start(event, PERF_EF_RELOAD);
> +
> +	return 0;
> +}
> +
> +static void cvm_pmu_del(struct perf_event *event, int flags)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	int i;
> +
> +	event->pmu->stop(event, PERF_EF_UPDATE);
> +
> +	/*
> +	 * For programmable counters we need to check where we installed it.
> +	 * To keep this function generic always test the more complicated
> +	 * case (free running counters won't need the loop).
> +	 */
> +	for (i = 0; i < pmu_dev->num_counters; i++)
> +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> +			break;
> +
> +	perf_event_update_userpage(event);
> +	hwc->idx = -1;
> +}
> +
> +static ssize_t cvm_pmu_event_sysfs_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *page)
> +{
> +	struct perf_pmu_events_attr *pmu_attr =
> +		container_of(attr, struct perf_pmu_events_attr, attr);
> +
> +	if (pmu_attr->event_str)
> +		return sprintf(page, "%s", pmu_attr->event_str);
> +
> +	return 0;
> +}
> +
> +/*
> + * The pmu events are independent from CPUs. Provide a cpumask
> + * nevertheless to prevent perf from adding the event per-cpu and just
> + * set the mask to one online CPU. Use the same cpumask for all "uncore"
> + * devices.
> + *
> + * There is a performance penalty for accessing a device from a CPU on
> + * another socket, but we do not care.
> + */
> +static int cvm_pmu_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
> +{
> +	struct cvm_pmu_dev *pmu_dev;
> +	int new_cpu;

unsigned int?

> +
> +	pmu_dev = hlist_entry_safe(node, struct cvm_pmu_dev, cpuhp_node);
> +	if (!cpumask_test_and_clear_cpu(old_cpu, &pmu_dev->active_mask))
> +		return 0;
> +
> +	new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
> +	if (new_cpu >= nr_cpu_ids)
> +		return 0;
> +
> +	perf_pmu_migrate_context(&pmu_dev->pmu, old_cpu, new_cpu);
> +	cpumask_set_cpu(new_cpu, &pmu_dev->active_mask);
> +
> +	return 0;
> +}
> +
> +static ssize_t cvm_pmu_attr_show_cpumask(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct pmu *pmu = dev_get_drvdata(dev);
> +	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
> +
> +	return cpumap_print_to_pagebuf(true, buf, &pmu_dev->active_mask);
> +}
> +
> +static DEVICE_ATTR(cpumask, S_IRUGO, cvm_pmu_attr_show_cpumask, NULL);
> +
> +static struct attribute *cvm_pmu_attrs[] = {
> +	&dev_attr_cpumask.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_attr_group = {
> +	.attrs = cvm_pmu_attrs,
> +};
> +
> +/*
> + * LMC (memory controller) counters:
> + * - not stoppable, always on, read-only
> + * - one PCI device per memory controller
> + */
> +#define LMC_CONFIG_OFFSET		0x188
> +#define LMC_CONFIG_RESET_BIT		BIT(17)
> +
> +/* LMC events */
> +#define LMC_EVENT_IFB_CNT		0x1d0
> +#define LMC_EVENT_OPS_CNT		0x1d8
> +#define LMC_EVENT_DCLK_CNT		0x1e0
> +#define LMC_EVENT_BANK_CONFLICT1	0x360
> +#define LMC_EVENT_BANK_CONFLICT2	0x368
> +
> +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> +	&((struct perf_pmu_events_attr[]) {						\
> +		{									\
> +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> +			_id,								\
> +			"lmc_event=" __stringify(_id),					\
> +		}									\
> +	})[0].attr.attr
> +
> +/* map counter numbers to register offsets */
> +static int lmc_events[] = {

Add const?

> +	LMC_EVENT_IFB_CNT,
> +	LMC_EVENT_OPS_CNT,
> +	LMC_EVENT_DCLK_CNT,
> +	LMC_EVENT_BANK_CONFLICT1,
> +	LMC_EVENT_BANK_CONFLICT2,
> +};
> +
> +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> +			   lmc_events[hwc->config]);
> +}
> +
> +PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> +
> +static struct attribute *cvm_pmu_lmc_format_attr[] = {
> +	&format_attr_lmc_event.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_format_group = {
> +	.name = "format",
> +	.attrs = cvm_pmu_lmc_format_attr,
> +};
> +
> +static struct attribute *cvm_pmu_lmc_events_attr[] = {
> +	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> +	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> +	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_events_group = {
> +	.name = "events",
> +	.attrs = cvm_pmu_lmc_events_attr,
> +};
> +
> +static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
> +	&cvm_pmu_attr_group,
> +	&cvm_pmu_lmc_format_group,
> +	&cvm_pmu_lmc_events_group,
> +	NULL,
> +};
> +
> +static bool cvm_pmu_lmc_event_valid(u64 config)
> +{
> +	return (config < ARRAY_SIZE(lmc_events));
> +}
> +
> +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct cvm_pmu_dev *next, *lmc;
> +	int nr = 0, ret = -ENOMEM;
> +
> +	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);

How about use devm_kzalloc?

> +	if (!lmc)
> +		return -ENOMEM;
> +
> +	lmc->map = ioremap(pci_resource_start(pdev, 0),
> +			   pci_resource_len(pdev, 0));
> +	if (!lmc->map)
> +		goto fail_ioremap;
> +
> +	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
> +		nr++;
> +	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);

Use devm_kasprintf, simplify fail handle and memory free?

> +	if (!lmc->pmu_name)
> +		goto fail_kasprintf;
> +
> +	lmc->pdev = pdev;
> +	lmc->num_counters = ARRAY_SIZE(lmc_events);
> +	lmc->pmu = (struct pmu) {
> +		.task_ctx_nr    = perf_invalid_context,
> +		.event_init	= cvm_pmu_event_init,
> +		.add		= cvm_pmu_lmc_add,
> +		.del		= cvm_pmu_del,
> +		.start		= cvm_pmu_start,
> +		.stop		= cvm_pmu_stop,
> +		.read		= cvm_pmu_read,
> +		.attr_groups	= cvm_pmu_lmc_attr_groups,
> +	};
> +
> +	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +					 &lmc->cpuhp_node);
> +
> +	/*
> +	 * perf PMU is CPU dependent so pick a random CPU and migrate away
> +	 * if it goes offline.
> +	 */
> +	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
> +
> +	list_add(&lmc->entry, &cvm_pmu_lmcs);
> +	lmc->event_valid = cvm_pmu_lmc_event_valid;
> +
> +	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
> +	if (ret)
> +		goto fail_pmu;
> +
> +	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
> +		 lmc->pmu_name, lmc->num_counters);

Blank line?

> +	return 0;
> +
> +fail_pmu:
> +	kfree(lmc->pmu_name);
> +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +				    &lmc->cpuhp_node);
> +fail_kasprintf:
> +	iounmap(lmc->map);
> +fail_ioremap:
> +	kfree(lmc);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_probe);
> +
> +void cvm_lmc_pmu_remove(struct pci_dev *pdev)
> +{
> +	struct list_head *l, *tmp;
> +	struct cvm_pmu_dev *lmc;
> +
> +	list_for_each_safe(l, tmp, &cvm_pmu_lmcs) {
> +		lmc = list_entry(l, struct cvm_pmu_dev, entry);
> +		if (pdev != lmc->pdev)
> +			continue;
> +
> +		perf_pmu_unregister(&lmc->pmu);
> +		iounmap(lmc->map);
> +		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +					    &lmc->cpuhp_node);
> +		list_del(&lmc->entry);
> +		kfree(lmc->pmu_name);
> +		kfree(lmc);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
> +
> +static int __init cvm_pmu_init(void)
> +{
> +	INIT_LIST_HEAD(&cvm_pmu_lmcs);
> +	INIT_LIST_HEAD(&cvm_pmu_tlks);
> +
> +	return cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +				       "perf/arm/cvm:online", NULL,
> +				       cvm_pmu_offline_cpu);
> +}
> +
> +static void __exit cvm_pmu_exit(void)
> +{
> +	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_CVM_ONLINE);
> +}
> +
> +module_init(cvm_pmu_init);
> +module_exit(cvm_pmu_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Cavium, Inc.");
> +MODULE_DESCRIPTION("PMU Driver for Cavium ThunderX SOC");
> diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
> index 87248e8..d21d59c 100644
> --- a/drivers/soc/cavium/cavium_lmc.c
> +++ b/drivers/soc/cavium/cavium_lmc.c
> @@ -17,6 +17,8 @@
>  static int cvm_lmc_probe(struct pci_dev *pdev,
>  			 const struct pci_device_id *ent)
>  {
> +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> +		cvm_lmc_pmu_probe(pdev, ent);
>  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
>  		thunderx_edac_lmc_probe(pdev, ent);
>  	return 0;
> @@ -24,6 +26,8 @@ static int cvm_lmc_probe(struct pci_dev *pdev,
>  
>  static void cvm_lmc_remove(struct pci_dev *pdev)
>  {
> +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> +		cvm_lmc_pmu_remove(pdev);
>  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
>  		thunderx_edac_lmc_remove(pdev);
>  }
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 82b30e6..ca84ac8 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -139,6 +139,7 @@ enum cpuhp_state {
>  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
>  	CPUHP_AP_WORKQUEUE_ONLINE,
>  	CPUHP_AP_RCUTREE_ONLINE,
> +	CPUHP_AP_PERF_ARM_CVM_ONLINE,

Alphabetic order?

>  	CPUHP_AP_ONLINE_DYN,
>  	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
>  	CPUHP_AP_X86_HPET_ONLINE,
> diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
> index 336f467..e5ad650 100644
> --- a/include/linux/soc/cavium/lmc.h
> +++ b/include/linux/soc/cavium/lmc.h
> @@ -3,6 +3,9 @@
>  
>  #include <linux/pci.h>
>  
> +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> +void cvm_lmc_pmu_remove(struct pci_dev *pdev);
> +
>  int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
>  void thunderx_edac_lmc_remove(struct pci_dev *pdev);
>  
> 

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> Hi Jan,
> 
> Some trivial things i noticed, please consider if you are glad.
> 
> Thanks,
> Shaokun

Hi Shaokun, thanks for the review.

> On 2017/8/29 21:12, Jan Glauber wrote:
> > Add support for the PMU counters on Cavium SOC memory controllers.
> > 
> > This patch also adds generic functions to allow supporting more
> > devices with PMU counters.
> > 
> > Properties of the LMC PMU counters:
> > - not stoppable
> > - fixed purpose
> > - read-only
> > - one PCI device per memory controller
> > 
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  drivers/perf/Kconfig            |   8 +
> >  drivers/perf/Makefile           |   1 +
> >  drivers/perf/cavium_pmu.c       | 445 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/soc/cavium/cavium_lmc.c |   4 +
> >  include/linux/cpuhotplug.h      |   1 +
> >  include/linux/soc/cavium/lmc.h  |   3 +
> >  6 files changed, 462 insertions(+)
> >  create mode 100644 drivers/perf/cavium_pmu.c
> > 
> > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> > index e5197ff..a787562 100644
> > --- a/drivers/perf/Kconfig
> > +++ b/drivers/perf/Kconfig
> > @@ -43,4 +43,12 @@ config XGENE_PMU
> >          help
> >            Say y if you want to use APM X-Gene SoC performance monitors.
> >  
> > +config CAVIUM_PMU_LMC
> > +	tristate "Cavium SOC memory controller PMU"
> > +	depends on ARCH_THUNDER && m
> > +	select CAVIUM_LMC
> > +	help
> > +	  Provides PMU counters for the memory controller on
> > +	  Cavium ThunderX or OcteonTX SOCs.
> > +
> >  endmenu
> > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> > index 6420bd4..077a15d 100644
> > --- a/drivers/perf/Makefile
> > +++ b/drivers/perf/Makefile
> > @@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
> >  obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
> >  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> >  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> > +obj-$(CONFIG_CAVIUM_PMU_LMC) += cavium_pmu.o
> 
> Keep in alphabetic order?
> 

OK.

> > diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
> > new file mode 100644
> > index 0000000..bcdedaa
> > --- /dev/null
> > +++ b/drivers/perf/cavium_pmu.c
> > @@ -0,0 +1,445 @@
> > +/*
> > + * Cavium ARM SOC "uncore" PMU counters
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + *
> > + * Copyright Cavium, Inc. 2017
> > + * Author(s): Jan Glauber <jan.glauber@cavium.com>
> > + *
> > + */
> > +#include <linux/cpumask.h>
> > +#include <linux/cpuhotplug.h>
> > +#include <linux/io.h>
> > +#include <linux/export.h>
> 
> Keep the include header files in alphabetic order too?
> 
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/pci.h>
> > +#include <linux/perf_event.h>
> > +#include <linux/slab.h>
> > +#include <linux/soc/cavium/lmc.h>
> > +
> > +enum cvm_pmu_type {
> > +	CVM_PMU_LMC,
> > +};
> > +
> > +/* maximum number of parallel hardware counters for all pmu types */
> > +#define CVM_PMU_MAX_COUNTERS 64
> > +
> > +/* generic struct to cover the different pmu types */
> > +struct cvm_pmu_dev {
> > +	struct pmu pmu;
> > +	const char *pmu_name;
> 
> It seems that pmu_name is redundant since struct pmu has a name field,
> Mark has mentioned it in HiSilicon uncore PMU driver, Link:
> https://patchwork.kernel.org/patch/9861821/

I don't get it. perf_pmu_register() just copies the char* from the
argument into pmu->name. Somewhere the string must be allocated.
That's why I have cvm_pmu_dev->pmu_name.

> > +	bool (*event_valid)(u64);
> > +	void __iomem *map;
> > +	struct pci_dev *pdev;
> > +	int num_counters;
> > +	struct perf_event *events[CVM_PMU_MAX_COUNTERS];
> > +	struct list_head entry;
> > +	struct hlist_node cpuhp_node;
> > +	cpumask_t active_mask;
> > +};
> > +
> > +static struct list_head cvm_pmu_lmcs;
> > +static struct list_head cvm_pmu_tlks;
> > +
> > +/*
> > + * Common Cavium PMU stuff
> > + *
> > + * Shared properties of the different PMU types:
> > + * - all counters are 64 bit long
> > + * - there are no overflow interrupts
> > + * - all devices with PMU counters appear as PCI devices
> > + *
> > + * Counter control, access and device association depends on the
> > + * PMU type.
> > + */
> > +
> > +#define to_pmu_dev(x) container_of((x), struct cvm_pmu_dev, pmu)
> > +
> > +static int cvm_pmu_event_init(struct perf_event *event)
> > +{
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	struct cvm_pmu_dev *pmu_dev;
> > +	struct perf_event *sibling;
> > +
> > +	if (event->attr.type != event->pmu->type)
> > +		return -ENOENT;
> > +
> > +	/* we do not support sampling */
> > +	if (is_sampling_event(event))
> > +		return -EINVAL;
> > +
> > +	/* PMU counters do not support any these bits */
> > +	if (event->attr.exclude_user	||
> > +	    event->attr.exclude_kernel	||
> > +	    event->attr.exclude_host	||
> > +	    event->attr.exclude_guest	||
> > +	    event->attr.exclude_hv	||
> > +	    event->attr.exclude_idle)
> > +		return -EINVAL;
> > +
> > +	pmu_dev = to_pmu_dev(event->pmu);
> > +	if (!pmu_dev->event_valid(event->attr.config))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Forbid groups containing mixed PMUs, software events are acceptable.
> > +	 */
> > +	if (event->group_leader->pmu != event->pmu &&
> > +	    !is_software_event(event->group_leader))
> > +		return -EINVAL;
> > +
> > +	list_for_each_entry(sibling, &event->group_leader->sibling_list,
> > +			    group_entry)
> > +		if (sibling->pmu != event->pmu &&
> > +		    !is_software_event(sibling))
> > +			return -EINVAL;
> > +
> > +	hwc->config = event->attr.config;
> > +	hwc->idx = -1;
> 
> Blank line?

OK.

> > +	return 0;
> > +}
> > +
> > +static void cvm_pmu_read(struct perf_event *event)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	u64 prev, delta, new;
> > +
> > +again:
> > +	prev = local64_read(&hwc->prev_count);
> > +	new = readq(hwc->event_base + pmu_dev->map);
> > +
> > +	if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev)
> > +		goto again;
> > +
> > +	delta = new - prev;
> > +	local64_add(delta, &event->count);
> > +}
> > +
> > +static void cvm_pmu_start(struct perf_event *event, int flags)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	u64 new;
> > +
> > +	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> > +		return;
> > +
> > +	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> > +	hwc->state = 0;
> > +
> > +	/* update prev_count always in order support unstoppable counters */
> > +	new = readq(hwc->event_base + pmu_dev->map);
> > +	local64_set(&hwc->prev_count, new);
> > +
> > +	perf_event_update_userpage(event);
> > +}
> > +
> > +static void cvm_pmu_stop(struct perf_event *event, int flags)
> > +{
> > +	struct hw_perf_event *hwc = &event->hw;
> > +
> > +	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> > +	hwc->state |= PERF_HES_STOPPED;
> > +
> > +	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
> > +		cvm_pmu_read(event);
> > +		hwc->state |= PERF_HES_UPTODATE;
> > +	}
> > +}
> > +
> > +static int cvm_pmu_add(struct perf_event *event, int flags, u64 config_base,
> > +		       u64 event_base)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +
> > +	if (!cmpxchg(&pmu_dev->events[hwc->config], NULL, event))
> > +		hwc->idx = hwc->config;
> > +
> > +	if (hwc->idx == -1)
> > +		return -EBUSY;
> > +
> > +	hwc->config_base = config_base;
> > +	hwc->event_base = event_base;
> > +	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> > +
> > +	if (flags & PERF_EF_START)
> > +		pmu_dev->pmu.start(event, PERF_EF_RELOAD);
> > +
> > +	return 0;
> > +}
> > +
> > +static void cvm_pmu_del(struct perf_event *event, int flags)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	int i;
> > +
> > +	event->pmu->stop(event, PERF_EF_UPDATE);
> > +
> > +	/*
> > +	 * For programmable counters we need to check where we installed it.
> > +	 * To keep this function generic always test the more complicated
> > +	 * case (free running counters won't need the loop).
> > +	 */
> > +	for (i = 0; i < pmu_dev->num_counters; i++)
> > +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> > +			break;
> > +
> > +	perf_event_update_userpage(event);
> > +	hwc->idx = -1;
> > +}
> > +
> > +static ssize_t cvm_pmu_event_sysfs_show(struct device *dev,
> > +					struct device_attribute *attr,
> > +					char *page)
> > +{
> > +	struct perf_pmu_events_attr *pmu_attr =
> > +		container_of(attr, struct perf_pmu_events_attr, attr);
> > +
> > +	if (pmu_attr->event_str)
> > +		return sprintf(page, "%s", pmu_attr->event_str);
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * The pmu events are independent from CPUs. Provide a cpumask
> > + * nevertheless to prevent perf from adding the event per-cpu and just
> > + * set the mask to one online CPU. Use the same cpumask for all "uncore"
> > + * devices.
> > + *
> > + * There is a performance penalty for accessing a device from a CPU on
> > + * another socket, but we do not care.
> > + */
> > +static int cvm_pmu_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev;
> > +	int new_cpu;
> 
> unsigned int?

I don't think we gonna overflow signed int soon, or? 

> > +
> > +	pmu_dev = hlist_entry_safe(node, struct cvm_pmu_dev, cpuhp_node);
> > +	if (!cpumask_test_and_clear_cpu(old_cpu, &pmu_dev->active_mask))
> > +		return 0;
> > +
> > +	new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
> > +	if (new_cpu >= nr_cpu_ids)
> > +		return 0;
> > +
> > +	perf_pmu_migrate_context(&pmu_dev->pmu, old_cpu, new_cpu);
> > +	cpumask_set_cpu(new_cpu, &pmu_dev->active_mask);
> > +
> > +	return 0;
> > +}
> > +
> > +static ssize_t cvm_pmu_attr_show_cpumask(struct device *dev,
> > +					 struct device_attribute *attr,
> > +					 char *buf)
> > +{
> > +	struct pmu *pmu = dev_get_drvdata(dev);
> > +	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
> > +
> > +	return cpumap_print_to_pagebuf(true, buf, &pmu_dev->active_mask);
> > +}
> > +
> > +static DEVICE_ATTR(cpumask, S_IRUGO, cvm_pmu_attr_show_cpumask, NULL);
> > +
> > +static struct attribute *cvm_pmu_attrs[] = {
> > +	&dev_attr_cpumask.attr,
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group cvm_pmu_attr_group = {
> > +	.attrs = cvm_pmu_attrs,
> > +};
> > +
> > +/*
> > + * LMC (memory controller) counters:
> > + * - not stoppable, always on, read-only
> > + * - one PCI device per memory controller
> > + */
> > +#define LMC_CONFIG_OFFSET		0x188
> > +#define LMC_CONFIG_RESET_BIT		BIT(17)
> > +
> > +/* LMC events */
> > +#define LMC_EVENT_IFB_CNT		0x1d0
> > +#define LMC_EVENT_OPS_CNT		0x1d8
> > +#define LMC_EVENT_DCLK_CNT		0x1e0
> > +#define LMC_EVENT_BANK_CONFLICT1	0x360
> > +#define LMC_EVENT_BANK_CONFLICT2	0x368
> > +
> > +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> > +	&((struct perf_pmu_events_attr[]) {						\
> > +		{									\
> > +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> > +			_id,								\
> > +			"lmc_event=" __stringify(_id),					\
> > +		}									\
> > +	})[0].attr.attr
> > +
> > +/* map counter numbers to register offsets */
> > +static int lmc_events[] = {
> 
> Add const?

Yes.

> > +	LMC_EVENT_IFB_CNT,
> > +	LMC_EVENT_OPS_CNT,
> > +	LMC_EVENT_DCLK_CNT,
> > +	LMC_EVENT_BANK_CONFLICT1,
> > +	LMC_EVENT_BANK_CONFLICT2,
> > +};
> > +
> > +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> > +{
> > +	struct hw_perf_event *hwc = &event->hw;
> > +
> > +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> > +			   lmc_events[hwc->config]);
> > +}
> > +
> > +PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> > +
> > +static struct attribute *cvm_pmu_lmc_format_attr[] = {
> > +	&format_attr_lmc_event.attr,
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group cvm_pmu_lmc_format_group = {
> > +	.name = "format",
> > +	.attrs = cvm_pmu_lmc_format_attr,
> > +};
> > +
> > +static struct attribute *cvm_pmu_lmc_events_attr[] = {
> > +	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> > +	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> > +	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> > +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> > +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group cvm_pmu_lmc_events_group = {
> > +	.name = "events",
> > +	.attrs = cvm_pmu_lmc_events_attr,
> > +};
> > +
> > +static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
> > +	&cvm_pmu_attr_group,
> > +	&cvm_pmu_lmc_format_group,
> > +	&cvm_pmu_lmc_events_group,
> > +	NULL,
> > +};
> > +
> > +static bool cvm_pmu_lmc_event_valid(u64 config)
> > +{
> > +	return (config < ARRAY_SIZE(lmc_events));
> > +}
> > +
> > +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > +{
> > +	struct cvm_pmu_dev *next, *lmc;
> > +	int nr = 0, ret = -ENOMEM;
> > +
> > +	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);
> 
> How about use devm_kzalloc?

I've not used devm_kzalloc before as I didn't have a traditional device
probe/remove with the previous versions. I'm not sure it would work now,
but I'll give it a try as devm_ is really great.

> > +	if (!lmc)
> > +		return -ENOMEM;
> > +
> > +	lmc->map = ioremap(pci_resource_start(pdev, 0),
> > +			   pci_resource_len(pdev, 0));
> > +	if (!lmc->map)
> > +		goto fail_ioremap;
> > +
> > +	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
> > +		nr++;
> > +	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);
> 
> Use devm_kasprintf, simplify fail handle and memory free?
> 
> > +	if (!lmc->pmu_name)
> > +		goto fail_kasprintf;
> > +
> > +	lmc->pdev = pdev;
> > +	lmc->num_counters = ARRAY_SIZE(lmc_events);
> > +	lmc->pmu = (struct pmu) {
> > +		.task_ctx_nr    = perf_invalid_context,
> > +		.event_init	= cvm_pmu_event_init,
> > +		.add		= cvm_pmu_lmc_add,
> > +		.del		= cvm_pmu_del,
> > +		.start		= cvm_pmu_start,
> > +		.stop		= cvm_pmu_stop,
> > +		.read		= cvm_pmu_read,
> > +		.attr_groups	= cvm_pmu_lmc_attr_groups,
> > +	};
> > +
> > +	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +					 &lmc->cpuhp_node);
> > +
> > +	/*
> > +	 * perf PMU is CPU dependent so pick a random CPU and migrate away
> > +	 * if it goes offline.
> > +	 */
> > +	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
> > +
> > +	list_add(&lmc->entry, &cvm_pmu_lmcs);
> > +	lmc->event_valid = cvm_pmu_lmc_event_valid;
> > +
> > +	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
> > +	if (ret)
> > +		goto fail_pmu;
> > +
> > +	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
> > +		 lmc->pmu_name, lmc->num_counters);
> 
> Blank line?

OK

> > +	return 0;
> > +
> > +fail_pmu:
> > +	kfree(lmc->pmu_name);
> > +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +				    &lmc->cpuhp_node);
> > +fail_kasprintf:
> > +	iounmap(lmc->map);
> > +fail_ioremap:
> > +	kfree(lmc);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_probe);
> > +
> > +void cvm_lmc_pmu_remove(struct pci_dev *pdev)
> > +{
> > +	struct list_head *l, *tmp;
> > +	struct cvm_pmu_dev *lmc;
> > +
> > +	list_for_each_safe(l, tmp, &cvm_pmu_lmcs) {
> > +		lmc = list_entry(l, struct cvm_pmu_dev, entry);
> > +		if (pdev != lmc->pdev)
> > +			continue;
> > +
> > +		perf_pmu_unregister(&lmc->pmu);
> > +		iounmap(lmc->map);
> > +		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +					    &lmc->cpuhp_node);
> > +		list_del(&lmc->entry);
> > +		kfree(lmc->pmu_name);
> > +		kfree(lmc);
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
> > +
> > +static int __init cvm_pmu_init(void)
> > +{
> > +	INIT_LIST_HEAD(&cvm_pmu_lmcs);
> > +	INIT_LIST_HEAD(&cvm_pmu_tlks);
> > +
> > +	return cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +				       "perf/arm/cvm:online", NULL,
> > +				       cvm_pmu_offline_cpu);
> > +}
> > +
> > +static void __exit cvm_pmu_exit(void)
> > +{
> > +	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_CVM_ONLINE);
> > +}
> > +
> > +module_init(cvm_pmu_init);
> > +module_exit(cvm_pmu_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Cavium, Inc.");
> > +MODULE_DESCRIPTION("PMU Driver for Cavium ThunderX SOC");
> > diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
> > index 87248e8..d21d59c 100644
> > --- a/drivers/soc/cavium/cavium_lmc.c
> > +++ b/drivers/soc/cavium/cavium_lmc.c
> > @@ -17,6 +17,8 @@
> >  static int cvm_lmc_probe(struct pci_dev *pdev,
> >  			 const struct pci_device_id *ent)
> >  {
> > +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> > +		cvm_lmc_pmu_probe(pdev, ent);
> >  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
> >  		thunderx_edac_lmc_probe(pdev, ent);
> >  	return 0;
> > @@ -24,6 +26,8 @@ static int cvm_lmc_probe(struct pci_dev *pdev,
> >  
> >  static void cvm_lmc_remove(struct pci_dev *pdev)
> >  {
> > +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> > +		cvm_lmc_pmu_remove(pdev);
> >  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
> >  		thunderx_edac_lmc_remove(pdev);
> >  }
> > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > index 82b30e6..ca84ac8 100644
> > --- a/include/linux/cpuhotplug.h
> > +++ b/include/linux/cpuhotplug.h
> > @@ -139,6 +139,7 @@ enum cpuhp_state {
> >  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> >  	CPUHP_AP_WORKQUEUE_ONLINE,
> >  	CPUHP_AP_RCUTREE_ONLINE,
> > +	CPUHP_AP_PERF_ARM_CVM_ONLINE,
> 
> Alphabetic order?

These don't look alphabetically ordered to me.

thanks, Jan

> 
> >  	CPUHP_AP_ONLINE_DYN,
> >  	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
> >  	CPUHP_AP_X86_HPET_ONLINE,
> > diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
> > index 336f467..e5ad650 100644
> > --- a/include/linux/soc/cavium/lmc.h
> > +++ b/include/linux/soc/cavium/lmc.h
> > @@ -3,6 +3,9 @@
> >  
> >  #include <linux/pci.h>
> >  
> > +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> > +void cvm_lmc_pmu_remove(struct pci_dev *pdev);
> > +
> >  int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> >  void thunderx_edac_lmc_remove(struct pci_dev *pdev);
> >  
> > 

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> Hi Jan,
> 
> Some trivial things i noticed, please consider if you are glad.
> 
> Thanks,
> Shaokun

Hi Shaokun, thanks for the review.

> On 2017/8/29 21:12, Jan Glauber wrote:
> > Add support for the PMU counters on Cavium SOC memory controllers.
> > 
> > This patch also adds generic functions to allow supporting more
> > devices with PMU counters.
> > 
> > Properties of the LMC PMU counters:
> > - not stoppable
> > - fixed purpose
> > - read-only
> > - one PCI device per memory controller
> > 
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  drivers/perf/Kconfig            |   8 +
> >  drivers/perf/Makefile           |   1 +
> >  drivers/perf/cavium_pmu.c       | 445 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/soc/cavium/cavium_lmc.c |   4 +
> >  include/linux/cpuhotplug.h      |   1 +
> >  include/linux/soc/cavium/lmc.h  |   3 +
> >  6 files changed, 462 insertions(+)
> >  create mode 100644 drivers/perf/cavium_pmu.c
> > 
> > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> > index e5197ff..a787562 100644
> > --- a/drivers/perf/Kconfig
> > +++ b/drivers/perf/Kconfig
> > @@ -43,4 +43,12 @@ config XGENE_PMU
> >          help
> >            Say y if you want to use APM X-Gene SoC performance monitors.
> >  
> > +config CAVIUM_PMU_LMC
> > +	tristate "Cavium SOC memory controller PMU"
> > +	depends on ARCH_THUNDER && m
> > +	select CAVIUM_LMC
> > +	help
> > +	  Provides PMU counters for the memory controller on
> > +	  Cavium ThunderX or OcteonTX SOCs.
> > +
> >  endmenu
> > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> > index 6420bd4..077a15d 100644
> > --- a/drivers/perf/Makefile
> > +++ b/drivers/perf/Makefile
> > @@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
> >  obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
> >  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> >  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> > +obj-$(CONFIG_CAVIUM_PMU_LMC) += cavium_pmu.o
> 
> Keep in alphabetic order?
> 

OK.

> > diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
> > new file mode 100644
> > index 0000000..bcdedaa
> > --- /dev/null
> > +++ b/drivers/perf/cavium_pmu.c
> > @@ -0,0 +1,445 @@
> > +/*
> > + * Cavium ARM SOC "uncore" PMU counters
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + *
> > + * Copyright Cavium, Inc. 2017
> > + * Author(s): Jan Glauber <jan.glauber@cavium.com>
> > + *
> > + */
> > +#include <linux/cpumask.h>
> > +#include <linux/cpuhotplug.h>
> > +#include <linux/io.h>
> > +#include <linux/export.h>
> 
> Keep the include header files in alphabetic order too?
> 
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/pci.h>
> > +#include <linux/perf_event.h>
> > +#include <linux/slab.h>
> > +#include <linux/soc/cavium/lmc.h>
> > +
> > +enum cvm_pmu_type {
> > +	CVM_PMU_LMC,
> > +};
> > +
> > +/* maximum number of parallel hardware counters for all pmu types */
> > +#define CVM_PMU_MAX_COUNTERS 64
> > +
> > +/* generic struct to cover the different pmu types */
> > +struct cvm_pmu_dev {
> > +	struct pmu pmu;
> > +	const char *pmu_name;
> 
> It seems that pmu_name is redundant since struct pmu has a name field,
> Mark has mentioned it in HiSilicon uncore PMU driver, Link:
> https://patchwork.kernel.org/patch/9861821/

I don't get it. perf_pmu_register() just copies the char* from the
argument into pmu->name. Somewhere the string must be allocated.
That's why I have cvm_pmu_dev->pmu_name.

> > +	bool (*event_valid)(u64);
> > +	void __iomem *map;
> > +	struct pci_dev *pdev;
> > +	int num_counters;
> > +	struct perf_event *events[CVM_PMU_MAX_COUNTERS];
> > +	struct list_head entry;
> > +	struct hlist_node cpuhp_node;
> > +	cpumask_t active_mask;
> > +};
> > +
> > +static struct list_head cvm_pmu_lmcs;
> > +static struct list_head cvm_pmu_tlks;
> > +
> > +/*
> > + * Common Cavium PMU stuff
> > + *
> > + * Shared properties of the different PMU types:
> > + * - all counters are 64 bit long
> > + * - there are no overflow interrupts
> > + * - all devices with PMU counters appear as PCI devices
> > + *
> > + * Counter control, access and device association depends on the
> > + * PMU type.
> > + */
> > +
> > +#define to_pmu_dev(x) container_of((x), struct cvm_pmu_dev, pmu)
> > +
> > +static int cvm_pmu_event_init(struct perf_event *event)
> > +{
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	struct cvm_pmu_dev *pmu_dev;
> > +	struct perf_event *sibling;
> > +
> > +	if (event->attr.type != event->pmu->type)
> > +		return -ENOENT;
> > +
> > +	/* we do not support sampling */
> > +	if (is_sampling_event(event))
> > +		return -EINVAL;
> > +
> > +	/* PMU counters do not support any these bits */
> > +	if (event->attr.exclude_user	||
> > +	    event->attr.exclude_kernel	||
> > +	    event->attr.exclude_host	||
> > +	    event->attr.exclude_guest	||
> > +	    event->attr.exclude_hv	||
> > +	    event->attr.exclude_idle)
> > +		return -EINVAL;
> > +
> > +	pmu_dev = to_pmu_dev(event->pmu);
> > +	if (!pmu_dev->event_valid(event->attr.config))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Forbid groups containing mixed PMUs, software events are acceptable.
> > +	 */
> > +	if (event->group_leader->pmu != event->pmu &&
> > +	    !is_software_event(event->group_leader))
> > +		return -EINVAL;
> > +
> > +	list_for_each_entry(sibling, &event->group_leader->sibling_list,
> > +			    group_entry)
> > +		if (sibling->pmu != event->pmu &&
> > +		    !is_software_event(sibling))
> > +			return -EINVAL;
> > +
> > +	hwc->config = event->attr.config;
> > +	hwc->idx = -1;
> 
> Blank line?

OK.

> > +	return 0;
> > +}
> > +
> > +static void cvm_pmu_read(struct perf_event *event)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	u64 prev, delta, new;
> > +
> > +again:
> > +	prev = local64_read(&hwc->prev_count);
> > +	new = readq(hwc->event_base + pmu_dev->map);
> > +
> > +	if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev)
> > +		goto again;
> > +
> > +	delta = new - prev;
> > +	local64_add(delta, &event->count);
> > +}
> > +
> > +static void cvm_pmu_start(struct perf_event *event, int flags)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	u64 new;
> > +
> > +	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> > +		return;
> > +
> > +	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> > +	hwc->state = 0;
> > +
> > +	/* update prev_count always in order support unstoppable counters */
> > +	new = readq(hwc->event_base + pmu_dev->map);
> > +	local64_set(&hwc->prev_count, new);
> > +
> > +	perf_event_update_userpage(event);
> > +}
> > +
> > +static void cvm_pmu_stop(struct perf_event *event, int flags)
> > +{
> > +	struct hw_perf_event *hwc = &event->hw;
> > +
> > +	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> > +	hwc->state |= PERF_HES_STOPPED;
> > +
> > +	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
> > +		cvm_pmu_read(event);
> > +		hwc->state |= PERF_HES_UPTODATE;
> > +	}
> > +}
> > +
> > +static int cvm_pmu_add(struct perf_event *event, int flags, u64 config_base,
> > +		       u64 event_base)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +
> > +	if (!cmpxchg(&pmu_dev->events[hwc->config], NULL, event))
> > +		hwc->idx = hwc->config;
> > +
> > +	if (hwc->idx == -1)
> > +		return -EBUSY;
> > +
> > +	hwc->config_base = config_base;
> > +	hwc->event_base = event_base;
> > +	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> > +
> > +	if (flags & PERF_EF_START)
> > +		pmu_dev->pmu.start(event, PERF_EF_RELOAD);
> > +
> > +	return 0;
> > +}
> > +
> > +static void cvm_pmu_del(struct perf_event *event, int flags)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	int i;
> > +
> > +	event->pmu->stop(event, PERF_EF_UPDATE);
> > +
> > +	/*
> > +	 * For programmable counters we need to check where we installed it.
> > +	 * To keep this function generic always test the more complicated
> > +	 * case (free running counters won't need the loop).
> > +	 */
> > +	for (i = 0; i < pmu_dev->num_counters; i++)
> > +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> > +			break;
> > +
> > +	perf_event_update_userpage(event);
> > +	hwc->idx = -1;
> > +}
> > +
> > +static ssize_t cvm_pmu_event_sysfs_show(struct device *dev,
> > +					struct device_attribute *attr,
> > +					char *page)
> > +{
> > +	struct perf_pmu_events_attr *pmu_attr =
> > +		container_of(attr, struct perf_pmu_events_attr, attr);
> > +
> > +	if (pmu_attr->event_str)
> > +		return sprintf(page, "%s", pmu_attr->event_str);
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * The pmu events are independent from CPUs. Provide a cpumask
> > + * nevertheless to prevent perf from adding the event per-cpu and just
> > + * set the mask to one online CPU. Use the same cpumask for all "uncore"
> > + * devices.
> > + *
> > + * There is a performance penalty for accessing a device from a CPU on
> > + * another socket, but we do not care.
> > + */
> > +static int cvm_pmu_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
> > +{
> > +	struct cvm_pmu_dev *pmu_dev;
> > +	int new_cpu;
> 
> unsigned int?

I don't think we gonna overflow signed int soon, or? 

> > +
> > +	pmu_dev = hlist_entry_safe(node, struct cvm_pmu_dev, cpuhp_node);
> > +	if (!cpumask_test_and_clear_cpu(old_cpu, &pmu_dev->active_mask))
> > +		return 0;
> > +
> > +	new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
> > +	if (new_cpu >= nr_cpu_ids)
> > +		return 0;
> > +
> > +	perf_pmu_migrate_context(&pmu_dev->pmu, old_cpu, new_cpu);
> > +	cpumask_set_cpu(new_cpu, &pmu_dev->active_mask);
> > +
> > +	return 0;
> > +}
> > +
> > +static ssize_t cvm_pmu_attr_show_cpumask(struct device *dev,
> > +					 struct device_attribute *attr,
> > +					 char *buf)
> > +{
> > +	struct pmu *pmu = dev_get_drvdata(dev);
> > +	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
> > +
> > +	return cpumap_print_to_pagebuf(true, buf, &pmu_dev->active_mask);
> > +}
> > +
> > +static DEVICE_ATTR(cpumask, S_IRUGO, cvm_pmu_attr_show_cpumask, NULL);
> > +
> > +static struct attribute *cvm_pmu_attrs[] = {
> > +	&dev_attr_cpumask.attr,
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group cvm_pmu_attr_group = {
> > +	.attrs = cvm_pmu_attrs,
> > +};
> > +
> > +/*
> > + * LMC (memory controller) counters:
> > + * - not stoppable, always on, read-only
> > + * - one PCI device per memory controller
> > + */
> > +#define LMC_CONFIG_OFFSET		0x188
> > +#define LMC_CONFIG_RESET_BIT		BIT(17)
> > +
> > +/* LMC events */
> > +#define LMC_EVENT_IFB_CNT		0x1d0
> > +#define LMC_EVENT_OPS_CNT		0x1d8
> > +#define LMC_EVENT_DCLK_CNT		0x1e0
> > +#define LMC_EVENT_BANK_CONFLICT1	0x360
> > +#define LMC_EVENT_BANK_CONFLICT2	0x368
> > +
> > +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> > +	&((struct perf_pmu_events_attr[]) {						\
> > +		{									\
> > +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> > +			_id,								\
> > +			"lmc_event=" __stringify(_id),					\
> > +		}									\
> > +	})[0].attr.attr
> > +
> > +/* map counter numbers to register offsets */
> > +static int lmc_events[] = {
> 
> Add const?

Yes.

> > +	LMC_EVENT_IFB_CNT,
> > +	LMC_EVENT_OPS_CNT,
> > +	LMC_EVENT_DCLK_CNT,
> > +	LMC_EVENT_BANK_CONFLICT1,
> > +	LMC_EVENT_BANK_CONFLICT2,
> > +};
> > +
> > +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> > +{
> > +	struct hw_perf_event *hwc = &event->hw;
> > +
> > +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> > +			   lmc_events[hwc->config]);
> > +}
> > +
> > +PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> > +
> > +static struct attribute *cvm_pmu_lmc_format_attr[] = {
> > +	&format_attr_lmc_event.attr,
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group cvm_pmu_lmc_format_group = {
> > +	.name = "format",
> > +	.attrs = cvm_pmu_lmc_format_attr,
> > +};
> > +
> > +static struct attribute *cvm_pmu_lmc_events_attr[] = {
> > +	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> > +	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> > +	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> > +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> > +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group cvm_pmu_lmc_events_group = {
> > +	.name = "events",
> > +	.attrs = cvm_pmu_lmc_events_attr,
> > +};
> > +
> > +static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
> > +	&cvm_pmu_attr_group,
> > +	&cvm_pmu_lmc_format_group,
> > +	&cvm_pmu_lmc_events_group,
> > +	NULL,
> > +};
> > +
> > +static bool cvm_pmu_lmc_event_valid(u64 config)
> > +{
> > +	return (config < ARRAY_SIZE(lmc_events));
> > +}
> > +
> > +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > +{
> > +	struct cvm_pmu_dev *next, *lmc;
> > +	int nr = 0, ret = -ENOMEM;
> > +
> > +	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);
> 
> How about use devm_kzalloc?

I've not used devm_kzalloc before as I didn't have a traditional device
probe/remove with the previous versions. I'm not sure it would work now,
but I'll give it a try as devm_ is really great.

> > +	if (!lmc)
> > +		return -ENOMEM;
> > +
> > +	lmc->map = ioremap(pci_resource_start(pdev, 0),
> > +			   pci_resource_len(pdev, 0));
> > +	if (!lmc->map)
> > +		goto fail_ioremap;
> > +
> > +	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
> > +		nr++;
> > +	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);
> 
> Use devm_kasprintf, simplify fail handle and memory free?
> 
> > +	if (!lmc->pmu_name)
> > +		goto fail_kasprintf;
> > +
> > +	lmc->pdev = pdev;
> > +	lmc->num_counters = ARRAY_SIZE(lmc_events);
> > +	lmc->pmu = (struct pmu) {
> > +		.task_ctx_nr    = perf_invalid_context,
> > +		.event_init	= cvm_pmu_event_init,
> > +		.add		= cvm_pmu_lmc_add,
> > +		.del		= cvm_pmu_del,
> > +		.start		= cvm_pmu_start,
> > +		.stop		= cvm_pmu_stop,
> > +		.read		= cvm_pmu_read,
> > +		.attr_groups	= cvm_pmu_lmc_attr_groups,
> > +	};
> > +
> > +	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +					 &lmc->cpuhp_node);
> > +
> > +	/*
> > +	 * perf PMU is CPU dependent so pick a random CPU and migrate away
> > +	 * if it goes offline.
> > +	 */
> > +	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
> > +
> > +	list_add(&lmc->entry, &cvm_pmu_lmcs);
> > +	lmc->event_valid = cvm_pmu_lmc_event_valid;
> > +
> > +	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
> > +	if (ret)
> > +		goto fail_pmu;
> > +
> > +	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
> > +		 lmc->pmu_name, lmc->num_counters);
> 
> Blank line?

OK

> > +	return 0;
> > +
> > +fail_pmu:
> > +	kfree(lmc->pmu_name);
> > +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +				    &lmc->cpuhp_node);
> > +fail_kasprintf:
> > +	iounmap(lmc->map);
> > +fail_ioremap:
> > +	kfree(lmc);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_probe);
> > +
> > +void cvm_lmc_pmu_remove(struct pci_dev *pdev)
> > +{
> > +	struct list_head *l, *tmp;
> > +	struct cvm_pmu_dev *lmc;
> > +
> > +	list_for_each_safe(l, tmp, &cvm_pmu_lmcs) {
> > +		lmc = list_entry(l, struct cvm_pmu_dev, entry);
> > +		if (pdev != lmc->pdev)
> > +			continue;
> > +
> > +		perf_pmu_unregister(&lmc->pmu);
> > +		iounmap(lmc->map);
> > +		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +					    &lmc->cpuhp_node);
> > +		list_del(&lmc->entry);
> > +		kfree(lmc->pmu_name);
> > +		kfree(lmc);
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
> > +
> > +static int __init cvm_pmu_init(void)
> > +{
> > +	INIT_LIST_HEAD(&cvm_pmu_lmcs);
> > +	INIT_LIST_HEAD(&cvm_pmu_tlks);
> > +
> > +	return cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > +				       "perf/arm/cvm:online", NULL,
> > +				       cvm_pmu_offline_cpu);
> > +}
> > +
> > +static void __exit cvm_pmu_exit(void)
> > +{
> > +	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_CVM_ONLINE);
> > +}
> > +
> > +module_init(cvm_pmu_init);
> > +module_exit(cvm_pmu_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Cavium, Inc.");
> > +MODULE_DESCRIPTION("PMU Driver for Cavium ThunderX SOC");
> > diff --git a/drivers/soc/cavium/cavium_lmc.c b/drivers/soc/cavium/cavium_lmc.c
> > index 87248e8..d21d59c 100644
> > --- a/drivers/soc/cavium/cavium_lmc.c
> > +++ b/drivers/soc/cavium/cavium_lmc.c
> > @@ -17,6 +17,8 @@
> >  static int cvm_lmc_probe(struct pci_dev *pdev,
> >  			 const struct pci_device_id *ent)
> >  {
> > +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> > +		cvm_lmc_pmu_probe(pdev, ent);
> >  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
> >  		thunderx_edac_lmc_probe(pdev, ent);
> >  	return 0;
> > @@ -24,6 +26,8 @@ static int cvm_lmc_probe(struct pci_dev *pdev,
> >  
> >  static void cvm_lmc_remove(struct pci_dev *pdev)
> >  {
> > +	if (IS_ENABLED(CONFIG_CAVIUM_PMU_LMC))
> > +		cvm_lmc_pmu_remove(pdev);
> >  	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
> >  		thunderx_edac_lmc_remove(pdev);
> >  }
> > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > index 82b30e6..ca84ac8 100644
> > --- a/include/linux/cpuhotplug.h
> > +++ b/include/linux/cpuhotplug.h
> > @@ -139,6 +139,7 @@ enum cpuhp_state {
> >  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> >  	CPUHP_AP_WORKQUEUE_ONLINE,
> >  	CPUHP_AP_RCUTREE_ONLINE,
> > +	CPUHP_AP_PERF_ARM_CVM_ONLINE,
> 
> Alphabetic order?

These don't look alphabetically ordered to me.

thanks, Jan

> 
> >  	CPUHP_AP_ONLINE_DYN,
> >  	CPUHP_AP_ONLINE_DYN_END		= CPUHP_AP_ONLINE_DYN + 30,
> >  	CPUHP_AP_X86_HPET_ONLINE,
> > diff --git a/include/linux/soc/cavium/lmc.h b/include/linux/soc/cavium/lmc.h
> > index 336f467..e5ad650 100644
> > --- a/include/linux/soc/cavium/lmc.h
> > +++ b/include/linux/soc/cavium/lmc.h
> > @@ -3,6 +3,9 @@
> >  
> >  #include <linux/pci.h>
> >  
> > +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> > +void cvm_lmc_pmu_remove(struct pci_dev *pdev);
> > +
> >  int thunderx_edac_lmc_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> >  void thunderx_edac_lmc_remove(struct pci_dev *pdev);
> >  
> > 

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Mark Rutland]

On Thu, Aug 31, 2017 at 11:57:46AM +0200, Jan Glauber wrote:
> On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> > On 2017/8/29 21:12, Jan Glauber wrote:
> > > Add support for the PMU counters on Cavium SOC memory controllers.
> > > 
> > > This patch also adds generic functions to allow supporting more
> > > devices with PMU counters.

> > > +/* generic struct to cover the different pmu types */
> > > +struct cvm_pmu_dev {
> > > +	struct pmu pmu;
> > > +	const char *pmu_name;
> > 
> > It seems that pmu_name is redundant since struct pmu has a name field,
> > Mark has mentioned it in HiSilicon uncore PMU driver, Link:
> > https://patchwork.kernel.org/patch/9861821/
> 
> I don't get it. perf_pmu_register() just copies the char* from the
> argument into pmu->name. Somewhere the string must be allocated.
> That's why I have cvm_pmu_dev->pmu_name.

I'm not sure I follow. cvm_pmu_dev->pmu_name is just a char *, so what
does that have to do with allocation?

... unless you mean you want to allocate this in some variant-specific
code prior to passing it to code which calls perf_pmu_register(), and
you just need a place to stash it in the mean time?

> > > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > > index 82b30e6..ca84ac8 100644
> > > --- a/include/linux/cpuhotplug.h
> > > +++ b/include/linux/cpuhotplug.h
> > > @@ -139,6 +139,7 @@ enum cpuhp_state {
> > >  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> > >  	CPUHP_AP_WORKQUEUE_ONLINE,
> > >  	CPUHP_AP_RCUTREE_ONLINE,
> > > +	CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > 
> > Alphabetic order?
> 
> These don't look alphabetically ordered to me.

Sure, the full list is ordered by dependency.

However, we've generally kept the uncore PMUs together, and within the
group of system PMU CPUHP_AP_PERF_ARM_* callbacks, we've retained
alphabetical order.

Does this PMU need workqueues and RCU up before its HP callback is
invoked? Or can this be moved into the group of CPUHP_AP_PERF_ARM_*
above CPUHP_AP_WORKQUEUE_ONLINE and CPUHP_AP_RCUTREE_ONLINE? i.e.
between CPUHP_AP_PERF_ARM_CCN_ONLINE and CPUHP_AP_PERF_ARM_L2X0_ONLINE.

THanks,
Mark.

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Mark Rutland]

On Thu, Aug 31, 2017 at 11:57:46AM +0200, Jan Glauber wrote:
> On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> > On 2017/8/29 21:12, Jan Glauber wrote:
> > > Add support for the PMU counters on Cavium SOC memory controllers.
> > > 
> > > This patch also adds generic functions to allow supporting more
> > > devices with PMU counters.

> > > +/* generic struct to cover the different pmu types */
> > > +struct cvm_pmu_dev {
> > > +	struct pmu pmu;
> > > +	const char *pmu_name;
> > 
> > It seems that pmu_name is redundant since struct pmu has a name field,
> > Mark has mentioned it in HiSilicon uncore PMU driver, Link:
> > https://patchwork.kernel.org/patch/9861821/
> 
> I don't get it. perf_pmu_register() just copies the char* from the
> argument into pmu->name. Somewhere the string must be allocated.
> That's why I have cvm_pmu_dev->pmu_name.

I'm not sure I follow. cvm_pmu_dev->pmu_name is just a char *, so what
does that have to do with allocation?

... unless you mean you want to allocate this in some variant-specific
code prior to passing it to code which calls perf_pmu_register(), and
you just need a place to stash it in the mean time?

> > > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > > index 82b30e6..ca84ac8 100644
> > > --- a/include/linux/cpuhotplug.h
> > > +++ b/include/linux/cpuhotplug.h
> > > @@ -139,6 +139,7 @@ enum cpuhp_state {
> > >  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> > >  	CPUHP_AP_WORKQUEUE_ONLINE,
> > >  	CPUHP_AP_RCUTREE_ONLINE,
> > > +	CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > 
> > Alphabetic order?
> 
> These don't look alphabetically ordered to me.

Sure, the full list is ordered by dependency.

However, we've generally kept the uncore PMUs together, and within the
group of system PMU CPUHP_AP_PERF_ARM_* callbacks, we've retained
alphabetical order.

Does this PMU need workqueues and RCU up before its HP callback is
invoked? Or can this be moved into the group of CPUHP_AP_PERF_ARM_*
above CPUHP_AP_WORKQUEUE_ONLINE and CPUHP_AP_RCUTREE_ONLINE? i.e.
between CPUHP_AP_PERF_ARM_CCN_ONLINE and CPUHP_AP_PERF_ARM_L2X0_ONLINE.

THanks,
Mark.

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Thu, Aug 31, 2017 at 11:31:20AM +0100, Mark Rutland wrote:
> On Thu, Aug 31, 2017 at 11:57:46AM +0200, Jan Glauber wrote:
> > On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> > > On 2017/8/29 21:12, Jan Glauber wrote:
> > > > Add support for the PMU counters on Cavium SOC memory controllers.
> > > > 
> > > > This patch also adds generic functions to allow supporting more
> > > > devices with PMU counters.
> 
> > > > +/* generic struct to cover the different pmu types */
> > > > +struct cvm_pmu_dev {
> > > > +	struct pmu pmu;
> > > > +	const char *pmu_name;
> > > 
> > > It seems that pmu_name is redundant since struct pmu has a name field,
> > > Mark has mentioned it in HiSilicon uncore PMU driver, Link:
> > > https://patchwork.kernel.org/patch/9861821/
> > 
> > I don't get it. perf_pmu_register() just copies the char* from the
> > argument into pmu->name. Somewhere the string must be allocated.
> > That's why I have cvm_pmu_dev->pmu_name.
> 
> I'm not sure I follow. cvm_pmu_dev->pmu_name is just a char *, so what
> does that have to do with allocation?

As you pointed out here:
https://lkml.org/lkml/2017/6/2/530
perf_pmu_register does not copy the string, so _somewhere_ the name must
be allocated and freed afterwards. Are you suggesting to use pmu.name
directly to allocate the name there and pass
perf_register_pmu(..., tlk->pmu.name, ...)?

--Jan

> ... unless you mean you want to allocate this in some variant-specific
> code prior to passing it to code which calls perf_pmu_register(), and
> you just need a place to stash it in the mean time?



[...]

> THanks,
> Mark.

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Thu, Aug 31, 2017 at 11:31:20AM +0100, Mark Rutland wrote:
> On Thu, Aug 31, 2017 at 11:57:46AM +0200, Jan Glauber wrote:
> > On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> > > On 2017/8/29 21:12, Jan Glauber wrote:
> > > > Add support for the PMU counters on Cavium SOC memory controllers.
> > > > 
> > > > This patch also adds generic functions to allow supporting more
> > > > devices with PMU counters.
> 
> > > > +/* generic struct to cover the different pmu types */
> > > > +struct cvm_pmu_dev {
> > > > +	struct pmu pmu;
> > > > +	const char *pmu_name;
> > > 
> > > It seems that pmu_name is redundant since struct pmu has a name field,
> > > Mark has mentioned it in HiSilicon uncore PMU driver, Link:
> > > https://patchwork.kernel.org/patch/9861821/
> > 
> > I don't get it. perf_pmu_register() just copies the char* from the
> > argument into pmu->name. Somewhere the string must be allocated.
> > That's why I have cvm_pmu_dev->pmu_name.
> 
> I'm not sure I follow. cvm_pmu_dev->pmu_name is just a char *, so what
> does that have to do with allocation?

As you pointed out here:
https://lkml.org/lkml/2017/6/2/530
perf_pmu_register does not copy the string, so _somewhere_ the name must
be allocated and freed afterwards. Are you suggesting to use pmu.name
directly to allocate the name there and pass
perf_register_pmu(..., tlk->pmu.name, ...)?

--Jan

> ... unless you mean you want to allocate this in some variant-specific
> code prior to passing it to code which calls perf_pmu_register(), and
> you just need a place to stash it in the mean time?



[...]

> THanks,
> Mark.

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Thu, Aug 31, 2017 at 11:31:20AM +0100, Mark Rutland wrote:
> On Thu, Aug 31, 2017 at 11:57:46AM +0200, Jan Glauber wrote:
> > On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> > > On 2017/8/29 21:12, Jan Glauber wrote:

[...]

> > > > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > > > index 82b30e6..ca84ac8 100644
> > > > --- a/include/linux/cpuhotplug.h
> > > > +++ b/include/linux/cpuhotplug.h
> > > > @@ -139,6 +139,7 @@ enum cpuhp_state {
> > > >  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> > > >  	CPUHP_AP_WORKQUEUE_ONLINE,
> > > >  	CPUHP_AP_RCUTREE_ONLINE,
> > > > +	CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > > 
> > > Alphabetic order?
> > 
> > These don't look alphabetically ordered to me.
> 
> Sure, the full list is ordered by dependency.
> 
> However, we've generally kept the uncore PMUs together, and within the
> group of system PMU CPUHP_AP_PERF_ARM_* callbacks, we've retained
> alphabetical order.
> 
> Does this PMU need workqueues and RCU up before its HP callback is
> invoked? Or can this be moved into the group of CPUHP_AP_PERF_ARM_*
> above CPUHP_AP_WORKQUEUE_ONLINE and CPUHP_AP_RCUTREE_ONLINE? i.e.
> between CPUHP_AP_PERF_ARM_CCN_ONLINE and CPUHP_AP_PERF_ARM_L2X0_ONLINE.

I think I can move it inside the CPUHP_AP_PERF_ARM_* group.

--Jan

> THanks,
> Mark.

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Thu, Aug 31, 2017 at 11:31:20AM +0100, Mark Rutland wrote:
> On Thu, Aug 31, 2017 at 11:57:46AM +0200, Jan Glauber wrote:
> > On Wed, Aug 30, 2017 at 10:54:03AM +0800, Zhangshaokun wrote:
> > > On 2017/8/29 21:12, Jan Glauber wrote:

[...]

> > > > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > > > index 82b30e6..ca84ac8 100644
> > > > --- a/include/linux/cpuhotplug.h
> > > > +++ b/include/linux/cpuhotplug.h
> > > > @@ -139,6 +139,7 @@ enum cpuhp_state {
> > > >  	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> > > >  	CPUHP_AP_WORKQUEUE_ONLINE,
> > > >  	CPUHP_AP_RCUTREE_ONLINE,
> > > > +	CPUHP_AP_PERF_ARM_CVM_ONLINE,
> > > 
> > > Alphabetic order?
> > 
> > These don't look alphabetically ordered to me.
> 
> Sure, the full list is ordered by dependency.
> 
> However, we've generally kept the uncore PMUs together, and within the
> group of system PMU CPUHP_AP_PERF_ARM_* callbacks, we've retained
> alphabetical order.
> 
> Does this PMU need workqueues and RCU up before its HP callback is
> invoked? Or can this be moved into the group of CPUHP_AP_PERF_ARM_*
> above CPUHP_AP_WORKQUEUE_ONLINE and CPUHP_AP_RCUTREE_ONLINE? i.e.
> between CPUHP_AP_PERF_ARM_CCN_ONLINE and CPUHP_AP_PERF_ARM_L2X0_ONLINE.

I think I can move it inside the CPUHP_AP_PERF_ARM_* group.

--Jan

> THanks,
> Mark.

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Suzuki K Poulose]

On 29/08/17 14:12, Jan Glauber wrote:
> Add support for the PMU counters on Cavium SOC memory controllers.
>
> This patch also adds generic functions to allow supporting more
> devices with PMU counters.
>
> Properties of the LMC PMU counters:
> - not stoppable
> - fixed purpose
> - read-only
> - one PCI device per memory controller
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>

Jan,

Some minor comments below.

> +static void cvm_pmu_del(struct perf_event *event, int flags)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	int i;
> +
> +	event->pmu->stop(event, PERF_EF_UPDATE);
> +
> +	/*
> +	 * For programmable counters we need to check where we installed it.
> +	 * To keep this function generic always test the more complicated
> +	 * case (free running counters won't need the loop).
> +	 */
> +	for (i = 0; i < pmu_dev->num_counters; i++)
> +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> +			break;

Does this mean, it is the only way to map any given event (for programmable counters)
to a hardware counter ? What do we store in hwc->idx ? We have 2 additional
struct hw_perf_event_extra fields. We should be able to use one field to map it
back to the counter, isn't it ?

> +
> +	perf_event_update_userpage(event);
> +	hwc->idx = -1;
> +}
> +

...

> +/* LMC events */
> +#define LMC_EVENT_IFB_CNT		0x1d0
> +#define LMC_EVENT_OPS_CNT		0x1d8
> +#define LMC_EVENT_DCLK_CNT		0x1e0
> +#define LMC_EVENT_BANK_CONFLICT1	0x360
> +#define LMC_EVENT_BANK_CONFLICT2	0x368
> +
> +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> +	&((struct perf_pmu_events_attr[]) {						\
> +		{									\
> +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> +			_id,								\
> +			"lmc_event=" __stringify(_id),					\
> +		}									\
> +	})[0].attr.attr
> +
> +/* map counter numbers to register offsets */
> +static int lmc_events[] = {
> +	LMC_EVENT_IFB_CNT,
> +	LMC_EVENT_OPS_CNT,
> +	LMC_EVENT_DCLK_CNT,
> +	LMC_EVENT_BANK_CONFLICT1,
> +	LMC_EVENT_BANK_CONFLICT2,
> +};
> +
> +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> +			   lmc_events[hwc->config]);
> +}
> +

Is there any reason why we can't use the LMC event code directly
here, avoiding the mapping altogether ?

> +PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> +
> +static struct attribute *cvm_pmu_lmc_format_attr[] = {
> +	&format_attr_lmc_event.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_format_group = {
> +	.name = "format",
> +	.attrs = cvm_pmu_lmc_format_attr,
> +};
> +
> +static struct attribute *cvm_pmu_lmc_events_attr[] = {
> +	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> +	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> +	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_events_group = {
> +	.name = "events",
> +	.attrs = cvm_pmu_lmc_events_attr,
> +};
> +
> +static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
> +	&cvm_pmu_attr_group,
> +	&cvm_pmu_lmc_format_group,
> +	&cvm_pmu_lmc_events_group,
> +	NULL,
> +};
> +
> +static bool cvm_pmu_lmc_event_valid(u64 config)
> +{
> +	return (config < ARRAY_SIZE(lmc_events));
> +}
> +
> +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct cvm_pmu_dev *next, *lmc;
> +	int nr = 0, ret = -ENOMEM;
> +
> +	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);
> +	if (!lmc)
> +		return -ENOMEM;

As Shaokun mentioned already, we could use devm_kzalloc() to avoid having to
cleanup the memory explicitly upon error and during module unload.

> +
> +	lmc->map = ioremap(pci_resource_start(pdev, 0),
> +			   pci_resource_len(pdev, 0));

Use devm_ioremap here. See below.

> +	if (!lmc->map)
> +		goto fail_ioremap;
> +
> +	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
> +		nr++;
> +	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);

Again, you could remove the field and use devm_kasprintf() into a local
variable for using it with pmu_register. It would be automatically free'd
upon error or device removal.

> +	if (!lmc->pmu_name)
> +		goto fail_kasprintf;
> +
> +	lmc->pdev = pdev;
> +	lmc->num_counters = ARRAY_SIZE(lmc_events);
> +	lmc->pmu = (struct pmu) {
> +		.task_ctx_nr    = perf_invalid_context,
> +		.event_init	= cvm_pmu_event_init,
> +		.add		= cvm_pmu_lmc_add,
> +		.del		= cvm_pmu_del,
> +		.start		= cvm_pmu_start,
> +		.stop		= cvm_pmu_stop,
> +		.read		= cvm_pmu_read,
> +		.attr_groups	= cvm_pmu_lmc_attr_groups,
> +	};
> +
> +	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +					 &lmc->cpuhp_node);
> +
> +	/*
> +	 * perf PMU is CPU dependent so pick a random CPU and migrate away
> +	 * if it goes offline.
> +	 */
> +	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
> +
> +	list_add(&lmc->entry, &cvm_pmu_lmcs);
> +	lmc->event_valid = cvm_pmu_lmc_event_valid;
> +
> +	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
> +	if (ret)
> +		goto fail_pmu;
> +
> +	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
> +		 lmc->pmu_name, lmc->num_counters);
> +	return 0;
> +
> +fail_pmu:
> +	kfree(lmc->pmu_name);
> +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +				    &lmc->cpuhp_node);
> +fail_kasprintf:
> +	iounmap(lmc->map);
> +fail_ioremap:
> +	kfree(lmc);

As mentioned above, if you switch to devm_* versions, you could get rid of this
kfrees and iounmap.

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Suzuki K Poulose]

On 29/08/17 14:12, Jan Glauber wrote:
> Add support for the PMU counters on Cavium SOC memory controllers.
>
> This patch also adds generic functions to allow supporting more
> devices with PMU counters.
>
> Properties of the LMC PMU counters:
> - not stoppable
> - fixed purpose
> - read-only
> - one PCI device per memory controller
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>

Jan,

Some minor comments below.

> +static void cvm_pmu_del(struct perf_event *event, int flags)
> +{
> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	int i;
> +
> +	event->pmu->stop(event, PERF_EF_UPDATE);
> +
> +	/*
> +	 * For programmable counters we need to check where we installed it.
> +	 * To keep this function generic always test the more complicated
> +	 * case (free running counters won't need the loop).
> +	 */
> +	for (i = 0; i < pmu_dev->num_counters; i++)
> +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> +			break;

Does this mean, it is the only way to map any given event (for programmable counters)
to a hardware counter ? What do we store in hwc->idx ? We have 2 additional
struct hw_perf_event_extra fields. We should be able to use one field to map it
back to the counter, isn't it ?

> +
> +	perf_event_update_userpage(event);
> +	hwc->idx = -1;
> +}
> +

...

> +/* LMC events */
> +#define LMC_EVENT_IFB_CNT		0x1d0
> +#define LMC_EVENT_OPS_CNT		0x1d8
> +#define LMC_EVENT_DCLK_CNT		0x1e0
> +#define LMC_EVENT_BANK_CONFLICT1	0x360
> +#define LMC_EVENT_BANK_CONFLICT2	0x368
> +
> +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> +	&((struct perf_pmu_events_attr[]) {						\
> +		{									\
> +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> +			_id,								\
> +			"lmc_event=" __stringify(_id),					\
> +		}									\
> +	})[0].attr.attr
> +
> +/* map counter numbers to register offsets */
> +static int lmc_events[] = {
> +	LMC_EVENT_IFB_CNT,
> +	LMC_EVENT_OPS_CNT,
> +	LMC_EVENT_DCLK_CNT,
> +	LMC_EVENT_BANK_CONFLICT1,
> +	LMC_EVENT_BANK_CONFLICT2,
> +};
> +
> +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> +			   lmc_events[hwc->config]);
> +}
> +

Is there any reason why we can't use the LMC event code directly
here, avoiding the mapping altogether ?

> +PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> +
> +static struct attribute *cvm_pmu_lmc_format_attr[] = {
> +	&format_attr_lmc_event.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_format_group = {
> +	.name = "format",
> +	.attrs = cvm_pmu_lmc_format_attr,
> +};
> +
> +static struct attribute *cvm_pmu_lmc_events_attr[] = {
> +	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> +	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> +	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> +	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> +	NULL,
> +};
> +
> +static struct attribute_group cvm_pmu_lmc_events_group = {
> +	.name = "events",
> +	.attrs = cvm_pmu_lmc_events_attr,
> +};
> +
> +static const struct attribute_group *cvm_pmu_lmc_attr_groups[] = {
> +	&cvm_pmu_attr_group,
> +	&cvm_pmu_lmc_format_group,
> +	&cvm_pmu_lmc_events_group,
> +	NULL,
> +};
> +
> +static bool cvm_pmu_lmc_event_valid(u64 config)
> +{
> +	return (config < ARRAY_SIZE(lmc_events));
> +}
> +
> +int cvm_lmc_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct cvm_pmu_dev *next, *lmc;
> +	int nr = 0, ret = -ENOMEM;
> +
> +	lmc = kzalloc(sizeof(*lmc), GFP_KERNEL);
> +	if (!lmc)
> +		return -ENOMEM;

As Shaokun mentioned already, we could use devm_kzalloc() to avoid having to
cleanup the memory explicitly upon error and during module unload.

> +
> +	lmc->map = ioremap(pci_resource_start(pdev, 0),
> +			   pci_resource_len(pdev, 0));

Use devm_ioremap here. See below.

> +	if (!lmc->map)
> +		goto fail_ioremap;
> +
> +	list_for_each_entry(next, &cvm_pmu_lmcs, entry)
> +		nr++;
> +	lmc->pmu_name = kasprintf(GFP_KERNEL, "lmc%d", nr);

Again, you could remove the field and use devm_kasprintf() into a local
variable for using it with pmu_register. It would be automatically free'd
upon error or device removal.

> +	if (!lmc->pmu_name)
> +		goto fail_kasprintf;
> +
> +	lmc->pdev = pdev;
> +	lmc->num_counters = ARRAY_SIZE(lmc_events);
> +	lmc->pmu = (struct pmu) {
> +		.task_ctx_nr    = perf_invalid_context,
> +		.event_init	= cvm_pmu_event_init,
> +		.add		= cvm_pmu_lmc_add,
> +		.del		= cvm_pmu_del,
> +		.start		= cvm_pmu_start,
> +		.stop		= cvm_pmu_stop,
> +		.read		= cvm_pmu_read,
> +		.attr_groups	= cvm_pmu_lmc_attr_groups,
> +	};
> +
> +	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +					 &lmc->cpuhp_node);
> +
> +	/*
> +	 * perf PMU is CPU dependent so pick a random CPU and migrate away
> +	 * if it goes offline.
> +	 */
> +	cpumask_set_cpu(smp_processor_id(), &lmc->active_mask);
> +
> +	list_add(&lmc->entry, &cvm_pmu_lmcs);
> +	lmc->event_valid = cvm_pmu_lmc_event_valid;
> +
> +	ret = perf_pmu_register(&lmc->pmu, lmc->pmu_name, -1);
> +	if (ret)
> +		goto fail_pmu;
> +
> +	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
> +		 lmc->pmu_name, lmc->num_counters);
> +	return 0;
> +
> +fail_pmu:
> +	kfree(lmc->pmu_name);
> +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
> +				    &lmc->cpuhp_node);
> +fail_kasprintf:
> +	iounmap(lmc->map);
> +fail_ioremap:
> +	kfree(lmc);

As mentioned above, if you switch to devm_* versions, you could get rid of this
kfrees and iounmap.

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:
> On 29/08/17 14:12, Jan Glauber wrote:
> >Add support for the PMU counters on Cavium SOC memory controllers.
> >
> >This patch also adds generic functions to allow supporting more
> >devices with PMU counters.
> >
> >Properties of the LMC PMU counters:
> >- not stoppable
> >- fixed purpose
> >- read-only
> >- one PCI device per memory controller
> >
> >Signed-off-by: Jan Glauber <jglauber@cavium.com>
> 
> Jan,
> 
> Some minor comments below.
> 
> >+static void cvm_pmu_del(struct perf_event *event, int flags)
> >+{
> >+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> >+	struct hw_perf_event *hwc = &event->hw;
> >+	int i;
> >+
> >+	event->pmu->stop(event, PERF_EF_UPDATE);
> >+
> >+	/*
> >+	 * For programmable counters we need to check where we installed it.
> >+	 * To keep this function generic always test the more complicated
> >+	 * case (free running counters won't need the loop).
> >+	 */
> >+	for (i = 0; i < pmu_dev->num_counters; i++)
> >+		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> >+			break;
> 
> Does this mean, it is the only way to map any given event (for programmable counters)
> to a hardware counter ? What do we store in hwc->idx ? We have 2 additional
> struct hw_perf_event_extra fields. We should be able to use one field to map it
> back to the counter, isn't it ?

Hmm, I might be able to use hwc-idx directly instead of the loop, will
check that.

> >+
> >+	perf_event_update_userpage(event);
> >+	hwc->idx = -1;
> >+}
> >+
> 
> ...
> 
> >+/* LMC events */
> >+#define LMC_EVENT_IFB_CNT		0x1d0
> >+#define LMC_EVENT_OPS_CNT		0x1d8
> >+#define LMC_EVENT_DCLK_CNT		0x1e0
> >+#define LMC_EVENT_BANK_CONFLICT1	0x360
> >+#define LMC_EVENT_BANK_CONFLICT2	0x368
> >+
> >+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> >+	&((struct perf_pmu_events_attr[]) {						\
> >+		{									\
> >+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> >+			_id,								\
> >+			"lmc_event=" __stringify(_id),					\
> >+		}									\
> >+	})[0].attr.attr
> >+
> >+/* map counter numbers to register offsets */
> >+static int lmc_events[] = {
> >+	LMC_EVENT_IFB_CNT,
> >+	LMC_EVENT_OPS_CNT,
> >+	LMC_EVENT_DCLK_CNT,
> >+	LMC_EVENT_BANK_CONFLICT1,
> >+	LMC_EVENT_BANK_CONFLICT2,
> >+};
> >+
> >+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> >+{
> >+	struct hw_perf_event *hwc = &event->hw;
> >+
> >+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> >+			   lmc_events[hwc->config]);
> >+}
> >+
> 
> Is there any reason why we can't use the LMC event code directly
> here, avoiding the mapping altogether ?

I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.

thanks,
Jan

> >+PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> >+
> >+static struct attribute *cvm_pmu_lmc_format_attr[] = {
> >+	&format_attr_lmc_event.attr,
> >+	NULL,
> >+};
> >+
> >+static struct attribute_group cvm_pmu_lmc_format_group = {
> >+	.name = "format",
> >+	.attrs = cvm_pmu_lmc_format_attr,
> >+};
> >+
> >+static struct attribute *cvm_pmu_lmc_events_attr[] = {
> >+	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> >+	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> >+	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> >+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> >+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> >+	NULL,
> >+};

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:
> On 29/08/17 14:12, Jan Glauber wrote:
> >Add support for the PMU counters on Cavium SOC memory controllers.
> >
> >This patch also adds generic functions to allow supporting more
> >devices with PMU counters.
> >
> >Properties of the LMC PMU counters:
> >- not stoppable
> >- fixed purpose
> >- read-only
> >- one PCI device per memory controller
> >
> >Signed-off-by: Jan Glauber <jglauber@cavium.com>
> 
> Jan,
> 
> Some minor comments below.
> 
> >+static void cvm_pmu_del(struct perf_event *event, int flags)
> >+{
> >+	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
> >+	struct hw_perf_event *hwc = &event->hw;
> >+	int i;
> >+
> >+	event->pmu->stop(event, PERF_EF_UPDATE);
> >+
> >+	/*
> >+	 * For programmable counters we need to check where we installed it.
> >+	 * To keep this function generic always test the more complicated
> >+	 * case (free running counters won't need the loop).
> >+	 */
> >+	for (i = 0; i < pmu_dev->num_counters; i++)
> >+		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
> >+			break;
> 
> Does this mean, it is the only way to map any given event (for programmable counters)
> to a hardware counter ? What do we store in hwc->idx ? We have 2 additional
> struct hw_perf_event_extra fields. We should be able to use one field to map it
> back to the counter, isn't it ?

Hmm, I might be able to use hwc-idx directly instead of the loop, will
check that.

> >+
> >+	perf_event_update_userpage(event);
> >+	hwc->idx = -1;
> >+}
> >+
> 
> ...
> 
> >+/* LMC events */
> >+#define LMC_EVENT_IFB_CNT		0x1d0
> >+#define LMC_EVENT_OPS_CNT		0x1d8
> >+#define LMC_EVENT_DCLK_CNT		0x1e0
> >+#define LMC_EVENT_BANK_CONFLICT1	0x360
> >+#define LMC_EVENT_BANK_CONFLICT2	0x368
> >+
> >+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> >+	&((struct perf_pmu_events_attr[]) {						\
> >+		{									\
> >+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> >+			_id,								\
> >+			"lmc_event=" __stringify(_id),					\
> >+		}									\
> >+	})[0].attr.attr
> >+
> >+/* map counter numbers to register offsets */
> >+static int lmc_events[] = {
> >+	LMC_EVENT_IFB_CNT,
> >+	LMC_EVENT_OPS_CNT,
> >+	LMC_EVENT_DCLK_CNT,
> >+	LMC_EVENT_BANK_CONFLICT1,
> >+	LMC_EVENT_BANK_CONFLICT2,
> >+};
> >+
> >+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> >+{
> >+	struct hw_perf_event *hwc = &event->hw;
> >+
> >+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> >+			   lmc_events[hwc->config]);
> >+}
> >+
> 
> Is there any reason why we can't use the LMC event code directly
> here, avoiding the mapping altogether ?

I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.

thanks,
Jan

> >+PMU_FORMAT_ATTR(lmc_event, "config:0-2");
> >+
> >+static struct attribute *cvm_pmu_lmc_format_attr[] = {
> >+	&format_attr_lmc_event.attr,
> >+	NULL,
> >+};
> >+
> >+static struct attribute_group cvm_pmu_lmc_format_group = {
> >+	.name = "format",
> >+	.attrs = cvm_pmu_lmc_format_attr,
> >+};
> >+
> >+static struct attribute *cvm_pmu_lmc_events_attr[] = {
> >+	CVM_PMU_LMC_EVENT_ATTR(ifb_cnt,		0),
> >+	CVM_PMU_LMC_EVENT_ATTR(ops_cnt,		1),
> >+	CVM_PMU_LMC_EVENT_ATTR(dclk_cnt,	2),
> >+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict1,	3),
> >+	CVM_PMU_LMC_EVENT_ATTR(bank_conflict2,	4),
> >+	NULL,
> >+};

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Suzuki K Poulose]

On 31/08/17 12:35, Jan Glauber wrote:
> On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:
>> On 29/08/17 14:12, Jan Glauber wrote:
>>> Add support for the PMU counters on Cavium SOC memory controllers.
>>>
>>> This patch also adds generic functions to allow supporting more
>>> devices with PMU counters.
>>>
>>> Properties of the LMC PMU counters:
>>> - not stoppable
>>> - fixed purpose
>>> - read-only
>>> - one PCI device per memory controller
>>>
>>> Signed-off-by: Jan Glauber <jglauber@cavium.com>
>>
>> Jan,
>>
>> Some minor comments below.
>>
>>> +static void cvm_pmu_del(struct perf_event *event, int flags)
>>> +{
>>> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
>>> +	struct hw_perf_event *hwc = &event->hw;
>>> +	int i;
>>> +
>>> +	event->pmu->stop(event, PERF_EF_UPDATE);
>>> +
>>> +	/*
>>> +	 * For programmable counters we need to check where we installed it.
>>> +	 * To keep this function generic always test the more complicated
>>> +	 * case (free running counters won't need the loop).
>>> +	 */
>>> +	for (i = 0; i < pmu_dev->num_counters; i++)
>>> +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
>>> +			break;
>>
>> Does this mean, it is the only way to map any given event (for programmable counters)
>> to a hardware counter ? What do we store in hwc->idx ? We have 2 additional
>> struct hw_perf_event_extra fields. We should be able to use one field to map it
>> back to the counter, isn't it ?
>
> Hmm, I might be able to use hwc-idx directly instead of the loop, will
> check that.
>
>>> +
>>> +	perf_event_update_userpage(event);
>>> +	hwc->idx = -1;
>>> +}
>>> +
>>
>> ...
>>
>>> +/* LMC events */
>>> +#define LMC_EVENT_IFB_CNT		0x1d0
>>> +#define LMC_EVENT_OPS_CNT		0x1d8
>>> +#define LMC_EVENT_DCLK_CNT		0x1e0
>>> +#define LMC_EVENT_BANK_CONFLICT1	0x360
>>> +#define LMC_EVENT_BANK_CONFLICT2	0x368
>>> +
>>> +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
>>> +	&((struct perf_pmu_events_attr[]) {						\
>>> +		{									\
>>> +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
>>> +			_id,								\
>>> +			"lmc_event=" __stringify(_id),					\
>>> +		}									\
>>> +	})[0].attr.attr
>>> +
>>> +/* map counter numbers to register offsets */
>>> +static int lmc_events[] = {
>>> +	LMC_EVENT_IFB_CNT,
>>> +	LMC_EVENT_OPS_CNT,
>>> +	LMC_EVENT_DCLK_CNT,
>>> +	LMC_EVENT_BANK_CONFLICT1,
>>> +	LMC_EVENT_BANK_CONFLICT2,
>>> +};
>>> +
>>> +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
>>> +{
>>> +	struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
>>> +			   lmc_events[hwc->config]);
>>> +}
>>> +
>>
>> Is there any reason why we can't use the LMC event code directly
>> here, avoiding the mapping altogether ?
>
> I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.

Thats the primarily the reason why we expose the "aliases" in events/.
The other problem with adding another layer of mapping is, you are preventing
someone from actually mapping the raw code used by the perf tool (which is now
a mapping index) to the real raw code used by the hardware unless they have
the kernel source handy. If you choose to expose the raw numbers, like *all*
the other PMUs, the user can map it by looking up the manual.

Cheers
Suzuki

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Suzuki K Poulose]

On 31/08/17 12:35, Jan Glauber wrote:
> On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:
>> On 29/08/17 14:12, Jan Glauber wrote:
>>> Add support for the PMU counters on Cavium SOC memory controllers.
>>>
>>> This patch also adds generic functions to allow supporting more
>>> devices with PMU counters.
>>>
>>> Properties of the LMC PMU counters:
>>> - not stoppable
>>> - fixed purpose
>>> - read-only
>>> - one PCI device per memory controller
>>>
>>> Signed-off-by: Jan Glauber <jglauber@cavium.com>
>>
>> Jan,
>>
>> Some minor comments below.
>>
>>> +static void cvm_pmu_del(struct perf_event *event, int flags)
>>> +{
>>> +	struct cvm_pmu_dev *pmu_dev = to_pmu_dev(event->pmu);
>>> +	struct hw_perf_event *hwc = &event->hw;
>>> +	int i;
>>> +
>>> +	event->pmu->stop(event, PERF_EF_UPDATE);
>>> +
>>> +	/*
>>> +	 * For programmable counters we need to check where we installed it.
>>> +	 * To keep this function generic always test the more complicated
>>> +	 * case (free running counters won't need the loop).
>>> +	 */
>>> +	for (i = 0; i < pmu_dev->num_counters; i++)
>>> +		if (cmpxchg(&pmu_dev->events[i], event, NULL) == event)
>>> +			break;
>>
>> Does this mean, it is the only way to map any given event (for programmable counters)
>> to a hardware counter ? What do we store in hwc->idx ? We have 2 additional
>> struct hw_perf_event_extra fields. We should be able to use one field to map it
>> back to the counter, isn't it ?
>
> Hmm, I might be able to use hwc-idx directly instead of the loop, will
> check that.
>
>>> +
>>> +	perf_event_update_userpage(event);
>>> +	hwc->idx = -1;
>>> +}
>>> +
>>
>> ...
>>
>>> +/* LMC events */
>>> +#define LMC_EVENT_IFB_CNT		0x1d0
>>> +#define LMC_EVENT_OPS_CNT		0x1d8
>>> +#define LMC_EVENT_DCLK_CNT		0x1e0
>>> +#define LMC_EVENT_BANK_CONFLICT1	0x360
>>> +#define LMC_EVENT_BANK_CONFLICT2	0x368
>>> +
>>> +#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
>>> +	&((struct perf_pmu_events_attr[]) {						\
>>> +		{									\
>>> +			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
>>> +			_id,								\
>>> +			"lmc_event=" __stringify(_id),					\
>>> +		}									\
>>> +	})[0].attr.attr
>>> +
>>> +/* map counter numbers to register offsets */
>>> +static int lmc_events[] = {
>>> +	LMC_EVENT_IFB_CNT,
>>> +	LMC_EVENT_OPS_CNT,
>>> +	LMC_EVENT_DCLK_CNT,
>>> +	LMC_EVENT_BANK_CONFLICT1,
>>> +	LMC_EVENT_BANK_CONFLICT2,
>>> +};
>>> +
>>> +static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
>>> +{
>>> +	struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
>>> +			   lmc_events[hwc->config]);
>>> +}
>>> +
>>
>> Is there any reason why we can't use the LMC event code directly
>> here, avoiding the mapping altogether ?
>
> I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.

Thats the primarily the reason why we expose the "aliases" in events/.
The other problem with adding another layer of mapping is, you are preventing
someone from actually mapping the raw code used by the perf tool (which is now
a mapping index) to the real raw code used by the hardware unless they have
the kernel source handy. If you choose to expose the raw numbers, like *all*
the other PMUs, the user can map it by looking up the manual.

Cheers
Suzuki

Re: [RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Thu, Aug 31, 2017 at 02:26:22PM +0100, Suzuki K Poulose wrote:
> On 31/08/17 12:35, Jan Glauber wrote:
> >On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:
> >>On 29/08/17 14:12, Jan Glauber wrote:

[...]

> >>>+/* LMC events */
> >>>+#define LMC_EVENT_IFB_CNT		0x1d0
> >>>+#define LMC_EVENT_OPS_CNT		0x1d8
> >>>+#define LMC_EVENT_DCLK_CNT		0x1e0
> >>>+#define LMC_EVENT_BANK_CONFLICT1	0x360
> >>>+#define LMC_EVENT_BANK_CONFLICT2	0x368
> >>>+
> >>>+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> >>>+	&((struct perf_pmu_events_attr[]) {						\
> >>>+		{									\
> >>>+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> >>>+			_id,								\
> >>>+			"lmc_event=" __stringify(_id),					\
> >>>+		}									\
> >>>+	})[0].attr.attr
> >>>+
> >>>+/* map counter numbers to register offsets */
> >>>+static int lmc_events[] = {
> >>>+	LMC_EVENT_IFB_CNT,
> >>>+	LMC_EVENT_OPS_CNT,
> >>>+	LMC_EVENT_DCLK_CNT,
> >>>+	LMC_EVENT_BANK_CONFLICT1,
> >>>+	LMC_EVENT_BANK_CONFLICT2,
> >>>+};
> >>>+
> >>>+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> >>>+{
> >>>+	struct hw_perf_event *hwc = &event->hw;
> >>>+
> >>>+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> >>>+			   lmc_events[hwc->config]);
> >>>+}
> >>>+
> >>
> >>Is there any reason why we can't use the LMC event code directly
> >>here, avoiding the mapping altogether ?
> >
> >I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.
> 
> Thats the primarily the reason why we expose the "aliases" in events/.
> The other problem with adding another layer of mapping is, you are preventing
> someone from actually mapping the raw code used by the perf tool (which is now
> a mapping index) to the real raw code used by the hardware unless they have
> the kernel source handy. If you choose to expose the raw numbers, like *all*
> the other PMUs, the user can map it by looking up the manual.

So what would that do to the config bits? Currently they are:
PMU_FORMAT_ATTR(lmc_event, "config:0-2");

Should I have config:0-9 then? Wouldn't that be confusing as there are
only 5 events?

Also I need to be very careful as we need to prevent a user from
accessing anything else then the counters. I can do that with the
event_valid callback though.

thanks,
Jan

> Cheers
> Suzuki

[RFC PATCH v9 5/7] perf: cavium: Support memory controller PMU counters

[Jan Glauber]

On Thu, Aug 31, 2017 at 02:26:22PM +0100, Suzuki K Poulose wrote:
> On 31/08/17 12:35, Jan Glauber wrote:
> >On Wed, Aug 30, 2017 at 11:03:00AM +0100, Suzuki K Poulose wrote:
> >>On 29/08/17 14:12, Jan Glauber wrote:

[...]

> >>>+/* LMC events */
> >>>+#define LMC_EVENT_IFB_CNT		0x1d0
> >>>+#define LMC_EVENT_OPS_CNT		0x1d8
> >>>+#define LMC_EVENT_DCLK_CNT		0x1e0
> >>>+#define LMC_EVENT_BANK_CONFLICT1	0x360
> >>>+#define LMC_EVENT_BANK_CONFLICT2	0x368
> >>>+
> >>>+#define CVM_PMU_LMC_EVENT_ATTR(_name, _id)						\
> >>>+	&((struct perf_pmu_events_attr[]) {						\
> >>>+		{									\
> >>>+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
> >>>+			_id,								\
> >>>+			"lmc_event=" __stringify(_id),					\
> >>>+		}									\
> >>>+	})[0].attr.attr
> >>>+
> >>>+/* map counter numbers to register offsets */
> >>>+static int lmc_events[] = {
> >>>+	LMC_EVENT_IFB_CNT,
> >>>+	LMC_EVENT_OPS_CNT,
> >>>+	LMC_EVENT_DCLK_CNT,
> >>>+	LMC_EVENT_BANK_CONFLICT1,
> >>>+	LMC_EVENT_BANK_CONFLICT2,
> >>>+};
> >>>+
> >>>+static int cvm_pmu_lmc_add(struct perf_event *event, int flags)
> >>>+{
> >>>+	struct hw_perf_event *hwc = &event->hw;
> >>>+
> >>>+	return cvm_pmu_add(event, flags, LMC_CONFIG_OFFSET,
> >>>+			   lmc_events[hwc->config]);
> >>>+}
> >>>+
> >>
> >>Is there any reason why we can't use the LMC event code directly
> >>here, avoiding the mapping altogether ?
> >
> >I wanted to avoid exposing the raw numbers (0x1d0 - 0x368) here.
> 
> Thats the primarily the reason why we expose the "aliases" in events/.
> The other problem with adding another layer of mapping is, you are preventing
> someone from actually mapping the raw code used by the perf tool (which is now
> a mapping index) to the real raw code used by the hardware unless they have
> the kernel source handy. If you choose to expose the raw numbers, like *all*
> the other PMUs, the user can map it by looking up the manual.

So what would that do to the config bits? Currently they are:
PMU_FORMAT_ATTR(lmc_event, "config:0-2");

Should I have config:0-9 then? Wouldn't that be confusing as there are
only 5 events?

Also I need to be very careful as we need to prevent a user from
accessing anything else then the counters. I can do that with the
event_valid callback though.

thanks,
Jan

> Cheers
> Suzuki

[RFC PATCH v9 6/7] perf: cavium: Support transmit-link PMU counters

[Jan Glauber]

Add support for the transmit-link (OCX TLK) PMU counters found
on Caviums SOCs with a processor interconnect.

Properties of the OCX TLK counters:
- per-unit control
- fixed purpose
- writable
- one PCI device with multiple TLK units

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/perf/Kconfig            |   7 ++
 drivers/perf/cavium_pmu.c       | 235 ++++++++++++++++++++++++++++++++++++++++
 drivers/soc/cavium/Kconfig      |   4 +
 drivers/soc/cavium/cavium_ocx.c |   4 +
 include/linux/soc/cavium/ocx.h  |   3 +
 5 files changed, 253 insertions(+)

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index a787562..efb2ace 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -51,4 +51,11 @@ config CAVIUM_PMU_LMC
 	  Provides PMU counters for the memory controller on
 	  Cavium ThunderX or OcteonTX SOCs.
 
+config CAVIUM_PMU_OCX_TLK
+	tristate "Cavium ThunderX interconnect PMU"
+	depends on ARCH_THUNDER && m
+	select CAVIUM_OCX
+	help
+	  Provides PMU counters for the processor interconnect on
+	  Cavium ThunderX processors.
 endmenu
diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
index bcdedaa..cba8266 100644
--- a/drivers/perf/cavium_pmu.c
+++ b/drivers/perf/cavium_pmu.c
@@ -20,9 +20,11 @@
 #include <linux/perf_event.h>
 #include <linux/slab.h>
 #include <linux/soc/cavium/lmc.h>
+#include <linux/soc/cavium/ocx.h>
 
 enum cvm_pmu_type {
 	CVM_PMU_LMC,
+	CVM_PMU_TLK,
 };
 
 /* maximum number of parallel hardware counters for all pmu types */
@@ -422,6 +424,239 @@ void cvm_lmc_pmu_remove(struct pci_dev *pdev)
 }
 EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
 
+/*
+ * CCPI interface controller (OCX) Transmit link (TLK) counters:
+ * - per-unit control
+ * - writable
+ * - one PCI device with multiple TLK units
+ */
+
+#define TLK_NR_UNITS			3
+#define TLK_UNIT_OFFSET			0x2000
+#define TLK_UNIT_LEN			0x7ff
+#define TLK_START_ADDR			0x10000
+#define TLK_STAT_CTL_OFFSET		0x40
+#define TLK_STAT_OFFSET			0x400
+
+#define TLK_STAT_ENABLE_BIT		BIT(0)
+#define TLK_STAT_RESET_BIT		BIT(1)
+
+#define CVM_PMU_TLK_EVENT_ATTR(_name, _id)						\
+	&((struct perf_pmu_events_attr[]) {						\
+		{									\
+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
+			_id,								\
+			"tlk_event=" __stringify(_id),					\
+		}									\
+	})[0].attr.attr
+
+static void cvm_pmu_tlk_enable_pmu(struct pmu *pmu)
+{
+	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
+
+	/* enable all counters */
+	writeb(TLK_STAT_ENABLE_BIT, pmu_dev->map + TLK_STAT_CTL_OFFSET);
+}
+
+static void cvm_pmu_tlk_disable_pmu(struct pmu *pmu)
+{
+	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
+
+	/* disable all counters */
+	writeb(0, pmu_dev->map + TLK_STAT_CTL_OFFSET);
+}
+
+static int cvm_pmu_tlk_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	return cvm_pmu_add(event, flags, TLK_STAT_CTL_OFFSET,
+			   TLK_STAT_OFFSET + hwc->config * 8);
+}
+
+PMU_FORMAT_ATTR(tlk_event, "config:0-5");
+
+static struct attribute *cvm_pmu_tlk_format_attr[] = {
+	&format_attr_tlk_event.attr,
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_tlk_format_group = {
+	.name = "format",
+	.attrs = cvm_pmu_tlk_format_attr,
+};
+
+static struct attribute *cvm_pmu_tlk_events_attr[] = {
+	CVM_PMU_TLK_EVENT_ATTR(idle_cnt,	0x00),
+	CVM_PMU_TLK_EVENT_ATTR(data_cnt,	0x01),
+	CVM_PMU_TLK_EVENT_ATTR(sync_cnt,	0x02),
+	CVM_PMU_TLK_EVENT_ATTR(retry_cnt,	0x03),
+	CVM_PMU_TLK_EVENT_ATTR(err_cnt,		0x04),
+	CVM_PMU_TLK_EVENT_ATTR(mat0_cnt,	0x08),
+	CVM_PMU_TLK_EVENT_ATTR(mat1_cnt,	0x09),
+	CVM_PMU_TLK_EVENT_ATTR(mat2_cnt,	0x0a),
+	CVM_PMU_TLK_EVENT_ATTR(mat3_cnt,	0x0b),
+	CVM_PMU_TLK_EVENT_ATTR(vc0_cmd,		0x10),
+	CVM_PMU_TLK_EVENT_ATTR(vc1_cmd,		0x11),
+	CVM_PMU_TLK_EVENT_ATTR(vc2_cmd,		0x12),
+	CVM_PMU_TLK_EVENT_ATTR(vc3_cmd,		0x13),
+	CVM_PMU_TLK_EVENT_ATTR(vc4_cmd,		0x14),
+	CVM_PMU_TLK_EVENT_ATTR(vc5_cmd,		0x15),
+	CVM_PMU_TLK_EVENT_ATTR(vc0_pkt,		0x20),
+	CVM_PMU_TLK_EVENT_ATTR(vc1_pkt,		0x21),
+	CVM_PMU_TLK_EVENT_ATTR(vc2_pkt,		0x22),
+	CVM_PMU_TLK_EVENT_ATTR(vc3_pkt,		0x23),
+	CVM_PMU_TLK_EVENT_ATTR(vc4_pkt,		0x24),
+	CVM_PMU_TLK_EVENT_ATTR(vc5_pkt,		0x25),
+	CVM_PMU_TLK_EVENT_ATTR(vc6_pkt,		0x26),
+	CVM_PMU_TLK_EVENT_ATTR(vc7_pkt,		0x27),
+	CVM_PMU_TLK_EVENT_ATTR(vc8_pkt,		0x28),
+	CVM_PMU_TLK_EVENT_ATTR(vc9_pkt,		0x29),
+	CVM_PMU_TLK_EVENT_ATTR(vc10_pkt,	0x2a),
+	CVM_PMU_TLK_EVENT_ATTR(vc11_pkt,	0x2b),
+	CVM_PMU_TLK_EVENT_ATTR(vc12_pkt,	0x2c),
+	CVM_PMU_TLK_EVENT_ATTR(vc13_pkt,	0x2d),
+	CVM_PMU_TLK_EVENT_ATTR(vc0_con,		0x30),
+	CVM_PMU_TLK_EVENT_ATTR(vc1_con,		0x31),
+	CVM_PMU_TLK_EVENT_ATTR(vc2_con,		0x32),
+	CVM_PMU_TLK_EVENT_ATTR(vc3_con,		0x33),
+	CVM_PMU_TLK_EVENT_ATTR(vc4_con,		0x34),
+	CVM_PMU_TLK_EVENT_ATTR(vc5_con,		0x35),
+	CVM_PMU_TLK_EVENT_ATTR(vc6_con,		0x36),
+	CVM_PMU_TLK_EVENT_ATTR(vc7_con,		0x37),
+	CVM_PMU_TLK_EVENT_ATTR(vc8_con,		0x38),
+	CVM_PMU_TLK_EVENT_ATTR(vc9_con,		0x39),
+	CVM_PMU_TLK_EVENT_ATTR(vc10_con,	0x3a),
+	CVM_PMU_TLK_EVENT_ATTR(vc11_con,	0x3b),
+	CVM_PMU_TLK_EVENT_ATTR(vc12_con,	0x3c),
+	CVM_PMU_TLK_EVENT_ATTR(vc13_con,	0x3d),
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_tlk_events_group = {
+	.name = "events",
+	.attrs = cvm_pmu_tlk_events_attr,
+};
+
+static const struct attribute_group *cvm_pmu_tlk_attr_groups[] = {
+	&cvm_pmu_attr_group,
+	&cvm_pmu_tlk_format_group,
+	&cvm_pmu_tlk_events_group,
+	NULL,
+};
+
+static bool cvm_pmu_tlk_event_valid(u64 config)
+{
+	struct perf_pmu_events_attr *attr;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cvm_pmu_tlk_events_attr) - 1; i++) {
+		attr = (struct perf_pmu_events_attr *)cvm_pmu_tlk_events_attr[i];
+		if (attr->id == config)
+			return true;
+	}
+	return false;
+}
+
+static int cvm_pmu_tlk_probe_unit(struct pci_dev *pdev, int nr)
+{
+	struct cvm_pmu_dev *tlk;
+	int ret = -ENOMEM;
+
+	tlk = kzalloc(sizeof(*tlk), GFP_KERNEL);
+	if (!tlk)
+		return -ENOMEM;
+
+	tlk->map = ioremap(pci_resource_start(pdev, 0) + TLK_START_ADDR +
+			   nr * TLK_UNIT_OFFSET, TLK_UNIT_LEN);
+	if (!tlk->map)
+		goto fail_ioremap;
+
+	tlk->pmu_name = kasprintf(GFP_KERNEL, "ocx_tlk%d", nr);
+	if (!tlk->pmu_name)
+		goto fail_kasprintf;
+
+	tlk->pdev = pdev;
+	tlk->num_counters = ARRAY_SIZE(cvm_pmu_tlk_events_attr) - 1;
+	tlk->pmu = (struct pmu) {
+		.task_ctx_nr    = perf_invalid_context,
+		.pmu_enable	= cvm_pmu_tlk_enable_pmu,
+		.pmu_disable	= cvm_pmu_tlk_disable_pmu,
+		.event_init	= cvm_pmu_event_init,
+		.add		= cvm_pmu_tlk_add,
+		.del		= cvm_pmu_del,
+		.start		= cvm_pmu_start,
+		.stop		= cvm_pmu_stop,
+		.read		= cvm_pmu_read,
+		.attr_groups	= cvm_pmu_tlk_attr_groups,
+	};
+
+	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					 &tlk->cpuhp_node);
+
+	/*
+	 * perf PMU is CPU dependent so pick a random CPU and migrate away
+	 * if it goes offline.
+	 */
+	cpumask_set_cpu(smp_processor_id(), &tlk->active_mask);
+
+	list_add(&tlk->entry, &cvm_pmu_tlks);
+	tlk->event_valid = cvm_pmu_tlk_event_valid;
+
+	ret = perf_pmu_register(&tlk->pmu, tlk->pmu_name, -1);
+	if (ret)
+		goto fail_pmu;
+
+	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
+		 tlk->pmu_name, tlk->num_counters);
+	return 0;
+
+fail_pmu:
+	kfree(tlk->pmu_name);
+	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+				    &tlk->cpuhp_node);
+fail_kasprintf:
+	iounmap(tlk->map);
+fail_ioremap:
+	kfree(tlk);
+	return ret;
+}
+
+int cvm_ocx_tlk_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	int rc, i;
+
+	for (i = 0; i < TLK_NR_UNITS; i++) {
+		rc = cvm_pmu_tlk_probe_unit(pdev, i);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cvm_ocx_tlk_pmu_probe);
+
+void cvm_ocx_tlk_pmu_remove(struct pci_dev *pdev)
+{
+	struct list_head *l, *tmp;
+	struct cvm_pmu_dev *tlk;
+
+	list_for_each_safe(l, tmp, &cvm_pmu_tlks) {
+		tlk = list_entry(l, struct cvm_pmu_dev, entry);
+
+		if (pdev != tlk->pdev)
+			continue;
+
+		perf_pmu_unregister(&tlk->pmu);
+		iounmap(tlk->map);
+		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					    &tlk->cpuhp_node);
+		list_del(&tlk->entry);
+		kfree(tlk->pmu_name);
+		kfree(tlk);
+	}
+}
+EXPORT_SYMBOL_GPL(cvm_ocx_tlk_pmu_remove);
+
 static int __init cvm_pmu_init(void)
 {
 	INIT_LIST_HEAD(&cvm_pmu_lmcs);
diff --git a/drivers/soc/cavium/Kconfig b/drivers/soc/cavium/Kconfig
index fe56503..2c74068 100644
--- a/drivers/soc/cavium/Kconfig
+++ b/drivers/soc/cavium/Kconfig
@@ -8,3 +8,7 @@ config CAVIUM_LMC
 config CAVIUM_OCX
 	depends on ARCH_THUNDER
 	def_tristate m
+
+config CAVIUM_OCX
+        depends on ARCH_THUNDER
+	def_tristate m
diff --git a/drivers/soc/cavium/cavium_ocx.c b/drivers/soc/cavium/cavium_ocx.c
index fa3341b..de1ad146c 100644
--- a/drivers/soc/cavium/cavium_ocx.c
+++ b/drivers/soc/cavium/cavium_ocx.c
@@ -13,6 +13,8 @@
 static int cvm_ocx_probe(struct pci_dev *pdev,
 			 const struct pci_device_id *ent)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_OCX_TLK))
+		cvm_ocx_tlk_pmu_probe(pdev, ent);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_ocx_probe(pdev, ent);
 	return 0;
@@ -20,6 +22,8 @@ static int cvm_ocx_probe(struct pci_dev *pdev,
 
 static void cvm_ocx_remove(struct pci_dev *pdev)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_OCX_TLK))
+		cvm_ocx_tlk_pmu_remove(pdev);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_ocx_remove(pdev);
 }
diff --git a/include/linux/soc/cavium/ocx.h b/include/linux/soc/cavium/ocx.h
index 29f55b3..f7b2caa 100644
--- a/include/linux/soc/cavium/ocx.h
+++ b/include/linux/soc/cavium/ocx.h
@@ -3,6 +3,9 @@
 
 #include <linux/pci.h>
 
+int cvm_ocx_tlk_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void cvm_ocx_tlk_pmu_remove(struct pci_dev *pdev);
+
 int thunderx_edac_ocx_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
 void thunderx_edac_ocx_remove(struct pci_dev *pdev);
 
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 6/7] perf: cavium: Support transmit-link PMU counters

[Jan Glauber]

Add support for the transmit-link (OCX TLK) PMU counters found
on Caviums SOCs with a processor interconnect.

Properties of the OCX TLK counters:
- per-unit control
- fixed purpose
- writable
- one PCI device with multiple TLK units

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/perf/Kconfig            |   7 ++
 drivers/perf/cavium_pmu.c       | 235 ++++++++++++++++++++++++++++++++++++++++
 drivers/soc/cavium/Kconfig      |   4 +
 drivers/soc/cavium/cavium_ocx.c |   4 +
 include/linux/soc/cavium/ocx.h  |   3 +
 5 files changed, 253 insertions(+)

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index a787562..efb2ace 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -51,4 +51,11 @@ config CAVIUM_PMU_LMC
 	  Provides PMU counters for the memory controller on
 	  Cavium ThunderX or OcteonTX SOCs.
 
+config CAVIUM_PMU_OCX_TLK
+	tristate "Cavium ThunderX interconnect PMU"
+	depends on ARCH_THUNDER && m
+	select CAVIUM_OCX
+	help
+	  Provides PMU counters for the processor interconnect on
+	  Cavium ThunderX processors.
 endmenu
diff --git a/drivers/perf/cavium_pmu.c b/drivers/perf/cavium_pmu.c
index bcdedaa..cba8266 100644
--- a/drivers/perf/cavium_pmu.c
+++ b/drivers/perf/cavium_pmu.c
@@ -20,9 +20,11 @@
 #include <linux/perf_event.h>
 #include <linux/slab.h>
 #include <linux/soc/cavium/lmc.h>
+#include <linux/soc/cavium/ocx.h>
 
 enum cvm_pmu_type {
 	CVM_PMU_LMC,
+	CVM_PMU_TLK,
 };
 
 /* maximum number of parallel hardware counters for all pmu types */
@@ -422,6 +424,239 @@ void cvm_lmc_pmu_remove(struct pci_dev *pdev)
 }
 EXPORT_SYMBOL_GPL(cvm_lmc_pmu_remove);
 
+/*
+ * CCPI interface controller (OCX) Transmit link (TLK) counters:
+ * - per-unit control
+ * - writable
+ * - one PCI device with multiple TLK units
+ */
+
+#define TLK_NR_UNITS			3
+#define TLK_UNIT_OFFSET			0x2000
+#define TLK_UNIT_LEN			0x7ff
+#define TLK_START_ADDR			0x10000
+#define TLK_STAT_CTL_OFFSET		0x40
+#define TLK_STAT_OFFSET			0x400
+
+#define TLK_STAT_ENABLE_BIT		BIT(0)
+#define TLK_STAT_RESET_BIT		BIT(1)
+
+#define CVM_PMU_TLK_EVENT_ATTR(_name, _id)						\
+	&((struct perf_pmu_events_attr[]) {						\
+		{									\
+			__ATTR(_name, S_IRUGO, cvm_pmu_event_sysfs_show, NULL),		\
+			_id,								\
+			"tlk_event=" __stringify(_id),					\
+		}									\
+	})[0].attr.attr
+
+static void cvm_pmu_tlk_enable_pmu(struct pmu *pmu)
+{
+	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
+
+	/* enable all counters */
+	writeb(TLK_STAT_ENABLE_BIT, pmu_dev->map + TLK_STAT_CTL_OFFSET);
+}
+
+static void cvm_pmu_tlk_disable_pmu(struct pmu *pmu)
+{
+	struct cvm_pmu_dev *pmu_dev = container_of(pmu, struct cvm_pmu_dev, pmu);
+
+	/* disable all counters */
+	writeb(0, pmu_dev->map + TLK_STAT_CTL_OFFSET);
+}
+
+static int cvm_pmu_tlk_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	return cvm_pmu_add(event, flags, TLK_STAT_CTL_OFFSET,
+			   TLK_STAT_OFFSET + hwc->config * 8);
+}
+
+PMU_FORMAT_ATTR(tlk_event, "config:0-5");
+
+static struct attribute *cvm_pmu_tlk_format_attr[] = {
+	&format_attr_tlk_event.attr,
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_tlk_format_group = {
+	.name = "format",
+	.attrs = cvm_pmu_tlk_format_attr,
+};
+
+static struct attribute *cvm_pmu_tlk_events_attr[] = {
+	CVM_PMU_TLK_EVENT_ATTR(idle_cnt,	0x00),
+	CVM_PMU_TLK_EVENT_ATTR(data_cnt,	0x01),
+	CVM_PMU_TLK_EVENT_ATTR(sync_cnt,	0x02),
+	CVM_PMU_TLK_EVENT_ATTR(retry_cnt,	0x03),
+	CVM_PMU_TLK_EVENT_ATTR(err_cnt,		0x04),
+	CVM_PMU_TLK_EVENT_ATTR(mat0_cnt,	0x08),
+	CVM_PMU_TLK_EVENT_ATTR(mat1_cnt,	0x09),
+	CVM_PMU_TLK_EVENT_ATTR(mat2_cnt,	0x0a),
+	CVM_PMU_TLK_EVENT_ATTR(mat3_cnt,	0x0b),
+	CVM_PMU_TLK_EVENT_ATTR(vc0_cmd,		0x10),
+	CVM_PMU_TLK_EVENT_ATTR(vc1_cmd,		0x11),
+	CVM_PMU_TLK_EVENT_ATTR(vc2_cmd,		0x12),
+	CVM_PMU_TLK_EVENT_ATTR(vc3_cmd,		0x13),
+	CVM_PMU_TLK_EVENT_ATTR(vc4_cmd,		0x14),
+	CVM_PMU_TLK_EVENT_ATTR(vc5_cmd,		0x15),
+	CVM_PMU_TLK_EVENT_ATTR(vc0_pkt,		0x20),
+	CVM_PMU_TLK_EVENT_ATTR(vc1_pkt,		0x21),
+	CVM_PMU_TLK_EVENT_ATTR(vc2_pkt,		0x22),
+	CVM_PMU_TLK_EVENT_ATTR(vc3_pkt,		0x23),
+	CVM_PMU_TLK_EVENT_ATTR(vc4_pkt,		0x24),
+	CVM_PMU_TLK_EVENT_ATTR(vc5_pkt,		0x25),
+	CVM_PMU_TLK_EVENT_ATTR(vc6_pkt,		0x26),
+	CVM_PMU_TLK_EVENT_ATTR(vc7_pkt,		0x27),
+	CVM_PMU_TLK_EVENT_ATTR(vc8_pkt,		0x28),
+	CVM_PMU_TLK_EVENT_ATTR(vc9_pkt,		0x29),
+	CVM_PMU_TLK_EVENT_ATTR(vc10_pkt,	0x2a),
+	CVM_PMU_TLK_EVENT_ATTR(vc11_pkt,	0x2b),
+	CVM_PMU_TLK_EVENT_ATTR(vc12_pkt,	0x2c),
+	CVM_PMU_TLK_EVENT_ATTR(vc13_pkt,	0x2d),
+	CVM_PMU_TLK_EVENT_ATTR(vc0_con,		0x30),
+	CVM_PMU_TLK_EVENT_ATTR(vc1_con,		0x31),
+	CVM_PMU_TLK_EVENT_ATTR(vc2_con,		0x32),
+	CVM_PMU_TLK_EVENT_ATTR(vc3_con,		0x33),
+	CVM_PMU_TLK_EVENT_ATTR(vc4_con,		0x34),
+	CVM_PMU_TLK_EVENT_ATTR(vc5_con,		0x35),
+	CVM_PMU_TLK_EVENT_ATTR(vc6_con,		0x36),
+	CVM_PMU_TLK_EVENT_ATTR(vc7_con,		0x37),
+	CVM_PMU_TLK_EVENT_ATTR(vc8_con,		0x38),
+	CVM_PMU_TLK_EVENT_ATTR(vc9_con,		0x39),
+	CVM_PMU_TLK_EVENT_ATTR(vc10_con,	0x3a),
+	CVM_PMU_TLK_EVENT_ATTR(vc11_con,	0x3b),
+	CVM_PMU_TLK_EVENT_ATTR(vc12_con,	0x3c),
+	CVM_PMU_TLK_EVENT_ATTR(vc13_con,	0x3d),
+	NULL,
+};
+
+static struct attribute_group cvm_pmu_tlk_events_group = {
+	.name = "events",
+	.attrs = cvm_pmu_tlk_events_attr,
+};
+
+static const struct attribute_group *cvm_pmu_tlk_attr_groups[] = {
+	&cvm_pmu_attr_group,
+	&cvm_pmu_tlk_format_group,
+	&cvm_pmu_tlk_events_group,
+	NULL,
+};
+
+static bool cvm_pmu_tlk_event_valid(u64 config)
+{
+	struct perf_pmu_events_attr *attr;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cvm_pmu_tlk_events_attr) - 1; i++) {
+		attr = (struct perf_pmu_events_attr *)cvm_pmu_tlk_events_attr[i];
+		if (attr->id == config)
+			return true;
+	}
+	return false;
+}
+
+static int cvm_pmu_tlk_probe_unit(struct pci_dev *pdev, int nr)
+{
+	struct cvm_pmu_dev *tlk;
+	int ret = -ENOMEM;
+
+	tlk = kzalloc(sizeof(*tlk), GFP_KERNEL);
+	if (!tlk)
+		return -ENOMEM;
+
+	tlk->map = ioremap(pci_resource_start(pdev, 0) + TLK_START_ADDR +
+			   nr * TLK_UNIT_OFFSET, TLK_UNIT_LEN);
+	if (!tlk->map)
+		goto fail_ioremap;
+
+	tlk->pmu_name = kasprintf(GFP_KERNEL, "ocx_tlk%d", nr);
+	if (!tlk->pmu_name)
+		goto fail_kasprintf;
+
+	tlk->pdev = pdev;
+	tlk->num_counters = ARRAY_SIZE(cvm_pmu_tlk_events_attr) - 1;
+	tlk->pmu = (struct pmu) {
+		.task_ctx_nr    = perf_invalid_context,
+		.pmu_enable	= cvm_pmu_tlk_enable_pmu,
+		.pmu_disable	= cvm_pmu_tlk_disable_pmu,
+		.event_init	= cvm_pmu_event_init,
+		.add		= cvm_pmu_tlk_add,
+		.del		= cvm_pmu_del,
+		.start		= cvm_pmu_start,
+		.stop		= cvm_pmu_stop,
+		.read		= cvm_pmu_read,
+		.attr_groups	= cvm_pmu_tlk_attr_groups,
+	};
+
+	cpuhp_state_add_instance_nocalls(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					 &tlk->cpuhp_node);
+
+	/*
+	 * perf PMU is CPU dependent so pick a random CPU and migrate away
+	 * if it goes offline.
+	 */
+	cpumask_set_cpu(smp_processor_id(), &tlk->active_mask);
+
+	list_add(&tlk->entry, &cvm_pmu_tlks);
+	tlk->event_valid = cvm_pmu_tlk_event_valid;
+
+	ret = perf_pmu_register(&tlk->pmu, tlk->pmu_name, -1);
+	if (ret)
+		goto fail_pmu;
+
+	dev_info(&pdev->dev, "Enabled %s PMU with %d counters\n",
+		 tlk->pmu_name, tlk->num_counters);
+	return 0;
+
+fail_pmu:
+	kfree(tlk->pmu_name);
+	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+				    &tlk->cpuhp_node);
+fail_kasprintf:
+	iounmap(tlk->map);
+fail_ioremap:
+	kfree(tlk);
+	return ret;
+}
+
+int cvm_ocx_tlk_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	int rc, i;
+
+	for (i = 0; i < TLK_NR_UNITS; i++) {
+		rc = cvm_pmu_tlk_probe_unit(pdev, i);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cvm_ocx_tlk_pmu_probe);
+
+void cvm_ocx_tlk_pmu_remove(struct pci_dev *pdev)
+{
+	struct list_head *l, *tmp;
+	struct cvm_pmu_dev *tlk;
+
+	list_for_each_safe(l, tmp, &cvm_pmu_tlks) {
+		tlk = list_entry(l, struct cvm_pmu_dev, entry);
+
+		if (pdev != tlk->pdev)
+			continue;
+
+		perf_pmu_unregister(&tlk->pmu);
+		iounmap(tlk->map);
+		cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_CVM_ONLINE,
+					    &tlk->cpuhp_node);
+		list_del(&tlk->entry);
+		kfree(tlk->pmu_name);
+		kfree(tlk);
+	}
+}
+EXPORT_SYMBOL_GPL(cvm_ocx_tlk_pmu_remove);
+
 static int __init cvm_pmu_init(void)
 {
 	INIT_LIST_HEAD(&cvm_pmu_lmcs);
diff --git a/drivers/soc/cavium/Kconfig b/drivers/soc/cavium/Kconfig
index fe56503..2c74068 100644
--- a/drivers/soc/cavium/Kconfig
+++ b/drivers/soc/cavium/Kconfig
@@ -8,3 +8,7 @@ config CAVIUM_LMC
 config CAVIUM_OCX
 	depends on ARCH_THUNDER
 	def_tristate m
+
+config CAVIUM_OCX
+        depends on ARCH_THUNDER
+	def_tristate m
diff --git a/drivers/soc/cavium/cavium_ocx.c b/drivers/soc/cavium/cavium_ocx.c
index fa3341b..de1ad146c 100644
--- a/drivers/soc/cavium/cavium_ocx.c
+++ b/drivers/soc/cavium/cavium_ocx.c
@@ -13,6 +13,8 @@
 static int cvm_ocx_probe(struct pci_dev *pdev,
 			 const struct pci_device_id *ent)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_OCX_TLK))
+		cvm_ocx_tlk_pmu_probe(pdev, ent);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_ocx_probe(pdev, ent);
 	return 0;
@@ -20,6 +22,8 @@ static int cvm_ocx_probe(struct pci_dev *pdev,
 
 static void cvm_ocx_remove(struct pci_dev *pdev)
 {
+	if (IS_ENABLED(CONFIG_CAVIUM_PMU_OCX_TLK))
+		cvm_ocx_tlk_pmu_remove(pdev);
 	if (IS_ENABLED(CONFIG_EDAC_THUNDERX))
 		thunderx_edac_ocx_remove(pdev);
 }
diff --git a/include/linux/soc/cavium/ocx.h b/include/linux/soc/cavium/ocx.h
index 29f55b3..f7b2caa 100644
--- a/include/linux/soc/cavium/ocx.h
+++ b/include/linux/soc/cavium/ocx.h
@@ -3,6 +3,9 @@
 
 #include <linux/pci.h>
 
+int cvm_ocx_tlk_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
+void cvm_ocx_tlk_pmu_remove(struct pci_dev *pdev);
+
 int thunderx_edac_ocx_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
 void thunderx_edac_ocx_remove(struct pci_dev *pdev);
 
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 7/7] perf: cavium: Add Documentation

[Jan Glauber]

Document Cavium SoC PMUs.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 Documentation/perf/cavium-pmu.txt | 75 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/perf/cavium-pmu.txt

diff --git a/Documentation/perf/cavium-pmu.txt b/Documentation/perf/cavium-pmu.txt
new file mode 100644
index 0000000..6fbf824
--- /dev/null
+++ b/Documentation/perf/cavium-pmu.txt
@@ -0,0 +1,75 @@
+Cavium ThunderX and OcteonTx Performance Monitoring Unit (PMU)
+==============================================================
+
+Cavium SoCs contain various system devices such as L2 caches, processor
+interconnect and memory controllers. Unfortunately the PMU counters
+are not following a common design so each device has a slightly different
+approach how to control and use the PMU counters.
+
+Common properties of all devices carrying PMU counters:
+- The devices are PCI devices and the counters are embedded somewhere
+  in the PCI register space.
+- All counters are 64 bit wide.
+- There are no overflow interrupts (unnecessary because of the 64 bit wide
+  counters).
+
+Properties depending on the device type:
+- How to start/stop the counters
+- Programmable vs. fixed purpose counters
+- Stoppable vs. always running counters
+- Independent vs. grouped counters
+- Read-only vs. writable counters
+- PCI device to PMU group relationship
+
+
+Devices with PMU counters
+-------------------------
+
+Memory controller (LMC):
+- one PCI device per LMC
+- fixed-purpose counters
+- always running counters without start/stop/reset control
+- read-only counters
+
+CCPI interface controller (OCX) Transmit link (TLK) counters:
+- writable counters
+- only one PCI device exposes multiple TLK units (3 units on T88)
+- start/stop control per unit
+- only present on multi-socket systems
+
+PMU (perf) driver
+-----------------
+
+The cavium-pmu driver registers several perf PMU drivers. Each of the perf
+driver provides description of its available events and configuration options
+in sysfs, see /sys/devices/<lmcX/ocx_tlkX>/.
+
+The "format" directory describes format of the config (event ID),
+The "events" directory shows the names of the events and provides configuration
+templates for all supported event types that can be used with perf tool. For
+example, "lmc0/dclk_cnt/" is an equivalent of "lmc0/config=2/".
+
+Each perf driver also provides a "cpumask" sysfs attribute, which contains a
+single CPU ID of the processor which will be used to handle all the PMU events.
+
+Example for perf tool use:
+
+ / # perf list | grep -e lmc
+   lmc0/bank_conflict1/                               [Kernel PMU event]
+   lmc0/bank_conflict2/                               [Kernel PMU event]
+   lmc0/dclk_cnt/                                     [Kernel PMU event]
+   lmc0/ifb_cnt/                                      [Kernel PMU event]
+   lmc0/ops_cnt/                                      [Kernel PMU event]
+
+ / # perf stat -a -e lmc0/ops_cnt/,lmc0/dclk_cnt/ -- sleep 1
+
+   Performance counter stats for 'system wide':
+
+             176,133      lmc0/ops_cnt/                                               
+         670,243,653      lmc0/dclk_cnt/                                              
+
+         1.005479295 seconds time elapsed
+
+The driver does not support sampling, therefore "perf record" will
+not work. System wide mode ("-a") must be used as per-task (without "-a")
+perf sessions are not supported.
-- 
2.9.0.rc0.21.g7777322

[RFC PATCH v9 7/7] perf: cavium: Add Documentation

[Jan Glauber]

Document Cavium SoC PMUs.

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 Documentation/perf/cavium-pmu.txt | 75 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/perf/cavium-pmu.txt

diff --git a/Documentation/perf/cavium-pmu.txt b/Documentation/perf/cavium-pmu.txt
new file mode 100644
index 0000000..6fbf824
--- /dev/null
+++ b/Documentation/perf/cavium-pmu.txt
@@ -0,0 +1,75 @@
+Cavium ThunderX and OcteonTx Performance Monitoring Unit (PMU)
+==============================================================
+
+Cavium SoCs contain various system devices such as L2 caches, processor
+interconnect and memory controllers. Unfortunately the PMU counters
+are not following a common design so each device has a slightly different
+approach how to control and use the PMU counters.
+
+Common properties of all devices carrying PMU counters:
+- The devices are PCI devices and the counters are embedded somewhere
+  in the PCI register space.
+- All counters are 64 bit wide.
+- There are no overflow interrupts (unnecessary because of the 64 bit wide
+  counters).
+
+Properties depending on the device type:
+- How to start/stop the counters
+- Programmable vs. fixed purpose counters
+- Stoppable vs. always running counters
+- Independent vs. grouped counters
+- Read-only vs. writable counters
+- PCI device to PMU group relationship
+
+
+Devices with PMU counters
+-------------------------
+
+Memory controller (LMC):
+- one PCI device per LMC
+- fixed-purpose counters
+- always running counters without start/stop/reset control
+- read-only counters
+
+CCPI interface controller (OCX) Transmit link (TLK) counters:
+- writable counters
+- only one PCI device exposes multiple TLK units (3 units on T88)
+- start/stop control per unit
+- only present on multi-socket systems
+
+PMU (perf) driver
+-----------------
+
+The cavium-pmu driver registers several perf PMU drivers. Each of the perf
+driver provides description of its available events and configuration options
+in sysfs, see /sys/devices/<lmcX/ocx_tlkX>/.
+
+The "format" directory describes format of the config (event ID),
+The "events" directory shows the names of the events and provides configuration
+templates for all supported event types that can be used with perf tool. For
+example, "lmc0/dclk_cnt/" is an equivalent of "lmc0/config=2/".
+
+Each perf driver also provides a "cpumask" sysfs attribute, which contains a
+single CPU ID of the processor which will be used to handle all the PMU events.
+
+Example for perf tool use:
+
+ / # perf list | grep -e lmc
+   lmc0/bank_conflict1/                               [Kernel PMU event]
+   lmc0/bank_conflict2/                               [Kernel PMU event]
+   lmc0/dclk_cnt/                                     [Kernel PMU event]
+   lmc0/ifb_cnt/                                      [Kernel PMU event]
+   lmc0/ops_cnt/                                      [Kernel PMU event]
+
+ / # perf stat -a -e lmc0/ops_cnt/,lmc0/dclk_cnt/ -- sleep 1
+
+   Performance counter stats for 'system wide':
+
+             176,133      lmc0/ops_cnt/                                               
+         670,243,653      lmc0/dclk_cnt/                                              
+
+         1.005479295 seconds time elapsed
+
+The driver does not support sampling, therefore "perf record" will
+not work. System wide mode ("-a") must be used as per-task (without "-a")
+perf sessions are not supported.
-- 
2.9.0.rc0.21.g7777322

Re: [RFC PATCH v9 0/7] Cavium ARM64 uncore PMU support

[Jan Glauber]

So what about the general idea with the wrapper, does this look sane?
Any objections to that?

thanks,
Jan

On Tue, Aug 29, 2017 at 03:12:31PM +0200, Jan Glauber wrote:
> I'm posting this as RFC following this discussion:
> https://marc.info/?l=linux-arm-kernel&m=150099526923838&w=2
> 
> I've implemented the wrapper for the PCI devices and put it under
> drivers/soc/cavium which I found more appropriate than drivers/misc.
> 
> I was not able to find a way to build the EDAC driver and the PMU driver
> with all combinations (builtin and module) so I limited the build options
> to module only. The problem is that the select from EDAC or PMU
> sets the wrappers build type to whatever EDAC or PMU choose.
> But all parts must be either built-in or modules, having the wrapper
> builtin and calling into module code will not work. If there is a better
> solution please let me know.
> 
> The PMU code is the same as in v8.
> 
> Add support for various PMU counters found on the Cavium ThunderX and
> OcteonTx SoC.
> 
> The PMU driver provides common "uncore" functions to avoid code duplication
> and support adding more device PMUs (like L2 cache) in the future.
> 
> Changes to v8:
> - Wrapper for PCI devices
> 
> Jan Glauber (7):
>   edac: thunderx: Remove suspend/resume support
>   edac,soc: thunderx: Add wrapper for EDAC LMC PCI device
>   edac,soc: thunderx: Add wrapper for EDAC OCX PCI device
>   perf: export perf_event_update_userpage()
>   perf: cavium: Support memory controller PMU counters
>   perf: cavium: Support transmit-link PMU counters
>   perf: cavium: Add Documentation
> 
>  Documentation/perf/cavium-pmu.txt |  75 +++++
>  drivers/edac/Kconfig              |   3 +
>  drivers/edac/thunderx_edac.c      |  92 +-----
>  drivers/perf/Kconfig              |  15 +
>  drivers/perf/Makefile             |   1 +
>  drivers/perf/cavium_pmu.c         | 680 ++++++++++++++++++++++++++++++++++++++
>  drivers/soc/Kconfig               |   1 +
>  drivers/soc/Makefile              |   1 +
>  drivers/soc/cavium/Kconfig        |  14 +
>  drivers/soc/cavium/Makefile       |   2 +
>  drivers/soc/cavium/cavium_lmc.c   |  53 +++
>  drivers/soc/cavium/cavium_ocx.c   |  49 +++
>  include/linux/cpuhotplug.h        |   1 +
>  include/linux/soc/cavium/lmc.h    |  12 +
>  include/linux/soc/cavium/ocx.h    |  12 +
>  kernel/events/core.c              |   1 +
>  16 files changed, 933 insertions(+), 79 deletions(-)
>  create mode 100644 Documentation/perf/cavium-pmu.txt
>  create mode 100644 drivers/perf/cavium_pmu.c
>  create mode 100644 drivers/soc/cavium/Kconfig
>  create mode 100644 drivers/soc/cavium/Makefile
>  create mode 100644 drivers/soc/cavium/cavium_lmc.c
>  create mode 100644 drivers/soc/cavium/cavium_ocx.c
>  create mode 100644 include/linux/soc/cavium/lmc.h
>  create mode 100644 include/linux/soc/cavium/ocx.h
> 
> -- 
> 2.9.0.rc0.21.g7777322

[RFC PATCH v9 0/7] Cavium ARM64 uncore PMU support

[Jan Glauber]

So what about the general idea with the wrapper, does this look sane?
Any objections to that?

thanks,
Jan

On Tue, Aug 29, 2017 at 03:12:31PM +0200, Jan Glauber wrote:
> I'm posting this as RFC following this discussion:
> https://marc.info/?l=linux-arm-kernel&m=150099526923838&w=2
> 
> I've implemented the wrapper for the PCI devices and put it under
> drivers/soc/cavium which I found more appropriate than drivers/misc.
> 
> I was not able to find a way to build the EDAC driver and the PMU driver
> with all combinations (builtin and module) so I limited the build options
> to module only. The problem is that the select from EDAC or PMU
> sets the wrappers build type to whatever EDAC or PMU choose.
> But all parts must be either built-in or modules, having the wrapper
> builtin and calling into module code will not work. If there is a better
> solution please let me know.
> 
> The PMU code is the same as in v8.
> 
> Add support for various PMU counters found on the Cavium ThunderX and
> OcteonTx SoC.
> 
> The PMU driver provides common "uncore" functions to avoid code duplication
> and support adding more device PMUs (like L2 cache) in the future.
> 
> Changes to v8:
> - Wrapper for PCI devices
> 
> Jan Glauber (7):
>   edac: thunderx: Remove suspend/resume support
>   edac,soc: thunderx: Add wrapper for EDAC LMC PCI device
>   edac,soc: thunderx: Add wrapper for EDAC OCX PCI device
>   perf: export perf_event_update_userpage()
>   perf: cavium: Support memory controller PMU counters
>   perf: cavium: Support transmit-link PMU counters
>   perf: cavium: Add Documentation
> 
>  Documentation/perf/cavium-pmu.txt |  75 +++++
>  drivers/edac/Kconfig              |   3 +
>  drivers/edac/thunderx_edac.c      |  92 +-----
>  drivers/perf/Kconfig              |  15 +
>  drivers/perf/Makefile             |   1 +
>  drivers/perf/cavium_pmu.c         | 680 ++++++++++++++++++++++++++++++++++++++
>  drivers/soc/Kconfig               |   1 +
>  drivers/soc/Makefile              |   1 +
>  drivers/soc/cavium/Kconfig        |  14 +
>  drivers/soc/cavium/Makefile       |   2 +
>  drivers/soc/cavium/cavium_lmc.c   |  53 +++
>  drivers/soc/cavium/cavium_ocx.c   |  49 +++
>  include/linux/cpuhotplug.h        |   1 +
>  include/linux/soc/cavium/lmc.h    |  12 +
>  include/linux/soc/cavium/ocx.h    |  12 +
>  kernel/events/core.c              |   1 +
>  16 files changed, 933 insertions(+), 79 deletions(-)
>  create mode 100644 Documentation/perf/cavium-pmu.txt
>  create mode 100644 drivers/perf/cavium_pmu.c
>  create mode 100644 drivers/soc/cavium/Kconfig
>  create mode 100644 drivers/soc/cavium/Makefile
>  create mode 100644 drivers/soc/cavium/cavium_lmc.c
>  create mode 100644 drivers/soc/cavium/cavium_ocx.c
>  create mode 100644 include/linux/soc/cavium/lmc.h
>  create mode 100644 include/linux/soc/cavium/ocx.h
> 
> -- 
> 2.9.0.rc0.21.g7777322