[PATCH 0/4] RISC-V: Dynamic ftrace support for RV32I

[PATCH 0/4] RISC-V: Dynamic ftrace support for RV32I

[Jamie Iles]

This series enables dynamic ftrace support for RV32I bringing it to 
parity with RV64I.  Most of the work is already there, this is largely 
just assembly fixes to handle register sizes, correct handling of the 
psABI calling convention and Kconfig change.

Validated with all ftrace boot time self test with qemu for RV32I and 
RV64I in addition to real tracing on an RV32I FPGA design.

Jamie Iles (4):
  RISC-V: use REG_S/REG_L for mcount
  RISC-V: reduce mcount save space on RV32
  RISC-V: preserve a1 in mcount
  RISC-V: enable dynamic ftrace for RV32I

 arch/riscv/Kconfig         | 10 ++++-----
 arch/riscv/kernel/mcount.S | 44 ++++++++++++++++++++------------------
 2 files changed, 28 insertions(+), 26 deletions(-)

-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/4] RISC-V: use REG_S/REG_L for mcount

[Jamie Iles]

In preparation for rv32i ftrace support, convert mcount routines to use
native sized loads/stores.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/riscv/kernel/mcount.S | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index 6d462681c9c0..9cf0904afd6d 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -15,8 +15,8 @@
 
 	.macro SAVE_ABI_STATE
 	addi	sp, sp, -16
-	sd	s0, 0(sp)
-	sd	ra, 8(sp)
+	REG_S	s0, 0(sp)
+	REG_S	ra, 8(sp)
 	addi	s0, sp, 16
 	.endm
 
@@ -26,22 +26,22 @@
 	 */
 	.macro SAVE_RET_ABI_STATE
 	addi	sp, sp, -32
-	sd	s0, 16(sp)
-	sd	ra, 24(sp)
-	sd	a0, 8(sp)
+	REG_S	s0, 16(sp)
+	REG_S	ra, 24(sp)
+	REG_S	a0, 8(sp)
 	addi	s0, sp, 32
 	.endm
 
 	.macro RESTORE_ABI_STATE
-	ld	ra, 8(sp)
-	ld	s0, 0(sp)
+	REG_L	ra, 8(sp)
+	REG_L	s0, 0(sp)
 	addi	sp, sp, 16
 	.endm
 
 	.macro RESTORE_RET_ABI_STATE
-	ld	ra, 24(sp)
-	ld	s0, 16(sp)
-	ld	a0, 8(sp)
+	REG_L	ra, 24(sp)
+	REG_L	s0, 16(sp)
+	REG_L	a0, 8(sp)
 	addi	sp, sp, 32
 	.endm
 
@@ -82,16 +82,16 @@ ENTRY(MCOUNT_NAME)
 	la	t4, ftrace_stub
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	la	t0, ftrace_graph_return
-	ld	t1, 0(t0)
+	REG_L	t1, 0(t0)
 	bne	t1, t4, do_ftrace_graph_caller
 
 	la	t3, ftrace_graph_entry
-	ld	t2, 0(t3)
+	REG_L	t2, 0(t3)
 	la	t6, ftrace_graph_entry_stub
 	bne	t2, t6, do_ftrace_graph_caller
 #endif
 	la	t3, ftrace_trace_function
-	ld	t5, 0(t3)
+	REG_L	t5, 0(t3)
 	bne	t5, t4, do_trace
 	ret
 
@@ -104,7 +104,7 @@ do_ftrace_graph_caller:
 	addi	a0, s0, -8
 	mv	a1, ra
 #ifdef HAVE_FUNCTION_GRAPH_FP_TEST
-	ld	a2, -16(s0)
+	REG_L	a2, -16(s0)
 #endif
 	SAVE_ABI_STATE
 	call	prepare_ftrace_return
@@ -117,7 +117,7 @@ do_ftrace_graph_caller:
  * (*ftrace_trace_function)(ra_to_caller, ra_to_caller_of_caller)
  */
 do_trace:
