Skip to contentSkip to conversation

arm_pmu: ACPI support

PATCH New Mark Rutland 8 participants 45 messages Mar 10, 2017 – Apr 12, 2017

Subsystems (4)
lore
Share & tools
lore

Mark Rutland

author maintainer
#0 lore raw

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 my IRQ rework patches [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.

Thanks,
Mark.

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

Mark Rutland (14):
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: kill acpi_set_mailbox_entry()
drivers/perf: arm_pmu: add ACPI framework
arm64: pmuv3: use arm_pmu ACPI framework

 arch/arm64/include/asm/acpi.h             |   7 +-
 arch/arm64/kernel/acpi_parking_protocol.c |  38 +--
 arch/arm64/kernel/perf_event.c            |  26 +-
 arch/arm64/kernel/smp.c                   |  19 +-
 drivers/perf/Kconfig                      |   4 +
 drivers/perf/Makefile                     |   3 +-
 drivers/perf/arm_pmu.c                    | 383 ++++++++----------------------
 drivers/perf/arm_pmu_acpi.c               | 237 ++++++++++++++++++
 drivers/perf/arm_pmu_platform.c           | 235 ++++++++++++++++++
 include/linux/cpuhotplug.h                |   1 +
 include/linux/perf/arm_pmu.h              |  22 +-
 11 files changed, 626 insertions(+), 349 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 1/14 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.

---
 drivers/perf/arm_pmu.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/perf/arm_pmu.c +3 -10
index 8b0aaa9..2bbce3b 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 2/14 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.

---
 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 2bbce3b..1dfda0b 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 3/14 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.

---
 drivers/perf/arm_pmu.c | 52 +++++++++++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 28 deletions(-)
drivers/perf/arm_pmu.c +24 -28
index 1dfda0b..e7529a0 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 @@ 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 4/14 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.

---
 drivers/perf/arm_pmu.c | 40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)
drivers/perf/arm_pmu.c +26 -14
index e7529a0..4ae9c49 100644
@@ -982,6 +982,31 @@ 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 5/14 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.

---
 drivers/perf/arm_pmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/perf/arm_pmu.c +3 -2
index 4ae9c49..6196777 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 6/14 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.

---
 drivers/perf/arm_pmu.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/perf/arm_pmu.c +9 -3
index 6196777..02fc70e 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 7/14 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'.

---
 drivers/perf/arm_pmu.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/perf/arm_pmu.c +10 -10
index 02fc70e..a39cf55 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 8/14 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.

---
 drivers/perf/arm_pmu.c | 88 +++++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 36 deletions(-)
drivers/perf/arm_pmu.c +52 -36
index a39cf55..5e5ea12 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;
+}
+
 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 9/14 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.

---
 drivers/perf/arm_pmu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/perf/arm_pmu.c +6 -5
index 5e5ea12..d202a73 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 10/14 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.

---
 drivers/perf/Makefile           |   2 +-
 drivers/perf/arm_pmu.c          | 222 +------------------------------------
 drivers/perf/arm_pmu_platform.c | 235 ++++++++++++++++++++++++++++++++++++++++
 include/linux/perf/arm_pmu.h    |   7 ++
 4 files changed, 245 insertions(+), 221 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_platform.c
drivers/perf/Makefile +1 -1
index ef24833..4c0d68d 100644
@@ -1,3 +1,3 @@
-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_XGENE_PMU) += xgene_pmu.o
drivers/perf/arm_pmu.c +2 -220

258 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 11/14 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.

Cc Catalin Marinas <[email protected]>
Cc Lorenzo Pieralisi <[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
#12 lore raw

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

Quoted 18 lines · 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.

Quoted 8 lines
Cc Catalin Marinas <[email protected]>
Cc Lorenzo Pieralisi <[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(+)

Most of the static array storage is a waste of memory but it makes things much simpler and if it does become a problem we know how to fix it so:

Quoted 51 lines
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
#13 lore raw

PATCH 12/14 arm64: 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.

Cc Catalin Marinas <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/acpi.h             |  5 ----
 arch/arm64/kernel/acpi_parking_protocol.c | 38 +++++++++----------------------
 arch/arm64/kernel/smp.c                   | 11 ---------
 3 files changed, 11 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 +11 -27
index 1f5655c..700a83a 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 struct parking_protocol_mailbox __iomem *mailboxes[NR_CPUS];
 
 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;
+	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,8 +106,7 @@ 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 = mailboxes[cpu];
 	__le64 entry_point;
 
 	entry_point = readl_relaxed(&mailbox->entry_point);
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
#14 lore raw

PATCH 12/14 arm64: kill acpi_set_mailbox_entry()

Quoted 51 lines · 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.

Cc Catalin Marinas <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/include/asm/acpi.h             |  5 ----
 arch/arm64/kernel/acpi_parking_protocol.c | 38 +++++++++----------------------
 arch/arm64/kernel/smp.c                   | 11 ---------
 3 files changed, 11 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 +0 -9
index 1f5655c..700a83a 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];
-

arch/arm64/kernel/acpi_parking_protocol.c struct parking_protocol_mailbox { · line 31

Quoted 10 lines
arch/arm64/kernel/acpi_parking_protocol.c +1 -9
-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 struct parking_protocol_mailbox __iomem *mailboxes[NR_CPUS];

Nit: If I am not mistaken it can be made a percpu pointer.

Other than that:

Quoted 90 lines
arch/arm64/kernel/acpi_parking_protocol.c +10 -9
 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;
+	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,8 +106,7 @@ 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 = mailboxes[cpu];
 	__le64 entry_point;
 
 	entry_point = readl_relaxed(&mailbox->entry_point);
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
#15 lore raw

PATCH 12/14 arm64: kill acpi_set_mailbox_entry()

Quoted 5 lines · Lorenzo Pieralisi
Quoted 1 line · Mark Rutland

+static struct parking_protocol_mailbox __iomem *mailboxes[NR_CPUS];

Nit: If I am not mistaken it can be made a percpu pointer.

Unfortunately, we don't set up the percpu areas until we know how many CPUs are possible (i.e. after we've parsed the MADT).

The flow looks like:

start_kernel()
setup_arch()
smp_init_cpus()
acpi_table_parse_madt()
acpi_parse_gic_cpu_interface()
acpi_map_gic_cpu_interface() // setup cpu mailbox here
...
setup_per_cpu_areas() // percpu available here

Otherwise, I completely agree that a percpu pointer would be the right thing to do.

Quoted 3 lines

Other than that:

I take it this stands, given the above?

Thanks,
Mark.

#16 lore raw

PATCH 12/14 arm64: kill acpi_set_mailbox_entry()

Quoted 10 lines · Mark Rutland
Quoted 5 lines · Lorenzo Pieralisi
Quoted 1 line · Mark Rutland

+static struct parking_protocol_mailbox __iomem *mailboxes[NR_CPUS];

Nit: If I am not mistaken it can be made a percpu pointer.

Unfortunately, we don't set up the percpu areas until we know how many CPUs are possible (i.e. after we've parsed the MADT).

Quoted 8 lines

The flow looks like:

start_kernel()
setup_arch()
smp_init_cpus()
acpi_table_parse_madt()
acpi_parse_gic_cpu_interface()
acpi_map_gic_cpu_interface() // setup cpu mailbox here

The mailbox data yes, not the pointer at which it is mapped (which is what gets stashed in mailboxes[]).

Quoted 5 lines

