[Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Eduardo Habkost]

Changes v1 -> v2:
* Fix build without CONFIG_KVM at lmce_supported()
* Rebased on top of my x86-next branch:
  https://github.com/ehabkost/qemu x86-next

Git branch for testing:
  https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg

libvirt code to use the new feature already exist, and were
submitted to libvir-list, at:
  https://www.mail-archive.com/libvir-list@redhat.com/msg142168.html

---

This is a replacement for the previous series that enabled the
"host" CPU model on TCG. Now a new "max" CPU is being added,
while keeping "host" KVM-specific.

In addition to simply adding "max" as a copy of the existing
"host" CPU model, additional patches change it to not use any
host CPUID information when in TCG mode.

---
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiri Denemark <jdenemar@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>

Eduardo Habkost (3):
  i386: Create "max" CPU model
  i386: Make "max" model not use any host CPUID info on TCG
  i386: Don't set CPUClass::cpu_def on "max" model

 target/i386/cpu-qom.h |  4 ++-
 target/i386/cpu.c     | 98 ++++++++++++++++++++++++++++++++-------------------
 2 files changed, 64 insertions(+), 38 deletions(-)

-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH v2 1/3] i386: Create "max" CPU model

[Eduardo Habkost]

Rename the existing "host" CPU model to "max, and set it to
kvm_enabled=false. The new "max" CPU model will be able to enable
all features supported by TCG out of the box, because its logic
is based on x86_cpu_get_supported_feature_word(), which already
works with TCG.

A new KVM-specific "host" class was added, that simply inherits
everything from "max" except the 'ordering' and 'description'
fields.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Fix build without CONFIG_KVM at lmce_supported()
---
 target/i386/cpu.c | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index feb2110872..f7bd59617e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1503,15 +1503,15 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value)
 static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
                                                    bool migratable_only);
 
-#ifdef CONFIG_KVM
-
 static bool lmce_supported(void)
 {
-    uint64_t mce_cap;
+    uint64_t mce_cap = 0;
 
+#ifdef CONFIG_KVM
     if (kvm_ioctl(kvm_state, KVM_X86_GET_MCE_CAP_SUPPORTED, &mce_cap) < 0) {
         return false;
     }
+#endif
 
     return !!(mce_cap & MCG_LMCE_P);
 }
@@ -1533,23 +1533,22 @@ static int cpu_x86_fill_model_id(char *str)
 
 static X86CPUDefinition host_cpudef;
 
-static Property host_x86_cpu_properties[] = {
+static Property max_x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
     DEFINE_PROP_BOOL("host-cache-info", X86CPU, cache_info_passthrough, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-/* class_init for the "host" CPU model
+/* class_init for the "max" CPU model
  *
  * This function may be called before KVM is initialized.
  */
-static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
+static void max_x86_cpu_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
     uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
-    xcc->kvm_required = true;
     xcc->ordering = 9;
 
     host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
@@ -1564,17 +1563,16 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
 
     xcc->cpu_def = &host_cpudef;
     xcc->model_description =
-        "KVM processor with all supported host features "
-        "(only available in KVM mode)";
+        "Enables all features supported by the accelerator in the current host";
 
     /* level, xlevel, xlevel2, and the feature words are initialized on
      * instance_init, because they require KVM to be initialized.
      */
 
-    dc->props = host_x86_cpu_properties;
+    dc->props = max_x86_cpu_properties;
 }
 
-static void host_x86_cpu_initfn(Object *obj)
+static void max_x86_cpu_initfn(Object *obj)
 {
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
@@ -1585,7 +1583,6 @@ static void host_x86_cpu_initfn(Object *obj)
      */
     cpu->max_features = true;
 
-    /* If KVM is disabled, x86_cpu_realizefn() will report an error later */
     if (kvm_enabled()) {
         env->cpuid_min_level =
             kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
@@ -1602,10 +1599,30 @@ static void host_x86_cpu_initfn(Object *obj)
     object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
 }
 
+static const TypeInfo max_x86_cpu_type_info = {
+    .name = X86_CPU_TYPE_NAME("max"),
+    .parent = TYPE_X86_CPU,
+    .instance_init = max_x86_cpu_initfn,
+    .class_init = max_x86_cpu_class_init,
+};
+
+#ifdef CONFIG_KVM
+
+static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+
+    xcc->kvm_required = true;
+    xcc->ordering = 8;
+
+    xcc->model_description =
+        "KVM processor with all supported host features "
+        "(only available in KVM mode)";
+}
+
 static const TypeInfo host_x86_cpu_type_info = {
     .name = X86_CPU_TYPE_NAME("host"),
-    .parent = TYPE_X86_CPU,
-    .instance_init = host_x86_cpu_initfn,
+    .parent = X86_CPU_TYPE_NAME("max"),
     .class_init = host_x86_cpu_class_init,
 };
 
@@ -3781,6 +3798,7 @@ static void x86_cpu_register_types(void)
     for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
         x86_register_cpudef_type(&builtin_x86_defs[i]);
     }
+    type_register_static(&max_x86_cpu_type_info);
 #ifdef CONFIG_KVM
     type_register_static(&host_x86_cpu_type_info);
 #endif
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH v2 2/3] i386: Make "max" model not use any host CPUID info on TCG

[Eduardo Habkost]

Instead of reporting host CPUID data on "max", use the qemu64 CPU
model as reference to initialize CPUID
vendor/family/model/stepping/model-id.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f7bd59617e..5164cd9ed5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1594,6 +1594,15 @@ static void max_x86_cpu_initfn(Object *obj)
         if (lmce_supported()) {
             object_property_set_bool(OBJECT(cpu), true, "lmce", &error_abort);
         }
+    } else {
+        object_property_set_str(OBJECT(cpu), CPUID_VENDOR_AMD,
+                                "vendor", &error_abort);
+        object_property_set_int(OBJECT(cpu), 6, "family", &error_abort);
+        object_property_set_int(OBJECT(cpu), 6, "model", &error_abort);
+        object_property_set_int(OBJECT(cpu), 3, "stepping", &error_abort);
+        object_property_set_str(OBJECT(cpu),
+                                "QEMU TCG CPU version " QEMU_HW_VERSION,
+                                "model-id", &error_abort);
     }
 
     object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH v2 3/3] i386: Don't set CPUClass::cpu_def on "max" model

[Eduardo Habkost]

Host CPUID info is used by the "max" CPU model only in KVM mode.
Move the initialization of CPUID data for "max" from class_init
to instance_init, and don't set CPUClass::cpu_def for "max".

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu-qom.h |  4 +++-
 target/i386/cpu.c     | 45 +++++++++++++++++++++------------------------
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
index 14abd0a8c1..f6c704c3a9 100644
--- a/target/i386/cpu-qom.h
+++ b/target/i386/cpu-qom.h
@@ -60,7 +60,9 @@ typedef struct X86CPUClass {
     CPUClass parent_class;
     /*< public >*/
 
-    /* Should be eventually replaced by subclass-specific property defaults. */
+    /* CPU definition, automatically loaded by instance_init if not NULL.
+     * Should be eventually replaced by subclass-specific property defaults.
+     */
     X86CPUDefinition *cpu_def;
 
     bool kvm_required;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5164cd9ed5..df0783e39b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1531,47 +1531,27 @@ static int cpu_x86_fill_model_id(char *str)
     return 0;
 }
 
-static X86CPUDefinition host_cpudef;
-
 static Property max_x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
     DEFINE_PROP_BOOL("host-cache-info", X86CPU, cache_info_passthrough, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-/* class_init for the "max" CPU model
- *
- * This function may be called before KVM is initialized.
- */
 static void max_x86_cpu_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
-    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
     xcc->ordering = 9;
 
-    host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
-    x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
-
-    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
-    host_cpudef.family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
-    host_cpudef.model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
-    host_cpudef.stepping = eax & 0x0F;
-
-    cpu_x86_fill_model_id(host_cpudef.model_id);
-
-    xcc->cpu_def = &host_cpudef;
     xcc->model_description =
         "Enables all features supported by the accelerator in the current host";
 
-    /* level, xlevel, xlevel2, and the feature words are initialized on
-     * instance_init, because they require KVM to be initialized.
-     */
-
     dc->props = max_x86_cpu_properties;
 }
 
