Re: [PATCH v5 1/6] kvm: Introduce kvm_arch_get_default_type hook

[Philippe Mathieu-Daudé]

On 4/8/23 19:38, Peter Maydell wrote:
> On Fri, 4 Aug 2023 at 18:26, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On Thu, 27 Jul 2023 at 08:31, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> > > kvm_arch_get_default_type() returns the default KVM type. This hook is
> > > particularly useful to derive a KVM type that is valid for "none"
> > > machine model, which is used by libvirt to probe the availability of
> > > KVM.
> > >
> > > For MIPS, the existing mips_kvm_type() is reused. This function ensures
> > > the availability of VZ which is mandatory to use KVM on the current
> > > QEMU.
> > >
> > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > ---
> > >   include/sysemu/kvm.h     | 2 ++
> > >   target/mips/kvm_mips.h   | 9 ---------
> > >   accel/kvm/kvm-all.c      | 4 +++-
> > >   hw/mips/loongson3_virt.c | 2 --
> > >   target/arm/kvm.c         | 5 +++++
> > >   target/i386/kvm/kvm.c    | 5 +++++
> > >   target/mips/kvm.c        | 2 +-
> > >   target/ppc/kvm.c         | 5 +++++
> > >   target/riscv/kvm.c       | 5 +++++
> > >   target/s390x/kvm/kvm.c   | 5 +++++
> > >   10 files changed, 31 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > > index 115f0cca79..ccaf55caf7 100644
> > > --- a/include/sysemu/kvm.h
> > > +++ b/include/sysemu/kvm.h
> > > @@ -369,6 +369,8 @@ int kvm_arch_get_registers(CPUState *cpu);
> > >
> > >   int kvm_arch_put_registers(CPUState *cpu, int level);
> > >
> > > +int kvm_arch_get_default_type(MachineState *ms);
> > > +
> >
> > New global functions should have a doc comment that explains
> > what they do, what their API is, etc. For instance, is
> > this allowed to return an error, and if so, how ?
>
> Looks like this was the only issue with this patchset. So
> I propose to take this into my target-arm queue for 8.2,
> with the following doc comment added:
>
> /**
>   * kvm_arch_get_default_type: Return default KVM type
>   * @ms: MachineState of the VM being created
>   *
>   * Return the default type argument to use in the
>   * KVM_CREATE_VM ioctl when creating the VM. This will
>   * only be used when the machine model did not specify a
>   * type to use via the MachineClass::kvm_type method.
>   *
>   * Returns: type to use, or a negative value on error.
>   */

Thank you.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>