-	ld	a1, -8(s0)
+	REG_L	a1, -8(s0)
 	mv	a0, ra
 
 	SAVE_ABI_STATE
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/4] RISC-V: reduce mcount save space on RV32

[Jamie Iles]

For RV32 we can reduce the size of the ABI save+restore state by using
SZREG so that register stores are packed rather than on an 8 byte
boundary.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/riscv/kernel/mcount.S | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index 9cf0904afd6d..4166909ce7b3 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -15,8 +15,8 @@
 
 	.macro SAVE_ABI_STATE
 	addi	sp, sp, -16
-	REG_S	s0, 0(sp)
-	REG_S	ra, 8(sp)
+	REG_S	s0, 0*SZREG(sp)
+	REG_S	ra, 1*SZREG(sp)
 	addi	s0, sp, 16
 	.endm
 
@@ -25,24 +25,24 @@
 	 * register if a0 was not saved.
 	 */
 	.macro SAVE_RET_ABI_STATE
-	addi	sp, sp, -32
-	REG_S	s0, 16(sp)
-	REG_S	ra, 24(sp)
-	REG_S	a0, 8(sp)
+	addi	sp, sp, -4*SZREG
+	REG_S	s0, 2*SZREG(sp)
+	REG_S	ra, 3*SZREG(sp)
+	REG_S	a0, 1*SZREG(sp)
 	addi	s0, sp, 32
 	.endm
 
 	.macro RESTORE_ABI_STATE
-	REG_L	ra, 8(sp)
-	REG_L	s0, 0(sp)
-	addi	sp, sp, 16
+	REG_L	ra, 1*SZREG(sp)
+	REG_L	s0, 0*SZREG(sp)
+	addi	sp, sp, 2*SZREG
 	.endm
 
 	.macro RESTORE_RET_ABI_STATE
-	REG_L	ra, 24(sp)
-	REG_L	s0, 16(sp)
-	REG_L	a0, 8(sp)
-	addi	sp, sp, 32
+	REG_L	ra, 3*SZREG(sp)
+	REG_L	s0, 2*SZREG(sp)
+	REG_L	a0, 1*SZREG(sp)
+	addi	sp, sp, 4*SZREG
 	.endm
 
 ENTRY(ftrace_stub)
@@ -101,10 +101,10 @@ ENTRY(MCOUNT_NAME)
  * prepare_to_return(&ra_to_caller_of_caller, ra_to_caller)
  */
 do_ftrace_graph_caller:
-	addi	a0, s0, -8
+	addi	a0, s0, -SZREG
 	mv	a1, ra
 #ifdef HAVE_FUNCTION_GRAPH_FP_TEST
-	REG_L	a2, -16(s0)
+	REG_L	a2, -2*SZREG(s0)
 #endif
 	SAVE_ABI_STATE
 	call	prepare_ftrace_return
@@ -117,7 +117,7 @@ do_ftrace_graph_caller:
  * (*ftrace_trace_function)(ra_to_caller, ra_to_caller_of_caller)
  */
 do_trace:
-	REG_L	a1, -8(s0)
+	REG_L	a1, -SZREG(s0)
 	mv	a0, ra
 
 	SAVE_ABI_STATE
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/4] RISC-V: preserve a1 in mcount

[Jamie Iles]

The RISC-V ELF psABI states that both a0 and a1 are used for return
values so we should preserve them both in return_to_handler.  This is
especially important for RV32 for functions returning a 64-bit quantity
otherwise the return value can be corrupted and undefined behaviour
results.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/riscv/kernel/mcount.S | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
index 4166909ce7b3..5aa29509f047 100644
--- a/arch/riscv/kernel/mcount.S
+++ b/arch/riscv/kernel/mcount.S
@@ -29,6 +29,7 @@
 	REG_S	s0, 2*SZREG(sp)
 	REG_S	ra, 3*SZREG(sp)
 	REG_S	a0, 1*SZREG(sp)
+	REG_S	a1, 0*SZREG(sp)
 	addi	s0, sp, 32
 	.endm
 
