On 04/09/2018 10:55 AM, Peter Maydell wrote: > On 9 April 2018 at 16:49, Wei Huang <w...@redhat.com> 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. > > Using "max" will make the VM not migratable by default, and also > means that the behaviour might differ between host machines and > between different QEMU versions. I'm not sure we want that for the > default ?
Understood. If you think the -cpu shouldn't be changed due to the reasons list above, we should at lease give a better warning message when cpu doesn't match. Right now, the error msg is something like "kvm_init_vcpu failed, ...". Secondly, how about gic-version part? Should we use "max" as the default gic-version? > >> Signed-off-by: Wei Huang <w...@redhat.com> >> --- >> 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); > > This change in the help string is a bug fix not related to the > rest of the patch. You should send that as a separate patch, > because we want that regardless of what we do about the default > value. > > thanks > -- PMM >