[PATCH v3 0/4] Fix PMU interrupt affinity for Juno*

[PATCH v3 0/4] Fix PMU interrupt affinity for Juno*

[Will Deacon]

Hi all,

This is version three of the patches previously posted here:

  v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/322251.html
  v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/326570.html

Changes since v2 include:

  - Split the binding docs out into a separate patch
  - Fixed failure path when we fail to find the logical CPU for an IRQ
  - Fixed of_node reference leak
  - Tidied up strings in error messages

Feedback welcome,

Will

--->8

Will Deacon (4):
  dt: pmu: extend ARM PMU binding to allow for explicit interrupt
    affinity
  ARM: pmu: add support for interrupt-affinity property
  arm64: pmu: add support for interrupt-affinity property
  arm64: dts: add interrupt-affinity property to pmu node for juno

 Documentation/devicetree/bindings/arm/pmu.txt |  7 +++
 arch/arm/include/asm/pmu.h                    |  1 +
 arch/arm/kernel/perf_event_cpu.c              | 69 ++++++++++++++++++++++++---
 arch/arm64/boot/dts/arm/juno.dts              | 14 ++++--
 arch/arm64/include/asm/pmu.h                  |  1 +
 arch/arm64/kernel/perf_event.c                | 57 ++++++++++++++++++++--
 6 files changed, 134 insertions(+), 15 deletions(-)

-- 
2.1.4

[PATCH v3 1/4] dt: pmu: extend ARM PMU binding to allow for explicit interrupt affinity

[Will Deacon]

The current ARM PMU binding relies on the PMU interrupts being listed in
CPU logical order, which the device-tree author simply cannot know
anything about.

This patch introduces a new "interrupt-affinity" property, which makes
the relationship between the PMU interrupts and their corresponding
CPU explicit.

Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 Documentation/devicetree/bindings/arm/pmu.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt
index 75ef91d08f3b..f52d05660dc9 100644
--- a/Documentation/devicetree/bindings/arm/pmu.txt
+++ b/Documentation/devicetree/bindings/arm/pmu.txt
@@ -24,6 +24,13 @@ Required properties:
 
 Optional properties:
 
+- interrupt-affinity : Valid only when using SPIs, specifies a list of phandles
+                       to CPU nodes corresponding directly to the affinity of
+		       the SPIs listed in the interrupts property.
+
+		       This property should be present when there is more than
+		       a single SPI.
+
 - qcom,no-pc-write : Indicates that this PMU doesn't support the 0xc and 0xd
                      events.
 
-- 
2.1.4

[PATCH v3 2/4] ARM: pmu: add support for interrupt-affinity property

[Will Deacon]

Historically, the PMU devicetree bindings have expected SPIs to be
listed in order of *logical* CPU number. This is problematic for
bootloaders, especially when the boot CPU (logical ID 0) isn't listed
first in the devicetree.

This patch adds a new optional property, interrupt-affinity, to the
PMU node which allows the interrupt affinity to be described using
a list of phandled to CPU nodes, with each entry in the list
corresponding to the SPI at the same index in the interrupts property.

Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/pmu.h       |  1 +
 arch/arm/kernel/perf_event_cpu.c | 69 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index b1596bd59129..675e4ab79f68 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h
@@ -92,6 +92,7 @@ struct pmu_hw_events {
 struct arm_pmu {
 	struct pmu	pmu;
 	cpumask_t	active_irqs;
+	int		*irq_affinity;
 	char		*name;
 	irqreturn_t	(*handle_irq)(int irq_num, void *dev);
 	void		(*enable)(struct perf_event *event);
diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index 61b53c46edfa..51ad167983e5 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -92,11 +92,16 @@ static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu)
 		free_percpu_irq(irq, &hw_events->percpu_pmu);
 	} else {
 		for (i = 0; i < irqs; ++i) {
-			if (!cpumask_test_and_clear_cpu(i, &cpu_pmu->active_irqs))
+			int cpu = i;
+
+			if (cpu_pmu->irq_affinity)
+				cpu = cpu_pmu->irq_affinity[i];
+
+			if (!cpumask_test_and_clear_cpu(cpu, &cpu_pmu->active_irqs))
 				continue;
 			irq = platform_get_irq(pmu_device, i);
 			if (irq >= 0)
-				free_irq(irq, per_cpu_ptr(&hw_events->percpu_pmu, i));
+				free_irq(irq, per_cpu_ptr(&hw_events->percpu_pmu, cpu));
 		}
 	}
 }
