Re: [PATCH] riscv: Change the default behavior if no -bios option is specified

[Alistair Francis <alistair23@gmail.com>]

On Tue, May 5, 2020 at 1:34 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, May 1, 2020 at 5:21 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Per QEMU deprecated doc, QEMU 4.1 introduced support for the -bios
> > option in QEMU for RISC-V for the virt machine and sifive_u machine.
> > The default behavior has been that QEMU does not automatically load
> > any firmware if no -bios option is included.
> >
> > Now 2 releases passed, it's time to change the default behavior to
> > load the default OpenSBI firmware automatically. The firmware is
> > included with the QEMU release and no user interaction is required.
> > All a user needs to do is specify the kernel they want to boot with
> > the -kernel option.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> Thanks!
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Applied to the RISC-V tree.

This fails `make check`

qemu-system-riscv64: Unable to load the RISC-V firmware
"opensbi-riscv64-spike-fw_jump.elf"
Broken pipe
/scratch/alistair/software/qemu/tests/qtest/libqtest.c:166:
kill_qemu() tried to terminate QEMU process but encountered exit
status 1 (expected 0)
ERROR - too few tests run (expected 7, got 2)
make: *** [/scratch/alistair/software/qemu/tests/Makefile.include:637:
check-qtest-riscv64] Error 1

Alistair

>
> Alistair
>
> > ---
> >
> >  hw/riscv/boot.c | 31 ++++---------------------------
> >  1 file changed, 4 insertions(+), 27 deletions(-)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 726300a..b76b2f3 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -41,34 +41,11 @@ void riscv_find_and_load_firmware(MachineState *machine,
> >  {
> >      char *firmware_filename = NULL;
> >
> > -    if (!machine->firmware) {
> > +    if ((!machine->firmware) || (!strcmp(machine->firmware, "default"))) {
> >          /*
> > -         * The user didn't specify -bios.
> > -         * At the moment we default to loading nothing when this hapens.
> > -         * In the future this defaul will change to loading the prebuilt
> > -         * OpenSBI firmware. Let's warn the user and then continue.
> > -        */
> > -        if (!qtest_enabled()) {
> > -            warn_report("No -bios option specified. Not loading a firmware.");
> > -            warn_report("This default will change in a future QEMU release. " \
> > -                        "Please use the -bios option to avoid breakages when "\
> > -                        "this happens.");
> > -            warn_report("See QEMU's deprecation documentation for details.");
> > -        }
> > -        return;
> > -    }
> > -
> > -    if (!strcmp(machine->firmware, "default")) {
> > -        /*
> > -         * The user has specified "-bios default". That means we are going to
> > -         * load the OpenSBI binary included in the QEMU source.
> > -         *
> > -         * We can't load the binary by default as it will break existing users
> > -         * as users are already loading their own firmware.
> > -         *
> > -         * Let's try to get everyone to specify the -bios option at all times,
> > -         * so then in the future we can make "-bios default" the default option
> > -         * if no -bios option is set without breaking anything.
> > +         * The user didn't specify -bios, or has specified "-bios default".
> > +         * That means we are going to load the OpenSBI binary included in
> > +         * the QEMU source.
> >           */
> >          firmware_filename = riscv_find_firmware(default_machine_firmware);
> >      } else if (strcmp(machine->firmware, "none")) {
> > --
> > 2.7.4
> >
> >