[kvm-unit-tests PATCH 01/10] riscv: sbi: Mark known fwft failures as kfails

[Andrew Jones]

On Tue, Feb 25, 2025 at 11:25:13AM +0100, Andrew Jones wrote:
> On Tue, Feb 25, 2025 at 11:14:13AM +0100, Clément Léger wrote:
> > 
> > 
> > On 21/02/2025 16:55, Andrew Jones wrote:
> > > Until we fix opensbi mark these known failures as kfails so we can
> > > pass CI.
> > > 
> > > Signed-off-by: Andrew Jones <andrew.jones at linux.dev>
> > > ---
> > >  riscv/sbi-fwft.c | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
> > > index b10c147f22dd..19340d6bb48c 100644
> > > --- a/riscv/sbi-fwft.c
> > > +++ b/riscv/sbi-fwft.c
> > > @@ -63,11 +63,17 @@ static void fwft_check_base(void)
> > >  		struct sbiret ret;
> > >  
> > >  		ret = fwft_get_raw(BIT(32));
> > > -		sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM,
> > > +		if (ret.error == 0)
> > > +			report_kfail(true, false, "get feature with bit 32 set error: SBI_ERR_INVALID_PARAM");
> > > +		else
> > > +			sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM,
> > >  				    "get feature with bit 32 set error");
> > 
> > 
> > Hey Andrew,
> > 
> > I'm actually a bit confused here. As raised by Anup, the spec states that:
> > 
> > "Every SBI function should prefer unsigned long as the data type. It
> > keeps the specification simple and easily adaptable for all RISC-V ISA
> > types. In case the data is defined as 32bit wide, higher privilege
> > software must ensure that it only uses 32 bit data."
> > 
> > Does this means 64 bits value are truncated to 32 bits ?
> 
> Ah, yes. I had noticed that required truncation while doing some KVM fixes
> and patch 9 of this series is even adding a test for it for SUSP. I just
> didn't look closely enough at why this FWFT test was failing. The test
> should get SBI_SUCCESS, as it does, but it should also check that
> feature = BIT(32) behaves the same as features = 1.

Eh, '...as feature = 0 (MISALIGNED_EXC_DELEG)'. For now, I think we should
just drop the test, though, because we need a feature that, when present,
always allows enable/disable in order to test for a change in behavior.
While we may be able to toggle 0/1 for MISALIGNED_EXC_DELEG, we can't
count on traps getting enabled. I'll add ADUE tests and do a truncated
feature ID test for it.

Thanks,
drew