ARM perf events support v5

This patch implements support for ARMv6 performance counters in the Linux performance events subsystem. ARMv6 architectures that have the performance counters should enable HW_PERF_EVENTS and define the interrupts for the counters in arch/arm/kernel/perf_event.c

arch/arm/kernel/perf_event.c new file +23 -0

+static int __init
+init_hw_perf_events(void)
+{
+#define CPUID_MASK	0xFFF0
+	unsigned long cpuid = read_cpuid_id() & CPUID_MASK;
+
+	switch (cpuid) {
+	case 0xB360:	/* ARM1136 */
+	case 0xB560:	/* ARM1156 */
+	case 0xB760:	/* ARM1176 */
+		armpmu = &armv6pmu;
+		memcpy(armpmu_perf_cache_map, armv6_perf_cache_map,
+				sizeof(armv6_perf_cache_map));
+		perf_max_events	= armv6pmu.num_events;
+		break;
+	case 0xB020:	/* ARM11mpcore */
+		armpmu = &armv6mpcore_pmu;
+		memcpy(armpmu_perf_cache_map, armv6mpcore_perf_cache_map,
+				sizeof(armv6mpcore_perf_cache_map));
+		perf_max_events = armv6mpcore_pmu.num_events;
+		break;
+	default:
+		pr_info("no hardware support available\n");

Given the difficulty in determining the CPU type 100%, this should be changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the moment. */
	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Quoted 13 lines

Given the difficulty in determining the CPU type 100%, this should be changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the moment. */
	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Whilst I understand that the whole cpuid thing is a complete mess [I saw the lkml posts yesterday], I'm not sure this is necessary for v7 cores. For v7, the PMU is part of the architecture and so *must* be implemented in the way described in the ARM ARM [Chapter 9], regardless of the implementer.

However, for v6 you're right - checking that ARM is the implementer is a sensible thing to do. This does rely on the implementer setting those bits correctly though :)

Additionally, it's quite possible for an implementer to extend the hardware counters on a v7 SoC. This means that you would have the PMU as described in the ARM ARM, but then additional counters, perhaps outside the core that can be accessed in a different way. In this case, switching on the implementer ID is essential to determine extra features, but the base features can be assumed to be present.

Hi Jamie,

Quoted 13 lines · Jamie Iles

Given the difficulty in determining the CPU type 100%, this should be changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the moment. */
	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Whilst I understand that the whole cpuid thing is a complete mess [I saw the lkml posts yesterday], I'm not sure this is necessary for v7 cores. For v7, the PMU is part of the architecture and so *must* be implemented in the way described in the ARM ARM [Chapter 9], regardless of the implementer.

This function is called whenever the PMU support is built in - and this is the first place a decision is made about how to handle stuff.

Merely checking the part number without checking the implementer is nonsense - the part number is defined by the implmenter, not ARM, so the part number can only be interpreted with knowledge of the implementer.

So, when v7 gets added, checking the main ID register is the wrong thing to do.

Given that cpu_architecture() appears to have been redefined to return the MMU architecture, we have no real way to properly determine if we have a v7 PMU present - in fact, the whole "are we v6 or v7 or something later" question seems to be extremely muddy and indeterminant.

So I don't think even checking cpu_architecture() == CPU_ARCH_ARMv7 is the right thing either.

Quoted 17 lines · Will Deacon

Hi Jamie,

Quoted 13 lines · Jamie Iles

Given the difficulty in determining the CPU type 100%, this should be changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the moment. */
	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Quoted 5 lines

Whilst I understand that the whole cpuid thing is a complete mess [I saw the lkml posts yesterday], I'm not sure this is necessary for v7 cores. For v7, the PMU is part of the architecture and so *must* be implemented in the way described in the ARM ARM [Chapter 9], regardless of the implementer.

This function is called whenever the PMU support is built in - and this is the first place a decision is made about how to handle stuff.

Merely checking the part number without checking the implementer is nonsense - the part number is defined by the implmenter, not ARM, so the part number can only be interpreted with knowledge of the implementer.

So, when v7 gets added, checking the main ID register is the wrong thing to do.

Given that cpu_architecture() appears to have been redefined to return the MMU architecture, we have no real way to properly determine if we have a v7 PMU present - in fact, the whole "are we v6 or v7 or something later" question seems to be extremely muddy and indeterminant.

So I don't think even checking cpu_architecture() == CPU_ARCH_ARMv7 is the right thing either.

Quoted 35 lines · On Thursday 21 January 2010 11:38:03 Russell King - ARM Linux wrote:

Quoted 17 lines · Will Deacon

Hi Jamie,

Quoted 13 lines · Jamie Iles

Given the difficulty in determining the CPU type 100%, this should be changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the moment. */
	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Quoted 5 lines

Whilst I understand that the whole cpuid thing is a complete mess [I saw the lkml posts yesterday], I'm not sure this is necessary for v7 cores. For v7, the PMU is part of the architecture and so *must* be implemented in the way described in the ARM ARM [Chapter 9], regardless of the implementer.

This function is called whenever the PMU support is built in - and this is the first place a decision is made about how to handle stuff.

Merely checking the part number without checking the implementer is nonsense - the part number is defined by the implmenter, not ARM, so the part number can only be interpreted with knowledge of the implementer.

So, when v7 gets added, checking the main ID register is the wrong thing to do.

Quoted 7 lines

Given that cpu_architecture() appears to have been redefined to return the MMU architecture, we have no real way to properly determine if we have a v7 PMU present - in fact, the whole "are we v6 or v7 or something later" question seems to be extremely muddy and indeterminant.

So I don't think even checking cpu_architecture() == CPU_ARCH_ARMv7 is the right thing either.

Agree. Here is the latest version of the detection code, after merging Jamie's latest version:

        unsigned long cpuid = read_cpuid_id() & CPUID_MASK;

        switch (cpuid) {
        case 0xB360:    /* ARM1136 */
        case 0xB560:    /* ARM1156 */
        case 0xB760:    /* ARM1176 */
	    ...
                break;
        case 0xB020:    /* ARM11mpcore */
	    ...
                break;
        case 0xC080:    /* Cortex-A8 */
	    ...
                break;
        case 0xC090:    /* Cortex-A9 */
	    ...
                break;
        default:
                pr_info("no hardware support available\n");
                perf_max_events = -1;
       }
       ...

Is that OK if we just add 'if (implementor == 0x41) {' before the switch statement, as proposed above?

Quoted 71 lines · Jean Pihet

Quoted 35 lines · On Thursday 21 January 2010 11:38:03 Russell King - ARM Linux wrote:

Quoted 17 lines · Will Deacon

Hi Jamie,

Quoted 13 lines · Jamie Iles

Given the difficulty in determining the CPU type 100%, this should be changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the moment. */
	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Quoted 5 lines

Whilst I understand that the whole cpuid thing is a complete mess [I saw the lkml posts yesterday], I'm not sure this is necessary for v7 cores. For v7, the PMU is part of the architecture and so *must* be implemented in the way described in the ARM ARM [Chapter 9], regardless of the implementer.

This function is called whenever the PMU support is built in - and this is the first place a decision is made about how to handle stuff.

Merely checking the part number without checking the implementer is nonsense - the part number is defined by the implmenter, not ARM, so the part number can only be interpreted with knowledge of the implementer.

So, when v7 gets added, checking the main ID register is the wrong thing to do.

Quoted 8 lines

Given that cpu_architecture() appears to have been redefined to return the MMU architecture, we have no real way to properly determine if we have a v7 PMU present - in fact, the whole "are we v6 or v7 or something later" question seems to be extremely muddy and indeterminant.

So I don't think even checking cpu_architecture() == CPU_ARCH_ARMv7 is the right thing either.

Agree. Here is the latest version of the detection code, after merging Jamie's latest version:

        unsigned long cpuid = read_cpuid_id() & CPUID_MASK;

        switch (cpuid) {
        case 0xB360:    /* ARM1136 */
        case 0xB560:    /* ARM1156 */
        case 0xB760:    /* ARM1176 */
	    ...
                break;
        case 0xB020:    /* ARM11mpcore */
	    ...
                break;
        case 0xC080:    /* Cortex-A8 */
	    ...
                break;
        case 0xC090:    /* Cortex-A9 */
	    ...
                break;
        default:
                pr_info("no hardware support available\n");
                perf_max_events = -1;
       }
       ...

Quoted 2 lines

Is that OK if we just add 'if (implementor == 0x41) {' before the switch statement, as proposed above?

That sounds good to me. Are these patches ready for merging?

Jamie

Quoted 4 lines

Quoted 2 lines

Is that OK if we just add 'if (implementor == 0x41) {' before the switch statement, as proposed above?

That sounds good to me. Are these patches ready for merging?

Yes the code is ready, testing it right now. I will rebase it on your latest version as soon as they are available.

Are you ok to post the patches to the ML and tp patchwork?

Agree. Here is the latest version of the detection code, after merging Jamie's latest version:

        unsigned long cpuid = read_cpuid_id() & CPUID_MASK;

        switch (cpuid) {
        case 0xB360:    /* ARM1136 */
        case 0xB560:    /* ARM1156 */
        case 0xB760:    /* ARM1176 */
	    ...
                break;
        case 0xB020:    /* ARM11mpcore */
	    ...
                break;
        case 0xC080:    /* Cortex-A8 */
	    ...
                break;
        case 0xC090:    /* Cortex-A9 */
	    ...
                break;
        default:
                pr_info("no hardware support available\n");
                perf_max_events = -1;
       }
       ...

Is that OK if we just add 'if (implementor == 0x41) {' before the switch statement, as proposed above?

Hi Jean,

Quoted 26 lines · Jean Pihet

Agree. Here is the latest version of the detection code, after merging Jamie's latest version:

        unsigned long cpuid = read_cpuid_id() & CPUID_MASK;

        switch (cpuid) {
        case 0xB360:    /* ARM1136 */
        case 0xB560:    /* ARM1156 */
        case 0xB760:    /* ARM1176 */
	    ...
                break;
        case 0xB020:    /* ARM11mpcore */
	    ...
                break;
        case 0xC080:    /* Cortex-A8 */
	    ...
                break;
        case 0xC090:    /* Cortex-A9 */
	    ...
                break;
        default:
                pr_info("no hardware support available\n");
                perf_max_events = -1;
       }
       ...

Quoted 2 lines

Is that OK if we just add 'if (implementor == 0x41) {' before the switch statement, as proposed above?

For the v7 PMU, we can do a bit better than that because the PMU is defined by the architecture. If you read the DIDR[19:16] and it returns either 3 or 4, you have a v7 PMU present so you can make use of the architecturally defined events regardless of the implementer.

So, in response to your question, I reckon you should wrap the switch statement with the implementer check, but add a DIDR check in the else block so that cores with a v7 PMU will at least get support for the standard events.

Cheers,

Will

Quoted 4 lines

So, in response to your question, I reckon you should wrap the switch statement with the implementer check, but add a DIDR check in the else block so that cores with a v7 PMU will at least get support for the standard events.

That makes sense. Are such chipsets already out or planned in the near future?

I propose to have the current code working and merged in before supporting the generic v7 cores. Doing so requires to add new set of events mappings.

What do you think?

For the v7 PMU, we can do a bit better than that because the PMU is defined by the architecture. If you read the DIDR[19:16] and it returns either 3 or 4, you have a v7 PMU present so you can make use of the architecturally defined events regardless of the implementer.

-----Original Message-----
From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm- kernel-bounces at lists.infradead.org] On Behalf Of Jean Pihet
Sent: Thursday, January 21, 2010 1:22 PM
To: Russell King - ARM Linux
Cc: 'Jamie Iles'; Will Deacon; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 5/5] arm/perfevents: implement perf event support for ARMv6