+static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp);
+
 static void max_x86_cpu_initfn(Object *obj)
 {
     X86CPU *cpu = X86_CPU(obj);
@@ -1584,6 +1564,21 @@ static void max_x86_cpu_initfn(Object *obj)
     cpu->max_features = true;
 
     if (kvm_enabled()) {
+        X86CPUDefinition host_cpudef = { };
+        uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+        host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
+        x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
+
+        host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
+        host_cpudef.family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+        host_cpudef.model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+        host_cpudef.stepping = eax & 0x0F;
+
+        cpu_x86_fill_model_id(host_cpudef.model_id);
+
+        x86_cpu_load_def(cpu, &host_cpudef, &error_abort);
+
         env->cpuid_min_level =
             kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
         env->cpuid_min_xlevel =
@@ -2186,7 +2181,7 @@ static void x86_cpu_list_entry(gpointer data, gpointer user_data)
     CPUListState *s = user_data;
     char *name = x86_cpu_class_get_model_name(cc);
     const char *desc = cc->model_description;
-    if (!desc) {
+    if (!desc && cc->cpu_def) {
         desc = cc->cpu_def->model_id;
     }
 
@@ -3644,7 +3639,9 @@ static void x86_cpu_initfn(Object *obj)
     object_property_add_alias(obj, "sse4_1", obj, "sse4.1", &error_abort);
     object_property_add_alias(obj, "sse4_2", obj, "sse4.2", &error_abort);
 
-    x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
+    if (xcc->cpu_def) {
+        x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
+    }
 }
 
 static int64_t x86_cpu_get_arch_id(CPUState *cs)
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Eduardo Habkost]

Ping?

As v1 was sitting on the list since Jan 19, if there are no
objections I will merge this and include in my next pull request
before soft freeze.

On Wed, Feb 22, 2017 at 03:39:16PM -0300, Eduardo Habkost wrote:
> Changes v1 -> v2:
> * Fix build without CONFIG_KVM at lmce_supported()
> * Rebased on top of my x86-next branch:
>   https://github.com/ehabkost/qemu x86-next
> 
> Git branch for testing:
>   https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg
> 
> libvirt code to use the new feature already exist, and were
> submitted to libvir-list, at:
>   https://www.mail-archive.com/libvir-list@redhat.com/msg142168.html
> 
> ---
> 
> This is a replacement for the previous series that enabled the
> "host" CPU model on TCG. Now a new "max" CPU is being added,
> while keeping "host" KVM-specific.
> 
> In addition to simply adding "max" as a copy of the existing
> "host" CPU model, additional patches change it to not use any
> host CPUID information when in TCG mode.
> 
> ---
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: "Daniel P. Berrange" <berrange@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Jiri Denemark <jdenemar@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> 
> Eduardo Habkost (3):
>   i386: Create "max" CPU model
>   i386: Make "max" model not use any host CPUID info on TCG
>   i386: Don't set CPUClass::cpu_def on "max" model
> 
>  target/i386/cpu-qom.h |  4 ++-
>  target/i386/cpu.c     | 98 ++++++++++++++++++++++++++++++++-------------------
>  2 files changed, 64 insertions(+), 38 deletions(-)
> 
> -- 
> 2.11.0.259.g40922b1
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Richard W.M. Jones]

On Thu, Feb 23, 2017 at 05:07:47PM -0300, Eduardo Habkost wrote:
> Ping?
> 
> As v1 was sitting on the list since Jan 19, if there are no
> objections I will merge this and include in my next pull request
> before soft freeze.

Do you have a copy which applies on top of current HEAD?  I get
loads of conflicts.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH v2 2/3] i386: Make "max" model not use any host CPUID info on TCG

