On Wed, Jun 25, 2025 at 1:33 AM Andrew Jones andrew.jones@linux.dev 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@rivosinc.com
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...
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/platf... https://github.com/riscv-software-src/riscv-pk/blob/master/machine/finisher....
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