Quoted 5 lines · On Thursday 21 January 2010 11:38:03 Russell King - ARM Linux wrote:

Quoted 4 lines · Will Deacon

Hi Jamie,

Quoted 1 line · Jamie Iles

Given the difficulty in determining the CPU type 100%, this

should be

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

changed to:

	unsigned long cpuid = read_cpuid_id();
	unsigned long implementor = (cpuid & 0xFF000000) >> 24;
	unsigned long part_number = (cpuid & 0xFFF0);

	/* We only support ARM CPUs implemented by ARM at the

moment. */

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

	if (implementor == 0x41) {
		switch (part_number) {
		case 0xB360:
		case 0xB560:
			etc

Whilst I understand that the whole cpuid thing is a complete mess

[I saw

Quoted 1 line

Quoted 1 line

the lkml posts yesterday], I'm not sure this is necessary for v7

cores.

Quoted 1 line

Quoted 1 line

For v7, the PMU is part of the architecture and so *must* be

implemented

Quoted 11 lines

Quoted 2 lines

in the way described in the ARM ARM [Chapter 9], regardless of the implementer.

This function is called whenever the PMU support is built in - and this is the first place a decision is made about how to handle stuff.

Merely checking the part number without checking the implementer is nonsense - the part number is defined by the implmenter, not ARM, so the part number can only be interpreted with knowledge of the implementer.

Quoted 4 lines

So, when v7 gets added, checking the main ID register is the wrong thing to do.

Given that cpu_architecture() appears to have been redefined to

return

Quoted 2 lines

the MMU architecture, we have no real way to properly determine if we have a v7 PMU present - in fact, the whole "are we v6 or v7 or

something

Quoted 4 lines

later" question seems to be extremely muddy and indeterminant.

So I don't think even checking cpu_architecture() == CPU_ARCH_ARMv7 is the right thing either.

Agree. Here is the latest version of the detection code, after merging Jamie's
latest version:

        unsigned long cpuid = read_cpuid_id() & CPUID_MASK;

        switch (cpuid) {
        case 0xB360:    /* ARM1136 */
        case 0xB560:    /* ARM1156 */
        case 0xB760:    /* ARM1176 */
	    ...
                break;
        case 0xB020:    /* ARM11mpcore */
	    ...
                break;
        case 0xC080:    /* Cortex-A8 */
	    ...
                break;
        case 0xC090:    /* Cortex-A9 */
	    ...
                break;
        default:
                pr_info("no hardware support available\n");
                perf_max_events = -1;
       }
       ...

Is that OK if we just add 'if (implementor == 0x41) {' before the switch
statement, as proposed above?

Jean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Where do I find a repository that contains the latest performance events/ARM efforts?

Quoted 2 lines · Tomasz Fujak

Where do I find a repository that contains the latest performance events/ARM efforts?

I've just submitted the generic/ARMv6 stuff to the patch system. Hopefully this will end up in Russell's tree soon.

Jamie

Hi,

Quoted 6 lines · On Tuesday 26 January 2010 17:09:29 Jamie Iles wrote:

Quoted 2 lines · Tomasz Fujak

Where do I find a repository that contains the latest performance events/ARM efforts?

I've just submitted the generic/ARMv6 stuff to the patch system. Hopefully this will end up in Russell's tree soon.

I am about to submit the patch for ARMv7, so hopefully it will end up in Russell's tree soon as well.

Quoted 1 line

Jamie

Jean

From 6e8e6e710c93eb16885c5c789f689a86d824bdba Mon Sep 17 00:00:00 2001

Here is the latest version of the patch, after review on the ML. Adds the Performance Events support for ARMv7 processor, using the PMNC unit in HW.

Hi Jean,

Quoted 3 lines · Jean Pihet

Here is the latest version of the patch, after review on the ML. Adds the Performance Events support for ARMv7 processor, using the PMNC unit in HW.

I tested this on a dual-core Cortex-A9 [Realview PBX board]. One thing I noticed was that, even if you don't add the PMU IRQs to kernel/pmu.c, perf will still report event counts [I guess this is due to sampling on context switch etc].

However, if the IRQs are
defined, but for some reason we fail to request them, then the armpmu_reserve_hardware function will fail.

Actually, the return
value appears to be uninitialised if you don't have any IRQs defined.

Anyway, apart from that, it appeared to work fine.

Tested-by Will Deacon <[email protected]>

Will

Quoted 3 lines

However, if the IRQs are
defined, but for some reason we fail to request them, then the armpmu_reserve_hardware function will fail.

That is the expected behaviour, isn't it?
Why is the PMU request failing? That is worth investigating. I never had that problem on the Cortex-A8. Is it caused because of the multicore?

Quoted 2 lines

Actually, the return
value appears to be uninitialised if you don't have any IRQs defined.

If the PMU request fails the IRQ should not be requested, so I think it is ok. Is that correct?

Quoted 2 lines

Anyway, apart from that, it appeared to work fine.

Good!

Thanks for testing!
Jean

Quoted 1 line

Tested-by Will Deacon <[email protected]>

Hi Jean,

Quoted 6 lines · Jean Pihet

Quoted 3 lines

However, if the IRQs are
defined, but for some reason we fail to request them, then the armpmu_reserve_hardware function will fail.

That is the expected behaviour, isn't it?
Why is the PMU request failing? That is worth investigating. I never had that problem on the Cortex-A8. Is it caused because of the multicore?

Don't worry, requesting the IRQs did not fail. I was simply speculating about the behaviour if it *does* fail. Then armpmu_reserve_hardware will return an error, which will prevent the event being added. Instead, we could print a warning, but continue without IRQs in the same manner as though they had not been defined in pmu.c.

Quoted 4 lines

Quoted 2 lines

Actually, the return
value appears to be uninitialised if you don't have any IRQs defined.

If the PMU request fails the IRQ should not be requested, so I think it is ok. Is that correct?

What I mean is, if pmu_irqs->num_irqs < 1, then we return err without ever setting it. In fact, we might even end up freeing IRQs which we haven't requested which could cause all sorts of problems.

Russell, shall I supersede the old patch in the patch system and roll in the patch above?

This was my mistake and I don't think we should continue - we'd _probably_ be ok for sampling counters, but for the counting counters we could hit problems if the counters weren't being scheduled in and out. For example if someone did 'perf stat -a -e cycles -- sleep 1m' then the cycle counter would not need to be scheduled out, the counter could wrap a few times and we wouldn't know because we didn't get an interrupt.

Quoted 6 lines · Jamie Iles

This was my mistake and I don't think we should continue - we'd _probably_ be ok for sampling counters, but for the counting counters we could hit problems if the counters weren't being scheduled in and out. For example if someone did 'perf stat -a -e cycles -- sleep 1m' then the cycle counter would not need to be scheduled out, the counter could wrap a few times and we wouldn't know because we didn't get an interrupt.

Jamie,

Please let me know when these patches have stabilised.

The perf events subsystem allows counting of both hardware and software events. This patch implements the bare minimum for software performance events.

Signed-off-by Jamie Iles <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

---
 arch/arm/Kconfig                  |    2 ++
 arch/arm/include/asm/perf_event.h |   31 +++++++++++++++++++++++++++++++
 arch/arm/mm/fault.c               |    7 +++++++
 3 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/perf_event.h

arch/arm/Kconfig +0 -0

index 293a879..72646b2 100644

arch/arm/Kconfig config ARM · line 23

Quoted 9 lines

arch/arm/Kconfig +0 -0

index 293a879..72646b2 100644
@@ -21,6 +21,8 @@ config ARM
 	select HAVE_KERNEL_GZIP
 	select HAVE_KERNEL_LZO
 	select GENERIC_ATOMIC64

This conflicts - this select is only if !CPU_32v6K in my kernel.

What's the implication? Does this perf stuff require these atomic64 stuff (in which case, those symbols should only be selected if GENERIC_ATOMIC64 is also selected.)

Quoted 19 lines · Jamie Iles

The perf events subsystem allows counting of both hardware and software events. This patch implements the bare minimum for software performance events.

Signed-off-by Jamie Iles <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

---
 arch/arm/Kconfig                  |    2 ++
 arch/arm/include/asm/perf_event.h |   31 +++++++++++++++++++++++++++++++
 arch/arm/mm/fault.c               |    7 +++++++
 3 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/perf_event.h

arch/arm/Kconfig +0 -0

index 293a879..72646b2 100644

arch/arm/Kconfig config ARM · line 23

Quoted 3 lines

arch/arm/Kconfig +0 -0

@@ -21,6 +21,8 @@ config ARM
 	select HAVE_KERNEL_GZIP
 	select HAVE_KERNEL_LZO
 	select GENERIC_ATOMIC64

This conflicts - this select is only if !CPU_32v6K in my kernel.

What's the implication? Does this perf stuff require these atomic64 stuff (in which case, those symbols should only be selected if GENERIC_ATOMIC64 is also selected.)

Please sort this out.

Quoted 26 lines · Russell King - ARM Linux

Quoted 19 lines · Jamie Iles

The perf events subsystem allows counting of both hardware and software events. This patch implements the bare minimum for software performance events.

Signed-off-by Jamie Iles <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

---
 arch/arm/Kconfig                  |    2 ++
 arch/arm/include/asm/perf_event.h |   31 +++++++++++++++++++++++++++++++
 arch/arm/mm/fault.c               |    7 +++++++
 3 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/perf_event.h

arch/arm/Kconfig +0 -0

index 293a879..72646b2 100644

arch/arm/Kconfig config ARM · line 23

Quoted 3 lines

arch/arm/Kconfig +0 -0

@@ -21,6 +21,8 @@ config ARM
 	select HAVE_KERNEL_GZIP
 	select HAVE_KERNEL_LZO
 	select GENERIC_ATOMIC64

This conflicts - this select is only if !CPU_32v6K in my kernel.

Quoted 5 lines

What's the implication? Does this perf stuff require these atomic64 stuff (in which case, those symbols should only be selected if GENERIC_ATOMIC64 is also selected.)

Please sort this out.

As Will mentioned, the correct case should be 'GENERIC_ATOMIC64 if (!CPU_32v6K)' due to my patches being off a tree without the native atomic64's. I can rebase onto next if that's easier.

Quoted 31 lines · Jamie Iles

Quoted 30 lines · Russell King - ARM Linux

Quoted 19 lines · Jamie Iles

The perf events subsystem allows counting of both hardware and software events. This patch implements the bare minimum for software performance events.

Signed-off-by Jamie Iles <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

---
 arch/arm/Kconfig                  |    2 ++
 arch/arm/include/asm/perf_event.h |   31 +++++++++++++++++++++++++++++++
 arch/arm/mm/fault.c               |    7 +++++++
 3 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/perf_event.h

arch/arm/Kconfig +0 -0

index 293a879..72646b2 100644

arch/arm/Kconfig config ARM · line 23

Quoted 3 lines

arch/arm/Kconfig +0 -0

@@ -21,6 +21,8 @@ config ARM
 	select HAVE_KERNEL_GZIP
 	select HAVE_KERNEL_LZO
 	select GENERIC_ATOMIC64

This conflicts - this select is only if !CPU_32v6K in my kernel.

What's the implication? Does this perf stuff require these atomic64 stuff (in which case, those symbols should only be selected if GENERIC_ATOMIC64 is also selected.)

Quoted 4 lines

Quoted 1 line

Please sort this out.

As Will mentioned, the correct case should be 'GENERIC_ATOMIC64 if (!CPU_32v6K)' due to my patches being off a tree without the native atomic64's. I can rebase onto next if that's easier.

Much prefer having them rebased onto 7558b1f, which is in -next if you can manage that.

Make sure that we have access to the performance counters and that they aren't being used by perf events or anything else.

Cc Will Deacon <[email protected]>

Cc Jean Pihet <[email protected]>

Signed-off-by Jamie Iles <[email protected]>

---
 arch/arm/oprofile/op_model_arm11_core.c |    4 +-
 arch/arm/oprofile/op_model_arm11_core.h |    4 +-
 arch/arm/oprofile/op_model_mpcore.c     |   42 ++++++++++++++++--------------
 arch/arm/oprofile/op_model_v6.c         |   30 ++++++++++++++--------
 arch/arm/oprofile/op_model_v7.c         |   30 ++++++++++++++--------
 arch/arm/oprofile/op_model_v7.h         |    4 +-
 arch/arm/oprofile/op_model_xscale.c     |   35 ++++++++++++++-----------
 7 files changed, 85 insertions(+), 64 deletions(-)

arch/arm/oprofile/op_model_mpcore.c +12 -10

index 4ce0f98..f73ce87 100644
@@ -32,6 +32,7 @@
 /* #define DEBUG */
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/err.h>
 #include <linux/sched.h>
 #include <linux/oprofile.h>
 #include <linux/interrupt.h>

@@ -43,6 +44,7 @@
 #include <mach/hardware.h>
 #include <mach/board-eb.h>
 #include <asm/system.h>
+#include <asm/pmu.h>
 
 #include "op_counter.h"
 #include "op_arm_model.h"

@@ -58,6 +60,7 @@
  * Bitmask of used SCU counters
  */
 static unsigned int scu_em_used;
+static const struct pmu_irqs *pmu_irqs;
 
 /*
  * 2 helper fns take a counter number from 0-7 (not the userspace-visible counter number)

@@ -225,33 +228,40 @@ static int em_setup_ctrs(void)
 	return 0;
 }
 
-static int arm11_irqs[] = {
-	[0]	= IRQ_EB11MP_PMU_CPU0,
-	[1]	= IRQ_EB11MP_PMU_CPU1,
-	[2]	= IRQ_EB11MP_PMU_CPU2,
-	[3]	= IRQ_EB11MP_PMU_CPU3
-};
-
 static int em_start(void)
 {
 	int ret;
 
-	ret = arm11_request_interrupts(arm11_irqs, ARRAY_SIZE(arm11_irqs));
+	pmu_irqs = reserve_pmu();
+	if (IS_ERR(pmu_irqs)) {
+		ret = PTR_ERR(pmu_irqs);
+		goto out;
+	}
+
+	ret = arm11_request_interrupts(pmu_irqs->irqs, pmu_irqs->num_irqs);
 	if (ret == 0) {
 		em_call_function(arm11_start_pmu);
 
 		ret = scu_start();
-		if (ret)
-			arm11_release_interrupts(arm11_irqs, ARRAY_SIZE(arm11_irqs));
+		if (ret) {
+			arm11_release_interrupts(pmu_irqs->irqs,

From 18018f4e439ebcf9358790887502764b18459c9c Mon Sep 17 00:00:00 2001
From: George G. Davis <[email protected]>
Date: Fri, 5 Feb 2010 00:38:01 -0500
Subject: [PATCH] ARM: Fix ARM11 MPCore OProfile breakage due to ARM patch 5901/2

The recent "fe6c67f ARM: 5901/2: arm/oprofile: reserve the PMU when starting" commit broke OProfile support on ARM11 MPCore targets by adding a misplaced "} else {" clause in function em_start() which results in a call to release_pmu() even though the PMU is in use which in turn results in an oops while using OProfile. Removing the stray else clause fixes the problem.

Signed-off-by George G. Davis <[email protected]>

---
 arch/arm/oprofile/op_model_mpcore.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

arch/arm/oprofile/op_model_mpcore.c +0 -0

index f73ce87..0d4f099 100644

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).

arch/arm/kernel/pmu.c new file +22 -0

new file mode 100644
index 0000000..688d450
@@ -0,0 +1,105 @@
+/*
+ *  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/cpumask.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <asm/pmu.h>
+
+/*
+ * Define the IRQs for the system. We could use something like a platform
+ * device but that seems fairly heavyweight for this. Also, the performance