Skip to contentSkip to conversation

arm_pmu: ACPI support

PATCH Superseded Mark Rutland 3 participants 25 messages Apr 6, 2017 – Apr 7, 2017

Subsystems (4)
perf events arm64 Arm PMU profiling and debugging CPU hotplug

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Mark Rutland

author maintainer
#0 lore raw

v2 cover arm_pmu: ACPI support

Hi,

This series implements ACPI support in the ARM PMU code. It borrows some code from Jeremy's series [1], but takes a different approach to probing and association, using the usual hotplug state machine, minimising external changes required, and simplifying the relationship with the common arm_pmu code.

The first few patches are preparatory cleanup/refactoring, with the latter half of the series being specific to ACPI support.

The series is based on Will's perf/updates branch [2]. I've pushed the whole series out to the arm/perf/acpi branch [3] of my kernel.org repo.

Due to the innards of the hotplug callback framework, it's not entirely safe to register a PMU in a hotplug callback. Due to this, we can only associated hotplugged CPUs with a PMU if a matching CPU was around at probe time. A similar restriction already applies to DT systems. We may be able to relax this with some future work.

I've given this some testing on a Juno platform (using SPIs). To see that IRQs are correctly associated, I've tested with the following:

  $ taskset -c ${SOME_CPU_HERE} perf record \
    -e armv8_pmuv3_0/cpu_cycles/ \
    -e armv8_pmuv3_1/cpu_cycles/ \
    cat /proc/interrupts

I've also booted with nr_cpus temporarily capped (passing maxcpus=) to test the association logic. This has also been tested in a VM using PPIs; I do not have access to a host machine which itself uses PPIs.

Since v1 [4]:
* Fix up ACPI bits per Lorenzo's request
* Accumulate tags
* Check presence of PMUv3

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/482397.html
[2] git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git perf/updates
[3] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm/perf/acpi
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/492897.html

Mark Rutland (16):
drivers/perf: arm_pmu: remove pointless PMU disabling drivers/perf: arm_pmu: define armpmu_init_fn drivers/perf: arm_pmu: fold init into alloc drivers/perf: arm_pmu: factor out pmu registration drivers/perf: arm_pmu: simplify cpu_pmu_request_irqs() drivers/perf: arm_pmu: handle no platform_device drivers/perf: arm_pmu: rename irq request/free functions drivers/perf: arm_pmu: split cpu-local irq request/free drivers/perf: arm_pmu: move irq request/free into probe drivers/perf: arm_pmu: split out platform device probe logic
arm64: add function to get a cpu's MADT GICC table
arm64: parking: kill acpi_set_mailbox_entry()
arm64: parking: fix type endianness
drivers/perf: arm_pmu: add ACPI framework
arm64: pmuv3: handle !PMUv3 when probing
arm64: pmuv3: use arm_pmu ACPI framework

 arch/arm64/include/asm/acpi.h             |   7 +-
 arch/arm64/kernel/acpi_parking_protocol.c |  44 ++--
 arch/arm64/kernel/perf_event.c            | 111 ++++++---
 arch/arm64/kernel/smp.c                   |  19 +-
 drivers/perf/Kconfig                      |   4 +
 drivers/perf/Makefile                     |   3 +-
 drivers/perf/arm_pmu.c                    | 387 ++++++++----------------------
 drivers/perf/arm_pmu_acpi.c               | 240 ++++++++++++++++++
 drivers/perf/arm_pmu_platform.c           | 235 ++++++++++++++++++
 include/linux/cpuhotplug.h                |   1 +
 include/linux/perf/arm_pmu.h              |  22 +-
 11 files changed, 704 insertions(+), 369 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c
 create mode 100644 drivers/perf/arm_pmu_platform.c
Signature · 1 line
1.9.1
author maintainer
#1 lore raw

PATCH v2 1/16 drivers/perf: arm_pmu: remove pointless PMU disabling

We currently disable the PMU temporarily in armpmu_add(). We may have required this historically, but the perf core always disables an event's PMU when calling event::pmu::add(), so this is not necessary.

We don't do similarly in armpmu_del(), or elsewhere, so this is unnecessary and inconsistent, and only serves to confuse the reader.

Remove the pointless disable, simplifying armpmu_add() in the process.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/perf/arm_pmu.c +3 -10
index a1dfe89..316c4dc 100644
@@ -235,20 +235,15 @@ static void armpmu_start(struct perf_event *event, int flags)
 	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
 	struct hw_perf_event *hwc = &event->hw;
 	int idx;
-	int err = 0;
 
 	/* An event following a process won't be stopped earlier */
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
 		return -ENOENT;
 
-	perf_pmu_disable(event->pmu);
-
 	/* If we don't have a space for the counter then finish early. */
 	idx = armpmu->get_event_idx(hw_events, event);
-	if (idx < 0) {
-		err = idx;
-		goto out;
-	}
+	if (idx < 0)
+		return idx;
 
 	/*
 	 * If there is an event in the counter we are going to use then make

@@ -265,9 +260,7 @@ static void armpmu_start(struct perf_event *event, int flags)
 	/* Propagate our changes to the userspace mapping. */
 	perf_event_update_userpage(event);
 
-out:
-	perf_pmu_enable(event->pmu);
-	return err;
+	return 0;
 }
 
 static int
Signature · 1 line
1.9.1
author maintainer
#2 lore raw

PATCH v2 2/16 drivers/perf: arm_pmu: define armpmu_init_fn

In reply to Mark Rutland

We expect an ARM PMU's init function to have a particular prototype, which we open-code in a few places. This is less than ideal, considering that we cast a void value to this type in one location, and a mismatch could easily be missed.

Add a typedef so that we can ensure this is consistent.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c       | 2 +-
 include/linux/perf/arm_pmu.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/perf/arm_pmu.c +1 -1
index 316c4dc..71a825d 100644
@@ -989,7 +989,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 			 const struct pmu_probe_info *probe_table)
 {
 	const struct of_device_id *of_id;
-	const int (*init_fn)(struct arm_pmu *);
+	armpmu_init_fn init_fn;
 	struct device_node *node = pdev->dev.of_node;
 	struct arm_pmu *pmu;
 	int ret = -ENODEV;
include/linux/perf/arm_pmu.h +3 -1
index 44f43fc..4249914 100644
@@ -132,10 +132,12 @@ int armpmu_map_event(struct perf_event *event,
 						[PERF_COUNT_HW_CACHE_RESULT_MAX],
 		     u32 raw_event_mask);
 
+typedef int (*armpmu_init_fn)(struct arm_pmu *);
+
 struct pmu_probe_info {
 	unsigned int cpuid;
 	unsigned int mask;
-	int (*init)(struct arm_pmu *);
+	armpmu_init_fn init;
 };
 
 #define PMU_PROBE(_cpuid, _mask, _fn)	\
Signature · 1 line
1.9.1
author maintainer
#3 lore raw

PATCH v2 3/16 drivers/perf: arm_pmu: fold init into alloc

In reply to Mark Rutland

Given we always want to initialise common fields on an allocated PMU, this patch folds this common initialisation into armpmu_alloc(). This will make it simpler to reuse this code for an ACPI-specific probe path.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 52 +++++++++++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 28 deletions(-)
drivers/perf/arm_pmu.c +24 -28
index 71a825d..1cb8b1a 100644
@@ -494,24 +494,6 @@ static ssize_t armpmu_cpumask_show(struct device *dev,
 	.attrs = armpmu_common_attrs,
 };
 
-static void armpmu_init(struct arm_pmu *armpmu)
-{
-	armpmu->pmu = (struct pmu) {
-		.pmu_enable	= armpmu_enable,
-		.pmu_disable	= armpmu_disable,
-		.event_init	= armpmu_event_init,
-		.add		= armpmu_add,
-		.del		= armpmu_del,
-		.start		= armpmu_start,
-		.stop		= armpmu_stop,
-		.read		= armpmu_read,
-		.filter_match	= armpmu_filter_match,
-		.attr_groups	= armpmu->attr_groups,
-	};
-	armpmu->attr_groups[ARMPMU_ATTR_GROUP_COMMON] =
-		&armpmu_common_attr_group;
-}
-
 /* Set at runtime when we know what CPU type we are. */
 static struct arm_pmu *__oprofile_cpu_pmu;
 

@@ -766,14 +748,6 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (err)
 		goto out_unregister;
 
-	/*
-	 * This is a CPU PMU potentially in a heterogeneous configuration (e.g.
-	 * big.LITTLE). This is not an uncore PMU, and we have taken ctx
-	 * sharing into account (e.g. with our pmu::filter_match callback and
-	 * pmu::event_init group validation).
-	 */
-	cpu_pmu->pmu.capabilities |= PERF_PMU_CAP_HETEROGENEOUS_CPUS;
-
 	return 0;
 
 out_unregister:

@@ -962,6 +936,30 @@ static struct arm_pmu *armpmu_alloc(void)
 		goto out_free_pmu;
 	}
 