...
setup_per_cpu_areas() // percpu available here

Otherwise, I completely agree that a percpu pointer would be the right thing to do.

Yes but the only purpose of the mailboxes array is to stash the ioremap'ed VA after a secondary has been booted (through cpu_ops->cpu_boot which is called to boot secondaries when per_cpu areas are already set-up) to run checks in cpu_postboot() method, hence my suggestion.

Quoted 6 lines
Quoted 4 lines

Other than that:

I take it this stands, given the above?

Yes, thanks for making this change.

Lorenzo

author maintainer
#17 lore raw

PATCH 12/14 arm64: kill acpi_set_mailbox_entry()

Quoted 20 lines · Lorenzo Pieralisi
Quoted 10 lines · Mark Rutland
Quoted 5 lines · Lorenzo Pieralisi
Quoted 1 line · Mark Rutland

+static struct parking_protocol_mailbox __iomem *mailboxes[NR_CPUS];

Nit: If I am not mistaken it can be made a percpu pointer.

Unfortunately, we don't set up the percpu areas until we know how many CPUs are possible (i.e. after we've parsed the MADT).

Quoted 8 lines

The flow looks like:

start_kernel()
setup_arch()
smp_init_cpus()
acpi_table_parse_madt()
acpi_parse_gic_cpu_interface()
acpi_map_gic_cpu_interface() // setup cpu mailbox here

Quoted 2 lines

The mailbox data yes, not the pointer at which it is mapped (which is what gets stashed in mailboxes[]).

Sorry, I got myself confused here between this and the MADT GICC stashing to which the above does apply.

You are correct that this can and should be a percpu pointer.

I'll fix this up. I just need to figure out where the __iomem attribute is supposed to live on a percpu definition.

Thanks,
Mark.

author maintainer
#18 lore raw

PATCH 12/14 arm64: kill acpi_set_mailbox_entry()

Quoted 24 lines · Mark Rutland
Quoted 20 lines · Lorenzo Pieralisi
Quoted 10 lines · Mark Rutland
Quoted 5 lines · Lorenzo Pieralisi
Quoted 1 line · Mark Rutland

+static struct parking_protocol_mailbox __iomem *mailboxes[NR_CPUS];

Nit: If I am not mistaken it can be made a percpu pointer.

Unfortunately, we don't set up the percpu areas until we know how many CPUs are possible (i.e. after we've parsed the MADT).

Quoted 8 lines

The flow looks like:

start_kernel()
setup_arch()
smp_init_cpus()
acpi_table_parse_madt()
acpi_parse_gic_cpu_interface()
acpi_map_gic_cpu_interface() // setup cpu mailbox here

Quoted 2 lines

The mailbox data yes, not the pointer at which it is mapped (which is what gets stashed in mailboxes[]).

Quoted 7 lines

Sorry, I got myself confused here between this and the MADT GICC stashing to which the above does apply.

You are correct that this can and should be a percpu pointer.

I'll fix this up. I just need to figure out where the __iomem attribute is supposed to live on a percpu definition.

I gave sparse a go and figured that out.

In the process I found another issue, so I'm gonig to add the below to the series.

Thanks,
Mark.

--->8----

Quoted 1 line

From 3045f18e798a37b4956f837ed264704500ca45b9 Mon Sep 17 00:00:00 2001

From: Mark Rutland <[email protected]>
Date: Wed, 22 Mar 2017 11:23:20 +0000
Subject: [PATCH] arm64: parking: fix type endianness

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.

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
#19 lore raw

PATCH 13/14 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.

Cc 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  | 237 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/cpuhotplug.h   |   1 +
 include/linux/perf/arm_pmu.h |  11 ++
 6 files changed, 256 insertions(+), 2 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c
drivers/perf/Kconfig +4 -0
index 9365190..7947b70 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 4c0d68d..e964dac 100644
@@ -1,3 +1,4 @@
 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_XGENE_PMU) += xgene_pmu.o
drivers/perf/arm_pmu.c +2 -2
index 8fcc20f..09d727a 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 +237 -0

237 lines not on this page · raw message

include/linux/cpuhotplug.h +1 -0
index 62d240e..6c9cfa7 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
author maintainer
#20 lore raw

PATCH 14/14 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.

Cc Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

One thing I'm not sure about here is how to verify that PMUv3 is supported. I can put a test in armv8_pmuv3_init(), but it feels like that should be verified earlier.

Mark.

arch/arm64/kernel/perf_event.c +9 -17
index 57ae9d9..daf95919 100644
@@ -1081,24 +1081,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 = {
@@ -1109,4 +1094,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
#21 lore raw

PATCH 14/14 arm64: pmuv3: use arm_pmu ACPI framework

Hi Mark,

Quoted 49 lines · 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.

Cc Jeremy Linton <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/kernel/perf_event.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

One thing I'm not sure about here is how to verify that PMUv3 is supported. I can put a test in armv8_pmuv3_init(), but it feels like that should be verified earlier.

Mark.

arch/arm64/kernel/perf_event.c +1 -16
index 57ae9d9..daf95919 100644
@@ -1081,24 +1081,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 = {

arch/arm64/kernel/perf_event.c static int armv8_pmu_device_probe(struct platform_device *pdev) · line 1102

Quoted 10 lines
arch/arm64/kernel/perf_event.c +6 -1
@@ -1109,4 +1094,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);

i think this function needs to be updated to
probe SoC specific init (armv8_a53_pmu_init, armv8_vulcan_pmu_init etc) as well.

Quoted 4 lines
arch/arm64/kernel/perf_event.c +2 -1
+}
+device_initcall(armv8_pmu_driver_init)
--

1.9.1

thanks
Ganapat

Quoted 6 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

author maintainer
#22 lore raw

PATCH 14/14 arm64: pmuv3: use arm_pmu ACPI framework

Hi Ganapatrao,

Quoted 10 lines · Ganapatrao Kulkarni

arch/arm64/kernel/perf_event.c static int armv8_pmu_device_probe(struct platform_device *pdev) · line 1102

Quoted 6 lines
arch/arm64/kernel/perf_event.c +6 -0
+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);

i think this function needs to be updated to
probe SoC specific init (armv8_a53_pmu_init, armv8_vulcan_pmu_init etc) as well.

Only exposing generic PMUv3 here was a deliberate decision, based on prior discussions, so as to keep things consistent on ACPI platforms regardless of which CPU types the PMU code is immediately aware of.

I should have documented this better; sorry about that.

All events which can be enumerated via the PMU ID registers will be exposed via sysfs.

Other events (e.g. those which are implementation defined) will not be exposed via sysfs, but can still be used as raw events.

There's been some perf tool work to allow these to be enumerated in userspace based on JSON files. This allows new events to be supported with a userspace update, event when the kernel itself is older than the CPU in question, and does not know anything about its events.

Thanks,
Mark.

#23 lore raw

PATCH 14/14 arm64: pmuv3: use arm_pmu ACPI framework

Quoted 14 lines · Mark Rutland

Hi Ganapatrao,

Quoted 10 lines · Ganapatrao Kulkarni

arch/arm64/kernel/perf_event.c static int armv8_pmu_device_probe(struct platform_device *pdev) · line 1102

Quoted 6 lines
arch/arm64/kernel/perf_event.c +6 -0
+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);

i think this function needs to be updated to
probe SoC specific init (armv8_a53_pmu_init, armv8_vulcan_pmu_init etc) as well.

Quoted 8 lines

Only exposing generic PMUv3 here was a deliberate decision, based on prior discussions, so as to keep things consistent on ACPI platforms regardless of which CPU types the PMU code is immediately aware of.

I should have documented this better; sorry about that.

All events which can be enumerated via the PMU ID registers will be exposed via sysfs.

If we take a specific case here of L1D read access, it will be mapped to either ARMV8_IMPDEF_PERFCTR_L1D_CACHE_RD or ARMV8_PMUV3_PERFCTR_L1D_CACHE depending on whether
you booted from ACPI or DT (on most platforms).

This does not look correct, am I missing something?

Thanks,
JC.

#24 lore raw

PATCH 14/14 arm64: pmuv3: use arm_pmu ACPI framework

In reply to Mark Rutland
Quoted 23 lines · Mark Rutland

Hi Ganapatrao,

Quoted 10 lines · Ganapatrao Kulkarni

arch/arm64/kernel/perf_event.c static int armv8_pmu_device_probe(struct platform_device *pdev) · line 1102

Quoted 6 lines
arch/arm64/kernel/perf_event.c +6 -0
+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);

i think this function needs to be updated to
probe SoC specific init (armv8_a53_pmu_init, armv8_vulcan_pmu_init etc) as well.

Only exposing generic PMUv3 here was a deliberate decision, based on prior discussions, so as to keep things consistent on ACPI platforms regardless of which CPU types the PMU code is immediately aware of.

I should have documented this better; sorry about that.

All events which can be enumerated via the PMU ID registers will be exposed via sysfs.

Quoted 7 lines

Other events (e.g. those which are implementation defined) will not be exposed via sysfs, but can still be used as raw events.

There's been some perf tool work to allow these to be enumerated in userspace based on JSON files. This allows new events to be supported with a userspace update, event when the kernel itself is older than the CPU in question, and does not know anything about its events.

At present, i see JSON script being added to x86 and powerpc is anybody working on it to extend this to arm64 as well?

Quoted 3 lines

Thanks,
Mark.

thanks
Ganapat

#25 lore raw

PATCH 14/14 arm64: pmuv3: use arm_pmu ACPI framework

In reply to Mark Rutland
Quoted 16 lines · Mark Rutland

Hi Ganapatrao,

Quoted 9 lines · Ganapatrao Kulkarni

arch/arm64/kernel/perf_event.c static int armv8_pmu_device_probe(struct platform_device *pdev) · line 1102

Quoted 6 lines
arch/arm64/kernel/perf_event.c +6 -0
+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);

i think this function needs to be updated to
probe SoC specific init (armv8_a53_pmu_init, armv8_vulcan_pmu_init etc) as well.

Only exposing generic PMUv3 here was a deliberate decision, based on prior discussions, so as to keep things consistent on ACPI platforms regardless of which CPU types the PMU code is immediately aware of.

Quoted 7 lines

I should have documented this better; sorry about that.

All events which can be enumerated via the PMU ID registers will be exposed via sysfs.

Other events (e.g. those which are implementation defined) will not be exposed via sysfs, but can still be used as raw events.

Hmm, this means inconsistency of pmu events for system booted with ACPI and DT (DT may represent more events), is there any plan to support events not in generic PMUv3 for further steps in the future?

Thanks
Hanjun

#26 lore raw

cover arm_pmu: ACPI support

In reply to Mark Rutland

Hi,

Quoted 10 lines · Mark Rutland

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.

Quoted 8 lines

