All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s
@ 2019-04-29  2:52 Madhavan Srinivasan
  2019-04-29  2:52 ` [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver Madhavan Srinivasan
  2019-04-29  5:38 ` [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Christophe Leroy
  0 siblings, 2 replies; 6+ messages in thread
From: Madhavan Srinivasan @ 2019-04-29  2:52 UTC (permalink / raw)
  To: mpe; +Cc: Madhavan Srinivasan, linuxppc-dev

Currenty pmu driver file for each ppc64 generation processor
has a __init call in itself. Refactor the code by moving the
__init call to core-books.c. This also clean's up compat mode
pmu driver registration.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Changelog v1:
- Added "internal.h" file and moved the extern definitions to that file

 arch/powerpc/perf/core-book3s.c | 28 ++++++++++++++++++++++++++++
 arch/powerpc/perf/internal.h    | 16 ++++++++++++++++
 arch/powerpc/perf/power5+-pmu.c |  4 +---
 arch/powerpc/perf/power5-pmu.c  |  4 +---
 arch/powerpc/perf/power6-pmu.c  |  4 +---
 arch/powerpc/perf/power7-pmu.c  |  4 +---
 arch/powerpc/perf/power8-pmu.c  |  3 +--
 arch/powerpc/perf/power9-pmu.c  |  3 +--
 arch/powerpc/perf/ppc970-pmu.c  |  4 +---
 9 files changed, 51 insertions(+), 19 deletions(-)
 create mode 100644 arch/powerpc/perf/internal.h

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index b0723002a396..a96f9420139c 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -22,6 +22,10 @@
 #include <asm/ptrace.h>
 #include <asm/code-patching.h>
 
+#ifdef CONFIG_PPC64
+#include "internal.h"
+#endif
+
 #define BHRB_MAX_ENTRIES	32
 #define BHRB_TARGET		0x0000000000000002
 #define BHRB_PREDICTION		0x0000000000000001
@@ -2294,3 +2298,27 @@ int register_power_pmu(struct power_pmu *pmu)
 			  power_pmu_prepare_cpu, NULL);
 	return 0;
 }
+
+#ifdef CONFIG_PPC64
+static int __init init_ppc64_pmu(void)
+{
+	/* run through all the pmu drivers one at a time */
+	if (!init_power5_pmu())
+		return 0;
+	else if (!init_power5p_pmu())
+		return 0;
+	else if (!init_power6_pmu())
+		return 0;
+	else if (!init_power7_pmu())
+		return 0;
+	else if (!init_power8_pmu())
+		return 0;
+	else if (!init_power9_pmu())
+		return 0;
+	else if (!init_ppc970_pmu())
+		return 0;
+	else
+		return -ENODEV;
+}
+early_initcall(init_ppc64_pmu);
+#endif
diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
new file mode 100644
index 000000000000..e54d524d4283
--- /dev/null
+++ b/arch/powerpc/perf/internal.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright 2019 Madhavan Srinivasan, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+extern int init_ppc970_pmu(void);
+extern int init_power5_pmu(void);
+extern int init_power5p_pmu(void);
+extern int init_power6_pmu(void);
+extern int init_power7_pmu(void);
+extern int init_power8_pmu(void);
+extern int init_power9_pmu(void);
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index 0526dac66007..9aa803504cb2 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -677,7 +677,7 @@ static struct power_pmu power5p_pmu = {
 	.cache_events		= &power5p_cache_events,
 };
 
