[PATCH] RFC: ux500: add PMU resources

[PATCH] RFC: ux500: add PMU resources

[Linus Walleij]

The ux500 SoCs have PMUs, both DB8500 and DB5500. However on the
DB8500 the individual per-core IRQs are not routed: instead they
are OR:ed into one single IRQ.

Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
Hi Will especially, I remember discussing this issue with you
back in Florida. IIRC you had some idea on how to go about with
this, like we add some tweak for this board where we don't set
affinity and let the IRQ be declared chained and then each CPU
it's not coming from has to NACK it, do you think it is
feasible and could you sort of point me into the right files
to poke at?
---
 arch/arm/mach-ux500/cpu-db5500.c |   26 ++++++++++++++++++++++++--
 arch/arm/mach-ux500/cpu-db8500.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db5500.c b/arch/arm/mach-ux500/cpu-db5500.c
index af04e08..3912354 100644
--- a/arch/arm/mach-ux500/cpu-db5500.c
+++ b/arch/arm/mach-ux500/cpu-db5500.c
@@ -11,6 +11,7 @@
 #include <linux/irq.h>
 
 #include <asm/mach/map.h>
+#include <asm/pmu.h>
 
 #include <plat/gpio.h>
 
@@ -43,6 +44,26 @@ static struct map_desc u5500_io_desc[] __initdata = {
 	__IO_DEV_DESC(U5500_PRCMU_BASE, SZ_4K),
 };
 
