On Wed, Jun 25, 2025 at 1:33 AM Andrew Jones <andrew.jones@xxxxxxxxx> wrote:
>
> On Tue, Jun 24, 2025 at 12:23:17PM -0700, Jesse Taube wrote:
> > When exiting it may be useful for the sbi implementation to know if
> > kvm-unit-tests passed or failed.
> > Add exit code to sbi_shutdown, and use it in exit() to pass
> > success/failure (0/1) to sbi.
> >
> > Signed-off-by: Jesse Taube <jesse@xxxxxxxxxxxx>
> > ---
> > lib/riscv/asm/sbi.h | 2 +-
> > lib/riscv/io.c | 2 +-
> > lib/riscv/sbi.c | 4 ++--
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
> > index a5738a5c..de11c109 100644
> > --- a/lib/riscv/asm/sbi.h
> > +++ b/lib/riscv/asm/sbi.h
> > @@ -250,7 +250,7 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
> > unsigned long arg3, unsigned long arg4,
> > unsigned long arg5);
> >
> > -void sbi_shutdown(void);
> > +void sbi_shutdown(unsigned int code);
> > struct sbiret sbi_hart_start(unsigned long hartid, unsigned long entry, unsigned long sp);
> > struct sbiret sbi_hart_stop(void);
> > struct sbiret sbi_hart_get_status(unsigned long hartid);
> > diff --git a/lib/riscv/io.c b/lib/riscv/io.c
> > index fb40adb7..0bde25d4 100644
> > --- a/lib/riscv/io.c
> > +++ b/lib/riscv/io.c
> > @@ -150,7 +150,7 @@ void halt(int code);
> > void exit(int code)
> > {
> > printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
> > - sbi_shutdown();
> > + sbi_shutdown(!!code);
> > halt(code);
> > __builtin_unreachable();
> > }
> > diff --git a/lib/riscv/sbi.c b/lib/riscv/sbi.c
> > index 2959378f..9dd11e9d 100644
> > --- a/lib/riscv/sbi.c
> > +++ b/lib/riscv/sbi.c
> > @@ -107,9 +107,9 @@ struct sbiret sbi_sse_inject(unsigned long event_id, unsigned long hart_id)
> > return sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_INJECT, event_id, hart_id, 0, 0, 0, 0);
> > }
> >
> > -void sbi_shutdown(void)
> > +void sbi_shutdown(unsigned int code)
> > {
> > - sbi_ecall(SBI_EXT_SRST, 0, 0, 0, 0, 0, 0, 0);
> > + sbi_ecall(SBI_EXT_SRST, 0, 0, code, 0, 0, 0, 0);
> > puts("SBI shutdown failed!\n");
> > }
> >
> > --
> > 2.43.0
> >
>
> I enhanced the commit message, changed the parameter to a boolean, and
> applied to riscv/sbi
>
> https://gitlab.com/jones-drew/kvm-unit-tests/-/commits/riscv/sbi
>
> but I'm having some second thoughts on it. It looks like opensbi and the
> two KVM VMMs I looked at (QEMU and kvmtool) all currently ignore this
> parameter and we don't know what they might choose to do if they stop
> ignoring it.
For the default syscon QEMU doesn't ignore it and exits with the exit
code given.
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/misc/sifive_test.c?ref_type=heads#L44
Both RustSBI and BBL implement the sifive_test device correctly and
provide an exit code,
OpenSBI ignores it, though it is trivial to add it.
https://github.com/rustsbi/rustsbi/blob/main/prototyper/prototyper/src/platform/reset.rs#L21
https://github.com/riscv-software-src/riscv-pk/blob/master/machine/finisher.c#L15
> For example, they could choose to hang, rather than complete
> the shutdown when they see a "system failure" reason. It may make sense
> to indicate system failure if the test aborts, since, in those cases,
> something unexpected with the testing occurred. However, successfully
> running tests which find and report failures isn't unexpected, so it
> shouldn't raise an alarm to the SBI implementation in those cases.
>
> Do you already have a usecase for this in mind?
Yes making CI easier, as the exit code is passed to QEMU rather than
having to parse the text.
> If so, we could make
> the behavior optional to enable that use case and use cases like it
> but we'd keep that behavior off by default to avoid problems with SBI
> implementations that do things with the "system failure" information we'd
> rather they not do.
Sure, do you want it to be a configure flag like --console?
Thanks,
Jesse Taube
>
> Thanks,
> drew