+	pmu->pmu = (struct pmu) {
+		.pmu_enable	= armpmu_enable,
+		.pmu_disable	= armpmu_disable,
+		.event_init	= armpmu_event_init,
+		.add		= armpmu_add,
+		.del		= armpmu_del,
+		.start		= armpmu_start,
+		.stop		= armpmu_stop,
+		.read		= armpmu_read,
+		.filter_match	= armpmu_filter_match,
+		.attr_groups	= pmu->attr_groups,
+		/*
+		 * This is a CPU PMU potentially in a heterogeneous
+		 * configuration (e.g. big.LITTLE). This is not an uncore PMU,
+		 * and we have taken ctx sharing into account (e.g. with our
+		 * pmu::filter_match callback and pmu::event_init group
+		 * validation).
+		 */
+		.capabilities	= PERF_PMU_CAP_HETEROGENEOUS_CPUS,
+	};
+
+	pmu->attr_groups[ARMPMU_ATTR_GROUP_COMMON] =
+		&armpmu_common_attr_group;
+
 	for_each_possible_cpu(cpu) {
 		struct pmu_hw_events *events;
 

@@ -998,8 +996,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	if (!pmu)
 		return -ENOMEM;
 
-	armpmu_init(pmu);
-
 	pmu->plat_device = pdev;
 
 	ret = pmu_parse_irqs(pmu);
Signature · 1 line
1.9.1
author maintainer
#4 lore raw

PATCH v2 4/16 drivers/perf: arm_pmu: factor out pmu registration

In reply to Mark Rutland

Currently arm_pmu_device_probe contains probing logic specific to the platform_device infrastructure, and some logic required to safely register the PMU with various systems.

This patch factors out the logic relating to the registration of the PMU. This makes arm_pmu_device_probe a little easier to read, and will make it easier to reuse the logic for an ACPI-specific probing mechanism.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)
drivers/perf/arm_pmu.c +26 -14
index 1cb8b1a..985bd08 100644
@@ -982,6 +982,31 @@ static void armpmu_free(struct arm_pmu *pmu)
 	kfree(pmu);
 }
 
+int armpmu_register(struct arm_pmu *pmu)
+{
+	int ret;
+
+	ret = cpu_pmu_init(pmu);
+	if (ret)
+		return ret;
+
+	ret = perf_pmu_register(&pmu->pmu, pmu->name, -1);
+	if (ret)
+		goto out_destroy;
+
+	if (!__oprofile_cpu_pmu)
+		__oprofile_cpu_pmu = pmu;
+
+	pr_info("enabled with %s PMU driver, %d counters available\n",
+		pmu->name, pmu->num_events);
+
+	return 0;
+
+out_destroy:
+	cpu_pmu_destroy(pmu);
+	return ret;
+}
+
 int arm_pmu_device_probe(struct platform_device *pdev,
 			 const struct of_device_id *of_table,
 			 const struct pmu_probe_info *probe_table)

@@ -1025,25 +1050,12 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		goto out_free;
 	}
 
-
-	ret = cpu_pmu_init(pmu);
+	ret = armpmu_register(pmu);
 	if (ret)
 		goto out_free;
 
-	ret = perf_pmu_register(&pmu->pmu, pmu->name, -1);
-	if (ret)
-		goto out_destroy;
-
-	if (!__oprofile_cpu_pmu)
-		__oprofile_cpu_pmu = pmu;
-
-	pr_info("enabled with %s PMU driver, %d counters available\n",
-			pmu->name, pmu->num_events);
-
 	return 0;
 
-out_destroy:
-	cpu_pmu_destroy(pmu);
 out_free:
 	pr_info("%s: failed to register PMU devices!\n",
 		of_node_full_name(node));
Signature · 1 line
1.9.1
author maintainer
#5 lore raw

PATCH v2 5/16 drivers/perf: arm_pmu: simplify cpu_pmu_request_irqs()

In reply to Mark Rutland

The ARM PMU framework code always uses armpmu_dispatch_irq as its common IRQ handler. Passing this down from cpu_pmu_init() is somewhat pointless, and gets in the way of refactoring.

This patch makes cpu_pmu_request_irqs() always use armpmu_dispatch_irq as the handler when requesting IRQs, and removes the handler parameter from its prototype.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/perf/arm_pmu.c +3 -2
index 985bd08..183c6fd 100644
@@ -543,10 +543,11 @@ static void cpu_pmu_free_irqs(struct arm_pmu *cpu_pmu)
 	}
 }
 
