* [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code. @ 2025-06-24 19:23 Jesse Taube 2025-06-25 8:33 ` Andrew Jones 2025-07-02 14:45 ` Andrew Jones 0 siblings, 2 replies; 5+ messages in thread From: Jesse Taube @ 2025-06-24 19:23 UTC (permalink / raw) To: kvm, kvm-riscv, linux-kselftest Cc: Clément Léger, Charlie Jenkins, Jesse Taube, Andrew Jones, James Raphael Tiovalen, Sean Christopherson, Cade Richard 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code. 2025-06-24 19:23 [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code Jesse Taube @ 2025-06-25 8:33 ` Andrew Jones 2025-06-25 14:31 ` Jesse Taube 2025-07-02 14:45 ` Andrew Jones 1 sibling, 1 reply; 5+ messages in thread From: Andrew Jones @ 2025-06-25 8:33 UTC (permalink / raw) To: Jesse Taube Cc: kvm, kvm-riscv, linux-kselftest, Clément Léger, Charlie Jenkins, James Raphael Tiovalen, Sean Christopherson, Cade Richard 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 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? 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. Thanks, drew ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code. 2025-06-25 8:33 ` Andrew Jones @ 2025-06-25 14:31 ` Jesse Taube 2025-06-25 15:47 ` Andrew Jones 0 siblings, 1 reply; 5+ messages in thread From: Jesse Taube @ 2025-06-25 14:31 UTC (permalink / raw) To: Andrew Jones Cc: kvm, kvm-riscv, linux-kselftest, Clément Léger, Charlie Jenkins, James Raphael Tiovalen, Sean Christopherson, Cade Richard 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_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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code. 2025-06-25 14:31 ` Jesse Taube @ 2025-06-25 15:47 ` Andrew Jones 0 siblings, 0 replies; 5+ messages in thread From: Andrew Jones @ 2025-06-25 15:47 UTC (permalink / raw) To: Jesse Taube Cc: kvm, kvm-riscv, linux-kselftest, Clément Léger, Charlie Jenkins, James Raphael Tiovalen, Sean Christopherson, Cade Richard On Wed, Jun 25, 2025 at 07:31:27AM -0700, Jesse Taube wrote: > 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_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. OK > > > 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? Yeah. I'd prefer to have configurable behavior controlled by environment variables, in order to allow the same binaries to run on multiple targets, but something like this will need to know what to do before variables are initialized. Hmm, maybe we can do both. A configure flag to set the default and an environment variable allowing it to be overridden. It should be possible to do that for the console as well. Thanks, drew ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code. 2025-06-24 19:23 [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code Jesse Taube 2025-06-25 8:33 ` Andrew Jones @ 2025-07-02 14:45 ` Andrew Jones 1 sibling, 0 replies; 5+ messages in thread From: Andrew Jones @ 2025-07-02 14:45 UTC (permalink / raw) To: Jesse Taube Cc: kvm, kvm-riscv, linux-kselftest, Clément Léger, Charlie Jenkins, James Raphael Tiovalen, Sean Christopherson, Cade Richard 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(-) > Merged. But I think a follow-on patch that makes it configurable is still a good idea. Thanks, drew ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-02 14:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-24 19:23 [kvm-unit-tests PATCH v2] riscv: lib: sbi_shutdown add pass/fail exit code Jesse Taube 2025-06-25 8:33 ` Andrew Jones 2025-06-25 14:31 ` Jesse Taube 2025-06-25 15:47 ` Andrew Jones 2025-07-02 14:45 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox