[kvm-unit-tests PATCH 3/3] riscv: sbi: Add sbiret_report_error

[Andrew Jones]

On Mon, Feb 03, 2025 at 10:00:39AM +0100, Clément Léger wrote:
> 
> 
> On 29/01/2025 18:01, Andrew Jones wrote:
> > An sbiret value may be unspecified in the case of errors, and even in
> > the case of success when the function doesn't return anything.
> > Provide an sbiret report function that only checks sbiret.error.
> > 
> > Signed-off-by: Andrew Jones <andrew.jones at linux.dev>
> > ---
> >  riscv/sbi-tests.h | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
> > index 2bdc9440d82d..f6e646a79dbe 100644
> > --- a/riscv/sbi-tests.h
> > +++ b/riscv/sbi-tests.h
> > @@ -36,20 +36,29 @@
> >  #ifndef __ASSEMBLY__
> >  #include <asm/sbi.h>
> >  
> > -#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({						\
> > +#define __sbiret_report(ret, expected_error, expected_value, has_value, fmt, ...) ({				\
> >  	long ex_err = expected_error;										\
> >  	long ex_val = expected_value;										\
> >  	bool ch_err = (ret)->error == ex_err;									\
> >  	bool ch_val = (ret)->value == ex_val;									\
> > -	bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__);						\
> > +	bool pass = report(ch_err && (!(has_value) || ch_val), fmt, ##__VA_ARGS__);				\
> 
> Nit: I would have moved the has_value check in the ch_val bool but I'm
> good with both way.

Thanks for the review. I'll spin a v2 with this change and also look into
outputting the expected error code.

drew

> 
> >  														\
> > -	if (!pass)												\
> > +	if (!pass && (has_value))										\
> >  		report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)",	\
> >  			    ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value);				\
> > +	else if (!pass)												\
> > +		report_info(fmt ": expected (error: %ld), received: (error: %ld)",				\
> > +			    ##__VA_ARGS__, ex_err, (ret)->error);						\
> >  														\
> >  	pass;													\
> >  })
> >  
> > +#define sbiret_report(ret, expected_error, expected_value, ...) \
> > +	__sbiret_report(ret, expected_error, expected_value, true, __VA_ARGS__)
> > +
> > +#define sbiret_report_error(ret, expected_error, ...) \
> > +	__sbiret_report(ret, expected_error, 0, false, __VA_ARGS__)
> > +
> >  #define sbiret_check(ret, expected_error, expected_value) \
> >  	sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
> >  
> 
> Tested-by: Clément Léger <cleger at rivosinc.com>
> Reviewed-by: Clément Léger <cleger at rivosinc.com>
> 
> Thanks,
> 
> Clément
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv