arm64: pmu: add support for interrupt-affinity property
Subsystems (6)
PATCH v2 1/3 arm64: pmu: add support for interrupt-affinity property
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.
--- Documentation/devicetree/bindings/arm/pmu.txt | 6 +++ arch/arm64/include/asm/pmu.h | 1 + arch/arm64/kernel/perf_event.c | 57 +++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/arm/pmu.txt +6 -0
@@ -24,6 +24,12 @@ 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. If absent, + the interrupts are assumed to be listed in logical CPU + order. + - qcom,no-pc-write : Indicates that this PMU doesn't support the 0xc and 0xd events.
arch/arm64/include/asm/pmu.h +1 -0
@@ -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);
arch/arm64/kernel/perf_event.c +53 -4
103 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH v2 2/3 ARM: pmu: add support for interrupt-affinity property
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.
--- arch/arm/include/asm/pmu.h | 1 + arch/arm/kernel/perf_event_cpu.c | 69 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 63 insertions(+), 7 deletions(-)
arch/arm/include/asm/pmu.h +1 -0
@@ -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);
arch/arm/kernel/perf_event_cpu.c +62 -7
118 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH v2 3/3 arm64: dts: add interrupt-affinity property to pmu node for juno
In reply to Will DeaconMake 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.
--- arch/arm64/boot/dts/arm/juno.dts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/arm/juno.dts +10 -4
@@ -106,12 +106,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"
Signature · 1 line
2.1.4
Liviu Dudau
maintainerPATCH v2 3/3 arm64: dts: add interrupt-affinity property to pmu node for juno
Quoted 3 lines · 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.
Thanks for spotting this. I can't remember where the PMU numbers originated from as my GitHub version has a different set as well.
Quoted 37 lines
Cc Mark Rutland <[email protected]>Signed-off-by Will Deacon <[email protected]>--- arch/arm64/boot/dts/arm/juno.dts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)arch/arm64/boot/dts/arm/juno.dts +10
-4@@ -106,12 +106,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"Signature · 1 line
2.1.4
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
?\_(?)_/?Mark Rutland
maintainerPATCH v2 1/3 arm64: pmu: add support for interrupt-affinity property
In reply to Will DeaconHi Will,
Quoted 10 lines · 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.
Quoted 7 lines
Cc Mark Rutland <[email protected]>Signed-off-by Will Deacon <[email protected]>--- Documentation/devicetree/bindings/arm/pmu.txt | 6 +++ arch/arm64/include/asm/pmu.h | 1 + arch/arm64/kernel/perf_event.c | 57 +++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 4 deletions(-)
Please split the DT portion out from the arm64 code; it's equally applicable to the arm and arm64 patches.
Quoted 6 lines
Documentation/devicetree/bindings/arm/pmu.txt +0
-0
Documentation/devicetree/bindings/arm/pmu.txt Required properties: · line 31
Quoted 8 lines
Documentation/devicetree/bindings/arm/pmu.txt +5
-0@@ -24,6 +24,12 @@ 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. If absent, + the interrupts are assumed to be listed in logical CPU + order.
I would prefer if this were:
----
- 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.
----
That makes it clear that new DTs should be using the property. It also gets rid of the mention of the logical CPU order, which DT authors simply cannot know anything about.
There are cases I can think of that this doesn't cover:
* A single muxed SPI
* Muxed SPIs per-cluster
* Differing PPIs per-cluster
I can see how we can cover the first quite easily with a simple binding extension, but I'm having difficulty with the last two. If they're unlikely to appear in practice, then we are fine, but it's not clear to me if that's the case.
If you think those are unlikely to appear in practice, then I think this binding is sufficient. So, for the binding split out into a seaprate patch, with those changes:
[...]
Now, for the code...
[...]
Documentation/devicetree/bindings/arm/pmu.txt Required properties:
Quoted 7 lines
Documentation/devicetree/bindings/arm/pmu.txt +2
-0static int armpmu_device_probe(struct platform_device *pdev) { + int i, *irqs; + if (!cpu_pmu) return -ENODEV;
We should pull the affinity parsing into a function that we only bother to call if there's an interrupt-affinity property present. That will make it possible to error out if there is a property but it is malformed.
Documentation/devicetree/bindings/arm/pmu.txt
Quoted 13 lines
Documentation/devicetree/bindings/arm/pmu.txt +13
-0+ 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 = -1; + + dn = of_parse_phandle(pdev->dev.of_node, "interrupt-affinity", + i); + if (!dn) { + pr_warn("Failed to parse interrupt-affinity for idx %d\n", + i);
This would be nicer with the node name:
pr_warn("Failed to parse %s/interrupt-affinity[%d]",
of_node_full_name(dn), i);Documentation/devicetree/bindings/arm/pmu.txt
Quoted 12 lines
Documentation/devicetree/bindings/arm/pmu.txt +12
-0+ break; + } + + for_each_possible_cpu(cpu) + if (arch_find_n_match_cpu_physical_id(dn, cpu, NULL)) + break; + + if (cpu == -1) { + pr_warn("Failed to find logical CPU for %s\n", + dn->name); + break; + }
Refcount leak on dn here. You can move the put immediately after the for_each_possible_cpu.
Documentation/devicetree/bindings/arm/pmu.txt
Quoted 4 lines
Documentation/devicetree/bindings/arm/pmu.txt +4
-0+ + irqs[i] = cpu; + of_node_put(dn); + }
Thanks,
Mark.