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

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

[Geert Uytterhoeven]

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

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

[PATCH 1/2] ARM: perf: Mark all accessor functions inline

[Geert Uytterhoeven]

When just including <asm/arm_pmuv3.h>:

    arch/arm/include/asm/arm_pmuv3.h:110:13: error: ‘write_pmevtypern’ defined but not used [-Werror=unused-function]
      110 | static void write_pmevtypern(int n, unsigned long val)
	  |             ^~~~~~~~~~~~~~~~
    arch/arm/include/asm/arm_pmuv3.h:103:13: error: ‘write_pmevcntrn’ defined but not used [-Werror=unused-function]
      103 | static void write_pmevcntrn(int n, unsigned long val)
	  |             ^~~~~~~~~~~~~~~
    arch/arm/include/asm/arm_pmuv3.h:95:22: error: ‘read_pmevcntrn’ defined but not used [-Werror=unused-function]
       95 | static unsigned long read_pmevcntrn(int n)
	  |                      ^~~~~~~~~~~~~~

Fix this by adding the missing "inline" keywords to the three accessor
functions that lack them.

Fixes: 009d6dc87a568db6 ("ARM: perf: Allow the use of the PMUv3 driver on 32bit ARM")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/include/asm/arm_pmuv3.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
index 78d3d4b82c6c2598..f4db3e75d75f024e 100644
--- a/arch/arm/include/asm/arm_pmuv3.h
+++ b/arch/arm/include/asm/arm_pmuv3.h
@@ -92,7 +92,7 @@
 
 #define RETURN_READ_PMEVCNTRN(n) \
 	return read_sysreg(PMEVCNTR##n)
-static unsigned long read_pmevcntrn(int n)
+static inline unsigned long read_pmevcntrn(int n)
 {
 	PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
 	return 0;
@@ -100,14 +100,14 @@ static unsigned long read_pmevcntrn(int n)
 
 #define WRITE_PMEVCNTRN(n) \
 	write_sysreg(val, PMEVCNTR##n)
-static void write_pmevcntrn(int n, unsigned long val)
+static inline void write_pmevcntrn(int n, unsigned long val)
 {
 	PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
 }
 
 #define WRITE_PMEVTYPERN(n) \
 	write_sysreg(val, PMEVTYPER##n)
-static void write_pmevtypern(int n, unsigned long val)
+static inline void write_pmevtypern(int n, unsigned long val)
 {
 	PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
 }
-- 
2.34.1

[PATCH 2/2] arm64: perf: Mark all accessor functions inline

[Geert Uytterhoeven]

When just including <asm/arm_pmuv3.h>:

    arch/arm64/include/asm/arm_pmuv3.h:31:13: error: ‘write_pmevtypern’ defined but not used [-Werror=unused-function]
       31 | static void write_pmevtypern(int n, unsigned long val)
	  |             ^~~~~~~~~~~~~~~~
    arch/arm64/include/asm/arm_pmuv3.h:24:13: error: ‘write_pmevcntrn’ defined but not used [-Werror=unused-function]
       24 | static void write_pmevcntrn(int n, unsigned long val)
	  |             ^~~~~~~~~~~~~~~
    arch/arm64/include/asm/arm_pmuv3.h:16:22: error: ‘read_pmevcntrn’ defined but not used [-Werror=unused-function]
       16 | static unsigned long read_pmevcntrn(int n)
	  |                      ^~~~~~~~~~~~~~

Fix this by adding the missing "inline" keywords to the three accessor
functions that lack them.

Fixes: df29ddf4f04b00cf ("arm64: perf: Abstract system register accesses away")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/include/asm/arm_pmuv3.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
index d6b51deb7bf0ff2f..18dc2fb3d7b7b2d0 100644
--- a/arch/arm64/include/asm/arm_pmuv3.h
+++ b/arch/arm64/include/asm/arm_pmuv3.h
@@ -13,7 +13,7 @@
 
 #define RETURN_READ_PMEVCNTRN(n) \
 	return read_sysreg(pmevcntr##n##_el0)
-static unsigned long read_pmevcntrn(int n)
+static inline unsigned long read_pmevcntrn(int n)
 {
 	PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
 	return 0;
@@ -21,14 +21,14 @@ static unsigned long read_pmevcntrn(int n)
 
 #define WRITE_PMEVCNTRN(n) \
 	write_sysreg(val, pmevcntr##n##_el0)
-static void write_pmevcntrn(int n, unsigned long val)
+static inline void write_pmevcntrn(int n, unsigned long val)
 {
 	PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
 }
 
 #define WRITE_PMEVTYPERN(n) \
 	write_sysreg(val, pmevtyper##n##_el0)
-static void write_pmevtypern(int n, unsigned long val)
+static inline void write_pmevtypern(int n, unsigned long val)
 {
 	PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
 }
-- 
2.34.1

Re: [PATCH 2/2] arm64: perf: Mark all accessor functions inline

[Marc Zyngier]

On Mon, 08 May 2023 17:05:19 +0100,
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> When just including <asm/arm_pmuv3.h>:
> 
>     arch/arm64/include/asm/arm_pmuv3.h:31:13: error: ‘write_pmevtypern’ defined but not used [-Werror=unused-function]
>        31 | static void write_pmevtypern(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~~
>     arch/arm64/include/asm/arm_pmuv3.h:24:13: error: ‘write_pmevcntrn’ defined but not used [-Werror=unused-function]
>        24 | static void write_pmevcntrn(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~
>     arch/arm64/include/asm/arm_pmuv3.h:16:22: error: ‘read_pmevcntrn’ defined but not used [-Werror=unused-function]
>        16 | static unsigned long read_pmevcntrn(int n)
> 	  |                      ^~~~~~~~~~~~~~
> 
> Fix this by adding the missing "inline" keywords to the three accessor
> functions that lack them.

Looks reasonable to me. Each of these accessors is used exactly once
in the PMUv3 driver, so the inlining doesn't result in extra bloat.

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 2/2] arm64: perf: Mark all accessor functions inline

[Mark Rutland]

On Mon, May 08, 2023 at 06:05:19PM +0200, Geert Uytterhoeven wrote:
> When just including <asm/arm_pmuv3.h>:
> 
>     arch/arm64/include/asm/arm_pmuv3.h:31:13: error: ‘write_pmevtypern’ defined but not used [-Werror=unused-function]
>        31 | static void write_pmevtypern(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~~
>     arch/arm64/include/asm/arm_pmuv3.h:24:13: error: ‘write_pmevcntrn’ defined but not used [-Werror=unused-function]
>        24 | static void write_pmevcntrn(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~
>     arch/arm64/include/asm/arm_pmuv3.h:16:22: error: ‘read_pmevcntrn’ defined but not used [-Werror=unused-function]
>        16 | static unsigned long read_pmevcntrn(int n)
> 	  |                      ^~~~~~~~~~~~~~
> 
> Fix this by adding the missing "inline" keywords to the three accessor
> functions that lack them.
> 
> Fixes: df29ddf4f04b00cf ("arm64: perf: Abstract system register accesses away")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/include/asm/arm_pmuv3.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
> index d6b51deb7bf0ff2f..18dc2fb3d7b7b2d0 100644
> --- a/arch/arm64/include/asm/arm_pmuv3.h
> +++ b/arch/arm64/include/asm/arm_pmuv3.h
> @@ -13,7 +13,7 @@
>  
>  #define RETURN_READ_PMEVCNTRN(n) \
>  	return read_sysreg(pmevcntr##n##_el0)
> -static unsigned long read_pmevcntrn(int n)
> +static inline unsigned long read_pmevcntrn(int n)
>  {
>  	PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
>  	return 0;
> @@ -21,14 +21,14 @@ static unsigned long read_pmevcntrn(int n)
>  
>  #define WRITE_PMEVCNTRN(n) \
>  	write_sysreg(val, pmevcntr##n##_el0)
> -static void write_pmevcntrn(int n, unsigned long val)
> +static inline void write_pmevcntrn(int n, unsigned long val)
>  {
>  	PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
>  }
>  
>  #define WRITE_PMEVTYPERN(n) \
>  	write_sysreg(val, pmevtyper##n##_el0)
> -static void write_pmevtypern(int n, unsigned long val)
> +static inline void write_pmevtypern(int n, unsigned long val)
>  {
>  	PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
>  }
> -- 
> 2.34.1
> 

Re: [PATCH 1/2] ARM: perf: Mark all accessor functions inline

[Marc Zyngier]

On Mon, 08 May 2023 17:05:18 +0100,
Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> When just including <asm/arm_pmuv3.h>:
> 
>     arch/arm/include/asm/arm_pmuv3.h:110:13: error: ‘write_pmevtypern’ defined but not used [-Werror=unused-function]
>       110 | static void write_pmevtypern(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~~
>     arch/arm/include/asm/arm_pmuv3.h:103:13: error: ‘write_pmevcntrn’ defined but not used [-Werror=unused-function]
>       103 | static void write_pmevcntrn(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~
>     arch/arm/include/asm/arm_pmuv3.h:95:22: error: ‘read_pmevcntrn’ defined but not used [-Werror=unused-function]
>        95 | static unsigned long read_pmevcntrn(int n)
> 	  |                      ^~~~~~~~~~~~~~
> 
> Fix this by adding the missing "inline" keywords to the three accessor
> functions that lack them.
> 
> Fixes: 009d6dc87a568db6 ("ARM: perf: Allow the use of the PMUv3 driver on 32bit ARM")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] ARM: perf: Mark all accessor functions inline

[Mark Rutland]

On Mon, May 08, 2023 at 06:05:18PM +0200, Geert Uytterhoeven wrote:
> When just including <asm/arm_pmuv3.h>:
> 
>     arch/arm/include/asm/arm_pmuv3.h:110:13: error: ‘write_pmevtypern’ defined but not used [-Werror=unused-function]
>       110 | static void write_pmevtypern(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~~
>     arch/arm/include/asm/arm_pmuv3.h:103:13: error: ‘write_pmevcntrn’ defined but not used [-Werror=unused-function]
>       103 | static void write_pmevcntrn(int n, unsigned long val)
> 	  |             ^~~~~~~~~~~~~~~
>     arch/arm/include/asm/arm_pmuv3.h:95:22: error: ‘read_pmevcntrn’ defined but not used [-Werror=unused-function]
>        95 | static unsigned long read_pmevcntrn(int n)
> 	  |                      ^~~~~~~~~~~~~~
> 
> Fix this by adding the missing "inline" keywords to the three accessor
> functions that lack them.
> 
> Fixes: 009d6dc87a568db6 ("ARM: perf: Allow the use of the PMUv3 driver on 32bit ARM")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm/include/asm/arm_pmuv3.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
> index 78d3d4b82c6c2598..f4db3e75d75f024e 100644
> --- a/arch/arm/include/asm/arm_pmuv3.h
> +++ b/arch/arm/include/asm/arm_pmuv3.h
> @@ -92,7 +92,7 @@
>  
>  #define RETURN_READ_PMEVCNTRN(n) \
>  	return read_sysreg(PMEVCNTR##n)
> -static unsigned long read_pmevcntrn(int n)
> +static inline unsigned long read_pmevcntrn(int n)
>  {
>  	PMEVN_SWITCH(n, RETURN_READ_PMEVCNTRN);
>  	return 0;
> @@ -100,14 +100,14 @@ static unsigned long read_pmevcntrn(int n)
>  
>  #define WRITE_PMEVCNTRN(n) \
>  	write_sysreg(val, PMEVCNTR##n)
> -static void write_pmevcntrn(int n, unsigned long val)
> +static inline void write_pmevcntrn(int n, unsigned long val)
>  {
>  	PMEVN_SWITCH(n, WRITE_PMEVCNTRN);
>  }
>  
>  #define WRITE_PMEVTYPERN(n) \
>  	write_sysreg(val, PMEVTYPER##n)
> -static void write_pmevtypern(int n, unsigned long val)
> +static inline void write_pmevtypern(int n, unsigned long val)
>  {
>  	PMEVN_SWITCH(n, WRITE_PMEVTYPERN);
>  }
> -- 
> 2.34.1
> 

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

[Marc Zyngier]

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.

-- 
Without deviation from the norm, progress is not possible.

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

[Mark Rutland]

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

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

[Mark Rutland]

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

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

[Will Deacon]

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,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev