[PATCH v6 00/11] Enable PMUs in ACPI systems

[PATCH v6 00/11] Enable PMUs in ACPI systems

[Jeremy Linton]

v6:
Added cpu affinity sysfs entry
Converted pmu_types array, to linked list
Restrict use of the armv8_pmu_probe_table to ACPI systems
Rename MADT parsing routines in smp.c
Convert sysfs PMU name to use index rather than partnum
Remove pr_devel statements
Other Minor cleanups
Add Partial Ack-by Will Deacon

v5:
Remove list of CPU types for ACPI systems. We now match a generic
event list, and use the PMCIED[01] to select events which exist on
the given PMU. This avoids the need to update the kernel every time
a new CPU is released.
Update the maintainers list to include the new file.

v4:
Correct build issues with ARM (!ARM64) kernels.
Add ThunderX to list of PMU types.

v3:
Enable ARM performance monitoring units on ACPI/arm64 machines.

This patch expands and reworks the patches published by Mark Salter
in order to clean up a few of the previous review comments, as well as
add support for newer CPUs and big/little configurations.


Jeremy Linton (9):
  arm64: pmu: Probe default hw/cache counters
  arm64: pmu: Hoist pmu platform device name
  arm64: Rename the common MADT parse routine
  arm: arm64: Add routine to determine cpuid of other cpus
  arm: arm64: pmu: Assign platform PMU CPU affinity
  arm64: pmu: Provide cpumask attribute for PMU
  arm64: pmu: Add routines for detecting differing PMU types in the
    system
  arm64: pmu: Enable multiple PMUs in an ACPI system
  MAINTAINERS: Tweak ARM PMU maintainers

Mark Salter (2):
  arm64: pmu: add fallback probe table
  arm64: pmu: Add support for probing with ACPI

 MAINTAINERS                      |   3 +-
 arch/arm/include/asm/cputype.h   |   6 +-
 arch/arm64/include/asm/cputype.h |   4 +
 arch/arm64/kernel/perf_event.c   |  79 ++++++++++++++-
 arch/arm64/kernel/smp.c          |  18 ++--
 drivers/perf/Kconfig             |   4 +
 drivers/perf/Makefile            |   1 +
 drivers/perf/arm_pmu.c           |  60 +++++++++---
 drivers/perf/arm_pmu_acpi.c      | 207 +++++++++++++++++++++++++++++++++++++++
 include/linux/perf/arm_pmu.h     |  12 +++
 10 files changed, 370 insertions(+), 24 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c

-- 
2.5.5

[PATCH 01/11] arm64: pmu: add fallback probe table

[Jeremy Linton]

From: Mark Salter <msalter@redhat.com>

In preparation for ACPI support, add a pmu_probe_info table to
the arm_pmu_device_probe() call. This table gets used when
probing in the absence of a devicetree node for PMU.

Signed-off-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/perf_event.c | 13 ++++++++++++-
 drivers/perf/arm_pmu.c         |  2 +-
 include/linux/perf/arm_pmu.h   |  3 +++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 838ccf1..3aac598 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -24,6 +24,7 @@
 #include <asm/sysreg.h>
 #include <asm/virt.h>
 
+#include <linux/acpi.h>
 #include <linux/of.h>
 #include <linux/perf/arm_pmu.h>
 #include <linux/platform_device.h>
@@ -1044,9 +1045,19 @@ static const struct of_device_id armv8_pmu_of_device_ids[] = {
 	{},
 };
 
+static const struct pmu_probe_info armv8_pmu_probe_table[] = {
+	PMU_PROBE(0, 0, armv8_pmuv3_init), /* if all else fails... */
+	{ /* sentinel value */ }
+};
+
 static int armv8_pmu_device_probe(struct platform_device *pdev)
 {
-	return arm_pmu_device_probe(pdev, armv8_pmu_of_device_ids, NULL);
+	if (acpi_disabled)
+		return arm_pmu_device_probe(pdev, armv8_pmu_of_device_ids,
+					    NULL);
+
+	return arm_pmu_device_probe(pdev, armv8_pmu_of_device_ids,
+				    armv8_pmu_probe_table);
 }
 
 static struct platform_driver armv8_pmu_driver = {
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 140436a..2286cbf 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -1009,7 +1009,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		ret = of_pmu_irq_cfg(pmu);
 		if (!ret)
 			ret = init_fn(pmu);
-	} else {
+	} else if (probe_table) {
 		cpumask_setall(&pmu->supported_cpus);
 		ret = probe_current_pmu(pmu, probe_table);
 	}
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d28ac05..7e814fe 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -147,6 +147,9 @@ struct pmu_probe_info {
 #define XSCALE_PMU_PROBE(_version, _fn) \
 	PMU_PROBE(ARM_CPU_IMP_INTEL << 24 | _version, ARM_PMU_XSCALE_MASK, _fn)
 
+#define ARMV8_PMU_PART_PROBE(_part, _fn) \
+	PMU_PROBE((_part) << MIDR_PARTNUM_SHIFT, MIDR_PARTNUM_MASK, _fn)
+
 int arm_pmu_device_probe(struct platform_device *pdev,
 			 const struct of_device_id *of_table,
 			 const struct pmu_probe_info *probe_table);
-- 
2.5.5

[PATCH 02/11] arm64: pmu: Probe default hw/cache counters

[Jeremy Linton]

ARMv8 machines can identify the micro/arch defined counters
that are available on a machine. Add all these counters to the
default armv8 perf map. At run-time disable the counters which
are not available on the given PMU.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/perf_event.c | 45 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 3aac598..f650548 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -191,13 +191,23 @@
 #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_MISS		0xED
 
 /* PMUv3 HW events mapping. */
+
+/*
+ * ARMv8 Architectural defined events, not all of these may
+ * be supported on any given implementation. Undefined events will
+ * be disabled at run-time.
+ */
 static const unsigned armv8_pmuv3_perf_map[PERF_COUNT_HW_MAX] = {
 	PERF_MAP_ALL_UNSUPPORTED,
 	[PERF_COUNT_HW_CPU_CYCLES]		= ARMV8_PMUV3_PERFCTR_CPU_CYCLES,
 	[PERF_COUNT_HW_INSTRUCTIONS]		= ARMV8_PMUV3_PERFCTR_INST_RETIRED,
 	[PERF_COUNT_HW_CACHE_REFERENCES]	= ARMV8_PMUV3_PERFCTR_L1D_CACHE,
 	[PERF_COUNT_HW_CACHE_MISSES]		= ARMV8_PMUV3_PERFCTR_L1D_CACHE_REFILL,
+	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS]	= ARMV8_PMUV3_PERFCTR_PC_WRITE_RETIRED,
 	[PERF_COUNT_HW_BRANCH_MISSES]		= ARMV8_PMUV3_PERFCTR_BR_MIS_PRED,
+	[PERF_COUNT_HW_BUS_CYCLES]		= ARMV8_PMUV3_PERFCTR_BUS_CYCLES,
+	[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND]	= ARMV8_PMUV3_PERFCTR_STALL_FRONTEND,
+	[PERF_COUNT_HW_STALLED_CYCLES_BACKEND]	= ARMV8_PMUV3_PERFCTR_STALL_BACKEND,
 };
 
 /* ARM Cortex-A53 HW events mapping. */
@@ -259,6 +269,15 @@ static const unsigned armv8_pmuv3_perf_cache_map[PERF_COUNT_HW_CACHE_MAX]
 	[C(L1D)][C(OP_WRITE)][C(RESULT_ACCESS)]	= ARMV8_PMUV3_PERFCTR_L1D_CACHE,
 	[C(L1D)][C(OP_WRITE)][C(RESULT_MISS)]	= ARMV8_PMUV3_PERFCTR_L1D_CACHE_REFILL,
 
+	[C(L1I)][C(OP_READ)][C(RESULT_ACCESS)]	= ARMV8_PMUV3_PERFCTR_L1I_CACHE,
+	[C(L1I)][C(OP_READ)][C(RESULT_MISS)]	= ARMV8_PMUV3_PERFCTR_L1I_CACHE_REFILL,
+
+	[C(DTLB)][C(OP_READ)][C(RESULT_MISS)]	= ARMV8_PMUV3_PERFCTR_L1D_TLB_REFILL,
+	[C(DTLB)][C(OP_READ)][C(RESULT_ACCESS)]	= ARMV8_PMUV3_PERFCTR_L1D_TLB,
+
+	[C(ITLB)][C(OP_READ)][C(RESULT_MISS)]	= ARMV8_PMUV3_PERFCTR_L1I_TLB_REFILL,
+	[C(ITLB)][C(OP_READ)][C(RESULT_ACCESS)]	= ARMV8_PMUV3_PERFCTR_L1I_TLB,
+
 	[C(BPU)][C(OP_READ)][C(RESULT_ACCESS)]	= ARMV8_PMUV3_PERFCTR_BR_PRED,
 	[C(BPU)][C(OP_READ)][C(RESULT_MISS)]	= ARMV8_PMUV3_PERFCTR_BR_MIS_PRED,
 	[C(BPU)][C(OP_WRITE)][C(RESULT_ACCESS)]	= ARMV8_PMUV3_PERFCTR_BR_PRED,
@@ -906,9 +925,22 @@ static void armv8pmu_reset(void *info)
 
 static int armv8_pmuv3_map_event(struct perf_event *event)
 {
-	return armpmu_map_event(event, &armv8_pmuv3_perf_map,
-				&armv8_pmuv3_perf_cache_map,
-				ARMV8_PMU_EVTYPE_EVENT);
+	int hw_event_id;
+	struct arm_pmu *armpmu = to_arm_pmu(event->pmu);
+
+	hw_event_id = armpmu_map_event(event, &armv8_pmuv3_perf_map,
+				       &armv8_pmuv3_perf_cache_map,
+				       ARMV8_PMU_EVTYPE_EVENT);
+	if (hw_event_id < 0)
+		return hw_event_id;
+
+	/* disable micro/arch events not supported by this PMU */
+	if ((hw_event_id < ARMV8_PMUV3_MAX_COMMON_EVENTS) &&
+		!test_bit(hw_event_id, armpmu->pmceid_bitmap)) {
+			return -EOPNOTSUPP;
+	}
+
+	return hw_event_id;
 }
 
 static int armv8_a53_map_event(struct perf_event *event)
@@ -1045,8 +1077,13 @@ static const struct of_device_id armv8_pmu_of_device_ids[] = {
 	{},
 };
 
+/*
+ * Non DT systems have their micro/arch events probed at run-time.
+ * A fairly complete list of generic events are provided and ones that
+ * aren't supported by the current PMU are disabled.
+ */
 static const struct pmu_probe_info armv8_pmu_probe_table[] = {
-	PMU_PROBE(0, 0, armv8_pmuv3_init), /* if all else fails... */
+	PMU_PROBE(0, 0, armv8_pmuv3_init), /* enable all defined counters */
 	{ /* sentinel value */ }
 };
 
-- 
2.5.5

[PATCH 03/11] arm64: pmu: Hoist pmu platform device name

[Jeremy Linton]

Move the PMU name into a common header file so it may
be referenced by other users.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/perf_event.c | 2 +-
 include/linux/perf/arm_pmu.h   | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index f650548..356fa6c 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1099,7 +1099,7 @@ static int armv8_pmu_device_probe(struct platform_device *pdev)
 
 static struct platform_driver armv8_pmu_driver = {
 	.driver		= {
-		.name	= "armv8-pmu",
+		.name	= ARMV8_PMU_PDEV_NAME,
 		.of_match_table = armv8_pmu_of_device_ids,
 	},
 	.probe		= armv8_pmu_device_probe,
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 7e814fe..0ef963b 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -154,6 +154,8 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 			 const struct of_device_id *of_table,
 			 const struct pmu_probe_info *probe_table);
 
+#define ARMV8_PMU_PDEV_NAME "armv8-pmu"
+
 #endif /* CONFIG_ARM_PMU */
 
 #endif /* __ARM_PMU_H__ */
-- 
2.5.5

[PATCH 04/11] arm64: Rename the common MADT parse routine

[Jeremy Linton]

The MADT parser in smp.c is now being used to parse
out NUMA, PMU and ACPI parking protocol information as
well as the GIC information for which it was originally
created. Rename it to avoid a misleading name.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/smp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index d099306..319f48b 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -507,13 +507,14 @@ static unsigned int cpu_count = 1;
 
 #ifdef CONFIG_ACPI
 /*
- * acpi_map_gic_cpu_interface - parse processor MADT entry
+ * acpi_verify_and_map_madt - parse processor MADT entry
  *
  * Carry out sanity checks on MADT processor entry and initialize
- * cpu_logical_map on success
+ * cpu_logical_map, the ACPI parking protocol, NUMA mapping
+ * and the PMU interrupts on success
  */
 static void __init
-acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
+acpi_verify_and_map_madt(struct acpi_madt_generic_interrupt *processor)
 {
 	u64 hwid = processor->arm_mpidr;
 
@@ -566,7 +567,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 }
 
 static int __init
-acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
+acpi_parse_madt_common(struct acpi_subtable_header *header,
 			     const unsigned long end)
 {
 	struct acpi_madt_generic_interrupt *processor;
@@ -577,7 +578,7 @@ acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
 
 	acpi_table_print_madt_entry(header);
 
-	acpi_map_gic_cpu_interface(processor);
+	acpi_verify_and_map_madt(processor);
 
 	return 0;
 }
@@ -660,7 +661,7 @@ void __init smp_init_cpus(void)
 		 * we need for SMP init
 		 */
 		acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
-				      acpi_parse_gic_cpu_interface, 0);
+				      acpi_parse_madt_common, 0);
 
 	if (cpu_count > NR_CPUS)
 		pr_warn("no. of cores (%d) greater than configured maximum of %d - clipping\n",
-- 
2.5.5

[PATCH 05/11] arm64: pmu: Add support for probing with ACPI

[Jeremy Linton]

From: Mark Salter <msalter@redhat.com>

In the case of ACPI, the PMU IRQ information is contained in the
MADT table. Also, since the PMU does not exist as a device in the
ACPI DSDT table, it is necessary to create a platform device so
that the appropriate driver probing is triggered.

Signed-off-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---

NOTE: Much of the code in pmu_acpi_init() is replaced in patches 0009 and
      0010 this set. The later version of the patch cleans up most of the
      possible style/error handling issues that have been pointed out with
      this version.

 arch/arm64/kernel/smp.c      |   5 +++
 drivers/perf/Kconfig         |   4 ++
 drivers/perf/Makefile        |   1 +
 drivers/perf/arm_pmu_acpi.c  | 100 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/perf/arm_pmu.h |   7 +++
 5 files changed, 117 insertions(+)
 create mode 100644 drivers/perf/arm_pmu_acpi.c

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 319f48b..49ec927 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -37,6 +37,7 @@
 #include <linux/completion.h>
 #include <linux/of.h>
 #include <linux/irq_work.h>
+#include <linux/perf/arm_pmu.h>
 
 #include <asm/alternative.h>
 #include <asm/atomic.h>
@@ -541,6 +542,7 @@ acpi_verify_and_map_madt(struct acpi_madt_generic_interrupt *processor)
 			return;
 		}
 		bootcpu_valid = true;
+		arm_pmu_parse_acpi(0, processor);
 		return;
 	}
 
@@ -561,6 +563,9 @@ acpi_verify_and_map_madt(struct acpi_madt_generic_interrupt *processor)
 	 */
 	acpi_set_mailbox_entry(cpu_count, processor);
 
+	/* get PMU irq info */
+	arm_pmu_parse_acpi(cpu_count, processor);
+
 	early_map_cpu_to_node(cpu_count, acpi_numa_get_nid(cpu_count, hwid));
 
 	cpu_count++;
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 04e2653..818fa3b 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -12,4 +12,8 @@ config ARM_PMU
 	  Say y if you want to use CPU performance monitors on ARM-based
 	  systems.
 
+config ARM_PMU_ACPI
+	def_bool y
+	depends on ARM_PMU && ACPI
+
 endmenu
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index acd2397..fd8090d 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ARM_PMU) += arm_pmu.o
+obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
new file mode 100644
index 0000000..a24cdd0
--- /dev/null
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -0,0 +1,100 @@
+/*
+ * PMU support
+ *
+ * Copyright (C) 2015 Red Hat Inc.
+ * Author: Mark Salter <msalter@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <linux/perf/arm_pmu.h>
+#include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+
+struct pmu_irq {
+	int gsi;
+	int trigger;
+};
+
+static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
+
+/*
+ * Called from acpi_map_gic_cpu_interface()'s MADT parsing during boot.
+ * This routine saves off the GSI's and their trigger state for use when we are
+ * ready to build the PMU platform device.
+*/
+void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
+{
+	pmu_irqs[cpu].gsi = gic->performance_interrupt;
+	if (gic->flags & ACPI_MADT_PERFORMANCE_IRQ_MODE)
+		pmu_irqs[cpu].trigger = ACPI_EDGE_SENSITIVE;
+	else
+		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
+}
+
+static int __init pmu_acpi_init(void)
+{
+	struct platform_device *pdev;
+	struct pmu_irq *pirq = pmu_irqs;
+	struct resource	*res, *r;
+	int err = -ENOMEM;
+	int i, count, irq;
+
+	if (acpi_disabled)
+		return 0;
+
+	/* Must have irq for boot cpu, at least */
+	if (pirq->gsi == 0)
+		return -EINVAL;
+
+	irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
+				ACPI_ACTIVE_HIGH);
+
+	if (irq_is_percpu(irq))
+		count = 1;
+	else
+		for (i = 1, count = 1; i < NR_CPUS; i++)
+			if (pmu_irqs[i].gsi)
+				++count;
+
+	pdev = platform_device_alloc(ARMV8_PMU_PDEV_NAME, -1);
+	if (!pdev)
+		goto err_free_gsi;
+
+	res = kcalloc(count, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		goto err_free_device;
+
+	for (i = 0, r = res; i < count; i++, pirq++, r++) {
+		if (i)
+			irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
+						ACPI_ACTIVE_HIGH);
+		r->start = r->end = irq;
+		r->flags = IORESOURCE_IRQ;
+		if (pirq->trigger == ACPI_EDGE_SENSITIVE)
+			r->flags |= IORESOURCE_IRQ_HIGHEDGE;
+		else
+			r->flags |= IORESOURCE_IRQ_HIGHLEVEL;
+	}
+
+	err = platform_device_add_resources(pdev, res, count);
+	if (!err)
+		err = platform_device_add(pdev);
+	kfree(res);
+	if (!err)
+		return 0;
+
+err_free_device:
+	platform_device_put(pdev);
+
+err_free_gsi:
+	for (i = 0; i < count; i++)
+		acpi_unregister_gsi(pmu_irqs[i].gsi);
+
+	return err;
+}
+arch_initcall(pmu_acpi_init);
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 0ef963b..0e86dfb 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -158,4 +158,11 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 
 #endif /* CONFIG_ARM_PMU */
 
+#ifdef CONFIG_ARM_PMU_ACPI
+struct acpi_madt_generic_interrupt;
+void arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic);
+#else
+#define arm_pmu_parse_acpi(a, b) do { } while (0)
+#endif /* CONFIG_ARM_PMU_ACPI */
+
 #endif /* __ARM_PMU_H__ */
-- 
2.5.5

[PATCH 05/11] arm64: pmu: Add support for probing with ACPI

[Will Deacon]

On Tue, Jun 21, 2016 at 12:11:43PM -0500, Jeremy Linton wrote:
> From: Mark Salter <msalter@redhat.com>
> 
> In the case of ACPI, the PMU IRQ information is contained in the
> MADT table. Also, since the PMU does not exist as a device in the
> ACPI DSDT table, it is necessary to create a platform device so
> that the appropriate driver probing is triggered.
> 
> Signed-off-by: Mark Salter <msalter@redhat.com>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> 
> NOTE: Much of the code in pmu_acpi_init() is replaced in patches 0009 and
>       0010 this set. The later version of the patch cleans up most of the
>       possible style/error handling issues that have been pointed out with
>       this version.

I agree with Punit that it would be a lot easier to review this series
if you could fold in the changes to pmu_acpi_init so the sum of the
changes can be viewed in one patch.

Will

[PATCH 06/11] arm: arm64: Add routine to determine cpuid of other cpus

[Jeremy Linton]

It is helpful if we can read the cpuid/midr of other CPUs
in the system independent of arm/arm64.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm/include/asm/cputype.h   | 6 +++++-
 arch/arm64/include/asm/cputype.h | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index 1ee94c7..e391b67 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -81,6 +81,8 @@
 #define ARM_CPU_XSCALE_ARCH_V2		0x4000
 #define ARM_CPU_XSCALE_ARCH_V3		0x6000
 
+#define ARM_PARTNUM(cpuid_id) (cpuid_id & ARM_CPU_PART_MASK)
+
 extern unsigned int processor_id;
 
 #ifdef CONFIG_CPU_CP15
@@ -180,7 +182,7 @@ static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
  */
 static inline unsigned int __attribute_const__ read_cpuid_part(void)
 {
-	return read_cpuid_id() & ARM_CPU_PART_MASK;
+	return ARM_PARTNUM(read_cpuid_id());
 }
 
 static inline unsigned int __attribute_const__ __deprecated read_cpuid_part_number(void)
@@ -208,6 +210,8 @@ static inline unsigned int __attribute_const__ read_cpuid_mpidr(void)
 	return read_cpuid(CPUID_MPIDR);
 }
 
+#define read_specific_cpuid(cpu_num) per_cpu_ptr(&cpu_data, cpu_num)->cpuid
+
 /*
  * Intel's XScale3 core supports some v6 features (supersections, L2)
  * but advertises itself as v5 as it does not support the v6 ISA.  For
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 87e1985..56fd8c1 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -38,6 +38,7 @@
 #define MIDR_PARTNUM_MASK	(0xfff << MIDR_PARTNUM_SHIFT)
 #define MIDR_PARTNUM(midr)	\
 	(((midr) & MIDR_PARTNUM_MASK) >> MIDR_PARTNUM_SHIFT)
+#define ARM_PARTNUM MIDR_PARTNUM
 #define MIDR_ARCHITECTURE_SHIFT	16
 #define MIDR_ARCHITECTURE_MASK	(0xf << MIDR_ARCHITECTURE_SHIFT)
 #define MIDR_ARCHITECTURE(midr)	\
@@ -126,6 +127,9 @@ static inline u32 __attribute_const__ read_cpuid_cachetype(void)
 {
 	return read_cpuid(CTR_EL0);
 }
+
+#define read_specific_cpuid(cpu_num) per_cpu_ptr(&cpu_data, cpu_num)->reg_midr
+
 #endif /* __ASSEMBLY__ */
 
 #endif
-- 
2.5.5

[PATCH 06/11] arm: arm64: Add routine to determine cpuid of other cpus

[Will Deacon]

On Tue, Jun 21, 2016 at 12:11:44PM -0500, Jeremy Linton wrote:
> It is helpful if we can read the cpuid/midr of other CPUs
> in the system independent of arm/arm64.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  arch/arm/include/asm/cputype.h   | 6 +++++-
>  arch/arm64/include/asm/cputype.h | 4 ++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index 1ee94c7..e391b67 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -81,6 +81,8 @@
>  #define ARM_CPU_XSCALE_ARCH_V2		0x4000
>  #define ARM_CPU_XSCALE_ARCH_V3		0x6000
>  
> +#define ARM_PARTNUM(cpuid_id) (cpuid_id & ARM_CPU_PART_MASK)
> +
>  extern unsigned int processor_id;
>  
>  #ifdef CONFIG_CPU_CP15
> @@ -180,7 +182,7 @@ static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
>   */
>  static inline unsigned int __attribute_const__ read_cpuid_part(void)
>  {
> -	return read_cpuid_id() & ARM_CPU_PART_MASK;
> +	return ARM_PARTNUM(read_cpuid_id());

I don't understand why you need to make this change.

Will

[PATCH 06/11] arm: arm64: Add routine to determine cpuid of other cpus

[Jeremy Linton]

On 07/06/2016 11:30 AM, Will Deacon wrote:
> On Tue, Jun 21, 2016 at 12:11:44PM -0500, Jeremy Linton wrote:
>> It is helpful if we can read the cpuid/midr of other CPUs
>> in the system independent of arm/arm64.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   arch/arm/include/asm/cputype.h   | 6 +++++-
>>   arch/arm64/include/asm/cputype.h | 4 ++++
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>> index 1ee94c7..e391b67 100644
>> --- a/arch/arm/include/asm/cputype.h
>> +++ b/arch/arm/include/asm/cputype.h
>> @@ -81,6 +81,8 @@
>>   #define ARM_CPU_XSCALE_ARCH_V2		0x4000
>>   #define ARM_CPU_XSCALE_ARCH_V3		0x6000
>>
>> +#define ARM_PARTNUM(cpuid_id) (cpuid_id & ARM_CPU_PART_MASK)
>> +
>>   extern unsigned int processor_id;
>>
>>   #ifdef CONFIG_CPU_CP15
>> @@ -180,7 +182,7 @@ static inline unsigned int __attribute_const__ read_cpuid_implementor(void)
>>    */
>>   static inline unsigned int __attribute_const__ read_cpuid_part(void)
>>   {
>> -	return read_cpuid_id() & ARM_CPU_PART_MASK;
>> +	return ARM_PARTNUM(read_cpuid_id());
>
> I don't understand why you need to make this change.

The short answer is that the ARM_PARTNUM stuff is left over from v4 (?) 
of the patch, where it seemed a good idea to create a macro that was 
arm/arm64 independent for use in arm_pmu.c. Somewhere along there I 
reverted the ARM_PARTNUM to MIDR_PARTNUM in the arm_pmu_acpi.c but 
didn't drop that portion from this patch. Partially because it seems 
like a good idea. OTOH, your right probably doesn't belong here without 
the large cleanup which would form their own patch set.

[PATCH 07/11] arm: arm64: pmu: Assign platform PMU CPU affinity

[Jeremy Linton]

On systems with multiple PMU types the PMU to CPU affinity
needs to be detected and set. The CPU to interrupt affinity
should also be set.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/perf/arm_pmu.c | 53 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 2286cbf..28cac3a 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -11,6 +11,7 @@
  */
 #define pr_fmt(fmt) "hw perfevents: " fmt
 
+#include <linux/acpi.h>
 #include <linux/bitmap.h>
 #include <linux/cpumask.h>
 #include <linux/cpu_pm.h>
@@ -24,6 +25,7 @@
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
 
+#include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/irq_regs.h>
 
@@ -872,25 +874,57 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 }
 
 /*
- * CPU PMU identification and probing.
+ * CPU PMU identification and probing. Its possible to have
+ * multiple CPU types in an ARM machine. Assure that we are
+ * picking the right PMU types based on the CPU in question
  */
-static int probe_current_pmu(struct arm_pmu *pmu,
-			     const struct pmu_probe_info *info)
+static int probe_plat_pmu(struct arm_pmu *pmu,
+			     const struct pmu_probe_info *info,
+			     unsigned int pmuid)
 {
-	int cpu = get_cpu();
-	unsigned int cpuid = read_cpuid_id();
 	int ret = -ENODEV;
+	int cpu;
+	int aff_ctr = 0;
+	static int duplicate_pmus;
+	struct platform_device *pdev = pmu->plat_device;
+	int irq = platform_get_irq(pdev, 0);
+
+	if (irq >= 0 && !irq_is_percpu(irq)) {
+		pmu->irq_affinity = kcalloc(pdev->num_resources, sizeof(int),
+					    GFP_KERNEL);
+		if (!pmu->irq_affinity)
+			return -ENOMEM;
+	}
+
+	for_each_possible_cpu(cpu) {
+		unsigned int cpuid = read_specific_cpuid(cpu);
 
-	pr_info("probing PMU on CPU %d\n", cpu);
+		if (cpuid == pmuid) {
+			cpumask_set_cpu(cpu, &pmu->supported_cpus);
+			if (pmu->irq_affinity) {
+				pmu->irq_affinity[aff_ctr] = cpu;
+				aff_ctr++;
+			}
+		}
+	}
 
+	/* find the type of PMU given the CPU */
 	for (; info->init != NULL; info++) {
-		if ((cpuid & info->mask) != info->cpuid)
+		if ((pmuid & info->mask) != info->cpuid)
 			continue;
 		ret = info->init(pmu);
+		if ((!info->cpuid) && (duplicate_pmus)) {
+			pmu->name = kasprintf(GFP_KERNEL, "%s_%d",
+					    pmu->name, duplicate_pmus);
+			if (!pmu->name) {
+				kfree(pmu->irq_affinity);
+				ret = -ENOMEM;
+			}
+		}
+		duplicate_pmus++;
 		break;
 	}
 
-	put_cpu();
 	return ret;
 }
 
@@ -1010,8 +1044,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		if (!ret)
 			ret = init_fn(pmu);
 	} else if (probe_table) {
-		cpumask_setall(&pmu->supported_cpus);
-		ret = probe_current_pmu(pmu, probe_table);
+		ret = probe_plat_pmu(pmu, probe_table, read_cpuid_id());
 	}
 
 	if (ret) {
-- 
2.5.5

[PATCH 07/11] arm: arm64: pmu: Assign platform PMU CPU affinity

[Punit Agrawal]

Hi Jeremy,

One typo below.

Jeremy Linton <jeremy.linton@arm.com> writes:

> On systems with multiple PMU types the PMU to CPU affinity
> needs to be detected and set. The CPU to interrupt affinity
> should also be set.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/perf/arm_pmu.c | 53 ++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 2286cbf..28cac3a 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -11,6 +11,7 @@
>   */
>  #define pr_fmt(fmt) "hw perfevents: " fmt
>  
> +#include <linux/acpi.h>
>  #include <linux/bitmap.h>
>  #include <linux/cpumask.h>
>  #include <linux/cpu_pm.h>
> @@ -24,6 +25,7 @@
>  #include <linux/irq.h>
>  #include <linux/irqdesc.h>
>  
> +#include <asm/cpu.h>
>  #include <asm/cputype.h>
>  #include <asm/irq_regs.h>
>  
> @@ -872,25 +874,57 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
>  }
>  
>  /*
> - * CPU PMU identification and probing.
> + * CPU PMU identification and probing. Its possible to have
> + * multiple CPU types in an ARM machine. Assure that we are
                                            Assume



> + * picking the right PMU types based on the CPU in question
>   */