@@ -128,32 +133,37 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 		on_each_cpu(cpu_pmu_enable_percpu_irq, &irq, 1);
 	} else {
 		for (i = 0; i < irqs; ++i) {
+			int cpu = i;
+
 			err = 0;
 			irq = platform_get_irq(pmu_device, i);
 			if (irq < 0)
 				continue;
 
+			if (cpu_pmu->irq_affinity)
+				cpu = cpu_pmu->irq_affinity[i];
+
 			/*
 			 * If we have a single PMU interrupt that we can't shift,
 			 * assume that we're running on a uniprocessor machine and
 			 * continue. Otherwise, continue without this interrupt.
 			 */
-			if (irq_set_affinity(irq, cpumask_of(i)) && irqs > 1) {
+			if (irq_set_affinity(irq, cpumask_of(cpu)) && irqs > 1) {
 				pr_warn("unable to set irq affinity (irq=%d, cpu=%u)\n",
-					irq, i);
+					irq, cpu);
 				continue;
 			}
 
 			err = request_irq(irq, handler,
 					  IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu",
-					  per_cpu_ptr(&hw_events->percpu_pmu, i));
+					  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(i, &cpu_pmu->active_irqs);
+			cpumask_set_cpu(cpu, &cpu_pmu->active_irqs);
 		}
 	}
 
@@ -289,6 +299,48 @@ static int probe_current_pmu(struct arm_pmu *pmu)
 	return ret;
 }
 
+static int of_pmu_irq_cfg(struct platform_device *pdev)
+{
+	int i;
+	int *irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
+
+	if (!irqs)
+		return -ENOMEM;
+
+	for (i = 0; i < pdev->num_resources; ++i) {
+		struct device_node *dn;
+		int cpu;
+
+		dn = of_parse_phandle(pdev->dev.of_node, "interrupt-affinity",
+				      i);
+		if (!dn) {
+			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
+				of_node_full_name(dn), i);
+			break;
+		}
+
+		for_each_possible_cpu(cpu)
+			if (arch_find_n_match_cpu_physical_id(dn, cpu, NULL))
+				break;
+
+		of_node_put(dn);
+		if (cpu >= nr_cpu_ids) {
+			pr_warn("Failed to find logical CPU for %s\n",
+				dn->name);
+			break;
+		}
+
+		irqs[i] = cpu;
+	}
+
+	if (i == pdev->num_resources)
+		cpu_pmu->irq_affinity = irqs;
+	else
+		kfree(irqs);
+
+	return 0;
+}
+
 static int cpu_pmu_device_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id;
@@ -313,7 +365,10 @@ static int cpu_pmu_device_probe(struct platform_device *pdev)
 
 	if (node && (of_id = of_match_node(cpu_pmu_of_device_ids, pdev->dev.of_node))) {
 		init_fn = of_id->data;
-		ret = init_fn(pmu);
+
+		ret = of_pmu_irq_cfg(pdev);
+		if (!ret)
+			ret = init_fn(pmu);
 	} else {
 		ret = probe_current_pmu(pmu);
 	}
-- 
2.1.4

[PATCH v3 3/4] arm64: pmu: add support for interrupt-affinity property

[Will Deacon]

Historically, the PMU devicetree bindings have expected SPIs to be
listed in order of *logical* CPU number. This is problematic for
bootloaders, especially when the boot CPU (logical ID 0) isn't listed
first in the devicetree.

This patch adds a new optional property, interrupt-affinity, to the
PMU node which allows the interrupt affinity to be described using
a list of phandled to CPU nodes, with each entry in the list
corresponding to the SPI at the same index in the interrupts property.

Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/pmu.h   |  1 +
 arch/arm64/kernel/perf_event.c | 57 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/pmu.h b/arch/arm64/include/asm/pmu.h