[David Hildenbrand]

Am 22.02.2017 um 19:39 schrieb Eduardo Habkost:
> Instead of reporting host CPUID data on "max", use the qemu64 CPU
Two questions:

1. Shouldn't "max" use cpuid of qemu64 with TARGET_X86_64 and qemu32
with !TARGET_X86_64?

2. Should we then change linux-user/main.c:main() to use the "max" model?

> model as reference to initialize CPUID
> vendor/family/model/stepping/model-id.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f7bd59617e..5164cd9ed5 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1594,6 +1594,15 @@ static void max_x86_cpu_initfn(Object *obj)
>          if (lmce_supported()) {
>              object_property_set_bool(OBJECT(cpu), true, "lmce", &error_abort);
>          }
> +    } else {
> +        object_property_set_str(OBJECT(cpu), CPUID_VENDOR_AMD,
> +                                "vendor", &error_abort);
> +        object_property_set_int(OBJECT(cpu), 6, "family", &error_abort);
> +        object_property_set_int(OBJECT(cpu), 6, "model", &error_abort);
> +        object_property_set_int(OBJECT(cpu), 3, "stepping", &error_abort);
> +        object_property_set_str(OBJECT(cpu),
> +                                "QEMU TCG CPU version " QEMU_HW_VERSION,
> +                                "model-id", &error_abort);
>      }
>  
>      object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
> 


-- 
Thanks,

David

Re: [Qemu-devel] [PATCH v2 3/3] i386: Don't set CPUClass::cpu_def on "max" model

[David Hildenbrand]

Am 22.02.2017 um 19:39 schrieb Eduardo Habkost:
> Host CPUID info is used by the "max" CPU model only in KVM mode.
> Move the initialization of CPUID data for "max" from class_init
> to instance_init, and don't set CPUClass::cpu_def for "max".

Doesn't that mean that the same code is now called for every cpu
instance vs. only once for the class?

-- 
Thanks,

David

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Eduardo Habkost]

On Thu, Feb 23, 2017 at 09:57:41PM +0000, Richard W.M. Jones wrote:
> On Thu, Feb 23, 2017 at 05:07:47PM -0300, Eduardo Habkost wrote:
> > Ping?
> > 
> > As v1 was sitting on the list since Jan 19, if there are no
> > objections I will merge this and include in my next pull request
> > before soft freeze.
> 
> Do you have a copy which applies on top of current HEAD?  I get
> loads of conflicts.

It is based on my x86-next branch. See cover letter for git URLs:

] * Rebased on top of my x86-next branch:
]   https://github.com/ehabkost/qemu x86-next
]
] Git branch for testing:
]   https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 2/3] i386: Make "max" model not use any host CPUID info on TCG

[Eduardo Habkost]

On Fri, Feb 24, 2017 at 12:09:32PM +0100, David Hildenbrand wrote:
> Am 22.02.2017 um 19:39 schrieb Eduardo Habkost:
> > Instead of reporting host CPUID data on "max", use the qemu64 CPU
> Two questions:
> 
> 1. Shouldn't "max" use cpuid of qemu64 with TARGET_X86_64 and qemu32
> with !TARGET_X86_64?

qemu32 has family=6,model=6,stepping=3 too, and the only
difference on TCG's "max" model on !TARGET_X86_64 will be the
lack of CPUID_EXT2_SYSCALL and CPUID_EXT2_LM.

> 
> 2. Should we then change linux-user/main.c:main() to use the "max" model?

We could, but I would like this to be done by somebody more
experienced with linux-user code, who could test it properly.

> 
> > model as reference to initialize CPUID
> > vendor/family/model/stepping/model-id.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target/i386/cpu.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index f7bd59617e..5164cd9ed5 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -1594,6 +1594,15 @@ static void max_x86_cpu_initfn(Object *obj)
> >          if (lmce_supported()) {
> >              object_property_set_bool(OBJECT(cpu), true, "lmce", &error_abort);
> >          }
> > +    } else {
> > +        object_property_set_str(OBJECT(cpu), CPUID_VENDOR_AMD,
> > +                                "vendor", &error_abort);
> > +        object_property_set_int(OBJECT(cpu), 6, "family", &error_abort);
> > +        object_property_set_int(OBJECT(cpu), 6, "model", &error_abort);
> > +        object_property_set_int(OBJECT(cpu), 3, "stepping", &error_abort);
> > +        object_property_set_str(OBJECT(cpu),
> > +                                "QEMU TCG CPU version " QEMU_HW_VERSION,
> > +                                "model-id", &error_abort);
> >      }
> >  
> >      object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
> > 
> 
> 
> -- 
> Thanks,
> 
> David

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 3/3] i386: Don't set CPUClass::cpu_def on "max" model

[Eduardo Habkost]

On Fri, Feb 24, 2017 at 12:13:05PM +0100, David Hildenbrand wrote:
> Am 22.02.2017 um 19:39 schrieb Eduardo Habkost:
> > Host CPUID info is used by the "max" CPU model only in KVM mode.
> > Move the initialization of CPUID data for "max" from class_init
> > to instance_init, and don't set CPUClass::cpu_def for "max".
> 
> Doesn't that mean that the same code is now called for every cpu
> instance vs. only once for the class?

Yes, but I don't see a problem with that.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 2/3] i386: Make "max" model not use any host CPUID info on TCG

[David Hildenbrand]

Am 24.02.2017 um 13:32 schrieb Eduardo Habkost:
> On Fri, Feb 24, 2017 at 12:09:32PM +0100, David Hildenbrand wrote:
>> Am 22.02.2017 um 19:39 schrieb Eduardo Habkost:
>>> Instead of reporting host CPUID data on "max", use the qemu64 CPU
>> Two questions:
>>
>> 1. Shouldn't "max" use cpuid of qemu64 with TARGET_X86_64 and qemu32
>> with !TARGET_X86_64?
> 
> qemu32 has family=6,model=6,stepping=3 too, and the only
> difference on TCG's "max" model on !TARGET_X86_64 will be the
> lack of CPUID_EXT2_SYSCALL and CPUID_EXT2_LM.

So this is already handled then, correct?

> 
>>
>> 2. Should we then change linux-user/main.c:main() to use the "max" model?
> 
> We could, but I would like this to be done by somebody more
> experienced with linux-user code, who could test it properly.
> 

Agreed!


-- 
Thanks,

David

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Richard W.M. Jones]

On Fri, Feb 24, 2017 at 09:26:30AM -0300, Eduardo Habkost wrote:
> On Thu, Feb 23, 2017 at 09:57:41PM +0000, Richard W.M. Jones wrote:
> > On Thu, Feb 23, 2017 at 05:07:47PM -0300, Eduardo Habkost wrote:
> > > Ping?
> > > 
> > > As v1 was sitting on the list since Jan 19, if there are no
> > > objections I will merge this and include in my next pull request
> > > before soft freeze.
> > 
> > Do you have a copy which applies on top of current HEAD?  I get
> > loads of conflicts.
> 
> It is based on my x86-next branch. See cover letter for git URLs:
> 
> ] * Rebased on top of my x86-next branch:
> ]   https://github.com/ehabkost/qemu x86-next
> ]
> ] Git branch for testing:
> ]   https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg

OK I've tried it now, and it works in both TCG and KVM cases.
If you want you can add:

 Tested-by: Richard W.M. Jones <rjones@redhat.com>

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH v2 2/3] i386: Make "max" model not use any host CPUID info on TCG

[Eduardo Habkost]