[...]

[PATCH 08/11] arm64: pmu: Provide cpumask attribute for PMU

[Jeremy Linton]

With heterogeneous PMUs its helpful to know which PMUs are bound
to each CPU. Provide that information with a cpumask sysfs entry
similar to other PMUs.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 356fa6c..dae73ea 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -533,6 +533,26 @@ static struct attribute_group armv8_pmuv3_events_attr_group = {
 
 PMU_FORMAT_ATTR(event, "config:0-9");
 
+static ssize_t
+cpumask_show(struct device *dev, struct device_attribute *attr, char *page)
+{
+	struct pmu *pmu = dev_get_drvdata(dev);
+	struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu);
+
+	return cpumap_print_to_pagebuf(true, page, &cpu_pmu->supported_cpus);
+}
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *armv8_pmuv3_attrs[] = {
+	 &dev_attr_cpumask.attr,
+	 NULL,
+};
+
+static struct attribute_group armv8_pmuv3_attr_group = {
+	.attrs = armv8_pmuv3_attrs,
+};
+
+
 static struct attribute *armv8_pmuv3_format_attrs[] = {
 	&format_attr_event.attr,
 	NULL,
@@ -544,6 +564,7 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
 };
 
 static const struct attribute_group *armv8_pmuv3_attr_groups[] = {
+	&armv8_pmuv3_attr_group,
 	&armv8_pmuv3_events_attr_group,
 	&armv8_pmuv3_format_attr_group,
 	NULL,
-- 
2.5.5

[PATCH 08/11] arm64: pmu: Provide cpumask attribute for PMU

[Mark Rutland]

Hi Jeremy,

Apologies for the late reply on this.

On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:
> With heterogeneous PMUs its helpful to know which PMUs are bound
> to each CPU. Provide that information with a cpumask sysfs entry
> similar to other PMUs.

Have you tested trying to stat on a particular PMU? e.g.

$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls

I found that the presence of a cpumask file would cause (at least some
versions) of perf-stat to hang, and was holding off adding a cpumask
until we had a solution to that.

See [1,2] for more details on that.

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

This should be generic across the arm-pmu code, and so should live under
drivers/perf/.

Thanks,
Mark.

[1] http://lkml.kernel.org/r/1467907474-3290-1-git-send-email-mark.rutland at arm.com
[2] http://lkml.kernel.org/r/1467907474-3290-2-git-send-email-mark.rutland at arm.com

> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 356fa6c..dae73ea 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -533,6 +533,26 @@ static struct attribute_group armv8_pmuv3_events_attr_group = {
>  
>  PMU_FORMAT_ATTR(event, "config:0-9");
>  
> +static ssize_t
> +cpumask_show(struct device *dev, struct device_attribute *attr, char *page)
> +{
> +	struct pmu *pmu = dev_get_drvdata(dev);
> +	struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu);
> +
> +	return cpumap_print_to_pagebuf(true, page, &cpu_pmu->supported_cpus);
> +}
> +static DEVICE_ATTR_RO(cpumask);
> +
> +static struct attribute *armv8_pmuv3_attrs[] = {
> +	 &dev_attr_cpumask.attr,
> +	 NULL,
> +};
> +
> +static struct attribute_group armv8_pmuv3_attr_group = {
> +	.attrs = armv8_pmuv3_attrs,
> +};
> +
> +
>  static struct attribute *armv8_pmuv3_format_attrs[] = {
>  	&format_attr_event.attr,
>  	NULL,
> @@ -544,6 +564,7 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
>  };
>  
>  static const struct attribute_group *armv8_pmuv3_attr_groups[] = {
> +	&armv8_pmuv3_attr_group,
>  	&armv8_pmuv3_events_attr_group,
>  	&armv8_pmuv3_format_attr_group,
>  	NULL,
> -- 
> 2.5.5
> 

[PATCH 08/11] arm64: pmu: Provide cpumask attribute for PMU

[Jeremy Linton]

Hi,

On 07/07/2016 11:21 AM, Mark Rutland wrote:
> Hi Jeremy,
>
> Apologies for the late reply on this.
>
> On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:
>> With heterogeneous PMUs its helpful to know which PMUs are bound
>> to each CPU. Provide that information with a cpumask sysfs entry
>> similar to other PMUs.
>
> Have you tested trying to stat on a particular PMU? e.g.
>
> $ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
>
> I found that the presence of a cpumask file would cause (at least some
> versions) of perf-stat to hang, and was holding off adding a cpumask
> until we had a solution to that.

Nice!

I guess that is more proof that any tiny change can break things.. 
Another "fix" is to make the cpumap_print_to_pagebuf's first parameter 
false which apparently keeps perf from understanding the cpu mask and 
forces the user to use numactl, which is ugly because numactrl doesn't 
understand the mask in that format either.

I guess fixing perf is probably the best bet here...



>
> See [1,2] for more details on that.
>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>
> This should be generic across the arm-pmu code, and so should live under
> drivers/perf/.

Which is where i had it initially, but (IIRC) getting to work there 
required some core pmu changes that seemed a little ugly. I guess I 
didn't like the idea of reallocating the per pmu attr group or tweaking 
the pmu core code.. So I just moved it into perf_event where it fit more 
naturally.



>
> Thanks,
> Mark.
>
> [1] http://lkml.kernel.org/r/1467907474-3290-1-git-send-email-mark.rutland at arm.com
> [2] http://lkml.kernel.org/r/1467907474-3290-2-git-send-email-mark.rutland at arm.com
>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index 356fa6c..dae73ea 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -533,6 +533,26 @@ static struct attribute_group armv8_pmuv3_events_attr_group = {
>>
>>   PMU_FORMAT_ATTR(event, "config:0-9");
>>
>> +static ssize_t
>> +cpumask_show(struct device *dev, struct device_attribute *attr, char *page)
>> +{
>> +	struct pmu *pmu = dev_get_drvdata(dev);
>> +	struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu);
>> +
>> +	return cpumap_print_to_pagebuf(true, page, &cpu_pmu->supported_cpus);
>> +}
>> +static DEVICE_ATTR_RO(cpumask);
>> +
>> +static struct attribute *armv8_pmuv3_attrs[] = {
>> +	 &dev_attr_cpumask.attr,
>> +	 NULL,
>> +};
>> +
>> +static struct attribute_group armv8_pmuv3_attr_group = {
>> +	.attrs = armv8_pmuv3_attrs,
>> +};
>> +
>> +
>>   static struct attribute *armv8_pmuv3_format_attrs[] = {
>>   	&format_attr_event.attr,
>>   	NULL,
>> @@ -544,6 +564,7 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
>>   };
>>
>>   static const struct attribute_group *armv8_pmuv3_attr_groups[] = {
>> +	&armv8_pmuv3_attr_group,
>>   	&armv8_pmuv3_events_attr_group,
>>   	&armv8_pmuv3_format_attr_group,
>>   	NULL,
>> --
>> 2.5.5
>>
>

[PATCH 08/11] arm64: pmu: Provide cpumask attribute for PMU

[Mark Rutland]

On Mon, Jul 11, 2016 at 10:05:39AM -0500, Jeremy Linton wrote:
> Hi,
> 
> On 07/07/2016 11:21 AM, Mark Rutland wrote:
> >Hi Jeremy,
> >
> >Apologies for the late reply on this.
> >
> >On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:
> >>With heterogeneous PMUs its helpful to know which PMUs are bound
> >>to each CPU. Provide that information with a cpumask sysfs entry
> >>similar to other PMUs.
> >
> >Have you tested trying to stat on a particular PMU? e.g.
> >
> >$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
> >
> >I found that the presence of a cpumask file would cause (at least some
> >versions) of perf-stat to hang, and was holding off adding a cpumask
> >until we had a solution to that.
> 
> Nice!
> 
> I guess that is more proof that any tiny change can break things..
> Another "fix" is to make the cpumap_print_to_pagebuf's first
> parameter false which apparently keeps perf from understanding the
> cpu mask and forces the user to use numactl, which is ugly because
> numactrl doesn't understand the mask in that format either.

If we have a cpumask, it should be in keeping with the usual style.

> I guess fixing perf is probably the best bet here...

Indeed.

The major issue is that it's not clear to me how to avoid breaking
existing binaries when adding the cpumask. I suspect that we have to
expose it under a different name. :/

> >See [1,2] for more details on that.
> >
> >>Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> >>---
> >>  arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
> >>  1 file changed, 21 insertions(+)
> >
> >This should be generic across the arm-pmu code, and so should live under
> >drivers/perf/.
> 
> Which is where i had it initially, but (IIRC) getting to work there
> required some core pmu changes that seemed a little ugly.

That was my experience from local refactoring, too.

> I guess I didn't like the idea of reallocating the per pmu attr group
> or tweaking the pmu core code.. So I just moved it into perf_event
> where it fit more naturally.

I don't think we need to allocate the attr group per pmu (we get the PMU
pointer from the dev, so the attr group can be shared).

I think we can share the logic, the cpumask attr, and the cpumask
attr_group in drivers/perf/arm_pmu.c, exposing a common
arm_pmu_cpumask_attr_group pointer via the arm_pmu header.

Then all each driver needs to wire up is a single pointer in its
attr_groups array, which isn't so bad.

Thanks,
Mark.

> 
> 
> 
> >
> >Thanks,
> >Mark.
> >
> >[1] http://lkml.kernel.org/r/1467907474-3290-1-git-send-email-mark.rutland at arm.com
> >[2] http://lkml.kernel.org/r/1467907474-3290-2-git-send-email-mark.rutland at arm.com
> >
> >>diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> >>index 356fa6c..dae73ea 100644
> >>--- a/arch/arm64/kernel/perf_event.c
> >>+++ b/arch/arm64/kernel/perf_event.c
> >>@@ -533,6 +533,26 @@ static struct attribute_group armv8_pmuv3_events_attr_group = {
> >>
> >>  PMU_FORMAT_ATTR(event, "config:0-9");
> >>
> >>+static ssize_t
> >>+cpumask_show(struct device *dev, struct device_attribute *attr, char *page)
> >>+{
> >>+	struct pmu *pmu = dev_get_drvdata(dev);
> >>+	struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu);
> >>+
> >>+	return cpumap_print_to_pagebuf(true, page, &cpu_pmu->supported_cpus);
> >>+}
> >>+static DEVICE_ATTR_RO(cpumask);
> >>+
> >>+static struct attribute *armv8_pmuv3_attrs[] = {
> >>+	 &dev_attr_cpumask.attr,
> >>+	 NULL,
> >>+};
> >>+
> >>+static struct attribute_group armv8_pmuv3_attr_group = {
> >>+	.attrs = armv8_pmuv3_attrs,
> >>+};
> >>+
> >>+
> >>  static struct attribute *armv8_pmuv3_format_attrs[] = {
> >>  	&format_attr_event.attr,
> >>  	NULL,
> >>@@ -544,6 +564,7 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
> >>  };
> >>
> >>  static const struct attribute_group *armv8_pmuv3_attr_groups[] = {
> >>+	&armv8_pmuv3_attr_group,
> >>  	&armv8_pmuv3_events_attr_group,
> >>  	&armv8_pmuv3_format_attr_group,
> >>  	NULL,
> >>--
> >>2.5.5
> >>
> >
> 

[PATCH 08/11] arm64: pmu: Provide cpumask attribute for PMU

[Will Deacon]

On Mon, Jul 11, 2016 at 04:58:35PM +0100, Mark Rutland wrote:
> On Mon, Jul 11, 2016 at 10:05:39AM -0500, Jeremy Linton wrote:
> > On 07/07/2016 11:21 AM, Mark Rutland wrote:
> > >Have you tested trying to stat on a particular PMU? e.g.
> > >
> > >$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
> > >
> > >I found that the presence of a cpumask file would cause (at least some
> > >versions) of perf-stat to hang, and was holding off adding a cpumask
> > >until we had a solution to that.
> > 
> > I guess that is more proof that any tiny change can break things..
> > Another "fix" is to make the cpumap_print_to_pagebuf's first
> > parameter false which apparently keeps perf from understanding the
> > cpu mask and forces the user to use numactl, which is ugly because
> > numactrl doesn't understand the mask in that format either.
> 
> If we have a cpumask, it should be in keeping with the usual style.
> 
> > I guess fixing perf is probably the best bet here...
> 
> Indeed.
> 
> The major issue is that it's not clear to me how to avoid breaking
> existing binaries when adding the cpumask. I suspect that we have to
> expose it under a different name. :/

Well, we need to understand exactly why the cpumask breaks those older
perf builds before we do that. Is it expecting some behaviour that we
don't honour, or does it break on other architectures providing a
cpumask too?

> > >See [1,2] for more details on that.

That makes it sound specific to big/little. Did perf used to work on
those systems without a cpumask? I understand that it might not have
hung, but did it actually provide meaningful data?

I'm not against renaming the cpumask if that's our only option, but I'd
like to understand (and document) how we arrive at that, if we actually
do.

Will

[PATCH 09/11] arm64: pmu: Add routines for detecting differing PMU types in the system

[Jeremy Linton]

In preparation for enabling heterogeneous PMUs on ACPI systems
add routines that detect this and group the resulting PMUs and
interrupts.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 134 insertions(+), 3 deletions(-)

diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index a24cdd0..482a54d 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -1,23 +1,36 @@
 /*
- * PMU support
+ * ARM ACPI PMU support
  *
  * Copyright (C) 2015 Red Hat Inc.
+ * Copyright (C) 2016 ARM Ltd.
  * Author: Mark Salter <msalter@redhat.com>
+ *         Jeremy Linton <jeremy.linton@arm.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
  *
  */
 
+#define pr_fmt(fmt) "ACPI-PMU: " fmt
+
+#include <asm/cpu.h>
 #include <linux/perf/arm_pmu.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
+#include <linux/list.h>
 
 struct pmu_irq {
-	int gsi;
-	int trigger;
+	int  gsi;
+	int  trigger;
+	bool registered;
+};
+
+struct pmu_types {
+	struct list_head list;
+	int		 cpu_type;
+	int		 cpu_count;
 };
 
 static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
@@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
 		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
 }
 
+/* Count number and type of CPU cores in the system. */
+void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
+{
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
+		u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);
+		struct pmu_types *pmu;
+
+		list_for_each_entry(pmu, pmus, list) {
+			if (pmu->cpu_type == partnum) {
+				pmu->cpu_count++;
+				break;
+			}
+		}
+
+		/* we didn't find the CPU type, add an entry to identify it */
+		if (&pmu->list == pmus) {
+			pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);
+			if (!pmu) {
+				pr_warn("Unable to allocate pmu_types\n");
+			} else {
+				pmu->cpu_type = partnum;
+				pmu->cpu_count++;
+				list_add_tail(&pmu->list, pmus);
+			}
+		}
+	}
+}
+
+/*
+ * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
+ * This group utilizes 'count' resources in the 'res'.
+ */
+int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
+					    int last_cpu_id)
+{
+	int i;
+	int err = -ENOMEM;
+	bool free_gsi = false;
+	struct platform_device *pdev;
+
+	if (count) {
+		pdev = platform_device_alloc(ARMV8_PMU_PDEV_NAME, last_cpu_id);
+		if (pdev) {
+			err = platform_device_add_resources(pdev, res, count);
+			if (!err) {
+				err = platform_device_add(pdev);
+				if (err) {
+					pr_warn("Unable to register PMU device\n");
+					free_gsi = true;
+				}
+			} else {
+				pr_warn("Unable to add resources to device\n");
+				free_gsi = true;
+				platform_device_put(pdev);
+			}
+		} else {
+			pr_warn("Unable to allocate platform device\n");
+			free_gsi = true;
+		}
+	}
+
+	/* unmark (and possibly unregister) registered GSIs */
+	for_each_possible_cpu(i) {
+		if (pmu_irqs[i].registered) {
+			if (free_gsi)
+				acpi_unregister_gsi(pmu_irqs[i].gsi);
+			pmu_irqs[i].registered = false;
+		}
+	}
+
+	return err;
+}
+
+/*
+ * For the given cpu/pmu type, walk all known GSIs, register them, and add
+ * them to the resource structure. Return the number of GSI's contained
+ * in the res structure, and the id of the last CPU/PMU we added.
+ */
+int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
+				       struct resource *res, int *last_cpu_id)
+{
+	int i, count;
+	int irq;
+
+	pr_info("Setting up %d PMUs for CPU type %X\n", pmus->cpu_count,
+							pmus->cpu_type);
+	/* lets group all the PMU's from similar CPU's together */
+	count = 0;
+	for_each_possible_cpu(i) {
+		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
+
+		if (pmus->cpu_type == MIDR_PARTNUM(cinfo->reg_midr)) {
+			if (pmu_irqs[i].gsi == 0)
+				continue;
+
+			irq = acpi_register_gsi(NULL, pmu_irqs[i].gsi,
+						pmu_irqs[i].trigger,
+						ACPI_ACTIVE_HIGH);
+
+			res[count].start = res[count].end = irq;
+			res[count].flags = IORESOURCE_IRQ;
+
+			if (pmu_irqs[i].trigger == ACPI_EDGE_SENSITIVE)
+				res[count].flags |= IORESOURCE_IRQ_HIGHEDGE;
+			else
+				res[count].flags |= IORESOURCE_IRQ_HIGHLEVEL;
+
+			pmu_irqs[i].registered = true;
+			count++;
+			(*last_cpu_id) = cinfo->reg_midr;
+		}
+	}
+	return count;
+}
+
 static int __init pmu_acpi_init(void)
 {
 	struct platform_device *pdev;
-- 
2.5.5

[PATCH 09/11] arm64: pmu: Add routines for detecting differing PMU types in the system

[Punit Agrawal]

Jeremy Linton <jeremy.linton@arm.com> writes:

> In preparation for enabling heterogeneous PMUs on ACPI systems
> add routines that detect this and group the resulting PMUs and
> interrupts.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 134 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index a24cdd0..482a54d 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -1,23 +1,36 @@
>  /*
> - * PMU support
> + * ARM ACPI PMU support
>   *
>   * Copyright (C) 2015 Red Hat Inc.
> + * Copyright (C) 2016 ARM Ltd.
>   * Author: Mark Salter <msalter@redhat.com>
> + *         Jeremy Linton <jeremy.linton@arm.com>
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2.  See
>   * the COPYING file in the top-level directory.
>   *
>   */
>  
> +#define pr_fmt(fmt) "ACPI-PMU: " fmt
> +
> +#include <asm/cpu.h>
>  #include <linux/perf/arm_pmu.h>
>  #include <linux/platform_device.h>
>  #include <linux/acpi.h>
>  #include <linux/irq.h>
>  #include <linux/irqdesc.h>
> +#include <linux/list.h>
>  
>  struct pmu_irq {
> -	int gsi;
> -	int trigger;
> +	int  gsi;
> +	int  trigger;
> +	bool registered;
> +};
> +
> +struct pmu_types {
> +	struct list_head list;
> +	int		 cpu_type;
> +	int		 cpu_count;
>  };

You can stash the associated resources in the above structure. That
should simplify some code below.

>  
>  static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
> @@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
>  		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
>  }
>  
> +/* Count number and type of CPU cores in the system. */
> +void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
> +{
> +	int i;
> +
> +	for_each_possible_cpu(i) {
> +		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
> +		u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);
> +		struct pmu_types *pmu;
> +
> +		list_for_each_entry(pmu, pmus, list) {
> +			if (pmu->cpu_type == partnum) {
> +				pmu->cpu_count++;
> +				break;
> +			}
> +		}
> +
> +		/* we didn't find the CPU type, add an entry to identify it */
> +		if (&pmu->list == pmus) {
> +			pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);

Use kzalloc here.

> +			if (!pmu) {
> +				pr_warn("Unable to allocate pmu_types\n");

Bail out with error if the memory can't be allocated. Otherwise, we risk
silently failing to register a PMU type.

> +			} else {
> +				pmu->cpu_type = partnum;
> +				pmu->cpu_count++;
> +				list_add_tail(&pmu->list, pmus);
> +			}
> +		}
> +	}
> +}
> +
> +/*
> + * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
> + * This group utilizes 'count' resources in the 'res'.
> + */
> +int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
> +					    int last_cpu_id)
> +{

With the addition of the irq resources to struct pmu_types, you can just pass
the pmu structure here.

> +	int i;
> +	int err = -ENOMEM;
> +	bool free_gsi = false;
> +	struct platform_device *pdev;
> +
> +	if (count) {

        if (!count)
           goto out;

That should help reduce the nesting below. Others might have a different
opinion, but I think it's ok to use goto when it helps make the code
more readable.

Similarly, some of the code below can be simplified as well.

> +		pdev = platform_device_alloc(ARMV8_PMU_PDEV_NAME, last_cpu_id);
> +		if (pdev) {
> +			err = platform_device_add_resources(pdev, res, count);
> +			if (!err) {
> +				err = platform_device_add(pdev);
> +				if (err) {
> +					pr_warn("Unable to register PMU device\n");
> +					free_gsi = true;
> +				}
> +			} else {
> +				pr_warn("Unable to add resources to device\n");
> +				free_gsi = true;
> +				platform_device_put(pdev);
> +			}
> +		} else {
> +			pr_warn("Unable to allocate platform device\n");
> +			free_gsi = true;
> +		}
> +	}
> +
> +	/* unmark (and possibly unregister) registered GSIs */
> +	for_each_possible_cpu(i) {
> +		if (pmu_irqs[i].registered) {
> +			if (free_gsi)
> +				acpi_unregister_gsi(pmu_irqs[i].gsi);
> +			pmu_irqs[i].registered = false;
> +		}
> +	}
> +

out:

> +	return err;
> +}
> +
> +/*
> + * For the given cpu/pmu type, walk all known GSIs, register them, and add
> + * them to the resource structure. Return the number of GSI's contained
> + * in the res structure, and the id of the last CPU/PMU we added.
> + */
> +int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
> +				       struct resource *res, int *last_cpu_id)

With struct resource as part of the pmu_types structure you can drop the
last two arguments and allocate the resources in this function.

> +{
> +	int i, count;
> +	int irq;
> +
> +	pr_info("Setting up %d PMUs for CPU type %X\n", pmus->cpu_count,
> +							pmus->cpu_type);

Please drop this pr_info.

> +	/* lets group all the PMU's from similar CPU's together */
> +	count = 0;
> +	for_each_possible_cpu(i) {
> +		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
> +
> +		if (pmus->cpu_type == MIDR_PARTNUM(cinfo->reg_midr)) {

You can invert the condition check here and reduce nesting.

> +			if (pmu_irqs[i].gsi == 0)
> +				continue;
> +
> +			irq = acpi_register_gsi(NULL, pmu_irqs[i].gsi,
> +						pmu_irqs[i].trigger,
> +						ACPI_ACTIVE_HIGH);
> +
> +			res[count].start = res[count].end = irq;
> +			res[count].flags = IORESOURCE_IRQ;
> +
> +			if (pmu_irqs[i].trigger == ACPI_EDGE_SENSITIVE)
> +				res[count].flags |= IORESOURCE_IRQ_HIGHEDGE;
> +			else
> +				res[count].flags |= IORESOURCE_IRQ_HIGHLEVEL;
> +
> +			pmu_irqs[i].registered = true;
> +			count++;
> +			(*last_cpu_id) = cinfo->reg_midr;
> +		}
> +	}
> +	return count;
> +}
> +
>  static int __init pmu_acpi_init(void)
>  {
>  	struct platform_device *pdev;

[PATCH 09/11] arm64: pmu: Add routines for detecting differing PMU types in the system

[Jeremy Linton]

On 07/01/2016 08:58 AM, Punit Agrawal wrote:
> Jeremy Linton <jeremy.linton@arm.com> writes:
>
>> In preparation for enabling heterogeneous PMUs on ACPI systems
>> add routines that detect this and group the resulting PMUs and
>> interrupts.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 134 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
>> index a24cdd0..482a54d 100644
>> --- a/drivers/perf/arm_pmu_acpi.c
>> +++ b/drivers/perf/arm_pmu_acpi.c
>> @@ -1,23 +1,36 @@
>>   /*
>> - * PMU support
>> + * ARM ACPI PMU support
>>    *
>>    * Copyright (C) 2015 Red Hat Inc.
>> + * Copyright (C) 2016 ARM Ltd.
>>    * Author: Mark Salter <msalter@redhat.com>
>> + *         Jeremy Linton <jeremy.linton@arm.com>
>>    *
>>    * This work is licensed under the terms of the GNU GPL, version 2.  See
>>    * the COPYING file in the top-level directory.
>>    *
>>    */
>>
>> +#define pr_fmt(fmt) "ACPI-PMU: " fmt
>> +
>> +#include <asm/cpu.h>
>>   #include <linux/perf/arm_pmu.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/acpi.h>
>>   #include <linux/irq.h>
>>   #include <linux/irqdesc.h>
>> +#include <linux/list.h>
>>
>>   struct pmu_irq {
>> -	int gsi;
>> -	int trigger;
>> +	int  gsi;
>> +	int  trigger;
>> +	bool registered;
>> +};
>> +
>> +struct pmu_types {
>> +	struct list_head list;
>> +	int		 cpu_type;
>> +	int		 cpu_count;
>>   };
>
> You can stash the associated resources in the above structure. That
> should simplify some code below.

	How is that? One structure is per cpu, the other is per pmu type in the 
system, they are actually completely independent and intertwining them 
will only server to obfuscate the code.


>
>>
>>   static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
>> @@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
>>   		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
>>   }
>>
>> +/* Count number and type of CPU cores in the system. */
>> +void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
>> +{
>> +	int i;
>> +
>> +	for_each_possible_cpu(i) {
>> +		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
>> +		u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);
>> +		struct pmu_types *pmu;
>> +
>> +		list_for_each_entry(pmu, pmus, list) {
>> +			if (pmu->cpu_type == partnum) {
>> +				pmu->cpu_count++;
>> +				break;
>> +			}
>> +		}
>> +
>> +		/* we didn't find the CPU type, add an entry to identify it */
>> +		if (&pmu->list == pmus) {
>> +			pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);
>
> Use kzalloc here.
Ok fair point.

>
>> +			if (!pmu) {
>> +				pr_warn("Unable to allocate pmu_types\n");
>
> Bail out with error if the memory can't be allocated. Otherwise, we risk
> silently failing to register a PMU type.

? Its not silent, it fails to allocate the space complains about it, and 
therefor this pmu type is not created. In a system with a single CPU 
this basically cancels the whole operation. If there is more than one 
pmu, the remaining PMUs continue to have a chance of being created, 
although if the memory allocation fails (this is pretty early boot code) 
there is a high probability there is something seriously wrong with the 
system.


>
>> +			} else {
>> +				pmu->cpu_type = partnum;
>> +				pmu->cpu_count++;
>> +				list_add_tail(&pmu->list, pmus);
>> +			}
>> +		}
>> +	}
>> +}
>> +
>> +/*
>> + * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
>> + * This group utilizes 'count' resources in the 'res'.
>> + */
>> +int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
>> +					    int last_cpu_id)
>> +{
>
> With the addition of the irq resources to struct pmu_types, you can just pass
> the pmu structure here.

	Thats a point, but the lifetimes of the structures are different and 
outside of their shared use in this single function never really 
interact. I prefer not unnecessarily intertwine independent data 
structures simply to reduce parameter counts for a single function. 
Especially since it complicates cleanup because the validity of the 
resource structure will have to be tracked relative to its successful 
registration.

[PATCH 09/11] arm64: pmu: Add routines for detecting differing PMU types in the system

[Punit Agrawal]

Jeremy Linton <jeremy.linton@arm.com> writes:

> On 07/01/2016 08:58 AM, Punit Agrawal wrote:
>> Jeremy Linton <jeremy.linton@arm.com> writes:
>>
>>> In preparation for enabling heterogeneous PMUs on ACPI systems
>>> add routines that detect this and group the resulting PMUs and
>>> interrupts.
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>>   drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 134 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
>>> index a24cdd0..482a54d 100644
>>> --- a/drivers/perf/arm_pmu_acpi.c
>>> +++ b/drivers/perf/arm_pmu_acpi.c
>>> @@ -1,23 +1,36 @@
>>>   /*
>>> - * PMU support
>>> + * ARM ACPI PMU support
>>>    *
>>>    * Copyright (C) 2015 Red Hat Inc.
>>> + * Copyright (C) 2016 ARM Ltd.
>>>    * Author: Mark Salter <msalter@redhat.com>
>>> + *         Jeremy Linton <jeremy.linton@arm.com>
>>>    *
>>>    * This work is licensed under the terms of the GNU GPL, version 2.  See
>>>    * the COPYING file in the top-level directory.
>>>    *
>>>    */
>>>
>>> +#define pr_fmt(fmt) "ACPI-PMU: " fmt
>>> +
>>> +#include <asm/cpu.h>
>>>   #include <linux/perf/arm_pmu.h>
>>>   #include <linux/platform_device.h>
>>>   #include <linux/acpi.h>
>>>   #include <linux/irq.h>
>>>   #include <linux/irqdesc.h>
>>> +#include <linux/list.h>
>>>
>>>   struct pmu_irq {
>>> -	int gsi;
>>> -	int trigger;
>>> +	int  gsi;
>>> +	int  trigger;
>>> +	bool registered;
>>> +};
>>> +
>>> +struct pmu_types {
>>> +	struct list_head list;
>>> +	int		 cpu_type;
>>> +	int		 cpu_count;
>>>   };
>>
>> You can stash the associated resources in the above structure. That
>> should simplify some code below.
>
> 	How is that? One structure is per cpu, the other is per pmu
> type in the system, they are actually completely independent and
> intertwining them will only server to obfuscate the code.

Just to clarify, I am referring to "struct resources" you allocate for
use with the pmu platform device in the next patch.

>
>
>>
>>>
>>>   static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
>>> @@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
>>>   		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
>>>   }
>>>
>>> +/* Count number and type of CPU cores in the system. */
>>> +void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
>>> +{
>>> +	int i;
>>> +
>>> +	for_each_possible_cpu(i) {
>>> +		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
>>> +		u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);
>>> +		struct pmu_types *pmu;
>>> +
>>> +		list_for_each_entry(pmu, pmus, list) {
>>> +			if (pmu->cpu_type == partnum) {
>>> +				pmu->cpu_count++;
>>> +				break;
>>> +			}
>>> +		}
>>> +
>>> +		/* we didn't find the CPU type, add an entry to identify it */
>>> +		if (&pmu->list == pmus) {
>>> +			pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);
>>
>> Use kzalloc here.
> Ok fair point.
>
>>
>>> +			if (!pmu) {
>>> +				pr_warn("Unable to allocate pmu_types\n");
>>
>> Bail out with error if the memory can't be allocated. Otherwise, we risk
>> silently failing to register a PMU type.
>
> ? Its not silent, it fails to allocate the space complains about it,
> and therefor this pmu type is not created.

Sorry for the confusion, I didn't mean PMU type in the above comment. I
was thinking of the missed cpu in cpu_count.

If the allocation succeeds in the next iteration (as we don't break out
of the loop), we'll end up undercounting cpus that have this type of
pmu. In turn, this will lead to accessing beyond the allocated "struct
resource" in arm_pmu_acpi_gsi_res.

Punit

> In a system with a single
> CPU this basically cancels the whole operation. If there is more than
> one pmu, the remaining PMUs continue to have a chance of being
> created, although if the memory allocation fails (this is pretty early
> boot code) there is a high probability there is something seriously
> wrong with the system.
>
>
>>
>>> +			} else {
>>> +				pmu->cpu_type = partnum;
>>> +				pmu->cpu_count++;
>>> +				list_add_tail(&pmu->list, pmus);
>>> +			}
>>> +		}
>>> +	}
>>> +}
>>> +
>>> +/*
>>> + * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
>>> + * This group utilizes 'count' resources in the 'res'.
>>> + */
>>> +int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
>>> +					    int last_cpu_id)
>>> +{
>>
>> With the addition of the irq resources to struct pmu_types, you can just pass
>> the pmu structure here.
>
> 	Thats a point, but the lifetimes of the structures are
> different and outside of their shared use in this single function
> never really interact. I prefer not unnecessarily intertwine
> independent data structures simply to reduce parameter counts for a
> single function. Especially since it complicates cleanup because the
> validity of the resource structure will have to be tracked relative to
> its successful registration.
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/11] arm64: pmu: Add routines for detecting differing PMU types in the system

