qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27
@ 2016-09-27 20:12 Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 01/20] target-i386: Remove unused X86CPUDefinition::xlevel2 field Eduardo Habkost
                   ` (20 more replies)
  0 siblings, 21 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

The following changes since commit 333ec4ca6a9f604331e2349cb91e9635f65d6462:

  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-09-27 16:23:08 +0100)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/x86-pull-request

for you to fetch changes up to 4f01a637795af77f1c191230b9f6e3a2547b0c28:

  sysbus: Remove ignored return value of FindSysbusDeviceFunc (2016-09-27 17:03:34 -0300)

----------------------------------------------------------------
x86 and machine queue, 2016-09-27

----------------------------------------------------------------

David Gibson (1):
  sysbus: Remove ignored return value of FindSysbusDeviceFunc

Eduardo Habkost (18):
  target-i386: Remove unused X86CPUDefinition::xlevel2 field
  target-i386: Add a marker to end of the region zeroed on reset
  tests: Add test code for CPUID level/xlevel handling
  tests: Test CPUID level handling for old machines
  target-i386: Automatically set level/xlevel/xlevel2 when needed
  target-i386: Enable CPUID[0x8000000A] if SVM is enabled
  target-i386: Move feature name arrays inside FeatureWordInfo
  target-i386: Don't try to enable PT State xsave component
  target-i386: xsave: Calculate enabled components only once
  target-i386: xsave: Simplify CPUID[0xD,0].{EAX,EDX} calculation
  target-i386: xsave: Helper function to calculate xsave area size
  target-i386: xsave: Calculate set of xsave components on realize
  target-i386: Move xsave component mask to features array
  target-i386: Remove has_msr_mtrr global variable
  target-i386: Remove has_msr_hv_apic global variable
  target-i386: Remove has_msr_hv_tsc global variable
  target-i386: Clear KVM CPUID features if KVM is disabled
  target-i386: Remove has_msr_* global vars for KVM features

Marc-André Lureau (1):
  linux-user: remove #define smp_{cores, threads}

 hw/arm/sysbus-fdt.c           |   4 +-
 hw/core/machine.c             |   2 +-
 hw/core/platform-bus.c        |   8 +-
 hw/ppc/e500.c                 |   4 +-
 hw/ppc/spapr.c                |   4 +-
 include/hw/i386/pc.h          |   5 +
 include/hw/sysbus.h           |   2 +-
 include/sysemu/cpus.h         |   5 +-
 target-i386/cpu.c             | 565 ++++++++++++++++++++++++------------------
 target-i386/cpu.h             |  15 +-
 target-i386/kvm.c             |  51 ++--
 target-ppc/translate_init.c   |   3 +-
 tests/.gitignore              |   1 +
 tests/Makefile.include        |   2 +
 tests/test-x86-cpuid-compat.c | 171 +++++++++++++
 15 files changed, 540 insertions(+), 302 deletions(-)
 create mode 100644 tests/test-x86-cpuid-compat.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 01/20] target-i386: Remove unused X86CPUDefinition::xlevel2 field
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 02/20] target-i386: Add a marker to end of the region zeroed on reset Eduardo Habkost
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

No CPU model in builtin_x86_defs has xlevel2 set, so it is always
zero. Delete the field.

Note that this is not an user-visible change. It doesn't remove
the ability to set xlevel2 on the command-line, it just removes
an unused field in builtin_x86_defs.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index db12728..920b78f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -761,7 +761,6 @@ struct X86CPUDefinition {
     const char *name;
     uint32_t level;
     uint32_t xlevel;
-    uint32_t xlevel2;
     /* vendor is zero-terminated, 12 character ASCII string */
     char vendor[CPUID_VENDOR_SZ + 1];
     int family;
@@ -2214,7 +2213,6 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
     object_property_set_int(OBJECT(cpu), def->model, "model", errp);
     object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
     object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
-    object_property_set_int(OBJECT(cpu), def->xlevel2, "xlevel2", errp);
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
     for (w = 0; w < FEATURE_WORDS; w++) {
         env->features[w] = def->features[w];
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 02/20] target-i386: Add a marker to end of the region zeroed on reset
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 01/20] target-i386: Remove unused X86CPUDefinition::xlevel2 field Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 03/20] tests: Add test code for CPUID level/xlevel handling Eduardo Habkost
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Instead of using cpuid_level, use an empty struct as a marker
(like we already did with {start,end}_init_save). This will avoid
accidentaly resetting the wrong fields if we change the field
ordering on CPUX86State.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 2 +-
 target-i386/cpu.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 920b78f..26f0e59 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2714,7 +2714,7 @@ static void x86_cpu_reset(CPUState *s)
 
     xcc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUX86State, cpuid_level));
+    memset(env, 0, offsetof(CPUX86State, end_reset_fields));
 
     tlb_flush(s, 1);
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 27af9c3..604d591 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1108,6 +1108,7 @@ typedef struct CPUX86State {
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
+    struct {} end_reset_fields;
 
     /* processor features (e.g. for CPUID insn) */
     uint32_t cpuid_level;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 03/20] tests: Add test code for CPUID level/xlevel handling
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 01/20] target-i386: Remove unused X86CPUDefinition::xlevel2 field Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 02/20] target-i386: Add a marker to end of the region zeroed on reset Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 04/20] tests: Test CPUID level handling for old machines Eduardo Habkost
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Add test code that will check if the automatic CPUID level
changes are working as expected.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/.gitignore              |   1 +
 tests/Makefile.include        |   2 +
 tests/test-x86-cpuid-compat.c | 108 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)
 create mode 100644 tests/test-x86-cpuid-compat.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 24ac6cf..0f0c79b 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -75,6 +75,7 @@ test-visitor-serialization
 test-vmstate
 test-write-threshold
 test-x86-cpuid
+test-x86-cpuid-compat
 test-xbzrle
 test-netfilter
 test-filter-mirror
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2aa78c1..8162f6f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -243,6 +243,7 @@ check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
 check-qtest-i386-y += tests/test-filter-redirector$(EXESUF)
 check-qtest-i386-y += tests/postcopy-test$(EXESUF)
+check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
 check-qtest-x86_64-y += $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -663,6 +664,7 @@ tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-o
 tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
 tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
 tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
+tests/test-x86-cpuid-compat$(EXESUF): tests/test-x86-cpuid-compat.o $(qtest-obj-y)
 tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o
 tests/ptimer-test$(EXESUF): tests/ptimer-test.o tests/ptimer-test-stubs.o hw/core/ptimer.o
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
new file mode 100644
index 0000000..b81cfeb
--- /dev/null
+++ b/tests/test-x86-cpuid-compat.c
@@ -0,0 +1,108 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qint.h"
+#include "libqtest.h"
+
+static char *get_cpu0_qom_path(void)
+{
+    QDict *resp;
+    QList *ret;
+    QDict *cpu0;
+    char *path;
+
+    resp = qmp("{'execute': 'query-cpus', 'arguments': {}}");
+    g_assert(qdict_haskey(resp, "return"));
+    ret = qdict_get_qlist(resp, "return");
+
+    cpu0 = qobject_to_qdict(qlist_peek(ret));
+    path = g_strdup(qdict_get_str(cpu0, "qom_path"));
+    QDECREF(resp);
+    return path;
+}
+
+static QObject *qom_get(const char *path, const char *prop)
+{
+    QDict *resp = qmp("{ 'execute': 'qom-get',"
+                      "  'arguments': { 'path': %s,"
+                      "                 'property': %s } }",
+                      path, prop);
+    QObject *ret = qdict_get(resp, "return");
+    qobject_incref(ret);
+    QDECREF(resp);
+    return ret;
+}
+
+typedef struct CpuidTestArgs {
+    const char *cmdline;
+    const char *property;
+    int64_t expected_value;
+} CpuidTestArgs;
+
+static void test_cpuid_prop(const void *data)
+{
+    const CpuidTestArgs *args = data;
+    char *path;
+    QInt *value;
+
+    qtest_start(args->cmdline);
+    path = get_cpu0_qom_path();
+    value = qobject_to_qint(qom_get(path, args->property));
+    g_assert_cmpint(qint_get_int(value), ==, args->expected_value);
+    qtest_end();
+
+    QDECREF(value);
+    g_free(path);
+}
+
+static void add_cpuid_test(const char *name, const char *cmdline,
+                           const char *property, int64_t expected_value)
+{
+    CpuidTestArgs *args = g_new0(CpuidTestArgs, 1);
+    args->cmdline = cmdline;
+    args->property = property;
+    args->expected_value = expected_value;
+    qtest_add_data_func(name, args, test_cpuid_prop);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    /* Original level values for CPU models: */
+    add_cpuid_test("x86/cpuid/phenom/level",
+                   "-cpu phenom", "level", 5);
+    add_cpuid_test("x86/cpuid/Conroe/level",
+                   "-cpu Conroe", "level", 10);
+    add_cpuid_test("x86/cpuid/SandyBridge/level",
+                   "-cpu SandyBridge", "level", 0xd);
+    add_cpuid_test("x86/cpuid/486/xlevel",
+                   "-cpu 486", "xlevel", 0);
+    add_cpuid_test("x86/cpuid/core2duo/xlevel",
+                   "-cpu core2duo", "xlevel", 0x80000008);
+    add_cpuid_test("x86/cpuid/phenom/xlevel",
+                   "-cpu phenom", "xlevel", 0x8000001A);
+
+    /* If level is not large enough, it should increase automatically: */
+    /* CPUID[EAX=7,ECX=0].EBX: */
+    add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
+                   "-cpu phenom,+fsgsbase", "level", 7);
+
+    /* If level is already large enough, it shouldn't change: */
+    add_cpuid_test("x86/cpuid/auto-level/SandyBridge/multiple",
+                   "-cpu SandyBridge,+arat,+fsgsbase,+avx512vbmi",
+                   "level", 0xd);
+
+    /* if xlevel is already large enough, it shouldn't change: */
+    add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
+                   "-cpu phenom,+3dnow,+sse4a,+invtsc,+npt",
+                   "xlevel", 0x8000001A);
+
+    /* if xlevel2 is already large enough, it shouldn't change: */
+    add_cpuid_test("x86/cpuid/auto-xlevel2/486/fixed",
+                   "-cpu 486,xlevel2=0xC0000002,+xstore",
+                   "xlevel2", 0xC0000002);
+
+    return g_test_run();
+}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 04/20] tests: Test CPUID level handling for old machines
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (2 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 03/20] tests: Add test code for CPUID level/xlevel handling Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 05/20] target-i386: Automatically set level/xlevel/xlevel2 when needed Eduardo Habkost
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

We're going to change the way level/xlevel/xlevel2 are handled
when enabling features, but we need to keep the old behavior on
existing machine types. Add test cases for that.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/test-x86-cpuid-compat.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index b81cfeb..f7003ee 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -104,5 +104,18 @@ int main(int argc, char **argv)
                    "-cpu 486,xlevel2=0xC0000002,+xstore",
                    "xlevel2", 0xC0000002);
 
+    /* Check compatibility of old machine-types that didn't
+     * auto-increase level/xlevel/xlevel2: */
+
+    add_cpuid_test("x86/cpuid/auto-level/pc-2.7",
+                   "-machine pc-i440fx-2.7 -cpu 486,+arat,+avx512vbmi,+xsaveopt",
+                   "level", 1);
+    add_cpuid_test("x86/cpuid/auto-xlevel/pc-2.7",
+                   "-machine pc-i440fx-2.7 -cpu 486,+3dnow,+sse4a,+invtsc,+npt",
+                   "xlevel", 0);
+    add_cpuid_test("x86/cpuid/auto-xlevel2/pc-2.7",
+                   "-machine pc-i440fx-2.7 -cpu 486,+xstore",
+                   "xlevel2", 0);
+
     return g_test_run();
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 05/20] target-i386: Automatically set level/xlevel/xlevel2 when needed
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (3 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 04/20] tests: Test CPUID level handling for old machines Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 06/20] target-i386: Enable CPUID[0x8000000A] if SVM is enabled Eduardo Habkost
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Instead of requiring users and management software to be aware of
required CPUID level/xlevel/xlevel2 values for each feature,
automatically increase those values when features need them.

This was already done for CPUID[7].EBX, and is now made generic
for all CPUID feature flags. Unit test included, to make sure we
don't break ABI on older machine-types and don't mess with the
CPUID level values if they are explicitly set by the user.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/i386/pc.h          |  5 +++
 target-i386/cpu.c             | 84 +++++++++++++++++++++++++++++++++++++------
 target-i386/cpu.h             | 12 +++++--
 tests/test-x86-cpuid-compat.c | 45 +++++++++++++++++++++++
 4 files changed, 133 insertions(+), 13 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 29a6c9b..47bdf10 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -374,6 +374,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = TYPE_X86_CPU,\
         .property = "l3-cache",\
         .value    = "off",\
+    },\
+    {\
+        .driver   = TYPE_X86_CPU,\
+        .property = "full-cpuid-auto-level",\
+        .value    = "off",\
     },
 
 #define PC_COMPAT_2_6 \
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 26f0e59..24893d2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1643,9 +1643,12 @@ static void host_x86_cpu_initfn(Object *obj)
 
     /* If KVM is disabled, x86_cpu_realizefn() will report an error later */
     if (kvm_enabled()) {
-        env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
-        env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
-        env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+        env->cpuid_min_level =
+            kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+        env->cpuid_min_xlevel =
+            kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+        env->cpuid_min_xlevel2 =
+            kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
 
         if (lmce_supported()) {
             object_property_set_bool(OBJECT(cpu), true, "lmce", &error_abort);
@@ -2208,11 +2211,13 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
     char host_vendor[CPUID_VENDOR_SZ + 1];
     FeatureWord w;
 
-    object_property_set_int(OBJECT(cpu), def->level, "level", errp);
+    /* CPU models only set _minimum_ values for level/xlevel: */
+    object_property_set_int(OBJECT(cpu), def->level, "min-level", errp);
+    object_property_set_int(OBJECT(cpu), def->xlevel, "min-xlevel", errp);
+
     object_property_set_int(OBJECT(cpu), def->family, "family", errp);
     object_property_set_int(OBJECT(cpu), def->model, "model", errp);
     object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
-    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
     for (w = 0; w < FEATURE_WORDS; w++) {
         env->features[w] = def->features[w];
@@ -2951,6 +2956,38 @@ static uint32_t x86_host_phys_bits(void)
     return host_phys_bits;
 }
 
+static void x86_cpu_adjust_level(X86CPU *cpu, uint32_t *min, uint32_t value)
+{
+    if (*min < value) {
+        *min = value;
+    }
+}
+
+/* Increase cpuid_min_{level,xlevel,xlevel2} automatically, if appropriate */
+static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
+{
+    CPUX86State *env = &cpu->env;
+    FeatureWordInfo *fi = &feature_word_info[w];
+    uint32_t eax = fi->cpuid_eax;
+    uint32_t region = eax & 0xF0000000;
+
+    if (!env->features[w]) {
+        return;
+    }
+
+    switch (region) {
+    case 0x00000000:
+        x86_cpu_adjust_level(cpu, &env->cpuid_min_level, eax);
+    break;
+    case 0x80000000:
+        x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, eax);
+    break;
+    case 0xC0000000:
+        x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel2, eax);
+    break;
+    }
+}
+
 #define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
                            (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
                            (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
@@ -2996,8 +3033,31 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         cpu->env.features[w] &= ~minus_features[w];
     }
 
-    if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) {
-        env->cpuid_level = 7;
+
+    /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
+    x86_cpu_adjust_feat_level(cpu, FEAT_7_0_EBX);
+    if (cpu->full_cpuid_auto_level) {
+        x86_cpu_adjust_feat_level(cpu, FEAT_1_EDX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_1_ECX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_6_EAX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_7_0_ECX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_8000_0001_EDX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_8000_0001_ECX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_8000_0007_EDX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_C000_0001_EDX);
+        x86_cpu_adjust_feat_level(cpu, FEAT_SVM);
+        x86_cpu_adjust_feat_level(cpu, FEAT_XSAVE);
+    }
+
+    /* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set */
+    if (env->cpuid_level == UINT32_MAX) {
+        env->cpuid_level = env->cpuid_min_level;
+    }
+    if (env->cpuid_xlevel == UINT32_MAX) {
+        env->cpuid_xlevel = env->cpuid_min_xlevel;
+    }
+    if (env->cpuid_xlevel2 == UINT32_MAX) {
+        env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
     }
 
     if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
@@ -3403,9 +3463,13 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
     DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
     DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
-    DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
-    DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
-    DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
+    DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, UINT32_MAX),
+    DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, UINT32_MAX),
+    DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, UINT32_MAX),
+    DEFINE_PROP_UINT32("min-level", X86CPU, env.cpuid_min_level, 0),
+    DEFINE_PROP_UINT32("min-xlevel", X86CPU, env.cpuid_min_xlevel, 0),
+    DEFINE_PROP_UINT32("min-xlevel2", X86CPU, env.cpuid_min_xlevel2, 0),
+    DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true),
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
     DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
     DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 604d591..aaa45f0 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1111,9 +1111,12 @@ typedef struct CPUX86State {
     struct {} end_reset_fields;
 
     /* processor features (e.g. for CPUID insn) */
-    uint32_t cpuid_level;
-    uint32_t cpuid_xlevel;
-    uint32_t cpuid_xlevel2;
+    /* Minimum level/xlevel/xlevel2, based on CPU model + features */
+    uint32_t cpuid_min_level, cpuid_min_xlevel, cpuid_min_xlevel2;
+    /* Maximum level/xlevel/xlevel2 value for auto-assignment: */
+    uint32_t cpuid_max_level, cpuid_max_xlevel, cpuid_max_xlevel2;
+    /* Actual level/xlevel/xlevel2 value: */
+    uint32_t cpuid_level, cpuid_xlevel, cpuid_xlevel2;
     uint32_t cpuid_vendor1;
     uint32_t cpuid_vendor2;
     uint32_t cpuid_vendor3;
@@ -1218,6 +1221,9 @@ struct X86CPU {
     /* Compatibility bits for old machine types: */
     bool enable_cpuid_0xb;
 
+    /* Enable auto level-increase for all CPUID leaves */
+    bool full_cpuid_auto_level;
+
     /* if true fill the top bits of the MTRR_PHYSMASKn variable range */
     bool fill_mtrr_mask;
 
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index f7003ee..83f1c80 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -85,19 +85,64 @@ int main(int argc, char **argv)
                    "-cpu phenom", "xlevel", 0x8000001A);
 
     /* If level is not large enough, it should increase automatically: */
+    /* CPUID[6].EAX: */
+    add_cpuid_test("x86/cpuid/auto-level/phenom/arat",
+                   "-cpu 486,+arat", "level", 6);
     /* CPUID[EAX=7,ECX=0].EBX: */
     add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
                    "-cpu phenom,+fsgsbase", "level", 7);
+    /* CPUID[EAX=7,ECX=0].ECX: */
+    add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
+                   "-cpu phenom,+avx512vbmi", "level", 7);
+    /* CPUID[EAX=0xd,ECX=1].EAX: */
+    add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
+                   "-cpu phenom,+xsaveopt", "level", 0xd);
+    /* CPUID[8000_0001].EDX: */
+    add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
+                   "-cpu 486,+3dnow", "xlevel", 0x80000001);
+    /* CPUID[8000_0001].ECX: */
+    add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
+                   "-cpu 486,+sse4a", "xlevel", 0x80000001);
+    /* CPUID[8000_0007].EDX: */
+    add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
+                   "-cpu 486,+invtsc", "xlevel", 0x80000007);
+    /* CPUID[8000_000A].EDX: */
+    add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
+                   "-cpu 486,+npt", "xlevel", 0x8000000A);
+    /* CPUID[C000_0001].EDX: */
+    add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
+                   "-cpu phenom,+xstore", "xlevel2", 0xC0000001);
+
 
     /* If level is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-level/SandyBridge/multiple",
                    "-cpu SandyBridge,+arat,+fsgsbase,+avx512vbmi",
                    "level", 0xd);
+    /* If level is explicitly set, it shouldn't change: */
+    add_cpuid_test("x86/cpuid/auto-level/486/fixed/0xF",
+                   "-cpu 486,level=0xF,+arat,+fsgsbase,+avx512vbmi,+xsaveopt",
+                   "level", 0xF);
+    add_cpuid_test("x86/cpuid/auto-level/486/fixed/2",
+                   "-cpu 486,level=2,+arat,+fsgsbase,+avx512vbmi,+xsaveopt",
+                   "level", 2);
+    add_cpuid_test("x86/cpuid/auto-level/486/fixed/0",
+                   "-cpu 486,level=0,+arat,+fsgsbase,+avx512vbmi,+xsaveopt",
+                   "level", 0);
 
     /* if xlevel is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
                    "-cpu phenom,+3dnow,+sse4a,+invtsc,+npt",
                    "xlevel", 0x8000001A);
+    /* If xlevel is explicitly set, it shouldn't change: */
+    add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/80000002",
+                   "-cpu 486,xlevel=0x80000002,+3dnow,+sse4a,+invtsc,+npt",
+                   "xlevel", 0x80000002);
+    add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/8000001A",
+                   "-cpu 486,xlevel=0x8000001A,+3dnow,+sse4a,+invtsc,+npt",
+                   "xlevel", 0x8000001A);
+    add_cpuid_test("x86/cpuid/auto-xlevel/phenom/fixed/0",
+                   "-cpu 486,xlevel=0,+3dnow,+sse4a,+invtsc,+npt",
+                   "xlevel", 0);
 
     /* if xlevel2 is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel2/486/fixed",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 06/20] target-i386: Enable CPUID[0x8000000A] if SVM is enabled
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (4 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 05/20] target-i386: Automatically set level/xlevel/xlevel2 when needed Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 07/20] linux-user: remove #define smp_{cores, threads} Eduardo Habkost
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

SVM needs CPUID[0x8000000A] to be available. So if SVM is enabled
in a CPU model or explicitly in the command-line, adjust CPUID
xlevel to expose the CPUID[0x8000000A] leaf.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c             |  4 ++++
 tests/test-x86-cpuid-compat.c | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 24893d2..7a5da99 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3047,6 +3047,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         x86_cpu_adjust_feat_level(cpu, FEAT_C000_0001_EDX);
         x86_cpu_adjust_feat_level(cpu, FEAT_SVM);
         x86_cpu_adjust_feat_level(cpu, FEAT_XSAVE);
+        /* SVM requires CPUID[0x8000000A] */
+        if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
+            x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A);
+        }
     }
 
     /* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set */
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 83f1c80..83162a4 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -83,6 +83,8 @@ int main(int argc, char **argv)
                    "-cpu core2duo", "xlevel", 0x80000008);
     add_cpuid_test("x86/cpuid/phenom/xlevel",
                    "-cpu phenom", "xlevel", 0x8000001A);
+    add_cpuid_test("x86/cpuid/athlon/xlevel",
+                   "-cpu athlon", "xlevel", 0x80000008);
 
     /* If level is not large enough, it should increase automatically: */
     /* CPUID[6].EAX: */
@@ -112,6 +114,9 @@ int main(int argc, char **argv)
     /* CPUID[C000_0001].EDX: */
     add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
                    "-cpu phenom,+xstore", "xlevel2", 0xC0000001);
+    /* SVM needs CPUID[0x8000000A] */
+    add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
+                   "-cpu athlon,+svm", "xlevel", 0x8000000A);
 
 
     /* If level is already large enough, it shouldn't change: */
@@ -131,17 +136,17 @@ int main(int argc, char **argv)
 
     /* if xlevel is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
-                   "-cpu phenom,+3dnow,+sse4a,+invtsc,+npt",
+                   "-cpu phenom,+3dnow,+sse4a,+invtsc,+npt,+svm",
                    "xlevel", 0x8000001A);
     /* If xlevel is explicitly set, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/80000002",
-                   "-cpu 486,xlevel=0x80000002,+3dnow,+sse4a,+invtsc,+npt",
+                   "-cpu 486,xlevel=0x80000002,+3dnow,+sse4a,+invtsc,+npt,+svm",
                    "xlevel", 0x80000002);
     add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/8000001A",
-                   "-cpu 486,xlevel=0x8000001A,+3dnow,+sse4a,+invtsc,+npt",
+                   "-cpu 486,xlevel=0x8000001A,+3dnow,+sse4a,+invtsc,+npt,+svm",
                    "xlevel", 0x8000001A);
     add_cpuid_test("x86/cpuid/auto-xlevel/phenom/fixed/0",
-                   "-cpu 486,xlevel=0,+3dnow,+sse4a,+invtsc,+npt",
+                   "-cpu 486,xlevel=0,+3dnow,+sse4a,+invtsc,+npt,+svm",
                    "xlevel", 0);
 
     /* if xlevel2 is already large enough, it shouldn't change: */
@@ -156,7 +161,7 @@ int main(int argc, char **argv)
                    "-machine pc-i440fx-2.7 -cpu 486,+arat,+avx512vbmi,+xsaveopt",
                    "level", 1);
     add_cpuid_test("x86/cpuid/auto-xlevel/pc-2.7",
-                   "-machine pc-i440fx-2.7 -cpu 486,+3dnow,+sse4a,+invtsc,+npt",
+                   "-machine pc-i440fx-2.7 -cpu 486,+3dnow,+sse4a,+invtsc,+npt,+svm",
                    "xlevel", 0);
     add_cpuid_test("x86/cpuid/auto-xlevel2/pc-2.7",
                    "-machine pc-i440fx-2.7 -cpu 486,+xstore",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 07/20] linux-user: remove #define smp_{cores, threads}
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (5 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 06/20] target-i386: Enable CPUID[0x8000000A] if SVM is enabled Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 08/20] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Paolo Bonzini, Richard Henderson, qemu-devel,
	Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Those are unneeded now that CPUState nr_{cores,threads} is always
initialized.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/sysemu/cpus.h       | 5 +----
 target-i386/cpu.c           | 8 ++++----
 target-ppc/translate_init.c | 3 ++-
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index fe992a8..3728a1e 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -29,12 +29,9 @@ void qtest_clock_warp(int64_t dest);
 
 #ifndef CONFIG_USER_ONLY
 /* vl.c */
+/* *-user doesn't have configurable SMP topology */
 extern int smp_cores;
 extern int smp_threads;
-#else
-/* *-user doesn't have configurable SMP topology */
-#define smp_cores   1
-#define smp_threads 1
 #endif
 
 void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7a5da99..a5d3b1a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2498,13 +2498,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 
         switch (count) {
         case 0:
-            *eax = apicid_core_offset(smp_cores, smp_threads);
-            *ebx = smp_threads;
+            *eax = apicid_core_offset(cs->nr_cores, cs->nr_threads);
+            *ebx = cs->nr_threads;
             *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
             break;
         case 1:
-            *eax = apicid_pkg_offset(smp_cores, smp_threads);
-            *ebx = smp_cores * smp_threads;
+            *eax = apicid_pkg_offset(cs->nr_cores, cs->nr_threads);
+            *ebx = cs->nr_cores * cs->nr_threads;
             *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
             break;
         default:
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 407ccb9..b66b40b 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9943,7 +9943,8 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
 
 int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
 {
-    int ret = MIN(smp_threads, kvmppc_smt_threads());
+    CPUState *cs = CPU(cpu);
+    int ret = MIN(cs->nr_threads, kvmppc_smt_threads());
 
     switch (cpu->cpu_version) {
     case CPU_POWERPC_LOGICAL_2_05:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 08/20] target-i386: Move feature name arrays inside FeatureWordInfo
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (6 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 07/20] linux-user: remove #define smp_{cores, threads} Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 09/20] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

It makes it easier to guarantee the arrays are the right size,
and to find information when looking at the code.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 370 +++++++++++++++++++++++++-----------------------------
 1 file changed, 170 insertions(+), 200 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a5d3b1a..cc07fdb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -181,185 +181,6 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
     dst[CPUID_VENDOR_SZ] = '\0';
 }
 
-/* feature flags taken from "Intel Processor Identification and the CPUID
- * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
- * between feature naming conventions, aliases may be added.
- */
-static const char *feature_name[] = {
-    "fpu", "vme", "de", "pse",
-    "tsc", "msr", "pae", "mce",
-    "cx8", "apic", NULL, "sep",
-    "mtrr", "pge", "mca", "cmov",
-    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
-    NULL, "ds" /* Intel dts */, "acpi", "mmx",
-    "fxsr", "sse", "sse2", "ss",
-    "ht" /* Intel htt */, "tm", "ia64", "pbe",
-};
-static const char *ext_feature_name[] = {
-    "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
-    "ds_cpl", "vmx", "smx", "est",
-    "tm2", "ssse3", "cid", NULL,
-    "fma", "cx16", "xtpr", "pdcm",
-    NULL, "pcid", "dca", "sse4.1|sse4_1",
-    "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
-    "tsc-deadline", "aes", "xsave", "osxsave",
-    "avx", "f16c", "rdrand", "hypervisor",
-};
-/* Feature names that are already defined on feature_name[] but are set on
- * CPUID[8000_0001].EDX on AMD CPUs don't have their names on
- * ext2_feature_name[]. They are copied automatically to cpuid_ext2_features
- * if and only if CPU vendor is AMD.
- */
-static const char *ext2_feature_name[] = {
-    NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
-    NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
-    NULL /* cx8 */ /* AMD CMPXCHG8B */, NULL /* apic */, NULL, "syscall",
-    NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
-    NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
-    "nx|xd", NULL, "mmxext", NULL /* mmx */,
-    NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb" /* AMD Page1GB */, "rdtscp",
-    NULL, "lm|i64", "3dnowext", "3dnow",
-};
-static const char *ext3_feature_name[] = {
-    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
-    "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
-    "3dnowprefetch", "osvw", "ibs", "xop",
-    "skinit", "wdt", NULL, "lwp",
-    "fma4", "tce", NULL, "nodeid_msr",
-    NULL, "tbm", "topoext", "perfctr_core",
-    "perfctr_nb", NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *ext4_feature_name[] = {
-    NULL, NULL, "xstore", "xstore-en",
-    NULL, NULL, "xcrypt", "xcrypt-en",
-    "ace2", "ace2-en", "phe", "phe-en",
-    "pmm", "pmm-en", NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *kvm_feature_name[] = {
-    "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
-    "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    "kvmclock-stable-bit", NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *hyperv_priv_feature_name[] = {
-    NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
-    NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
-    NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
-    NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
-    NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
-    NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *hyperv_ident_feature_name[] = {
-    NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
-    NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
-    NULL /* hv_post_messages */, NULL /* hv_signal_events */,
-    NULL /* hv_create_port */, NULL /* hv_connect_port */,
-    NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
-    NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
-    NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *hyperv_misc_feature_name[] = {
-    NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
-    NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
-    NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
-    NULL, NULL,
-    NULL, NULL, NULL /* hv_guest_crash_msr */, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *svm_feature_name[] = {
-    "npt", "lbrv", "svm_lock", "nrip_save",
-    "tsc_scale", "vmcb_clean",  "flushbyasid", "decodeassists",
-    NULL, NULL, "pause_filter", NULL,
-    "pfthreshold", NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_7_0_ebx_feature_name[] = {
-    "fsgsbase", "tsc_adjust", NULL, "bmi1",
-    "hle", "avx2", NULL, "smep",
-    "bmi2", "erms", "invpcid", "rtm",
-    NULL, NULL, "mpx", NULL,
-    "avx512f", "avx512dq", "rdseed", "adx",
-    "smap", "avx512ifma", "pcommit", "clflushopt",
-    "clwb", NULL, "avx512pf", "avx512er",
-    "avx512cd", NULL, "avx512bw", "avx512vl",
-};
-
-static const char *cpuid_7_0_ecx_feature_name[] = {
-    NULL, "avx512vbmi", "umip", "pku",
-    "ospke", NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, "rdpid", NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_apm_edx_feature_name[] = {
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    "invtsc", NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_xsave_feature_name[] = {
-    "xsaveopt", "xsavec", "xgetbv1", "xsaves",
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_6_feature_name[] = {
-    NULL, NULL, "arat", NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-    NULL, NULL, NULL, NULL,
-};
-
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
@@ -425,7 +246,12 @@ static const char *cpuid_6_feature_name[] = {
           CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
 
 typedef struct FeatureWordInfo {
-    const char **feat_names;
+    /* feature flags names are taken from "Intel Processor Identification and
+     * the CPUID Instruction" and AMD's "CPUID Specification".
+     * In cases of disagreement between feature naming conventions,
+     * aliases may be added.
+     */
+    const char *feat_names[32];
     uint32_t cpuid_eax;   /* Input EAX for CPUID */
     bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
     uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
@@ -436,82 +262,230 @@ typedef struct FeatureWordInfo {
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     [FEAT_1_EDX] = {
-        .feat_names = feature_name,
+        .feat_names = {
+            "fpu", "vme", "de", "pse",
+            "tsc", "msr", "pae", "mce",
+            "cx8", "apic", NULL, "sep",
+            "mtrr", "pge", "mca", "cmov",
+            "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
+            NULL, "ds" /* Intel dts */, "acpi", "mmx",
+            "fxsr", "sse", "sse2", "ss",
+            "ht" /* Intel htt */, "tm", "ia64", "pbe",
+        },
         .cpuid_eax = 1, .cpuid_reg = R_EDX,
         .tcg_features = TCG_FEATURES,
     },
     [FEAT_1_ECX] = {
-        .feat_names = ext_feature_name,
+        .feat_names = {
+            "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
+            "ds_cpl", "vmx", "smx", "est",
+            "tm2", "ssse3", "cid", NULL,
+            "fma", "cx16", "xtpr", "pdcm",
+            NULL, "pcid", "dca", "sse4.1|sse4_1",
+            "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
+            "tsc-deadline", "aes", "xsave", "osxsave",
+            "avx", "f16c", "rdrand", "hypervisor",
+        },
         .cpuid_eax = 1, .cpuid_reg = R_ECX,
         .tcg_features = TCG_EXT_FEATURES,
     },
+    /* Feature names that are already defined on feature_name[] but
+     * are set on CPUID[8000_0001].EDX on AMD CPUs don't have their
+     * names on feat_names below. They are copied automatically
+     * to features[FEAT_8000_0001_EDX] if and only if CPU vendor is AMD.
+     */
     [FEAT_8000_0001_EDX] = {
-        .feat_names = ext2_feature_name,
+        .feat_names = {
+            NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
+            NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
+            NULL /* cx8 */, NULL /* apic */, NULL, "syscall",
+            NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
+            NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
+            "nx|xd", NULL, "mmxext", NULL /* mmx */,
+            NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb", "rdtscp",
+            NULL, "lm|i64", "3dnowext", "3dnow",
+        },
         .cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
         .tcg_features = TCG_EXT2_FEATURES,
     },
     [FEAT_8000_0001_ECX] = {
-        .feat_names = ext3_feature_name,
+        .feat_names = {
+            "lahf_lm", "cmp_legacy", "svm", "extapic",
+            "cr8legacy", "abm", "sse4a", "misalignsse",
+            "3dnowprefetch", "osvw", "ibs", "xop",
+            "skinit", "wdt", NULL, "lwp",
+            "fma4", "tce", NULL, "nodeid_msr",
+            NULL, "tbm", "topoext", "perfctr_core",
+            "perfctr_nb", NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
         .tcg_features = TCG_EXT3_FEATURES,
     },
     [FEAT_C000_0001_EDX] = {
-        .feat_names = ext4_feature_name,
+        .feat_names = {
+            NULL, NULL, "xstore", "xstore-en",
+            NULL, NULL, "xcrypt", "xcrypt-en",
+            "ace2", "ace2-en", "phe", "phe-en",
+            "pmm", "pmm-en", NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0xC0000001, .cpuid_reg = R_EDX,
         .tcg_features = TCG_EXT4_FEATURES,
     },
     [FEAT_KVM] = {
-        .feat_names = kvm_feature_name,
+        .feat_names = {
+            "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
+            "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            "kvmclock-stable-bit", NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
         .tcg_features = TCG_KVM_FEATURES,
     },
     [FEAT_HYPERV_EAX] = {
-        .feat_names = hyperv_priv_feature_name,
+        .feat_names = {
+            NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
+            NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
+            NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
+            NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
+            NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
+            NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0x40000003, .cpuid_reg = R_EAX,
     },
     [FEAT_HYPERV_EBX] = {
-        .feat_names = hyperv_ident_feature_name,
+        .feat_names = {
+            NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
+            NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
+            NULL /* hv_post_messages */, NULL /* hv_signal_events */,
+            NULL /* hv_create_port */, NULL /* hv_connect_port */,
+            NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
+            NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
+            NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0x40000003, .cpuid_reg = R_EBX,
     },
     [FEAT_HYPERV_EDX] = {
-        .feat_names = hyperv_misc_feature_name,
+        .feat_names = {
+            NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
+            NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
+            NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
+            NULL, NULL,
+            NULL, NULL, NULL /* hv_guest_crash_msr */, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0x40000003, .cpuid_reg = R_EDX,
     },
     [FEAT_SVM] = {
-        .feat_names = svm_feature_name,
+        .feat_names = {
+            "npt", "lbrv", "svm_lock", "nrip_save",
+            "tsc_scale", "vmcb_clean",  "flushbyasid", "decodeassists",
+            NULL, NULL, "pause_filter", NULL,
+            "pfthreshold", NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
         .tcg_features = TCG_SVM_FEATURES,
     },
     [FEAT_7_0_EBX] = {
-        .feat_names = cpuid_7_0_ebx_feature_name,
+        .feat_names = {
+            "fsgsbase", "tsc_adjust", NULL, "bmi1",
+            "hle", "avx2", NULL, "smep",
+            "bmi2", "erms", "invpcid", "rtm",
+            NULL, NULL, "mpx", NULL,
+            "avx512f", "avx512dq", "rdseed", "adx",
+            "smap", "avx512ifma", "pcommit", "clflushopt",
+            "clwb", NULL, "avx512pf", "avx512er",
+            "avx512cd", NULL, "avx512bw", "avx512vl",
+        },
         .cpuid_eax = 7,
         .cpuid_needs_ecx = true, .cpuid_ecx = 0,
         .cpuid_reg = R_EBX,
         .tcg_features = TCG_7_0_EBX_FEATURES,
     },
     [FEAT_7_0_ECX] = {
-        .feat_names = cpuid_7_0_ecx_feature_name,
+        .feat_names = {
+            NULL, "avx512vbmi", "umip", "pku",
+            "ospke", NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, "rdpid", NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 7,
         .cpuid_needs_ecx = true, .cpuid_ecx = 0,
         .cpuid_reg = R_ECX,
         .tcg_features = TCG_7_0_ECX_FEATURES,
     },
     [FEAT_8000_0007_EDX] = {
-        .feat_names = cpuid_apm_edx_feature_name,
+        .feat_names = {
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            "invtsc", NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0x80000007,
         .cpuid_reg = R_EDX,
         .tcg_features = TCG_APM_FEATURES,
         .unmigratable_flags = CPUID_APM_INVTSC,
     },
     [FEAT_XSAVE] = {
-        .feat_names = cpuid_xsave_feature_name,
+        .feat_names = {
+            "xsaveopt", "xsavec", "xgetbv1", "xsaves",
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 0xd,
         .cpuid_needs_ecx = true, .cpuid_ecx = 1,
         .cpuid_reg = R_EAX,
         .tcg_features = TCG_XSAVE_FEATURES,
     },
     [FEAT_6_EAX] = {
-        .feat_names = cpuid_6_feature_name,
+        .feat_names = {
+            NULL, NULL, "arat", NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
         .cpuid_eax = 6, .cpuid_reg = R_EAX,
         .tcg_features = TCG_6_EAX_FEATURES,
     },
@@ -711,8 +685,7 @@ static void add_flagname_to_bitmaps(const char *flagname,
     FeatureWord w;
     for (w = 0; w < FEATURE_WORDS; w++) {
         FeatureWordInfo *wi = &feature_word_info[w];
-        if (wi->feat_names &&
-            lookup_feature(&words[w], flagname, NULL, wi->feat_names)) {
+        if (lookup_feature(&words[w], flagname, NULL, wi->feat_names)) {
             break;
         }
     }
@@ -3324,9 +3297,6 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
     char **names;
     FeatureWordInfo *fi = &feature_word_info[w];
 
-    if (!fi->feat_names) {
-        return;
-    }
     if (!fi->feat_names[bitnr]) {
         return;
     }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 09/20] target-i386: Don't try to enable PT State xsave component
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (7 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 08/20] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 10/20] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

The code that calculates the set of supported XSAVE components on
CPUID looks at ext_save_areas to find out which components should
be enabled. However, if there are zeroed entries in the
ext_save_areas array, the
  ((env->features[esa->feature] & esa->bits) == esa->bits)
check will always succeed and QEMU will unconditionally try to
enable the component.

Luckily this never caused any problems because the only missing
entry in ext_save_areas is the PT State component (bit 8), and
KVM currently doesn't support it (so it was cleared on ena_mask).
But the code was still incorrect and would break if KVM starts
returning CPUID[EAX=0xD,ECX=0].EAX[bit 8] as supported on
GET_SUPPORTED_CPUID.

Fix the problem by changing the code to not enable a XSAVE
component if ExtSaveArea::bits is zero.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cc07fdb..25ab4f8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2514,7 +2514,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ecx = 0x240;
             for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
                 const ExtSaveArea *esa = &x86_ext_save_areas[i];
-                if ((env->features[esa->feature] & esa->bits) == esa->bits
+                if ((env->features[esa->feature] & esa->bits)
                     && ((ena_mask >> i) & 1) != 0) {
                     if (i < 32) {
                         *eax |= 1u << i;
@@ -2530,7 +2530,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *eax = env->features[FEAT_XSAVE];
         } else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
             const ExtSaveArea *esa = &x86_ext_save_areas[count];
-            if ((env->features[esa->feature] & esa->bits) == esa->bits
+            if ((env->features[esa->feature] & esa->bits)
                 && ((ena_mask >> count) & 1) != 0) {
                 *eax = esa->size;
                 *ebx = esa->offset;
@@ -2766,7 +2766,7 @@ static void x86_cpu_reset(CPUState *s)
     }
     for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
         const ExtSaveArea *esa = &x86_ext_save_areas[i];
-        if ((env->features[esa->feature] & esa->bits) == esa->bits) {
+        if (env->features[esa->feature] & esa->bits) {
             xcr0 |= 1ull << i;
         }
     }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 10/20] target-i386: xsave: Calculate enabled components only once
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (8 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 09/20] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 11/20] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Instead of checking both env->features and ena_mask at two
different places in the CPUID code, initialize ena_mask based on
the features that are enabled for the CPU, and then clear
unsupported bits based on kvm_arch_get_supported_cpuid().

The results should be exactly the same, but it will make it
easier to move the mask calculation elsewhare, and reuse
x86_cpu_filter_features() for the kvm_arch_get_supported_cpuid()
check.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 25ab4f8..9968581 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2490,7 +2490,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx &= 0xffff; /* The count doesn't need to be reliable. */
         break;
     case 0xD: {
-        KVMState *s = cs->kvm_state;
         uint64_t ena_mask;
         int i;
 
@@ -2502,20 +2501,28 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
             break;
         }
+
+        ena_mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+        for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+            const ExtSaveArea *esa = &x86_ext_save_areas[i];
+            if (env->features[esa->feature] & esa->bits) {
+                ena_mask |= (1ULL << i);
+            }
+        }
+
         if (kvm_enabled()) {
-            ena_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
-            ena_mask <<= 32;
-            ena_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
-        } else {
-            ena_mask = -1;
+            KVMState *s = cs->kvm_state;
+            uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
+            kvm_mask <<= 32;
+            kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
+            ena_mask &= kvm_mask;
         }
 
         if (count == 0) {
             *ecx = 0x240;
             for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
                 const ExtSaveArea *esa = &x86_ext_save_areas[i];
-                if ((env->features[esa->feature] & esa->bits)
-                    && ((ena_mask >> i) & 1) != 0) {
+                if ((ena_mask >> i) & 1) {
                     if (i < 32) {
                         *eax |= 1u << i;
                     } else {
@@ -2530,8 +2537,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *eax = env->features[FEAT_XSAVE];
         } else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
             const ExtSaveArea *esa = &x86_ext_save_areas[count];
-            if ((env->features[esa->feature] & esa->bits)
-                && ((ena_mask >> count) & 1) != 0) {
+            if ((ena_mask >> count) & 1) {
                 *eax = esa->size;
                 *ebx = esa->offset;
             }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 11/20] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (9 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 10/20] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 12/20] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Instead of assigning individual bits in a loop, just copy the
values from ena_mask.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9968581..7e66003 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2523,15 +2523,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
                 const ExtSaveArea *esa = &x86_ext_save_areas[i];
                 if ((ena_mask >> i) & 1) {
-                    if (i < 32) {
-                        *eax |= 1u << i;
-                    } else {
-                        *edx |= 1u << (i - 32);
-                    }
                     *ecx = MAX(*ecx, esa->offset + esa->size);
                 }
             }
-            *eax |= ena_mask & (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+            *eax = ena_mask;
+            *edx = ena_mask >> 32;
             *ebx = *ecx;
         } else if (count == 1) {
             *eax = env->features[FEAT_XSAVE];
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 12/20] target-i386: xsave: Helper function to calculate xsave area size
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (10 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 11/20] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 13/20] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Move the xsave area size calculation from cpu_x86_cpuid() inside
its own function. While doing it, change it to use the XSAVE area
struct sizes for the initial size, instead of the magic 0x240
number.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7e66003..9034d8e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -548,6 +548,20 @@ static const ExtSaveArea x86_ext_save_areas[] = {
             .size = sizeof(XSavePKRU) },
 };
 
+static uint32_t xsave_area_size(uint64_t mask)
+{
+    int i;
+    uint64_t ret = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader);
+
+    for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+        const ExtSaveArea *esa = &x86_ext_save_areas[i];
+        if ((mask >> i) & 1) {
+            ret = MAX(ret, esa->offset + esa->size);
+        }
+    }
+    return ret;
+}
+
 const char *get_register_name_32(unsigned int reg)
 {
     if (reg >= CPU_NB_REGS32) {
@@ -2519,13 +2533,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
 
         if (count == 0) {
-            *ecx = 0x240;
-            for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
-                const ExtSaveArea *esa = &x86_ext_save_areas[i];
-                if ((ena_mask >> i) & 1) {
-                    *ecx = MAX(*ecx, esa->offset + esa->size);
-                }
-            }
+            *ecx = xsave_area_size(ena_mask);;
             *eax = ena_mask;
             *edx = ena_mask >> 32;
             *ebx = *ecx;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 13/20] target-i386: xsave: Calculate set of xsave components on realize
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (11 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 12/20] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array Eduardo Habkost
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

Instead of doing complex calculations and calling
kvm_arch_get_supported_cpuid() inside cpu_x86_cpuid(), calculate
the set of required XSAVE components earlier, at realize time.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 55 ++++++++++++++++++++++++++++++++-----------------------
 target-i386/cpu.h |  1 +
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9034d8e..8bef3cf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2504,9 +2504,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx &= 0xffff; /* The count doesn't need to be reliable. */
         break;
     case 0xD: {
-        uint64_t ena_mask;
-        int i;
-
         /* Processor Extended State */
         *eax = 0;
         *ebx = 0;
@@ -2516,32 +2513,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             break;
         }
 
-        ena_mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
-        for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
-            const ExtSaveArea *esa = &x86_ext_save_areas[i];
-            if (env->features[esa->feature] & esa->bits) {
-                ena_mask |= (1ULL << i);
-            }
-        }
-
-        if (kvm_enabled()) {
-            KVMState *s = cs->kvm_state;
-            uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
-            kvm_mask <<= 32;
-            kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
-            ena_mask &= kvm_mask;
-        }
-
         if (count == 0) {
-            *ecx = xsave_area_size(ena_mask);;
-            *eax = ena_mask;
-            *edx = ena_mask >> 32;
+            *ecx = xsave_area_size(env->xsave_components);
+            *eax = env->xsave_components;
+            *edx = env->xsave_components >> 32;
             *ebx = *ecx;
         } else if (count == 1) {
             *eax = env->features[FEAT_XSAVE];
         } else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
             const ExtSaveArea *esa = &x86_ext_save_areas[count];
-            if ((ena_mask >> count) & 1) {
+            if ((env->xsave_components >> count) & 1) {
                 *eax = esa->size;
                 *ebx = esa->offset;
             }
@@ -2971,6 +2952,33 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
     }
 }
 
+/* Calculate XSAVE components based on the configured CPU feature flags */
+static void x86_cpu_enable_xsave_components(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+    int i;
+
+    if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
+        return;
+    }
+
+    env->xsave_components = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+    for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+        const ExtSaveArea *esa = &x86_ext_save_areas[i];
+        if (env->features[esa->feature] & esa->bits) {
+            env->xsave_components |= (1ULL << i);
+        }
+    }
+
+    if (kvm_enabled()) {
+        KVMState *s = kvm_state;
+        uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
+        kvm_mask <<= 32;
+        kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
+        env->xsave_components &= kvm_mask;
+    }
+}
+
 #define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
                            (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
                            (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
@@ -3016,6 +3024,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         cpu->env.features[w] &= ~minus_features[w];
     }
 
+    x86_cpu_enable_xsave_components(cpu);
 
     /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
     x86_cpu_adjust_feat_level(cpu, FEAT_7_0_EBX);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index aaa45f0..6c457ed 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1122,6 +1122,7 @@ typedef struct CPUX86State {
     uint32_t cpuid_vendor3;
     uint32_t cpuid_version;
     FeatureWordArray features;
+    uint64_t xsave_components;
     uint32_t cpuid_model[12];
 
     /* MTRRs */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (12 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 13/20] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-10-03 16:42   ` Daniel P. Berrange
  2016-09-27 20:12 ` [Qemu-devel] [PULL 15/20] target-i386: Remove has_msr_mtrr global variable Eduardo Habkost
                   ` (6 subsequent siblings)
  20 siblings, 1 reply; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

