[0/2] ARM/arm64: Mark all accessor functions inline

Message ID cover.1683561087.git.geert+renesas@glider.be
Headers
Series ARM/arm64: Mark all accessor functions inline |

Message

Geert Uytterhoeven May 8, 2023, 4:05 p.m. UTC
  Hi all,

This patch series adds missing "inline" keywords to the few perf
accessors that lack them.

BTW, I tried converting my local timing code to the new unified system.
This works fine on arm64, but broke on arm32.  Is read_pmccntr()
supposed to work on arm32? I get an undefined instruction exception on
Cortex A15 and A9.  Before, my custom code used "mrc p15, 0, %0, c9,
c13, 0" (as is also used in arch/arm/kernel/perf_event_v7.c), for which
there is no accessor yet.

Thanks for your comments!

Geert Uytterhoeven (2):
  ARM: perf: Mark all accessor functions inline
  arm64: perf: Mark all accessor functions inline

 arch/arm/include/asm/arm_pmuv3.h   | 6 +++---
 arch/arm64/include/asm/arm_pmuv3.h | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)
  

Comments

Marc Zyngier May 9, 2023, 7:42 a.m. UTC | #1
Hi Geert,

On Mon, 08 May 2023 17:05:17 +0100,
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> 	Hi all,
> 
> This patch series adds missing "inline" keywords to the few perf
> accessors that lack them.

I'll get to that shortly.

> 
> BTW, I tried converting my local timing code to the new unified system.
> This works fine on arm64, but broke on arm32.  Is read_pmccntr()
> supposed to work on arm32? I get an undefined instruction exception on
> Cortex A15 and A9.  Before, my custom code used "mrc p15, 0, %0, c9,
> c13, 0" (as is also used in arch/arm/kernel/perf_event_v7.c), for which
> there is no accessor yet.

You can only use this instruction on a CPU that implements PMUv3,
which limits it to an ARMv8 implementation (either a 64bit CPU in
32bit mode, or something like an A32). A15 and A9 being firmly in the
ARMv7 camp, they cannot make use of this (hence the UNDEF you're
getting).

Thanks,

	M.
  
Mark Rutland May 9, 2023, 11:26 a.m. UTC | #2
On Mon, May 08, 2023 at 06:05:17PM +0200, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series adds missing "inline" keywords to the few perf
> accessors that lack them.
> 
> BTW, I tried converting my local timing code to the new unified system.
> This works fine on arm64, but broke on arm32.  Is read_pmccntr()
> supposed to work on arm32? I get an undefined instruction exception on
> Cortex A15 and A9.

That's expected.

This code is for PMUv3 (which was added as part of ARMv8), and one of the
things changed in PMUv3 was that PMCCNTR was expanded to 64 bits accessible via
MRRC and MCRR.

Previously, PMCCNTR was only 32 bits, and that's what Cortex-A9 and Cortex-A15
implement.

Thanks,
Mark.

> Before, my custom code used "mrc p15, 0, %0, c9,
> c13, 0" (as is also used in arch/arm/kernel/perf_event_v7.c), for which
> there is no accessor yet.
> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (2):
>   ARM: perf: Mark all accessor functions inline
>   arm64: perf: Mark all accessor functions inline
> 
>  arch/arm/include/asm/arm_pmuv3.h   | 6 +++---
>  arch/arm64/include/asm/arm_pmuv3.h | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> -- 
> 2.34.1
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds
  
Mark Rutland May 9, 2023, 11:30 a.m. UTC | #3
On Mon, May 08, 2023 at 06:05:17PM +0200, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series adds missing "inline" keywords to the few perf
> accessors that lack them.

Will, I assume that you'll pick these up as a cleanup/fix.

Thanks,
Mark.

> BTW, I tried converting my local timing code to the new unified system.
> This works fine on arm64, but broke on arm32.  Is read_pmccntr()
> supposed to work on arm32? I get an undefined instruction exception on
> Cortex A15 and A9.  Before, my custom code used "mrc p15, 0, %0, c9,
> c13, 0" (as is also used in arch/arm/kernel/perf_event_v7.c), for which
> there is no accessor yet.
> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (2):
>   ARM: perf: Mark all accessor functions inline
>   arm64: perf: Mark all accessor functions inline
> 
>  arch/arm/include/asm/arm_pmuv3.h   | 6 +++---
>  arch/arm64/include/asm/arm_pmuv3.h | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> -- 
> 2.34.1
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds
  
Will Deacon May 16, 2023, 3:14 p.m. UTC | #4
On Mon, 8 May 2023 18:05:17 +0200, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series adds missing "inline" keywords to the few perf
> accessors that lack them.
> 
> BTW, I tried converting my local timing code to the new unified system.
> This works fine on arm64, but broke on arm32.  Is read_pmccntr()
> supposed to work on arm32? I get an undefined instruction exception on
> Cortex A15 and A9.  Before, my custom code used "mrc p15, 0, %0, c9,
> c13, 0" (as is also used in arch/arm/kernel/perf_event_v7.c), for which
> there is no accessor yet.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/2] ARM: perf: Mark all accessor functions inline
      https://git.kernel.org/arm64/c/68e3f61eb9f5
[2/2] arm64: perf: Mark all accessor functions inline
      https://git.kernel.org/arm64/c/3bc879e355da

Cheers,