@@ -42,6 +43,7 @@
 	REG_L	ra, 3*SZREG(sp)
 	REG_L	s0, 2*SZREG(sp)
 	REG_L	a0, 1*SZREG(sp)
+	REG_L	a1, 0*SZREG(sp)
 	addi	sp, sp, 4*SZREG
 	.endm
 
@@ -71,9 +73,9 @@ ENTRY(return_to_handler)
 	mv	a0, t6
 #endif
 	call	ftrace_return_to_handler
-	mv	a1, a0
+	mv	a2, a0
 	RESTORE_RET_ABI_STATE
-	jalr	a1
+	jalr	a2
 ENDPROC(return_to_handler)
 #endif
 
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 4/4] RISC-V: enable dynamic ftrace for RV32I

[Jamie Iles]

The RISC-V mcount function is now capable of supporting RV32I so make it
available in the kernel config.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 arch/riscv/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 6b48a3ae9843..5ae4f7ce2a05 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -129,6 +129,11 @@ config RISCV
 	select TRACE_IRQFLAGS_SUPPORT
 	select UACCESS_MEMCPY if !MMU
 	select ZONE_DMA32 if 64BIT
+	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
+	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
+	select HAVE_FUNCTION_GRAPH_TRACER
+	select HAVE_FUNCTION_TRACER if !XIP_KERNEL
 
 config ARCH_MMAP_RND_BITS_MIN
 	default 18 if 64BIT
@@ -274,11 +279,6 @@ config ARCH_RV64I
 	bool "RV64I"
 	select 64BIT
 	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
-	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
-	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
-	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
-	select HAVE_FUNCTION_GRAPH_TRACER
-	select HAVE_FUNCTION_TRACER if !XIP_KERNEL
 	select SWIOTLB if MMU
 
 endchoice
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/4] RISC-V: reduce mcount save space on RV32

[Andrew Jones]

On Thu, Oct 27, 2022 at 06:24:33PM +0100, Jamie Iles wrote:
> For RV32 we can reduce the size of the ABI save+restore state by using
> SZREG so that register stores are packed rather than on an 8 byte
> boundary.
> 
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
>  arch/riscv/kernel/mcount.S | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> index 9cf0904afd6d..4166909ce7b3 100644
> --- a/arch/riscv/kernel/mcount.S
> +++ b/arch/riscv/kernel/mcount.S
> @@ -15,8 +15,8 @@
>  
>  	.macro SAVE_ABI_STATE
>  	addi	sp, sp, -16
> -	REG_S	s0, 0(sp)
> -	REG_S	ra, 8(sp)
> +	REG_S	s0, 0*SZREG(sp)
> +	REG_S	ra, 1*SZREG(sp)
>  	addi	s0, sp, 16
>  	.endm

Shouldn't we only be adjusting sp by 2*SZREG?. Indeed RESTORE_ABI_STATE
below is getting modified that way.

>  
> @@ -25,24 +25,24 @@
>  	 * register if a0 was not saved.
>  	 */
>  	.macro SAVE_RET_ABI_STATE
> -	addi	sp, sp, -32
> -	REG_S	s0, 16(sp)
> -	REG_S	ra, 24(sp)
> -	REG_S	a0, 8(sp)
> +	addi	sp, sp, -4*SZREG
> +	REG_S	s0, 2*SZREG(sp)
> +	REG_S	ra, 3*SZREG(sp)
> +	REG_S	a0, 1*SZREG(sp)
>  	addi	s0, sp, 32

This should be 'addi s0, sp, -4*SZREG'

