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

[Ralf Ramsauer]

On 23/03/2022 22:05, Alistair Francis wrote:
> On Thu, Mar 24, 2022 at 3:13 AM Ralf Ramsauer
> <ralf.ramsauer@oth-regensburg.de> 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>
>
> Thanks for the patch.
>
> > ---
> >   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"))
>
> You need curly braces around the if statement. You can run checkpatch
> on the patch to catch issues like this with:
>
> git show | ./scripts/checkpatch.pl -

total: 0 errors, 0 warnings, 9 lines checked

Your patch has no obvious style problems and is ready for submission.

> > +            warn_report("BIOS is not supported in combination with KVM. "
> > +                        "Ignoring BIOS.");
>
> Maybe say
>
> "Machine mode firmware is not supported in combination with KVM. Ignoring -bios"

Anyway, will provide a V2 with an improved warning message.

Thanks
  Ralf

> Alistair
>
> >           g_free(machine->firmware);
> >           machine->firmware = g_strdup("none");
> >       }
> > --
> > 2.32.0