This will reuse the existing check/enforce logic in
x86_cpu_filter_features() to check the xsave component bits
against GET_SUPPORTED_CPUID.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
 target-i386/cpu.h |  3 ++-
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8bef3cf..ad09246 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -489,6 +489,18 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         .cpuid_eax = 6, .cpuid_reg = R_EAX,
         .tcg_features = TCG_6_EAX_FEATURES,
     },
+    [FEAT_XSAVE_COMP_LO] = {
+        .cpuid_eax = 0xD,
+        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+        .cpuid_reg = R_EAX,
+        .tcg_features = ~0U,
+    },
+    [FEAT_XSAVE_COMP_HI] = {
+        .cpuid_eax = 0xD,
+        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+        .cpuid_reg = R_EDX,
+        .tcg_features = ~0U,
+    },
 };
 
 typedef struct X86RegisterInfo32 {
@@ -562,6 +574,12 @@ static uint32_t xsave_area_size(uint64_t mask)
     return ret;
 }
 
+static inline uint64_t x86_cpu_xsave_components(X86CPU *cpu)
+{
+    return ((uint64_t)cpu->env.features[FEAT_XSAVE_COMP_HI]) << 32 |
+           cpu->env.features[FEAT_XSAVE_COMP_LO];
+}
+
 const char *get_register_name_32(unsigned int reg)
 {
     if (reg >= CPU_NB_REGS32) {
@@ -2514,15 +2532,15 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
 
         if (count == 0) {
-            *ecx = xsave_area_size(env->xsave_components);
-            *eax = env->xsave_components;
-            *edx = env->xsave_components >> 32;
+            *ecx = xsave_area_size(x86_cpu_xsave_components(cpu));
+            *eax = env->features[FEAT_XSAVE_COMP_LO];
+            *edx = env->features[FEAT_XSAVE_COMP_HI];
             *ebx = *ecx;
         } else if (count == 1) {
             *eax = env->features[FEAT_XSAVE];
         } else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
-            const ExtSaveArea *esa = &x86_ext_save_areas[count];
-            if ((env->xsave_components >> count) & 1) {
+            if ((x86_cpu_xsave_components(cpu) >> count) & 1) {
+                const ExtSaveArea *esa = &x86_ext_save_areas[count];
                 *eax = esa->size;
                 *ebx = esa->offset;
             }
@@ -2957,26 +2975,22 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
     int i;
+    uint64_t mask;
 
     if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
         return;
     }
 
-    env->xsave_components = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+    mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
     for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
         const ExtSaveArea *esa = &x86_ext_save_areas[i];
         if (env->features[esa->feature] & esa->bits) {
-            env->xsave_components |= (1ULL << i);
+            mask |= (1ULL << i);
         }
     }
 
-    if (kvm_enabled()) {
-        KVMState *s = kvm_state;
-        uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
-        kvm_mask <<= 32;
-        kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
-        env->xsave_components &= kvm_mask;
-    }
+    env->features[FEAT_XSAVE_COMP_LO] = mask;
+    env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
 }
 
 #define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6c457ed..1cb32ae 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -453,6 +453,8 @@ typedef enum FeatureWord {
     FEAT_SVM,           /* CPUID[8000_000A].EDX */
     FEAT_XSAVE,         /* CPUID[EAX=0xd,ECX=1].EAX */
     FEAT_6_EAX,         /* CPUID[6].EAX */