>  	.endm
>  
>  	.macro RESTORE_ABI_STATE
> -	REG_L	ra, 8(sp)
> -	REG_L	s0, 0(sp)
> -	addi	sp, sp, 16
> +	REG_L	ra, 1*SZREG(sp)
> +	REG_L	s0, 0*SZREG(sp)
> +	addi	sp, sp, 2*SZREG
>  	.endm
>  
>  	.macro RESTORE_RET_ABI_STATE
> -	REG_L	ra, 24(sp)
> -	REG_L	s0, 16(sp)
> -	REG_L	a0, 8(sp)
> -	addi	sp, sp, 32
> +	REG_L	ra, 3*SZREG(sp)
> +	REG_L	s0, 2*SZREG(sp)
> +	REG_L	a0, 1*SZREG(sp)
> +	addi	sp, sp, 4*SZREG
>  	.endm
>  
>  ENTRY(ftrace_stub)
> @@ -101,10 +101,10 @@ ENTRY(MCOUNT_NAME)
>   * prepare_to_return(&ra_to_caller_of_caller, ra_to_caller)
>   */
>  do_ftrace_graph_caller:
> -	addi	a0, s0, -8
> +	addi	a0, s0, -SZREG
>  	mv	a1, ra
>  #ifdef HAVE_FUNCTION_GRAPH_FP_TEST
> -	REG_L	a2, -16(s0)
> +	REG_L	a2, -2*SZREG(s0)
>  #endif
>  	SAVE_ABI_STATE
>  	call	prepare_ftrace_return
> @@ -117,7 +117,7 @@ do_ftrace_graph_caller:
>   * (*ftrace_trace_function)(ra_to_caller, ra_to_caller_of_caller)
>   */
>  do_trace:
> -	REG_L	a1, -8(s0)
> +	REG_L	a1, -SZREG(s0)
>  	mv	a0, ra
>  
>  	SAVE_ABI_STATE
> -- 
> 2.34.1
>

Thanks,
drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/4] RISC-V: use REG_S/REG_L for mcount

[Andrew Jones]

On Thu, Oct 27, 2022 at 06:24:32PM +0100, Jamie Iles wrote:
> In preparation for rv32i ftrace support, convert mcount routines to use
> native sized loads/stores.
> 
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
>  arch/riscv/kernel/mcount.S | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/4] RISC-V: preserve a1 in mcount

[Andrew Jones]

On Thu, Oct 27, 2022 at 06:24:34PM +0100, Jamie Iles wrote:
> The RISC-V ELF psABI states that both a0 and a1 are used for return
> values so we should preserve them both in return_to_handler.  This is
> especially important for RV32 for functions returning a 64-bit quantity
> otherwise the return value can be corrupted and undefined behaviour
> results.
> 
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
>  arch/riscv/kernel/mcount.S | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> index 4166909ce7b3..5aa29509f047 100644
> --- a/arch/riscv/kernel/mcount.S
> +++ b/arch/riscv/kernel/mcount.S
> @@ -29,6 +29,7 @@
>  	REG_S	s0, 2*SZREG(sp)
>  	REG_S	ra, 3*SZREG(sp)
>  	REG_S	a0, 1*SZREG(sp)
> +	REG_S	a1, 0*SZREG(sp)
>  	addi	s0, sp, 32
>  	.endm
>  
> @@ -42,6 +43,7 @@
>  	REG_L	ra, 3*SZREG(sp)
>  	REG_L	s0, 2*SZREG(sp)
>  	REG_L	a0, 1*SZREG(sp)
> +	REG_L	a1, 0*SZREG(sp)
>  	addi	sp, sp, 4*SZREG
>  	.endm
>  
> @@ -71,9 +73,9 @@ ENTRY(return_to_handler)
>  	mv	a0, t6
>  #endif
>  	call	ftrace_return_to_handler
> -	mv	a1, a0
> +	mv	a2, a0
>  	RESTORE_RET_ABI_STATE
> -	jalr	a1
> +	jalr	a2
>  ENDPROC(return_to_handler)
>  #endif
>  
> -- 
> 2.34.1
>


Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/4] RISC-V: reduce mcount save space on RV32

[Andrew Jones]

On Fri, Oct 28, 2022 at 04:49:25PM +0200, Andrew Jones wrote:
> On Thu, Oct 27, 2022 at 06:24:33PM +0100, Jamie Iles wrote:
> > For RV32 we can reduce the size of the ABI save+restore state by using
> > SZREG so that register stores are packed rather than on an 8 byte
> > boundary.
> > 
> > Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> > ---
> >  arch/riscv/kernel/mcount.S | 32 ++++++++++++++++----------------
> >  1 file changed, 16 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> > index 9cf0904afd6d..4166909ce7b3 100644
> > --- a/arch/riscv/kernel/mcount.S
> > +++ b/arch/riscv/kernel/mcount.S
> > @@ -15,8 +15,8 @@
> >  
> >  	.macro SAVE_ABI_STATE
> >  	addi	sp, sp, -16
> > -	REG_S	s0, 0(sp)
> > -	REG_S	ra, 8(sp)
> > +	REG_S	s0, 0*SZREG(sp)
> > +	REG_S	ra, 1*SZREG(sp)
> >  	addi	s0, sp, 16
> >  	.endm
> 
> Shouldn't we only be adjusting sp by 2*SZREG?. Indeed RESTORE_ABI_STATE
> below is getting modified that way.

It just occurred to me that leaving this adjustment as 16 is correct,
since we need to maintain 16-byte alignment, so the issue is below
in RESTORE_ABI_STATE where it isn't adding back 16.

> 
> >  
> > @@ -25,24 +25,24 @@
> >  	 * register if a0 was not saved.
> >  	 */
> >  	.macro SAVE_RET_ABI_STATE
> > -	addi	sp, sp, -32
> > -	REG_S	s0, 16(sp)
> > -	REG_S	ra, 24(sp)
> > -	REG_S	a0, 8(sp)
> > +	addi	sp, sp, -4*SZREG
> > +	REG_S	s0, 2*SZREG(sp)
> > +	REG_S	ra, 3*SZREG(sp)
> > +	REG_S	a0, 1*SZREG(sp)
> >  	addi	s0, sp, 32
> 
> This should be 'addi s0, sp, -4*SZREG'

And here I made copy+paste mistake, it should of course be 4*SZREG.

Thanks,
drew

> 
> >  	.endm
> >  
> >  	.macro RESTORE_ABI_STATE
> > -	REG_L	ra, 8(sp)
> > -	REG_L	s0, 0(sp)
> > -	addi	sp, sp, 16
> > +	REG_L	ra, 1*SZREG(sp)
> > +	REG_L	s0, 0*SZREG(sp)
> > +	addi	sp, sp, 2*SZREG
> >  	.endm
> >  
> >  	.macro RESTORE_RET_ABI_STATE
> > -	REG_L	ra, 24(sp)
> > -	REG_L	s0, 16(sp)
> > -	REG_L	a0, 8(sp)
> > -	addi	sp, sp, 32
> > +	REG_L	ra, 3*SZREG(sp)
> > +	REG_L	s0, 2*SZREG(sp)
> > +	REG_L	a0, 1*SZREG(sp)
> > +	addi	sp, sp, 4*SZREG
> >  	.endm
> >  
> >  ENTRY(ftrace_stub)
> > @@ -101,10 +101,10 @@ ENTRY(MCOUNT_NAME)
> >   * prepare_to_return(&ra_to_caller_of_caller, ra_to_caller)
> >   */
> >  do_ftrace_graph_caller:
> > -	addi	a0, s0, -8
> > +	addi	a0, s0, -SZREG
> >  	mv	a1, ra
> >  #ifdef HAVE_FUNCTION_GRAPH_FP_TEST
> > -	REG_L	a2, -16(s0)
> > +	REG_L	a2, -2*SZREG(s0)
> >  #endif
> >  	SAVE_ABI_STATE
> >  	call	prepare_ftrace_return
> > @@ -117,7 +117,7 @@ do_ftrace_graph_caller:
> >   * (*ftrace_trace_function)(ra_to_caller, ra_to_caller_of_caller)
> >   */
> >  do_trace:
> > -	REG_L	a1, -8(s0)
> > +	REG_L	a1, -SZREG(s0)
> >  	mv	a0, ra
> >  
> >  	SAVE_ABI_STATE
> > -- 
> > 2.34.1
> >
> 
> Thanks,
> drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv