linux-kernel - Re: [PATCH 1/2] RISC-V: sbi: turn sbi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250612170632-59116c0a-6b38-4cd9-8df1-b193251d598c@linutronix.de>
Date: Thu, 12 Jun 2025 17:14:09 +0200
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Radim Krčmář <rkrcmar@...tanamicro.com>
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, 
	Atish Patra <atishp@...osinc.com>, Andrew Jones <ajones@...tanamicro.com>, 
	Clément Léger <cleger@...osinc.com>, Anup Patel <apatel@...tanamicro.com>
Subject: Re: [PATCH 1/2] RISC-V: sbi: turn sbi_ecall into variadic macro

On Thu, Jun 12, 2025 at 04:57:54PM +0200, Radim Krčmář wrote:
> Counting the arguments to sbi_ecall() and padding with zeros gets old
> pretty quick.  It's also harder to distinguish a tailing 0 argument and
> the padding.  The patch changes sbi_ecall() to accept anything between 1
> and 8 integer arguments.
> 
> Those who can count are also given sbi_ecall1() to sbi_ecall8(), which
> the variadic magic uses under the hood.  The error messages upon a
> programmer error are a bit hairy, as expected of macros, and the
> static_assert is there to improve them a bit.
> 
> The final goal would be avoid clobbering registers that are not used in
> the ecall, but we first have to fix the code generation around
> tracepoints if sbi_ecall is expected to be used in paths where
> performance is critical.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@...tanamicro.com>
> ---
>  arch/riscv/include/asm/sbi.h | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 341e74238aa0..c62db61bd018 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -10,6 +10,7 @@
>  #include <linux/types.h>
>  #include <linux/cpumask.h>
>  #include <linux/jump_label.h>
> +#include <linux/build_bug.h>
>  
>  #ifdef CONFIG_RISCV_SBI
>  enum sbi_ext_id {
> @@ -465,9 +466,40 @@ 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)	\
> +
> +#define sbi_ecall1(e) \
> +		__sbi_ecall(0, 0, 0, 0, 0, 0, 0, e)
> +#define sbi_ecall2(e, f) \
> +		__sbi_ecall(0, 0, 0, 0, 0, 0, f, e)
> +#define sbi_ecall3(e, f, a0) \
> +		__sbi_ecall(a0, 0, 0, 0, 0, 0, f, e)
> +#define sbi_ecall4(e, f, a0, a1) \
> +		__sbi_ecall(a0, a1, 0, 0, 0, 0, f, e)
> +#define sbi_ecall5(e, f, a0, a1, a2) \
> +		__sbi_ecall(a0, a1, a2, 0, 0, 0, f, e)
> +#define sbi_ecall6(e, f, a0, a1, a2, a3) \
> +		__sbi_ecall(a0, a1, a2, a3, 0, 0, f, e)
> +#define sbi_ecall7(e, f, a0, a1, a2, a3, a4) \
> +		__sbi_ecall(a0, a1, a2, a3, a4, 0, f, e)
> +#define sbi_ecall8(e, f, a0, a1, a2, a3, a4, a5) \
>  		__sbi_ecall(a0, a1, a2, a3, a4, a5, f, e)
>  
> +#define __sbi_count_args_magic(_, a, b, c, d, e, f, g, h, N, ...) N
> +#define __sbi_count_args(...) \
> +	__sbi_count_args_magic(_, ## __VA_ARGS__, 8, 7, 6, 5, 4, 3, 2, 1, 0)

This looks a lot like COUNT_ARGS() from include/linux/args.h.

> +#define __sbi_count_args2(...) \
> +	(sizeof((unsigned long[]){0, ## __VA_ARGS__}) / sizeof(unsigned long) - 1)
> +#define __sbi_concat_expanded(a, b) a ## b

... and CONCATENATE()

> +#define __sbi_concat(n) __sbi_concat_expanded(sbi_ecall, n)
> +
> +/* sbi_ecall selects the appropriate sbi_ecall1 to sbi_ecall8 */
> +#define sbi_ecall(...)  \
> +	({ \
> +		static_assert(__sbi_count_args2(__VA_ARGS__) <= 8); \

Why does this need to use __sbi_count_args2() ?

> +		__sbi_concat(__sbi_count_args(__VA_ARGS__)) \
> +				(__VA_ARGS__); \
> +	})
> +
>  #ifdef CONFIG_RISCV_SBI_V01
>  void sbi_console_putchar(int ch);
>  int sbi_console_getchar(void);
> -- 
> 2.49.0
>