[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Will Deacon]

* Jamie Iles <jamie.iles@picochip.com> wrote:

> To add support for perf events and to allow the hardware
> counters to be shared with oprofile, we need a way to reserve
> access to the pmu (performance monitor unit).
>
> Signed-off-by: Jamie Iles <jamie.iles <at> picochip.com>
> ---
> arch/arm/include/asm/pmu.h |   54 ++++++++++++++++++++++++++++++++++++++++++++
> arch/arm/kernel/Makefile   |    1 +
> arch/arm/kernel/pmu.c      |   32 ++++++++++++++++++++++++++
> arch/arm/mm/Kconfig        |    4 +++
> 4 files changed, 91 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/pmu.h
> create mode 100644 arch/arm/kernel/pmu.c

Hi Jamie,

I like the idea of having a separate file for reserving the PMU across subsystems.
I also think it would be neat to extend it to request the relevant IRQs (or at
least return the PMU IRQs) once you've got it reserved.

This way, there will be no need to duplicate the inevitable collection of machine 
#ifdefs across tools [such as oprofile and perf]. It would also ensure that you can't 
reserve the PMU if somebody else has claimed the IRQs [and similarly, you can't
release it without giving them up].

What do you think?

Will

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Jamie Iles]

On Fri, Dec 11, 2009 at 03:31:21PM -0000, Will Deacon wrote:
> * Jamie Iles <jamie.iles@picochip.com> wrote:
> 
> > To add support for perf events and to allow the hardware
> > counters to be shared with oprofile, we need a way to reserve
> > access to the pmu (performance monitor unit).
> >
> > Signed-off-by: Jamie Iles <jamie.iles <at> picochip.com>
> > ---
> > arch/arm/include/asm/pmu.h |   54 ++++++++++++++++++++++++++++++++++++++++++++
> > arch/arm/kernel/Makefile   |    1 +
> > arch/arm/kernel/pmu.c      |   32 ++++++++++++++++++++++++++
> > arch/arm/mm/Kconfig        |    4 +++
> > 4 files changed, 91 insertions(+), 0 deletions(-)
> > create mode 100644 arch/arm/include/asm/pmu.h
> > create mode 100644 arch/arm/kernel/pmu.c
> 
> Hi Jamie,
> 
> I like the idea of having a separate file for reserving the PMU across subsystems.
> I also think it would be neat to extend it to request the relevant IRQs (or at
> least return the PMU IRQs) once you've got it reserved.
> 
> This way, there will be no need to duplicate the inevitable collection of machine 
> #ifdefs across tools [such as oprofile and perf]. It would also ensure that you can't 
> reserve the PMU if somebody else has claimed the IRQs [and similarly, you can't
> release it without giving them up].
> 
> What do you think?
That sounds like a good plan. How about something like this?

#define MAX_PMU_IRQS	8   /* Maximum number of IRQs for the PMU(s). */
struct pmu_irqs {
	int	    irqs[MAX_PMU_IRQS];
	unsigned    num_irqs;
};

/**
 * reserve_pmu() - reserve the hardware performance counters
 *
 * Reserve the hardware performance counters in the system for exclusive use.
 * The 'struct pmu_irqs' for the system is returned on success, ERR_PTR()
 * encoded error on failure.
 */
struct pmu_irqs *
reserve_pmu(void);

/**
 * release_pmu() - Relinquish control of the performance counters
 *
 * Release the performance counters and allow someone else to use them.
 * Callers must have disabled the counters and released IRQs before calling
 * this. The 'struct pmu_irqs' returned from reserve_pmu() must be passed as
 * a cookie.
 */
void
release_pmu(struct pmu_irqs *irqs);

Jamie

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Will Deacon]

> From: Jamie Iles [mailto:jamie.iles at picochip.com]
> Sent: 11 December 2009 15:42

> > What do you think?
> That sounds like a good plan. How about something like this?
> 
> #define MAX_PMU_IRQS	8   /* Maximum number of IRQs for the PMU(s). */
> struct pmu_irqs {
> 	int	    irqs[MAX_PMU_IRQS];
> 	unsigned    num_irqs;
> };
> 
> /**
>  * reserve_pmu() - reserve the hardware performance counters
>  *
>  * Reserve the hardware performance counters in the system for exclusive use.
>  * The 'struct pmu_irqs' for the system is returned on success, ERR_PTR()
>  * encoded error on failure.
>  */
> struct pmu_irqs *
> reserve_pmu(void);
> 
> /**
>  * release_pmu() - Relinquish control of the performance counters
>  *
>  * Release the performance counters and allow someone else to use them.
>  * Callers must have disabled the counters and released IRQs before calling
>  * this. The 'struct pmu_irqs' returned from reserve_pmu() must be passed as
>  * a cookie.
>  */
> void
> release_pmu(struct pmu_irqs *irqs);

That looks good to me. This allows SMP systems to set the affinity of the PMU
IRQs too if need be - or should this also be done here? It might also be worth making
the returned struct const to stop people poking, but I'm not sure.

I've got some oprofile patches which I hope to post soon - I'll put a note in the
covering letter to say they should be modified to use these PMU functions when they make
it in.

Will

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Jamie Iles]

On Fri, Dec 11, 2009 at 05:17:59PM -0000, Will Deacon wrote:
> That looks good to me. This allows SMP systems to set the affinity of the PMU
> IRQs too if need be - or should this also be done here? It might also be worth making
> the returned struct const to stop people poking, but I'm not sure.
> 
> I've got some oprofile patches which I hope to post soon - I'll put a note in the
> covering letter to say they should be modified to use these PMU functions when they make
> it in.
I was just looking at that. I've changed all of the oprofile models but I'm
not sure what to do for mpcore as it also has the SCU and the setup. How about
I also add the following calls?

/* Reserve the SCU in the same manner as the PMU. */
struct pmu_irqs *reserve_scu(void):

/* Release the SCU in the same manner as the PMU. */
void release_scu(struct pmu_irqs *);

/* Initialise the PMU. For SMP systems this will be setting the IRQ affinity.
 * For other systems this may be a nop. */
int init_pmu(void);

/* Initialise the SCU. For SMP systems this will be setting the IRQ affinity.
 * For other systems this may be a nop. */
int init_scu(void);

Jamie

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Will Deacon]

> From: Jamie Iles [mailto:jamie.iles at picochip.com]
> Sent: 11 December 2009 17:30

> I was just looking at that. I've changed all of the oprofile models but I'm
> not sure what to do for mpcore as it also has the SCU and the setup. How about
> I also add the following calls?
> 
> /* Reserve the SCU in the same manner as the PMU. */
> struct pmu_irqs *reserve_scu(void):
> 
> /* Release the SCU in the same manner as the PMU. */
> void release_scu(struct pmu_irqs *);
> 
> /* Initialise the PMU. For SMP systems this will be setting the IRQ affinity.
>  * For other systems this may be a nop. */
> int init_pmu(void);
> 
> /* Initialise the SCU. For SMP systems this will be setting the IRQ affinity.
>  * For other systems this may be a nop. */
> int init_scu(void);

I'm not sure I like that as much since the SCU is only explicitly used for v6 profiling.
In v7, the SCU events are accessed and programmed via the PMU interface. Additionally, the
SCU is parameterised by its base address as well as its interrupts.

Perhaps you could add it with #ifdef CONFIG_CPU_V6 around the SCU functions and then
perf and oprofile can use the functions only when they are actually needed.

Will

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Jamie Iles]

On Fri, Dec 11, 2009 at 05:34:38PM -0000, Will Deacon wrote:
> > From: Jamie Iles [mailto:jamie.iles at picochip.com]
> > Sent: 11 December 2009 17:30
> 
> > I was just looking at that. I've changed all of the oprofile models but I'm
> > not sure what to do for mpcore as it also has the SCU and the setup. How about
> > I also add the following calls?
> > 
> > /* Reserve the SCU in the same manner as the PMU. */
> > struct pmu_irqs *reserve_scu(void):
> > 
> > /* Release the SCU in the same manner as the PMU. */
> > void release_scu(struct pmu_irqs *);
> > 
> > /* Initialise the PMU. For SMP systems this will be setting the IRQ affinity.
> >  * For other systems this may be a nop. */
> > int init_pmu(void);
> > 
> > /* Initialise the SCU. For SMP systems this will be setting the IRQ affinity.
> >  * For other systems this may be a nop. */
> > int init_scu(void);
> 
> I'm not sure I like that as much since the SCU is only explicitly used for v6 profiling.
> In v7, the SCU events are accessed and programmed via the PMU interface. Additionally, the
> SCU is parameterised by its base address as well as its interrupts.
> 
> Perhaps you could add it with #ifdef CONFIG_CPU_V6 around the SCU functions and then
> perf and oprofile can use the functions only when they are actually needed.
Ok, I agree with you on that. Actually, the only v6 SMP system that uses the
SCU in mainline is realview. My patches for perf events only use the PMU so
the only user of the SCU is oprofile so lets leave it where it is. If someone
else adds support for the SCU in the perf events code then we can revisit this
later.

I'll make the {reserve,release}_pmu() functions take/return const pointers and
I'll add the init_pmu() call for setting the IRQ affinity as the perf events
code will need this too. 

Jamie

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Russell King - ARM Linux]

On Fri, Dec 11, 2009 at 05:17:59PM -0000, Will Deacon wrote:
> That looks good to me. This allows SMP systems to set the affinity of the
> PMU IRQs too if need be - or should this also be done here? It might also
> be worth making the returned struct const to stop people poking, but I'm
> not sure.
> 
> I've got some oprofile patches which I hope to post soon - I'll put a
> note in the covering letter to say they should be modified to use these
> PMU functions when they make it in.