[Jeremy Linton]

On 07/01/2016 10:43 AM, Punit Agrawal wrote:
> Jeremy Linton <jeremy.linton@arm.com> writes:
>
>> On 07/01/2016 08:58 AM, Punit Agrawal wrote:
>>> Jeremy Linton <jeremy.linton@arm.com> writes:
>>>
>>>> In preparation for enabling heterogeneous PMUs on ACPI systems
>>>> add routines that detect this and group the resulting PMUs and
>>>> interrupts.
>>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>>    drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
>>>>    1 file changed, 134 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
>>>> index a24cdd0..482a54d 100644
>>>> --- a/drivers/perf/arm_pmu_acpi.c
>>>> +++ b/drivers/perf/arm_pmu_acpi.c
>>>> @@ -1,23 +1,36 @@
>>>>    /*
>>>> - * PMU support
>>>> + * ARM ACPI PMU support
>>>>     *
>>>>     * Copyright (C) 2015 Red Hat Inc.
>>>> + * Copyright (C) 2016 ARM Ltd.
>>>>     * Author: Mark Salter <msalter@redhat.com>
>>>> + *         Jeremy Linton <jeremy.linton@arm.com>
>>>>     *
>>>>     * This work is licensed under the terms of the GNU GPL, version 2.  See
>>>>     * the COPYING file in the top-level directory.
>>>>     *
>>>>     */
>>>>
>>>> +#define pr_fmt(fmt) "ACPI-PMU: " fmt
>>>> +
>>>> +#include <asm/cpu.h>
>>>>    #include <linux/perf/arm_pmu.h>
>>>>    #include <linux/platform_device.h>
>>>>    #include <linux/acpi.h>
>>>>    #include <linux/irq.h>
>>>>    #include <linux/irqdesc.h>
>>>> +#include <linux/list.h>
>>>>
>>>>    struct pmu_irq {
>>>> -	int gsi;
>>>> -	int trigger;
>>>> +	int  gsi;
>>>> +	int  trigger;
>>>> +	bool registered;
>>>> +};
>>>> +
>>>> +struct pmu_types {
>>>> +	struct list_head list;
>>>> +	int		 cpu_type;
>>>> +	int		 cpu_count;
>>>>    };
>>>
>>> You can stash the associated resources in the above structure. That
>>> should simplify some code below.
>>
>> 	How is that? One structure is per cpu, the other is per pmu
>> type in the system, they are actually completely independent and
>> intertwining them will only server to obfuscate the code.
>
> Just to clarify, I am referring to "struct resources" you allocate for
> use with the pmu platform device in the next patch.

