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

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

[Wei Huang]

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(-)

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

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

[Peter Maydell]

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

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

[Wei Huang]

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
> 

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 <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
-- 
|: 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: [Qemu-devel] [PATCH 1/1] mach-virt: Change default cpu and gic-version setting to "max"

[Wei Huang]

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
> 

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

[Andrea Bolognani]

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 ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Daniel P. Berrangé]

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
-- 
|: 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: [Qemu-devel] [PATCH 1/1] mach-virt: Change default cpu and gic-version setting to "max"

[Andrea Bolognani]

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.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Daniel P. Berrangé]

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