+    FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
+    FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
     FEATURE_WORDS,
 } FeatureWord;
 
@@ -1122,7 +1124,6 @@ typedef struct CPUX86State {
     uint32_t cpuid_vendor3;
     uint32_t cpuid_version;
     FeatureWordArray features;
-    uint64_t xsave_components;
     uint32_t cpuid_model[12];
 
     /* MTRRs */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 15/20] target-i386: Remove has_msr_mtrr global variable
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (13 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 16/20] target-i386: Remove has_msr_hv_apic " Eduardo Habkost
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

The global variable is not necessary because we can check the CPU
feature flags directly.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/kvm.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a0e42b2..5118562 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -99,7 +99,6 @@ static bool has_msr_hv_vpindex;
 static bool has_msr_hv_runtime;
 static bool has_msr_hv_synic;
 static bool has_msr_hv_stimer;
-static bool has_msr_mtrr;
 static bool has_msr_xss;
 
 static bool has_msr_architectural_pmu;
@@ -975,9 +974,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
     }
     cpu->kvm_msr_buf = g_malloc0(MSR_BUF_SIZE);
 
-    if (env->features[FEAT_1_EDX] & CPUID_MTRR) {
-        has_msr_mtrr = true;
-    }
     if (!(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_RDTSCP)) {
         has_msr_tsc_aux = false;
     }