Ah yes, I understood that for the later comment but for this one I 
thought you were suggesting nesting these too. Thanks for clarifying.

>
>>
>>
>>>
>>>>
>>>>    static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
>>>> @@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
>>>>    		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
>>>>    }
>>>>
>>>> +/* Count number and type of CPU cores in the system. */
>>>> +void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
>>>> +{
>>>> +	int i;
>>>> +
>>>> +	for_each_possible_cpu(i) {
>>>> +		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
>>>> +		u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);
>>>> +		struct pmu_types *pmu;
>>>> +
>>>> +		list_for_each_entry(pmu, pmus, list) {
>>>> +			if (pmu->cpu_type == partnum) {
>>>> +				pmu->cpu_count++;
>>>> +				break;
>>>> +			}
>>>> +		}
>>>> +
>>>> +		/* we didn't find the CPU type, add an entry to identify it */
>>>> +		if (&pmu->list == pmus) {
>>>> +			pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);
>>>
>>> Use kzalloc here.
>> Ok fair point.
>>
>>>
>>>> +			if (!pmu) {
>>>> +				pr_warn("Unable to allocate pmu_types\n");
>>>
>>> Bail out with error if the memory can't be allocated. Otherwise, we risk
>>> silently failing to register a PMU type.
>>
>> ? Its not silent, it fails to allocate the space complains about it,
>> and therefor this pmu type is not created.
>
> Sorry for the confusion, I didn't mean PMU type in the above comment. I
> was thinking of the missed cpu in cpu_count.
>
> If the allocation succeeds in the next iteration (as we don't break out
> of the loop), we'll end up undercounting cpus that have this type of
> pmu. In turn, this will lead to accessing beyond the allocated "struct
> resource" in arm_pmu_acpi_gsi_res.

Oh! Yah, that is a good point. I guess I missed that when i converted 
from the fixed buffer (which couldn't have this problem).

Actually I think i'm going to add a bad_alloc flag to stop further 
allocation attempts in this function. That way in theory the other side 
of the problem doesn't occur either (aka we get one allocation that 
succeeds, then one that fails, and the cpu's get under counted for the 
one that succeeded).




>
> Punit
>
>> In a system with a single
>> CPU this basically cancels the whole operation. If there is more than
>> one pmu, the remaining PMUs continue to have a chance of being
>> created, although if the memory allocation fails (this is pretty early
>> boot code) there is a high probability there is something seriously
>> wrong with the system.
>>
>>
>>>
>>>> +			} else {
>>>> +				pmu->cpu_type = partnum;
>>>> +				pmu->cpu_count++;
>>>> +				list_add_tail(&pmu->list, pmus);
>>>> +			}
>>>> +		}
>>>> +	}
>>>> +}
>>>> +
>>>> +/*
>>>> + * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
>>>> + * This group utilizes 'count' resources in the 'res'.
>>>> + */
>>>> +int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
>>>> +					    int last_cpu_id)
>>>> +{
>>>
>>> With the addition of the irq resources to struct pmu_types, you can just pass
>>> the pmu structure here.
>>
>> 	Thats a point, but the lifetimes of the structures are
>> different and outside of their shared use in this single function
>> never really interact. I prefer not unnecessarily intertwine
>> independent data structures simply to reduce parameter counts for a
>> single function. Especially since it complicates cleanup because the
>> validity of the resource structure will have to be tracked relative to
>> its successful registration.
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/11] arm64: pmu: Add routines for detecting differing PMU types in the system

[Jeremy Linton]

On 07/01/2016 08:58 AM, Punit Agrawal wrote:
(trimming)
>> +			if (!pmu) {
>> +				pr_warn("Unable to allocate pmu_types\n");
>
> Bail out with error if the memory can't be allocated. Otherwise, we risk
> silently failing to register a PMU type.

Reading this again, I think I misunderstood what you were saying.. Aka, 
this should be pr_error() not that the error handing is failing to catch 
the failure.

Anyway, I will promote this.

[PATCH 10/11] arm64: pmu: Enable multiple PMUs in an ACPI system

[Jeremy Linton]

Its possible that an ACPI system has multiple CPU types in it
with differing PMU counters. Use the newly provided acpi_pmu routines
to detect that case, and instantiate more than one set of counters.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 drivers/perf/arm_pmu.c      |  7 +++-
 drivers/perf/arm_pmu_acpi.c | 82 ++++++++++++++++-----------------------------
 2 files changed, 35 insertions(+), 54 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 28cac3a..f94d279 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -1044,7 +1044,12 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		if (!ret)
 			ret = init_fn(pmu);
 	} else if (probe_table) {
-		ret = probe_plat_pmu(pmu, probe_table, read_cpuid_id());
+		if (acpi_disabled) {
+			/* use the current cpu. */
+			ret = probe_plat_pmu(pmu, probe_table,
+					     read_cpuid_id());
+		} else
+			ret = probe_plat_pmu(pmu, probe_table, pdev->id);
 	}
 
 	if (ret) {
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index 482a54d..6f5df1a 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -50,7 +50,7 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
 }
 
 /* Count number and type of CPU cores in the system. */
