qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: Change the default behavior if no -bios option is specified
@ 2020-05-01 12:19 Bin Meng
  2020-05-05 20:34 ` Alistair Francis
  0 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2020-05-01 12:19 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: 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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: Change the default behavior if no -bios option is specified
  2020-05-01 12:19 [PATCH] riscv: Change the default behavior if no -bios option is specified Bin Meng
@ 2020-05-05 20:34 ` Alistair Francis
  2020-05-05 22:28   ` Alistair Francis
  0 siblings, 1 reply; 7+ messages in thread
From: Alistair Francis @ 2020-05-05 20:34 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers,
	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
>
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: Change the default behavior if no -bios option is specified
  2020-05-05 20:34 ` Alistair Francis
@ 2020-05-05 22:28   ` Alistair Francis
  2020-05-06  1:34     ` Bin Meng
  0 siblings, 1 reply; 7+ messages in thread
From: Alistair Francis @ 2020-05-05 22:28 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers,
	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
> >
> >


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: Change the default behavior if no -bios option is specified
  2020-05-05 22:28   ` Alistair Francis
@ 2020-05-06  1:34     ` Bin Meng
  2020-05-06 20:53       ` Alistair Francis
  0 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2020-05-06  1:34 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers,
	Alistair Francis

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: Change the default behavior if no -bios option is specified
  2020-05-06  1:34     ` Bin Meng
@ 2020-05-06 20:53       ` Alistair Francis
  2020-05-21  1:37         ` Bin Meng
  0 siblings, 1 reply; 7+ messages in thread
From: Alistair Francis @ 2020-05-06 20:53 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers,
	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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: Change the default behavior if no -bios option is specified
  2020-05-06 20:53       ` Alistair Francis
@ 2020-05-21  1:37         ` Bin Meng
  2020-05-21 15:52           ` Alistair Francis
  0 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2020-05-21  1:37 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers,
	Alistair Francis

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] riscv: Change the default behavior if no -bios option is specified
  2020-05-21  1:37         ` Bin Meng
@ 2020-05-21 15:52           ` Alistair Francis
  0 siblings, 0 replies; 7+ messages in thread
From: Alistair Francis @ 2020-05-21 15:52 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers,
	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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-21 16:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-01 12:19 [PATCH] riscv: Change the default behavior if no -bios option is specified Bin Meng
2020-05-05 20:34 ` Alistair Francis
2020-05-05 22:28   ` Alistair Francis
2020-05-06  1:34     ` Bin Meng
2020-05-06 20:53       ` Alistair Francis
2020-05-21  1:37         ` Bin Meng
2020-05-21 15:52           ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).