-static int cpu_pmu_request_irqs(struct arm_pmu *cpu_pmu, irq_handler_t handler)
+static int cpu_pmu_request_irqs(struct arm_pmu *cpu_pmu)
 {
 	int cpu, err;
 	struct pmu_hw_events __percpu *hw_events = cpu_pmu->hw_events;
+	const irq_handler_t handler = armpmu_dispatch_irq;
 
 	for_each_cpu(cpu, &cpu_pmu->supported_cpus) {
 		int irq = per_cpu(hw_events->irq, cpu);

@@ -735,7 +736,7 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	int err;
 
-	err = cpu_pmu_request_irqs(cpu_pmu, armpmu_dispatch_irq);
+	err = cpu_pmu_request_irqs(cpu_pmu);
 	if (err)
 		goto out;
Signature · 1 line
1.9.1
author maintainer
#6 lore raw

PATCH v2 6/16 drivers/perf: arm_pmu: handle no platform_device

In reply to Mark Rutland

In armpmu_dispatch_irq() we look at arm_pmu::plat_device to acquire platdata, so that we can defer to platform-specific IRQ handling, required on some 32-bit parts. With the advent of ACPI we won't always have a platform_device, and so we must avoid trying to dereference fields from it.

This patch fixes up armpmu_dispatch_irq() to avoid doing so, introducing a new armpmu_get_platdata() helper.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/perf/arm_pmu.c +9 -3
index 183c6fd..2442038 100644
@@ -316,10 +316,16 @@ static void armpmu_start(struct perf_event *event, int flags)
 	return 0;
 }
 
+static struct arm_pmu_platdata *armpmu_get_platdata(struct arm_pmu *armpmu)
+{
+	struct platform_device *pdev = armpmu->plat_device;
+
+	return pdev ? dev_get_platdata(&pdev->dev) : NULL;
+}
+
 static irqreturn_t armpmu_dispatch_irq(int irq, void *dev)
 {
 	struct arm_pmu *armpmu;
-	struct platform_device *plat_device;
 	struct arm_pmu_platdata *plat;
 	int ret;
 	u64 start_clock, finish_clock;

@@ -331,8 +337,8 @@ static irqreturn_t armpmu_dispatch_irq(int irq, void *dev)
 	 * dereference.
 	 */
 	armpmu = *(void **)dev;
-	plat_device = armpmu->plat_device;
-	plat = dev_get_platdata(&plat_device->dev);
+
+	plat = armpmu_get_platdata(armpmu);
 
 	start_clock = sched_clock();
 	if (plat && plat->handle_irq)
Signature · 1 line
1.9.1
author maintainer
#7 lore raw

PATCH v2 7/16 drivers/perf: arm_pmu: rename irq request/free functions

In reply to Mark Rutland

For historical reasons, portions of the arm_pmu code use a cpu_pmu_ prefix rather than an armpmu_ prefix. While a minor annoyance, this hasn't been a problem thusfar.

However, to enable ACPI support, we'll need to expose a few things in header files, and we should aim to keep those consistently namespaced. In preparation for exporting our IRQ request/free functions, rename these to have an armpmu_ prefix. For consistency, the 'cpu_pmu' parameter is also renamed to 'armpmu'.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/perf/arm_pmu.c +10 -10
index 2442038..3c4e97d 100644
@@ -527,12 +527,12 @@ int perf_num_counters(void)
 }
 EXPORT_SYMBOL_GPL(perf_num_counters);
 
-static void cpu_pmu_free_irqs(struct arm_pmu *cpu_pmu)
+static void armpmu_free_irqs(struct arm_pmu *armpmu)
 {
 	int cpu;
-	struct pmu_hw_events __percpu *hw_events = cpu_pmu->hw_events;
+	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
 
-	for_each_cpu(cpu, &cpu_pmu->supported_cpus) {
+	for_each_cpu(cpu, &armpmu->supported_cpus) {
 		int irq = per_cpu(hw_events->irq, cpu);
 		if (!irq)
 			continue;

@@ -542,20 +542,20 @@ static void cpu_pmu_free_irqs(struct arm_pmu *cpu_pmu)
 			break;
 		}
 
-		if (!cpumask_test_and_clear_cpu(cpu, &cpu_pmu->active_irqs))
+		if (!cpumask_test_and_clear_cpu(cpu, &armpmu->active_irqs))
 			continue;
 
 		free_irq(irq, per_cpu_ptr(&hw_events->percpu_pmu, cpu));
 	}
 }
 
-static int cpu_pmu_request_irqs(struct arm_pmu *cpu_pmu)
+static int armpmu_request_irqs(struct arm_pmu *armpmu)
 {
 	int cpu, err;
-	struct pmu_hw_events __percpu *hw_events = cpu_pmu->hw_events;
+	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
 	const irq_handler_t handler = armpmu_dispatch_irq;
 
-	for_each_cpu(cpu, &cpu_pmu->supported_cpus) {
+	for_each_cpu(cpu, &armpmu->supported_cpus) {
 		int irq = per_cpu(hw_events->irq, cpu);
 		if (!irq)
 			continue;

@@ -580,7 +580,7 @@ static int cpu_pmu_request_irqs(struct arm_pmu *cpu_pmu)
 			return err;
 		}
 
-		cpumask_set_cpu(cpu, &cpu_pmu->active_irqs);
+		cpumask_set_cpu(cpu, &armpmu->active_irqs);
 	}
 
 	return 0;

@@ -742,7 +742,7 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	int err;
 
-	err = cpu_pmu_request_irqs(cpu_pmu);
+	err = armpmu_request_irqs(cpu_pmu);
 	if (err)
 		goto out;
 

@@ -761,7 +761,7 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_STARTING,
 					    &cpu_pmu->node);
 out:
-	cpu_pmu_free_irqs(cpu_pmu);
+	armpmu_free_irqs(cpu_pmu);
 	return err;
 }
Signature · 1 line
1.9.1
author maintainer
#8 lore raw

PATCH v2 8/16 drivers/perf: arm_pmu: split cpu-local irq request/free

In reply to Mark Rutland

Currently we have functions to request/free all IRQs for a given PMU. While this works today, this won't work for ACPI, where we don't know the full set of IRQs up front, and need to request them separately.

To enable supporting ACPI, this patch splits out the cpu-local request/free into new functions, allowing us to request/free individual IRQs.

As this makes it possible/necessary to request a PPI once per cpu, an additional check is added to detect mismatched PPIs. This shouldn't matter for the DT / platform case, as we check this when parsing.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 88 +++++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 36 deletions(-)
drivers/perf/arm_pmu.c +52 -36
index 3c4e97d..c09c379 100644
@@ -527,65 +527,81 @@ int perf_num_counters(void)
 }
 EXPORT_SYMBOL_GPL(perf_num_counters);
 
-static void armpmu_free_irqs(struct arm_pmu *armpmu)
+static void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)
 {
-	int cpu;
 	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
+	int irq = per_cpu(hw_events->irq, cpu);
 
-	for_each_cpu(cpu, &armpmu->supported_cpus) {
-		int irq = per_cpu(hw_events->irq, cpu);
-		if (!irq)
-			continue;
+	if (!cpumask_test_and_clear_cpu(cpu, &armpmu->active_irqs))
+		return;
 
-		if (irq_is_percpu(irq)) {
-			free_percpu_irq(irq, &hw_events->percpu_pmu);
-			break;
-		}
+	if (irq_is_percpu(irq)) {
+		free_percpu_irq(irq, &hw_events->percpu_pmu);
+		cpumask_clear(&armpmu->active_irqs);
+		return;
+	}
 
-		if (!cpumask_test_and_clear_cpu(cpu, &armpmu->active_irqs))
-			continue;
+	free_irq(irq, per_cpu_ptr(&hw_events->percpu_pmu, cpu));
+}
 
-		free_irq(irq, per_cpu_ptr(&hw_events->percpu_pmu, cpu));
-	}
+static void armpmu_free_irqs(struct arm_pmu *armpmu)
+{
+	int cpu;
+
+	for_each_cpu(cpu, &armpmu->supported_cpus)
+		armpmu_free_irq(armpmu, cpu);
 }
 
-static int armpmu_request_irqs(struct arm_pmu *armpmu)
+static int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)
 {
-	int cpu, err;
+	int err = 0;
 	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
 	const irq_handler_t handler = armpmu_dispatch_irq;
+	int irq = per_cpu(hw_events->irq, cpu);
+	if (!irq)
+		return 0;
 
-	for_each_cpu(cpu, &armpmu->supported_cpus) {
-		int irq = per_cpu(hw_events->irq, cpu);
-		if (!irq)
-			continue;
+	if (irq_is_percpu(irq) && cpumask_empty(&armpmu->active_irqs)) {
+		err = request_percpu_irq(irq, handler, "arm-pmu",
+					 &hw_events->percpu_pmu);
+	} else if (irq_is_percpu(irq)) {
+		int other_cpu = cpumask_first(&armpmu->active_irqs);
+		int other_irq = per_cpu(hw_events->irq, other_cpu);
 
-		if (irq_is_percpu(irq)) {
-			err = request_percpu_irq(irq, handler, "arm-pmu",
-						 &hw_events->percpu_pmu);
-			if (err) {
-				pr_err("unable to request IRQ%d for ARM PMU counters\n",
-					irq);
-			}
-
-			return err;
+		if (irq != other_irq) {
+			pr_warn("mismatched PPIs detected.\n");
+			err = -EINVAL;
 		}
-
+	} else {
 		err = request_irq(irq, handler,
 				  IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu",
 				  per_cpu_ptr(&hw_events->percpu_pmu, cpu));
-		if (err) {
-			pr_err("unable to request IRQ%d for ARM PMU counters\n",
-				irq);
-			return err;
-		}
+	}
 
-		cpumask_set_cpu(cpu, &armpmu->active_irqs);
+	if (err) {
+		pr_err("unable to request IRQ%d for ARM PMU counters\n",
+			irq);
+		return err;
 	}
 
+	cpumask_set_cpu(cpu, &armpmu->active_irqs);
+
 	return 0;
 }
 
+static int armpmu_request_irqs(struct arm_pmu *armpmu)
+{
+	int cpu, err;
+
+	for_each_cpu(cpu, &armpmu->supported_cpus) {
+		err = armpmu_request_irq(armpmu, cpu);
+		if (err)
+			break;
+	}
+
+	return err;
+}
+
 static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)
 {
 	struct pmu_hw_events __percpu *hw_events = pmu->hw_events;
Signature · 1 line
1.9.1
author maintainer
#9 lore raw

PATCH v2 9/16 drivers/perf: arm_pmu: move irq request/free into probe

In reply to Mark Rutland

Currently we request (and potentially free) all IRQs for a given PMU in cpu_pmu_init(). This works for platform/DT probing today, but it doesn't fit ACPI well as we don't have all our affinity data up-front.

In preparation for ACPI support, fold the IRQ request/free into arm_pmu_device_probe(), which will remain specific to platform/DT probing.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/arm_pmu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/perf/arm_pmu.c +6 -5
index c09c379..f387d61 100644
@@ -758,10 +758,6 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	int err;
 
-	err = armpmu_request_irqs(cpu_pmu);
-	if (err)
-		goto out;
-
 	err = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_STARTING,
 				       &cpu_pmu->node);
 	if (err)

@@ -777,7 +773,6 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_STARTING,
 					    &cpu_pmu->node);
 out:
-	armpmu_free_irqs(cpu_pmu);
 	return err;
 }
 

@@ -1073,12 +1068,18 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 		goto out_free;
 	}
 
+	ret = armpmu_request_irqs(pmu);
+	if (ret)
+		goto out_free_irqs;
+
 	ret = armpmu_register(pmu);
 	if (ret)
 		goto out_free;
 
 	return 0;
 
+out_free_irqs:
+	armpmu_free_irqs(pmu);
 out_free:
 	pr_info("%s: failed to register PMU devices!\n",
 		of_node_full_name(node));
Signature · 1 line
1.9.1
author maintainer
#10 lore raw

PATCH v2 10/16 drivers/perf: arm_pmu: split out platform device probe logic

In reply to Mark Rutland

Now that we've split the pdev and DT probing logic from the runtime management, let's move the former into its own file. We gain a few lines due to the copyright header and includes, but this should keep the logic clearly separated, and paves the way for adding ACPI support in a similar fashion.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/Makefile           |   2 +-
 drivers/perf/arm_pmu.c          | 226 +-------------------------------------
 drivers/perf/arm_pmu_platform.c | 235 ++++++++++++++++++++++++++++++++++++++++
 include/linux/perf/arm_pmu.h    |   7 ++
 4 files changed, 247 insertions(+), 223 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_platform.c
drivers/perf/Makefile +1 -1
index ef0c6b2..925cd39 100644
@@ -1,4 +1,4 @@
-obj-$(CONFIG_ARM_PMU) += arm_pmu.o
+obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o
 obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
 obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
drivers/perf/arm_pmu.c +4 -222

268 lines not on this page · raw message

drivers/perf/arm_pmu_platform.c new file +235 -0

235 lines not on this page · raw message

include/linux/perf/arm_pmu.h +7 -0
index 4249914..25556eb 100644
@@ -159,6 +159,13 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 			 const struct of_device_id *of_table,
 			 const struct pmu_probe_info *probe_table);
 
+/* Internal functions only for core arm_pmu code */
+struct arm_pmu *armpmu_alloc(void);
+void armpmu_free(struct arm_pmu *pmu);
+int armpmu_register(struct arm_pmu *pmu);
+int armpmu_request_irqs(struct arm_pmu *armpmu);
+void armpmu_free_irqs(struct arm_pmu *armpmu);
+
 #define ARMV8_PMU_PDEV_NAME "armv8-pmu"
 
 #endif /* CONFIG_ARM_PMU */
Signature · 1 line
1.9.1
author maintainer
#11 lore raw

PATCH v2 11/16 arm64: add function to get a cpu's MADT GICC table

In reply to Mark Rutland

Currently the ACPI parking protocol code needs to parse each CPU's MADT GICC table to extract the mailbox address and so on. Each time we parse a GICC table, we call back to the parking protocol code to parse it.

This has been fine so far, but we're about to have more code that needs to extract data from the GICC tables, and adding a callback for each user is going to get unwieldy.

Instead, this patch ensures that we stash a copy of each CPU's GICC table at boot time, such that anything needing to parse it can later request it. This will allow for other parsers of GICC, and for simplification to the ACPI parking protocol code. Note that we must store a copy, rather than a pointer, since the core ACPI code temporarily maps/unmaps tables while iterating over them.

Since we parse the MADT before we know how many CPUs we have (and hence before we setup the percpu areas), we must use an NR_CPUS sized array.

Signed-off-by Mark Rutland <[email protected]>
Reviewed-by Lorenzo Pieralisi <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/acpi.h |  2 ++
 arch/arm64/kernel/smp.c       | 10 ++++++++++
 2 files changed, 12 insertions(+)
arch/arm64/include/asm/acpi.h +2 -0
index c1976c0..0e99978 100644
@@ -85,6 +85,8 @@ static inline bool acpi_has_cpu_in_madt(void)
 	return true;
 }
 
+struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu);
+
 static inline void arch_fix_phys_package_id(int num, u32 slot) { }
 void __init acpi_init_cpus(void);
arch/arm64/kernel/smp.c +10 -0
index ef1caae..390c277 100644
@@ -518,6 +518,13 @@ static int __init smp_cpu_setup(int cpu)
 static unsigned int cpu_count = 1;
 
 #ifdef CONFIG_ACPI
+static struct acpi_madt_generic_interrupt cpu_madt_gicc[NR_CPUS];
+
+struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu)
+{
+	return &cpu_madt_gicc[cpu];
+}
+
 /*
  * acpi_map_gic_cpu_interface - parse processor MADT entry
  *

@@ -552,6 +559,7 @@ static int __init smp_cpu_setup(int cpu)
 			return;
 		}
 		bootcpu_valid = true;
+		cpu_madt_gicc[0] = *processor;
 		early_map_cpu_to_node(0, acpi_numa_get_nid(0, hwid));
 		return;
 	}

@@ -562,6 +570,8 @@ static int __init smp_cpu_setup(int cpu)
 	/* map the logical cpu id to cpu MPIDR */
 	cpu_logical_map(cpu_count) = hwid;
 