Well, I don't think that it makes sense to change the affinity of PMU
IRQs that much - if you route the CPUs own PMU interrupts to another
CPU, then how do you do things such as a backtrace?  You can't because
you don't know where the owner CPU actually is.

This is why oprofile ensures that the CPUs own PMU interrupts are routed
to that CPU - routing them elsewhere makes no sense for oprofile.

perf events for ARMv6

[Jamie Iles]

This patch series provides support for software perf events on all
ARM platforms and hardware support for v6 CPUs. In addition to adding
support for the hardware counters, these patches provide a way of
arbitrating access to the PMU and patches the oprofile code to use this.

Jamie

[PATCH 1/4] arm: provide a mechanism to reserve performance counters

[Jamie Iles]

To add support for perf events and to allow the hardware
counters to be shared with oprofile, we need a way to reserve
access to the pmu (performance monitor unit).

Signed-off-by: Jamie Iles <jamie.iles@picochip.com>
---
 arch/arm/include/asm/pmu.h |   54 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/Makefile   |    1 +
 arch/arm/kernel/pmu.c      |   32 ++++++++++++++++++++++++++
 arch/arm/mm/Kconfig        |    4 +++
 4 files changed, 91 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/pmu.h
 create mode 100644 arch/arm/kernel/pmu.c

diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
new file mode 100644
index 0000000..94d6c67
--- /dev/null
+++ b/arch/arm/include/asm/pmu.h
@@ -0,0 +1,54 @@
+/*
+ *  linux/arch/arm/include/asm/pmu.h
+ *
+ *  Copyright (C) 2009 picoChip Designs Ltd, Jamie Iles
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __ARM_PMU_H__
+#define __ARM_PMU_H__
+
+#ifdef CONFIG_CPU_HAS_PMU
+
+/**
+ * reserve_pmu() - Reserve the hardware performance counters
+ *
+ * Reserve the hardware performance counters in the system for exclusive use.
+ * Note: success here does not guarantee exclusive access - callers must
+ * respect the return value.
+ *
+ * Returns zero on success, negative error on failure.
+ */
+extern int
+reserve_pmu(void);
+
+/**
+ * release_pmu() - Relinquish control of the performance counters
+ *
+ * Release the performance counters and allow someone else to use them.
+ * Callers must have disabled the counters and released IRQs before calling
+ * this.
+ */
+extern void
+release_pmu(void);
+
+#else /* CONFIG_CPU_HAS_PMU */
+
+static inline int
+reserve_pmu(void)
+{
+	return -ENODEV;
+}
+
+static inline void
+release_pmu(void)
+{
+}
+
+#endif /* CONFIG_CPU_HAS_PMU */
+
+#endif /* __ARM_PMU_H__ */
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index e7ccf7e..286a276 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_CPU_XSCALE)	+= xscale-cp0.o
 obj-$(CONFIG_CPU_XSC3)		+= xscale-cp0.o
 obj-$(CONFIG_CPU_MOHAWK)	+= xscale-cp0.o
 obj-$(CONFIG_IWMMXT)		+= iwmmxt.o
+obj-$(CONFIG_CPU_HAS_PMU)	+= pmu.o
 AFLAGS_iwmmxt.o			:= -Wa,-mcpu=iwmmxt
 
 ifneq ($(CONFIG_ARCH_EBSA110),y)
diff --git a/arch/arm/kernel/pmu.c b/arch/arm/kernel/pmu.c
new file mode 100644
index 0000000..a2b1a26
--- /dev/null
+++ b/arch/arm/kernel/pmu.c
@@ -0,0 +1,32 @@
+/*
+ *  linux/arch/arm/kernel/pmu.c
+ *
+ *  Copyright (C) 2009 picoChip Designs Ltd, Jamie Iles
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/semaphore.h>
+
+#include <asm/pmu.h>
+
+static DECLARE_MUTEX(pmu_mutex);
+
+int
+reserve_pmu(void)
+{
+	return down_trylock(&pmu_mutex) ? -EBUSY : 0;
+}
+EXPORT_SYMBOL_GPL(reserve_pmu);
+
+void
+release_pmu(void)
+{
+	up(&pmu_mutex);
+}
+EXPORT_SYMBOL_GPL(release_pmu);
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index dd4698c..60edbfe 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -398,6 +398,7 @@ config CPU_V6
 	select CPU_HAS_ASID if MMU
 	select CPU_COPY_V6 if MMU
 	select CPU_TLB_V6 if MMU
+	select CPU_HAS_PMU
 
 # ARMv6k
 config CPU_32v6K
@@ -536,6 +537,9 @@ config CPU_COPY_FA
 config CPU_COPY_V6
 	bool
 
+config CPU_HAS_PMU
+	bool
+
 # This selects the TLB model
 config CPU_TLB_V3
 	bool
-- 
1.6.5.4