[Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Eduardo Habkost]

The following changes since commit 9e071429e649346c14b2dc76902f84f8352d2333:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-09-25 21:52:30 +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 139454c5119cf0da270901810de4f51cc0f28bdd:

  target-i386: get/put MSR_TSC_AUX across reset and migration (2015-09-28 12:46:40 -0300)

----------------------------------------------------------------
X86 queue, 2015-09-28

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

Amit Shah (1):
  target-i386: get/put MSR_TSC_AUX across reset and migration

Chen Fan (1):
  cpu: Introduce X86CPUTopoInfo structure for argument simplification

Eduardo Habkost (2):
  target-i386: Enable "check" mode by default
  target-i386: Convert kvm_default_*features to property/value pairs

Richard Henderson (2):
  target-i386: Move breakpoint related functions to new file
  target-i386: Make check_hw_breakpoints static

Thomas Huth (1):
  vl: Add another sanity check to smp_parse() function

 hw/i386/pc.c               |   6 +-
 hw/i386/pc_piix.c          |   8 +-
 hw/i386/pc_q35.c           |   4 +-
 include/hw/i386/topology.h |  33 ++++----
 target-i386/Makefile.objs  |   2 +-
 target-i386/bpt_helper.c   | 182 +++++++++++++++++++++++++++++++++++++++++++++
 target-i386/cpu.c          |  89 ++++++++++++----------
 target-i386/cpu.h          |  12 ++-
 target-i386/helper.c       | 128 -------------------------------
 target-i386/kvm.c          |  14 ++++
 target-i386/misc_helper.c  |  34 ---------
 vl.c                       |   8 +-
 12 files changed, 290 insertions(+), 230 deletions(-)
 create mode 100644 target-i386/bpt_helper.c

-- 
2.1.0

[Qemu-devel] [PULL 1/7] cpu: Introduce X86CPUTopoInfo structure for argument simplification

[Eduardo Habkost]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

In order to simplify arguments of function, introduce a new struct
named X86CPUTopoInfo.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c               |  6 +++---
 include/hw/i386/topology.h | 33 +++++++++++++++++----------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 461c128..3c28706 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1933,10 +1933,10 @@ static void pc_machine_initfn(Object *obj)
 
 static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
 {
-    unsigned pkg_id, core_id, smt_id;
+    X86CPUTopoInfo topo;
     x86_topo_ids_from_idx(smp_cores, smp_threads, cpu_index,
-                          &pkg_id, &core_id, &smt_id);
-    return pkg_id;
+                          &topo);
+    return topo.pkg_id;
 }
 
 static void pc_machine_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 9c6f3a9..148cc1b 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -47,6 +47,12 @@
  */
 typedef uint32_t apic_id_t;
 
+typedef struct X86CPUTopoInfo {
+    unsigned pkg_id;
+    unsigned core_id;
+    unsigned smt_id;
+} X86CPUTopoInfo;
+
 /* Return the bit width needed for 'count' IDs
  */
 static unsigned apicid_bitwidth_for_count(unsigned count)
@@ -92,13 +98,11 @@ static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads)
  */
 static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
                                              unsigned nr_threads,
