arm-cc*: fix PMU interrupt flags

As discovered during review of the X-Gene SoC PMU [1], the arm-cc{i,n} drivers don't ensure that IRQ balancers don't migrate interrupts.

This is problematic for the perf core code, which requires mutual exclusion of certain operations (e.g. event rotation, cross-calls, and irq handling) to be provided by disabling IRQs, which only works if all operations occur on the same CPU. This is also required for safe (lockless) manipulation of some data structures.

To avoid this problem, we must request interrupts with IRQF_NOBALANCING, as is already the case for CPU PMU drivers which make use of interrupts.

To ensure synchronisation between IRQ handlers and other manipulation of said data structures or HW state, we must also ensure that the interrupt handlers are not threaded, by requesting them with IRQF_NO_THREAD, as is already the case for CPU PMU drivers.

Since v1 [2]:
* Reword commit messages
* Drop IRQF_SHARED from arm-cci

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/439589.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/439739.html

Mark Rutland (2):
arm-ccn: fix PMU interrupt flags
arm-cci: fix PMU interrupt flags

 drivers/bus/arm-cci.c | 3 ++-
 drivers/bus/arm-ccn.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

1.9.1

Currently the IRQ core is permitted to make the CCN PMU IRQ handler threaded, and will allow userspace to change the CPU affinity of the interrupt behind our back. Both of these could violate our synchronisation requirements with the core perf code, which relies upon strict CPU affinity and disabling of interrupts to guarantee mutual exclusion in some cases.

As with the CPU PMU drivers, we should request the interrupt with IRQF_NOBALANCING and IRQF_NO_THREAD, to avoid these issues.

---
 drivers/bus/arm-ccn.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index acc3eb5..2c99ab2 100644
@@ -1479,8 +1479,9 @@ static int arm_ccn_probe(struct platform_device *pdev)
 		/* Can set 'disable' bits, so can acknowledge interrupts */
 		writel(CCN_MN_ERRINT_STATUS__PMU_EVENTS__ENABLE,
 				ccn->base + CCN_MN_ERRINT_STATUS);
-		err = devm_request_irq(ccn->dev, irq, arm_ccn_irq_handler, 0,
-				dev_name(ccn->dev), ccn);
+		err = devm_request_irq(ccn->dev, irq, arm_ccn_irq_handler,
+				       IRQF_NOBALANCING | IRQF_NO_THREAD,
+				       dev_name(ccn->dev), ccn);
 		if (err)
 			return err;
 

1.9.1

Currently the IRQ core is permitted to make the CCI PMU IRQ handler threaded, and will allow userspace to change the CPU affinity of the interrupt behind our back. Both of these could violate our synchronisation requirements with the core perf code, which relies upon strict CPU affinity and disabling of interrupts to guarantee mutual exclusion in some cases.

As with the CPU PMU drivers, we should request the interrupt with IRQF_NOBALANCING and IRQF_NO_THREAD, to avoid these issues. At the same time, we drop IRQF_SHARED. The interrupt is not shared in practice, and the driver detects and skips duplicate IRQs at probe time, so it is not necessary.

---
 drivers/bus/arm-cci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index a49b283..ffce9e3 100644
@@ -881,7 +881,8 @@ static int pmu_request_irq(struct cci_pmu *cci_pmu, irq_handler_t handler)
 	 * This should allow handling of non-unique interrupt for the counters.
 	 */
 	for (i = 0; i < cci_pmu->nr_irqs; i++) {
-		int err = request_irq(cci_pmu->irqs[i], handler, IRQF_SHARED,
+		int err = request_irq(cci_pmu->irqs[i], handler,
+				IRQF_NOBALANCING | IRQF_NO_THREAD,
 				"arm-cci-pmu", cci_pmu);
 		if (err) {
 			dev_err(&pmu_device->dev, "unable to request IRQ%d for ARM CCI PMU counters\n",

1.9.1

Thanks, looks good now:

Will

Cheers!

I take it that as with the other recent CCI patch [1] you expect the patches to go via arm-soc.

Arnd, Olof, are you happy to pick these patches?

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/440916.html