index e6f087806aaf..b7710a59672c 100644
--- a/arch/arm64/include/asm/pmu.h
+++ b/arch/arm64/include/asm/pmu.h
@@ -44,6 +44,7 @@ struct pmu_hw_events {
 struct arm_pmu {
 	struct pmu		pmu;
 	cpumask_t		active_irqs;
+	int			*irq_affinity;
 	const char		*name;
 	irqreturn_t		(*handle_irq)(int irq_num, void *dev);
 	void			(*enable)(struct hw_perf_event *evt, int idx);
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 25a5308744b1..0173fb5e0fd9 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -25,8 +25,10 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
+#include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
+#include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/uaccess.h>
 
@@ -396,7 +398,12 @@ armpmu_release_hardware(struct arm_pmu *armpmu)
 		free_percpu_irq(irq, &cpu_hw_events);
 	} else {
 		for (i = 0; i < irqs; ++i) {
-			if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
+			int cpu = i;
+
+			if (armpmu->irq_affinity)
+				cpu = armpmu->irq_affinity[i];
+
+			if (!cpumask_test_and_clear_cpu(cpu, &armpmu->active_irqs))
 				continue;
 			irq = platform_get_irq(pmu_device, i);
 			if (irq > 0)
@@ -450,19 +457,24 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
 		on_each_cpu(armpmu_enable_percpu_irq, &irq, 1);
 	} else {
 		for (i = 0; i < irqs; ++i) {
+			int cpu = i;
+
 			err = 0;
 			irq = platform_get_irq(pmu_device, i);
 			if (irq <= 0)
 				continue;
 
+			if (armpmu->irq_affinity)
+				cpu = armpmu->irq_affinity[i];
+
 			/*
 			 * If we have a single PMU interrupt that we can't shift,
 			 * assume that we're running on a uniprocessor machine and
 			 * continue. Otherwise, continue without this interrupt.
 			 */
-			if (irq_set_affinity(irq, cpumask_of(i)) && irqs > 1) {
+			if (irq_set_affinity(irq, cpumask_of(cpu)) && irqs > 1) {
 				pr_warning("unable to set irq affinity (irq=%d, cpu=%u)\n",
-						irq, i);
+						irq, cpu);
 				continue;
 			}
 
@@ -476,7 +488,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
 				return err;
 			}
 
-			cpumask_set_cpu(i, &armpmu->active_irqs);
+			cpumask_set_cpu(cpu, &armpmu->active_irqs);
 		}
 	}
 
@@ -1289,9 +1301,46 @@ static const struct of_device_id armpmu_of_device_ids[] = {
 
 static int armpmu_device_probe(struct platform_device *pdev)
 {
+	int i, *irqs;
+
 	if (!cpu_pmu)
 		return -ENODEV;
 
+	irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
+	if (!irqs)
+		return -ENOMEM;
+
+	for (i = 0; i < pdev->num_resources; ++i) {
+		struct device_node *dn;
+		int cpu;
+
+		dn = of_parse_phandle(pdev->dev.of_node, "interrupt-affinity",
+				      i);
+		if (!dn) {
+			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
+				of_node_full_name(dn), i);
+			break;
+		}
+
+		for_each_possible_cpu(cpu)
+			if (arch_find_n_match_cpu_physical_id(dn, cpu, NULL))
+				break;
+
+		of_node_put(dn);
+		if (cpu >= nr_cpu_ids) {
+			pr_warn("Failed to find logical CPU for %s\n",
+				dn->name);
+			break;
+		}
+
+		irqs[i] = cpu;
+	}
+
+	if (i == pdev->num_resources)
+		cpu_pmu->irq_affinity = irqs;
+	else
+		kfree(irqs);
+
 	cpu_pmu->plat_device = pdev;
 	return 0;
 }
-- 
2.1.4

[PATCH v3 4/4] arm64: dts: add interrupt-affinity property to pmu node for juno

[Will Deacon]

Make the Juno .dts robust against potential reordering of the CPU nodes
by adding an explicit interrupt-affinity property to the PMU node. While
we're at it, fix the PMU interrupts numbers too.

Cc: Mark Rutland <mark.rutland@arm.com>
Acked-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/boot/dts/arm/juno.dts | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index 133ee59de2d7..5e9110a3353d 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -120,12 +120,18 @@
 
 	pmu {
 		compatible = "arm,armv8-pmuv3";
-		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
+		interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
-			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
+			     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-affinity = <&A57_0>,
+				     <&A57_1>,
+				     <&A53_0>,
+				     <&A53_1>,
+				     <&A53_2>,
+				     <&A53_3>;
 	};
 
 	/include/ "juno-clocks.dtsi"
-- 
2.1.4

[PATCH v3 4/4] arm64: dts: add interrupt-affinity property to pmu node for juno

[Will Deacon]

[adding arm-soc folks]

Hi Arnd, Olof,

Could you take this Juno .dts patch via arm-soc for 4.1 please? I'll queue
the perf code to make use of the new binding, but there's not a strict
dependency (the interrupt-affinity property will be ignored by older
kernels).

Cheers,

Will

On Fri, Mar 06, 2015 at 11:54:11AM +0000, Will Deacon wrote:
> Make the Juno .dts robust against potential reordering of the CPU nodes
> by adding an explicit interrupt-affinity property to the PMU node. While
> we're at it, fix the PMU interrupts numbers too.
> 
> Cc: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/boot/dts/arm/juno.dts | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
> index 133ee59de2d7..5e9110a3353d 100644
> --- a/arch/arm64/boot/dts/arm/juno.dts
> +++ b/arch/arm64/boot/dts/arm/juno.dts
> @@ -120,12 +120,18 @@
>  
>  	pmu {
>  		compatible = "arm,armv8-pmuv3";
> -		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> +		interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
>  			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
>  			     <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
> -			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
> +			     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-affinity = <&A57_0>,
> +				     <&A57_1>,
> +				     <&A53_0>,
> +				     <&A53_1>,
> +				     <&A53_2>,
> +				     <&A53_3>;
>  	};
>  
>  	/include/ "juno-clocks.dtsi"
> -- 
> 2.1.4
> 

[PATCH v3 4/4] arm64: dts: add interrupt-affinity property to pmu node for juno

[Will Deacon]

On Tue, Mar 24, 2015 at 03:05:28PM +0000, Will Deacon wrote:
> [adding arm-soc folks]
> 
> Hi Arnd, Olof,
> 
> Could you take this Juno .dts patch via arm-soc for 4.1 please? I'll queue
> the perf code to make use of the new binding, but there's not a strict
> dependency (the interrupt-affinity property will be ignored by older
> kernels).

Gentle ping, although I guess most people were tied up with ELC/ACPI this
week.

Happy to resend the patch if necessary.

Will

> On Fri, Mar 06, 2015 at 11:54:11AM +0000, Will Deacon wrote:
> > Make the Juno .dts robust against potential reordering of the CPU nodes
> > by adding an explicit interrupt-affinity property to the PMU node. While
> > we're at it, fix the PMU interrupts numbers too.
> > 
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Liviu Dudau <liviu.dudau@arm.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/boot/dts/arm/juno.dts | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
> > index 133ee59de2d7..5e9110a3353d 100644
> > --- a/arch/arm64/boot/dts/arm/juno.dts
> > +++ b/arch/arm64/boot/dts/arm/juno.dts
> > @@ -120,12 +120,18 @@
> >  
> >  	pmu {
> >  		compatible = "arm,armv8-pmuv3";
> > -		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > +		interrupts = <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> >  			     <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
> > -			     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
> > -			     <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
> > -			     <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
> > +			     <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-affinity = <&A57_0>,
> > +				     <&A57_1>,
> > +				     <&A53_0>,
> > +				     <&A53_1>,
> > +				     <&A53_2>,
> > +				     <&A53_3>;
> >  	};
> >  
> >  	/include/ "juno-clocks.dtsi"
> > -- 
> > 2.1.4
> > 

[PATCH v3 4/4] arm64: dts: add interrupt-affinity property to pmu node for juno

[Olof Johansson]

On Fri, Mar 27, 2015 at 05:33:19PM +0000, Will Deacon wrote:
> On Tue, Mar 24, 2015 at 03:05:28PM +0000, Will Deacon wrote:
> > [adding arm-soc folks]
> > 
> > Hi Arnd, Olof,
> > 
> > Could you take this Juno .dts patch via arm-soc for 4.1 please? I'll queue
> > the perf code to make use of the new binding, but there's not a strict
> > dependency (the interrupt-affinity property will be ignored by older
> > kernels).
> 
> Gentle ping, although I guess most people were tied up with ELC/ACPI this
> week.
> 
> Happy to resend the patch if necessary.

Yeah, please resend. Getting just a comment on the thread to
arm at kernel.org works to get our attention, but it's awkward to apply
patches that way. Resend and I'll be happy to apply.


-Olof