[PATCH 0/8] arm64: pmu: Detect multiple PMU types in an ACPI system

[PATCH 0/8] arm64: pmu: Detect multiple PMU types in an ACPI system

[Jeremy Linton]

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 (6):
  arm64: pmu: Probe default hw/cache counters
  arm: arm64: Add routine to determine cpuid of other cpus
  arm: arm64: pmu: Assign platform PMU CPU affinity
  arm64: pmu: Add routines for detecting differing PMU types in the
  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   |  51 ++++++++-
 arch/arm64/kernel/smp.c          |   5 +
 drivers/perf/Kconfig             |   4 +
 drivers/perf/Makefile            |   1 +
 drivers/perf/arm_pmu.c           |  57 +++++++++--
 drivers/perf/arm_pmu_acpi.c      | 216 +++++++++++++++++++++++++++++++++++++++
 include/linux/perf/arm_pmu.h     |  10 ++
 10 files changed, 341 insertions(+), 16 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c

-- 
2.5.5

[PATCH 1/8] 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 | 8 +++++++-
 include/linux/perf/arm_pmu.h   | 3 +++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 838ccf1..917c3e2 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1044,9 +1044,15 @@ 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);
+	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/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 1/8] arm64: pmu: add fallback probe table

[Will Deacon]

On Thu, Jun 09, 2016 at 05:23:26PM -0500, Jeremy Linton wrote:
> 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 | 8 +++++++-
>  include/linux/perf/arm_pmu.h   | 3 +++
>  2 files changed, 10 insertions(+), 1 deletion(-)

This is likely to do the wrong thing for big/little systems, so I'm not
keen on this for DT. We should mandate that the PMU is described there.

If you need this as a building block for ACPI, please ensure that the
code is only executed when booting via ACPI.

Will

[PATCH 2/8] 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>
---
 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 917c3e2..e1a3bce 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -190,13 +190,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. */
@@ -258,6 +268,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,
@@ -905,9 +924,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)
@@ -1044,8 +1076,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 2/8] arm64: pmu: Probe default hw/cache counters

[Will Deacon]

On Thu, Jun 09, 2016 at 05:23:27PM -0500, Jeremy Linton wrote:
> 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>
> ---
>  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 917c3e2..e1a3bce 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -190,13 +190,23 @@
>  #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_MISS		0xED

Minus the last hunk (which I mentioned in the previous patch):

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH 3/8] 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 a later version
       of this patch. The later version of the patch cleans up some 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  | 97 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/perf/arm_pmu.h |  7 ++++
 5 files changed, 114 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 678e084..5c96d23 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>
@@ -540,6 +541,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 			return;
 		}
 		bootcpu_valid = true;
+		arm_pmu_parse_acpi(0, processor);
 		return;
 	}
 
@@ -560,6 +562,9 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 	 */
 	acpi_set_mailbox_entry(cpu_count, processor);
 
+	/* get PMU irq info */
+	arm_pmu_parse_acpi(cpu_count, processor);
+
 	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..98c452d
--- /dev/null
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -0,0 +1,97 @@
+/*
+ * 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>
+
+#define PMU_PDEV_NAME "armv8-pmu"
+
+struct pmu_irq {
+	int gsi;
+	int trigger;
+};
+
+static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
+
+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 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(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 7e814fe..9ed9fdc 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -156,4 +156,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 3/8] arm64: pmu: Add support for probing with ACPI

[Will Deacon]

On Thu, Jun 09, 2016 at 05:23:28PM -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 a later version
>        of this patch. The later version of the patch cleans up some 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  | 97 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/perf/arm_pmu.h |  7 ++++
>  5 files changed, 114 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 678e084..5c96d23 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>
> @@ -540,6 +541,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>  			return;
>  		}
>  		bootcpu_valid = true;
> +		arm_pmu_parse_acpi(0, processor);
>  		return;
>  	}
>  
> @@ -560,6 +562,9 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>  	 */
>  	acpi_set_mailbox_entry(cpu_count, processor);
>  
> +	/* get PMU irq info */
> +	arm_pmu_parse_acpi(cpu_count, processor);
> +

