Re: [kvm-unit-tests PATCH 2/3] riscv: sbi: Add kfail versions of sbiret_report functions

["Clément Léger" <cleger@rivosinc.com>]

On 21/03/2025 17:54, Andrew Jones wrote:
> report_kfail is useful for SBI testing to allowing CI to PASS even
> when SBI implementations have known failures. Since sbiret_report
> functions are frequently used by SBI tests, make kfail versions of
> them too.
> 
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
>  riscv/sbi-tests.h | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
> index ddfad7fef293..d5c4ae709632 100644
> --- a/riscv/sbi-tests.h
> +++ b/riscv/sbi-tests.h
> @@ -39,7 +39,8 @@
>  #include <libcflat.h>
>  #include <asm/sbi.h>
>  
> -#define __sbiret_report(ret, expected_error, expected_value, has_value, expected_error_name, fmt, ...) ({	\
> +#define __sbiret_report(kfail, ret, expected_error, expected_value,						\
> +			has_value, expected_error_name, fmt, ...) ({						\
>  	long ex_err = expected_error;										\
>  	long ex_val = expected_value;										\
>  	bool has_val = !!(has_value);										\
> @@ -48,9 +49,9 @@
>  	bool pass;												\
>  														\
>  	if (has_val)												\
> -		pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__);						\
> +		pass = report_kfail(kfail, ch_err && ch_val, fmt, ##__VA_ARGS__);				\
>  	else													\
> -		pass = report(ch_err, fmt ": %s", ##__VA_ARGS__, expected_error_name);				\
> +		pass = report_kfail(kfail, ch_err, fmt ": %s", ##__VA_ARGS__, expected_error_name);		\
>  														\
>  	if (!pass && has_val)											\
>  		report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)",	\
> @@ -63,14 +64,23 @@
>  })
>  
>  #define sbiret_report(ret, expected_error, expected_value, ...) \
> -	__sbiret_report(ret, expected_error, expected_value, true, #expected_error, __VA_ARGS__)
> +	__sbiret_report(false, ret, expected_error, expected_value, true, #expected_error, __VA_ARGS__)
>  
>  #define sbiret_report_error(ret, expected_error, ...) \
> -	__sbiret_report(ret, expected_error, 0, false, #expected_error, __VA_ARGS__)
> +	__sbiret_report(false, ret, expected_error, 0, false, #expected_error, __VA_ARGS__)
>  
>  #define sbiret_check(ret, expected_error, expected_value) \
>  	sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
>  
> +#define sbiret_kfail(kfail, ret, expected_error, expected_value, ...) \
> +	__sbiret_report(kfail, ret, expected_error, expected_value, true, #expected_error, __VA_ARGS__)
> +
> +#define sbiret_kfail_error(kfail, ret, expected_error, ...) \
> +	__sbiret_report(kfail, ret, expected_error, 0, false, #expected_error, __VA_ARGS__)
> +
> +#define sbiret_check_kfail(kfail, ret, expected_error, expected_value) \
> +	sbiret_kfail(kfail, ret, expected_error, expected_value, "check sbi.error and sbi.value")
> +
>  static inline bool env_or_skip(const char *env)
>  {
>  	if (!getenv(env)) {

Hi Andrew,

I needed that as well in another test so:

Reviewed-by: Clément Léger <cleger@rivosinc.com>

Thanks,

Clément