@@ -1735,7 +1731,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
                                 env->msr_hv_stimer_count[j]);
             }
         }
-        if (has_msr_mtrr) {
+        if (env->features[FEAT_1_EDX] & CPUID_MTRR) {
             uint64_t phys_mask = MAKE_64BIT_MASK(0, cpu->phys_bits);
 
             kvm_msr_entry_add(cpu, MSR_MTRRdefType, env->mtrr_deftype);
@@ -2125,7 +2121,7 @@ static int kvm_get_msrs(X86CPU *cpu)
             kvm_msr_entry_add(cpu, msr, 0);
         }
     }
-    if (has_msr_mtrr) {
+    if (env->features[FEAT_1_EDX] & CPUID_MTRR) {
         kvm_msr_entry_add(cpu, MSR_MTRRdefType, 0);
         kvm_msr_entry_add(cpu, MSR_MTRRfix64K_00000, 0);
         kvm_msr_entry_add(cpu, MSR_MTRRfix16K_80000, 0);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 16/20] target-i386: Remove has_msr_hv_apic global variable
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (14 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 15/20] target-i386: Remove has_msr_mtrr global variable Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 17/20] target-i386: Remove has_msr_hv_tsc " Eduardo Habkost
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

The global variable is not necessary because we can check
cpu->hyperv_vapic directly.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/kvm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5118562..031ae90 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -91,7 +91,6 @@ static bool has_msr_bndcfgs;
 static bool has_msr_kvm_steal_time;
 static int lm_capable_kernel;
 static bool has_msr_hv_hypercall;