-                                             unsigned pkg_id,
-                                             unsigned core_id,
-                                             unsigned smt_id)
+                                             const X86CPUTopoInfo *topo)
 {
-    return (pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
-           (core_id << apicid_core_offset(nr_cores, nr_threads)) |
-           smt_id;
+    return (topo->pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
+           (topo->core_id << apicid_core_offset(nr_cores, nr_threads)) |
+           topo->smt_id;
 }
 
 /* Calculate thread/core/package IDs for a specific topology,
@@ -107,14 +111,12 @@ static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
 static inline void x86_topo_ids_from_idx(unsigned nr_cores,
                                          unsigned nr_threads,
                                          unsigned cpu_index,
-                                         unsigned *pkg_id,
-                                         unsigned *core_id,
-                                         unsigned *smt_id)
+                                         X86CPUTopoInfo *topo)
 {
     unsigned core_index = cpu_index / nr_threads;
-    *smt_id = cpu_index % nr_threads;
-    *core_id = core_index % nr_cores;
-    *pkg_id = core_index / nr_cores;
+    topo->smt_id = cpu_index % nr_threads;
+    topo->core_id = core_index % nr_cores;
+    topo->pkg_id = core_index / nr_cores;
 }
 
 /* Make APIC ID for the CPU 'cpu_index'
@@ -125,10 +127,9 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
                                                 unsigned nr_threads,
                                                 unsigned cpu_index)
 {
-    unsigned pkg_id, core_id, smt_id;
-    x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index,
-                          &pkg_id, &core_id, &smt_id);
-    return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id, smt_id);
+    X86CPUTopoInfo topo;
+    x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, &topo);
+    return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
 }
 
 #endif /* HW_I386_TOPOLOGY_H */
-- 
2.1.0

[Qemu-devel] [PULL 2/7] vl: Add another sanity check to smp_parse() function

[Eduardo Habkost]

From: Thomas Huth <thuth@redhat.com>

The code in smp_parse already checks the topology information for
sockets * cores * threads < cpus and bails out with an error in
that case. However, it is still possible to supply a bad configuration
the other way round, e.g. with:

 qemu-system-xxx -smp 4,sockets=1,cores=4,threads=2

QEMU then still starts the guest, with topology configuration that
is rather incomprehensible and likely not what the user wanted.
So let's add another check to refuse such wrong configurations.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index e211f6a..cbc8f25 100644
--- a/vl.c
+++ b/vl.c
@@ -1222,7 +1222,13 @@ static void smp_parse(QemuOpts *opts)
             exit(1);
         }
 
-        max_cpus = qemu_opt_get_number(opts, "maxcpus", 0);
+        max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
+        if (sockets * cores * threads > max_cpus) {
+            fprintf(stderr, "cpu topology: error: "
+                    "sockets (%u) * cores (%u) * threads (%u) > maxcpus (%u)\n",
+                    sockets, cores, threads, max_cpus);
+            exit(1);
+        }
 
         smp_cpus = cpus;
         smp_cores = cores > 0 ? cores : 1;
-- 
2.1.0

[Qemu-devel] [PULL 3/7] target-i386: Enable "check" mode by default

[Eduardo Habkost]

Current default behavior of QEMU is to silently disable features that
are not supported by the host when a CPU model is requested in the
command-line. This means that in addition to risking breaking guest ABI
by default, we are silent about it.

I would like to enable "enforce" by default, but this can easily break
existing production systems because of the way libvirt makes assumptions
about CPU models today (this will change in the future, once QEMU
provide a proper interface for checking if a CPU model is runnable).

But there's no reason we should be silent about it. So, change
target-i386 to enable "check" mode by default so at least we have some
warning printed to stderr (and hopefully logged somewhere) when QEMU
disables a feature that is not supported by the host system.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bd411b9..3d39e8b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3116,7 +3116,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("hv-vapic", X86CPU, hyperv_vapic, false),
     DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
     DEFINE_PROP_BOOL("hv-crash", X86CPU, hyperv_crash, false),
-    DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
+    DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
     DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
-- 
2.1.0

[Qemu-devel] [PULL 4/7] target-i386: Convert kvm_default_*features to property/value pairs

[Eduardo Habkost]

Convert the kvm_default_features and kvm_default_unset_features arrays
into a simple list of property/value pairs that will be applied to
X86CPU objects when using KVM.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c |  8 ++---
 hw/i386/pc_q35.c  |  4 +--
 target-i386/cpu.c | 87 ++++++++++++++++++++++++++++++++-----------------------
 target-i386/cpu.h | 11 +++++--
 4 files changed, 65 insertions(+), 45 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3ffb05f..006163e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -325,7 +325,7 @@ static void pc_compat_2_1(MachineState *machine)
 
     pc_compat_2_2(machine);
     smbios_uuid_encoded = false;
-    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
+    x86_cpu_change_kvm_default("svm", NULL);
     pcms->enforce_aligned_dimm = false;
 }
 
@@ -361,7 +361,7 @@ static void pc_compat_1_7(MachineState *machine)
     gigabyte_align = false;
     option_rom_has_mr = true;
     legacy_acpi_table_size = 6414;
-    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
+    x86_cpu_change_kvm_default("x2apic", NULL);
 }
 
 static void pc_compat_1_6(MachineState *machine)
@@ -391,7 +391,7 @@ static void pc_compat_1_3(MachineState *machine)
 static void pc_compat_1_2(MachineState *machine)
 {
     pc_compat_1_3(machine);
-    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, 1 << KVM_FEATURE_PV_EOI);
+    x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
 }
 
 /* PC compat function for pc-0.10 to pc-0.13 */
