[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a4d340f0-960f-47bc-b45c-f89053526a22@rivosinc.com>
Date: Fri, 22 Mar 2024 11:57:49 -0700
From: Atish Patra <atishp@...osinc.com>
To: Alexandre Ghiti <alexghiti@...osinc.com>,
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
Subject: Re: [PATCH] riscv: Improve sbi_ecall() code generation by reordering
arguments
On 3/22/24 04:26, Alexandre Ghiti wrote:
> The sbi_ecall() function arguments are not in the same order as the
> ecall arguments, so we end up re-ordering the registers before the
> ecall which is useless and costly.
>
> So simply reorder the arguments in the same way as expected by ecall.
> Instead of reordering directly the arguments of sbi_ecall(), use a proxy
> macro since the current ordering is more natural.
>
> Before:
>
> Dump of assembler code for function sbi_ecall:
> 0xffffffff800085e0 <+0>: add sp,sp,-32
> 0xffffffff800085e2 <+2>: sd s0,24(sp)
> 0xffffffff800085e4 <+4>: mv t1,a0
> 0xffffffff800085e6 <+6>: add s0,sp,32
> 0xffffffff800085e8 <+8>: mv t3,a1
> 0xffffffff800085ea <+10>: mv a0,a2
> 0xffffffff800085ec <+12>: mv a1,a3
> 0xffffffff800085ee <+14>: mv a2,a4
> 0xffffffff800085f0 <+16>: mv a3,a5
> 0xffffffff800085f2 <+18>: mv a4,a6
> 0xffffffff800085f4 <+20>: mv a5,a7
> 0xffffffff800085f6 <+22>: mv a6,t3
> 0xffffffff800085f8 <+24>: mv a7,t1
> 0xffffffff800085fa <+26>: ecall
> 0xffffffff800085fe <+30>: ld s0,24(sp)
> 0xffffffff80008600 <+32>: add sp,sp,32
> 0xffffffff80008602 <+34>: ret
>
> After:
>
> Dump of assembler code for function __sbi_ecall:
> 0xffffffff8000b6b2 <+0>: add sp,sp,-32
> 0xffffffff8000b6b4 <+2>: sd s0,24(sp)
> 0xffffffff8000b6b6 <+4>: add s0,sp,32
> 0xffffffff8000b6b8 <+6>: ecall
> 0xffffffff8000b6bc <+10>: ld s0,24(sp)
> 0xffffffff8000b6be <+12>: add sp,sp,32
> 0xffffffff8000b6c0 <+14>: ret
>
Nice!
> Signed-off-by: Alexandre Ghiti <alexghiti@...osinc.com>
> ---
> arch/riscv/include/asm/sbi.h | 10 ++++++----
> arch/riscv/kernel/sbi.c | 10 +++++-----
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 6e68f8dff76b..9041b009d3b5 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -292,10 +292,12 @@ struct sbiret {
> };
>
> void sbi_init(void);
> -struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
> - unsigned long arg1, unsigned long arg2,
> - unsigned long arg3, unsigned long arg4,
> - unsigned long arg5);
> +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1,
> + unsigned long arg2, unsigned long arg3,
> + unsigned long arg4, unsigned long arg5,
> + int fid, int ext);
> +#define sbi_ecall(e, f, a0, a1, a2, a3, a4, a5) \
> + __sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
>
> #ifdef CONFIG_RISCV_SBI_V01
> void sbi_console_putchar(int ch);
> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> index e66e0999a800..5719fa03c3d1 100644
> --- a/arch/riscv/kernel/sbi.c
> +++ b/arch/riscv/kernel/sbi.c
> @@ -24,10 +24,10 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask,
> unsigned long start, unsigned long size,
> unsigned long arg4, unsigned long arg5) __ro_after_init;
>
> -struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
> - unsigned long arg1, unsigned long arg2,
> - unsigned long arg3, unsigned long arg4,
> - unsigned long arg5)
> +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1,
> + unsigned long arg2, unsigned long arg3,
> + unsigned long arg4, unsigned long arg5,
> + int fid, int ext)
> {
> struct sbiret ret;
>
> @@ -48,7 +48,7 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
>
> return ret;
> }
> -EXPORT_SYMBOL(sbi_ecall);
> +EXPORT_SYMBOL(__sbi_ecall);
>
> int sbi_err_map_linux_errno(int err)
> {
Reviewed-by: Atish Patra <atishp@...osinc.com>
Powered by blists - more mailing lists