-static int __init init_power5p_pmu(void)
+int init_power5p_pmu(void)
 {
 	if (!cur_cpu_spec->oprofile_cpu_type ||
 	    (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
@@ -686,5 +686,3 @@ static int __init init_power5p_pmu(void)
 
 	return register_power_pmu(&power5p_pmu);
 }
-
-early_initcall(init_power5p_pmu);
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index 4dc99f9f7962..30cb13d081a9 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -618,7 +618,7 @@ static struct power_pmu power5_pmu = {
 	.flags			= PPMU_HAS_SSLOT,
 };
 
-static int __init init_power5_pmu(void)
+int init_power5_pmu(void)
 {
 	if (!cur_cpu_spec->oprofile_cpu_type ||
 	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
@@ -626,5 +626,3 @@ static int __init init_power5_pmu(void)
 
 	return register_power_pmu(&power5_pmu);
 }
-
-early_initcall(init_power5_pmu);
diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
index 9c9d646b68a1..80ec48632cfe 100644
--- a/arch/powerpc/perf/power6-pmu.c
+++ b/arch/powerpc/perf/power6-pmu.c
@@ -540,7 +540,7 @@ static struct power_pmu power6_pmu = {
 	.cache_events		= &power6_cache_events,
 };
 
-static int __init init_power6_pmu(void)
+int init_power6_pmu(void)
 {
 	if (!cur_cpu_spec->oprofile_cpu_type ||
 	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
@@ -548,5 +548,3 @@ static int __init init_power6_pmu(void)
 
 	return register_power_pmu(&power6_pmu);
 }
-
-early_initcall(init_power6_pmu);
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 6dbae9884ec4..bb6efd5d2530 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -445,7 +445,7 @@ static struct power_pmu power7_pmu = {
 	.cache_events		= &power7_cache_events,
 };
 
-static int __init init_power7_pmu(void)
+int init_power7_pmu(void)
 {
 	if (!cur_cpu_spec->oprofile_cpu_type ||
 	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
@@ -456,5 +456,3 @@ static int __init init_power7_pmu(void)
 
 	return register_power_pmu(&power7_pmu);
 }
-
-early_initcall(init_power7_pmu);
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d12a2db26353..bcc3409a06de 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -379,7 +379,7 @@ static struct power_pmu power8_pmu = {
 	.bhrb_nr		= 32,
 };
 
-static int __init init_power8_pmu(void)
+int init_power8_pmu(void)
 {
 	int rc;
 
@@ -399,4 +399,3 @@ static int __init init_power8_pmu(void)
 
 	return 0;
 }
-early_initcall(init_power8_pmu);
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 030544e35959..3a31ac6f4805 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -437,7 +437,7 @@ static struct power_pmu power9_pmu = {
 	.bhrb_nr		= 32,
 };
 
-static int __init init_power9_pmu(void)
+int init_power9_pmu(void)
 {
 	int rc = 0;
 	unsigned int pvr = mfspr(SPRN_PVR);
@@ -467,4 +467,3 @@ static int __init init_power9_pmu(void)
 
 	return 0;
 }
-early_initcall(init_power9_pmu);
diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
index 8b6a8a36fa38..1d3370914022 100644
--- a/arch/powerpc/perf/ppc970-pmu.c
+++ b/arch/powerpc/perf/ppc970-pmu.c
@@ -490,7 +490,7 @@ static struct power_pmu ppc970_pmu = {
 	.flags			= PPMU_NO_SIPR | PPMU_NO_CONT_SAMPLING,
 };
 
-static int __init init_ppc970_pmu(void)
+int init_ppc970_pmu(void)
 {
 	if (!cur_cpu_spec->oprofile_cpu_type ||
 	    (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
@@ -499,5 +499,3 @@ static int __init init_ppc970_pmu(void)
 
 	return register_power_pmu(&ppc970_pmu);
 }
-
-early_initcall(init_ppc970_pmu);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver
  2019-04-29  2:52 [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Madhavan Srinivasan
@ 2019-04-29  2:52 ` Madhavan Srinivasan
  2019-04-29  5:42   ` Christophe Leroy
  2019-04-29  5:38 ` [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Christophe Leroy
  1 sibling, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2019-04-29  2:52 UTC (permalink / raw)
  To: mpe; +Cc: Madhavan Srinivasan, linuxppc-dev

Most of the power processor generation performance monitoring
unit (PMU) driver code is bundled in the kernel and one of those
is enabled/registered based on the oprofile_cpu_type check at
the boot.

But things get little tricky incase of "compat" mode boot.
IBM POWER System Server based processors has a compactibility
mode feature, which simpily put is, Nth generation processor
(lets say POWER8) will act and appear in a mode consistent
with an earlier generation (N-1) processor (that is POWER7).
And in this "compat" mode boot, kernel modify the
"oprofile_cpu_type" to be Nth generation (POWER8). If Nth
generation pmu driver is bundled (POWER8), it gets registered.

Key dependency here is to have distro support for latest
processor performance monitoring support. Patch here adds
a generic "compat-mode" performance monitoring driver to
be register in absence of powernv platform specific pmu driver.

Driver supports "cycles", "instruction" and "branch-miss" events.
"0x100F0" used as event code for "cycles", "0x00002"
used as event code for "instruction" events and "0x400F6"
used as event code for "branch miss". These are architected events
as part of ISA. New file called "generic-compat-pmu.c" is
created to contain the driver specific code. And base raw event
code format modeled on PPMU_ARCH_207S.

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Changelog v1:
- Updated architected event opcodes
- included branch miss with architected event opcode

 arch/powerpc/perf/Makefile             |   3 +-
 arch/powerpc/perf/core-book3s.c        |   2 +-
 arch/powerpc/perf/generic-compat-pmu.c | 245 +++++++++++++++++++++++++++++++++
 arch/powerpc/perf/internal.h           |   1 +
 4 files changed, 249 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/perf/generic-compat-pmu.c

diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
index ab26df5bacb9..c155dcbb8691 100644
--- a/arch/powerpc/perf/Makefile
+++ b/arch/powerpc/perf/Makefile
@@ -5,7 +5,8 @@ obj-$(CONFIG_PERF_EVENTS)	+= callchain.o perf_regs.o
 obj-$(CONFIG_PPC_PERF_CTRS)	+= core-book3s.o bhrb.o
 obj64-$(CONFIG_PPC_PERF_CTRS)	+= ppc970-pmu.o power5-pmu.o \
 				   power5+-pmu.o power6-pmu.o power7-pmu.o \
-				   isa207-common.o power8-pmu.o power9-pmu.o
+				   isa207-common.o power8-pmu.o power9-pmu.o \
+				   generic-compat-pmu.o
 obj32-$(CONFIG_PPC_PERF_CTRS)	+= mpc7450-pmu.o
 
 obj-$(CONFIG_PPC_POWERNV)	+= imc-pmu.o
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index a96f9420139c..a66fb9c01c9e 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2318,7 +2318,7 @@ static int __init init_ppc64_pmu(void)
 	else if (!init_ppc970_pmu())
 		return 0;
 	else
-		return -ENODEV;
+		return init_generic_compat_pmu();
 }
 early_initcall(init_ppc64_pmu);
 #endif
diff --git a/arch/powerpc/perf/generic-compat-pmu.c b/arch/powerpc/perf/generic-compat-pmu.c
new file mode 100644
index 000000000000..9c2d4bbc5c87
--- /dev/null
+++ b/arch/powerpc/perf/generic-compat-pmu.c
@@ -0,0 +1,245 @@
+/*
+ * Performance counter support.
+ *
+ * Copyright 2019 Madhavan Srinivasan, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or later version.
+ */
+
+#define pr_fmt(fmt)	"generic-compat-pmu: " fmt
+
+#include "isa207-common.h"
+
+/*
+ * Raw event encoding:
+ *
+ *        60        56        52        48        44        40        36        32
+ * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
+ *
+ *        28        24        20        16        12         8         4         0
+ * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
+ *                                 [ pmc ]   [unit ]   [ ]   m   [    pmcxsel    ]
+ *                                                     |     |
+ *                                                     |     *- mark
+ *                                                     |
+ *                                                     |
+ *                                                     *- combine
+ *
+ * Below uses IBM bit numbering.
+ *
+ * MMCR1[x:y] = unit    (PMCxUNIT)
+ * MMCR1[24]   = pmc1combine[0]
+ * MMCR1[25]   = pmc1combine[1]
+ * MMCR1[26]   = pmc2combine[0]
+ * MMCR1[27]   = pmc2combine[1]
+ * MMCR1[28]   = pmc3combine[0]
+ * MMCR1[29]   = pmc3combine[1]
+ * MMCR1[30]   = pmc4combine[0]
+ * MMCR1[31]   = pmc4combine[1]
+ *
+ */
+
+/*
+ * Some power9 event codes.
+ */
+#define EVENT(_name, _code)	_name = _code,
+
+enum {
+EVENT(PM_CYC,					0x100F0)
+EVENT(PM_INST_CMPL,				0x00002)
+EVENT(PM_BR_MPRED_CMPL,				0x400F6)
+};
+
+#undef EVENT
+
+GENERIC_EVENT_ATTR(cpu-cycles,			PM_CYC);
+GENERIC_EVENT_ATTR(instructions,		PM_INST_CMPL);
+GENERIC_EVENT_ATTR(branch-misses,               PM_BR_MPRED_CMPL);
+
+static struct attribute *generic_compat_events_attr[] = {
+	GENERIC_EVENT_PTR(PM_CYC),
+	GENERIC_EVENT_PTR(PM_INST_CMPL),
+	GENERIC_EVENT_PTR(PM_BR_MPRED_CMPL),
+	NULL
+};
+
+static struct attribute_group generic_compat_pmu_events_group = {
+	.name = "events",
+	.attrs = generic_compat_events_attr,
+};
+
+PMU_FORMAT_ATTR(event,		"config:0-19");
+PMU_FORMAT_ATTR(pmcxsel,	"config:0-7");
+PMU_FORMAT_ATTR(mark,		"config:8");
+PMU_FORMAT_ATTR(combine,	"config:10-11");
+PMU_FORMAT_ATTR(unit,		"config:12-15");
+PMU_FORMAT_ATTR(pmc,		"config:16-19");
+
+static struct attribute *generic_compat_pmu_format_attr[] = {
+	&format_attr_event.attr,
+	&format_attr_pmcxsel.attr,
+	&format_attr_mark.attr,
+	&format_attr_combine.attr,
+	&format_attr_unit.attr,
+	&format_attr_pmc.attr,
+	NULL,
+};
+
+static struct attribute_group generic_compat_pmu_format_group = {
+	.name = "format",
+	.attrs = generic_compat_pmu_format_attr,
+};
+
+static const struct attribute_group *generic_compat_pmu_attr_groups[] = {
+	&generic_compat_pmu_format_group,
+	&generic_compat_pmu_events_group,
+	NULL,
+};
+
+static int compat_generic_events[] = {
+	[PERF_COUNT_HW_CPU_CYCLES] =			PM_CYC,
+	[PERF_COUNT_HW_INSTRUCTIONS] =			PM_INST_CMPL,
+	[PERF_COUNT_HW_BRANCH_MISSES] =                 PM_BR_MPRED_CMPL,
+};
+
+#define C(x)	PERF_COUNT_HW_CACHE_##x
+
+/*
+ * Table of generalized cache-related events.
+ * 0 means not supported, -1 means nonsensical, other values
+ * are event codes.
+ */
+static int generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
+	[ C(L1D) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+	},
+	[ C(L1I) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+	},
+	[ C(LL) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+	},
+	[ C(DTLB) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+	},
+	[ C(ITLB) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+	},
+	[ C(BPU) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = 0,
+			[ C(RESULT_MISS)   ] = 0,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+	},
+	[ C(NODE) ] = {
+		[ C(OP_READ) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+		[ C(OP_WRITE) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+		[ C(OP_PREFETCH) ] = {
+			[ C(RESULT_ACCESS) ] = -1,
+			[ C(RESULT_MISS)   ] = -1,
+		},
+	},
+};
+
+#undef C
+
+static struct power_pmu generic_compat_pmu = {
+	.name			= "GENERIC_COMPAT",
+	.n_counter		= MAX_PMU_COUNTERS,
+	.add_fields		= ISA207_ADD_FIELDS,
+	.test_adder		= ISA207_TEST_ADDER,
+	.compute_mmcr		= isa207_compute_mmcr,
+	.get_constraint		= isa207_get_constraint,
+	.disable_pmc		= isa207_disable_pmc,
+	.flags			= PPMU_HAS_SIER | PPMU_ARCH_207S,
+	.n_generic		= ARRAY_SIZE(compat_generic_events),
+	.generic_events		= compat_generic_events,
+	.cache_events		= &generic_compat_cache_events,
+	.attr_groups		= generic_compat_pmu_attr_groups,
+};
+
+int init_generic_compat_pmu(void)
+{
+	int rc = 0;
+
+	rc = register_power_pmu(&generic_compat_pmu);
+	if (rc)
+		return rc;
+
+	/* Tell userspace that EBB is supported */
+	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_EBB;
+
+	return 0;
+}
diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
index e54d524d4283..185a40d1adff 100644
--- a/arch/powerpc/perf/internal.h
+++ b/arch/powerpc/perf/internal.h
@@ -14,3 +14,4 @@ extern int init_power6_pmu(void);
 extern int init_power7_pmu(void);
 extern int init_power8_pmu(void);
 extern int init_power9_pmu(void);
+extern int init_generic_compat_pmu(void);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s
  2019-04-29  2:52 [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Madhavan Srinivasan
  2019-04-29  2:52 ` [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver Madhavan Srinivasan
@ 2019-04-29  5:38 ` Christophe Leroy
  2019-04-30  6:34   ` Madhavan Srinivasan
  1 sibling, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2019-04-29  5:38 UTC (permalink / raw)
  To: Madhavan Srinivasan, mpe; +Cc: linuxppc-dev



Le 29/04/2019 à 04:52, Madhavan Srinivasan a écrit :
> Currenty pmu driver file for each ppc64 generation processor
> has a __init call in itself. Refactor the code by moving the
> __init call to core-books.c. This also clean's up compat mode
> pmu driver registration.

Can you explain the advantage of doing so ?
For me it makes more sense to have independant drivers with their own 
init call.


> 
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> Changelog v1:
> - Added "internal.h" file and moved the extern definitions to that file
> 
>   arch/powerpc/perf/core-book3s.c | 28 ++++++++++++++++++++++++++++
>   arch/powerpc/perf/internal.h    | 16 ++++++++++++++++
>   arch/powerpc/perf/power5+-pmu.c |  4 +---
>   arch/powerpc/perf/power5-pmu.c  |  4 +---
>   arch/powerpc/perf/power6-pmu.c  |  4 +---
>   arch/powerpc/perf/power7-pmu.c  |  4 +---
>   arch/powerpc/perf/power8-pmu.c  |  3 +--
>   arch/powerpc/perf/power9-pmu.c  |  3 +--
>   arch/powerpc/perf/ppc970-pmu.c  |  4 +---
>   9 files changed, 51 insertions(+), 19 deletions(-)
>   create mode 100644 arch/powerpc/perf/internal.h
> 
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index b0723002a396..a96f9420139c 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -22,6 +22,10 @@
>   #include <asm/ptrace.h>
>   #include <asm/code-patching.h>
>   
> +#ifdef CONFIG_PPC64

Can we avoid that CONFIG_PPC64 ifdef ? Why isn't it compatible with PPC32 ?

> +#include "internal.h"
> +#endif
> +
>   #define BHRB_MAX_ENTRIES	32
>   #define BHRB_TARGET		0x0000000000000002
>   #define BHRB_PREDICTION		0x0000000000000001
> @@ -2294,3 +2298,27 @@ int register_power_pmu(struct power_pmu *pmu)
>   			  power_pmu_prepare_cpu, NULL);
>   	return 0;
>   }
> +
> +#ifdef CONFIG_PPC64

Same, why PPC64 ?

> +static int __init init_ppc64_pmu(void)
> +{
> +	/* run through all the pmu drivers one at a time */
> +	if (!init_power5_pmu())
> +		return 0;
> +	else if (!init_power5p_pmu())
> +		return 0;
> +	else if (!init_power6_pmu())
> +		return 0;
> +	else if (!init_power7_pmu())
> +		return 0;
> +	else if (!init_power8_pmu())
> +		return 0;
> +	else if (!init_power9_pmu())
> +		return 0;
> +	else if (!init_ppc970_pmu())
> +		return 0;
> +	else
> +		return -ENODEV;
> +}
> +early_initcall(init_ppc64_pmu);
> +#endif
> diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
> new file mode 100644
> index 000000000000..e54d524d4283
> --- /dev/null
> +++ b/arch/powerpc/perf/internal.h
> @@ -0,0 +1,16 @@
> +/*
> + * Copyright 2019 Madhavan Srinivasan, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +extern int init_ppc970_pmu(void);
> +extern int init_power5_pmu(void);
> +extern int init_power5p_pmu(void);
> +extern int init_power6_pmu(void);
> +extern int init_power7_pmu(void);
> +extern int init_power8_pmu(void);
> +extern int init_power9_pmu(void);

'extern' keyword is pointless, please remove it (checkpatch --strict 
probably told it to you).


Christophe


> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
> index 0526dac66007..9aa803504cb2 100644
> --- a/arch/powerpc/perf/power5+-pmu.c
> +++ b/arch/powerpc/perf/power5+-pmu.c
> @@ -677,7 +677,7 @@ static struct power_pmu power5p_pmu = {
>   	.cache_events		= &power5p_cache_events,
>   };
>   
> -static int __init init_power5p_pmu(void)
> +int init_power5p_pmu(void)
>   {
>   	if (!cur_cpu_spec->oprofile_cpu_type ||
>   	    (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
> @@ -686,5 +686,3 @@ static int __init init_power5p_pmu(void)
>   
>   	return register_power_pmu(&power5p_pmu);
>   }
> -
> -early_initcall(init_power5p_pmu);
> diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
> index 4dc99f9f7962..30cb13d081a9 100644
> --- a/arch/powerpc/perf/power5-pmu.c
> +++ b/arch/powerpc/perf/power5-pmu.c
> @@ -618,7 +618,7 @@ static struct power_pmu power5_pmu = {
>   	.flags			= PPMU_HAS_SSLOT,
>   };
>   
> -static int __init init_power5_pmu(void)
> +int init_power5_pmu(void)
>   {
>   	if (!cur_cpu_spec->oprofile_cpu_type ||
>   	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
> @@ -626,5 +626,3 @@ static int __init init_power5_pmu(void)
>   
>   	return register_power_pmu(&power5_pmu);
>   }
> -
> -early_initcall(init_power5_pmu);
> diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
> index 9c9d646b68a1..80ec48632cfe 100644
> --- a/arch/powerpc/perf/power6-pmu.c
> +++ b/arch/powerpc/perf/power6-pmu.c
> @@ -540,7 +540,7 @@ static struct power_pmu power6_pmu = {
>   	.cache_events		= &power6_cache_events,
>   };
>   
> -static int __init init_power6_pmu(void)
> +int init_power6_pmu(void)
>   {
>   	if (!cur_cpu_spec->oprofile_cpu_type ||
>   	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
> @@ -548,5 +548,3 @@ static int __init init_power6_pmu(void)
>   
>   	return register_power_pmu(&power6_pmu);
>   }
> -
> -early_initcall(init_power6_pmu);
> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index 6dbae9884ec4..bb6efd5d2530 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -445,7 +445,7 @@ static struct power_pmu power7_pmu = {
>   	.cache_events		= &power7_cache_events,
>   };
>   
> -static int __init init_power7_pmu(void)
> +int init_power7_pmu(void)
>   {
>   	if (!cur_cpu_spec->oprofile_cpu_type ||
>   	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
> @@ -456,5 +456,3 @@ static int __init init_power7_pmu(void)
>   
>   	return register_power_pmu(&power7_pmu);
>   }
> -
> -early_initcall(init_power7_pmu);
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index d12a2db26353..bcc3409a06de 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -379,7 +379,7 @@ static struct power_pmu power8_pmu = {
>   	.bhrb_nr		= 32,
>   };
>   
> -static int __init init_power8_pmu(void)
> +int init_power8_pmu(void)
>   {
>   	int rc;
>   
> @@ -399,4 +399,3 @@ static int __init init_power8_pmu(void)
>   
>   	return 0;
>   }
> -early_initcall(init_power8_pmu);
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 030544e35959..3a31ac6f4805 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -437,7 +437,7 @@ static struct power_pmu power9_pmu = {
>   	.bhrb_nr		= 32,
>   };
>   
> -static int __init init_power9_pmu(void)
> +int init_power9_pmu(void)
>   {
>   	int rc = 0;
>   	unsigned int pvr = mfspr(SPRN_PVR);
> @@ -467,4 +467,3 @@ static int __init init_power9_pmu(void)
>   
>   	return 0;
>   }
> -early_initcall(init_power9_pmu);
> diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
> index 8b6a8a36fa38..1d3370914022 100644
> --- a/arch/powerpc/perf/ppc970-pmu.c
> +++ b/arch/powerpc/perf/ppc970-pmu.c
> @@ -490,7 +490,7 @@ static struct power_pmu ppc970_pmu = {
>   	.flags			= PPMU_NO_SIPR | PPMU_NO_CONT_SAMPLING,
>   };
>   
> -static int __init init_ppc970_pmu(void)
> +int init_ppc970_pmu(void)
>   {
>   	if (!cur_cpu_spec->oprofile_cpu_type ||
>   	    (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
> @@ -499,5 +499,3 @@ static int __init init_ppc970_pmu(void)
>   
>   	return register_power_pmu(&ppc970_pmu);
>   }
> -
> -early_initcall(init_ppc970_pmu);
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver
  2019-04-29  2:52 ` [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver Madhavan Srinivasan
@ 2019-04-29  5:42   ` Christophe Leroy
  2019-04-30  6:42     ` Madhavan Srinivasan
  0 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2019-04-29  5:42 UTC (permalink / raw)
  To: Madhavan Srinivasan, mpe; +Cc: linuxppc-dev



Le 29/04/2019 à 04:52, Madhavan Srinivasan a écrit :
> Most of the power processor generation performance monitoring
> unit (PMU) driver code is bundled in the kernel and one of those
> is enabled/registered based on the oprofile_cpu_type check at
> the boot.
> 
> But things get little tricky incase of "compat" mode boot.
> IBM POWER System Server based processors has a compactibility
> mode feature, which simpily put is, Nth generation processor
> (lets say POWER8) will act and appear in a mode consistent
> with an earlier generation (N-1) processor (that is POWER7).
> And in this "compat" mode boot, kernel modify the
> "oprofile_cpu_type" to be Nth generation (POWER8). If Nth
> generation pmu driver is bundled (POWER8), it gets registered.
> 
> Key dependency here is to have distro support for latest
> processor performance monitoring support. Patch here adds
> a generic "compat-mode" performance monitoring driver to
> be register in absence of powernv platform specific pmu driver.
> 
> Driver supports "cycles", "instruction" and "branch-miss" events.
> "0x100F0" used as event code for "cycles", "0x00002"
> used as event code for "instruction" events and "0x400F6"
> used as event code for "branch miss". These are architected events
> as part of ISA. New file called "generic-compat-pmu.c" is
> created to contain the driver specific code. And base raw event
> code format modeled on PPMU_ARCH_207S.
> 
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> Changelog v1:
> - Updated architected event opcodes
> - included branch miss with architected event opcode
> 
>   arch/powerpc/perf/Makefile             |   3 +-
>   arch/powerpc/perf/core-book3s.c        |   2 +-
>   arch/powerpc/perf/generic-compat-pmu.c | 245 +++++++++++++++++++++++++++++++++
>   arch/powerpc/perf/internal.h           |   1 +
>   4 files changed, 249 insertions(+), 2 deletions(-)
>   create mode 100644 arch/powerpc/perf/generic-compat-pmu.c
> 
> diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
> index ab26df5bacb9..c155dcbb8691 100644
> --- a/arch/powerpc/perf/Makefile
> +++ b/arch/powerpc/perf/Makefile
> @@ -5,7 +5,8 @@ obj-$(CONFIG_PERF_EVENTS)	+= callchain.o perf_regs.o
>   obj-$(CONFIG_PPC_PERF_CTRS)	+= core-book3s.o bhrb.o
>   obj64-$(CONFIG_PPC_PERF_CTRS)	+= ppc970-pmu.o power5-pmu.o \
>   				   power5+-pmu.o power6-pmu.o power7-pmu.o \
> -				   isa207-common.o power8-pmu.o power9-pmu.o
> +				   isa207-common.o power8-pmu.o power9-pmu.o \
> +				   generic-compat-pmu.o

Isn't that name a bit long ? What about compat-pmu instead ?

>   obj32-$(CONFIG_PPC_PERF_CTRS)	+= mpc7450-pmu.o
>   
>   obj-$(CONFIG_PPC_POWERNV)	+= imc-pmu.o
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index a96f9420139c..a66fb9c01c9e 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2318,7 +2318,7 @@ static int __init init_ppc64_pmu(void)
>   	else if (!init_ppc970_pmu())
>   		return 0;
>   	else
> -		return -ENODEV;
> +		return init_generic_compat_pmu();
>   }
>   early_initcall(init_ppc64_pmu);
>   #endif
> diff --git a/arch/powerpc/perf/generic-compat-pmu.c b/arch/powerpc/perf/generic-compat-pmu.c
> new file mode 100644
> index 000000000000..9c2d4bbc5c87
> --- /dev/null
> +++ b/arch/powerpc/perf/generic-compat-pmu.c
> @@ -0,0 +1,245 @@
> +/*
> + * Performance counter support.
> + *
> + * Copyright 2019 Madhavan Srinivasan, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or later version.

Shouldn't we use the new licence format for new files ? ie:

// SPDX-License-Identifier: GPL-2.0+

> + */
> +
> +#define pr_fmt(fmt)	"generic-compat-pmu: " fmt
> +
> +#include "isa207-common.h"
> +
> +/*
> + * Raw event encoding:
> + *
> + *        60        56        52        48        44        40        36        32
> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
> + *
> + *        28        24        20        16        12         8         4         0
> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
> + *                                 [ pmc ]   [unit ]   [ ]   m   [    pmcxsel    ]
> + *                                                     |     |
> + *                                                     |     *- mark
> + *                                                     |
> + *                                                     |
> + *                                                     *- combine
> + *
> + * Below uses IBM bit numbering.
> + *
> + * MMCR1[x:y] = unit    (PMCxUNIT)
> + * MMCR1[24]   = pmc1combine[0]
> + * MMCR1[25]   = pmc1combine[1]
> + * MMCR1[26]   = pmc2combine[0]
> + * MMCR1[27]   = pmc2combine[1]
> + * MMCR1[28]   = pmc3combine[0]
> + * MMCR1[29]   = pmc3combine[1]
> + * MMCR1[30]   = pmc4combine[0]
> + * MMCR1[31]   = pmc4combine[1]
> + *
> + */
> +
> +/*
> + * Some power9 event codes.
> + */
> +#define EVENT(_name, _code)	_name = _code,
> +
> +enum {
> +EVENT(PM_CYC,					0x100F0)
> +EVENT(PM_INST_CMPL,				0x00002)
> +EVENT(PM_BR_MPRED_CMPL,				0x400F6)
> +};
> +
> +#undef EVENT
> +
> +GENERIC_EVENT_ATTR(cpu-cycles,			PM_CYC);
> +GENERIC_EVENT_ATTR(instructions,		PM_INST_CMPL);
> +GENERIC_EVENT_ATTR(branch-misses,               PM_BR_MPRED_CMPL);
> +
> +static struct attribute *generic_compat_events_attr[] = {
> +	GENERIC_EVENT_PTR(PM_CYC),
> +	GENERIC_EVENT_PTR(PM_INST_CMPL),
> +	GENERIC_EVENT_PTR(PM_BR_MPRED_CMPL),
> +	NULL
> +};
> +
> +static struct attribute_group generic_compat_pmu_events_group = {
> +	.name = "events",
> +	.attrs = generic_compat_events_attr,
> +};
> +
> +PMU_FORMAT_ATTR(event,		"config:0-19");
> +PMU_FORMAT_ATTR(pmcxsel,	"config:0-7");
> +PMU_FORMAT_ATTR(mark,		"config:8");
> +PMU_FORMAT_ATTR(combine,	"config:10-11");
> +PMU_FORMAT_ATTR(unit,		"config:12-15");
> +PMU_FORMAT_ATTR(pmc,		"config:16-19");
> +
> +static struct attribute *generic_compat_pmu_format_attr[] = {
> +	&format_attr_event.attr,
> +	&format_attr_pmcxsel.attr,
> +	&format_attr_mark.attr,
> +	&format_attr_combine.attr,
> +	&format_attr_unit.attr,
> +	&format_attr_pmc.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group generic_compat_pmu_format_group = {
> +	.name = "format",
> +	.attrs = generic_compat_pmu_format_attr,
> +};
> +
> +static const struct attribute_group *generic_compat_pmu_attr_groups[] = {
> +	&generic_compat_pmu_format_group,
> +	&generic_compat_pmu_events_group,
> +	NULL,
> +};
> +
> +static int compat_generic_events[] = {
> +	[PERF_COUNT_HW_CPU_CYCLES] =			PM_CYC,
> +	[PERF_COUNT_HW_INSTRUCTIONS] =			PM_INST_CMPL,
> +	[PERF_COUNT_HW_BRANCH_MISSES] =                 PM_BR_MPRED_CMPL,
> +};
> +
> +#define C(x)	PERF_COUNT_HW_CACHE_##x
> +
> +/*
> + * Table of generalized cache-related events.
> + * 0 means not supported, -1 means nonsensical, other values
> + * are event codes.
> + */
> +static int generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
> +	[ C(L1D) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +	},
> +	[ C(L1I) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +	},
> +	[ C(LL) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +	},
> +	[ C(DTLB) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +	},
> +	[ C(ITLB) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +	},
> +	[ C(BPU) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = 0,
> +			[ C(RESULT_MISS)   ] = 0,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +	},
> +	[ C(NODE) ] = {
> +		[ C(OP_READ) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +		[ C(OP_WRITE) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +		[ C(OP_PREFETCH) ] = {
> +			[ C(RESULT_ACCESS) ] = -1,
> +			[ C(RESULT_MISS)   ] = -1,
> +		},
> +	},
> +};
> +
> +#undef C
> +
> +static struct power_pmu generic_compat_pmu = {
> +	.name			= "GENERIC_COMPAT",
> +	.n_counter		= MAX_PMU_COUNTERS,
> +	.add_fields		= ISA207_ADD_FIELDS,
> +	.test_adder		= ISA207_TEST_ADDER,
> +	.compute_mmcr		= isa207_compute_mmcr,
> +	.get_constraint		= isa207_get_constraint,
> +	.disable_pmc		= isa207_disable_pmc,
> +	.flags			= PPMU_HAS_SIER | PPMU_ARCH_207S,
> +	.n_generic		= ARRAY_SIZE(compat_generic_events),
> +	.generic_events		= compat_generic_events,
> +	.cache_events		= &generic_compat_cache_events,
> +	.attr_groups		= generic_compat_pmu_attr_groups,
> +};
> +
> +int init_generic_compat_pmu(void)
> +{
> +	int rc = 0;
> +
> +	rc = register_power_pmu(&generic_compat_pmu);
> +	if (rc)
> +		return rc;
> +
> +	/* Tell userspace that EBB is supported */
> +	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_EBB;
> +
> +	return 0;
> +}
> diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
> index e54d524d4283..185a40d1adff 100644
> --- a/arch/powerpc/perf/internal.h
> +++ b/arch/powerpc/perf/internal.h
> @@ -14,3 +14,4 @@ extern int init_power6_pmu(void);
>   extern int init_power7_pmu(void);
>   extern int init_power8_pmu(void);
>   extern int init_power9_pmu(void);
> +extern int init_generic_compat_pmu(void);

Don't use 'extern' keyword.

Christophe


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s
  2019-04-29  5:38 ` [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Christophe Leroy
@ 2019-04-30  6:34   ` Madhavan Srinivasan
  0 siblings, 0 replies; 6+ messages in thread
From: Madhavan Srinivasan @ 2019-04-30  6:34 UTC (permalink / raw)
  To: Christophe Leroy, mpe; +Cc: linuxppc-dev


On 29/04/19 11:08 AM, Christophe Leroy wrote:
>
>
> Le 29/04/2019 à 04:52, Madhavan Srinivasan a écrit :
>> Currenty pmu driver file for each ppc64 generation processor
>> has a __init call in itself. Refactor the code by moving the
>> __init call to core-books.c. This also clean's up compat mode
>> pmu driver registration.
>
> Can you explain the advantage of doing so ?

Was not comfortable having dependency on the link ordering, so
took this approach. This will avoid registering generic driver
when there is a platform specific driver.


> For me it makes more sense to have independant drivers with their own 
> init call.
>
>
>>
>> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>> Changelog v1:
>> - Added "internal.h" file and moved the extern definitions to that file
>>
>>   arch/powerpc/perf/core-book3s.c | 28 ++++++++++++++++++++++++++++
>>   arch/powerpc/perf/internal.h    | 16 ++++++++++++++++
>>   arch/powerpc/perf/power5+-pmu.c |  4 +---
>>   arch/powerpc/perf/power5-pmu.c  |  4 +---
>>   arch/powerpc/perf/power6-pmu.c  |  4 +---
>>   arch/powerpc/perf/power7-pmu.c  |  4 +---
>>   arch/powerpc/perf/power8-pmu.c  |  3 +--
>>   arch/powerpc/perf/power9-pmu.c  |  3 +--
>>   arch/powerpc/perf/ppc970-pmu.c  |  4 +---
>>   9 files changed, 51 insertions(+), 19 deletions(-)
>>   create mode 100644 arch/powerpc/perf/internal.h
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c 
>> b/arch/powerpc/perf/core-book3s.c
>> index b0723002a396..a96f9420139c 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -22,6 +22,10 @@
>>   #include <asm/ptrace.h>
>>   #include <asm/code-patching.h>
>>   +#ifdef CONFIG_PPC64
>
> Can we avoid that CONFIG_PPC64 ifdef ? Why isn't it compatible with 
> PPC32 ?

IIUC, Driver handled here are specific to server side ppc and secondly, 
infrastructure
can be extend for ppc32 if needed.
>> +#include "internal.h"
>> +#endif
>> +
>>   #define BHRB_MAX_ENTRIES    32
>>   #define BHRB_TARGET        0x0000000000000002
>>   #define BHRB_PREDICTION        0x0000000000000001
>> @@ -2294,3 +2298,27 @@ int register_power_pmu(struct power_pmu *pmu)
>>                 power_pmu_prepare_cpu, NULL);
>>       return 0;
>>   }
>> +
>> +#ifdef CONFIG_PPC64
>
> Same, why PPC64 ?
>
>> +static int __init init_ppc64_pmu(void)
>> +{
>> +    /* run through all the pmu drivers one at a time */
>> +    if (!init_power5_pmu())
>> +        return 0;
>> +    else if (!init_power5p_pmu())
>> +        return 0;
>> +    else if (!init_power6_pmu())
>> +        return 0;
>> +    else if (!init_power7_pmu())
>> +        return 0;
>> +    else if (!init_power8_pmu())
>> +        return 0;
>> +    else if (!init_power9_pmu())
>> +        return 0;
>> +    else if (!init_ppc970_pmu())
>> +        return 0;
>> +    else
>> +        return -ENODEV;
>> +}
>> +early_initcall(init_ppc64_pmu);
>> +#endif
>> diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
>> new file mode 100644
>> index 000000000000..e54d524d4283
>> --- /dev/null
>> +++ b/arch/powerpc/perf/internal.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * Copyright 2019 Madhavan Srinivasan, IBM Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +extern int init_ppc970_pmu(void);
>> +extern int init_power5_pmu(void);
>> +extern int init_power5p_pmu(void);
>> +extern int init_power6_pmu(void);
>> +extern int init_power7_pmu(void);
>> +extern int init_power8_pmu(void);
>> +extern int init_power9_pmu(void);
>
> 'extern' keyword is pointless, please remove it (checkpatch --strict 
> probably told it to you).

Ok will re-spin it (will use --strict in future patches thanks :) )

Thanks for review
Maddy

>
>
> Christophe
>
>
>> diff --git a/arch/powerpc/perf/power5+-pmu.c 
>> b/arch/powerpc/perf/power5+-pmu.c
>> index 0526dac66007..9aa803504cb2 100644
>> --- a/arch/powerpc/perf/power5+-pmu.c
>> +++ b/arch/powerpc/perf/power5+-pmu.c
>> @@ -677,7 +677,7 @@ static struct power_pmu power5p_pmu = {
>>       .cache_events        = &power5p_cache_events,
>>   };
>>   -static int __init init_power5p_pmu(void)
>> +int init_power5p_pmu(void)
>>   {
>>       if (!cur_cpu_spec->oprofile_cpu_type ||
>>           (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
>> @@ -686,5 +686,3 @@ static int __init init_power5p_pmu(void)
>>         return register_power_pmu(&power5p_pmu);
>>   }
>> -
>> -early_initcall(init_power5p_pmu);
>> diff --git a/arch/powerpc/perf/power5-pmu.c 
>> b/arch/powerpc/perf/power5-pmu.c
>> index 4dc99f9f7962..30cb13d081a9 100644
>> --- a/arch/powerpc/perf/power5-pmu.c
>> +++ b/arch/powerpc/perf/power5-pmu.c
>> @@ -618,7 +618,7 @@ static struct power_pmu power5_pmu = {
>>       .flags            = PPMU_HAS_SSLOT,
>>   };
>>   -static int __init init_power5_pmu(void)
>> +int init_power5_pmu(void)
>>   {
>>       if (!cur_cpu_spec->oprofile_cpu_type ||
>>           strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
>> @@ -626,5 +626,3 @@ static int __init init_power5_pmu(void)
>>         return register_power_pmu(&power5_pmu);
>>   }
>> -
>> -early_initcall(init_power5_pmu);
>> diff --git a/arch/powerpc/perf/power6-pmu.c 
>> b/arch/powerpc/perf/power6-pmu.c
>> index 9c9d646b68a1..80ec48632cfe 100644
>> --- a/arch/powerpc/perf/power6-pmu.c
>> +++ b/arch/powerpc/perf/power6-pmu.c
>> @@ -540,7 +540,7 @@ static struct power_pmu power6_pmu = {
>>       .cache_events        = &power6_cache_events,
>>   };
>>   -static int __init init_power6_pmu(void)
>> +int init_power6_pmu(void)
>>   {
>>       if (!cur_cpu_spec->oprofile_cpu_type ||
>>           strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
>> @@ -548,5 +548,3 @@ static int __init init_power6_pmu(void)
>>         return register_power_pmu(&power6_pmu);
>>   }
>> -
>> -early_initcall(init_power6_pmu);
>> diff --git a/arch/powerpc/perf/power7-pmu.c 
>> b/arch/powerpc/perf/power7-pmu.c
>> index 6dbae9884ec4..bb6efd5d2530 100644
>> --- a/arch/powerpc/perf/power7-pmu.c
>> +++ b/arch/powerpc/perf/power7-pmu.c
>> @@ -445,7 +445,7 @@ static struct power_pmu power7_pmu = {
>>       .cache_events        = &power7_cache_events,
>>   };
>>   -static int __init init_power7_pmu(void)
>> +int init_power7_pmu(void)
>>   {
>>       if (!cur_cpu_spec->oprofile_cpu_type ||
>>           strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
>> @@ -456,5 +456,3 @@ static int __init init_power7_pmu(void)
>>         return register_power_pmu(&power7_pmu);
>>   }
>> -
>> -early_initcall(init_power7_pmu);
>> diff --git a/arch/powerpc/perf/power8-pmu.c 
>> b/arch/powerpc/perf/power8-pmu.c
>> index d12a2db26353..bcc3409a06de 100644
>> --- a/arch/powerpc/perf/power8-pmu.c
>> +++ b/arch/powerpc/perf/power8-pmu.c
>> @@ -379,7 +379,7 @@ static struct power_pmu power8_pmu = {
>>       .bhrb_nr        = 32,
>>   };
>>   -static int __init init_power8_pmu(void)
>> +int init_power8_pmu(void)
>>   {
>>       int rc;
>>   @@ -399,4 +399,3 @@ static int __init init_power8_pmu(void)
>>         return 0;
>>   }
>> -early_initcall(init_power8_pmu);
>> diff --git a/arch/powerpc/perf/power9-pmu.c 
>> b/arch/powerpc/perf/power9-pmu.c
>> index 030544e35959..3a31ac6f4805 100644
>> --- a/arch/powerpc/perf/power9-pmu.c
>> +++ b/arch/powerpc/perf/power9-pmu.c
>> @@ -437,7 +437,7 @@ static struct power_pmu power9_pmu = {
>>       .bhrb_nr        = 32,
>>   };
>>   -static int __init init_power9_pmu(void)
>> +int init_power9_pmu(void)
>>   {
>>       int rc = 0;
>>       unsigned int pvr = mfspr(SPRN_PVR);
>> @@ -467,4 +467,3 @@ static int __init init_power9_pmu(void)
>>         return 0;
>>   }
>> -early_initcall(init_power9_pmu);
>> diff --git a/arch/powerpc/perf/ppc970-pmu.c 
>> b/arch/powerpc/perf/ppc970-pmu.c
>> index 8b6a8a36fa38..1d3370914022 100644
>> --- a/arch/powerpc/perf/ppc970-pmu.c
>> +++ b/arch/powerpc/perf/ppc970-pmu.c
>> @@ -490,7 +490,7 @@ static struct power_pmu ppc970_pmu = {
>>       .flags            = PPMU_NO_SIPR | PPMU_NO_CONT_SAMPLING,
>>   };
>>   -static int __init init_ppc970_pmu(void)
>> +int init_ppc970_pmu(void)
>>   {
>>       if (!cur_cpu_spec->oprofile_cpu_type ||
>>           (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
>> @@ -499,5 +499,3 @@ static int __init init_ppc970_pmu(void)
>>         return register_power_pmu(&ppc970_pmu);
>>   }
>> -
>> -early_initcall(init_ppc970_pmu);
>>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver
  2019-04-29  5:42   ` Christophe Leroy
@ 2019-04-30  6:42     ` Madhavan Srinivasan
  0 siblings, 0 replies; 6+ messages in thread
From: Madhavan Srinivasan @ 2019-04-30  6:42 UTC (permalink / raw)
  To: linuxppc-dev


On 29/04/19 11:12 AM, Christophe Leroy wrote:
>
>
> Le 29/04/2019 à 04:52, Madhavan Srinivasan a écrit :
>> Most of the power processor generation performance monitoring
>> unit (PMU) driver code is bundled in the kernel and one of those
>> is enabled/registered based on the oprofile_cpu_type check at
>> the boot.
>>
>> But things get little tricky incase of "compat" mode boot.
>> IBM POWER System Server based processors has a compactibility
>> mode feature, which simpily put is, Nth generation processor
>> (lets say POWER8) will act and appear in a mode consistent
>> with an earlier generation (N-1) processor (that is POWER7).
>> And in this "compat" mode boot, kernel modify the
>> "oprofile_cpu_type" to be Nth generation (POWER8). If Nth
>> generation pmu driver is bundled (POWER8), it gets registered.
>>
>> Key dependency here is to have distro support for latest
>> processor performance monitoring support. Patch here adds
>> a generic "compat-mode" performance monitoring driver to
>> be register in absence of powernv platform specific pmu driver.
>>
>> Driver supports "cycles", "instruction" and "branch-miss" events.
>> "0x100F0" used as event code for "cycles", "0x00002"
>> used as event code for "instruction" events and "0x400F6"
>> used as event code for "branch miss". These are architected events
>> as part of ISA. New file called "generic-compat-pmu.c" is
>> created to contain the driver specific code. And base raw event
>> code format modeled on PPMU_ARCH_207S.
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>> Changelog v1:
>> - Updated architected event opcodes
>> - included branch miss with architected event opcode
>>
>>   arch/powerpc/perf/Makefile             |   3 +-
>>   arch/powerpc/perf/core-book3s.c        |   2 +-
>>   arch/powerpc/perf/generic-compat-pmu.c | 245 
>> +++++++++++++++++++++++++++++++++
>>   arch/powerpc/perf/internal.h           |   1 +
>>   4 files changed, 249 insertions(+), 2 deletions(-)
>>   create mode 100644 arch/powerpc/perf/generic-compat-pmu.c
>>
>> diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
>> index ab26df5bacb9..c155dcbb8691 100644
>> --- a/arch/powerpc/perf/Makefile
>> +++ b/arch/powerpc/perf/Makefile
>> @@ -5,7 +5,8 @@ obj-$(CONFIG_PERF_EVENTS)    += callchain.o perf_regs.o
>>   obj-$(CONFIG_PPC_PERF_CTRS)    += core-book3s.o bhrb.o
>>   obj64-$(CONFIG_PPC_PERF_CTRS)    += ppc970-pmu.o power5-pmu.o \
>>                      power5+-pmu.o power6-pmu.o power7-pmu.o \
>> -                   isa207-common.o power8-pmu.o power9-pmu.o
>> +                   isa207-common.o power8-pmu.o power9-pmu.o \
>> +                   generic-compat-pmu.o
>
> Isn't that name a bit long ? What about compat-pmu instead ?

yeah I guess. Will fix it.

>
>>   obj32-$(CONFIG_PPC_PERF_CTRS)    += mpc7450-pmu.o
>>     obj-$(CONFIG_PPC_POWERNV)    += imc-pmu.o
>> diff --git a/arch/powerpc/perf/core-book3s.c 
>> b/arch/powerpc/perf/core-book3s.c
>> index a96f9420139c..a66fb9c01c9e 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -2318,7 +2318,7 @@ static int __init init_ppc64_pmu(void)
>>       else if (!init_ppc970_pmu())
>>           return 0;
>>       else
>> -        return -ENODEV;
>> +        return init_generic_compat_pmu();
>>   }
>>   early_initcall(init_ppc64_pmu);
>>   #endif
>> diff --git a/arch/powerpc/perf/generic-compat-pmu.c 
>> b/arch/powerpc/perf/generic-compat-pmu.c
>> new file mode 100644
>> index 000000000000..9c2d4bbc5c87
>> --- /dev/null
>> +++ b/arch/powerpc/perf/generic-compat-pmu.c
>> @@ -0,0 +1,245 @@
>> +/*
>> + * Performance counter support.
>> + *
>> + * Copyright 2019 Madhavan Srinivasan, IBM Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or later version.
>
> Shouldn't we use the new licence format for new files ? ie:
>
> // SPDX-License-Identifier: GPL-2.0+

My bad. Thanks for pointing out.
Will fix and re-spin.

Thanks for review
Maddy


>
>> + */
>> +
>> +#define pr_fmt(fmt)    "generic-compat-pmu: " fmt
>> +
>> +#include "isa207-common.h"
>> +
>> +/*
>> + * Raw event encoding:
>> + *
>> + *        60        56        52        48        44 40        
>> 36        32
>> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - 
>> - - | - - - - |
>> + *
>> + *        28        24        20        16        12 8         
>> 4         0
>> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - 
>> - - | - - - - |
>> + *                                 [ pmc ]   [unit ]   [ ] m   [    
>> pmcxsel    ]
>> + *                                                     |     |
>> + *                                                     |     *- mark
>> + *                                                     |
>> + *                                                     |
>> + *                                                     *- combine
>> + *
>> + * Below uses IBM bit numbering.
>> + *
>> + * MMCR1[x:y] = unit    (PMCxUNIT)
>> + * MMCR1[24]   = pmc1combine[0]
>> + * MMCR1[25]   = pmc1combine[1]
>> + * MMCR1[26]   = pmc2combine[0]
>> + * MMCR1[27]   = pmc2combine[1]
>> + * MMCR1[28]   = pmc3combine[0]
>> + * MMCR1[29]   = pmc3combine[1]
>> + * MMCR1[30]   = pmc4combine[0]
>> + * MMCR1[31]   = pmc4combine[1]
>> + *
>> + */
>> +
>> +/*
>> + * Some power9 event codes.
>> + */
>> +#define EVENT(_name, _code)    _name = _code,
>> +
>> +enum {
>> +EVENT(PM_CYC,                    0x100F0)
>> +EVENT(PM_INST_CMPL,                0x00002)
>> +EVENT(PM_BR_MPRED_CMPL,                0x400F6)
>> +};
>> +
>> +#undef EVENT
>> +
>> +GENERIC_EVENT_ATTR(cpu-cycles,            PM_CYC);
>> +GENERIC_EVENT_ATTR(instructions,        PM_INST_CMPL);
>> +GENERIC_EVENT_ATTR(branch-misses, PM_BR_MPRED_CMPL);
>> +
>> +static struct attribute *generic_compat_events_attr[] = {
>> +    GENERIC_EVENT_PTR(PM_CYC),
>> +    GENERIC_EVENT_PTR(PM_INST_CMPL),
>> +    GENERIC_EVENT_PTR(PM_BR_MPRED_CMPL),
>> +    NULL
>> +};
>> +
>> +static struct attribute_group generic_compat_pmu_events_group = {
>> +    .name = "events",
>> +    .attrs = generic_compat_events_attr,
>> +};
>> +
>> +PMU_FORMAT_ATTR(event,        "config:0-19");
>> +PMU_FORMAT_ATTR(pmcxsel,    "config:0-7");
>> +PMU_FORMAT_ATTR(mark,        "config:8");
>> +PMU_FORMAT_ATTR(combine,    "config:10-11");
>> +PMU_FORMAT_ATTR(unit,        "config:12-15");
>> +PMU_FORMAT_ATTR(pmc,        "config:16-19");
>> +
>> +static struct attribute *generic_compat_pmu_format_attr[] = {
>> +    &format_attr_event.attr,
>> +    &format_attr_pmcxsel.attr,
>> +    &format_attr_mark.attr,
>> +    &format_attr_combine.attr,
>> +    &format_attr_unit.attr,
>> +    &format_attr_pmc.attr,
>> +    NULL,
>> +};
>> +
>> +static struct attribute_group generic_compat_pmu_format_group = {
>> +    .name = "format",
>> +    .attrs = generic_compat_pmu_format_attr,
>> +};
>> +
>> +static const struct attribute_group 
>> *generic_compat_pmu_attr_groups[] = {
>> +    &generic_compat_pmu_format_group,
>> +    &generic_compat_pmu_events_group,
>> +    NULL,
>> +};
>> +
>> +static int compat_generic_events[] = {
>> +    [PERF_COUNT_HW_CPU_CYCLES] =            PM_CYC,
>> +    [PERF_COUNT_HW_INSTRUCTIONS] =            PM_INST_CMPL,
>> +    [PERF_COUNT_HW_BRANCH_MISSES] = PM_BR_MPRED_CMPL,
>> +};
>> +
>> +#define C(x)    PERF_COUNT_HW_CACHE_##x
>> +
>> +/*
>> + * Table of generalized cache-related events.
>> + * 0 means not supported, -1 means nonsensical, other values
>> + * are event codes.
>> + */
>> +static int 
>> generic_compat_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
>> +    [ C(L1D) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +    },
>> +    [ C(L1I) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +    },
>> +    [ C(LL) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +    },
>> +    [ C(DTLB) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +    },
>> +    [ C(ITLB) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +    },
>> +    [ C(BPU) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = 0,
>> +            [ C(RESULT_MISS)   ] = 0,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +    },
>> +    [ C(NODE) ] = {
>> +        [ C(OP_READ) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +        [ C(OP_WRITE) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +        [ C(OP_PREFETCH) ] = {
>> +            [ C(RESULT_ACCESS) ] = -1,
>> +            [ C(RESULT_MISS)   ] = -1,
>> +        },
>> +    },
>> +};
>> +
>> +#undef C
>> +
>> +static struct power_pmu generic_compat_pmu = {
>> +    .name            = "GENERIC_COMPAT",
>> +    .n_counter        = MAX_PMU_COUNTERS,
>> +    .add_fields        = ISA207_ADD_FIELDS,
>> +    .test_adder        = ISA207_TEST_ADDER,
>> +    .compute_mmcr        = isa207_compute_mmcr,
>> +    .get_constraint        = isa207_get_constraint,
>> +    .disable_pmc        = isa207_disable_pmc,
>> +    .flags            = PPMU_HAS_SIER | PPMU_ARCH_207S,
>> +    .n_generic        = ARRAY_SIZE(compat_generic_events),
>> +    .generic_events        = compat_generic_events,
>> +    .cache_events        = &generic_compat_cache_events,
>> +    .attr_groups        = generic_compat_pmu_attr_groups,
>> +};
>> +
>> +int init_generic_compat_pmu(void)
>> +{
>> +    int rc = 0;
>> +
>> +    rc = register_power_pmu(&generic_compat_pmu);
>> +    if (rc)
>> +        return rc;
>> +
>> +    /* Tell userspace that EBB is supported */
>> +    cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_EBB;
>> +
>> +    return 0;
>> +}
>> diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
>> index e54d524d4283..185a40d1adff 100644
>> --- a/arch/powerpc/perf/internal.h
>> +++ b/arch/powerpc/perf/internal.h
>> @@ -14,3 +14,4 @@ extern int init_power6_pmu(void);
>>   extern int init_power7_pmu(void);
>>   extern int init_power8_pmu(void);
>>   extern int init_power9_pmu(void);
>> +extern int init_generic_compat_pmu(void);
>
> Don't use 'extern' keyword.
>
> Christophe
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-04-30  6:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29  2:52 [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Madhavan Srinivasan
2019-04-29  2:52 ` [PATCH v2 2/2] powerpc/perf: Add generic compat mode pmu driver Madhavan Srinivasan
2019-04-29  5:42   ` Christophe Leroy
2019-04-30  6:42     ` Madhavan Srinivasan
2019-04-29  5:38 ` [PATCH v2 1/2] powerpc/perf: init pmu from core-book3s Christophe Leroy
2019-04-30  6:34   ` Madhavan Srinivasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.