diff mbox series

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

Message ID 1588335545-649-1-git-send-email-bmeng.cn@gmail.com
State New
Headers show
Series riscv: Change the default behavior if no -bios option is specified | expand

Commit Message

Bin Meng May 1, 2020, 12:19 p.m. UTC
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(-)

Comments

Alistair Francis May 5, 2020, 8:34 p.m. UTC | #1
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
>
>
Alistair Francis May 5, 2020, 10:28 p.m. UTC | #2
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
> >
> >
Bin Meng May 6, 2020, 1:34 a.m. UTC | #3
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
Alistair Francis May 6, 2020, 8:53 p.m. UTC | #4
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
Bin Meng May 21, 2020, 1:37 a.m. UTC | #5
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
Alistair Francis May 21, 2020, 3:52 p.m. UTC | #6
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
diff mbox series

Patch

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")) {