[PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Philippe Mathieu-Daudé]

Hi,

This series aims to improve user experience by providing
a better error message when the user tries to enable KVM
on machines not supporting it.

Regards,

Phil.

Philippe Mathieu-Daudé (7):
  accel/kvm: Check MachineClass kvm_type() return value
  hw/boards: Introduce 'kvm_supported' field to MachineClass
  hw/arm: Set kvm_supported for KVM-compatible machines
  hw/mips: Set kvm_supported for KVM-compatible machines
  hw/ppc: Set kvm_supported for KVM-compatible machines
  hw/s390x: Set kvm_supported to s390-ccw-virtio machines
  accel/kvm: Exit gracefully when KVM is not supported

 include/hw/boards.h        |  6 +++++-
 accel/kvm/kvm-all.c        | 12 ++++++++++++
 hw/arm/sbsa-ref.c          |  1 +
 hw/arm/virt.c              |  1 +
 hw/arm/xlnx-versal-virt.c  |  1 +
 hw/mips/loongson3_virt.c   |  1 +
 hw/mips/malta.c            |  1 +
 hw/ppc/e500plat.c          |  1 +
 hw/ppc/mac_newworld.c      |  1 +
 hw/ppc/mac_oldworld.c      |  1 +
 hw/ppc/mpc8544ds.c         |  1 +
 hw/ppc/ppc440_bamboo.c     |  1 +
 hw/ppc/prep.c              |  1 +
 hw/ppc/sam460ex.c          |  1 +
 hw/ppc/spapr.c             |  1 +
 hw/s390x/s390-virtio-ccw.c |  1 +
 16 files changed, 31 insertions(+), 1 deletion(-)

-- 
2.26.2

[PATCH 1/7] accel/kvm: Check MachineClass kvm_type() return value

[Philippe Mathieu-Daudé]

MachineClass::kvm_type() can return -1 on failure.
Document it, and add a check in kvm_init().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/boards.h | 3 ++-
 accel/kvm/kvm-all.c | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index a46dfe5d1a6..68d3d10f6b0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -127,7 +127,8 @@ typedef struct {
  *    implement and a stub device is required.
  * @kvm_type:
  *    Return the type of KVM corresponding to the kvm-type string option or
- *    computed based on other criteria such as the host kernel capabilities.
+ *    computed based on other criteria such as the host kernel capabilities
+ *    (which can't be negative), or -1 on error.
  * @numa_mem_supported:
  *    true if '--numa node.mem' option is supported and false otherwise
  * @smp_parse:
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 84c943fcdb2..b069938d881 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2057,6 +2057,12 @@ static int kvm_init(MachineState *ms)
                                                             "kvm-type",
                                                             &error_abort);
         type = mc->kvm_type(ms, kvm_type);
+        if (type < 0) {
+            ret = -EINVAL;
+            fprintf(stderr, "Failed to detect kvm-type for machine '%s'\n",
+                    mc->name);
+            goto err;
+        }
     }
 
     do {
-- 
2.26.2

[PATCH 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass

[Philippe Mathieu-Daudé]

Introduce the 'kvm_supported' field to express whether
a machine supports KVM acceleration or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/boards.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 68d3d10f6b0..0959aa743ee 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -129,6 +129,8 @@ typedef struct {
  *    Return the type of KVM corresponding to the kvm-type string option or
  *    computed based on other criteria such as the host kernel capabilities
  *    (which can't be negative), or -1 on error.
+ * @kvm_supported:
+ *    true if '-enable-kvm' option is supported and false otherwise.
  * @numa_mem_supported:
  *    true if '--numa node.mem' option is supported and false otherwise
  * @smp_parse:
@@ -209,6 +211,7 @@ struct MachineClass {
     bool nvdimm_supported;
     bool numa_mem_supported;
     bool auto_enable_numa;
+    bool kvm_supported;
     const char *default_ram_id;
 
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
-- 
2.26.2

Re: [PATCH 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass

[Daniel P. Berrangé]

On Fri, Feb 19, 2021 at 12:44:23PM +0100, Philippe Mathieu-Daudé wrote:
> Introduce the 'kvm_supported' field to express whether
> a machine supports KVM acceleration or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/boards.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 68d3d10f6b0..0959aa743ee 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -129,6 +129,8 @@ typedef struct {
>   *    Return the type of KVM corresponding to the kvm-type string option or
>   *    computed based on other criteria such as the host kernel capabilities
>   *    (which can't be negative), or -1 on error.
> + * @kvm_supported:
> + *    true if '-enable-kvm' option is supported and false otherwise.

Is the behaviour reported really related to KVM specifically, as opposed
to all hardware based virt backends ?

eg is it actually a case of some machine types being  "tcg_only" ?



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 :|

Re: [PATCH 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass

[Peter Maydell]

On Fri, 19 Feb 2021 at 11:58, Daniel P. Berrangé <berrange@redhat.com> wrote:
> Is the behaviour reported really related to KVM specifically, as opposed
> to all hardware based virt backends ?
>
> eg is it actually a case of some machine types being  "tcg_only" ?

Interesting question. At least for Arm the major items are:
 * does the accelerator support emulation of EL3/TrustZone?
   (KVM doesn't; this is the proximate cause of the assertion
   failure if you try to enable KVM for the raspi boards.)
 * does the board type require a particular CPU type which
   KVM doesn't/can't support?
Non-KVM accelerators could at least in theory have different answers
to those questions, though in practice I think they do not.

I think my take is that we probably should mark the boards
as 'tcg-only' vs 'not-tcg-only', because in practice that's
the interesting distinction. Specifically, our security policy
https://qemu.readthedocs.io/en/latest/system/security.html
draws a boundary between "virtualization use case" and
"emulated", so it's really helpful to be able to say clearly
"this board model does not support virtualization, and therefore
any bugs in it or its devices are simply outside the realm of
being security issues" when doing analysis of the codebase or
when writing or reviewing new code.

If we ever have support for some new accelerator type where there's
a board type distinction between KVM and that new accelerator and
it makes sense to try to say "this board is supported by the new
thing even though it won't work with KVM", the folks interested in
adding that new accelerator will have the motivation to look
into exactly which boards they want to enable support for and
can add a funky_accelerator_supported flag or whatever at that time.

Summary: we should name this machine class field
"virtualization_supported" and check it in all the virtualization
accelerators (kvm, hvf, whpx, xen).

thanks
-- PMM

Re: [PATCH 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass

[Daniel P. Berrangé]

On Fri, Feb 19, 2021 at 12:08:05PM +0000, Peter Maydell wrote:
> On Fri, 19 Feb 2021 at 11:58, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > Is the behaviour reported really related to KVM specifically, as opposed
> > to all hardware based virt backends ?
> >
> > eg is it actually a case of some machine types being  "tcg_only" ?
> 
> Interesting question. At least for Arm the major items are:
>  * does the accelerator support emulation of EL3/TrustZone?
>    (KVM doesn't; this is the proximate cause of the assertion
>    failure if you try to enable KVM for the raspi boards.)
>  * does the board type require a particular CPU type which
>    KVM doesn't/can't support?
> Non-KVM accelerators could at least in theory have different answers
> to those questions, though in practice I think they do not.
> 
> I think my take is that we probably should mark the boards
> as 'tcg-only' vs 'not-tcg-only', because in practice that's
> the interesting distinction. Specifically, our security policy
> https://qemu.readthedocs.io/en/latest/system/security.html
> draws a boundary between "virtualization use case" and
> "emulated", so it's really helpful to be able to say clearly
> "this board model does not support virtualization, and therefore
> any bugs in it or its devices are simply outside the realm of
> being security issues" when doing analysis of the codebase or
> when writing or reviewing new code.

Oh, yes, that is useful to correlate with.

> If we ever have support for some new accelerator type where there's
> a board type distinction between KVM and that new accelerator and
> it makes sense to try to say "this board is supported by the new
> thing even though it won't work with KVM", the folks interested in
> adding that new accelerator will have the motivation to look
> into exactly which boards they want to enable support for and
> can add a funky_accelerator_supported flag or whatever at that time.
> 
> Summary: we should name this machine class field
> "virtualization_supported" and check it in all the virtualization
> accelerators (kvm, hvf, whpx, xen).

Agreed.

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 :|

Re: [PATCH 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass

[Leif Lindholm]

On Fri, Feb 19, 2021 at 12:08:05 +0000, Peter Maydell wrote:
> On Fri, 19 Feb 2021 at 11:58, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > Is the behaviour reported really related to KVM specifically, as opposed
> > to all hardware based virt backends ?
> >
> > eg is it actually a case of some machine types being  "tcg_only" ?
> 
> Interesting question. At least for Arm the major items are:
>  * does the accelerator support emulation of EL3/TrustZone?
>    (KVM doesn't; this is the proximate cause of the assertion
>    failure if you try to enable KVM for the raspi boards.)
>  * does the board type require a particular CPU type which
>    KVM doesn't/can't support?
> Non-KVM accelerators could at least in theory have different answers
> to those questions, though in practice I think they do not.
> 
> I think my take is that we probably should mark the boards
> as 'tcg-only' vs 'not-tcg-only', because in practice that's
> the interesting distinction. Specifically, our security policy
> https://qemu.readthedocs.io/en/latest/system/security.html
> draws a boundary between "virtualization use case" and
> "emulated", so it's really helpful to be able to say clearly
> "this board model does not support virtualization, and therefore
> any bugs in it or its devices are simply outside the realm of
> being security issues" when doing analysis of the codebase or
> when writing or reviewing new code.

Yes. This applies to sbsa-ref, for example.
We explicitly want to start in EL3, so no KVM for us.

/
    Leif

> If we ever have support for some new accelerator type where there's
> a board type distinction between KVM and that new accelerator and
> it makes sense to try to say "this board is supported by the new
> thing even though it won't work with KVM", the folks interested in
> adding that new accelerator will have the motivation to look
> into exactly which boards they want to enable support for and
> can add a funky_accelerator_supported flag or whatever at that time.
> 
> Summary: we should name this machine class field
> "virtualization_supported" and check it in all the virtualization
> accelerators (kvm, hvf, whpx, xen).
> 
> thanks
> -- PMM

[PATCH 3/7] hw/arm: Set kvm_supported for KVM-compatible machines

[Philippe Mathieu-Daudé]

The following ARM machines support KVM:
- virt
- sbsa-ref
- xlnx-versal-virt

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/sbsa-ref.c         | 1 +
 hw/arm/virt.c             | 1 +
 hw/arm/xlnx-versal-virt.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 9f707351531..6923b6e77ff 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -858,6 +858,7 @@ static void sbsa_ref_class_init(ObjectClass *oc, void *data)
     mc->possible_cpu_arch_ids = sbsa_ref_possible_cpu_arch_ids;
     mc->cpu_index_to_instance_props = sbsa_ref_cpu_index_to_props;
     mc->get_default_cpu_node_id = sbsa_ref_get_default_cpu_node_id;
+    mc->kvm_supported = true;
 }
 
 static const TypeInfo sbsa_ref_info = {
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 371147f3ae9..7e3748b6cd6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2582,6 +2582,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
     mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
+    mc->kvm_supported = true;
     mc->kvm_type = virt_kvm_type;
     assert(!mc->get_hotplug_handler);
     mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 8482cd61960..fb623c0cd54 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -621,6 +621,7 @@ static void versal_virt_machine_class_init(ObjectClass *oc, void *data)
     mc->default_cpus = XLNX_VERSAL_NR_ACPUS;
     mc->no_cdrom = true;
     mc->default_ram_id = "ddr";
+    mc->kvm_supported = true;
 }
 
 static const TypeInfo versal_virt_machine_init_typeinfo = {
-- 
2.26.2

[PATCH 4/7] hw/mips: Set kvm_supported for KVM-compatible machines

[Philippe Mathieu-Daudé]

The following MIPS machines support KVM:
- malta
- loongson3-virt

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/mips/loongson3_virt.c | 1 +
 hw/mips/malta.c          | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index d4a82fa5367..c5ef44819a7 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -622,6 +622,7 @@ static void loongson3v_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = LOONGSON_MAX_VCPUS;
     mc->default_ram_id = "loongson3.highram";
     mc->default_ram_size = 1600 * MiB;
+    mc->kvm_supported = true;
     mc->kvm_type = mips_kvm_type;
     mc->minimum_page_bits = 14;
 }
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9afc0b427bf..f06bd3eff25 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1456,6 +1456,7 @@ static void mips_malta_machine_init(MachineClass *mc)
     mc->default_cpu_type = MIPS_CPU_TYPE_NAME("24Kf");
 #endif
     mc->default_ram_id = "mips_malta.ram";
+    mc->kvm_supported = true;
 }
 
 DEFINE_MACHINE("malta", mips_malta_machine_init)
-- 
2.26.2

[RFC PATCH 5/7] hw/ppc: Set kvm_supported for KVM-compatible machines

[Philippe Mathieu-Daudé]

The following PowerPC machines support KVM:
- 40p
- bamboo
- g3beige
- mac99
- mpc8544ds
- ppce500
- pseries
- sam460ex
- virtex-ml507

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC: I'm surprise by this list, but this is the result of
     auditing calls to kvm_enabled() checks.
---
 hw/ppc/e500plat.c      | 1 +
 hw/ppc/mac_newworld.c  | 1 +
 hw/ppc/mac_oldworld.c  | 1 +
 hw/ppc/mpc8544ds.c     | 1 +
 hw/ppc/ppc440_bamboo.c | 1 +
 hw/ppc/prep.c          | 1 +
 hw/ppc/sam460ex.c      | 1 +
 hw/ppc/spapr.c         | 1 +
 8 files changed, 8 insertions(+)

diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index bddd5e7c48f..bf95b68bc03 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -98,6 +98,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 32;
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
     mc->default_ram_id = "mpc8544ds.ram";
+    mc->kvm_supported = true;
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_ETSEC_COMMON);
  }
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index e991db4addb..573502f7b5b 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -595,6 +595,7 @@ static void core99_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = MAX_CPUS;
     mc->default_boot_order = "cd";
     mc->default_display = "std";
+    mc->kvm_supported = true;
     mc->kvm_type = core99_kvm_type;
 #ifdef TARGET_PPC64
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 44ee99be886..4b34106919d 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -444,6 +444,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
 #endif
     /* TOFIX "cad" when Mac floppy is implemented */
     mc->default_boot_order = "cd";
+    mc->kvm_supported = true;
     mc->kvm_type = heathrow_kvm_type;
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("750_v3.1");
     mc->default_display = "std";
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 81177505f02..4b750ca0555 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -56,6 +56,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 15;
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
     mc->default_ram_id = "mpc8544ds.ram";
+    mc->kvm_supported = true;
 }
 
 #define TYPE_MPC8544DS_MACHINE  MACHINE_TYPE_NAME("mpc8544ds")
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index b156bcb9990..f3258a1f229 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -304,6 +304,7 @@ static void bamboo_machine_init(MachineClass *mc)
     mc->init = bamboo_init;
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("440epb");
     mc->default_ram_id = "ppc4xx.sdram";
+    mc->kvm_supported = true;
 }
 
 DEFINE_MACHINE("bamboo", bamboo_machine_init)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 7e72f6e4a9b..96b6f68d663 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -441,6 +441,7 @@ static void ibm_40p_machine_init(MachineClass *mc)
     mc->default_boot_order = "c";
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("604");
     mc->default_display = "std";
+    mc->kvm_supported = true;
 }
 
 DEFINE_MACHINE("40p", ibm_40p_machine_init)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index e459b43065b..43cccad1591 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -513,6 +513,7 @@ static void sam460ex_machine_init(MachineClass *mc)
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
     mc->default_ram_size = 512 * MiB;
     mc->default_ram_id = "ppc4xx.sdram";
+    mc->kvm_supported = true;
 }
 
 DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 85fe65f8947..3f83c2ce2ca 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4426,6 +4426,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 512 * MiB;
     mc->default_ram_id = "ppc_spapr.ram";
     mc->default_display = "std";
+    mc->kvm_supported = true;
     mc->kvm_type = spapr_kvm_type;
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SPAPR_PCI_HOST_BRIDGE);
     mc->pci_allow_0_address = true;
-- 
2.26.2

[PATCH 6/7] hw/s390x: Set kvm_supported to s390-ccw-virtio machines

[Philippe Mathieu-Daudé]

All s390-ccw-virtio machines support KVM.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 2972b607f36..259b4b4397e 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -612,6 +612,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
     mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
     /* it is overridden with 'host' cpu *in kvm_arch_init* */
     mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
+    mc->kvm_supported = true;
     hc->plug = s390_machine_device_plug;
     hc->unplug_request = s390_machine_device_unplug_request;
     nc->nmi_monitor_handler = s390_nmi;
-- 
2.26.2

[PATCH 7/7] accel/kvm: Exit gracefully when KVM is not supported

[Philippe Mathieu-Daudé]

Now that we added the 'kvm_supported' field to MachineClass
and all our machines able to use KVM have this field set,
we can check it in kvm_init() and exit gracefully with
a friendly error message.

Before:

  $ qemu-system-aarch64 -M raspi3b -enable-kvm
  qemu-system-aarch64: /build/qemu-ETIdrs/qemu-4.2/exec.c:865: cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed.
  Aborted

  $ qemu-system-aarch64 -M xlnx-zcu102 -enable-kvm -smp 6
  qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument

After:

  $ qemu-system-aarch64 -M raspi3b -enable-kvm
  Machine 'raspi3b' does not support KVM

  $ qemu-system-aarch64 -M xlnx-zcu102 -enable-kvm -smp 6
  Machine 'xlnx-zcu102' does not support KVM

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 accel/kvm/kvm-all.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b069938d881..8a8d3f64248 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2001,6 +2001,12 @@ static int kvm_init(MachineState *ms)
 
     s = KVM_STATE(ms->accelerator);
 
+    if (!mc->kvm_supported) {
+        ret = -EINVAL;
+        fprintf(stderr, "Machine '%s' does not support KVM\n", mc->name);
+        exit(1);
+    }
+
     /*
      * On systems where the kernel can support different base page
      * sizes, host page size may be different from TARGET_PAGE_SIZE,
-- 
2.26.2

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Peter Maydell]

On Fri, 19 Feb 2021 at 11:44, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> This series aims to improve user experience by providing
> a better error message when the user tries to enable KVM
> on machines not supporting it.

Thanks for having a look at this; fixing the ugly assertion
failure if you try to enable KVM for the raspi boards has
been vaguely on my todo list but never made it up to the top...

> Philippe Mathieu-Daudé (7):
>   accel/kvm: Check MachineClass kvm_type() return value
>   hw/boards: Introduce 'kvm_supported' field to MachineClass
>   hw/arm: Set kvm_supported for KVM-compatible machines
>   hw/mips: Set kvm_supported for KVM-compatible machines
>   hw/ppc: Set kvm_supported for KVM-compatible machines
>   hw/s390x: Set kvm_supported to s390-ccw-virtio machines
>   accel/kvm: Exit gracefully when KVM is not supported

Don't we also need to set kvm_supported for the relevant
machine types in hw/i386 ?

thanks
-- PMM

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Philippe Mathieu-Daudé]

On 2/19/21 12:55 PM, Peter Maydell wrote:
> On Fri, 19 Feb 2021 at 11:44, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> This series aims to improve user experience by providing
>> a better error message when the user tries to enable KVM
>> on machines not supporting it.
> 
> Thanks for having a look at this; fixing the ugly assertion
> failure if you try to enable KVM for the raspi boards has
> been vaguely on my todo list but never made it up to the top...

The other one annoying was the xlnx-zcu102 when creating
the Cortex-R cores.

>> Philippe Mathieu-Daudé (7):
>>   accel/kvm: Check MachineClass kvm_type() return value
>>   hw/boards: Introduce 'kvm_supported' field to MachineClass
>>   hw/arm: Set kvm_supported for KVM-compatible machines
>>   hw/mips: Set kvm_supported for KVM-compatible machines
>>   hw/ppc: Set kvm_supported for KVM-compatible machines
>>   hw/s390x: Set kvm_supported to s390-ccw-virtio machines
>>   accel/kvm: Exit gracefully when KVM is not supported
> 
> Don't we also need to set kvm_supported for the relevant
> machine types in hw/i386 ?

Lol, clearly a parapraxis =)

I'll send it as 8/7 until I get more review comments for a
v2 (in particular on the PPC patch):

-- >8 --
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6329f90ef90..da895aa051d 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1218,6 +1218,7 @@ static void x86_machine_class_init(ObjectClass
*oc, void *data)
     mc->cpu_index_to_instance_props = x86_cpu_index_to_props;
     mc->get_default_cpu_node_id = x86_get_default_cpu_node_id;
     mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids;
+    mc->kvm_supported = true;
     x86mc->compat_apic_id_mode = false;
     x86mc->save_tsc_khz = true;
     nc->nmi_monitor_handler = x86_nmi;
---

Regards,

Phil.

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Daniel P. Berrangé]

On Fri, Feb 19, 2021 at 12:44:21PM +0100, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series aims to improve user experience by providing
> a better error message when the user tries to enable KVM
> on machines not supporting it.

Improved error message is good, but it is better if the mgmt apps knows
not to try this in the first place.

IOW, I think we want "query-machines" to filter out machines
which are not available with the currently configured accelerator.

libvirt will probe separately with both TCG and KVM enabled, so if
query-machines can give the right answer in these cases, libvirt
will probably "just work" and not offer to even start such a VM.


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 :|

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Philippe Mathieu-Daudé]

On 2/19/21 1:00 PM, Daniel P. Berrangé wrote:
> On Fri, Feb 19, 2021 at 12:44:21PM +0100, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This series aims to improve user experience by providing
>> a better error message when the user tries to enable KVM
>> on machines not supporting it.
> 
> Improved error message is good, but it is better if the mgmt apps knows
> not to try this in the first place.

I am not sure this is the same problem. This series addresses
users from the command line (without mgmt app).

> IOW, I think we want "query-machines" to filter out machines
> which are not available with the currently configured accelerator.
> 
> libvirt will probe separately with both TCG and KVM enabled, so if
> query-machines can give the right answer in these cases, libvirt
> will probably "just work" and not offer to even start such a VM.

Yes, agreed. There are other discussions about 'query-machines'
and an eventual 'query-accels'. This series doesn't aim to fix
the mgmt app problems.

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Daniel P. Berrangé]

On Fri, Feb 19, 2021 at 01:15:25PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/19/21 1:00 PM, Daniel P. Berrangé wrote:
> > On Fri, Feb 19, 2021 at 12:44:21PM +0100, Philippe Mathieu-Daudé wrote:
> >> Hi,
> >>
> >> This series aims to improve user experience by providing
> >> a better error message when the user tries to enable KVM
> >> on machines not supporting it.
> > 
> > Improved error message is good, but it is better if the mgmt apps knows
> > not to try this in the first place.
> 
> I am not sure this is the same problem. This series addresses
> users from the command line (without mgmt app).

Users of mgmt apps can launch the same problematic raspbi + KVM config
as people who  don't use a mgmt app.

> > IOW, I think we want "query-machines" to filter out machines
> > which are not available with the currently configured accelerator.
> > 
> > libvirt will probe separately with both TCG and KVM enabled, so if
> > query-machines can give the right answer in these cases, libvirt
> > will probably "just work" and not offer to even start such a VM.
> 
> Yes, agreed. There are other discussions about 'query-machines'
> and an eventual 'query-accels'. This series doesn't aim to fix
> the mgmt app problems.

I think this should be fixing query-machines right now. It shouldn't
be much harder than a single if (...) test in the code.

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 :|

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Philippe Mathieu-Daudé]

On 2/19/21 1:18 PM, Daniel P. Berrangé wrote:
> On Fri, Feb 19, 2021 at 01:15:25PM +0100, Philippe Mathieu-Daudé wrote:
>> On 2/19/21 1:00 PM, Daniel P. Berrangé wrote:
>>> On Fri, Feb 19, 2021 at 12:44:21PM +0100, Philippe Mathieu-Daudé wrote:
>>>> Hi,
>>>>
>>>> This series aims to improve user experience by providing
>>>> a better error message when the user tries to enable KVM
>>>> on machines not supporting it.
>>>
>>> Improved error message is good, but it is better if the mgmt apps knows
>>> not to try this in the first place.
>>
>> I am not sure this is the same problem. This series addresses
>> users from the command line (without mgmt app).
> 
> Users of mgmt apps can launch the same problematic raspbi + KVM config
> as people who  don't use a mgmt app.
> 
>>> IOW, I think we want "query-machines" to filter out machines
>>> which are not available with the currently configured accelerator.
>>>
>>> libvirt will probe separately with both TCG and KVM enabled, so if
>>> query-machines can give the right answer in these cases, libvirt
>>> will probably "just work" and not offer to even start such a VM.
>>
>> Yes, agreed. There are other discussions about 'query-machines'
>> and an eventual 'query-accels'. This series doesn't aim to fix
>> the mgmt app problems.
> 
> I think this should be fixing query-machines right now. It shouldn't
> be much harder than a single if (...) test in the code.

OK I misunderstood you at first, now I got it. Will include that in v2.

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Claudio Fontana]

On 2/19/21 12:44 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series aims to improve user experience by providing
> a better error message when the user tries to enable KVM
> on machines not supporting it.
> 
> Regards,
> 
> Phil.

Hi Philippe, not sure if it fits in this series,

but also the experience of a user running on a machine with cortex-a72,
choosing that very same cpu with -cpu and then getting:

qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument

is not super-friendly. Maybe some suggestion to use -cpu host with KVM could be good?

Thanks,

Claudio

> 
> Philippe Mathieu-Daudé (7):
>   accel/kvm: Check MachineClass kvm_type() return value
>   hw/boards: Introduce 'kvm_supported' field to MachineClass
>   hw/arm: Set kvm_supported for KVM-compatible machines
>   hw/mips: Set kvm_supported for KVM-compatible machines
>   hw/ppc: Set kvm_supported for KVM-compatible machines
>   hw/s390x: Set kvm_supported to s390-ccw-virtio machines
>   accel/kvm: Exit gracefully when KVM is not supported
> 
>  include/hw/boards.h        |  6 +++++-
>  accel/kvm/kvm-all.c        | 12 ++++++++++++
>  hw/arm/sbsa-ref.c          |  1 +
>  hw/arm/virt.c              |  1 +
>  hw/arm/xlnx-versal-virt.c  |  1 +
>  hw/mips/loongson3_virt.c   |  1 +
>  hw/mips/malta.c            |  1 +
>  hw/ppc/e500plat.c          |  1 +
>  hw/ppc/mac_newworld.c      |  1 +
>  hw/ppc/mac_oldworld.c      |  1 +
>  hw/ppc/mpc8544ds.c         |  1 +
>  hw/ppc/ppc440_bamboo.c     |  1 +
>  hw/ppc/prep.c              |  1 +
>  hw/ppc/sam460ex.c          |  1 +
>  hw/ppc/spapr.c             |  1 +
>  hw/s390x/s390-virtio-ccw.c |  1 +
>  16 files changed, 31 insertions(+), 1 deletion(-)
> 

Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported

[Philippe Mathieu-Daudé]

On 2/19/21 1:34 PM, Claudio Fontana wrote:
> On 2/19/21 12:44 PM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This series aims to improve user experience by providing
>> a better error message when the user tries to enable KVM
>> on machines not supporting it.
>>
>> Regards,
>>
>> Phil.
> 
> Hi Philippe, not sure if it fits in this series,
> 
> but also the experience of a user running on a machine with cortex-a72,
> choosing that very same cpu with -cpu and then getting:
> 
> qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> 
> is not super-friendly. Maybe some suggestion to use -cpu host with KVM could be good?

I agree this should be improved, but it is out of the scope of this
series :)