-static bool has_msr_hv_vapic;
 static bool has_msr_hv_tsc;
 static bool has_msr_hv_crash;
 static bool has_msr_hv_reset;
@@ -609,7 +608,6 @@ static int hyperv_handle_properties(CPUState *cs)
     if (cpu->hyperv_vapic) {
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
-        has_msr_hv_vapic = true;
     }
     if (cpu->hyperv_time &&
             kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) {
@@ -728,7 +726,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         if (cpu->hyperv_relaxed_timing) {
             c->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED;
         }
-        if (has_msr_hv_vapic) {
+        if (cpu->hyperv_vapic) {
             c->eax |= HV_X64_APIC_ACCESS_RECOMMENDED;
         }
         c->ebx = cpu->hyperv_spinlock_attempts;
@@ -1681,7 +1679,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
             kvm_msr_entry_add(cpu, HV_X64_MSR_HYPERCALL,
                               env->msr_hv_hypercall);
         }
-        if (has_msr_hv_vapic) {
+        if (cpu->hyperv_vapic) {
             kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
                               env->msr_hv_vapic);
         }
@@ -2086,7 +2084,7 @@ static int kvm_get_msrs(X86CPU *cpu)
         kvm_msr_entry_add(cpu, HV_X64_MSR_HYPERCALL, 0);
         kvm_msr_entry_add(cpu, HV_X64_MSR_GUEST_OS_ID, 0);
     }
-    if (has_msr_hv_vapic) {
+    if (cpu->hyperv_vapic) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE, 0);
     }
     if (has_msr_hv_tsc) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 17/20] target-i386: Remove has_msr_hv_tsc global variable
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (15 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 16/20] target-i386: Remove has_msr_hv_apic " Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 18/20] target-i386: Clear KVM CPUID features if KVM is disabled Eduardo Habkost
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

The global variable is not necessary because we can check
cpu->hyperv_time directly.

We just need to ensure cpu->hyperv_time will be cleared if the
feature is not really being exposed to the guest due to missing
KVM_CAP_HYPERV_TIME capability.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/kvm.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 031ae90..4046030 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -91,7 +91,6 @@ static bool has_msr_bndcfgs;
 static bool has_msr_kvm_steal_time;
 static int lm_capable_kernel;
 static bool has_msr_hv_hypercall;
-static bool has_msr_hv_tsc;
 static bool has_msr_hv_crash;
 static bool has_msr_hv_reset;
 static bool has_msr_hv_vpindex;
@@ -602,6 +601,11 @@ static int hyperv_handle_properties(CPUState *cs)
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
+    if (cpu->hyperv_time &&
+            kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) {
+        cpu->hyperv_time = false;
+    }
+
     if (cpu->hyperv_relaxed_timing) {
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
     }
