On Mon, Jul 07, 2025 at 08:57:37AM -0700, Jesse Taube wrote:
> On Fri, Jul 4, 2025 at 1:17 AM Andrew Jones <andrew.jo...@linux.dev> wrote:
> >
> > On Thu, Jul 03, 2025 at 06:36:00AM -0700, Jesse Taube wrote:
> > > Add --[enable|disable]-sbi-exit-code to configure script.
> > > With the default value disabled.
> > > Add a check for SBI_PASS_EXIT_CODE in the environment, so that passing
> > > of the test status is configurable from both the
> > > environment and the configure script
> > >
> > > Signed-off-by: Jesse Taube <je...@rivosinc.com>
> > > ---
> > > configure | 11 +++++++++++
> > > lib/riscv/io.c | 12 +++++++++++-
> > > 2 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/configure b/configure
> > > index 20bf5042..7c949bdc 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -67,6 +67,7 @@ earlycon=
> > > console=
> > > efi=
> > > efi_direct=
> > > +sbi_exit_code=0
> > > target_cpu=
> > >
> > > # Enable -Werror by default for git repositories only (i.e. developer
> > > builds)
> > > @@ -141,6 +142,9 @@ usage() {
> > > system and run from the UEFI shell.
> > > Ignored when efi isn't enabled
> > > and defaults to enabled when efi is
> > > enabled for riscv64.
> > > (arm64 and riscv64 only)
> > > + --[enable|disable]-sbi-exit-code
> > > + Enable or disable sending pass/fail exit
> > > code to SBI SRST.
> > > + (disabled by default, riscv only)
> > > EOF
> > > exit 1
> > > }
> > > @@ -236,6 +240,12 @@ while [[ $optno -le $argc ]]; do
> > > --disable-efi-direct)
> > > efi_direct=n
> > > ;;
> > > + --enable-sbi-exit-code)
> > > + sbi_exit_code=1
> > > + ;;
> > > + --disable-sbi-exit-code)
> > > + sbi_exit_code=0
> > > + ;;
> > > --enable-werror)
> > > werror=-Werror
> > > ;;
> > > @@ -551,6 +561,7 @@ EOF
> > > elif [ "$arch" = "riscv32" ] || [ "$arch" = "riscv64" ]; then
> > > echo "#define CONFIG_UART_EARLY_BASE ${uart_early_addr}" >>
> > > lib/config.h
> > > [ "$console" = "sbi" ] && echo "#define CONFIG_SBI_CONSOLE" >>
> > > lib/config.h
> > > + echo "#define CONFIG_SBI_EXIT_CODE ${sbi_exit_code}" >> lib/config.h
> > > echo >> lib/config.h
> > > fi
> > > echo "#endif" >> lib/config.h
> > > diff --git a/lib/riscv/io.c b/lib/riscv/io.c
> > > index b1163404..0e666009 100644
> > > --- a/lib/riscv/io.c
> > > +++ b/lib/riscv/io.c
> > > @@ -162,8 +162,18 @@ void halt(int code);
> > >
> > > void exit(int code)
> > > {
> > > + char *s = getenv("SBI_PASS_EXIT_CODE");
> > > + bool pass_exit = CONFIG_SBI_EXIT_CODE;
> >
> > This is the first case of what may become more common - a config variable
> > which also has an env override. I think it may be good convention to
> > name them the same, i.e. the env name would also be CONFIG_SBI_EXIT_CODE,
> > unless you think that would be confusing for some reason?
>
> I changed the name because the configure option seemed very long.
> I will make them both SBI_EXIT_CODE.
We need the CONFIG_ part on the config name in order keep it consistent
with other configs, so I think we just need a long name for both.
> Should I add a macro to simplify
> future uses
> of a config variable which also has an env override.
Sounds good to me.
>
> >
> > > +
> > > printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
> > > - sbi_shutdown(code == 0);
> > > +
> > > + if (s)
> > > + pass_exit = (*s == '1' || *s == 'y' || *s == 'Y');
> >
> > We now have this logic in four places[1]. I think it's time we factor it,
> > and it's counterpart "!(s && (*s == '0' || *s == 'n' || *s == 'N'))"
> > into a couple helper macros. I'm not sure where the best place for
> > those macros to live is, though. I guess libcflat.h, but we really
> > ought to split that thing apart someday...
>
> Is lib/argv.h an ok place?
That could work.
> getenv is defined in lib/string.c which is interesting. I wonder if it
> could be moved to lib/argv.c?
getenv is libc and in lives in stdlib in libc, so we should move our
prototype to lib/stdlib.h from libcflat.h, but we don't have a stdlib.c
file so string.c and argv.c are both "wrong", but at least string.c
also has all the functions getenv depends on locally...
Thanks,
drew