+static struct resource db5500_pmu_resources[] = {
+	[0] = {
+		.start		= IRQ_DB5500_PMU0,
+		.end		= IRQ_DB5500_PMU0,
+		.flags		= IORESOURCE_IRQ,
+	},
+	[1] = {
+		.start		= IRQ_DB5500_PMU1,
+		.end		= IRQ_DB5500_PMU1,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device db5500_pmu_device = {
+	.name			= "arm-pmu",
+	.id			= ARM_PMU_DEVICE_CPU,
+	.num_resources		= ARRAY_SIZE(db5500_pmu_resources),
+	.resource		= db5500_pmu_resources,
+};
+
 static struct resource mbox0_resources[] = {
 	{
 		.name = "mbox_peer",
@@ -127,7 +148,8 @@ static struct platform_device mbox2_device = {
 	.num_resources = ARRAY_SIZE(mbox2_resources),
 };
 
-static struct platform_device *u5500_platform_devs[] __initdata = {
+static struct platform_device *db5500_platform_devs[] __initdata = {
+	&db5500_pmu_device,
 	&mbox0_device,
 	&mbox1_device,
 	&mbox2_device,
@@ -172,6 +194,6 @@ void __init u5500_init_devices(void)
 	db5500_dma_init();
 	db5500_add_rtc();
 
-	platform_add_devices(u5500_platform_devs,
+	platform_add_devices(db5500_platform_devs,
 			     ARRAY_SIZE(u5500_platform_devs));
 }
diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index 1748fbc..a49641c 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 
 #include <asm/mach/map.h>
+#include <asm/pmu.h>
 #include <mach/hardware.h>
 #include <mach/setup.h>
 #include <mach/devices.h>
@@ -112,6 +113,31 @@ static void __init db8500_add_gpios(void)
 }
 
 /*
+ * The DB8500 PMU has only one IRQ line which is the OR:ed result
+ * from both CPU cores. This means that currently kernel measurements
+ * such as done with perf can onlt be done by locking the subject
+ * task to CPU0.
+ */
+static struct resource db8500_pmu_resources[] = {
+	[0] = {
+		.start		= IRQ_DB8500_PMU,
+		.end		= IRQ_DB8500_PMU,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device db8500_pmu_device = {
+	.name			= "arm-pmu",
+	.id			= ARM_PMU_DEVICE_CPU,
+	.num_resources		= ARRAY_SIZE(db8500_pmu_resources),
+	.resource		= db8500_pmu_resources,
+};
+
+static struct platform_device *db8500_platform_devs[] __initdata = {
+	&db8500_pmu_device,
+};
+
+/*
  * This function is called from the board init
  */
 void __init u8500_init_devices(void)
@@ -121,6 +147,8 @@ void __init u8500_init_devices(void)
 
 	db8500_add_rtc();
 	db8500_add_gpios();
+	platform_add_devices(db8500_platform_devs,
+			     ARRAY_SIZE(db8500_platform_devs));
 
 	platform_device_register_simple("cpufreq-u8500", -1, NULL, 0);
 	platform_add_devices(platform_devs, ARRAY_SIZE(platform_devs));
-- 
1.7.3.2

[PATCH] RFC: ux500: add PMU resources

[Will Deacon]

Hi Linus,

> The ux500 SoCs have PMUs, both DB8500 and DB5500. However on the
> DB8500 the individual per-core IRQs are not routed: instead they
> are OR:ed into one single IRQ.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
> Hi Will especially, I remember discussing this issue with you
> back in Florida. IIRC you had some idea on how to go about with
> this, like we add some tweak for this board where we don't set
> affinity and let the IRQ be declared chained and then each CPU
> it's not coming from has to NACK it, do you think it is
> feasible and could you sort of point me into the right files
> to poke at?

Ah yes, I have some hazy memories of this conversation! I think you
have three options:

1.) Don't use the PMU interrupt and instead use a timer which is
    guaranteed to fire before we overflow. sh has this restriction
    because its PMU doesn't have an IRQ line at all. There's a comment 
    in arch/sh/kernel/perf_event.c:

	/*
	 * All of the on-chip counters are "limited", in that they have
	 * no interrupts, and are therefore unable to do sampling without
	 * further work and timer assistance.
	 */
	if (hwc->sample_period)
		return -EINVAL;
     
    
    but it might be worth asking if they have any preliminary code for
    this.

2.) Use IPI to cross-call the IRQ handler when the interrupted CPU doesn't
    have an interrupt asserted on its PMU. You'll need to sort out problems
    with re-enabling interrupts for the cross-call and also nesting of the
    PMU interrupt (because it could fire as a result of another PMU overflowing
    before you've completed the cross-call).

3.) Rework the GIC code so that an IRQ can target multiple CPUs and remove the
    distributor-level masking. I think this was originally done so that we can
    service different IRQs simultaneously, but with the deprecation of IRQF_DISABLED
    I'm not sure if this is still an issue. If not, then we can change to masking
    at the CPU interfaces which will make supporting your combined IRQ much easier.
    You'll need to talk to Russell about this as he has a better idea about the
    current approach that Linux takes than I do.

>  arch/arm/mach-ux500/cpu-db5500.c |   26 ++++++++++++++++++++++++--
>  arch/arm/mach-ux500/cpu-db8500.c |   28 ++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-ux500/cpu-db5500.c b/arch/arm/mach-ux500/cpu-db5500.c
> index af04e08..3912354 100644
> --- a/arch/arm/mach-ux500/cpu-db5500.c
> +++ b/arch/arm/mach-ux500/cpu-db5500.c
> @@ -11,6 +11,7 @@
>  #include <linux/irq.h>
> 
>  #include <asm/mach/map.h>
> +#include <asm/pmu.h>
> 
>  #include <plat/gpio.h>
> 
> @@ -43,6 +44,26 @@ static struct map_desc u5500_io_desc[] __initdata = {
>  	__IO_DEV_DESC(U5500_PRCMU_BASE, SZ_4K),
>  };
> 
> +static struct resource db5500_pmu_resources[] = {
> +	[0] = {
> +		.start		= IRQ_DB5500_PMU0,
> +		.end		= IRQ_DB5500_PMU0,
> +		.flags		= IORESOURCE_IRQ,
> +	},
> +	[1] = {
> +		.start		= IRQ_DB5500_PMU1,
> +		.end		= IRQ_DB5500_PMU1,
> +		.flags		= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device db5500_pmu_device = {
> +	.name			= "arm-pmu",
> +	.id			= ARM_PMU_DEVICE_CPU,
> +	.num_resources		= ARRAY_SIZE(db5500_pmu_resources),
> +	.resource		= db5500_pmu_resources,
> +};
> +
>  static struct resource mbox0_resources[] = {
>  	{
>  		.name = "mbox_peer",
> @@ -127,7 +148,8 @@ static struct platform_device mbox2_device = {
>  	.num_resources = ARRAY_SIZE(mbox2_resources),
>  };
> 
> -static struct platform_device *u5500_platform_devs[] __initdata = {
> +static struct platform_device *db5500_platform_devs[] __initdata = {
> +	&db5500_pmu_device,
>  	&mbox0_device,
>  	&mbox1_device,
>  	&mbox2_device,
> @@ -172,6 +194,6 @@ void __init u5500_init_devices(void)
>  	db5500_dma_init();
>  	db5500_add_rtc();
> 
> -	platform_add_devices(u5500_platform_devs,
> +	platform_add_devices(db5500_platform_devs,
>  			     ARRAY_SIZE(u5500_platform_devs));
>  }

For the db5500 stuff:

Acked-by: Will Deacon <will.deacon@arm.com>

The db8500 code obviously won't work yet!

Will

[PATCH] RFC: ux500: add PMU resources

[Russell King - ARM Linux]

On Wed, Jan 19, 2011 at 11:39:09AM -0000, Will Deacon wrote:
> 3.) Rework the GIC code so that an IRQ can target multiple CPUs and remove
>     the distributor-level masking. I think this was originally done so that
>     we can service different IRQs simultaneously, but with the deprecation
>     of IRQF_DISABLED I'm not sure if this is still an issue. If not, then
>     we can change to masking at the CPU interfaces which will make
>     supporting your combined IRQ much easier.

If an interrupt is routed to multiple cores simultaneously, then you end
up with a number of problems:

1. Each time an interrupt occurs, you wake up all CPUs.
2. One CPU wins the race and starts to handle the interrupt.  The others
   are left spinning on a lock waiting.  Eventually the lock is dropped
   and they too enter the handler.
3. Another race ensues on the drivers own spinlock.  The winning CPU
   possibly holds this lock for the duration of its handling.  Meanwhile
   the other CPUs are left spinning waiting for the lock to be dropped.
4. When the winning CPU drops the lock, it returns IRQ_HANDLED.  The other
   CPUs find that there is no IRQ pending from the device, and return
   IRQ_NONE.

(4) may result in the spurious/unhandled IRQ code eventually disabling
the interrupt.

TI also raised a point that knocking the losing CPUs out of low power
mode will waste power needlessly.

Not only that, but the losing CPUs waste time and power spinning.  So
this really isn't a good idea.

[PATCH] RFC: ux500: add PMU resources

[Will Deacon]

Hi Russell,

Thanks for the insight.

> On Wed, Jan 19, 2011 at 11:39:09AM -0000, Will Deacon wrote:
> > 3.) Rework the GIC code so that an IRQ can target multiple CPUs and remove
> >     the distributor-level masking. I think this was originally done so that
> >     we can service different IRQs simultaneously, but with the deprecation
> >     of IRQF_DISABLED I'm not sure if this is still an issue. If not, then
> >     we can change to masking at the CPU interfaces which will make
> >     supporting your combined IRQ much easier.
> 
> If an interrupt is routed to multiple cores simultaneously, then you end
> up with a number of problems:
> 
> 1. Each time an interrupt occurs, you wake up all CPUs.
> 2. One CPU wins the race and starts to handle the interrupt.  The others
>    are left spinning on a lock waiting.  Eventually the lock is dropped
>    and they too enter the handler.
> 3. Another race ensues on the drivers own spinlock.  The winning CPU
>    possibly holds this lock for the duration of its handling.  Meanwhile
>    the other CPUs are left spinning waiting for the lock to be dropped.
> 4. When the winning CPU drops the lock, it returns IRQ_HANDLED.  The other
>    CPUs find that there is no IRQ pending from the device, and return
>    IRQ_NONE.
> 
> (4) may result in the spurious/unhandled IRQ code eventually disabling
> the interrupt.

I don't think these are a problem if we only allow IRQs to be affine to
multiple CPUs when the IRQF_PERCPU flag is set. All other interrupts will
concern only a single CPU and the corresponding CPU interface.

For these PERCPU interrupts, the wakeups and lock contention is part of the
deal - that's a consequence of setting an affinity mask containing multiple
CPUs.

It's worth noting that the Virtualisation Extensions (Cortex-A15) only provide
a virtualised view of the CPU interfaces. The distributor must be trapped using
a second-stage translation by the hypervisor, so accessing it becomes a massive
overhead in the critical interrupt path in Linux.

Will

[PATCH] RFC: ux500: add PMU resources

[Lee Jones]

It sounds Will's timer idea would be the ideal solution in this case. As Will has previously mentioned, this would also help other venders who are experiencing buggy/non-existent PMU IRQs. I'd be happy to attempt coding this up. Although, to limit wastage of valuable time, I'd like to have the idea sanctioned by Russel et al prior to doing so.

[PATCH] RFC: ux500: add PMU resources

[Rabin Vincent]

On Wed, Jan 19, 2011 at 17:09, Will Deacon <will.deacon@arm.com> wrote:
> Ah yes, I have some hazy memories of this conversation! I think you
> have three options:

Here's the implementation of a different approach which bounces the
IRQ to the other CPU by setting the affinity when the current CPU
would return IRQ_NONE.

It has the advantage over the IPI method that Core0 does not need
to spin (in the interrupt handler) waiting for Core1 to handle
the IPI:

(patch attached as well, since it will probably be mangled)

8<------------

[PATCH] RFC: ux500: add PMU resources

[Rabin Vincent]

On Wed, Jan 19, 2011 at 17:09, Will Deacon <will.deacon@arm.com> wrote:
> 2.) Use IPI to cross-call the IRQ handler when the interrupted CPU doesn't
> ? ?have an interrupt asserted on its PMU. You'll need to sort out problems
> ? ?with re-enabling interrupts for the cross-call and also nesting of the
> ? ?PMU interrupt (because it could fire as a result of another PMU overflowing
> ? ?before you've completed the cross-call).

Here's the IPI version as well, for comparison with the other
approach.  The IRQ handling code will mask the interrupt before
handling it, so it can't nest, can it?

8<----------

[PATCH] RFC: ux500: add PMU resources

[Lee Jones]

Linus and I have already added our SoBs.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>

On 07/02/11 05:56, Rabin Vincent wrote:
> On Wed, Jan 19, 2011 at 17:09, Will Deacon <will.deacon@arm.com> wrote:
>> 2.) Use IPI to cross-call the IRQ handler when the interrupted CPU doesn't
>>    have an interrupt asserted on its PMU. You'll need to sort out problems
>>    with re-enabling interrupts for the cross-call and also nesting of the
>>    PMU interrupt (because it could fire as a result of another PMU overflowing
>>    before you've completed the cross-call).
> Here's the IPI version as well, for comparison with the other
> approach.  The IRQ handling code will mask the interrupt before
> handling it, so it can't nest, can it?
>
> 8<----------
> From 84e5936f9285fb03845ac04801430762c1c9b60d Mon Sep 17 00:00:00 2001
> From: Rabin Vincent <rabin.vincent@stericsson.com>
> Date: Fri, 4 Feb 2011 11:59:26 +0530
> Subject: [PATCH] ARM: perf_event: support dual-core with single PMU IRQ
>
> DB8500 (dual-core) has the PMU interrupts of both cores ORed into one.  Support
> this by keeping the interrupt directed at Core0 and IPIing Core1 when Core0
> receives the interrupt and finds that its counter has not overflowed.
>
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> ---
>  arch/arm/kernel/perf_event.c |   48 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 47 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index 5efa264..a97e50f 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -19,6 +19,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/spinlock.h>
>  #include <linux/uaccess.h>
> +#include <linux/smp.h>
>
>  #include <asm/cputype.h>
>  #include <asm/irq.h>
> @@ -377,9 +378,40 @@ validate_group(struct perf_event *event)
>  	return 0;
>  }
>
> +static irqreturn_t armpmu_core2_irqret;
> +
> +/* Called via IPI on the second core */
> +static void armpmu_kick(void *data)
> +{
> +	int irq = (int) data;
> +
> +	armpmu_core2_irqret = armpmu->handle_irq(irq, NULL);
> +	smp_wmb();
> +}
> +
> +static irqreturn_t armpmu_single_interrupt(int irq, void *dev)
> +{
> +	irqreturn_t irqret = armpmu->handle_irq(irq, dev);
> +	int err;
> +
> +	if (irqret != IRQ_NONE)
> +		return irqret;
> +
> +	local_irq_enable();
> +	err = smp_call_function_single(1, armpmu_kick, (void *) irq, true);
> +	local_irq_disable();
> +
> +	if (err)
> +		return irqret;
> +
> +	smp_rmb();
> +	return armpmu_core2_irqret;
> +}
> +
>  static int
>  armpmu_reserve_hardware(void)
>  {
> +	irq_handler_t irq_handler = armpmu->handle_irq;
>  	int i, err = -ENODEV, irq;
>
>  	pmu_device = reserve_pmu(ARM_PMU_DEVICE_CPU);
> @@ -395,12 +427,26 @@ armpmu_reserve_hardware(void)
>  		return -ENODEV;
>  	}
>
> +	/*
> +	 * Some SoCs have the PMU IRQ lines of two cores wired together into a
> +	 * single interrupt.  Support these by poking the second core with an
> +	 * IPI when its counters overflow.
> +	 */
> +	if (pmu_device->num_resources < num_online_cpus()) {
> +		if (num_online_cpus() > 2) {
> +			pr_err(">2 cpus not supported for single-irq workaround");
> +			return -ENODEV;
> +		}
> +
> +		irq_handler = armpmu_single_interrupt;
> +	}
> +
>  	for (i = 0; i < pmu_device->num_resources; ++i) {
>  		irq = platform_get_irq(pmu_device, i);
>  		if (irq < 0)
>  			continue;
>
> -		err = request_irq(irq, armpmu->handle_irq,
> +		err = request_irq(irq, irq_handler,
>  				  IRQF_DISABLED | IRQF_NOBALANCING,
>  				  "armpmu", NULL);
>  		if (err) {

[PATCH] RFC: ux500: add PMU resources

[Linus Walleij]

On 02/07/2011 08:55 AM, Lee Jones wrote:
> Linus and I have already added our SoBs.
>
> Signed-off-by: Lee Jones<lee.jones@linaro.org>
> Signed-off-by: Linus Walleij<linus.walleij@stericsson.com>
>    

SoB is for the delivery path, I assume you mean 
s/Signed-off-by/Acked-by/g...

Yours,
Linus Walleij

[PATCH] RFC: ux500: add PMU resources

[Will Deacon]

Hi Rabin,

Thanks for looking at this.

> Here's the implementation of a different approach which bounces the
> IRQ to the other CPU by setting the affinity when the current CPU
> would return IRQ_NONE.

I considered this, but I think it has some issues.

> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index 5efa264..d07990e 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -377,9 +377,27 @@ validate_group(struct perf_event *event)
>  	return 0;
>  }
> 
> +static irqreturn_t armpmu_bounce_interrupt(int irq, void *dev)
> +{
> +	irqreturn_t ret = armpmu->handle_irq(irq, dev);
> +
> +	if (ret == IRQ_NONE) {
> +		int other = !smp_processor_id();
> +		irq_set_affinity(irq, cpumask_of(other));
> +	}

Will this work for edge-triggered IRQs? I don't think it will and I'd
rather this code lived in some SoC-specific place than in the general
ARM code. Maybe you could pass an IRQ callback in the platform data?

> +	/*
> +	 * We should be able to get away with the amount of IRQ_NONEs we give,
> +	 * while still having the spurious IRQ detection code kick in if the
> +	 * interrupt really starts hitting spuriously.
> +	 */
> +	return ret;
> +}

It looks like you have to have less than 1:1000 IRQ_HANDLED:IRQ_NONE for the
IRQ to be disabled so you should be safe.

> +	/*
> +	 * Some SoCs have the PMU IRQ lines of two cores wired together into a
> +	 * single interrupt.
> +	 */

Well, it's only one SoC as far as I know :) A more common problem that I
anticipate is lack of an IRQ altogether, which this solution doesn't help us
with unfortunately.

Will

[PATCH] RFC: ux500: add PMU resources

[Russell King - ARM Linux]

On Mon, Feb 07, 2011 at 10:08:18AM -0000, Will Deacon wrote:
> > +static irqreturn_t armpmu_bounce_interrupt(int irq, void *dev)
> > +{
> > +	irqreturn_t ret = armpmu->handle_irq(irq, dev);
> > +
> > +	if (ret == IRQ_NONE) {
> > +		int other = !smp_processor_id();
> > +		irq_set_affinity(irq, cpumask_of(other));
> > +	}
> 
> Will this work for edge-triggered IRQs? I don't think it will and I'd
> rather this code lived in some SoC-specific place than in the general
> ARM code. Maybe you could pass an IRQ callback in the platform data?

Especially as it assumes the presence of only one other CPU, which is
not always the case.

[PATCH] RFC: ux500: add PMU resources

[Lee Jones]

Yes, sorry.

More haste, less speed and all that.

Acked-by: Lee Jones<lee.jones@linaro.org>
Acked-by: Linus Walleij<linus.walleij@stericsson.com>

On 07/02/11 10:04, Linus Walleij wrote:
> On 02/07/2011 08:55 AM, Lee Jones wrote:
>> Linus and I have already added our SoBs.
>>
>> Signed-off-by: Lee Jones<lee.jones@linaro.org>
>> Signed-off-by: Linus Walleij<linus.walleij@stericsson.com>
>>    
>
> SoB is for the delivery path, I assume you mean s/Signed-off-by/Acked-by/g...
>
> Yours,
> Linus Walleij

[PATCH] RFC: ux500: add PMU resources

[Will Deacon]

> Here's the IPI version as well, for comparison with the other
> approach.  The IRQ handling code will mask the interrupt before
> handling it, so it can't nest, can it?

It's not safe to cross-call from an IRQ handler with interrupts disabled
though as you're asking for deadlock. I think you'll need to schedule the
IPI for later, which is why you end up being nested.
 
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index 5efa264..a97e50f 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -19,6 +19,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/spinlock.h>
>  #include <linux/uaccess.h>
> +#include <linux/smp.h>

[...]

> +static irqreturn_t armpmu_core2_irqret;
> +
> +/* Called via IPI on the second core */
> +static void armpmu_kick(void *data)
> +{
> +	int irq = (int) data;
> +
> +	armpmu_core2_irqret = armpmu->handle_irq(irq, NULL);
> +	smp_wmb();
> +}

That barrier isn't going to do much. If you want to make the value
visible, you'll need a dsb().

> +static irqreturn_t armpmu_single_interrupt(int irq, void *dev)
> +{
> +	irqreturn_t irqret = armpmu->handle_irq(irq, dev);
> +	int err;
> +
> +	if (irqret != IRQ_NONE)
> +		return irqret;
> +
> +	local_irq_enable();
> +	err = smp_call_function_single(1, armpmu_kick, (void *) irq, true);
> +	local_irq_disable();

Ah, I'd not considered re-enabling interrupts from the handler. I think that will
work, but this code doesn't belong in the main perf stuff as it's too platform
dependent.

> +	if (err)
> +		return irqret;
> +
> +	smp_rmb();
> +	return armpmu_core2_irqret;
> +}

This read barrier won't help either because there's only one shared variable.
The visibility of the return value should be guaranteed by the cross-call code
(see csd_{un}lock) so you can probably lose the barriers altogether.

Will