@@ -414,7 +414,7 @@ static void pc_init_isa(MachineState *machine)
     if (!machine->cpu_model) {
         machine->cpu_model = "486";
     }
-    x86_cpu_compat_kvm_no_autoenable(FEAT_KVM, 1 << KVM_FEATURE_PV_EOI);
+    x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
     enable_compat_apic_id_mode();
     pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1b7d3b6..48d4183 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -309,7 +309,7 @@ static void pc_compat_2_1(MachineState *machine)
     pc_compat_2_2(machine);
     pcms->enforce_aligned_dimm = false;
     smbios_uuid_encoded = false;
-    x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
+    x86_cpu_change_kvm_default("svm", NULL);
 }
 
 static void pc_compat_2_0(MachineState *machine)
@@ -326,7 +326,7 @@ static void pc_compat_1_7(MachineState *machine)
     smbios_defaults = false;
     gigabyte_align = false;
     option_rom_has_mr = true;
-    x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
+    x86_cpu_change_kvm_default("x2apic", NULL);
 }
 
 static void pc_compat_1_6(MachineState *machine)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3d39e8b..7b29336 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -478,38 +478,6 @@ const char *get_register_name_32(unsigned int reg)
     return x86_reg_info_32[reg].name;
 }
 
-/* KVM-specific features that are automatically added to all CPU models
- * when KVM is enabled.
- */
-static uint32_t kvm_default_features[FEATURE_WORDS] = {
-    [FEAT_KVM] = (1 << KVM_FEATURE_CLOCKSOURCE) |
-        (1 << KVM_FEATURE_NOP_IO_DELAY) |
-        (1 << KVM_FEATURE_CLOCKSOURCE2) |
-        (1 << KVM_FEATURE_ASYNC_PF) |
-        (1 << KVM_FEATURE_STEAL_TIME) |
-        (1 << KVM_FEATURE_PV_EOI) |
-        (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT),
-    [FEAT_1_ECX] = CPUID_EXT_X2APIC,
-};
-
-/* Features that are not added by default to any CPU model when KVM is enabled.
- */
-static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
-    [FEAT_1_EDX] = CPUID_ACPI,
-    [FEAT_1_ECX] = CPUID_EXT_MONITOR,
-    [FEAT_8000_0001_ECX] = CPUID_EXT3_SVM,
-};
-
-void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features)
-{
-    kvm_default_features[w] &= ~features;
-}
-
-void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features)
-{
-    kvm_default_unset_features[w] &= ~features;
-}
-
 /*
  * Returns the set of feature flags that are supported and migratable by
  * QEMU, for a given FeatureWord.
@@ -1392,6 +1360,43 @@ static X86CPUDefinition builtin_x86_defs[] = {
     },
 };
 
+typedef struct PropValue {
+    const char *prop, *value;
+} PropValue;
+
+/* KVM-specific features that are automatically added/removed
+ * from all CPU models when KVM is enabled.
+ */
+static PropValue kvm_default_props[] = {
+    { "kvmclock", "on" },
+    { "kvm-nopiodelay", "on" },
+    { "kvm-asyncpf", "on" },
+    { "kvm-steal-time", "on" },
+    { "kvm-pv-eoi", "on" },
+    { "kvmclock-stable-bit", "on" },
+    { "x2apic", "on" },
+    { "acpi", "off" },
+    { "monitor", "off" },
+    { "svm", "off" },
+    { NULL, NULL },
+};
+
+void x86_cpu_change_kvm_default(const char *prop, const char *value)
+{
+    PropValue *pv;
+    for (pv = kvm_default_props; pv->prop; pv++) {
+        if (!strcmp(pv->prop, prop)) {
+            pv->value = value;
+            break;
+        }
+    }
+
+    /* It is valid to call this function only for properties that
+     * are already present in the kvm_default_props table.
+     */
+    assert(pv->prop);
+}
+
 static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
                                                    bool migratable_only);
 
@@ -2061,6 +2066,18 @@ static int x86_cpu_filter_features(X86CPU *cpu)
     return rv;
 }
 
+static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
+{
+    PropValue *pv;
+    for (pv = props; pv->prop; pv++) {
+        if (!pv->value) {
+            continue;
+        }
+        object_property_parse(OBJECT(cpu), pv->value, pv->prop,
+                              &error_abort);
+    }
+}
+
 /* Load data from X86CPUDefinition
  */
 static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