Nit: the outer functions are now misnomers, since this has nothing to do
with the GIC. It feels like acpi_parse_gic_cpu_interface could use some
slight restructuring so that the MADT parsing looks less confused.

> 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..98c452d
> --- /dev/null
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -0,0 +1,97 @@
> +/*
> + * 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>
> +
> +#define PMU_PDEV_NAME "armv8-pmu"

Stick this in include/linux/perf/arm_pmu.h where we can use it in the driver
code too?

> +
> +struct pmu_irq {
> +	int gsi;
> +	int trigger;
> +};
> +
> +static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
> +
> +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 boot cpu, at least */

boot boot

> +	if (pirq->gsi == 0)
> +		return -EINVAL;
> +
> +	irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
> +				ACPI_ACTIVE_HIGH);

This is quite tricky to read, thanks to the aliasing of pirq and
pmu_irqs[0]. Why is it necessary to register the first gsi separately,
rather than just register it later in the loop with all the other
interrupts?

Will

[PATCH 3/8] arm64: pmu: Add support for probing with ACPI

[Jeremy Linton]

On 06/15/2016 06:33 AM, Will Deacon wrote:
> On Thu, Jun 09, 2016 at 05:23:28PM -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 a later version
>>         of this patch. The later version of the patch cleans up some 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  | 97 ++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/perf/arm_pmu.h |  7 ++++
>>   5 files changed, 114 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 678e084..5c96d23 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>
>> @@ -540,6 +541,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>>   			return;
>>   		}
>>   		bootcpu_valid = true;
>> +		arm_pmu_parse_acpi(0, processor);
>>   		return;
>>   	}
>>
>> @@ -560,6 +562,9 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>>   	 */
>>   	acpi_set_mailbox_entry(cpu_count, processor);
>>
>> +	/* get PMU irq info */
>> +	arm_pmu_parse_acpi(cpu_count, processor);
>> +
>
> Nit: the outer functions are now misnomers, since this has nothing to do
> with the GIC. It feels like acpi_parse_gic_cpu_interface could use some
> slight restructuring so that the MADT parsing looks less confused.


Ok, I will clean up the naming a bit.


>
>> 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..98c452d
>> --- /dev/null
>> +++ b/drivers/perf/arm_pmu_acpi.c
>> @@ -0,0 +1,97 @@
>> +/*
>> + * 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>
>> +
>> +#define PMU_PDEV_NAME "armv8-pmu"
>
> Stick this in include/linux/perf/arm_pmu.h where we can use it in the driver
> code too?

Sure..

>
>> +
>> +struct pmu_irq {
>> +	int gsi;
>> +	int trigger;
>> +};
>> +
>> +static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
>> +
>> +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 boot cpu, at least */
>
> boot boot
>
>> +	if (pirq->gsi == 0)
>> +		return -EINVAL;
>> +
>> +	irq = acpi_register_gsi(NULL, pirq->gsi, pirq->trigger,
>> +				ACPI_ACTIVE_HIGH);
>
> This is quite tricky to read, thanks to the aliasing of pirq and
> pmu_irqs[0]. Why is it necessary to register the first gsi separately,
> rather than just register it later in the loop with all the other
> interrupts?

	Short answer, no particular reason. If you notice patch 6, 
arm_pmu_acpi_gsi_res() reworks this to register all the irqs for a 
particular PMU at the same time.

[PATCH 4/8] 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 4/8] arm: arm64: Add routine to determine cpuid of other cpus

[Punit Agrawal]

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

> 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

IMO, something like read_cpuid_of might be better name.