The series is based on my IRQ rework patches [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.

Which is disappointing, as I thought that was one of Lorenzo's complaints about the last patch series.

Quoted 4 lines

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:

Quoted 8 lines
  $ 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.

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them. Although its quite possibly PEBCAK.

Given that SPCR/etc is also broken and the kernel itself seems to fail if I give it a full earlycon line, I can't pin down whats going on with the PMU code until I fix the console.

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I'm OOO for the next week, and will debug this further when I return if no one else makes any progress.

Quoted 39 lines

Thanks,
Mark.

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

Mark Rutland (14):
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: kill acpi_set_mailbox_entry()
drivers/perf: arm_pmu: add ACPI framework
arm64: pmuv3: use arm_pmu ACPI framework

 arch/arm64/include/asm/acpi.h             |   7 +-
 arch/arm64/kernel/acpi_parking_protocol.c |  38 +--
 arch/arm64/kernel/perf_event.c            |  26 +-
 arch/arm64/kernel/smp.c                   |  19 +-
 drivers/perf/Kconfig                      |   4 +
 drivers/perf/Makefile                     |   3 +-
 drivers/perf/arm_pmu.c                    | 383 ++++++++----------------------
 drivers/perf/arm_pmu_acpi.c               | 237 ++++++++++++++++++
 drivers/perf/arm_pmu_platform.c           | 235 ++++++++++++++++++
 include/linux/cpuhotplug.h                |   1 +
 include/linux/perf/arm_pmu.h              |  22 +-
 11 files changed, 626 insertions(+), 349 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c
 create mode 100644 drivers/perf/arm_pmu_platform.c
author maintainer
#27 lore raw

cover arm_pmu: ACPI support

Hi,

Quoted 7 lines · Jeremy Linton
Quoted 1 line · Mark Rutland

This series implements ACPI support in the ARM PMU code.

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Indeed; sorry about this. I'll see if I can get access to a board to try local debugging.

I assume that these have successfully booted with the other ACPI PMU patches?

Quoted 3 lines

Given that SPCR/etc is also broken and the kernel itself seems to fail if I give it a full earlycon line, I can't pin down whats going on with the PMU code until I fix the console.

If there are known broken tables, it's possible that the MADT GICC is also broken, and we're registering/unregistering an erroneously described interrupt.

Quoted 5 lines

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I guess that's at the point it transfers over to the real console driver. If you're not already doing so, also passing "keep_bootcon" may get more output.

Quoted 2 lines

I'm OOO for the next week, and will debug this further when I return if no one else makes any progress.

I'll see what I can do until then.

Thanks,
Mark.

author maintainer
#28 lore raw

cover arm_pmu: ACPI support

Quoted 14 lines · Mark Rutland
Quoted 4 lines · Jeremy Linton

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Indeed; sorry about this. I'll see if I can get access to a board to try local debugging.

Quoted 5 lines

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I believe that this is a latent FW bug, in a beta FW, exposed by recent changes.

I managed to get access to two APM Mustang boards. I can reproduce the issue with a vanilla v4.11-rc1 defconfig on one, which has a beta FW. The same kernel boots fine on the other, which has a later released FW.

I bisected the issue down to commit:

d44fa3d46079dc09 ("ACPI: Add support for ResourceSource/IRQ domain mapping")

It seems the beta FW describes the UART interrupt with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. Per the spec, this means "This device produces and consumes this resource", which doesn't make sense here given the UART is not an interrupt controller.

The (working) released FW doesn't use an Extended Interrupt Descriptor for this interrupt, sidestepping the issue.

Given that (AFAICT) this only affects a known-broken, beta FW, I don't think that we care that much.

However, I think there is a larger potential issue here.

In acpi_irq_parse_one_cb(), we skip interrupts with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. It sounds like devices which behave as interrupt controllers could legitimately have this bit set for interrupts they generate and deliver to themselves, and we'd erroneously skip these when parsing interrupts.

It's not entirely clear to me why this bit exists at all, given that it's not used as part of mapping the interrupt, and if you really want to know you can map the interrupt and look at the result.

Thanks,
Mark.

#29 lore raw

cover arm_pmu: ACPI support

Hi Mark,

Quoted 49 lines · Mark Rutland
Quoted 15 lines · Mark Rutland
Quoted 4 lines · Jeremy Linton

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Indeed; sorry about this. I'll see if I can get access to a board to try
local debugging.

Quoted 5 lines

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I believe that this is a latent FW bug, in a beta FW, exposed by recent changes.

I managed to get access to two APM Mustang boards. I can reproduce the issue with a vanilla v4.11-rc1 defconfig on one, which has a beta FW. The same kernel boots fine on the other, which has a later released FW.

I bisected the issue down to commit:

d44fa3d46079dc09 ("ACPI: Add support for ResourceSource/IRQ domain
mapping")

It seems the beta FW describes the UART interrupt with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. Per the spec,
this means "This device produces and consumes this resource", which doesn't make sense here given the UART is not an interrupt controller.

The (working) released FW doesn't use an Extended Interrupt Descriptor for this interrupt, sidestepping the issue.

Given that (AFAICT) this only affects a known-broken, beta FW, I don't think that we care that much.

However, I think there is a larger potential issue here.

In acpi_irq_parse_one_cb(), we skip interrupts with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. It sounds like devices which behave as interrupt controllers could legitimately have this bit set for interrupts they generate and deliver to themselves, and we'd erroneously skip these when parsing interrupts.

Quoted 4 lines

It's not entirely clear to me why this bit exists at all, given that it's not used as part of mapping the interrupt, and if you really want to know you can map the interrupt and look at the result.

I think the best approach would be to try to amend the spec and make zero mean producer only which is the way it is being treated in Linux. That would mean that a device consuming its own interrupts would have to have two resources, one for describing the resource as producer and one as consumer.

Unfortunately the spec is vague in some areas like the feature we used to implement secondary IRQ controllers which require this patch.

Hanjun, can you bring this up at the next ASWG meeting? I don't normally attend but I will ask our ASWG lead to invite me so we can discuss this in that forum.

Thanks,
Agustin

Signature · 4 lines
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.
#30 lore raw

cover arm_pmu: ACPI support

Quoted 57 lines · Agustin Vega-Frias

Hi Mark,

Quoted 49 lines · Mark Rutland
Quoted 6 lines · Mark Rutland
Quoted 4 lines · Jeremy Linton

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Quoted 10 lines

Indeed; sorry about this. I'll see if I can get access to a board to try local debugging.

Quoted 1 line

About the only thing it says with any meaning when earlycon is

passed is:

Quoted 1 line
[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing

enabled

Quoted 3 lines
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I believe that this is a latent FW bug, in a beta FW, exposed by recent changes.

I managed to get access to two APM Mustang boards. I can reproduce the issue with a vanilla v4.11-rc1 defconfig on one, which has a beta FW. The same kernel boots fine on the other, which has a later released FW.

I bisected the issue down to commit:

d44fa3d46079dc09 ("ACPI: Add support for ResourceSource/IRQ domain
mapping")

It seems the beta FW describes the UART interrupt with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. Per the spec, this means "This device produces and consumes this resource", which doesn't make sense here given the UART is not an interrupt controller.

The (working) released FW doesn't use an Extended Interrupt Descriptor for this interrupt, sidestepping the issue.

Given that (AFAICT) this only affects a known-broken, beta FW, I don't think that we care that much.

However, I think there is a larger potential issue here.

In acpi_irq_parse_one_cb(), we skip interrupts with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. It sounds like devices which behave as interrupt controllers could legitimately have this bit set for interrupts they generate and deliver to themselves, and we'd erroneously skip these when parsing interrupts.

Quoted 4 lines

It's not entirely clear to me why this bit exists at all, given that it's not used as part of mapping the interrupt, and if you really want to know you can map the interrupt and look at the result.

Quoted 5 lines

I think the best approach would be to try to amend the spec and make zero mean producer only which is the way it is being treated in Linux. That would mean that a device consuming its own interrupts would have to have two resources, one for describing the resource as producer and one as consumer.

Agreed. I think it's a typo in the spec, should be only produce resource if it's 0, and that's the behavior in ACPICA core now (both used by linux kernel and windows).

Quoted 7 lines

Unfortunately the spec is vague in some areas like the feature we used to implement secondary IRQ controllers which require this patch.

Hanjun, can you bring this up at the next ASWG meeting? I don't normally attend but I will ask our ASWG lead to invite me so we can discuss this in that forum.

I will do that, and I will send an email to report this issue first with you and Mark CCed.

Thanks
Hanjun

#31 lore raw

cover arm_pmu: ACPI support

In reply to Mark Rutland
Quoted 46 lines · Mark Rutland
Quoted 14 lines · Mark Rutland
Quoted 4 lines · Jeremy Linton

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Indeed; sorry about this. I'll see if I can get access to a board to try local debugging.

Quoted 5 lines

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I believe that this is a latent FW bug, in a beta FW, exposed by recent changes.

I managed to get access to two APM Mustang boards. I can reproduce the issue with a vanilla v4.11-rc1 defconfig on one, which has a beta FW. The same kernel boots fine on the other, which has a later released FW.

I bisected the issue down to commit:

d44fa3d46079dc09 ("ACPI: Add support for ResourceSource/IRQ domain mapping")

It seems the beta FW describes the UART interrupt with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. Per the spec, this means "This device produces and consumes this resource", which doesn't make sense here given the UART is not an interrupt controller.

The (working) released FW doesn't use an Extended Interrupt Descriptor for this interrupt, sidestepping the issue.

Given that (AFAICT) this only affects a known-broken, beta FW, I don't think that we care that much.

However, I think there is a larger potential issue here.

In acpi_irq_parse_one_cb(), we skip interrupts with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. It sounds like devices which behave as interrupt controllers could legitimately have this bit set for interrupts they generate and deliver to themselves, and we'd erroneously skip these when parsing interrupts.

Quoted 3 lines

It's not entirely clear to me why this bit exists at all, given that it's not used as part of mapping the interrupt, and if you really want to know you can map the interrupt and look at the result.

It is not entirely clear to anyone (and it has been a source of major trouble for other descriptors - ie memory - to the point that it has been deprecated for *some* descriptors). IIRC (Hanjun ?) the only reason why Agustin added the check for that bit in drivers/acpi/resource.c (is_gsi()) is to prevent ACPI core code allocating IRQs for the MBIgen device that, _erroneously_, was using an extended interrupt descriptor to list IRQ lines (that I have no idea what they represent) in order to count how many MSI vectors it would have to allocate.

The gist is: removing the check for the extended interrupt descriptor producer/consumer bit in ACPI code therefore ignoring that bit should be harmless (given that the MBIgen bindings should be changed anyway).

The meaning of that bit (and how we can flag up a FW bug) it is still a bit fuzzy IMO.

Agustin, Hanjun, comments ?

Lorenzo

#32 lore raw

cover arm_pmu: ACPI support

Hi Lorenzo,

Quoted 65 lines · Lorenzo Pieralisi
Quoted 50 lines · Mark Rutland
Quoted 14 lines · Mark Rutland
Quoted 4 lines · Jeremy Linton

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Indeed; sorry about this. I'll see if I can get access to a board to try local debugging.

Quoted 5 lines

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I believe that this is a latent FW bug, in a beta FW, exposed by recent
changes.

I managed to get access to two APM Mustang boards. I can reproduce the issue with a vanilla v4.11-rc1 defconfig on one, which has a beta FW. The same kernel boots fine on the other, which has a later released FW.

I bisected the issue down to commit:

d44fa3d46079dc09 ("ACPI: Add support for ResourceSource/IRQ domain
mapping")

It seems the beta FW describes the UART interrupt with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. Per the spec,
this means "This device produces and consumes this resource", which doesn't make sense here given the UART is not an interrupt controller.

The (working) released FW doesn't use an Extended Interrupt Descriptor for this interrupt, sidestepping the issue.

Given that (AFAICT) this only affects a known-broken, beta FW, I don't think that we care that much.

However, I think there is a larger potential issue here.

In acpi_irq_parse_one_cb(), we skip interrupts with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. It sounds like devices which behave as interrupt controllers could legitimately have this bit set for interrupts they generate and deliver to themselves, and we'd erroneously skip these when parsing interrupts.

Quoted 3 lines

It's not entirely clear to me why this bit exists at all, given that it's not used as part of mapping the interrupt, and if you really want to know you can map the interrupt and look at the result.

It is not entirely clear to anyone (and it has been a source of major trouble for other descriptors - ie memory - to the point that it has been
deprecated for *some* descriptors). IIRC (Hanjun ?) the only reason why Agustin added the check for that bit in drivers/acpi/resource.c (is_gsi()) is to prevent ACPI core code allocating IRQs for the MBIgen device that, _erroneously_, was using an extended interrupt descriptor to list IRQ lines (that I have no idea what they represent) in order to count how many MSI vectors it would have to allocate.

Quoted 9 lines

The gist is: removing the check for the extended interrupt descriptor producer/consumer bit in ACPI code therefore ignoring that bit should be harmless (given that the MBIgen bindings should be changed anyway).

The meaning of that bit (and how we can flag up a FW bug) it is still a bit fuzzy IMO.

Agustin, Hanjun, comments ?

Yes, that was the reason the check was added, if MBIgen will use a different mechanism to describe the IRQs it produces we can remove the check.

I'd still like for the ASWG to get to the bottom of the issue, given that the resource descriptor and the macro definitions contradict each other. Hanjun, was this discussed on last week's meeting?

Thanks,
Agustin

Signature · 4 lines
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.
#33 lore raw

cover arm_pmu: ACPI support

In reply to Lorenzo Pieralisi
Quoted 51 lines · Lorenzo Pieralisi
Quoted 46 lines · Mark Rutland
Quoted 14 lines · Mark Rutland
Quoted 4 lines · Jeremy Linton

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them.

Indeed; sorry about this. I'll see if I can get access to a board to try local debugging.

Quoted 5 lines

About the only thing it says with any meaning when earlycon is passed is:

[   10.965147] Serial: 8250/16550 driver, 32 ports, IRQ sharing enabled
[   11.064193] dw-apb-uart APMC0D08:00: cannot get irq

and promptly hangs up.

I believe that this is a latent FW bug, in a beta FW, exposed by recent changes.

I managed to get access to two APM Mustang boards. I can reproduce the issue with a vanilla v4.11-rc1 defconfig on one, which has a beta FW. The same kernel boots fine on the other, which has a later released FW.

I bisected the issue down to commit:

d44fa3d46079dc09 ("ACPI: Add support for ResourceSource/IRQ domain mapping")

It seems the beta FW describes the UART interrupt with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. Per the spec, this means "This device produces and consumes this resource", which doesn't make sense here given the UART is not an interrupt controller.

The (working) released FW doesn't use an Extended Interrupt Descriptor for this interrupt, sidestepping the issue.

Given that (AFAICT) this only affects a known-broken, beta FW, I don't think that we care that much.

However, I think there is a larger potential issue here.

In acpi_irq_parse_one_cb(), we skip interrupts with an Extended Interrupt Descriptor with the Consumer/Producer bit clear. It sounds like devices which behave as interrupt controllers could legitimately have this bit set for interrupts they generate and deliver to themselves, and we'd erroneously skip these when parsing interrupts.

Quoted 3 lines

It's not entirely clear to me why this bit exists at all, given that it's not used as part of mapping the interrupt, and if you really want to know you can map the interrupt and look at the result.

Quoted 8 lines

It is not entirely clear to anyone (and it has been a source of major trouble for other descriptors - ie memory - to the point that it has been deprecated for *some* descriptors). IIRC (Hanjun ?) the only reason why Agustin added the check for that bit in drivers/acpi/resource.c (is_gsi()) is to prevent ACPI core code allocating IRQs for the MBIgen device that, _erroneously_, was using an extended interrupt descriptor to list IRQ lines (that I have no idea what they represent) in order to count how many MSI vectors it would have to allocate.

I'm not sure it's error for a interrupt producer device (such as MBIgen) to represent its interrupt resources using Interrupt under _CRS, from the spec, we didn't violate anything, but in the OS side we expect a better way to count MSI vectors.

Quoted 9 lines

The gist is: removing the check for the extended interrupt descriptor producer/consumer bit in ACPI code therefore ignoring that bit should be harmless (given that the MBIgen bindings should be changed anyway).

The meaning of that bit (and how we can flag up a FW bug) it is still a bit fuzzy IMO.

Agustin, Hanjun, comments ?

I think it's firmware bug in the first place, PRODUCER bit can't be clear if the device itself is not an interrupt controller, as we can see that it's UART interrupt and UART can't be a interrupt controller right? and the default value for the Consumer/Producer bit is 1, not 0, the spec clearly say that.

So I think we need to fix the firmware (then the code is still not violate any platforms), and also we need to fix the spec to clarify things (As Agustin proposed), I reported this issue to ASWG but seems my email was blocked, I will raise an ECR for discussion.

Thanks
Hanjun

author maintainer
#34 lore raw

cover arm_pmu: ACPI support

In reply to Jeremy Linton
Quoted 9 lines · Jeremy Linton
Quoted 8 lines · Mark Rutland

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
Quoted 8 lines
Quoted 3 lines

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.

I tried these patches on a m400 (which uses PPIs), and the kernel fails to come up enough to login via the network (which works with 4.11rc1 without these patches). So, I suspect there is something wrong with them. Although its quite possibly PEBCAK.

It looks like we've gotten to the bottom of the hang in the other thread; I had a go with a hack to bodge around that (as I have not yet upgraded the FW).

I had a go with the above taskset pattern (limited to a single PMU) on an APM Mustang board. Everything seems happy, even when I booted with maxcpus temporarily capped, where CPUs successfully associated themselves with the PMU later on.

Thanks,
Mark.

#35 lore raw

cover arm_pmu: ACPI support

In reply to Mark Rutland
Quoted 22 lines · Mark Rutland

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 my IRQ rework patches [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:

Quoted 8 lines
  $ 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.

Booted OK and 'perf list' got on Hisilicon D03:

d03-09:~ # perf list

List of pre-defined events (to be used in -e):

  branch-misses                                      [Hardware event]
  bus-cycles                                         [Hardware event]
  cache-misses                                       [Hardware event]
  cache-references                                   [Hardware event]
  cpu-cycles OR cycles                               [Hardware event]
  instructions                                       [Hardware event]
  alignment-faults                                   [Software event]
  context-switches OR cs                             [Software event]
  cpu-clock                                          [Software event]
  cpu-migrations OR migrations                       [Software event]
  dummy                                              [Software event]
  emulation-faults                                   [Software event]
  major-faults                                       [Software event]
  minor-faults                                       [Software event]
  page-faults OR faults                              [Software event]
  task-clock                                         [Software event]
  L1-dcache-load-misses                              [Hardware cache event]
  L1-dcache-loads                                    [Hardware cache event]
  L1-dcache-store-misses                             [Hardware cache event]
  L1-dcache-stores                                   [Hardware cache event]
  L1-icache-load-misses                              [Hardware cache event]
  L1-icache-loads                                    [Hardware cache event]
  branch-load-misses                                 [Hardware cache event]
  branch-loads                                       [Hardware cache event]
  dTLB-load-misses                                   [Hardware cache event]
  iTLB-load-misses                                   [Hardware cache event]
  armv8_pmuv3_0/br_mis_pred/                         [Kernel PMU event]
  armv8_pmuv3_0/br_pred/                             [Kernel PMU event]
  armv8_pmuv3_0/bus_access/                          [Kernel PMU event]
  armv8_pmuv3_0/bus_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/cid_write_retired/                   [Kernel PMU event]
  armv8_pmuv3_0/cpu_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_return/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_taken/                           [Kernel PMU event]
  armv8_pmuv3_0/inst_retired/                        [Kernel PMU event]
  armv8_pmuv3_0/inst_spec/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/l1d_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1i_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/mem_access/                          [Kernel PMU event]
  armv8_pmuv3_0/memory_error/                        [Kernel PMU event]
  armv8_pmuv3_0/sw_incr/                             [Kernel PMU event]
  armv8_pmuv3_0/ttbr_write_retired/                  [Kernel PMU event]

rNNN [Raw hardware event descriptor]
cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
(see 'man perf-list' on how to encode it)

mem:<addr>[/len][:access] [Hardware breakpoint]

Try some basic perf event and it works, anything else I can try in specific?

Thanks
Hanjun

#36 lore raw

cover arm_pmu: ACPI support

Hi Hanjun,

Quoted 92 lines · Hanjun Guo
Quoted 22 lines · Mark Rutland

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 my IRQ rework patches [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:

Quoted 8 lines
  $ 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.

Booted OK and 'perf list' got on Hisilicon D03:

d03-09:~ # perf list

List of pre-defined events (to be used in -e):

  branch-misses                                      [Hardware event]
  bus-cycles                                         [Hardware event]
  cache-misses                                       [Hardware event]
  cache-references                                   [Hardware event]
  cpu-cycles OR cycles                               [Hardware event]
  instructions                                       [Hardware event]
  alignment-faults                                   [Software event]
  context-switches OR cs                             [Software event]
  cpu-clock                                          [Software event]
  cpu-migrations OR migrations                       [Software event]
  dummy                                              [Software event]
  emulation-faults                                   [Software event]
  major-faults                                       [Software event]
  minor-faults                                       [Software event]
  page-faults OR faults                              [Software event]
  task-clock                                         [Software event]
  L1-dcache-load-misses                              [Hardware cache event]
  L1-dcache-loads                                    [Hardware cache event]
  L1-dcache-store-misses                             [Hardware cache event]
  L1-dcache-stores                                   [Hardware cache event]
  L1-icache-load-misses                              [Hardware cache event]
  L1-icache-loads                                    [Hardware cache event]
  branch-load-misses                                 [Hardware cache event]
  branch-loads                                       [Hardware cache event]
  dTLB-load-misses                                   [Hardware cache event]
  iTLB-load-misses                                   [Hardware cache event]
  armv8_pmuv3_0/br_mis_pred/                         [Kernel PMU event]
  armv8_pmuv3_0/br_pred/                             [Kernel PMU event]
  armv8_pmuv3_0/bus_access/                          [Kernel PMU event]
  armv8_pmuv3_0/bus_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/cid_write_retired/                   [Kernel PMU event]
  armv8_pmuv3_0/cpu_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_return/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_taken/                           [Kernel PMU event]
  armv8_pmuv3_0/inst_retired/                        [Kernel PMU event]
  armv8_pmuv3_0/inst_spec/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/l1d_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1i_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/mem_access/                          [Kernel PMU event]
  armv8_pmuv3_0/memory_error/                        [Kernel PMU event]
  armv8_pmuv3_0/sw_incr/                             [Kernel PMU event]
  armv8_pmuv3_0/ttbr_write_retired/                  [Kernel PMU event]
Quoted 8 lines

rNNN [Raw hardware event descriptor]
cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
(see 'man perf-list' on how to encode it)

mem:<addr>[/len][:access] [Hardware breakpoint]

Try some basic perf event and it works, anything else I can try in specific?

you can try perf fuzzer

thanks
Ganapat

Quoted 9 lines

Thanks
Hanjun

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#37 lore raw

cover arm_pmu: ACPI support

i am not able to run with latest perf tool, i see issue with memory corruption in malloc
is this seen from anyone else too?
i am not sure, is this issue with my setup?

root at VAL1-13>perf>> perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1

Performance counter stats for 'sleep 1':

            10,815      armv8_pmuv3_0/br_mis_pred/

       1.001232167 seconds time elapsed

root at VAL1-13>perf>> ./perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1 *** Error in `./perf': free(): invalid next size (fast): 0x00000000086a7a00 *** Aborted (core dumped)
root at VAL1-13>perf>> ./perf -v
perf version 4.11.rc3.g093b99
root at VAL1-13>perf>> perf -v
perf version 4.8.rc6.g21c488
root at VAL1-13>perf>> uname -a
Linux VAL1-13 4.11.0-rc2-12139-g3804e12 #1 SMP PREEMPT Wed Mar 22 06:26:28 UTC 2017 aarch64 aarch64 aarch64 GNU/Linux
root at VAL1-13>perf>>

Quoted 102 lines · Ganapatrao Kulkarni

Hi Hanjun,

Quoted 99 lines · Hanjun Guo
Quoted 22 lines · Mark Rutland

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 my IRQ rework patches [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:

Quoted 8 lines
  $ 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.

Booted OK and 'perf list' got on Hisilicon D03:

d03-09:~ # perf list

List of pre-defined events (to be used in -e):

  branch-misses                                      [Hardware event]
  bus-cycles                                         [Hardware event]
  cache-misses                                       [Hardware event]
  cache-references                                   [Hardware event]
  cpu-cycles OR cycles                               [Hardware event]
  instructions                                       [Hardware event]
  alignment-faults                                   [Software event]
  context-switches OR cs                             [Software event]
  cpu-clock                                          [Software event]
  cpu-migrations OR migrations                       [Software event]
  dummy                                              [Software event]
  emulation-faults                                   [Software event]
  major-faults                                       [Software event]
  minor-faults                                       [Software event]
  page-faults OR faults                              [Software event]
  task-clock                                         [Software event]
  L1-dcache-load-misses                              [Hardware cache event]
  L1-dcache-loads                                    [Hardware cache event]
  L1-dcache-store-misses                             [Hardware cache event]
  L1-dcache-stores                                   [Hardware cache event]
  L1-icache-load-misses                              [Hardware cache event]
  L1-icache-loads                                    [Hardware cache event]
  branch-load-misses                                 [Hardware cache event]
  branch-loads                                       [Hardware cache event]
  dTLB-load-misses                                   [Hardware cache event]
  iTLB-load-misses                                   [Hardware cache event]
  armv8_pmuv3_0/br_mis_pred/                         [Kernel PMU event]
  armv8_pmuv3_0/br_pred/                             [Kernel PMU event]
  armv8_pmuv3_0/bus_access/                          [Kernel PMU event]
  armv8_pmuv3_0/bus_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/cid_write_retired/                   [Kernel PMU event]
  armv8_pmuv3_0/cpu_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_return/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_taken/                           [Kernel PMU event]
  armv8_pmuv3_0/inst_retired/                        [Kernel PMU event]
  armv8_pmuv3_0/inst_spec/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/l1d_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1i_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/mem_access/                          [Kernel PMU event]
  armv8_pmuv3_0/memory_error/                        [Kernel PMU event]
  armv8_pmuv3_0/sw_incr/                             [Kernel PMU event]
  armv8_pmuv3_0/ttbr_write_retired/                  [Kernel PMU event]

rNNN [Raw hardware event descriptor]
cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
(see 'man perf-list' on how to encode it)

mem:<addr>[/len][:access] [Hardware breakpoint]

Quoted 16 lines
Quoted 1 line

Try some basic perf event and it works, anything else I can try in specific?

you can try perf fuzzer

thanks
Ganapat

Quoted 9 lines

Thanks
Hanjun

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

thanks
Ganapat

author maintainer
#38 lore raw

cover arm_pmu: ACPI support

Quoted 13 lines · Ganapatrao Kulkarni

i am not able to run with latest perf tool, i see issue with memory corruption in malloc
is this seen from anyone else too?
i am not sure, is this issue with my setup?

root at VAL1-13>perf>> perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1

Performance counter stats for 'sleep 1':

            10,815      armv8_pmuv3_0/br_mis_pred/

       1.001232167 seconds time elapsed
Quoted 11 lines

root at VAL1-13>perf>> ./perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1 *** Error in `./perf': free(): invalid next size (fast): 0x00000000086a7a00 *** Aborted (core dumped)
root at VAL1-13>perf>> ./perf -v
perf version 4.11.rc3.g093b99
root at VAL1-13>perf>> perf -v
perf version 4.8.rc6.g21c488
root at VAL1-13>perf>> uname -a
Linux VAL1-13 4.11.0-rc2-12139-g3804e12 #1 SMP PREEMPT Wed Mar 22 06:26:28 UTC 2017 aarch64 aarch64 aarch64 GNU/Linux
root at VAL1-13>perf>>

I've managed to reproduce this on a Seatle system using DT, with the perf tool from v4.11-rc3.

I'm digging into this now.

Thanks,
Mark.

#39 lore raw

cover arm_pmu: ACPI support

In reply to Ganapatrao Kulkarni

Hi Mark,

Quoted 13 lines · Ganapatrao Kulkarni

i am not able to run with latest perf tool, i see issue with memory corruption in malloc
is this seen from anyone else too?
i am not sure, is this issue with my setup?

root at VAL1-13>perf>> perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1

Performance counter stats for 'sleep 1':

            10,815      armv8_pmuv3_0/br_mis_pred/

       1.001232167 seconds time elapsed
Quoted 12 lines

root at VAL1-13>perf>> ./perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1 *** Error in `./perf': free(): invalid next size (fast): 0x00000000086a7a00 *** Aborted (core dumped)
root at VAL1-13>perf>> ./perf -v
perf version 4.11.rc3.g093b99
root at VAL1-13>perf>> perf -v
perf version 4.8.rc6.g21c488
root at VAL1-13>perf>> uname -a
Linux VAL1-13 4.11.0-rc2-12139-g3804e12 #1 SMP PREEMPT Wed Mar 22 06:26:28 UTC 2017 aarch64 aarch64 aarch64 GNU/Linux
root at VAL1-13>perf>>

i did bisect, and i see your patch,

7e3fcffe955440101493cd8f32f75840ddf87b6f
perf pmu: Support alternative sysfs cpumask

is causing this failure. if i suppress the call to function cpu_map__read then memory corruption is not seen.

Quoted 117 lines
Quoted 114 lines · Ganapatrao Kulkarni

Hi Hanjun,

Quoted 92 lines · Hanjun Guo
Quoted 22 lines · Mark Rutland

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 my IRQ rework patches [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:

Quoted 8 lines
  $ 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.

Booted OK and 'perf list' got on Hisilicon D03:

d03-09:~ # perf list

List of pre-defined events (to be used in -e):

  branch-misses                                      [Hardware event]
  bus-cycles                                         [Hardware event]
  cache-misses                                       [Hardware event]
  cache-references                                   [Hardware event]
  cpu-cycles OR cycles                               [Hardware event]
  instructions                                       [Hardware event]
  alignment-faults                                   [Software event]
  context-switches OR cs                             [Software event]
  cpu-clock                                          [Software event]
  cpu-migrations OR migrations                       [Software event]
  dummy                                              [Software event]
  emulation-faults                                   [Software event]
  major-faults                                       [Software event]
  minor-faults                                       [Software event]
  page-faults OR faults                              [Software event]
  task-clock                                         [Software event]
  L1-dcache-load-misses                              [Hardware cache event]
  L1-dcache-loads                                    [Hardware cache event]
  L1-dcache-store-misses                             [Hardware cache event]
  L1-dcache-stores                                   [Hardware cache event]
  L1-icache-load-misses                              [Hardware cache event]
  L1-icache-loads                                    [Hardware cache event]
  branch-load-misses                                 [Hardware cache event]
  branch-loads                                       [Hardware cache event]
  dTLB-load-misses                                   [Hardware cache event]
  iTLB-load-misses                                   [Hardware cache event]
  armv8_pmuv3_0/br_mis_pred/                         [Kernel PMU event]
  armv8_pmuv3_0/br_pred/                             [Kernel PMU event]
  armv8_pmuv3_0/bus_access/                          [Kernel PMU event]
  armv8_pmuv3_0/bus_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/cid_write_retired/                   [Kernel PMU event]
  armv8_pmuv3_0/cpu_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_return/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_taken/                           [Kernel PMU event]
  armv8_pmuv3_0/inst_retired/                        [Kernel PMU event]
  armv8_pmuv3_0/inst_spec/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/l1d_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1i_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/mem_access/                          [Kernel PMU event]
  armv8_pmuv3_0/memory_error/                        [Kernel PMU event]
  armv8_pmuv3_0/sw_incr/                             [Kernel PMU event]
  armv8_pmuv3_0/ttbr_write_retired/                  [Kernel PMU event]
Quoted 8 lines

rNNN [Raw hardware event descriptor]
cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
(see 'man perf-list' on how to encode it)

mem:<addr>[/len][:access] [Hardware breakpoint]

Try some basic perf event and it works, anything else I can try in specific?

you can try perf fuzzer

thanks
Ganapat

Quoted 5 lines

Thanks
Hanjun

Quoted 7 lines
Quoted 4 lines
Quoted 4 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

thanks
Ganapat

thanks
Ganapat

author maintainer
#40 lore raw

cover arm_pmu: ACPI support

Hi,

Quoted 27 lines · Ganapatrao Kulkarni
Quoted 13 lines · Ganapatrao Kulkarni

i am not able to run with latest perf tool, i see issue with memory corruption in malloc
is this seen from anyone else too?
i am not sure, is this issue with my setup?

root at VAL1-13>perf>> perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1

Performance counter stats for 'sleep 1':

            10,815      armv8_pmuv3_0/br_mis_pred/

       1.001232167 seconds time elapsed
Quoted 11 lines

root at VAL1-13>perf>> ./perf stat -e armv8_pmuv3_0/br_mis_pred/ sleep 1 *** Error in `./perf': free(): invalid next size (fast): 0x00000000086a7a00 *** Aborted (core dumped)
root at VAL1-13>perf>> ./perf -v
perf version 4.11.rc3.g093b99
root at VAL1-13>perf>> perf -v
perf version 4.8.rc6.g21c488
root at VAL1-13>perf>> uname -a
Linux VAL1-13 4.11.0-rc2-12139-g3804e12 #1 SMP PREEMPT Wed Mar 22 06:26:28 UTC 2017 aarch64 aarch64 aarch64 GNU/Linux
root at VAL1-13>perf>>

Quoted 7 lines

i did bisect, and i see your patch,

7e3fcffe955440101493cd8f32f75840ddf87b6f
perf pmu: Support alternative sysfs cpumask

is causing this failure. if i suppress the call to function cpu_map__read then memory corruption is not seen.

Thanks for tracking this down.

I can reproduce this on some machines; I'll try to see how this can be fixed shortly.

Thanks,
Mark.

#41 lore raw

cover arm_pmu: ACPI support

In reply to Ganapatrao Kulkarni
Quoted 98 lines · Ganapatrao Kulkarni

Hi Hanjun,

Quoted 95 lines · Hanjun Guo
Quoted 22 lines · Mark Rutland

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 my IRQ rework patches [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:

Quoted 8 lines
  $ 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.

Booted OK and 'perf list' got on Hisilicon D03:

d03-09:~ # perf list

List of pre-defined events (to be used in -e):

  branch-misses                                      [Hardware event]
  bus-cycles                                         [Hardware event]
  cache-misses                                       [Hardware event]
  cache-references                                   [Hardware event]
  cpu-cycles OR cycles                               [Hardware event]
  instructions                                       [Hardware event]
  alignment-faults                                   [Software event]
  context-switches OR cs                             [Software event]
  cpu-clock                                          [Software event]
  cpu-migrations OR migrations                       [Software event]
  dummy                                              [Software event]
  emulation-faults                                   [Software event]
  major-faults                                       [Software event]
  minor-faults                                       [Software event]
  page-faults OR faults                              [Software event]
  task-clock                                         [Software event]
  L1-dcache-load-misses                              [Hardware cache event]
  L1-dcache-loads                                    [Hardware cache event]
  L1-dcache-store-misses                             [Hardware cache event]
  L1-dcache-stores                                   [Hardware cache event]
  L1-icache-load-misses                              [Hardware cache event]
  L1-icache-loads                                    [Hardware cache event]
  branch-load-misses                                 [Hardware cache event]
  branch-loads                                       [Hardware cache event]
  dTLB-load-misses                                   [Hardware cache event]
  iTLB-load-misses                                   [Hardware cache event]
  armv8_pmuv3_0/br_mis_pred/                         [Kernel PMU event]
  armv8_pmuv3_0/br_pred/                             [Kernel PMU event]
  armv8_pmuv3_0/bus_access/                          [Kernel PMU event]
  armv8_pmuv3_0/bus_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/cid_write_retired/                   [Kernel PMU event]
  armv8_pmuv3_0/cpu_cycles/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_return/                          [Kernel PMU event]
  armv8_pmuv3_0/exc_taken/                           [Kernel PMU event]
  armv8_pmuv3_0/inst_retired/                        [Kernel PMU event]
  armv8_pmuv3_0/inst_spec/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/l1d_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l1i_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l1i_tlb_refill/                      [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache/                           [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_refill/                    [Kernel PMU event]
  armv8_pmuv3_0/l2d_cache_wb/                        [Kernel PMU event]
  armv8_pmuv3_0/mem_access/                          [Kernel PMU event]
  armv8_pmuv3_0/memory_error/                        [Kernel PMU event]
  armv8_pmuv3_0/sw_incr/                             [Kernel PMU event]
  armv8_pmuv3_0/ttbr_write_retired/                  [Kernel PMU event]

rNNN [Raw hardware event descriptor]
cpu/t1=v1[,t2=v2,t3 ...]/modifier [Raw hardware event descriptor]
(see 'man perf-list' on how to encode it)

Quoted 5 lines
Quoted 4 lines

mem:<addr>[/len][:access] [Hardware breakpoint]

Try some basic perf event and it works, anything else I can try in specific?

you can try perf fuzzer

Sorry, just back from my paternity leave, I will try Mark's v2 (or v3 if there is a updated one) :)

Thanks
Hanjun

#42 lore raw

cover arm_pmu: ACPI support

In reply to Mark Rutland
Quoted 27 lines · Mark Rutland

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 my IRQ rework patches [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
Quoted 6 lines

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.

Thanks,
Mark.

I've been testing these on a few machines (m400 & dual socket cavium for starters) over the last few days. So far everything seems to be working fine excepting the usual set of strange messages from the perf fuzzer.

Thanks,

Quoted 36 lines

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

Mark Rutland (14):
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: kill acpi_set_mailbox_entry()
drivers/perf: arm_pmu: add ACPI framework
arm64: pmuv3: use arm_pmu ACPI framework

 arch/arm64/include/asm/acpi.h             |   7 +-
 arch/arm64/kernel/acpi_parking_protocol.c |  38 +--
 arch/arm64/kernel/perf_event.c            |  26 +-
 arch/arm64/kernel/smp.c                   |  19 +-
 drivers/perf/Kconfig                      |   4 +
 drivers/perf/Makefile                     |   3 +-
 drivers/perf/arm_pmu.c                    | 383 ++++++++----------------------
 drivers/perf/arm_pmu_acpi.c               | 237 ++++++++++++++++++
 drivers/perf/arm_pmu_platform.c           | 235 ++++++++++++++++++
 include/linux/cpuhotplug.h                |   1 +
 include/linux/perf/arm_pmu.h              |  22 +-
 11 files changed, 626 insertions(+), 349 deletions(-)
 create mode 100644 drivers/perf/arm_pmu_acpi.c
 create mode 100644 drivers/perf/arm_pmu_platform.c
author maintainer
#43 lore raw

cover arm_pmu: ACPI support

Hi Jeremy,

Quoted 6 lines · Jeremy Linton

I've been testing these on a few machines (m400 & dual socket cavium for starters) over the last few days. So far everything seems to be working fine excepting the usual set of strange messages from the perf fuzzer.

Thanks for giving this a go, especially with the fuzzer!

Just to check, should that email be jeremy.linton at arm.com, or is that an alias that also works? I've added it as-is for the moment.

Thanks,
Mark.

#44 lore raw

cover arm_pmu: ACPI support

Quoted 8 lines · Mark Rutland

Hi Jeremy,

Quoted 5 lines · Jeremy Linton

I've been testing these on a few machines (m400 & dual socket cavium for starters) over the last few days. So far everything seems to be working fine excepting the usual set of strange messages from the perf fuzzer.

Quoted 6 lines
Quoted 1 line

Thanks for giving this a go, especially with the fuzzer!

Just to check, should that email be jeremy.linton at arm.com, or is that an alias that also works? I've added it as-is for the moment.

Yes, my fingers tend to out type my brain on a regular basis, particularly when context switching between things.

jeremy.linton at arm.com is correct.

Thanks,