-void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
+static void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
 {
 	int i;
 
@@ -84,7 +84,7 @@ void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
  * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
  * This group utilizes 'count' resources in the 'res'.
  */
-int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
+static int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
 					    int last_cpu_id)
 {
 	int i;
@@ -130,7 +130,7 @@ int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
  * them to the resource structure. Return the number of GSI's contained
  * in the res structure, and the id of the last CPU/PMU we added.
  */
-int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
+static int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
 				       struct resource *res, int *last_cpu_id)
 {
 	int i, count;
@@ -169,63 +169,39 @@ int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
 
 static int __init pmu_acpi_init(void)
 {
-	struct platform_device *pdev;
-	struct pmu_irq *pirq = pmu_irqs;
-	struct resource	*res, *r;
+	struct resource	*res;
 	int err = -ENOMEM;
-	int i, count, irq;
+	int count, cpu_id;
+	struct pmu_types *pmu, *safe_temp;
+	LIST_HEAD(pmus);
 
 	if (acpi_disabled)
 		return 0;
 
-	/* Must have irq for boot cpu, at least */
-	if (pirq->gsi == 0)
-		return -EINVAL;
-
-	irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
-				ACPI_ACTIVE_HIGH);
-
-	if (irq_is_percpu(irq))
-		count = 1;
-	else
-		for (i = 1, count = 1; i < NR_CPUS; i++)
-			if (pmu_irqs[i].gsi)
-				++count;
-
-	pdev = platform_device_alloc(ARMV8_PMU_PDEV_NAME, -1);
-	if (!pdev)
-		goto err_free_gsi;
-
-	res = kcalloc(count, sizeof(*res), GFP_KERNEL);
-	if (!res)
-		goto err_free_device;
-
-	for (i = 0, r = res; i < count; i++, pirq++, r++) {
-		if (i)
-			irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
-						ACPI_ACTIVE_HIGH);
-		r->start = r->end = irq;
-		r->flags = IORESOURCE_IRQ;
-		if (pirq->trigger == ACPI_EDGE_SENSITIVE)
-			r->flags |= IORESOURCE_IRQ_HIGHEDGE;
-		else
-			r->flags |= IORESOURCE_IRQ_HIGHLEVEL;
+	arm_pmu_acpi_determine_cpu_types(&pmus);
+
+	list_for_each_entry_safe(pmu, safe_temp, &pmus, list) {
+		res = kcalloc(pmu->cpu_count,
+			      sizeof(struct resource), GFP_KERNEL);
+
+		/* for a given PMU type collect all the GSIs. */
+		if (res) {
+			count = arm_pmu_acpi_gsi_res(pmu, res,
+						     &cpu_id);
+			/*
+			 * register this set of interrupts
+			 * with a new PMU device
+			 */
+			err = arm_pmu_acpi_register_pmu(count, res, cpu_id);
+			kfree(res);
+		} else
+			pr_warn("PMU unable to allocate interrupt resource space\n");
+
+		list_del(&pmu->list);
+		kfree(pmu);
 	}
 
-	err = platform_device_add_resources(pdev, res, count);
-	if (!err)
-		err = platform_device_add(pdev);
-	kfree(res);
-	if (!err)
-		return 0;
-
-err_free_device:
-	platform_device_put(pdev);
-
-err_free_gsi:
-	for (i = 0; i < count; i++)
-		acpi_unregister_gsi(pmu_irqs[i].gsi);
-
 	return err;
 }
+
 arch_initcall(pmu_acpi_init);
-- 
2.5.5

[PATCH 10/11] arm64: pmu: Enable multiple PMUs in an ACPI system

[Punit Agrawal]

Jeremy Linton <jeremy.linton@arm.com> writes:

> Its possible that an ACPI system has multiple CPU types in it
> with differing PMU counters. Use the newly provided acpi_pmu routines
> to detect that case, and instantiate more than one set of counters.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/perf/arm_pmu.c      |  7 +++-
>  drivers/perf/arm_pmu_acpi.c | 82 ++++++++++++++++-----------------------------
>  2 files changed, 35 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 28cac3a..f94d279 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -1044,7 +1044,12 @@ int arm_pmu_device_probe(struct platform_device *pdev,
>  		if (!ret)
>  			ret = init_fn(pmu);
>  	} else if (probe_table) {
> -		ret = probe_plat_pmu(pmu, probe_table, read_cpuid_id());
> +		if (acpi_disabled) {
> +			/* use the current cpu. */
> +			ret = probe_plat_pmu(pmu, probe_table,
> +					     read_cpuid_id());
> +		} else
> +			ret = probe_plat_pmu(pmu, probe_table, pdev->id);
>  	}
>  
>  	if (ret) {
> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index 482a54d..6f5df1a 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -50,7 +50,7 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
>  }
>  
>  /* Count number and type of CPU cores in the system. */
> -void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
> +static void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
>  {
>  	int i;
>  
> @@ -84,7 +84,7 @@ void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)
>   * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
>   * This group utilizes 'count' resources in the 'res'.
>   */
> -int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
> +static int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
>  					    int last_cpu_id)
>  {
>  	int i;
> @@ -130,7 +130,7 @@ int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
>   * them to the resource structure. Return the number of GSI's contained
>   * in the res structure, and the id of the last CPU/PMU we added.
>   */
> -int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
> +static int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
>  				       struct resource *res, int *last_cpu_id)
>  {
>  	int i, count;
> @@ -169,63 +169,39 @@ int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
>  
>  static int __init pmu_acpi_init(void)
>  {
> -	struct platform_device *pdev;
> -	struct pmu_irq *pirq = pmu_irqs;
> -	struct resource	*res, *r;
> +	struct resource	*res;
>  	int err = -ENOMEM;
> -	int i, count, irq;
> +	int count, cpu_id;
> +	struct pmu_types *pmu, *safe_temp;
> +	LIST_HEAD(pmus);
>  
>  	if (acpi_disabled)
>  		return 0;
>  
> -	/* Must have irq for boot cpu, at least */
> -	if (pirq->gsi == 0)
> -		return -EINVAL;
> -
> -	irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
> -				ACPI_ACTIVE_HIGH);
> -
> -	if (irq_is_percpu(irq))
> -		count = 1;
> -	else
> -		for (i = 1, count = 1; i < NR_CPUS; i++)
> -			if (pmu_irqs[i].gsi)
> -				++count;
> -
> -	pdev = platform_device_alloc(ARMV8_PMU_PDEV_NAME, -1);
> -	if (!pdev)
> -		goto err_free_gsi;
> -
> -	res = kcalloc(count, sizeof(*res), GFP_KERNEL);
> -	if (!res)
> -		goto err_free_device;
> -
> -	for (i = 0, r = res; i < count; i++, pirq++, r++) {
> -		if (i)
> -			irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
> -						ACPI_ACTIVE_HIGH);
> -		r->start = r->end = irq;
> -		r->flags = IORESOURCE_IRQ;
> -		if (pirq->trigger == ACPI_EDGE_SENSITIVE)
> -			r->flags |= IORESOURCE_IRQ_HIGHEDGE;
> -		else
> -			r->flags |= IORESOURCE_IRQ_HIGHLEVEL;
> +	arm_pmu_acpi_determine_cpu_types(&pmus);
> +
> +	list_for_each_entry_safe(pmu, safe_temp, &pmus, list) {
> +		res = kcalloc(pmu->cpu_count,
> +			      sizeof(struct resource), GFP_KERNEL);
> +
> +		/* for a given PMU type collect all the GSIs. */
> +		if (res) {
> +			count = arm_pmu_acpi_gsi_res(pmu, res,
> +						     &cpu_id);
> +			/*
> +			 * register this set of interrupts
> +			 * with a new PMU device
> +			 */
> +			err = arm_pmu_acpi_register_pmu(count, res, cpu_id);
> +			kfree(res);
> +		} else
> +			pr_warn("PMU unable to allocate interrupt resource space\n");
> +
> +		list_del(&pmu->list);
> +		kfree(pmu);
>  	}
>  
> -	err = platform_device_add_resources(pdev, res, count);
> -	if (!err)
> -		err = platform_device_add(pdev);
> -	kfree(res);
> -	if (!err)
> -		return 0;
> -
> -err_free_device:
> -	platform_device_put(pdev);
> -
> -err_free_gsi:
> -	for (i = 0; i < count; i++)
> -		acpi_unregister_gsi(pmu_irqs[i].gsi);
> -
>  	return err;
>  }
> +
>  arch_initcall(pmu_acpi_init);

The changes to arm_acpi_pmu.c should be moved to the previous
patch. It'll help see introduction of functionality and it's use in one
patch - aiding review.

[PATCH 11/11] MAINTAINERS: Tweak ARM PMU maintainers

[Jeremy Linton]

Update the ARM PMU file list, and add the arm mailing list.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e1b090f..8293773 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -878,12 +878,13 @@ ARM PMU PROFILING AND DEBUGGING
 M:	Will Deacon <will.deacon@arm.com>
 R:	Mark Rutland <mark.rutland@arm.com>
 S:	Maintained
+L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
 F:	arch/arm*/kernel/perf_*
 F:	arch/arm/oprofile/common.c
 F:	arch/arm*/kernel/hw_breakpoint.c
 F:	arch/arm*/include/asm/hw_breakpoint.h
 F:	arch/arm*/include/asm/perf_event.h
-F:	drivers/perf/arm_pmu.c
+F:	drivers/perf/arm_pmu*
 F:	include/linux/perf/arm_pmu.h
 
 ARM PORT
-- 
2.5.5