> +
>  /*
>   * 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

[PATCH 5/8] 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 | 52 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index fee4be0e8..865a9db 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,56 @@ 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;
+	struct platform_device *pdev = pmu->plat_device;
+	int irq = platform_get_irq(pdev, 0);
 
-	pr_info("probing PMU on CPU %d\n", cpu);
+	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);
+
+		if (cpuid == pmuid) {
+			cpumask_set_cpu(cpu, &pmu->supported_cpus);
+			pr_devel("enable pmu on cpu %d\n", cpu);
+			if (pmu->irq_affinity) {
+				pmu->irq_affinity[aff_ctr] = cpu;
+				aff_ctr++;
+			}
+		}
+	}
 
+	pr_debug("probing PMU %X\n", pmuid);
+	/* 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;
+		pr_devel("Found PMU\n");
 		ret = info->init(pmu);
+		if (!info->cpuid) {
+			pmu->name = kasprintf(GFP_KERNEL, "%s_0x%x",
+					    pmu->name, ARM_PARTNUM(pmuid));
+			if (!pmu->name)
+				return -ENOMEM;
+		}
 		break;
 	}
 
-	put_cpu();
 	return ret;
 }
 
@@ -1016,8 +1049,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		if (!ret)
 			ret = init_fn(pmu);
 	} else {
-		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 5/8] arm: arm64: pmu: Assign platform PMU CPU affinity

[Will Deacon]

On Thu, Jun 09, 2016 at 05:23:30PM -0500, Jeremy Linton wrote:
> 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 | 52 ++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index fee4be0e8..865a9db 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,56 @@ 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;
> +	struct platform_device *pdev = pmu->plat_device;
> +	int irq = platform_get_irq(pdev, 0);
>  
> -	pr_info("probing PMU on CPU %d\n", cpu);
> +	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);
> +
> +		if (cpuid == pmuid) {
> +			cpumask_set_cpu(cpu, &pmu->supported_cpus);
> +			pr_devel("enable pmu on cpu %d\n", cpu);

Please remove the pr_devels that you've added (similarly elsewhere in
the patch).

> +			if (pmu->irq_affinity) {
> +				pmu->irq_affinity[aff_ctr] = cpu;
> +				aff_ctr++;
> +			}
> +		}
> +	}
>  
> +	pr_debug("probing PMU %X\n", pmuid);

You can also kill this now -- I don't think it's much use.

> +	/* 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;
> +		pr_devel("Found PMU\n");
>  		ret = info->init(pmu);
> +		if (!info->cpuid) {
> +			pmu->name = kasprintf(GFP_KERNEL, "%s_0x%x",
> +					    pmu->name, ARM_PARTNUM(pmuid));

Hmm, I'm not so keen on this. I think we should name the first PMU
"armv8_pmuv3" (i.e. pmu->name) and then any subsequent PMUs should inherit
an index, e.g. "armv8_pmuv3_1". That follows a similar style to uncore
PMUs and also keeps this backwards compatible with what's currently done
with devicetree.

You can create a 'cpumask' file in the sysfs directory that identifies
the cores for the PMU (again, there is precedent for this elsewhere).

Will

[PATCH 5/8] arm: arm64: pmu: Assign platform PMU CPU affinity

[Punit Agrawal]

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.
>

One comment below.

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  drivers/perf/arm_pmu.c | 52 ++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index fee4be0e8..865a9db 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c

[...]

> @@ -872,25 +874,56 @@ 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;
> +	struct platform_device *pdev = pmu->plat_device;
> +	int irq = platform_get_irq(pdev, 0);
>  
> -	pr_info("probing PMU on CPU %d\n", cpu);
> +	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);
> +
> +		if (cpuid == pmuid) {
> +			cpumask_set_cpu(cpu, &pmu->supported_cpus);
> +			pr_devel("enable pmu on cpu %d\n", cpu);
> +			if (pmu->irq_affinity) {

The check for irq_affinity can be dropped as you already do that earlier.

> +				pmu->irq_affinity[aff_ctr] = cpu;
> +				aff_ctr++;
> +			}
> +		}
> +	}
>  
> +	pr_debug("probing PMU %X\n", pmuid);
> +	/* 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;
> +		pr_devel("Found PMU\n");
>  		ret = info->init(pmu);
> +		if (!info->cpuid) {
> +			pmu->name = kasprintf(GFP_KERNEL, "%s_0x%x",
> +					    pmu->name, ARM_PARTNUM(pmuid));
> +			if (!pmu->name)
> +				return -ENOMEM;
> +		}
>  		break;
>  	}
>  
> -	put_cpu();
>  	return ret;
>  }
>  

[...]

[PATCH 5/8] arm: arm64: pmu: Assign platform PMU CPU affinity

[Jeremy Linton]

|-----Original Message-----
|From: Punit Agrawal [mailto:punit.agrawal at arm.com]
|Sent: Monday, June 20, 2016 11:41 AM
|To: Jeremy Linton
|Cc: linux-arm-kernel at lists.infradead.org; Mark Rutland; Lorenzo Pieralisi;
|mlangsdorf at redhat.com; peterz at infradead.org; Catalin Marinas; Will
|Deacon; acme at kernel.org; linux-acpi at vger.kernel.org;
|alexander.shishkin at linux.intel.com; mingo at redhat.com
|Subject: Re: [PATCH 5/8] arm: arm64: pmu: Assign platform PMU CPU affinity
|
|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.
|>
|
|One comment below.
|
|> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
|> ---
|>  drivers/perf/arm_pmu.c | 52
|++++++++++++++++++++++++++++++++++++++++----------
|>  1 file changed, 42 insertions(+), 10 deletions(-)
|>
|> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
|> index fee4be0e8..865a9db 100644
|> --- a/drivers/perf/arm_pmu.c
|> +++ b/drivers/perf/arm_pmu.c
|
|[...]
|
|> @@ -872,25 +874,56 @@ 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;
|> +struct platform_device *pdev = pmu->plat_device;
|> +int irq = platform_get_irq(pdev, 0);
|>
|> -pr_info("probing PMU on CPU %d\n", cpu);
|> +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);
|> +
|> +if (cpuid == pmuid) {
|> +cpumask_set_cpu(cpu, &pmu->supported_cpus);
|> +pr_devel("enable pmu on cpu %d\n", cpu);
|> +if (pmu->irq_affinity) {
|
|The check for irq_affinity can be dropped as you already do that earlier.

There isn't an irq_affinity structure for machines with PMU's using PPI's, that is why the check is there...

Thanks,

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH 5/8] arm: arm64: pmu: Assign platform PMU CPU affinity

[Punit Agrawal]

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

> |-----Original Message-----
> |From: Punit Agrawal [mailto:punit.agrawal at arm.com]
> |Sent: Monday, June 20, 2016 11:41 AM
> |To: Jeremy Linton
> |Cc: linux-arm-kernel at lists.infradead.org; Mark Rutland; Lorenzo Pieralisi;
> |mlangsdorf at redhat.com; peterz at infradead.org; Catalin Marinas; Will
> |Deacon; acme at kernel.org; linux-acpi at vger.kernel.org;
> |alexander.shishkin at linux.intel.com; mingo at redhat.com
> |Subject: Re: [PATCH 5/8] arm: arm64: pmu: Assign platform PMU CPU affinity
> |
> |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.
> |>
> |
> |One comment below.
> |
> |> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> |> ---
> |>  drivers/perf/arm_pmu.c | 52
> |++++++++++++++++++++++++++++++++++++++++----------
> |>  1 file changed, 42 insertions(+), 10 deletions(-)
> |>
> |> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> |> index fee4be0e8..865a9db 100644
> |> --- a/drivers/perf/arm_pmu.c
> |> +++ b/drivers/perf/arm_pmu.c
> |
> |[...]
> |
> |> @@ -872,25 +874,56 @@ 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;
> |> +struct platform_device *pdev = pmu->plat_device;
> |> +int irq = platform_get_irq(pdev, 0);
> |>
> |> -pr_info("probing PMU on CPU %d\n", cpu);
> |> +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);
> |> +
> |> +if (cpuid == pmuid) {
> |> +cpumask_set_cpu(cpu, &pmu->supported_cpus);
> |> +pr_devel("enable pmu on cpu %d\n", cpu);
> |> +if (pmu->irq_affinity) {
> |
> |The check for irq_affinity can be dropped as you already do that earlier.
>
> There isn't an irq_affinity structure for machines with PMU's using
> PPI's, that is why the check is there...

You're right. I missed the PPIs case. Apologies for the noise.

>
> Thanks,
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose
> the contents to any other person, use it for any purpose, or store or
> copy the information in any medium. Thank you.
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/8] 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 | 135 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 132 insertions(+), 3 deletions(-)

diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index 98c452d..a257fc0 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -1,14 +1,19 @@
 /*
- * 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>
@@ -18,8 +23,14 @@
 #define PMU_PDEV_NAME "armv8-pmu"
 
 struct pmu_irq {
-	int gsi;
-	int trigger;
+	int  gsi;
+	int  trigger;
+	bool registered;
+};
+
+struct pmu_types {
+	int cpu_type;
+	int cpu_count;
 };
 
 static struct pmu_irq pmu_irqs[NR_CPUS] __initdata;
@@ -31,6 +42,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)
 		pmu_irqs[cpu].trigger = ACPI_EDGE_SENSITIVE;
 	else
 		pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE;
+	pr_devel("Assign CPU %d girq %d level %d\n", cpu, pmu_irqs[cpu].gsi,
+						   pmu_irqs[cpu].trigger);
+}
+
+/* Count number and type of CPU cores in the system. */
+void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
+{
+	int i, j;
+
+	for_each_possible_cpu(i) {
+		struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
+		u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);
+
+		pr_devel("Present CPU %d is a %X\n", i, partnum);
+		for (j = 0; j < num_possible_cpus(); j++) {
+			if (pmus[j].cpu_type == partnum) {
+				pmus[j].cpu_count++;
+				break;
+			}
+			if (pmus[j].cpu_count == 0) {
+				pmus[j].cpu_type = partnum;
+				pmus[j].cpu_count++;
+				break;
+			}
+		}
+	}
+}
+
+/*
+ * Registers the group of PMU interfaces which corrispond to the 'last_cpu_id'.
+ * This group utlizes '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(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)) {
+			pr_devel("Setting up CPU %d\n", i);
+			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;
+
+			if (irq_is_percpu(irq))
+				pr_debug("PPI detected\n");
+		}
+	}
+	return count;
 }
 
 static int __init pmu_acpi_init(void)
-- 
2.5.5

[PATCH 7/8] 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 | 98 ++++++++++++++++++++-------------------------
 2 files changed, 50 insertions(+), 55 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 865a9db..97007ec 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -1049,7 +1049,12 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		if (!ret)
 			ret = init_fn(pmu);
 	} else {
-		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 a257fc0..8f9bea3 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -35,6 +35,11 @@ struct pmu_types {
 
 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;
@@ -47,7 +52,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 pmu_types *pmus)
+static void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
 {
 	int i, j;
 
@@ -74,7 +79,7 @@ void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
  * Registers the group of PMU interfaces which corrispond to the 'last_cpu_id'.
  * This group utlizes '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;
@@ -121,7 +126,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;
@@ -164,63 +169,48 @@ 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;
+	int j, last_cpu_id;
+	struct pmu_types *pmus;
 
+	pr_debug("Prepare registration\n");
 	if (acpi_disabled)
 		return 0;
 
-	/* Must have irq for boot 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(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);
+	pmus = kcalloc(num_possible_cpus(), sizeof(struct pmu_types),
+		       GFP_KERNEL);
+
+	if (pmus) {
+		arm_pmu_acpi_determine_cpu_types(pmus);
+
+		for (j = 0; pmus[j].cpu_count; j++) {
+			pr_devel("CPU type %X, count %d\n", pmus[j].cpu_type,
+				 pmus[j].cpu_count);
+			res = kcalloc(pmus[j].cpu_count,
+				      sizeof(struct resource), GFP_KERNEL);
+
+			/* for a given PMU type collect all the GSIs. */
+			if (res) {
+				count = arm_pmu_acpi_gsi_res(&pmus[j], res,
+							     &last_cpu_id);
+				/*
+				 * register this set of interrupts
+				 * with a new PMU device
+				 */
+				err = arm_pmu_acpi_register_pmu(count,
+								res,
+								last_cpu_id);
+				kfree(res);
+			} else
+				pr_warn("PMU unable to allocate interrupt resource space\n");
+		}
 