@@ -2084,11 +2101,7 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
 
     /* Special cases not set in the X86CPUDefinition structs: */
     if (kvm_enabled()) {
-        FeatureWord w;
-        for (w = 0; w < FEATURE_WORDS; w++) {
-            env->features[w] |= kvm_default_features[w];
-            env->features[w] &= ~kvm_default_unset_features[w];
-        }
+        x86_cpu_apply_props(cpu, kvm_default_props);
     }
 
     env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 034fab6..dc7654d 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1341,8 +1341,15 @@ void cpu_smm_update(X86CPU *cpu);
 
 void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
 
-void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
-void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features);
+/* Change the value of a KVM-specific default
+ *
+ * If value is NULL, no default will be set and the original
+ * value from the CPU model table will be kept.
+ *
+ * It is valid to call this funciton only for properties that
+ * are already present in the kvm_default_props table.
+ */
+void x86_cpu_change_kvm_default(const char *prop, const char *value);
 
 
 /* Return name of 32-bit register, from a R_* constant */
-- 
2.1.0

[Qemu-devel] [PULL 5/7] target-i386: Move breakpoint related functions to new file

[Eduardo Habkost]

From: Richard Henderson <rth@twiddle.net>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/Makefile.objs |   2 +-
 target-i386/bpt_helper.c  | 182 ++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/helper.c      | 128 --------------------------------
 target-i386/misc_helper.c |  34 ---------
 4 files changed, 183 insertions(+), 163 deletions(-)
 create mode 100644 target-i386/bpt_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 3da413e..437d997 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o helper.o cpu.o
+obj-y += translate.o helper.o cpu.o bpt_helper.o
 obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
 obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o
 obj-y += gdbstub.o
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
new file mode 100644
index 0000000..6f6537d
--- /dev/null
+++ b/target-i386/bpt_helper.c
@@ -0,0 +1,182 @@
+/*
+ *  i386 breakpoint helpers
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "cpu.h"
+#include "exec/helper-proto.h"
+
+
+void hw_breakpoint_insert(CPUX86State *env, int index)
+{
+    CPUState *cs = CPU(x86_env_get_cpu(env));
+    int type = 0, err = 0;
+
+    switch (hw_breakpoint_type(env->dr[7], index)) {
+    case DR7_TYPE_BP_INST:
+        if (hw_breakpoint_enabled(env->dr[7], index)) {
+            err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
+                                        &env->cpu_breakpoint[index]);
+        }
+        break;
+    case DR7_TYPE_DATA_WR:
+        type = BP_CPU | BP_MEM_WRITE;
+        break;
+    case DR7_TYPE_IO_RW:
+        /* No support for I/O watchpoints yet */
+        break;
+    case DR7_TYPE_DATA_RW:
+        type = BP_CPU | BP_MEM_ACCESS;
+        break;
+    }
+
+    if (type != 0) {
+        err = cpu_watchpoint_insert(cs, env->dr[index],
+                                    hw_breakpoint_len(env->dr[7], index),
+                                    type, &env->cpu_watchpoint[index]);
+    }
+
+    if (err) {
+        env->cpu_breakpoint[index] = NULL;
+    }
+}
+
+void hw_breakpoint_remove(CPUX86State *env, int index)
+{
+    CPUState *cs;
+
+    if (!env->cpu_breakpoint[index]) {
+        return;
+    }
+    cs = CPU(x86_env_get_cpu(env));
+    switch (hw_breakpoint_type(env->dr[7], index)) {
+    case DR7_TYPE_BP_INST:
+        if (hw_breakpoint_enabled(env->dr[7], index)) {
+            cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
+        }
+        break;
+    case DR7_TYPE_DATA_WR:
+    case DR7_TYPE_DATA_RW:
+        cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+        break;
+    case DR7_TYPE_IO_RW:
+        /* No support for I/O watchpoints yet */
+        break;
+    }
+}
+
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
+{
+    target_ulong dr6;
+    int reg;
+    bool hit_enabled = false;
+
+    dr6 = env->dr[6] & ~0xf;
+    for (reg = 0; reg < DR7_MAX_BP; reg++) {
+        bool bp_match = false;
+        bool wp_match = false;
+
+        switch (hw_breakpoint_type(env->dr[7], reg)) {
+        case DR7_TYPE_BP_INST:
+            if (env->dr[reg] == env->eip) {
+                bp_match = true;
+            }
+            break;
+        case DR7_TYPE_DATA_WR:
+        case DR7_TYPE_DATA_RW:
+            if (env->cpu_watchpoint[reg] &&
+                env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
+                wp_match = true;
+            }
+            break;
+        case DR7_TYPE_IO_RW:
+            break;
+        }
+        if (bp_match || wp_match) {
+            dr6 |= 1 << reg;
+            if (hw_breakpoint_enabled(env->dr[7], reg)) {
+                hit_enabled = true;
+            }
+        }
+    }
+
+    if (hit_enabled || force_dr6_update) {
+        env->dr[6] = dr6;
+    }
+
+    return hit_enabled;
+}
+
+void breakpoint_handler(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    CPUBreakpoint *bp;
+
+    if (cs->watchpoint_hit) {
+        if (cs->watchpoint_hit->flags & BP_CPU) {
+            cs->watchpoint_hit = NULL;
+            if (check_hw_breakpoints(env, false)) {
+                raise_exception(env, EXCP01_DB);
+            } else {
+                cpu_resume_from_signal(cs, NULL);
+            }
+        }
+    } else {
+        QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
+            if (bp->pc == env->eip) {
+                if (bp->flags & BP_CPU) {
+                    check_hw_breakpoints(env, true);
+                    raise_exception(env, EXCP01_DB);
+                }
+                break;
+            }
+        }
+    }
+}
+
+void helper_single_step(CPUX86State *env)
+{
+#ifndef CONFIG_USER_ONLY
+    check_hw_breakpoints(env, true);
+    env->dr[6] |= DR6_BS;
+#endif
+    raise_exception(env, EXCP01_DB);
+}
+
+void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
+{
+#ifndef CONFIG_USER_ONLY
+    int i;
+
+    if (reg < 4) {
+        hw_breakpoint_remove(env, reg);
+        env->dr[reg] = t0;
+        hw_breakpoint_insert(env, reg);
+    } else if (reg == 7) {
+        for (i = 0; i < DR7_MAX_BP; i++) {
+            hw_breakpoint_remove(env, i);
+        }
+        env->dr[7] = t0;
+        for (i = 0; i < DR7_MAX_BP; i++) {
+            hw_breakpoint_insert(env, i);
+        }
+    } else {
+        env->dr[reg] = t0;
+    }
+#endif
+}
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 9364d96..d18be95 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1096,134 +1096,6 @@ out:
     return pte | page_offset;
 }
 