On Fri, Feb 24, 2017 at 01:34:04PM +0100, David Hildenbrand wrote:
> Am 24.02.2017 um 13:32 schrieb Eduardo Habkost:
> > On Fri, Feb 24, 2017 at 12:09:32PM +0100, David Hildenbrand wrote:
> >> Am 22.02.2017 um 19:39 schrieb Eduardo Habkost:
> >>> Instead of reporting host CPUID data on "max", use the qemu64 CPU
> >> Two questions:
> >>
> >> 1. Shouldn't "max" use cpuid of qemu64 with TARGET_X86_64 and qemu32
> >> with !TARGET_X86_64?
> > 
> > qemu32 has family=6,model=6,stepping=3 too, and the only
> > difference on TCG's "max" model on !TARGET_X86_64 will be the
> > lack of CPUID_EXT2_SYSCALL and CPUID_EXT2_LM.
> 
> So this is already handled then, correct?

Yes. I thought you were talking specifically about
family/model/stepping (which is the same on qemu64 and qemu32).

About the rest of the CPUID flags, the qemu64 and qemu32 flags
don't really reflect the TCG capabilities and shouldn't be used
as reference for TCG's "max" model. The TCG capabilities are
found in the TCG_*_FEATURES macros used as values for
FeatureWordInfo::tcg_features.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Peter Maydell]

On 22 February 2017 at 18:39, Eduardo Habkost <ehabkost@redhat.com> wrote:
> Changes v1 -> v2:
> * Fix build without CONFIG_KVM at lmce_supported()
> * Rebased on top of my x86-next branch:
>   https://github.com/ehabkost/qemu x86-next
>
> Git branch for testing:
>   https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg
>
> libvirt code to use the new feature already exist, and were
> submitted to libvir-list, at:
>   https://www.mail-archive.com/libvir-list@redhat.com/msg142168.html
>
> ---
>
> This is a replacement for the previous series that enabled the
> "host" CPU model on TCG. Now a new "max" CPU is being added,
> while keeping "host" KVM-specific.
>
> In addition to simply adding "max" as a copy of the existing
> "host" CPU model, additional patches change it to not use any
> host CPUID information when in TCG mode.


I had a look at implementing this for ARM, and ran into problems
because of how we've done '-cpu host'. For us the "host" CPU
type is registered dynamically when kvm_init() is called,
because (a) it only exists if -enable-kvm and (b) it probes
the kernel to find out what's available. So I could easily
add 'max' in the same place; but then where should I add the
type definition of 'max' for the non-KVM case?

Any suggestions?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[David Hildenbrand]

Am 24.02.2017 um 14:48 schrieb Peter Maydell:
> On 22 February 2017 at 18:39, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> Changes v1 -> v2:
>> * Fix build without CONFIG_KVM at lmce_supported()
>> * Rebased on top of my x86-next branch:
>>   https://github.com/ehabkost/qemu x86-next
>>
>> Git branch for testing:
>>   https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg
>>
>> libvirt code to use the new feature already exist, and were
>> submitted to libvir-list, at:
>>   https://www.mail-archive.com/libvir-list@redhat.com/msg142168.html
>>
>> ---
>>
>> This is a replacement for the previous series that enabled the
>> "host" CPU model on TCG. Now a new "max" CPU is being added,
>> while keeping "host" KVM-specific.
>>
>> In addition to simply adding "max" as a copy of the existing
>> "host" CPU model, additional patches change it to not use any
>> host CPUID information when in TCG mode.
> 
> 
> I had a look at implementing this for ARM, and ran into problems
> because of how we've done '-cpu host'. For us the "host" CPU
> type is registered dynamically when kvm_init() is called,
> because (a) it only exists if -enable-kvm and (b) it probes
> the kernel to find out what's available. So I could easily
> add 'max' in the same place; but then where should I add the
> type definition of 'max' for the non-KVM case?
> 
> Any suggestions?

Maybe switch to some late initialization?

On s390x, we register the host model right away, but fill it with life
in the initfn. If KVM is disabled, the host model will be available, but
we will deny to use it (via kvm_required).

For max, it would be the same thing. The initfn will simply inititalize
the model differently, depending on the selected accelerator.

When the models are inititalized, kvm is already up and running, so it
the kernel can be properly probed.

Not sure if that helps :)

-- 
Thanks,

David

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Eduardo Habkost]