@@ -609,12 +613,10 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
     }
-    if (cpu->hyperv_time &&
-            kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) > 0) {
+    if (cpu->hyperv_time) {
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_X64_MSR_TIME_REF_COUNT_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= 0x200;
-        has_msr_hv_tsc = true;
     }
     if (cpu->hyperv_crash && has_msr_hv_crash) {
         env->features[FEAT_HYPERV_EDX] |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
@@ -1683,7 +1685,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
             kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
                               env->msr_hv_vapic);
         }
-        if (has_msr_hv_tsc) {
+        if (cpu->hyperv_time) {
             kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, env->msr_hv_tsc);
         }
         if (has_msr_hv_crash) {
@@ -2087,7 +2089,7 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (cpu->hyperv_vapic) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE, 0);
     }
-    if (has_msr_hv_tsc) {
+    if (cpu->hyperv_time) {
         kvm_msr_entry_add(cpu, HV_X64_MSR_REFERENCE_TSC, 0);
     }
     if (has_msr_hv_crash) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 18/20] target-i386: Clear KVM CPUID features if KVM is disabled
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (16 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 17/20] target-i386: Remove has_msr_hv_tsc " Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 19/20] target-i386: Remove has_msr_* global vars for KVM features Eduardo Habkost
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

This will ensure all checks for features[FEAT_KVM] in the code
will be correct in case the KVM CPUID leaf is completely
disabled.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ad09246..333309b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3038,6 +3038,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         cpu->env.features[w] &= ~minus_features[w];
     }
 
+    if (!kvm_enabled() || !cpu->expose_kvm) {
+        env->features[FEAT_KVM] = 0;
+    }
+
     x86_cpu_enable_xsave_components(cpu);
 
     /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 19/20] target-i386: Remove has_msr_* global vars for KVM features
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (17 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 18/20] target-i386: Clear KVM CPUID features if KVM is disabled Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 20:12 ` [Qemu-devel] [PULL 20/20] sysbus: Remove ignored return value of FindSysbusDeviceFunc Eduardo Habkost
  2016-09-27 23:31 ` [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Peter Maydell
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel

The global variables are not necessary because we can check KVM
feature flags in X86CPU directly.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/kvm.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 4046030..30b63b7 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -83,12 +83,9 @@ static bool has_msr_tsc_aux;
 static bool has_msr_tsc_adjust;
 static bool has_msr_tsc_deadline;
 static bool has_msr_feature_control;
-static bool has_msr_async_pf_en;
-static bool has_msr_pv_eoi_en;
 static bool has_msr_misc_enable;
 static bool has_msr_smbase;
 static bool has_msr_bndcfgs;
-static bool has_msr_kvm_steal_time;
 static int lm_capable_kernel;
 static bool has_msr_hv_hypercall;
 static bool has_msr_hv_crash;
@@ -754,12 +751,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
         c = &cpuid_data.entries[cpuid_i++];
         c->function = KVM_CPUID_FEATURES | kvm_base;
         c->eax = env->features[FEAT_KVM];
-
-        has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
-
-        has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
-
-        has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
     }
 
     cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
@@ -1639,13 +1630,13 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
         kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
         kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
-        if (has_msr_async_pf_en) {
+        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
         }
-        if (has_msr_pv_eoi_en) {
+        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
             kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, env->pv_eoi_en_msr);
         }
-        if (has_msr_kvm_steal_time) {
+        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
             kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
         }
         if (has_msr_architectural_pmu) {
@@ -2048,13 +2039,13 @@ static int kvm_get_msrs(X86CPU *cpu)
 #endif
     kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
     kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
-    if (has_msr_async_pf_en) {
+    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, 0);
     }
-    if (has_msr_pv_eoi_en) {
+    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
         kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, 0);
     }
-    if (has_msr_kvm_steal_time) {
+    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
         kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0);
     }
     if (has_msr_architectural_pmu) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PULL 20/20] sysbus: Remove ignored return value of FindSysbusDeviceFunc
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (18 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 19/20] target-i386: Remove has_msr_* global vars for KVM features Eduardo Habkost
@ 2016-09-27 20:12 ` Eduardo Habkost
  2016-09-27 23:31 ` [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Peter Maydell
  20 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Richard Henderson, qemu-devel, David Gibson

From: David Gibson <david@gibson.dropbear.id.au>

Functions of type FindSysbusDeviceFunc currently return an integer.
However, this return value is always ignored by the caller in
find_sysbus_device().

This changes the function type to return void, to avoid confusion over
the function semantics.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/arm/sysbus-fdt.c    | 4 ++--
 hw/core/machine.c      | 2 +-
 hw/core/platform-bus.c | 8 ++------
 hw/ppc/e500.c          | 4 +---
 hw/ppc/spapr.c         | 4 +---
 include/hw/sysbus.h    | 2 +-
 6 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 5debb33..d68e3dc 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -436,7 +436,7 @@ static const NodeCreationPair add_fdt_node_functions[] = {
  * are dynamically instantiable and if so call the node creation
  * function.
  */
-static int add_fdt_node(SysBusDevice *sbdev, void *opaque)
+static void add_fdt_node(SysBusDevice *sbdev, void *opaque)
 {
     int i, ret;
 
@@ -445,7 +445,7 @@ static int add_fdt_node(SysBusDevice *sbdev, void *opaque)
                     add_fdt_node_functions[i].typename)) {
             ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
             assert(!ret);
-            return 0;
+            return;
         }
     }
     error_report("Device %s can not be dynamically instantiated",
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 00fbe3e..afd84ac 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -332,7 +332,7 @@ static bool machine_get_enforce_config_section(Object *obj, Error **errp)
     return ms->enforce_config_section;
 }
 
-static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
+static void error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
     error_report("Option '-device %s' cannot be handled by this machine",
                  object_class_get_name(object_get_class(OBJECT(sbdev))));
diff --git a/hw/core/platform-bus.c b/hw/core/platform-bus.c
index 36f84ab..329ac67 100644
--- a/hw/core/platform-bus.c
+++ b/hw/core/platform-bus.c
@@ -74,7 +74,7 @@ hwaddr platform_bus_get_mmio_addr(PlatformBusDevice *pbus, SysBusDevice *sbdev,
     return object_property_get_int(OBJECT(sbdev_mr), "addr", NULL);
 }
 
-static int platform_bus_count_irqs(SysBusDevice *sbdev, void *opaque)
+static void platform_bus_count_irqs(SysBusDevice *sbdev, void *opaque)
 {
     PlatformBusDevice *pbus = opaque;
     qemu_irq sbirq;
@@ -93,8 +93,6 @@ static int platform_bus_count_irqs(SysBusDevice *sbdev, void *opaque)
             }
         }
     }
-
-    return 0;
 }
 
 /*
@@ -168,7 +166,7 @@ static void platform_bus_map_mmio(PlatformBusDevice *pbus, SysBusDevice *sbdev,
  * For each sysbus device, look for unassigned IRQ lines as well as
  * unassociated MMIO regions. Connect them to the platform bus if available.
  */
-static int link_sysbus_device(SysBusDevice *sbdev, void *opaque)
+static void link_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
     PlatformBusDevice *pbus = opaque;
     int i;
@@ -180,8 +178,6 @@ static int link_sysbus_device(SysBusDevice *sbdev, void *opaque)
     for (i = 0; sysbus_has_mmio(sbdev, i); i++) {
         platform_bus_map_mmio(pbus, sbdev, i);
     }
-
-    return 0;
 }
 
 static void platform_bus_init_notify(Notifier *notifier, void *data)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 0cd534d..cf8b122 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -196,7 +196,7 @@ static int create_devtree_etsec(SysBusDevice *sbdev, PlatformDevtreeData *data)
     return 0;
 }
 
-static int sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
+static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
 {
     PlatformDevtreeData *data = opaque;
     bool matched = false;
@@ -211,8 +211,6 @@ static int sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
                      qdev_fw_name(DEVICE(sbdev)));
         exit(1);
     }
-
-    return 0;
 }
 
 static void platform_bus_create_devtree(PPCE500Params *params, void *fdt,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9b506d5..648576e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1110,7 +1110,7 @@ static void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
     }
 }
 
-static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
+static void find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
     bool matched = false;
 