-void hw_breakpoint_insert(CPUX86State *env, int index)
-{
-    CPUState *cs = CPU(x86_env_get_cpu(env));
-    int type = 0, err = 0;
-
-    switch (hw_breakpoint_type(env->dr[7], index)) {
-    case DR7_TYPE_BP_INST:
-        if (hw_breakpoint_enabled(env->dr[7], index)) {
-            err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
-                                        &env->cpu_breakpoint[index]);
-        }
-        break;
-    case DR7_TYPE_DATA_WR:
-        type = BP_CPU | BP_MEM_WRITE;
-        break;
-    case DR7_TYPE_IO_RW:
-        /* No support for I/O watchpoints yet */
-        break;
-    case DR7_TYPE_DATA_RW:
-        type = BP_CPU | BP_MEM_ACCESS;
-        break;
-    }
-
-    if (type != 0) {
-        err = cpu_watchpoint_insert(cs, env->dr[index],
-                                    hw_breakpoint_len(env->dr[7], index),
-                                    type, &env->cpu_watchpoint[index]);
-    }
-
-    if (err) {
-        env->cpu_breakpoint[index] = NULL;
-    }
-}
-
-void hw_breakpoint_remove(CPUX86State *env, int index)
-{
-    CPUState *cs;
-
-    if (!env->cpu_breakpoint[index]) {
-        return;
-    }
-    cs = CPU(x86_env_get_cpu(env));
-    switch (hw_breakpoint_type(env->dr[7], index)) {
-    case DR7_TYPE_BP_INST:
-        if (hw_breakpoint_enabled(env->dr[7], index)) {
-            cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
-        }
-        break;
-    case DR7_TYPE_DATA_WR:
-    case DR7_TYPE_DATA_RW:
-        cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
-        break;
-    case DR7_TYPE_IO_RW:
-        /* No support for I/O watchpoints yet */
-        break;
-    }
-}
-
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
-{
-    target_ulong dr6;
-    int reg;
-    bool hit_enabled = false;
-
-    dr6 = env->dr[6] & ~0xf;
-    for (reg = 0; reg < DR7_MAX_BP; reg++) {
-        bool bp_match = false;
-        bool wp_match = false;
-
-        switch (hw_breakpoint_type(env->dr[7], reg)) {
-        case DR7_TYPE_BP_INST:
-            if (env->dr[reg] == env->eip) {
-                bp_match = true;
-            }
-            break;
-        case DR7_TYPE_DATA_WR:
-        case DR7_TYPE_DATA_RW:
-            if (env->cpu_watchpoint[reg] &&
-                env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
-                wp_match = true;
-            }
-            break;
-        case DR7_TYPE_IO_RW:
-            break;
-        }
-        if (bp_match || wp_match) {
-            dr6 |= 1 << reg;
-            if (hw_breakpoint_enabled(env->dr[7], reg)) {
-                hit_enabled = true;
-            }
-        }
-    }
-
-    if (hit_enabled || force_dr6_update) {
-        env->dr[6] = dr6;
-    }
-
-    return hit_enabled;
-}
-
-void breakpoint_handler(CPUState *cs)
-{
-    X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
-    CPUBreakpoint *bp;
-
-    if (cs->watchpoint_hit) {
-        if (cs->watchpoint_hit->flags & BP_CPU) {
-            cs->watchpoint_hit = NULL;
-            if (check_hw_breakpoints(env, false)) {
-                raise_exception(env, EXCP01_DB);
-            } else {
-                cpu_resume_from_signal(cs, NULL);
-            }
-        }
-    } else {
-        QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
-            if (bp->pc == env->eip) {
-                if (bp->flags & BP_CPU) {
-                    check_hw_breakpoints(env, true);
-                    raise_exception(env, EXCP01_DB);
-                }
-                break;
-            }
-        }
-    }
-}
-
 typedef struct MCEInjectionParams {
     Monitor *mon;
     X86CPU *cpu;
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 6bfc7dd..13bd4f5 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -95,15 +95,6 @@ void helper_into(CPUX86State *env, int next_eip_addend)
     }
 }
 