On Fri, Feb 24, 2017 at 01:48:46PM +0000, Peter Maydell wrote:
> On 22 February 2017 at 18:39, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > Changes v1 -> v2:
> > * Fix build without CONFIG_KVM at lmce_supported()
> > * Rebased on top of my x86-next branch:
> >   https://github.com/ehabkost/qemu x86-next
> >
> > Git branch for testing:
> >   https://github.com/ehabkost/qemu-hacks work/x86-cpu-max-tcg
> >
> > libvirt code to use the new feature already exist, and were
> > submitted to libvir-list, at:
> >   https://www.mail-archive.com/libvir-list@redhat.com/msg142168.html
> >
> > ---
> >
> > This is a replacement for the previous series that enabled the
> > "host" CPU model on TCG. Now a new "max" CPU is being added,
> > while keeping "host" KVM-specific.
> >
> > In addition to simply adding "max" as a copy of the existing
> > "host" CPU model, additional patches change it to not use any
> > host CPUID information when in TCG mode.
> 
> 
> I had a look at implementing this for ARM, and ran into problems
> because of how we've done '-cpu host'. For us the "host" CPU
> type is registered dynamically when kvm_init() is called,
> because (a) it only exists if -enable-kvm and (b) it probes
> the kernel to find out what's available. So I could easily
> add 'max' in the same place; but then where should I add the
> type definition of 'max' for the non-KVM case?

I recommend registering the type unconditionally, moving
KVM-specific initialization to ->instance_init() and/or
->realize() (being careful to make instance_init() not crash if
KVM is disabled), and make ->realize() fail if KVM is disabled.
IIRC, we did exactly that on x86 a while ago.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Peter Maydell]

On 24 February 2017 at 17:11, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Feb 24, 2017 at 01:48:46PM +0000, Peter Maydell wrote:
>> I had a look at implementing this for ARM, and ran into problems
>> because of how we've done '-cpu host'. For us the "host" CPU
>> type is registered dynamically when kvm_init() is called,
>> because (a) it only exists if -enable-kvm and (b) it probes
>> the kernel to find out what's available. So I could easily
>> add 'max' in the same place; but then where should I add the
>> type definition of 'max' for the non-KVM case?
>
> I recommend registering the type unconditionally, moving
> KVM-specific initialization to ->instance_init() and/or
> ->realize() (being careful to make instance_init() not crash if
> KVM is disabled), and make ->realize() fail if KVM is disabled.
> IIRC, we did exactly that on x86 a while ago.

Thanks, sounds reasonable. Also sounds like more rework than
I want to try to write, test and cram into QEMU before freeze
deadline on Tuesday, so I guess we'll have to wait til after
2.9; a pity, but can't be helped.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] i386: Add "max" CPU model to TCG and KVM

[Peter Maydell]

On 24 February 2017 at 18:04, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 24 February 2017 at 17:11, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> On Fri, Feb 24, 2017 at 01:48:46PM +0000, Peter Maydell wrote:
>>> I had a look at implementing this for ARM, and ran into problems
>>> because of how we've done '-cpu host'. For us the "host" CPU
>>> type is registered dynamically when kvm_init() is called,
>>> because (a) it only exists if -enable-kvm and (b) it probes
>>> the kernel to find out what's available. So I could easily
>>> add 'max' in the same place; but then where should I add the
>>> type definition of 'max' for the non-KVM case?
>>
>> I recommend registering the type unconditionally, moving
>> KVM-specific initialization to ->instance_init() and/or
>> ->realize() (being careful to make instance_init() not crash if
>> KVM is disabled), and make ->realize() fail if KVM is disabled.
>> IIRC, we did exactly that on x86 a while ago.
>
> Thanks, sounds reasonable. Also sounds like more rework than
> I want to try to write, test and cram into QEMU before freeze
> deadline on Tuesday, so I guess we'll have to wait til after
> 2.9; a pity, but can't be helped.

I remembered this week that I'd completely forgotten that
we should have a go at "cpu max" for ARM in the 2.10 cycle.
Maybe for 2.11 :-)

thanks
-- PMM