+		kfree(pmus);
+	} else
+		pr_warn("PMU: Unable to allocate pmu count structures\n");
 	return err;
 }
+
 arch_initcall(pmu_acpi_init);
-- 
2.5.5

[PATCH 7/8] arm64: pmu: Enable multiple PMUs in an ACPI system

[Will Deacon]

On Thu, Jun 09, 2016 at 05:23:32PM -0500, Jeremy Linton wrote:
> 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 | 98 ++++++++++++++++++++-------------------------
>  2 files changed, 50 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 865a9db..97007ec 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -1049,7 +1049,12 @@ int arm_pmu_device_probe(struct platform_device *pdev,
>  		if (!ret)
>  			ret = init_fn(pmu);
>  	} else {
> -		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 a257fc0..8f9bea3 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -35,6 +35,11 @@ struct pmu_types {

[...]

> +	pmus = kcalloc(num_possible_cpus(), sizeof(struct pmu_types),
> +		       GFP_KERNEL);
> +
> +	if (pmus) {
> +		arm_pmu_acpi_determine_cpu_types(pmus);
> +
> +		for (j = 0; pmus[j].cpu_count; j++) {
> +			pr_devel("CPU type %X, count %d\n", pmus[j].cpu_type,
> +				 pmus[j].cpu_count);
> +			res = kcalloc(pmus[j].cpu_count,
> +				      sizeof(struct resource), GFP_KERNEL);

Given that you already have dynamic allocation in here, why not use a
linked-list for the pmus list, and avoid having a potentially huge temporary
data structure?

Will

[PATCH 7/8] arm64: pmu: Enable multiple PMUs in an ACPI system

[Jeremy Linton]

On 06/15/2016 08:22 AM, Will Deacon wrote:
> On Thu, Jun 09, 2016 at 05:23:32PM -0500, Jeremy Linton wrote:
>> 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.
>>
>
> [...]
>
>> +	pmus = kcalloc(num_possible_cpus(), sizeof(struct pmu_types),
>> +		       GFP_KERNEL);
>> +
>> +	if (pmus) {
>> +		arm_pmu_acpi_determine_cpu_types(pmus);
>> +
>> +		for (j = 0; pmus[j].cpu_count; j++) {
>> +			pr_devel("CPU type %X, count %d\n", pmus[j].cpu_type,
>> +				 pmus[j].cpu_count);
>> +			res = kcalloc(pmus[j].cpu_count,
>> +				      sizeof(struct resource), GFP_KERNEL);
>
> Given that you already have dynamic allocation in here, why not use a
> linked-list for the pmus list, and avoid having a potentially huge temporary
> data structure?

Sure... But, its really only going to be 2 entries on any existing 
system, I considered limiting this to something reasonable like "4" with 
a WARN() because who will ever build a machine with more than 4 
different CPU types in it? <chuckle> Is that an acceptable solution, or 
do you prefer the list?

Although, maybe the list is better, because then when someone does build 
such a machine it will "just work (TM)". <shrug>

[PATCH 7/8] arm64: pmu: Enable multiple PMUs in an ACPI system

[Will Deacon]

On Wed, Jun 15, 2016 at 10:21:12AM -0500, Jeremy Linton wrote:
> On 06/15/2016 08:22 AM, Will Deacon wrote:
> >On Thu, Jun 09, 2016 at 05:23:32PM -0500, Jeremy Linton wrote:
> >>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.
> >>
> >
> >[...]
> >
> >>+	pmus = kcalloc(num_possible_cpus(), sizeof(struct pmu_types),
> >>+		       GFP_KERNEL);
> >>+
> >>+	if (pmus) {
> >>+		arm_pmu_acpi_determine_cpu_types(pmus);
> >>+
> >>+		for (j = 0; pmus[j].cpu_count; j++) {
> >>+			pr_devel("CPU type %X, count %d\n", pmus[j].cpu_type,
> >>+				 pmus[j].cpu_count);
> >>+			res = kcalloc(pmus[j].cpu_count,
> >>+				      sizeof(struct resource), GFP_KERNEL);
> >
> >Given that you already have dynamic allocation in here, why not use a
> >linked-list for the pmus list, and avoid having a potentially huge temporary
> >data structure?
> 
> Sure... But, its really only going to be 2 entries on any existing system, I
> considered limiting this to something reasonable like "4" with a WARN()
> because who will ever build a machine with more than 4 different CPU types
> in it? <chuckle> Is that an acceptable solution, or do you prefer the list?

I do prefer the list, just because kcalloc(num_possible_cpus(), ...) could
be pretty large, and like you say, we're likely to need 2-3 entries in
practice.

Will

[PATCH 7/8] 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 | 98 ++++++++++++++++++++-------------------------
>  2 files changed, 50 insertions(+), 55 deletions(-)
>

[...]

> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index a257fc0..8f9bea3 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -35,6 +35,11 @@ struct pmu_types {
>  
>  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;


This hunk should be in Patch 3.

> @@ -47,7 +52,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 pmu_types *pmus)
> +static void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
>  {
>  	int i, j;
>  

Please move this and the following two hunks to the previous patch where you
introduce the functions.

> @@ -74,7 +79,7 @@ void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
>   * Registers the group of PMU interfaces which corrispond to the 'last_cpu_id'.
>   * This group utlizes '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;
> @@ -121,7 +126,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;
> @@ -164,63 +169,48 @@ 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;
> +	int j, last_cpu_id;
> +	struct pmu_types *pmus;
>  
> +	pr_debug("Prepare registration\n");
>  	if (acpi_disabled)
>  		return 0;
>  
> -	/* Must have irq for boot 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(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);
> +	pmus = kcalloc(num_possible_cpus(), sizeof(struct pmu_types),
> +		       GFP_KERNEL);
> +
> +	if (pmus) {

Instead, check for error here. That should allow you to reduce
indentation on the block below.

> +		arm_pmu_acpi_determine_cpu_types(pmus);
> +
> +		for (j = 0; pmus[j].cpu_count; j++) {
> +			pr_devel("CPU type %X, count %d\n", pmus[j].cpu_type,
> +				 pmus[j].cpu_count);
> +			res = kcalloc(pmus[j].cpu_count,
> +				      sizeof(struct resource), GFP_KERNEL);
> +
> +			/* for a given PMU type collect all the GSIs. */
> +			if (res) {
> +				count = arm_pmu_acpi_gsi_res(&pmus[j], res,
> +							     &last_cpu_id);
> +				/*
> +				 * register this set of interrupts
> +				 * with a new PMU device
> +				 */
> +				err = arm_pmu_acpi_register_pmu(count,
> +								res,
> +								last_cpu_id);
> +				kfree(res);
> +			} else
> +				pr_warn("PMU unable to allocate interrupt resource space\n");
> +		}
>  
> +		kfree(pmus);
> +	} else
> +		pr_warn("PMU: Unable to allocate pmu count structures\n");
>  	return err;
>  }
> +
>  arch_initcall(pmu_acpi_init);

[PATCH 7/8] arm64: pmu: Enable multiple PMUs in an ACPI system

[Jeremy Linton]

Hi Punit,

On 06/20/2016 11:37 AM, Punit Agrawal wrote:
> 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 | 98 ++++++++++++++++++++-------------------------
>>   2 files changed, 50 insertions(+), 55 deletions(-)
>>
>
> [...]
>
>> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
>> index a257fc0..8f9bea3 100644
>> --- a/drivers/perf/arm_pmu_acpi.c
>> +++ b/drivers/perf/arm_pmu_acpi.c
>> @@ -35,6 +35,11 @@ struct pmu_types {
>>
>>   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;
>
>
> This hunk should be in Patch 3.


Ok...

>
>> @@ -47,7 +52,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 pmu_types *pmus)
>> +static void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
>>   {
>>   	int i, j;
>>
>
> Please move this and the following two hunks to the previous patch where you
> introduce the functions.


Oh, I did this on purpose to avoid:

"warning: 'xxx' defined but not used" messages.



>
>> @@ -74,7 +79,7 @@ void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
>>    * Registers the group of PMU interfaces which corrispond to the 'last_cpu_id'.
>>    * This group utlizes '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,

[PATCH 7/8] arm64: pmu: Enable multiple PMUs in an ACPI system

[Punit Agrawal]

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

> Hi Punit,
>
> On 06/20/2016 11:37 AM, Punit Agrawal wrote:
>> 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 | 98 ++++++++++++++++++++-------------------------
>>>   2 files changed, 50 insertions(+), 55 deletions(-)
>>>

[...]

>>> @@ -47,7 +52,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 pmu_types *pmus)
>>> +static void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
>>>   {
>>>   	int i, j;
>>>
>>
>> Please move this and the following two hunks to the previous patch where you
>> introduce the functions.
>
>
> Oh, I did this on purpose to avoid:
>
> "warning: 'xxx' defined but not used" messages.
>

In that case, I'd suggest merging the patches. IMO, it's better to have
self-contained patches be they slightly large than to introduce
artifacts like here.

>
>
>>
>>> @@ -74,7 +79,7 @@ void __init arm_pmu_acpi_determine_cpu_types(struct pmu_types *pmus)
>>>    * Registers the group of PMU interfaces which corrispond to the 'last_cpu_id'.
>>>    * This group utlizes '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,
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 8/8] 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 ed42cb6..cd1f9d9 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

[PATCH 0/8] arm64: pmu: Detect multiple PMU types in an ACPI system

[Punit Agrawal]

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

> 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.
>

Hi Jeremy,

I have a few comments on the individual patches but overall I found the
patches a bit confusing due to introduction of functionality and then
changing them in later patches.

IMHO, some patch re-organisation to make the series better as a whole will
help in review (even though it may lead to more invasive changes
to Mark's patches).

Thanks,
Punit


> Jeremy Linton (6):
>   arm64: pmu: Probe default hw/cache counters
>   arm: arm64: Add routine to determine cpuid of other cpus
>   arm: arm64: pmu: Assign platform PMU CPU affinity
>   arm64: pmu: Add routines for detecting differing PMU types in the
>   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   |  51 ++++++++-
>  arch/arm64/kernel/smp.c          |   5 +
>  drivers/perf/Kconfig             |   4 +
>  drivers/perf/Makefile            |   1 +
>  drivers/perf/arm_pmu.c           |  57 +++++++++--
>  drivers/perf/arm_pmu_acpi.c      | 216 +++++++++++++++++++++++++++++++++++++++
>  include/linux/perf/arm_pmu.h     |  10 ++
>  10 files changed, 341 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/perf/arm_pmu_acpi.c