[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZnYXoSDCeQK0Lcz8@ghost>
Date: Fri, 21 Jun 2024 17:15:29 -0700
From: Charlie Jenkins <charlie@...osinc.com>
To: Jisheng Zhang <jszhang@...nel.org>
Cc: Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Samuel Holland <samuel.holland@...ive.com>,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
Anton Blanchard <antonb@...storrent.com>,
Cyril Bur <cyrilbur@...storrent.com>
Subject: Re: [PATCH 1/6] riscv: Improve exception and system call latency
On Mon, Jun 17, 2024 at 01:05:48AM +0800, Jisheng Zhang wrote:
> From: Anton Blanchard <antonb@...storrent.com>
>
> Many CPUs implement return address branch prediction as a stack. The
> RISCV architecture refers to this as a return address stack (RAS). If
> this gets corrupted then the CPU will mispredict at least one but
> potentally many function returns.
>
> There are two issues with the current RISCV exception code:
>
> - We are using the alternate link stack (x5/t0) for the indirect branch
> which makes the hardware think this is a function return. This will
> corrupt the RAS.
>
> - We modify the return address of handle_exception to point to
> ret_from_exception. This will also corrupt the RAS.
>
> Testing the null system call latency before and after the patch:
>
> Visionfive2 (StarFive JH7110 / U74)
> baseline: 189.87 ns
> patched: 176.76 ns
>
> Lichee pi 4a (T-Head TH1520 / C910)
> baseline: 666.58 ns
> patched: 636.90 ns
>
> Just over 7% on the U74 and just over 4% on the C910.
>
> Signed-off-by: Anton Blanchard <antonb@...storrent.com>
> Signed-off-by: Cyril Bur <cyrilbur@...storrent.com>
Do you need to sign this off since you're sending this Jisheng?
> ---
> arch/riscv/kernel/entry.S | 17 ++++++++++-------
> arch/riscv/kernel/stacktrace.c | 4 ++--
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 68a24cf9481a..c933460ed3e9 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -88,7 +88,6 @@ SYM_CODE_START(handle_exception)
> call riscv_v_context_nesting_start
> #endif
> move a0, sp /* pt_regs */
> - la ra, ret_from_exception
>
> /*
> * MSB of cause differentiates between
> @@ -97,7 +96,8 @@ SYM_CODE_START(handle_exception)
> bge s4, zero, 1f
>
> /* Handle interrupts */
> - tail do_irq
> + call do_irq
> + j ret_from_exception
> 1:
> /* Handle other exceptions */
> slli t0, s4, RISCV_LGPTR
> @@ -105,11 +105,14 @@ SYM_CODE_START(handle_exception)
> la t2, excp_vect_table_end
> add t0, t1, t0
> /* Check if exception code lies within bounds */
> - bgeu t0, t2, 1f
> - REG_L t0, 0(t0)
> - jr t0
> -1:
> - tail do_trap_unknown
> + bgeu t0, t2, 3f
> + REG_L t1, 0(t0)
> +2: jalr t1
> + j ret_from_exception
> +3:
> +
The whitespace is odd here, but nonetheless:
Reviewed-by: Charlie Jenkins <charlie@...osinc.com>
> + la t1, do_trap_unknown
> + j 2b
> SYM_CODE_END(handle_exception)
> ASM_NOKPROBE(handle_exception)
>
> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
> index 528ec7cc9a62..5eb3d135b717 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
> @@ -16,7 +16,7 @@
>
> #ifdef CONFIG_FRAME_POINTER
>
> -extern asmlinkage void ret_from_exception(void);
> +extern asmlinkage void handle_exception(void);
>
> static inline int fp_is_valid(unsigned long fp, unsigned long sp)
> {
> @@ -70,7 +70,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
> fp = frame->fp;
> pc = ftrace_graph_ret_addr(current, NULL, frame->ra,
> &frame->ra);
> - if (pc == (unsigned long)ret_from_exception) {
> + if (pc == (unsigned long)handle_exception) {
> if (unlikely(!__kernel_text_address(pc) || !fn(arg, pc)))
> break;
>
> --
> 2.43.0
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Powered by blists - more mailing lists