+	cpu_madt_gicc[cpu_count] = *processor;
+
 	/*
 	 * Set-up the ACPI parking protocol cpu entries
 	 * while initializing the cpu_logical_map to
Signature · 1 line
1.9.1
author maintainer
#12 lore raw

PATCH v2 12/16 arm64: parking: kill acpi_set_mailbox_entry()

In reply to Mark Rutland

Now that we have acpi_cpu_get_madt_gicc(), we can use values from the MADT GICC table directly, and no longer need to stash a copy in our own datastructure.

This allows us to kill off the acpi_set_mailbox_entry() callback, too.

Signed-off-by Mark Rutland <[email protected]>
Reviewed-by Lorenzo Pieralisi <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/acpi.h             |  5 ----
 arch/arm64/kernel/acpi_parking_protocol.c | 40 ++++++++++---------------------
 arch/arm64/kernel/smp.c                   | 11 ---------
 3 files changed, 13 insertions(+), 43 deletions(-)
arch/arm64/include/asm/acpi.h +0 -5
index 0e99978..ebf88e3 100644
@@ -96,13 +96,8 @@ static inline void acpi_init_cpus(void) { }
 
 #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
 bool acpi_parking_protocol_valid(int cpu);
-void __init
-acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
 #else
 static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
-static inline void
-acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
-{}
 #endif
 
 static inline const char *acpi_get_enable_method(int cpu)
arch/arm64/kernel/acpi_parking_protocol.c +13 -27
index 1f5655c..e2c7399 100644
@@ -28,36 +28,21 @@ struct parking_protocol_mailbox {
 	__le64 entry_point;
 };
 
-struct cpu_mailbox_entry {
-	struct parking_protocol_mailbox __iomem *mailbox;
-	phys_addr_t mailbox_addr;
-	u8 version;
-	u8 gic_cpu_id;
-};
-
-static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
-
-void __init acpi_set_mailbox_entry(int cpu,
-				   struct acpi_madt_generic_interrupt *p)
-{
-	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
-
-	cpu_entry->mailbox_addr = p->parked_address;
-	cpu_entry->version = p->parking_version;
-	cpu_entry->gic_cpu_id = p->cpu_interface_number;
-}
+static DEFINE_PER_CPU(struct parking_protocol_mailbox __iomem * , mailboxes);
 
 bool acpi_parking_protocol_valid(int cpu)
 {
-	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+	struct acpi_madt_generic_interrupt *p = acpi_cpu_get_madt_gicc(cpu);
 
-	return cpu_entry->mailbox_addr && cpu_entry->version;
+	return p->parked_address && p->parking_version;
 }
 
 static int acpi_parking_protocol_cpu_init(unsigned int cpu)
 {
+	struct acpi_madt_generic_interrupt *p = acpi_cpu_get_madt_gicc(cpu);
+
 	pr_debug("%s: ACPI parked addr=%llx\n", __func__,
-		  cpu_mailbox_entries[cpu].mailbox_addr);
+		 p->parked_address);
 
 	return 0;
 }

@@ -69,7 +54,7 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
 
 static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 {
-	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+	struct acpi_madt_generic_interrupt *p = acpi_cpu_get_madt_gicc(cpu);
 	struct parking_protocol_mailbox __iomem *mailbox;
 	__le32 cpu_id;
 

@@ -84,7 +69,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 	 * by the kernel (it clashes with the linear mapping attributes
 	 * specifications).
 	 */
-	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
+	mailbox = ioremap(p->parked_address, sizeof(*mailbox));
 	if (!mailbox)
 		return -EIO;
 

@@ -102,7 +87,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 	 * stash the mailbox address mapping to use it for further FW
 	 * checks in the postboot method
 	 */
-	cpu_entry->mailbox = mailbox;
+	per_cpu(mailboxes, cpu) = mailbox;
 
 	/*
 	 * We write the entry point and cpu id as LE regardless of the

@@ -111,7 +96,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 	 * Boot-Loader's endianness before jumping.
 	 */
 	writeq_relaxed(__pa_symbol(secondary_entry), &mailbox->entry_point);
-	writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
+	writel_relaxed(p->cpu_interface_number, &mailbox->cpu_id);
 
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 

@@ -121,10 +106,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 static void acpi_parking_protocol_cpu_postboot(void)
 {
 	int cpu = smp_processor_id();
-	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
-	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
+	struct parking_protocol_mailbox __iomem *mailbox;
 	__le64 entry_point;
 
+	mailbox = per_cpu(mailboxes, cpu);
+
 	entry_point = readl_relaxed(&mailbox->entry_point);
 	/*
 	 * Check if firmware has cleared the entry_point as expected
arch/arm64/kernel/smp.c +0 -11
index 390c277..b37dd83 100644
@@ -572,17 +572,6 @@ struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu)
 
 	cpu_madt_gicc[cpu_count] = *processor;
 
-	/*
-	 * Set-up the ACPI parking protocol cpu entries
-	 * while initializing the cpu_logical_map to
-	 * avoid parsing MADT entries multiple times for
-	 * nothing (ie a valid cpu_logical_map entry should
-	 * contain a valid parking protocol data set to
-	 * initialize the cpu if the parking protocol is
-	 * the only available enable method).
-	 */
-	acpi_set_mailbox_entry(cpu_count, processor);
-
 	early_map_cpu_to_node(cpu_count, acpi_numa_get_nid(cpu_count, hwid));
 
 	cpu_count++;
Signature · 1 line
1.9.1
author maintainer
#13 lore raw

PATCH v2 13/16 arm64: parking: fix type endianness

In reply to Mark Rutland

The read*{,_relaxed}() return a value of native CPU endianness, swizzling the LE value they read from an __iomem pointer.

Thus it isn't correct to treat their return values as __le32 or __le64, and we should use u32 and u64 respectively instead.

This makes sparse much happier, but should have no functional impact.

Signed-off-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/kernel/acpi_parking_protocol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/acpi_parking_protocol.c +2 -2
index e2c7399..b878b15 100644
@@ -56,7 +56,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 {
 	struct acpi_madt_generic_interrupt *p = acpi_cpu_get_madt_gicc(cpu);
 	struct parking_protocol_mailbox __iomem *mailbox;
-	__le32 cpu_id;
+	u32 cpu_id;
 
 	/*
 	 * Map mailbox memory with attribute device nGnRE (ie ioremap -

@@ -107,7 +107,7 @@ static void acpi_parking_protocol_cpu_postboot(void)
 {
 	int cpu = smp_processor_id();
 	struct parking_protocol_mailbox __iomem *mailbox;
-	__le64 entry_point;
+	u64 entry_point;
 
 	mailbox = per_cpu(mailboxes, cpu);
Signature · 1 line
1.9.1
author maintainer
#14 lore raw

PATCH v2 14/16 drivers/perf: arm_pmu: add ACPI framework

In reply to Mark Rutland

This patch adds framework code to handle parsing PMU data out of the MADT, sanity checking this, and managing the association of CPUs (and their interrupts) with appropriate logical PMUs.

For the time being, we expect that only one PMU driver (PMUv3) will make use of this, and we simply pass in a single probe function.

This is based on an earlier patch from Jeremy Linton.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 drivers/perf/Kconfig         |   4 +
 drivers/perf/Makefile        |   1 +
 drivers/perf/arm_pmu.c       |   4 +-
 drivers/perf/arm_pmu_acpi.c  | 240 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/cpuhotplug.h   |   1 +
 include/linux/perf/arm_pmu.h |  11 ++
 6 files changed, 259 insertions(+), 2 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c
drivers/perf/Kconfig +4 -0
index c436e0d..aa587ed 100644
@@ -12,6 +12,10 @@ config ARM_PMU
 	  Say y if you want to use CPU performance monitors on ARM-based
 	  systems.
 
+config ARM_PMU_ACPI
+	depends on ARM_PMU && ACPI
+	def_bool y
+
 config QCOM_L2_PMU
 	bool "Qualcomm Technologies L2-cache PMU"
 	depends on ARCH_QCOM && ARM64 && PERF_EVENTS && ACPI
drivers/perf/Makefile +1 -0
index 925cd39..6420bd4 100644
@@ -1,4 +1,5 @@
 obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o
+obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
 obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
 obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
drivers/perf/arm_pmu.c +2 -2
index b3bedfa..dc459eb 100644
@@ -525,7 +525,7 @@ int perf_num_counters(void)
 }
 EXPORT_SYMBOL_GPL(perf_num_counters);
 
-static void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)
+void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)
 {
 	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
 	int irq = per_cpu(hw_events->irq, cpu);

@@ -550,7 +550,7 @@ void armpmu_free_irqs(struct arm_pmu *armpmu)
 		armpmu_free_irq(armpmu, cpu);
 }
 
-static int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)
+int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)
 {
 	int err = 0;
 	struct pmu_hw_events __percpu *hw_events = armpmu->hw_events;
drivers/perf/arm_pmu_acpi.c new file +240 -0

240 lines not on this page · raw message

include/linux/cpuhotplug.h +1 -0
index cfcfab3..0f2a803 100644
@@ -94,6 +94,7 @@ enum cpuhp_state {
 	CPUHP_AP_ARM_VFP_STARTING,
 	CPUHP_AP_ARM64_DEBUG_MONITORS_STARTING,
 	CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING,
+	CPUHP_AP_PERF_ARM_ACPI_STARTING,
 	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_ARM_L2X0_STARTING,
 	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
include/linux/perf/arm_pmu.h +11 -0
index 25556eb..1360dd6 100644
@@ -117,6 +117,9 @@ struct arm_pmu {
 	struct notifier_block	cpu_pm_nb;
 	/* the attr_groups array must be NULL-terminated */
 	const struct attribute_group *attr_groups[ARMPMU_NR_ATTR_GROUPS + 1];
+
+	/* Only to be used by ACPI probing code */
+	unsigned long acpi_cpuid;
 };
 
 #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))

@@ -159,12 +162,20 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 			 const struct of_device_id *of_table,
 			 const struct pmu_probe_info *probe_table);
 
+#ifdef CONFIG_ACPI
+int arm_pmu_acpi_probe(armpmu_init_fn init_fn);
+#else
+static inline int arm_pmu_acpi_probe(armpmu_init_fn init_fn) { return 0; }
+#endif
+
 /* Internal functions only for core arm_pmu code */
 struct arm_pmu *armpmu_alloc(void);
 void armpmu_free(struct arm_pmu *pmu);
 int armpmu_register(struct arm_pmu *pmu);
 int armpmu_request_irqs(struct arm_pmu *armpmu);
 void armpmu_free_irqs(struct arm_pmu *armpmu);
+int armpmu_request_irq(struct arm_pmu *armpmu, int cpu);
+void armpmu_free_irq(struct arm_pmu *armpmu, int cpu);
 
 #define ARMV8_PMU_PDEV_NAME "armv8-pmu"
Signature · 1 line
1.9.1
#15 lore raw

PATCH v2 14/16 drivers/perf: arm_pmu: add ACPI framework

Hi Mark,

Just a couple of minor comments below.

Quoted 45 lines · Mark Rutland
drivers/perf/arm_pmu_acpi.c new file +39 -0
new file mode 100644
index 0000000..5cacd7b
@@ -0,0 +1,240 @@
+/*
+ * ACPI probing code for ARM performance counters.
+ *
+ * Copyright (C) 2017 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/cpumask.h>
+#include <linux/init.h>
+#include <linux/percpu.h>
+#include <linux/perf/arm_pmu.h>
+
+#include <asm/cputype.h>
+
+static DEFINE_PER_CPU(struct arm_pmu *, probed_pmus);
+static DEFINE_PER_CPU(int, pmu_irqs);
+
+static int arm_pmu_acpi_parse_irq(int cpu)
+{
+	struct acpi_madt_generic_interrupt *gicc;
+	int gsi, trigger;
+
+	gicc = acpi_cpu_get_madt_gicc(cpu);
+	if (WARN_ON(!gicc))
+		return -EINVAL;
+
+	gsi = gicc->performance_interrupt;
+	if (gicc->flags & ACPI_MADT_PERFORMANCE_IRQ_MODE)
+		trigger = ACPI_EDGE_SENSITIVE;
+	else
+		trigger = ACPI_LEVEL_SENSITIVE;
+
+	/*
+	 * Helpfully, the MADT GICC doesn't have a polarity flag for the
+	 * "performance interrupt". Luckily, on compliant GICs the polarity is

drivers/perf/arm_pmu_acpi.c line 49

Quoted 10 lines
drivers/perf/arm_pmu_acpi.c new file +10 -0
+	 * fixed in HW (for both SPIs and PPIs), and thus we don't care. Other
+	 * interrupt controllers are not supported with ACPI.
+	 *
+	 * Here we pass in ACPI_ACTIVE_HIGH to keep the core code happy.
+	 */
+	return acpi_register_gsi(NULL, gsi, trigger, ACPI_ACTIVE_HIGH);
+}
+
+static void arm_pmu_acpi_unparse_irq(int cpu)
+{

"unparse" is a bit weird. Can you rename this register/unregister please?

Quoted 25 lines
drivers/perf/arm_pmu_acpi.c new file +25 -0
+	struct acpi_madt_generic_interrupt *gicc;
+	int gsi;
+
+	gicc = acpi_cpu_get_madt_gicc(cpu);
+	if (!gicc)
+		return;
+
+	gsi = gicc->performance_interrupt;
+	acpi_unregister_gsi(gsi);
+}
+
+static int arm_pmu_acpi_parse_irqs(void)
+{
+	int cpu, irq;
+
+	for_each_possible_cpu(cpu) {
+		int irq = arm_pmu_acpi_parse_irq(cpu);
+		if (irq < 0) {
+			pr_warn("Unable to parse ACPI PMU IRQ for CPU%d: %d\n",
+				cpu, irq);
+			goto out_err;
+		} else if (irq == 0) {
+			pr_warn("No ACPI PMU IRQ for CPU%d\n", cpu);
+		}
+

drivers/perf/arm_pmu_acpi.c line 84

Quoted 10 lines
drivers/perf/arm_pmu_acpi.c new file +10 -0
+		per_cpu(pmu_irqs, cpu) = irq;
+	}
+
+	return 0;
+
+out_err:
+	for_each_possible_cpu(cpu) {
+		arm_pmu_acpi_unparse_irq(cpu);
+		per_cpu(pmu_irqs, cpu) = 0;
+	}

It might work at the moment (I really can't tell), but I'd rather we didn't unregister GSIs that we didn't register.

Will

author maintainer
#16 lore raw

PATCH v2 14/16 drivers/perf: arm_pmu: add ACPI framework

Quoted 11 lines · Will Deacon

Hi Mark,

Just a couple of minor comments below.

drivers/perf/arm_pmu_acpi.c line 48

Quoted 3 lines · Mark Rutland
drivers/perf/arm_pmu_acpi.c new file +2 -0
+static int arm_pmu_acpi_parse_irq(int cpu)
 
+static void arm_pmu_acpi_unparse_irq(int cpu)

"unparse" is a bit weird. Can you rename this register/unregister please?

Sure thing; that's much better.

[...]

Quoted 19 lines
Quoted 19 lines
drivers/perf/arm_pmu_acpi.c new file +19 -0
+static int arm_pmu_acpi_parse_irqs(void)
+{
+	int cpu, irq;
+
+	for_each_possible_cpu(cpu) {
+		int irq = arm_pmu_acpi_parse_irq(cpu);
+		if (irq < 0) {
+			pr_warn("Unable to parse ACPI PMU IRQ for CPU%d: %d\n",
+				cpu, irq);
+			goto out_err;
+		} else if (irq == 0) {
+			pr_warn("No ACPI PMU IRQ for CPU%d\n", cpu);
+		}
+
+		per_cpu(pmu_irqs, cpu) = irq;
+	}
+
+	return 0;
+
Quoted 8 lines

drivers/perf/arm_pmu_acpi.c line 84

Quoted 5 lines
drivers/perf/arm_pmu_acpi.c new file +5 -0
+out_err:
+	for_each_possible_cpu(cpu) {
+		arm_pmu_acpi_unparse_irq(cpu);
+		per_cpu(pmu_irqs, cpu) = 0;
+	}

It might work at the moment (I really can't tell), but I'd rather we didn't unregister GSIs that we didn't register.

Urgh, yes.

That gets really more fun if you consider PPIs (which could be mismatched).

If we're happy with a hideous loop we can do:

	int irq, cpu, irq_cpu;

	for_each_possible_cpu(cpu) {
		irq = per_cpu(pmu_irqs, cpu);
		if (!irq)
			continue;

		arm_pmu_acpi_unparse_irq(cpu);

		/* blat all instances of the irq we just freed */
		for_each_possible_cpu(irq_cpu) {
			if (per_cpu(pmu_irqs, irq_cpu) != irq)
				continue;
			per_cpu(pmu_irqs, irq_cpu) = 0;
		}
	}

... which I'll spin for v3.

Thanks,
Mark.

author maintainer
#17 lore raw

PATCH v2 15/16 arm64: pmuv3: handle !PMUv3 when probing

In reply to Mark Rutland

When probing via ACPI, we won't know up-front whether a CPU has a PMUv3 compatible PMU. Thus we need to consult ID registers during probe time.

This patch updates our PMUv3 probing code to test for the presence of PMUv3 functionality before touching an PMUv3-specific registers, and before updating the struct arm_pmu with PMUv3 data.

When a PMUv3-compatible PMU is not present, probing will return -ENODEV.

Signed-off-by Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 85 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 16 deletions(-)
arch/arm64/kernel/perf_event.c +69 -16
index 57ae9d9..50790d7 100644
@@ -957,11 +957,24 @@ static int armv8_vulcan_map_event(struct perf_event *event)
 				ARMV8_PMU_EVTYPE_EVENT);
 }
 
+struct armv8pmu_probe_info {
+	struct arm_pmu *pmu;
+	bool present;
+};
+
 static void __armv8pmu_probe_pmu(void *info)
 {
-	struct arm_pmu *cpu_pmu = info;
+	struct armv8pmu_probe_info *probe = info;
+	struct arm_pmu *cpu_pmu = probe->pmu;
+	u64 dfr0;
 	u32 pmceid[2];
 
+	dfr0 = read_sysreg(id_aa64dfr0_el1);
+	if (((dfr0 >> ID_AA64DFR0_PMUVER_SHIFT) & 0xf) != 1)
+		return;
+
+	probe->present = true;
+
 	/* Read the nb of CNTx counters supported from PMNC */
 	cpu_pmu->num_events = (armv8pmu_pmcr_read() >> ARMV8_PMU_PMCR_N_SHIFT)
 		& ARMV8_PMU_PMCR_N_MASK;

@@ -979,13 +992,27 @@ static void __armv8pmu_probe_pmu(void *info)
 
 static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
 {
-	return smp_call_function_any(&cpu_pmu->supported_cpus,
+	struct armv8pmu_probe_info probe = {
+		.pmu = cpu_pmu,
+		.present = false,
+	};
+	int ret;
+
+	ret = smp_call_function_any(&cpu_pmu->supported_cpus,
 				    __armv8pmu_probe_pmu,
-				    cpu_pmu, 1);
+				    &probe, 1);
+	if (ret)
+		return ret;
+
+	return probe.present ? 0 : -ENODEV;
 }
 
-static void armv8_pmu_init(struct arm_pmu *cpu_pmu)
+static int armv8_pmu_init(struct arm_pmu *cpu_pmu)
 {
+	int ret = armv8pmu_probe_pmu(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->handle_irq		= armv8pmu_handle_irq,
 	cpu_pmu->enable			= armv8pmu_enable_event,
 	cpu_pmu->disable		= armv8pmu_disable_event,

@@ -997,78 +1024,104 @@ static void armv8_pmu_init(struct arm_pmu *cpu_pmu)
 	cpu_pmu->reset			= armv8pmu_reset,
 	cpu_pmu->max_period		= (1LLU << 32) - 1,
 	cpu_pmu->set_event_filter	= armv8pmu_set_event_filter;
+
+	return 0;
 }
 
 static int armv8_pmuv3_init(struct arm_pmu *cpu_pmu)
 {
-	armv8_pmu_init(cpu_pmu);
+	int ret = armv8_pmu_init(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->name			= "armv8_pmuv3";
 	cpu_pmu->map_event		= armv8_pmuv3_map_event;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
 		&armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] =
 		&armv8_pmuv3_format_attr_group;
-	return armv8pmu_probe_pmu(cpu_pmu);
+
+	return 0;
 }
 
 static int armv8_a53_pmu_init(struct arm_pmu *cpu_pmu)
 {
-	armv8_pmu_init(cpu_pmu);
+	int ret = armv8_pmu_init(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->name			= "armv8_cortex_a53";
 	cpu_pmu->map_event		= armv8_a53_map_event;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
 		&armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] =
 		&armv8_pmuv3_format_attr_group;
-	return armv8pmu_probe_pmu(cpu_pmu);
+
+	return 0;
 }
 
 static int armv8_a57_pmu_init(struct arm_pmu *cpu_pmu)
 {
-	armv8_pmu_init(cpu_pmu);
+	int ret = armv8_pmu_init(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->name			= "armv8_cortex_a57";
 	cpu_pmu->map_event		= armv8_a57_map_event;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
 		&armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] =
 		&armv8_pmuv3_format_attr_group;
-	return armv8pmu_probe_pmu(cpu_pmu);
+
+	return 0;
 }
 
 static int armv8_a72_pmu_init(struct arm_pmu *cpu_pmu)
 {
-	armv8_pmu_init(cpu_pmu);
+	int ret = armv8_pmu_init(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->name			= "armv8_cortex_a72";
 	cpu_pmu->map_event		= armv8_a57_map_event;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
 		&armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] =
 		&armv8_pmuv3_format_attr_group;
-	return armv8pmu_probe_pmu(cpu_pmu);
+
+	return 0;
 }
 
 static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
 {
-	armv8_pmu_init(cpu_pmu);
+	int ret = armv8_pmu_init(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->name			= "armv8_cavium_thunder";
 	cpu_pmu->map_event		= armv8_thunder_map_event;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
 		&armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] =
 		&armv8_pmuv3_format_attr_group;
-	return armv8pmu_probe_pmu(cpu_pmu);
+
+	return 0;
 }
 
 static int armv8_vulcan_pmu_init(struct arm_pmu *cpu_pmu)
 {
-	armv8_pmu_init(cpu_pmu);
+	int ret = armv8_pmu_init(cpu_pmu);
+	if (ret)
+		return ret;
+
 	cpu_pmu->name			= "armv8_brcm_vulcan";
 	cpu_pmu->map_event		= armv8_vulcan_map_event;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] =
 		&armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] =
 		&armv8_pmuv3_format_attr_group;
-	return armv8pmu_probe_pmu(cpu_pmu);
+
+	return 0;
 }
 
 static const struct of_device_id armv8_pmu_of_device_ids[] = {
Signature · 1 line
1.9.1
#18 lore raw

PATCH v2 15/16 arm64: pmuv3: handle !PMUv3 when probing

Quoted 29 lines · Mark Rutland

When probing via ACPI, we won't know up-front whether a CPU has a PMUv3 compatible PMU. Thus we need to consult ID registers during probe time.

This patch updates our PMUv3 probing code to test for the presence of PMUv3 functionality before touching an PMUv3-specific registers, and before updating the struct arm_pmu with PMUv3 data.

When a PMUv3-compatible PMU is not present, probing will return -ENODEV.

Signed-off-by Mark Rutland <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 85 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 16 deletions(-)
arch/arm64/kernel/perf_event.c +5 -0
index 57ae9d9..50790d7 100644
@@ -957,11 +957,24 @@ static int armv8_vulcan_map_event(struct perf_event *event)
 				ARMV8_PMU_EVTYPE_EVENT);
 }
 
+struct armv8pmu_probe_info {
+	struct arm_pmu *pmu;
+	bool present;
+};
+
 static void __armv8pmu_probe_pmu(void *info)

arch/arm64/kernel/perf_event.c static int armv8_vulcan_map_event(struct perf_event *event) · line 974

Quoted 10 lines
arch/arm64/kernel/perf_event.c +6 -1
 {
-	struct arm_pmu *cpu_pmu = info;
+	struct armv8pmu_probe_info *probe = info;
+	struct arm_pmu *cpu_pmu = probe->pmu;
+	u64 dfr0;
 	u32 pmceid[2];
 
+	dfr0 = read_sysreg(id_aa64dfr0_el1);
+	if (((dfr0 >> ID_AA64DFR0_PMUVER_SHIFT) & 0xf) != 1)
+		return;

Shouldn't we be using one of those fancy cpuid_feature_extract_*_field helpers?

It would also be worth spinning this up on qemu, if you get a chance, as I don't think that implements the PMU.

Will

author maintainer
#19 lore raw

PATCH v2 15/16 arm64: pmuv3: handle !PMUv3 when probing

Quoted 8 lines · Will Deacon
Quoted 8 lines

static void __armv8pmu_probe_pmu(void *info)
{

arch/arm64/kernel/perf_event.c +3 -1
-	struct arm_pmu *cpu_pmu = info;
+	struct armv8pmu_probe_info *probe = info;
+	struct arm_pmu *cpu_pmu = probe->pmu;
+	u64 dfr0;
 	u32 pmceid[2];
Quoted 5 lines

arch/arm64/kernel/perf_event.c static int armv8_vulcan_map_event(struct perf_event *event) · line 974

Quoted 3 lines
arch/arm64/kernel/perf_event.c +3 -0
+	dfr0 = read_sysreg(id_aa64dfr0_el1);
+	if (((dfr0 >> ID_AA64DFR0_PMUVER_SHIFT) & 0xf) != 1)
+		return;

Shouldn't we be using one of those fancy cpuid_feature_extract_*_field helpers?

Sure. I can make this:

	u64 dfr0, pmuver;

	...

	dfr0 = read_sysreg(id_aa64dfr0_el1);
	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
			ID_AA64DFR0_PMUVER_SHIFT);
	if (pmuver != 1)
		return;
Quoted 2 lines

It would also be worth spinning this up on qemu, if you get a chance, as I don't think that implements the PMU.

I will try to give that a go shortly.

Thanks,
Mark.

author maintainer
#20 lore raw

PATCH v2 15/16 arm64: pmuv3: handle !PMUv3 when probing

In reply to Will Deacon
Quoted 7 lines · Will Deacon
Quoted 6 lines · Mark Rutland

When probing via ACPI, we won't know up-front whether a CPU has a PMUv3 compatible PMU. Thus we need to consult ID registers during probe time.

This patch updates our PMUv3 probing code to test for the presence of PMUv3 functionality before touching an PMUv3-specific registers, and before updating the struct arm_pmu with PMUv3 data.

[...]

Quoted 2 lines

It would also be worth spinning this up on qemu, if you get a chance, as I don't think that implements the PMU.

That raises the usual problem with GSIs in static ACPI tables; the MADT GICC doesn't have any way of describing the absense of a PMU interrupt.

Booting a TCG qemu, the PMU GSI is zero, which is a valid GSI...

There's a spec bug to fix here.

Urrgh.

Thanks,
Mark.

#21 lore raw

PATCH v2 15/16 arm64: pmuv3: handle !PMUv3 when probing

Quoted 14 lines · Mark Rutland
Quoted 7 lines · Will Deacon
Quoted 6 lines · Mark Rutland

When probing via ACPI, we won't know up-front whether a CPU has a PMUv3 compatible PMU. Thus we need to consult ID registers during probe time.

This patch updates our PMUv3 probing code to test for the presence of PMUv3 functionality before touching an PMUv3-specific registers, and before updating the struct arm_pmu with PMUv3 data.

[...]

Quoted 2 lines

It would also be worth spinning this up on qemu, if you get a chance, as I don't think that implements the PMU.

Quoted 6 lines

That raises the usual problem with GSIs in static ACPI tables; the MADT GICC doesn't have any way of describing the absense of a PMU interrupt.

Booting a TCG qemu, the PMU GSI is zero, which is a valid GSI...

There's a spec bug to fix here.

Or a QEMU bug... The SBSA mandates a PMU AFAIK. So you won't have an ACPI system without one.

Quoted 6 lines

Urrgh.

Thanks,
Mark.

author maintainer
#22 lore raw

PATCH v2 16/16 arm64: pmuv3: use arm_pmu ACPI framework

In reply to Mark Rutland

Now that we have a framework to handle the ACPI bits, make the PMUv3 code use this. The framework is a little different to what was originally envisaged, and we can drop some unused support code in the process of moving over to it.

Signed-off-by Mark Rutland <[email protected]>
Tested-by Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)
arch/arm64/kernel/perf_event.c +9 -17
index 50790d7..48bc505 100644
@@ -1134,24 +1134,9 @@ static int armv8_vulcan_pmu_init(struct arm_pmu *cpu_pmu)
 	{},
 };
 
-/*
- * 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), /* enable all defined counters */
-	{ /* sentinel value */ }
-};
-
 static int armv8_pmu_device_probe(struct platform_device *pdev)
 {
-	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);
+	return arm_pmu_device_probe(pdev, armv8_pmu_of_device_ids, NULL);
 }
 
 static struct platform_driver armv8_pmu_driver = {

@@ -1162,4 +1147,11 @@ static int armv8_pmu_device_probe(struct platform_device *pdev)
 	.probe		= armv8_pmu_device_probe,
 };
 
-builtin_platform_driver(armv8_pmu_driver);
+int __init armv8_pmu_driver_init(void)
+{
+	if (acpi_disabled)
+		return platform_driver_register(&armv8_pmu_driver);
+	else
+		return arm_pmu_acpi_probe(armv8_pmuv3_init);
+}
+device_initcall(armv8_pmu_driver_init)
Signature · 1 line
1.9.1
#23 lore raw

v2 cover arm_pmu: ACPI support

In reply to Mark Rutland
Quoted 4 lines · Mark Rutland

This series implements ACPI support in the ARM PMU code. It borrows some code from Jeremy's series [1], but takes a different approach to probing and association, using the usual hotplug state machine, minimising external changes required, and simplifying the relationship with the common arm_pmu code.

Patches 12 and 13 seem unrelated to this series, and I've made some small comments on a couple of other patches. Other than that, looks good.

Will

author maintainer
#24 lore raw

v2 cover arm_pmu: ACPI support

Quoted 8 lines · Will Deacon
Quoted 4 lines · Mark Rutland

This series implements ACPI support in the ARM PMU code. It borrows some code from Jeremy's series [1], but takes a different approach to probing and association, using the usual hotplug state machine, minimising external changes required, and simplifying the relationship with the common arm_pmu code.

Patches 12 and 13 seem unrelated to this series, and I've made some small comments on a couple of other patches.

Those were cleanup that relied on the GICC rework in this series.

I'll drop those for v3, and send them separately once the rest is in.

Thanks,
Mark.