| Message ID | 20180409154921.29906-1-wei@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] mach-virt: Change default cpu and gic-version setting to "max" | expand |
On 9 April 2018 at 16:49, Wei Huang <wei@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 ? > Signed-off-by: Wei Huang <wei@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
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 <wei@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); > > if (vmc->no_its) { > vms->its = false; > -- > 2.14.3 > > Regards, Daniel
On 04/09/2018 10:56 AM, Daniel P. Berrangé wrote: > 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. I understand this principle. But in reality, under KVM mode, the default config most time doesn't work. If end users specify cpu type manually, it still doesn't work because the host CPU is vendor-specific design (e.g. "cortex-a57" doesn't work on QCOM's machine). So we end up with using "-cpu host" all the time. My argument for this patch is that "-cpu max" isn't worse than "-cpu host". > > 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 <wei@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); >> >> if (vmc->no_its) { >> vms->its = false; >> -- >> 2.14.3 >> >> > > Regards, > Daniel >
On 04/09/2018 10:55 AM, Peter Maydell wrote: > On 9 April 2018 at 16:49, Wei Huang <wei@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 <wei@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 >
On Mon, 2018-04-09 at 11:29 -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. > > I understand this principle. But in reality, under KVM mode, the default > config most time doesn't work. If end users specify cpu type manually, > it still doesn't work because the host CPU is vendor-specific design > (e.g. "cortex-a57" doesn't work on QCOM's machine). So we end up with > using "-cpu host" all the time. My argument for this patch is that "-cpu > max" isn't worse than "-cpu host". I figure the people not explicitly specifying a CPU model on the command line will probably also use '-M virt' instead of versioned machine types, which means they will get a different guest behavior after upgrading QEMU regardless. Defaulting to 'max' for '-cpu' and 'gic-version' makes it convenient to quickly and concisely start a guest; if you care about guest ABI at all, then you are already specifying everything explicitly on the command line instead of relying on defaults - or using libvirt ;)
On Tue, Apr 10, 2018 at 09:41:33AM +0200, Andrea Bolognani wrote: > On Mon, 2018-04-09 at 11:29 -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. > > > > I understand this principle. But in reality, under KVM mode, the default > > config most time doesn't work. If end users specify cpu type manually, > > it still doesn't work because the host CPU is vendor-specific design > > (e.g. "cortex-a57" doesn't work on QCOM's machine). So we end up with > > using "-cpu host" all the time. My argument for this patch is that "-cpu > > max" isn't worse than "-cpu host". > > I figure the people not explicitly specifying a CPU model on the > command line will probably also use '-M virt' instead of versioned > machine types, which means they will get a different guest behavior > after upgrading QEMU regardless. Libvirt uses versioned machine types and does not specify -cpu unless the user has added <cpu> to their XML. IOW libvirt assumes the default CPU model is stable because that's what QEMU has promised in the past. > Defaulting to 'max' for '-cpu' and 'gic-version' makes it convenient > to quickly and concisely start a guest; if you care about guest ABI > at all, then you are already specifying everything explicitly on the > command line instead of relying on defaults - or using libvirt ;) Regards, Daniel
On Tue, 2018-04-10 at 09:52 +0100, Daniel P. Berrangé wrote: > On Tue, Apr 10, 2018 at 09:41:33AM +0200, Andrea Bolognani wrote: > > I figure the people not explicitly specifying a CPU model on the > > command line will probably also use '-M virt' instead of versioned > > machine types, which means they will get a different guest behavior > > after upgrading QEMU regardless. > > Libvirt uses versioned machine types and does not specify -cpu unless the > user has added <cpu> to their XML. IOW libvirt assumes the default CPU > model is stable because that's what QEMU has promised in the past. Hm, you have a point. I wonder how well that works in practice, though. I started a guest with no <cpu> element on my laptop and it ended up having vendor_id : GenuineIntel cpu family : 6 model : 6 model name : QEMU Virtual CPU version 2.5+ stepping : 3 which I guess translates to the qemu64 CPU model, based on the description. I have verified the -cpu option is not present on the command line. The name seems to imply that if I were using a QEMU release older than 2.5 I would get a different CPU model, but maybe the stable CPU guarantee you mention is just a fairly recent development. I also know that ppc64 performs some trickery if you don't specify a CPU model, so by default you get a behavior which is pretty close to using -cpu host. Basically I'm wondering how reasonable it is to expect a migratable machine and a stable guest ABI when relying on QEMU defaults instead of explicitly picking a CPU model.
On Wed, Apr 11, 2018 at 05:35:55PM +0200, Andrea Bolognani wrote: > On Tue, 2018-04-10 at 09:52 +0100, Daniel P. Berrangé wrote: > > On Tue, Apr 10, 2018 at 09:41:33AM +0200, Andrea Bolognani wrote: > > > I figure the people not explicitly specifying a CPU model on the > > > command line will probably also use '-M virt' instead of versioned > > > machine types, which means they will get a different guest behavior > > > after upgrading QEMU regardless. > > > > Libvirt uses versioned machine types and does not specify -cpu unless the > > user has added <cpu> to their XML. IOW libvirt assumes the default CPU > > model is stable because that's what QEMU has promised in the past. > > Hm, you have a point. > > I wonder how well that works in practice, though. I started a guest > with no <cpu> element on my laptop and it ended up having > > vendor_id : GenuineIntel > cpu family : 6 > model : 6 > model name : QEMU Virtual CPU version 2.5+ > stepping : 3 > > which I guess translates to the qemu64 CPU model, based on the > description. I have verified the -cpu option is not present on the > command line. > > The name seems to imply that if I were using a QEMU release older > than 2.5 I would get a different CPU model, but maybe the stable CPU > guarantee you mention is just a fairly recent development. That model ID string is tied to the machine type, so it is stable. Older machine types use PC_CPU_MODEL_IDS() in pc.h to ensure they have not changed. > I also know that ppc64 performs some trickery if you don't specify a > CPU model, so by default you get a behavior which is pretty close to > using -cpu host. > > Basically I'm wondering how reasonable it is to expect a migratable > machine and a stable guest ABI when relying on QEMU defaults instead > of explicitly picking a CPU model. Provided you have picked a versioned machine type, it is expected that all other defaults are stable, and that includes CPU models. Regards, Daniel
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;
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. Signed-off-by: Wei Huang <wei@redhat.com> --- hw/arm/virt.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)