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

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

[Bin Meng]

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>
---

 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

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

[Alistair Francis]

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.

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
>
>

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

[Alistair Francis]

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
> >
> >

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

[Bin Meng]

Hi Alistair,

On Wed, May 6, 2020 at 6:37 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> 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
>

Please apply this patch to fix the "make check" as well.

[5/5] riscv: Suppress the error report for QEMU testing with
riscv_find_firmware()
http://patchwork.ozlabs.org/project/qemu-devel/patch/1588348254-7241-6-git-send-email-bmeng.cn@gmail.com/

Regards,
Bin

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

[Alistair Francis]

On Tue, May 5, 2020 at 6:34 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Wed, May 6, 2020 at 6:37 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > 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
> >
>
> Please apply this patch to fix the "make check" as well.
>
> [5/5] riscv: Suppress the error report for QEMU testing with
> riscv_find_firmware()
> http://patchwork.ozlabs.org/project/qemu-devel/patch/1588348254-7241-6-git-send-email-bmeng.cn@gmail.com/

In future please send all related patches in a single series.

I have applied those two patches.

Alistair

>
> Regards,
> Bin

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

[Bin Meng]

Hi Alistair,

On Thu, May 7, 2020 at 5:02 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, May 5, 2020 at 6:34 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Alistair,
> >
> > On Wed, May 6, 2020 at 6:37 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > 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
> > >
> >
> > Please apply this patch to fix the "make check" as well.
> >
> > [5/5] riscv: Suppress the error report for QEMU testing with
> > riscv_find_firmware()
> > http://patchwork.ozlabs.org/project/qemu-devel/patch/1588348254-7241-6-git-send-email-bmeng.cn@gmail.com/
>
> In future please send all related patches in a single series.
>
> I have applied those two patches.

I checked https://github.com/alistair23/qemu/ but could not find where
these 2 patches applied. Just make sure I was not looking at the wrong
place?

Regards,
Bin

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

[Alistair Francis]

On Wed, May 20, 2020 at 6:37 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Thu, May 7, 2020 at 5:02 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, May 5, 2020 at 6:34 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Alistair,
> > >
> > > On Wed, May 6, 2020 at 6:37 AM Alistair Francis <alistair23@gmail.com> wrote:
> > > >
> > > > 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
> > > >
> > >
> > > Please apply this patch to fix the "make check" as well.
> > >
> > > [5/5] riscv: Suppress the error report for QEMU testing with
> > > riscv_find_firmware()
> > > http://patchwork.ozlabs.org/project/qemu-devel/patch/1588348254-7241-6-git-send-email-bmeng.cn@gmail.com/
> >
> > In future please send all related patches in a single series.
> >
> > I have applied those two patches.
>
> I checked https://github.com/alistair23/qemu/ but could not find where
> these 2 patches applied. Just make sure I was not looking at the wrong
> place?

That's the right place, I just hadn't pushed it there yet.

I have an internal Git instance that does all the testing so I usually
push it there.

Alistair

>
> Regards,
> Bin