-void helper_single_step(CPUX86State *env)
-{
-#ifndef CONFIG_USER_ONLY
-    check_hw_breakpoints(env, true);
-    env->dr[6] |= DR6_BS;
-#endif
-    raise_exception(env, EXCP01_DB);
-}
-
 void helper_cpuid(CPUX86State *env)
 {
     uint32_t eax, ebx, ecx, edx;
@@ -127,10 +118,6 @@ target_ulong helper_read_crN(CPUX86State *env, int reg)
 void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
 {
 }
-
-void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
-{
-}
 #else
 target_ulong helper_read_crN(CPUX86State *env, int reg)
 {
@@ -176,27 +163,6 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
         break;
     }
 }
-
-void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
-{
-    int i;
-
-    if (reg < 4) {
-        hw_breakpoint_remove(env, reg);
-        env->dr[reg] = t0;
-        hw_breakpoint_insert(env, reg);
-    } else if (reg == 7) {
-        for (i = 0; i < DR7_MAX_BP; i++) {
-            hw_breakpoint_remove(env, i);
-        }
-        env->dr[7] = t0;
-        for (i = 0; i < DR7_MAX_BP; i++) {
-            hw_breakpoint_insert(env, i);
-        }
-    } else {
-        env->dr[reg] = t0;
-    }
-}
 #endif
 
 void helper_lmsw(CPUX86State *env, target_ulong t0)
-- 
2.1.0

[Qemu-devel] [PULL 6/7] target-i386: Make check_hw_breakpoints static

[Eduardo Habkost]

From: Richard Henderson <rth@twiddle.net>

The function is now only used from within a single file.

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

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 6f6537d..c071c24 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -79,7 +79,7 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
     }
 }
 
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
+static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
 {
     target_ulong dr6;
     int reg;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index dc7654d..92426dc 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1154,7 +1154,6 @@ static inline int hw_breakpoint_len(unsigned long dr7, int index)
 
 void hw_breakpoint_insert(CPUX86State *env, int index);
 void hw_breakpoint_remove(CPUX86State *env, int index);
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
 void breakpoint_handler(CPUState *cs);
 
 /* will be suppressed */
-- 
2.1.0

[Qemu-devel] [PULL 7/7] target-i386: get/put MSR_TSC_AUX across reset and migration

[Eduardo Habkost]

From: Amit Shah <amit.shah@redhat.com>

There's one report of migration breaking due to missing MSR_TSC_AUX
save/restore.  Fix this by adding a new subsection that saves the state
of this MSR.

https://bugzilla.redhat.com/show_bug.cgi?id=1261797

Reported-by: Xiaoqing Wei <xwei@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Juan Quintela <quintela@redhat.com>
CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
CC: Marcelo Tosatti <mtosatti@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/kvm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7b0ba17..80d1a7e 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -67,6 +67,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static bool has_msr_star;
 static bool has_msr_hsave_pa;
+static bool has_msr_tsc_aux;
 static bool has_msr_tsc_adjust;
 static bool has_msr_tsc_deadline;
 static bool has_msr_feature_control;
@@ -825,6 +826,10 @@ static int kvm_get_supported_msrs(KVMState *s)
                     has_msr_hsave_pa = true;
                     continue;
                 }
+                if (kvm_msr_list->indices[i] == MSR_TSC_AUX) {
+                    has_msr_tsc_aux = true;
+                    continue;
+                }
                 if (kvm_msr_list->indices[i] == MSR_TSC_ADJUST) {
                     has_msr_tsc_adjust = true;
                     continue;
@@ -1299,6 +1304,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
     if (has_msr_hsave_pa) {
         kvm_msr_entry_set(&msrs[n++], MSR_VM_HSAVE_PA, env->vm_hsave);
     }
+    if (has_msr_tsc_aux) {
+        kvm_msr_entry_set(&msrs[n++], MSR_TSC_AUX, env->tsc_aux);
+    }
     if (has_msr_tsc_adjust) {
         kvm_msr_entry_set(&msrs[n++], MSR_TSC_ADJUST, env->tsc_adjust);
     }
@@ -1671,6 +1679,9 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (has_msr_hsave_pa) {
         msrs[n++].index = MSR_VM_HSAVE_PA;
     }
+    if (has_msr_tsc_aux) {
+        msrs[n++].index = MSR_TSC_AUX;
+    }
     if (has_msr_tsc_adjust) {
         msrs[n++].index = MSR_TSC_ADJUST;
     }
@@ -1820,6 +1831,9 @@ static int kvm_get_msrs(X86CPU *cpu)
         case MSR_IA32_TSC:
             env->tsc = msrs[i].data;
             break;
+        case MSR_TSC_AUX:
+            env->tsc_aux = msrs[i].data;
+            break;
         case MSR_TSC_ADJUST:
             env->tsc_adjust = msrs[i].data;
             break;
-- 
2.1.0

Re: [Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Peter Maydell]

On 28 September 2015 at 18:31, Eduardo Habkost <ehabkost@redhat.com> wrote:
> The following changes since commit 9e071429e649346c14b2dc76902f84f8352d2333:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-09-25 21:52:30 +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 139454c5119cf0da270901810de4f51cc0f28bdd:
>
>   target-i386: get/put MSR_TSC_AUX across reset and migration (2015-09-28 12:46:40 -0300)
>
> ----------------------------------------------------------------
> X86 queue, 2015-09-28
>
> ----------------------------------------------------------------

I get a bunch of new warnings in the course of 'make check' now:

warning: TCG doesn't support requested feature: CPUID.01H:EDX.de [bit 2]

/home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:1494:15:
runtime error: left shift of 1 by 31 places cannot be represented in
type 'int'

and also
warning: TCG doesn't support requested feature: CPUID.80000001H:EDX [bit 2]

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Peter Maydell]

On 29 September 2015 at 12:28, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 28 September 2015 at 18:31, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> The following changes since commit 9e071429e649346c14b2dc76902f84f8352d2333:
>>
>>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-09-25 21:52:30 +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 139454c5119cf0da270901810de4f51cc0f28bdd:
>>
>>   target-i386: get/put MSR_TSC_AUX across reset and migration (2015-09-28 12:46:40 -0300)
>>
>> ----------------------------------------------------------------
>> X86 queue, 2015-09-28
>>
>> ----------------------------------------------------------------
>
> I get a bunch of new warnings in the course of 'make check' now:
>
> warning: TCG doesn't support requested feature: CPUID.01H:EDX.de [bit 2]
>
> /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:1494:15:
> runtime error: left shift of 1 by 31 places cannot be represented in
> type 'int'
>
> and also
> warning: TCG doesn't support requested feature: CPUID.80000001H:EDX [bit 2]

...and these also appear for simple running of the x86_64-linux-user/qemu-x86_64
binaries, which is definitely not ok.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Eduardo Habkost]

On Tue, Sep 29, 2015 at 12:28:15PM +0100, Peter Maydell wrote:
> On 28 September 2015 at 18:31, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > The following changes since commit 9e071429e649346c14b2dc76902f84f8352d2333:
> >
> >   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-09-25 21:52:30 +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 139454c5119cf0da270901810de4f51cc0f28bdd:
> >
> >   target-i386: get/put MSR_TSC_AUX across reset and migration (2015-09-28 12:46:40 -0300)
> >
> > ----------------------------------------------------------------
> > X86 queue, 2015-09-28
> >
> > ----------------------------------------------------------------
> 
> I get a bunch of new warnings in the course of 'make check' now:
> 
> warning: TCG doesn't support requested feature: CPUID.01H:EDX.de [bit 2]
> 
> /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:1494:15:
> runtime error: left shift of 1 by 31 places cannot be represented in
> type 'int'

Which compiler are you using to generate this check?


> 
> and also
> warning: TCG doesn't support requested feature: CPUID.80000001H:EDX [bit 2]

The warnings were expected and will be fixed by the series that
implements DE (to be merged soon). But I didn't notice they were so
intrusive (and triggered an existing bug at
report_unavailable_features()).

I will wait until the DE series get included before enabling the
warnings in the next pull request. Sorry for the noise.

-- 
Eduardo

Re: [Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Peter Maydell]

On 29 September 2015 at 19:22, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Sep 29, 2015 at 12:28:15PM +0100, Peter Maydell wrote:
>> I get a bunch of new warnings in the course of 'make check' now:
>>
>> warning: TCG doesn't support requested feature: CPUID.01H:EDX.de [bit 2]
>>
>> /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:1494:15:
>> runtime error: left shift of 1 by 31 places cannot be represented in
>> type 'int'
>
> Which compiler are you using to generate this check?

That's clang's -fsanitize=undefined (which makes it emit runtime
warnings about undefined behaviour).

>
>>
>> and also
>> warning: TCG doesn't support requested feature: CPUID.80000001H:EDX [bit 2]
>
> The warnings were expected and will be fixed by the series that
> implements DE (to be merged soon). But I didn't notice they were so
> intrusive (and triggered an existing bug at
> report_unavailable_features()).
>
> I will wait until the DE series get included before enabling the
> warnings in the next pull request. Sorry for the noise.

Generally we make minor emulation issues report via
qemu_log(LOG_UNIMP, ...) rather than stderr (and only when
the guest actually triggers them, ideally). That way you
only see them if you care.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] X86 queue, 2015-09-28

[Eduardo Habkost]

On Tue, Sep 29, 2015 at 08:21:49PM +0100, Peter Maydell wrote:
> On 29 September 2015 at 19:22, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > On Tue, Sep 29, 2015 at 12:28:15PM +0100, Peter Maydell wrote:
> >> I get a bunch of new warnings in the course of 'make check' now:
> >>
> >> warning: TCG doesn't support requested feature: CPUID.01H:EDX.de [bit 2]
> >>
> >> /home/petmay01/linaro/qemu-for-merges/target-i386/cpu.c:1494:15:
> >> runtime error: left shift of 1 by 31 places cannot be represented in
> >> type 'int'
> >
> > Which compiler are you using to generate this check?
> 
> That's clang's -fsanitize=undefined (which makes it emit runtime
> warnings about undefined behaviour).

Thanks. I seeing other similar runtime errors in target-i386, I will
take a look at them.

> 
> >
> >>
> >> and also
> >> warning: TCG doesn't support requested feature: CPUID.80000001H:EDX [bit 2]
> >
> > The warnings were expected and will be fixed by the series that
> > implements DE (to be merged soon). But I didn't notice they were so
> > intrusive (and triggered an existing bug at
> > report_unavailable_features()).
> >
> > I will wait until the DE series get included before enabling the
> > warnings in the next pull request. Sorry for the noise.
> 
> Generally we make minor emulation issues report via
> qemu_log(LOG_UNIMP, ...) rather than stderr (and only when
> the guest actually triggers them, ideally). That way you
> only see them if you care.

That warning is for features that were explicitly requested in the
command-line (and in that case, they are not minor issues, because QEMU
is not doing what the user asked for). The bug is that the default CPU
configuration is triggering the warning, but this is going to be fixed
when we add the series implementing Debugging Extensions on TCG.

-- 
Eduardo