Re: [Qemu-devel] [PATCH 1/1] mach-virt: Change default cpu and gic-version setting to "max"

[Daniel P . Berrangé]

On Mon, Apr 09, 2018 at 10:49:21AM -0500, Wei Huang wrote:
> Running mach-virt machine types (i.e. "-M virt") on different systems can
> result in various misleading warnings if -cpu and/or gic-version not 
> specified.
> For KVM, this can be solved mostly by using "host" type. But the "host" type
> doesn't work for TCG. Compared with "host", the "max" type not only supports
> auto detection under KVM mode, but also works with TCG. So this patch set
> "max" as the default types for both -cpu and gic-version.

Hmm, generally we aim for the config provided by a machine type to be stable
across QEMU versions.

By specifying "max", the machine type will potentially change if new QEMU
turns on new features in the "max" CPU model, as well as also varying
depending on what the host OS supports.

This is a general conceptual problem with the "host" CPU model for all
target arches and is unfixable by design. This is fine if you accept
the caveats with using "-cpu host" and opt-in to using it knowing the
tradeoffs. I'm just not convinced it is reasonable to make "-cpu host"
be the default value for a machine type out of the box.


> Signed-off-by: Wei Huang <address@hidden>
> ---
>  hw/arm/virt.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 94dcb125d3..1a9d68b8d5 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1555,7 +1555,7 @@ static void virt_machine_class_init(ObjectClass *oc, 
> void *data)
>      mc->minimum_page_bits = 12;
>      mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids;
>      mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
> -    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("max");
>      mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
>  }
>  
> @@ -1609,13 +1609,13 @@ static void virt_2_12_instance_init(Object *obj)
>                                      "Set on/off to enable/disable using "
>                                      "physical address space above 32 bits",
>                                      NULL);
> -    /* Default GIC type is v2 */
> -    vms->gic_version = 2;
> +    /* Default GIC type is max */
> +    vms->gic_version = -1;
>      object_property_add_str(obj, "gic-version", virt_get_gic_version,
>                          virt_set_gic_version, NULL);
>      object_property_set_description(obj, "gic-version",
> -                                    "Set GIC version. "
> -                                    "Valid values are 2, 3 and host", NULL);
> +                                    "Set GIC version. Valid values are 2, 3, 
> "
> +                                    "host, and max", NULL);
>  
>      if (vmc->no_its) {
>          vms->its = false;
> -- 
> 2.14.3
> 
> 

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