Re: [EXT] Re: [PATCH] hw/riscv: virt: Warn the user if -bios is provided when using KVM

[Ralf Ramsauer]

On 31/03/2022 02:11, Alistair Francis wrote:
> On Thu, Mar 24, 2022 at 7:08 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Wed, Mar 23, 2022 at 06:13:46PM +0100, Ralf Ramsauer wrote:
> > > The -bios option is silently ignored if used in combination with -enable-kvm.
> > > The reason is that the machine starts in S-Mode, and the bios typically runs in
> > > M-Mode.
> > >
> > > Warn the user that the bios won't be loaded.
> > >
> > > Signed-off-by: Ralf Ramsauer <ralf.ramsauer@oth-regensburg.de>
> > > ---
> > >   hw/riscv/virt.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > >
> > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > > index 4496a15346..a4d13114ee 100644
> > > --- a/hw/riscv/virt.c
> > > +++ b/hw/riscv/virt.c
> > > @@ -1312,6 +1312,9 @@ static void virt_machine_init(MachineState *machine)
> > >        * when KVM is enabled.
> > >        */
> > >       if (kvm_enabled()) {
> > > +        if (machine->firmware && strcmp(machine->firmware, "none"))
> > > +            warn_report("BIOS is not supported in combination with KVM. "
> > > +                        "Ignoring BIOS.");
> >
> > If the usage scenario isn't supportable, then ultimately we should be
> > raising an error and immediately exiting.
> >
> > If you know of common usage that is already mistakenly passing -bios,
> > then we could start with a warning and list it as deprecated, then
> > change to an error_report 2 releases later. If we don't thing people
> > are often mistakenly passing -bios, then go straight for error_report
> > and exit.
>
> That's a good point. The original thinking was that we did support
> -bios and so we should warn the user that it's unlikely they want to
> use it. This would still allow S mode UEFI loaders to be used (they
> don't exist today).
>
> Considering we are currently just ignoring the option I agree it's
> better to report an error.
>
> Do you mind sending a v2 Ralf?

Yes, will return with another revision. Anyway, I'll choose to exit 
immediately, as I doubt that there are any non-development users of this 
particular feature (RISCV/Qemu + KVM) due to the lack of physical hardware.

Thanks
  Ralf

> Alistair
>
> > >           g_free(machine->firmware);
> > >           machine->firmware = g_strdup("none");
> > >       }
> > > --
> > > 2.32.0
> >
> > With regards,
> > Daniel
> > --
> > |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> > |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> > |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|