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@ventanamicro.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)
+#define __sbi_count_args2(...) \
+	(sizeof((unsigned long[]){0, ## __VA_ARGS__}) / sizeof(unsigned long) - 1)
+#define __sbi_concat_expanded(a, b) a ## b
+#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); \
+		__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

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@ventanamicro.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
> 

2025-06-12T17:14:09+02:00, Thomas Weißschuh <thomas.weissschuh@linutronix.de>:
> 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@ventanamicro.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()

Thanks for pointing them out, I will use them in v2.

>> +#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() ?

When you go over the hardcoded maximum count in COUNT_ARGS, the macro
stops counting and returns the arguments instead.
The array approach in __sbi_count_args2() counts any number of
arguments, so it gives a much more understandable error.

I guess using COUNT_ARGS() and condemning programmers that overshoot the
15 counted arguments is better.  We don't really need a static_assert
then either, and v2 coud be just:

#define sbi_ecall(...) \
	CONCATENATE(sbi_ecall, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__);