@@ -1123,8 +1123,6 @@ static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
                      qdev_fw_name(DEVICE(sbdev)));
         exit(1);
     }
-
-    return 0;
 }
 
 static void ppc_spapr_reset(void)
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index e73a5b2..e88bb6d 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -75,7 +75,7 @@ struct SysBusDevice {
     uint32_t pio[QDEV_MAX_PIO];
 };
 
-typedef int FindSysbusDeviceFunc(SysBusDevice *sbdev, void *opaque);
+typedef void FindSysbusDeviceFunc(SysBusDevice *sbdev, void *opaque);
 
 void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory);
 MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27
  2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
                   ` (19 preceding siblings ...)
  2016-09-27 20:12 ` [Qemu-devel] [PULL 20/20] sysbus: Remove ignored return value of FindSysbusDeviceFunc Eduardo Habkost
@ 2016-09-27 23:31 ` Peter Maydell
  20 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2016-09-27 23:31 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Paolo Bonzini, Richard Henderson, QEMU Developers

On 27 September 2016 at 13:12, Eduardo Habkost <ehabkost@redhat.com> wrote:
> The following changes since commit 333ec4ca6a9f604331e2349cb91e9635f65d6462:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-09-27 16:23:08 +0100)
>
> are available in the git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-pull-request
>
> for you to fetch changes up to 4f01a637795af77f1c191230b9f6e3a2547b0c28:
>
>   sysbus: Remove ignored return value of FindSysbusDeviceFunc (2016-09-27 17:03:34 -0300)
>
> ----------------------------------------------------------------
> x86 and machine queue, 2016-09-27
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array
  2016-09-27 20:12 ` [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array Eduardo Habkost
@ 2016-10-03 16:42   ` Daniel P. Berrange
  2016-10-03 19:10     ` Eduardo Habkost
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel P. Berrange @ 2016-10-03 16:42 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Paolo Bonzini, qemu-devel, Richard Henderson

On Tue, Sep 27, 2016 at 05:12:24PM -0300, Eduardo Habkost wrote:
> This will reuse the existing check/enforce logic in
> x86_cpu_filter_features() to check the xsave component bits
> against GET_SUPPORTED_CPUID.
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
>  target-i386/cpu.h |  3 ++-
>  2 files changed, 30 insertions(+), 15 deletions(-)

git bisect tells me that this change is responsible for breaking booting
of a guest of mine which uses -cpu host

GRUB works, but the guest kernel hangs immediately after printing

"Probing EDD (edd=off to disable)... ok"

Removing '-cpu host' lets it work again.

My command line is:

/home/berrange/usr/qemu-git/bin/qemu-system-x86_64
-name guest=f23x86_64,debug-threads=on
-S
-object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-18-f23x86_64/master-key.aes
-machine pc-q35-2.6,accel=kvm,usb=off,vmport=off
-cpu host
-m 8000
-realtime mlock=off
-smp 8,sockets=8,cores=1,threads=1
-numa node,nodeid=0,cpus=0-3,mem=4000
-numa node,nodeid=1,cpus=4-5,mem=2000
-numa node,nodeid=2,cpus=6-7,mem=2000
-uuid ac4c9e05-6137-4bde-a33a-5c3623f44fb2
-no-user-config
-nodefaults
-chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-18-f23x86_64/monitor.sock,server,nowait
-mon chardev=charmonitor,id=monitor,mode=control
-rtc base=utc,driftfix=slew
-global kvm-pit.lost_tick_policy=discard
-no-hpet
-no-shutdown
-global ICH9-LPC.disable_s3=1
-global ICH9-LPC.disable_s4=1
-boot strict=on
-device intel-iommu
-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0
-device pxb-pcie,bus_nr=214,id=pci.3,numa_node=0,bus=pcie.0,addr=0x2
-device pxb-pcie,bus_nr=234,id=pci.4,numa_node=1,bus=pcie.0,addr=0x3
-device pxb-pcie,bus_nr=254,id=pci.5,numa_node=2,bus=pcie.0,addr=0x4
-device i82801b11-bridge,id=pci.6,bus=pci.3,addr=0x0
-device i82801b11-bridge,id=pci.7,bus=pci.4,addr=0x0
-device i82801b11-bridge,id=pci.8,bus=pci.5,addr=0x0
-device ich9-usb-ehci1,id=usb,bus=pcie.0,addr=0x1d.0x7
-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pcie.0,multifunction=on,addr=0x1d
-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pcie.0,addr=0x1d.0x1
-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pcie.0,addr=0x1d.0x2
-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x3
-drive file=/var/lib/libvirt/images/f23x86_64.img,format=qcow2,if=none,id=drive-virtio-disk0
-device virtio-blk-pci,scsi=off,bus=pci.2,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
-netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=28
-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:9f:58:3c,bus=pci.2,addr=0x1
-netdev user,id=hostnet1
-device e1000,netdev=hostnet1,id=net1,mac=52:54:00:7e:6e:c6,bus=pci.6,addr=0x1
-netdev user,id=hostnet2
-device e1000,netdev=hostnet2,id=net2,mac=52:54:00:7e:6e:c7,bus=pci.6,addr=0x2
-netdev user,id=hostnet3
-device e1000,netdev=hostnet3,id=net3,mac=52:54:00:7e:6e:c8,bus=pci.6,addr=0x3
-netdev user,id=hostnet4
-device e1000,netdev=hostnet4,id=net4,mac=52:54:00:7e:6e:d6,bus=pci.7,addr=0x1
-netdev user,id=hostnet5
-device e1000,netdev=hostnet5,id=net5,mac=52:54:00:7e:6e:d7,bus=pci.7,addr=0x2
-netdev user,id=hostnet6
-device e1000,netdev=hostnet6,id=net6,mac=52:54:00:7e:6e:d8,bus=pci.7,addr=0x3
-netdev user,id=hostnet7
-device e1000,netdev=hostnet7,id=net7,mac=52:54:00:7e:6e:e6,bus=pci.8,addr=0x1
-netdev user,id=hostnet8
-device e1000,netdev=hostnet8,id=net8,mac=52:54:00:7e:6e:e7,bus=pci.8,addr=0x2
-netdev user,id=hostnet9
-device e1000,netdev=hostnet9,id=net9,mac=52:54:00:7e:6e:e8,bus=pci.8,addr=0x3
-chardev pty,id=charserial0
-device isa-serial,chardev=charserial0,id=serial0
-chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-18-f23x86_64/org.qemu.guest_agent.0,server,nowait
-device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
-device usb-tablet,id=input0,bus=usb.0,port=1
-vnc 127.0.0.1:0
-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pcie.0,addr=0x1
-device intel-hda,id=sound0,bus=pci.2,addr=0x2
-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0
-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x5
-msg timestamp=on


In case its relevant, QEMU prints this to stderr:

warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array
  2016-10-03 16:42   ` Daniel P. Berrange
@ 2016-10-03 19:10     ` Eduardo Habkost
  0 siblings, 0 replies; 24+ messages in thread
From: Eduardo Habkost @ 2016-10-03 19:10 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Peter Maydell, Paolo Bonzini, qemu-devel, Richard Henderson

On Mon, Oct 03, 2016 at 05:42:36PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 27, 2016 at 05:12:24PM -0300, Eduardo Habkost wrote:
> > This will reuse the existing check/enforce logic in
> > x86_cpu_filter_features() to check the xsave component bits
> > against GET_SUPPORTED_CPUID.
> > 
> > Reviewed-by: Richard Henderson <rth@twiddle.net>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
> >  target-i386/cpu.h |  3 ++-
> >  2 files changed, 30 insertions(+), 15 deletions(-)
> 
> git bisect tells me that this change is responsible for breaking booting
> of a guest of mine which uses -cpu host
> 
> GRUB works, but the guest kernel hangs immediately after printing
> 
> "Probing EDD (edd=off to disable)... ok"
> 
> Removing '-cpu host' lets it work again.
[...]
> In case its relevant, QEMU prints this to stderr:
> 
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 0]
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 1]
> warning: host doesn't support requested feature: CPUID.0DH:EAX [bit 2]

This was reported by Wanpeng Li last week. The fix ("target-i386:
Report known CPUID[EAX=0xD,ECX=0]:EAX bits as migratable") is in
the pull request I sent today.

You can also work around it by using "-cpu host,migratable=off".

-- 
Eduardo

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2016-10-03 19:11 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-27 20:12 [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 01/20] target-i386: Remove unused X86CPUDefinition::xlevel2 field Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 02/20] target-i386: Add a marker to end of the region zeroed on reset Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 03/20] tests: Add test code for CPUID level/xlevel handling Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 04/20] tests: Test CPUID level handling for old machines Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 05/20] target-i386: Automatically set level/xlevel/xlevel2 when needed Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 06/20] target-i386: Enable CPUID[0x8000000A] if SVM is enabled Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 07/20] linux-user: remove #define smp_{cores, threads} Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 08/20] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 09/20] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 10/20] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 11/20] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 12/20] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 13/20] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 14/20] target-i386: Move xsave component mask to features array Eduardo Habkost
2016-10-03 16:42   ` Daniel P. Berrange
2016-10-03 19:10     ` Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 15/20] target-i386: Remove has_msr_mtrr global variable Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 16/20] target-i386: Remove has_msr_hv_apic " Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 17/20] target-i386: Remove has_msr_hv_tsc " Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 18/20] target-i386: Clear KVM CPUID features if KVM is disabled Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 19/20] target-i386: Remove has_msr_* global vars for KVM features Eduardo Habkost
2016-09-27 20:12 ` [Qemu-devel] [PULL 20/20] sysbus: Remove ignored return value of FindSysbusDeviceFunc Eduardo Habkost
2016-09-27 23:31 ` [Qemu-devel] [PULL 00/20] x86 and machine queue, 2016-09-27 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).