[PATCH v6 0/3] target-i386: save/restore vcpu's TSC rate during migration

[PATCH v6 0/3] target-i386: save/restore vcpu's TSC rate during migration

[Haozhong Zhang]

This patchset enables QEMU to save/restore vcpu's TSC rate during the
migration on machine types pc-*-2.5 or newer.

On the source machine:
 * If the vcpu's TSC rate is specified by the cpu option 'tsc-freq',
   then this user-specified TSC rate will be migrated.
 * Otherwise, the TSC rate returned by KVM_GET_TSC_KHZ will be
   migrated. For a fresh VM, this is the host TSC rate.

On the destination machine:
 * If the vcpu's TSC rate has been specified by the cpu option
   'tsc-freq' and is inconsistent with the migrated TSC rate, then
   the migration will be aborted.
 * Otherwise, QEMU will try to use the migrated TSC rate. If KVM on
   the destination supports TSC scaling, guest programs will observe a
   consistent TSC rate across the migration. If TSC scaling is not
   supported, the migration will not be aborted and QEMU will behave
   like before, i.e using the host TSC rate instead.

Changes in v6:
 * Make kvm_arch_init_vcpu() fail if KVM_CAP_TSC_CONTROL is unavailable
   when setting user-specified TSC rate.
 * Minor adjustment of kvm_arch_set_tsc_khz() suggested by Eduardo.
 * Other comments and error handling adjustments suggested by Eduardo.

Changes in v5:
 * Move KVM_GET_TSC_KHZ call to kvm_arch_init_vcpu().
 * Remove an unnecessary warning message.
 * Unify TSC rate setting code in kvm_arch_init_vcpu() and
   kvm_arch_put_registers().

Changes in v4:
 * Make all code x86 specific.
 * Abort the migration if the user-specified TSC rate is inconsistent
   with the migrated TSC rate.
 * Move the sanity check to cpu_post_load().
 * All KVM_SET_TSC_KHZ and save/restore use env->tsc_khz.
 * Replace env->tsc_khz_saved with env->user_tsc_khz, and only use the
   latter for sanity check.

Changes in v3:
 * Change the cpu option 'save-tsc-freq' to an internal flag.
 * Remove the cpu option 'load-tsc-freq' and change the logic of
   loading the migrated TSC rate as above.
 * Move the setup of migrated TSC rate back to
   do_kvm_cpu_synchronize_post_init().

Changes in v2:
 * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
   control the migration of vcpu's TSC rate.
 * Move all logic of setting TSC rate to target-i386.
 * Remove the duplicated TSC setup in kvm_arch_init_vcpu().

Haozhong Zhang (3):
  target-i386: fallback vcpu's TSC rate to value returned by KVM
  target-i386: reorganize TSC rate setting code
  target-i386: add support to migrate vcpu's TSC rate

 hw/i386/pc.c          |  1 +
 hw/i386/pc_piix.c     |  1 +
 hw/i386/pc_q35.c      |  1 +
 include/hw/i386/pc.h  |  1 +
 target-i386/cpu.c     |  2 +-
 target-i386/cpu.h     |  1 +
 target-i386/kvm.c     | 74 ++++++++++++++++++++++++++++++++++++++++++++++-----
 target-i386/machine.c | 30 +++++++++++++++++++++
 8 files changed, 104 insertions(+), 7 deletions(-)

-- 
2.4.8

[Qemu-devel] [PATCH v6 0/3] target-i386: save/restore vcpu's TSC rate during migration

[Haozhong Zhang]

This patchset enables QEMU to save/restore vcpu's TSC rate during the
migration on machine types pc-*-2.5 or newer.

On the source machine:
 * If the vcpu's TSC rate is specified by the cpu option 'tsc-freq',
   then this user-specified TSC rate will be migrated.
 * Otherwise, the TSC rate returned by KVM_GET_TSC_KHZ will be
   migrated. For a fresh VM, this is the host TSC rate.

On the destination machine:
 * If the vcpu's TSC rate has been specified by the cpu option
   'tsc-freq' and is inconsistent with the migrated TSC rate, then
   the migration will be aborted.
 * Otherwise, QEMU will try to use the migrated TSC rate. If KVM on
   the destination supports TSC scaling, guest programs will observe a
   consistent TSC rate across the migration. If TSC scaling is not
   supported, the migration will not be aborted and QEMU will behave
   like before, i.e using the host TSC rate instead.

Changes in v6:
 * Make kvm_arch_init_vcpu() fail if KVM_CAP_TSC_CONTROL is unavailable
   when setting user-specified TSC rate.
 * Minor adjustment of kvm_arch_set_tsc_khz() suggested by Eduardo.
 * Other comments and error handling adjustments suggested by Eduardo.

Changes in v5:
 * Move KVM_GET_TSC_KHZ call to kvm_arch_init_vcpu().
 * Remove an unnecessary warning message.
 * Unify TSC rate setting code in kvm_arch_init_vcpu() and
   kvm_arch_put_registers().

Changes in v4:
 * Make all code x86 specific.
 * Abort the migration if the user-specified TSC rate is inconsistent
   with the migrated TSC rate.
 * Move the sanity check to cpu_post_load().
 * All KVM_SET_TSC_KHZ and save/restore use env->tsc_khz.
 * Replace env->tsc_khz_saved with env->user_tsc_khz, and only use the
   latter for sanity check.

Changes in v3:
 * Change the cpu option 'save-tsc-freq' to an internal flag.
 * Remove the cpu option 'load-tsc-freq' and change the logic of
   loading the migrated TSC rate as above.
 * Move the setup of migrated TSC rate back to
   do_kvm_cpu_synchronize_post_init().

Changes in v2:
 * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
   control the migration of vcpu's TSC rate.
 * Move all logic of setting TSC rate to target-i386.
 * Remove the duplicated TSC setup in kvm_arch_init_vcpu().

Haozhong Zhang (3):
  target-i386: fallback vcpu's TSC rate to value returned by KVM
  target-i386: reorganize TSC rate setting code
  target-i386: add support to migrate vcpu's TSC rate

 hw/i386/pc.c          |  1 +
 hw/i386/pc_piix.c     |  1 +
 hw/i386/pc_q35.c      |  1 +
 include/hw/i386/pc.h  |  1 +
 target-i386/cpu.c     |  2 +-
 target-i386/cpu.h     |  1 +
 target-i386/kvm.c     | 74 ++++++++++++++++++++++++++++++++++++++++++++++-----
 target-i386/machine.c | 30 +++++++++++++++++++++
 8 files changed, 104 insertions(+), 7 deletions(-)

-- 
2.4.8

[PATCH v6 1/3] target-i386: fallback vcpu's TSC rate to value returned by KVM

[Haozhong Zhang]

If no user-specified TSC rate is present, we will try to set
env->tsc_khz to the value returned by KVM_GET_TSC_KHZ. This patch does
not change the current functionality of QEMU and just prepares for later
patches to enable migrating vcpu's TSC rate.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 target-i386/kvm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2a9953b..a0fe9d4 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -832,6 +832,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
+    /* vcpu's TSC frequency is either specified by user, or following
+     * the value used by KVM if the former is not present. In the
+     * latter case, we query it from KVM and record in env->tsc_khz,
+     * so that vcpu's TSC frequency can be migrated later via this field.
+     */
+    if (!env->tsc_khz) {
+        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
+            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
+            -ENOTSUP;
+        if (r > 0) {
+            env->tsc_khz = r;
+        }
+    }
+
     if (has_xsave) {
         env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
     }
-- 
2.4.8

[Qemu-devel] [PATCH v6 1/3] target-i386: fallback vcpu's TSC rate to value returned by KVM

[Haozhong Zhang]

If no user-specified TSC rate is present, we will try to set
env->tsc_khz to the value returned by KVM_GET_TSC_KHZ. This patch does
not change the current functionality of QEMU and just prepares for later
patches to enable migrating vcpu's TSC rate.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 target-i386/kvm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2a9953b..a0fe9d4 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -832,6 +832,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
+    /* vcpu's TSC frequency is either specified by user, or following
+     * the value used by KVM if the former is not present. In the
+     * latter case, we query it from KVM and record in env->tsc_khz,
+     * so that vcpu's TSC frequency can be migrated later via this field.
+     */
+    if (!env->tsc_khz) {
+        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
+            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
+            -ENOTSUP;
+        if (r > 0) {
+            env->tsc_khz = r;
+        }
+    }
+
     if (has_xsave) {
         env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
     }
-- 
2.4.8

[PATCH v6 2/3] target-i386: reorganize TSC rate setting code

[Haozhong Zhang]

Following changes are made to the TSC rate setting code in
kvm_arch_init_vcpu():
 * The code is moved to a new function kvm_arch_set_tsc_khz().
 * If kvm_arch_set_tsc_khz() fails, i.e. following two conditions are
   both satisfied:
   * KVM does not support the TSC scaling or it fails to set vcpu's
     TSC rate by KVM_SET_TSC_KHZ,
   * the TSC rate to be set is different than the value currently used
     by KVM,
   then kvm_arch_init_vcpu() will fail. Prevously,
   * the lack of TSC scaling never failed kvm_arch_init_vcpu(),
   * the failure of KVM_SET_TSC_KHZ failed kvm_arch_init_vcpu()
     unconditionally, even though the TSC rate to be set is identical
     to the value currently used by KVM.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 target-i386/kvm.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a0fe9d4..1e811ee 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -524,6 +524,36 @@ static bool hyperv_enabled(X86CPU *cpu)
             cpu->hyperv_runtime);
 }
 
+static int kvm_arch_set_tsc_khz(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    int r;
+
+    if (!env->tsc_khz) {
+        return 0;
+    }
+
+    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
+        kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) :
+        -ENOTSUP;
+    if (r < 0) {
+        /* When KVM_SET_TSC_KHZ fails, it's an error only if the current
+         * TSC frequency doesn't match the one we want.
+         */
+        int cur_freq = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
+                       kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
+                       -ENOTSUP;
+        if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
+            error_report("warning: TSC frequency mismatch between "
+                         "VM and host, and TSC scaling unavailable");
+            return r;
+        }
+    }
+
+    return 0;
+}
+
 static Error *invtsc_mig_blocker;
 
 #define KVM_MAX_CPUID_ENTRIES  100
@@ -823,13 +853,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return r;
     }
 
-    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL);
-    if (r && env->tsc_khz) {
-        r = kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz);
-        if (r < 0) {
-            fprintf(stderr, "KVM_SET_TSC_KHZ failed\n");
-            return r;
-        }
+    r = kvm_arch_set_tsc_khz(cs);
+    if (r < 0) {
+        return r;
     }
 
     /* vcpu's TSC frequency is either specified by user, or following
-- 
2.4.8

[Qemu-devel] [PATCH v6 2/3] target-i386: reorganize TSC rate setting code

[Haozhong Zhang]

Following changes are made to the TSC rate setting code in
kvm_arch_init_vcpu():
 * The code is moved to a new function kvm_arch_set_tsc_khz().
 * If kvm_arch_set_tsc_khz() fails, i.e. following two conditions are
   both satisfied:
   * KVM does not support the TSC scaling or it fails to set vcpu's
     TSC rate by KVM_SET_TSC_KHZ,
   * the TSC rate to be set is different than the value currently used
     by KVM,
   then kvm_arch_init_vcpu() will fail. Prevously,
   * the lack of TSC scaling never failed kvm_arch_init_vcpu(),
   * the failure of KVM_SET_TSC_KHZ failed kvm_arch_init_vcpu()
     unconditionally, even though the TSC rate to be set is identical
     to the value currently used by KVM.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 target-i386/kvm.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a0fe9d4..1e811ee 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -524,6 +524,36 @@ static bool hyperv_enabled(X86CPU *cpu)
             cpu->hyperv_runtime);
 }
 
+static int kvm_arch_set_tsc_khz(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    int r;
+
+    if (!env->tsc_khz) {
+        return 0;
+    }
+
+    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
+        kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) :
+        -ENOTSUP;
+    if (r < 0) {
+        /* When KVM_SET_TSC_KHZ fails, it's an error only if the current
+         * TSC frequency doesn't match the one we want.
+         */
+        int cur_freq = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
+                       kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
+                       -ENOTSUP;
+        if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
+            error_report("warning: TSC frequency mismatch between "
+                         "VM and host, and TSC scaling unavailable");
+            return r;
+        }
+    }
+
+    return 0;
+}
+
 static Error *invtsc_mig_blocker;
 
 #define KVM_MAX_CPUID_ENTRIES  100
@@ -823,13 +853,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return r;
     }
 
-    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL);
-    if (r && env->tsc_khz) {
-        r = kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz);
-        if (r < 0) {
-            fprintf(stderr, "KVM_SET_TSC_KHZ failed\n");
-            return r;
-        }
+    r = kvm_arch_set_tsc_khz(cs);
+    if (r < 0) {
+        return r;
     }
 
     /* vcpu's TSC frequency is either specified by user, or following
-- 
2.4.8

[PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Haozhong Zhang]

This patch enables migrating vcpu's TSC rate. If KVM on the destination
machine supports TSC scaling, guest programs will observe a consistent
TSC rate across the migration.

If TSC scaling is not supported on the destination machine, the
migration will not be aborted and QEMU on the destination will not set
vcpu's TSC rate to the migrated value.

If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
machine is inconsistent with the migrated TSC rate, the migration will
be aborted.

For backwards compatibility, the migration of vcpu's TSC rate is
disabled on pc-*-2.4 and older machine types.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 hw/i386/pc.c          |  1 +
 hw/i386/pc_piix.c     |  1 +
 hw/i386/pc_q35.c      |  1 +
 include/hw/i386/pc.h  |  1 +
 target-i386/cpu.c     |  2 +-
 target-i386/cpu.h     |  1 +
 target-i386/kvm.c     | 22 ++++++++++++++++++++++
 target-i386/machine.c | 30 ++++++++++++++++++++++++++++++
 8 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e20e07..72d9b9c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1952,6 +1952,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     pcmc->get_hotplug_handler = mc->get_hotplug_handler;
+    pcmc->save_tsc_khz = true;
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
     mc->default_boot_order = "cad";
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 07d0baa..7c5b0d2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -489,6 +489,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
     m->alias = NULL;
     m->is_default = 0;
     pcmc->broken_reserved_end = true;
+    pcmc->save_tsc_khz = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0fdae09..fd8efe3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -387,6 +387,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
     m->hw_version = "2.4.0";
     m->alias = NULL;
     pcmc->broken_reserved_end = true;
+    pcmc->save_tsc_khz = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..3b8f368 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -60,6 +60,7 @@ struct PCMachineClass {
 
     /*< public >*/
     bool broken_reserved_end;
+    bool save_tsc_khz;
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
 };
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 11e5e39..3c0b720 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1728,7 +1728,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
-    cpu->env.tsc_khz = value / 1000;
+    cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000;
 }
 
 static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index fc4a605..ffe0bce 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -973,6 +973,7 @@ typedef struct CPUX86State {
     uint32_t sipi_vector;
     bool tsc_valid;
     int64_t tsc_khz;
+    int64_t user_tsc_khz; /* for sanity check only */
     void *kvm_xsave_buf;
 
     uint64_t mcg_cap;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1e811ee..2a0fd54 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2381,6 +2381,28 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
         }
     }
 
+    if (level == KVM_PUT_FULL_STATE) {
+        /* kvm_arch_set_tsc_khz() below can be called in two control flows and
+         * we don't need to handle its errors in both of them.
+         *
+         * One is the control flow that creates a vcpu, where
+         * kvm_arch_set_tsc_khz() has already been called once before by
+         * kvm_arch_init_vcpu(). The latter will abort the control flow if there
+         * are any errors of kvm_arch_set_tsc_khz(). Thus, in this control flow,
+         * kvm_arch_set_tsc_khz() below never fails and we can safely ignore its
+         * return values here.
+         *
+         * Another is the control flow of migration that sets vcpu's TSC
+         * frequency on the destination. The only error that can fail the
+         * migration is the mismatch between the migrated and the user-specified
+         * TSC frequencies, which has been handled by cpu_post_load(). Other
+         * errors, i.e. those from kvm_arch_set_tsc_khz(), never fail the
+         * migration, so we also safely ignore its return values in this control
+         * flow.
+         */
+        kvm_arch_set_tsc_khz(cpu);
+    }
+
     ret = kvm_getput_regs(x86_cpu, 1);
     if (ret < 0) {
         return ret;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a18e16e..e560ca3 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -6,6 +6,8 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 
+#include "qemu/error-report.h"
+
 static const VMStateDescription vmstate_segment = {
     .name = "segment",
     .version_id = 1,
@@ -331,6 +333,13 @@ static int cpu_post_load(void *opaque, int version_id)
     CPUX86State *env = &cpu->env;
     int i;
 
+    if (env->tsc_khz && env->user_tsc_khz &&
+        env->tsc_khz != env->user_tsc_khz) {
+        error_report("Mismatch between user-specified TSC frequency and "
+                     "migrated TSC frequency");
+        return -EINVAL;
+    }
+
     /*
      * Real mode guest segments register DPL should be zero.
      * Older KVM version were setting it wrongly.
@@ -775,6 +784,26 @@ static const VMStateDescription vmstate_xss = {
     }
 };
 
+static bool tsc_khz_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
+    return env->tsc_khz && pcmc->save_tsc_khz;
+}
+
+static const VMStateDescription vmstate_tsc_khz = {
+    .name = "cpu/tsc_khz",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = tsc_khz_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT64(env.tsc_khz, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
@@ -895,6 +924,7 @@ VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_hyperv_runtime,
         &vmstate_avx512,
         &vmstate_xss,
+        &vmstate_tsc_khz,
         NULL
     }
 };
-- 
2.4.8

[Qemu-devel] [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Haozhong Zhang]

This patch enables migrating vcpu's TSC rate. If KVM on the destination
machine supports TSC scaling, guest programs will observe a consistent
TSC rate across the migration.

If TSC scaling is not supported on the destination machine, the
migration will not be aborted and QEMU on the destination will not set
vcpu's TSC rate to the migrated value.

If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
machine is inconsistent with the migrated TSC rate, the migration will
be aborted.

For backwards compatibility, the migration of vcpu's TSC rate is
disabled on pc-*-2.4 and older machine types.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 hw/i386/pc.c          |  1 +
 hw/i386/pc_piix.c     |  1 +
 hw/i386/pc_q35.c      |  1 +
 include/hw/i386/pc.h  |  1 +
 target-i386/cpu.c     |  2 +-
 target-i386/cpu.h     |  1 +
 target-i386/kvm.c     | 22 ++++++++++++++++++++++
 target-i386/machine.c | 30 ++++++++++++++++++++++++++++++
 8 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e20e07..72d9b9c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1952,6 +1952,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     pcmc->get_hotplug_handler = mc->get_hotplug_handler;
+    pcmc->save_tsc_khz = true;
     mc->get_hotplug_handler = pc_get_hotpug_handler;
     mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
     mc->default_boot_order = "cad";
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 07d0baa..7c5b0d2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -489,6 +489,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
     m->alias = NULL;
     m->is_default = 0;
     pcmc->broken_reserved_end = true;
+    pcmc->save_tsc_khz = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0fdae09..fd8efe3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -387,6 +387,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
     m->hw_version = "2.4.0";
     m->alias = NULL;
     pcmc->broken_reserved_end = true;
+    pcmc->save_tsc_khz = false;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
 }
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..3b8f368 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -60,6 +60,7 @@ struct PCMachineClass {
 
     /*< public >*/
     bool broken_reserved_end;
+    bool save_tsc_khz;
     HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
                                            DeviceState *dev);
 };
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 11e5e39..3c0b720 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1728,7 +1728,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
-    cpu->env.tsc_khz = value / 1000;
+    cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000;
 }
 
 static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index fc4a605..ffe0bce 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -973,6 +973,7 @@ typedef struct CPUX86State {
     uint32_t sipi_vector;
     bool tsc_valid;
     int64_t tsc_khz;
+    int64_t user_tsc_khz; /* for sanity check only */
     void *kvm_xsave_buf;
 
     uint64_t mcg_cap;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1e811ee..2a0fd54 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2381,6 +2381,28 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
         }
     }
 
+    if (level == KVM_PUT_FULL_STATE) {
+        /* kvm_arch_set_tsc_khz() below can be called in two control flows and
+         * we don't need to handle its errors in both of them.
+         *
+         * One is the control flow that creates a vcpu, where
+         * kvm_arch_set_tsc_khz() has already been called once before by
+         * kvm_arch_init_vcpu(). The latter will abort the control flow if there
+         * are any errors of kvm_arch_set_tsc_khz(). Thus, in this control flow,
+         * kvm_arch_set_tsc_khz() below never fails and we can safely ignore its
+         * return values here.
+         *
+         * Another is the control flow of migration that sets vcpu's TSC
+         * frequency on the destination. The only error that can fail the
+         * migration is the mismatch between the migrated and the user-specified
+         * TSC frequencies, which has been handled by cpu_post_load(). Other
+         * errors, i.e. those from kvm_arch_set_tsc_khz(), never fail the
+         * migration, so we also safely ignore its return values in this control
+         * flow.
+         */
+        kvm_arch_set_tsc_khz(cpu);
+    }
+
     ret = kvm_getput_regs(x86_cpu, 1);
     if (ret < 0) {
         return ret;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a18e16e..e560ca3 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -6,6 +6,8 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 
+#include "qemu/error-report.h"
+
 static const VMStateDescription vmstate_segment = {
     .name = "segment",
     .version_id = 1,
@@ -331,6 +333,13 @@ static int cpu_post_load(void *opaque, int version_id)
     CPUX86State *env = &cpu->env;
     int i;
 
+    if (env->tsc_khz && env->user_tsc_khz &&
+        env->tsc_khz != env->user_tsc_khz) {
+        error_report("Mismatch between user-specified TSC frequency and "
+                     "migrated TSC frequency");
+        return -EINVAL;
+    }
+
     /*
      * Real mode guest segments register DPL should be zero.
      * Older KVM version were setting it wrongly.
@@ -775,6 +784,26 @@ static const VMStateDescription vmstate_xss = {
     }
 };
 
+static bool tsc_khz_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
+    return env->tsc_khz && pcmc->save_tsc_khz;
+}
+
+static const VMStateDescription vmstate_tsc_khz = {
+    .name = "cpu/tsc_khz",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = tsc_khz_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT64(env.tsc_khz, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
@@ -895,6 +924,7 @@ VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_hyperv_runtime,
         &vmstate_avx512,
         &vmstate_xss,
+        &vmstate_tsc_khz,
         NULL
     }
 };
-- 
2.4.8

Re: [PATCH v6 1/3] target-i386: fallback vcpu's TSC rate to value returned by KVM

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:55AM +0800, Haozhong Zhang wrote:
> If no user-specified TSC rate is present, we will try to set
> env->tsc_khz to the value returned by KVM_GET_TSC_KHZ. This patch does
> not change the current functionality of QEMU and just prepares for later
> patches to enable migrating vcpu's TSC rate.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-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 2a9953b..a0fe9d4 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -832,6 +832,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          }
>      }
>  
> +    /* vcpu's TSC frequency is either specified by user, or following
> +     * the value used by KVM if the former is not present. In the
> +     * latter case, we query it from KVM and record in env->tsc_khz,
> +     * so that vcpu's TSC frequency can be migrated later via this field.
> +     */
> +    if (!env->tsc_khz) {
> +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> +            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> +            -ENOTSUP;
> +        if (r > 0) {
> +            env->tsc_khz = r;
> +        }
> +    }
> +
>      if (has_xsave) {
>          env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
>      }
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 1/3] target-i386: fallback vcpu's TSC rate to value returned by KVM

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:55AM +0800, Haozhong Zhang wrote:
> If no user-specified TSC rate is present, we will try to set
> env->tsc_khz to the value returned by KVM_GET_TSC_KHZ. This patch does
> not change the current functionality of QEMU and just prepares for later
> patches to enable migrating vcpu's TSC rate.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-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 2a9953b..a0fe9d4 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -832,6 +832,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          }
>      }
>  
> +    /* vcpu's TSC frequency is either specified by user, or following
> +     * the value used by KVM if the former is not present. In the
> +     * latter case, we query it from KVM and record in env->tsc_khz,
> +     * so that vcpu's TSC frequency can be migrated later via this field.
> +     */
> +    if (!env->tsc_khz) {
> +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> +            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> +            -ENOTSUP;
> +        if (r > 0) {
> +            env->tsc_khz = r;
> +        }
> +    }
> +
>      if (has_xsave) {
>          env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
>      }
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [PATCH v6 1/3] target-i386: fallback vcpu's TSC rate to value returned by KVM

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:55AM +0800, Haozhong Zhang wrote:
> If no user-specified TSC rate is present, we will try to set
> env->tsc_khz to the value returned by KVM_GET_TSC_KHZ. This patch does
> not change the current functionality of QEMU and just prepares for later
> patches to enable migrating vcpu's TSC rate.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-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 2a9953b..a0fe9d4 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -832,6 +832,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          }
>      }
>  
> +    /* vcpu's TSC frequency is either specified by user, or following
> +     * the value used by KVM if the former is not present. In the
> +     * latter case, we query it from KVM and record in env->tsc_khz,
> +     * so that vcpu's TSC frequency can be migrated later via this field.
> +     */
> +    if (!env->tsc_khz) {
> +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> +            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> +            -ENOTSUP;
> +        if (r > 0) {
> +            env->tsc_khz = r;
> +        }
> +    }
> +
>      if (has_xsave) {
>          env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
>      }
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 1/3] target-i386: fallback vcpu's TSC rate to value returned by KVM

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:55AM +0800, Haozhong Zhang wrote:
> If no user-specified TSC rate is present, we will try to set
> env->tsc_khz to the value returned by KVM_GET_TSC_KHZ. This patch does
> not change the current functionality of QEMU and just prepares for later
> patches to enable migrating vcpu's TSC rate.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-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 2a9953b..a0fe9d4 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -832,6 +832,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          }
>      }
>  
> +    /* vcpu's TSC frequency is either specified by user, or following
> +     * the value used by KVM if the former is not present. In the
> +     * latter case, we query it from KVM and record in env->tsc_khz,
> +     * so that vcpu's TSC frequency can be migrated later via this field.
> +     */
> +    if (!env->tsc_khz) {
> +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> +            kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> +            -ENOTSUP;
> +        if (r > 0) {
> +            env->tsc_khz = r;
> +        }
> +    }
> +
>      if (has_xsave) {
>          env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
>      }
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [PATCH v6 2/3] target-i386: reorganize TSC rate setting code

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:56AM +0800, Haozhong Zhang wrote:
> Following changes are made to the TSC rate setting code in
> kvm_arch_init_vcpu():
>  * The code is moved to a new function kvm_arch_set_tsc_khz().
>  * If kvm_arch_set_tsc_khz() fails, i.e. following two conditions are
>    both satisfied:
>    * KVM does not support the TSC scaling or it fails to set vcpu's
>      TSC rate by KVM_SET_TSC_KHZ,
>    * the TSC rate to be set is different than the value currently used
>      by KVM,
>    then kvm_arch_init_vcpu() will fail. Prevously,
>    * the lack of TSC scaling never failed kvm_arch_init_vcpu(),
>    * the failure of KVM_SET_TSC_KHZ failed kvm_arch_init_vcpu()
>      unconditionally, even though the TSC rate to be set is identical
>      to the value currently used by KVM.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/kvm.c | 40 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index a0fe9d4..1e811ee 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -524,6 +524,36 @@ static bool hyperv_enabled(X86CPU *cpu)
>              cpu->hyperv_runtime);
>  }
>  
> +static int kvm_arch_set_tsc_khz(CPUState *cs)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +    int r;
> +
> +    if (!env->tsc_khz) {
> +        return 0;
> +    }
> +
> +    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> +        kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) :
> +        -ENOTSUP;
> +    if (r < 0) {
> +        /* When KVM_SET_TSC_KHZ fails, it's an error only if the current
> +         * TSC frequency doesn't match the one we want.
> +         */
> +        int cur_freq = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> +                       kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> +                       -ENOTSUP;
> +        if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
> +            error_report("warning: TSC frequency mismatch between "
> +                         "VM and host, and TSC scaling unavailable");
> +            return r;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static Error *invtsc_mig_blocker;
>  
>  #define KVM_MAX_CPUID_ENTRIES  100
> @@ -823,13 +853,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          return r;
>      }
>  
> -    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL);
> -    if (r && env->tsc_khz) {
> -        r = kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz);
> -        if (r < 0) {
> -            fprintf(stderr, "KVM_SET_TSC_KHZ failed\n");
> -            return r;
> -        }
> +    r = kvm_arch_set_tsc_khz(cs);
> +    if (r < 0) {
> +        return r;
>      }
>  
>      /* vcpu's TSC frequency is either specified by user, or following
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 2/3] target-i386: reorganize TSC rate setting code

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:56AM +0800, Haozhong Zhang wrote:
> Following changes are made to the TSC rate setting code in
> kvm_arch_init_vcpu():
>  * The code is moved to a new function kvm_arch_set_tsc_khz().
>  * If kvm_arch_set_tsc_khz() fails, i.e. following two conditions are
>    both satisfied:
>    * KVM does not support the TSC scaling or it fails to set vcpu's
>      TSC rate by KVM_SET_TSC_KHZ,
>    * the TSC rate to be set is different than the value currently used
>      by KVM,
>    then kvm_arch_init_vcpu() will fail. Prevously,
>    * the lack of TSC scaling never failed kvm_arch_init_vcpu(),
>    * the failure of KVM_SET_TSC_KHZ failed kvm_arch_init_vcpu()
>      unconditionally, even though the TSC rate to be set is identical
>      to the value currently used by KVM.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target-i386/kvm.c | 40 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index a0fe9d4..1e811ee 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -524,6 +524,36 @@ static bool hyperv_enabled(X86CPU *cpu)
>              cpu->hyperv_runtime);
>  }
>  
> +static int kvm_arch_set_tsc_khz(CPUState *cs)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +    int r;
> +
> +    if (!env->tsc_khz) {
> +        return 0;
> +    }
> +
> +    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> +        kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) :
> +        -ENOTSUP;
> +    if (r < 0) {
> +        /* When KVM_SET_TSC_KHZ fails, it's an error only if the current
> +         * TSC frequency doesn't match the one we want.
> +         */
> +        int cur_freq = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> +                       kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ) :
> +                       -ENOTSUP;
> +        if (cur_freq <= 0 || cur_freq != env->tsc_khz) {
> +            error_report("warning: TSC frequency mismatch between "
> +                         "VM and host, and TSC scaling unavailable");
> +            return r;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static Error *invtsc_mig_blocker;
>  
>  #define KVM_MAX_CPUID_ENTRIES  100
> @@ -823,13 +853,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          return r;
>      }
>  
> -    r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL);
> -    if (r && env->tsc_khz) {
> -        r = kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz);
> -        if (r < 0) {
> -            fprintf(stderr, "KVM_SET_TSC_KHZ failed\n");
> -            return r;
> -        }
> +    r = kvm_arch_set_tsc_khz(cs);
> +    if (r < 0) {
> +        return r;
>      }
>  
>      /* vcpu's TSC frequency is either specified by user, or following
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:57AM +0800, Haozhong Zhang wrote:
> This patch enables migrating vcpu's TSC rate. If KVM on the destination
> machine supports TSC scaling, guest programs will observe a consistent
> TSC rate across the migration.
> 
> If TSC scaling is not supported on the destination machine, the
> migration will not be aborted and QEMU on the destination will not set
> vcpu's TSC rate to the migrated value.
> 
> If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
> machine is inconsistent with the migrated TSC rate, the migration will
> be aborted.
> 
> For backwards compatibility, the migration of vcpu's TSC rate is
> disabled on pc-*-2.4 and older machine types.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Assuming the PC compat code will be moved to
pc_*_2_5_machine_options(), because the patch will be included
after QEMU 2.5.0:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

One comment below:

> ---
[...]
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 1e811ee..2a0fd54 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2381,6 +2381,28 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
>          }
>      }
>  
> +    if (level == KVM_PUT_FULL_STATE) {
> +        /* kvm_arch_set_tsc_khz() below can be called in two control flows and
> +         * we don't need to handle its errors in both of them.
> +         *
> +         * One is the control flow that creates a vcpu, where
> +         * kvm_arch_set_tsc_khz() has already been called once before by
> +         * kvm_arch_init_vcpu(). The latter will abort the control flow if there
> +         * are any errors of kvm_arch_set_tsc_khz(). Thus, in this control flow,
> +         * kvm_arch_set_tsc_khz() below never fails and we can safely ignore its
> +         * return values here.
> +         *
> +         * Another is the control flow of migration that sets vcpu's TSC
> +         * frequency on the destination. The only error that can fail the
> +         * migration is the mismatch between the migrated and the user-specified
> +         * TSC frequencies, which has been handled by cpu_post_load(). Other
> +         * errors, i.e. those from kvm_arch_set_tsc_khz(), never fail the
> +         * migration, so we also safely ignore its return values in this control
> +         * flow.
> +         */

This could be more succint. Something like:

/* We don't check for kvm_arch_set_tsc_khz() errors here, because
 * TSC frequency mismatch shouldn't abort migration, unless the
 * user explicitly asked for a more strict TSC setting (e.g.
 * using an explicit "tsc-freq" option).
 */

No need to resubmit because of that, though. The comment can be
changed when applying the patch.

> +        kvm_arch_set_tsc_khz(cpu);
> +    }
> +
>      ret = kvm_getput_regs(x86_cpu, 1);
>      if (ret < 0) {
>          return ret;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index a18e16e..e560ca3 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -6,6 +6,8 @@
>  #include "cpu.h"
>  #include "sysemu/kvm.h"
>  
> +#include "qemu/error-report.h"
> +
>  static const VMStateDescription vmstate_segment = {
>      .name = "segment",
>      .version_id = 1,
> @@ -331,6 +333,13 @@ static int cpu_post_load(void *opaque, int version_id)
>      CPUX86State *env = &cpu->env;
>      int i;
>  
> +    if (env->tsc_khz && env->user_tsc_khz &&
> +        env->tsc_khz != env->user_tsc_khz) {
> +        error_report("Mismatch between user-specified TSC frequency and "
> +                     "migrated TSC frequency");
> +        return -EINVAL;
> +    }
> +
>      /*
>       * Real mode guest segments register DPL should be zero.
>       * Older KVM version were setting it wrongly.
> @@ -775,6 +784,26 @@ static const VMStateDescription vmstate_xss = {
>      }
>  };
>  
> +static bool tsc_khz_needed(void *opaque)
> +{
> +    X86CPU *cpu = opaque;
> +    CPUX86State *env = &cpu->env;
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
> +    return env->tsc_khz && pcmc->save_tsc_khz;
> +}
> +
> +static const VMStateDescription vmstate_tsc_khz = {
> +    .name = "cpu/tsc_khz",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = tsc_khz_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT64(env.tsc_khz, X86CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  VMStateDescription vmstate_x86_cpu = {
>      .name = "cpu",
>      .version_id = 12,
> @@ -895,6 +924,7 @@ VMStateDescription vmstate_x86_cpu = {
>          &vmstate_msr_hyperv_runtime,
>          &vmstate_avx512,
>          &vmstate_xss,
> +        &vmstate_tsc_khz,
>          NULL
>      }
>  };
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Eduardo Habkost]

On Tue, Nov 24, 2015 at 11:33:57AM +0800, Haozhong Zhang wrote:
> This patch enables migrating vcpu's TSC rate. If KVM on the destination
> machine supports TSC scaling, guest programs will observe a consistent
> TSC rate across the migration.
> 
> If TSC scaling is not supported on the destination machine, the
> migration will not be aborted and QEMU on the destination will not set
> vcpu's TSC rate to the migrated value.
> 
> If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
> machine is inconsistent with the migrated TSC rate, the migration will
> be aborted.
> 
> For backwards compatibility, the migration of vcpu's TSC rate is
> disabled on pc-*-2.4 and older machine types.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Assuming the PC compat code will be moved to
pc_*_2_5_machine_options(), because the patch will be included
after QEMU 2.5.0:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

One comment below:

> ---
[...]
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 1e811ee..2a0fd54 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2381,6 +2381,28 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
>          }
>      }
>  
> +    if (level == KVM_PUT_FULL_STATE) {
> +        /* kvm_arch_set_tsc_khz() below can be called in two control flows and
> +         * we don't need to handle its errors in both of them.
> +         *
> +         * One is the control flow that creates a vcpu, where
> +         * kvm_arch_set_tsc_khz() has already been called once before by
> +         * kvm_arch_init_vcpu(). The latter will abort the control flow if there
> +         * are any errors of kvm_arch_set_tsc_khz(). Thus, in this control flow,
> +         * kvm_arch_set_tsc_khz() below never fails and we can safely ignore its
> +         * return values here.
> +         *
> +         * Another is the control flow of migration that sets vcpu's TSC
> +         * frequency on the destination. The only error that can fail the
> +         * migration is the mismatch between the migrated and the user-specified
> +         * TSC frequencies, which has been handled by cpu_post_load(). Other
> +         * errors, i.e. those from kvm_arch_set_tsc_khz(), never fail the
> +         * migration, so we also safely ignore its return values in this control
> +         * flow.
> +         */

This could be more succint. Something like:

/* We don't check for kvm_arch_set_tsc_khz() errors here, because
 * TSC frequency mismatch shouldn't abort migration, unless the
 * user explicitly asked for a more strict TSC setting (e.g.
 * using an explicit "tsc-freq" option).
 */

No need to resubmit because of that, though. The comment can be
changed when applying the patch.

> +        kvm_arch_set_tsc_khz(cpu);
> +    }
> +
>      ret = kvm_getput_regs(x86_cpu, 1);
>      if (ret < 0) {
>          return ret;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index a18e16e..e560ca3 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -6,6 +6,8 @@
>  #include "cpu.h"
>  #include "sysemu/kvm.h"
>  
> +#include "qemu/error-report.h"
> +
>  static const VMStateDescription vmstate_segment = {
>      .name = "segment",
>      .version_id = 1,
> @@ -331,6 +333,13 @@ static int cpu_post_load(void *opaque, int version_id)
>      CPUX86State *env = &cpu->env;
>      int i;
>  
> +    if (env->tsc_khz && env->user_tsc_khz &&
> +        env->tsc_khz != env->user_tsc_khz) {
> +        error_report("Mismatch between user-specified TSC frequency and "
> +                     "migrated TSC frequency");
> +        return -EINVAL;
> +    }
> +
>      /*
>       * Real mode guest segments register DPL should be zero.
>       * Older KVM version were setting it wrongly.
> @@ -775,6 +784,26 @@ static const VMStateDescription vmstate_xss = {
>      }
>  };
>  
> +static bool tsc_khz_needed(void *opaque)
> +{
> +    X86CPU *cpu = opaque;
> +    CPUX86State *env = &cpu->env;
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
> +    return env->tsc_khz && pcmc->save_tsc_khz;
> +}
> +
> +static const VMStateDescription vmstate_tsc_khz = {
> +    .name = "cpu/tsc_khz",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = tsc_khz_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT64(env.tsc_khz, X86CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  VMStateDescription vmstate_x86_cpu = {
>      .name = "cpu",
>      .version_id = 12,
> @@ -895,6 +924,7 @@ VMStateDescription vmstate_x86_cpu = {
>          &vmstate_msr_hyperv_runtime,
>          &vmstate_avx512,
>          &vmstate_xss,
> +        &vmstate_tsc_khz,
>          NULL
>      }
>  };
> -- 
> 2.4.8
> 

-- 
Eduardo

Re: [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Haozhong Zhang]

On 11/26/15 12:19, Eduardo Habkost wrote:
> On Tue, Nov 24, 2015 at 11:33:57AM +0800, Haozhong Zhang wrote:
> > This patch enables migrating vcpu's TSC rate. If KVM on the destination
> > machine supports TSC scaling, guest programs will observe a consistent
> > TSC rate across the migration.
> > 
> > If TSC scaling is not supported on the destination machine, the
> > migration will not be aborted and QEMU on the destination will not set
> > vcpu's TSC rate to the migrated value.
> > 
> > If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
> > machine is inconsistent with the migrated TSC rate, the migration will
> > be aborted.
> > 
> > For backwards compatibility, the migration of vcpu's TSC rate is
> > disabled on pc-*-2.4 and older machine types.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Assuming the PC compat code will be moved to
> pc_*_2_5_machine_options(), because the patch will be included
> after QEMU 2.5.0:
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> One comment below:

Hi Eduardo,

Thank you for reviewing!

Besides the comment, should I submit a new version which updates the
compat code after pc-*-2.6 machine types are added?

Haozhong

> 
> > ---
> [...]
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 1e811ee..2a0fd54 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2381,6 +2381,28 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
> >          }
> >      }
> >  
> > +    if (level == KVM_PUT_FULL_STATE) {
> > +        /* kvm_arch_set_tsc_khz() below can be called in two control flows and
> > +         * we don't need to handle its errors in both of them.
> > +         *
> > +         * One is the control flow that creates a vcpu, where
> > +         * kvm_arch_set_tsc_khz() has already been called once before by
> > +         * kvm_arch_init_vcpu(). The latter will abort the control flow if there
> > +         * are any errors of kvm_arch_set_tsc_khz(). Thus, in this control flow,
> > +         * kvm_arch_set_tsc_khz() below never fails and we can safely ignore its
> > +         * return values here.
> > +         *
> > +         * Another is the control flow of migration that sets vcpu's TSC
> > +         * frequency on the destination. The only error that can fail the
> > +         * migration is the mismatch between the migrated and the user-specified
> > +         * TSC frequencies, which has been handled by cpu_post_load(). Other
> > +         * errors, i.e. those from kvm_arch_set_tsc_khz(), never fail the
> > +         * migration, so we also safely ignore its return values in this control
> > +         * flow.
> > +         */
> 
> This could be more succint. Something like:
> 
> /* We don't check for kvm_arch_set_tsc_khz() errors here, because
>  * TSC frequency mismatch shouldn't abort migration, unless the
>  * user explicitly asked for a more strict TSC setting (e.g.
>  * using an explicit "tsc-freq" option).
>  */
> 
> No need to resubmit because of that, though. The comment can be
> changed when applying the patch.
> 
> > +        kvm_arch_set_tsc_khz(cpu);
> > +    }
> > +
> >      ret = kvm_getput_regs(x86_cpu, 1);
> >      if (ret < 0) {
> >          return ret;
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index a18e16e..e560ca3 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -6,6 +6,8 @@
> >  #include "cpu.h"
> >  #include "sysemu/kvm.h"
> >  
> > +#include "qemu/error-report.h"
> > +
> >  static const VMStateDescription vmstate_segment = {
> >      .name = "segment",
> >      .version_id = 1,
> > @@ -331,6 +333,13 @@ static int cpu_post_load(void *opaque, int version_id)
> >      CPUX86State *env = &cpu->env;
> >      int i;
> >  
> > +    if (env->tsc_khz && env->user_tsc_khz &&
> > +        env->tsc_khz != env->user_tsc_khz) {
> > +        error_report("Mismatch between user-specified TSC frequency and "
> > +                     "migrated TSC frequency");
> > +        return -EINVAL;
> > +    }
> > +
> >      /*
> >       * Real mode guest segments register DPL should be zero.
> >       * Older KVM version were setting it wrongly.
> > @@ -775,6 +784,26 @@ static const VMStateDescription vmstate_xss = {
> >      }
> >  };
> >  
> > +static bool tsc_khz_needed(void *opaque)
> > +{
> > +    X86CPU *cpu = opaque;
> > +    CPUX86State *env = &cpu->env;
> > +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> > +    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
> > +    return env->tsc_khz && pcmc->save_tsc_khz;
> > +}
> > +
> > +static const VMStateDescription vmstate_tsc_khz = {
> > +    .name = "cpu/tsc_khz",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = tsc_khz_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_INT64(env.tsc_khz, X86CPU),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >  VMStateDescription vmstate_x86_cpu = {
> >      .name = "cpu",
> >      .version_id = 12,
> > @@ -895,6 +924,7 @@ VMStateDescription vmstate_x86_cpu = {
> >          &vmstate_msr_hyperv_runtime,
> >          &vmstate_avx512,
> >          &vmstate_xss,
> > +        &vmstate_tsc_khz,
> >          NULL
> >      }
> >  };
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Haozhong Zhang]

On 11/26/15 12:19, Eduardo Habkost wrote:
> On Tue, Nov 24, 2015 at 11:33:57AM +0800, Haozhong Zhang wrote:
> > This patch enables migrating vcpu's TSC rate. If KVM on the destination
> > machine supports TSC scaling, guest programs will observe a consistent
> > TSC rate across the migration.
> > 
> > If TSC scaling is not supported on the destination machine, the
> > migration will not be aborted and QEMU on the destination will not set
> > vcpu's TSC rate to the migrated value.
> > 
> > If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
> > machine is inconsistent with the migrated TSC rate, the migration will
> > be aborted.
> > 
> > For backwards compatibility, the migration of vcpu's TSC rate is
> > disabled on pc-*-2.4 and older machine types.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Assuming the PC compat code will be moved to
> pc_*_2_5_machine_options(), because the patch will be included
> after QEMU 2.5.0:
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> One comment below:

Hi Eduardo,

Thank you for reviewing!

Besides the comment, should I submit a new version which updates the
compat code after pc-*-2.6 machine types are added?

Haozhong

> 
> > ---
> [...]
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 1e811ee..2a0fd54 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2381,6 +2381,28 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
> >          }
> >      }
> >  
> > +    if (level == KVM_PUT_FULL_STATE) {
> > +        /* kvm_arch_set_tsc_khz() below can be called in two control flows and
> > +         * we don't need to handle its errors in both of them.
> > +         *
> > +         * One is the control flow that creates a vcpu, where
> > +         * kvm_arch_set_tsc_khz() has already been called once before by
> > +         * kvm_arch_init_vcpu(). The latter will abort the control flow if there
> > +         * are any errors of kvm_arch_set_tsc_khz(). Thus, in this control flow,
> > +         * kvm_arch_set_tsc_khz() below never fails and we can safely ignore its
> > +         * return values here.
> > +         *
> > +         * Another is the control flow of migration that sets vcpu's TSC
> > +         * frequency on the destination. The only error that can fail the
> > +         * migration is the mismatch between the migrated and the user-specified
> > +         * TSC frequencies, which has been handled by cpu_post_load(). Other
> > +         * errors, i.e. those from kvm_arch_set_tsc_khz(), never fail the
> > +         * migration, so we also safely ignore its return values in this control
> > +         * flow.
> > +         */
> 
> This could be more succint. Something like:
> 
> /* We don't check for kvm_arch_set_tsc_khz() errors here, because
>  * TSC frequency mismatch shouldn't abort migration, unless the
>  * user explicitly asked for a more strict TSC setting (e.g.
>  * using an explicit "tsc-freq" option).
>  */
> 
> No need to resubmit because of that, though. The comment can be
> changed when applying the patch.
> 
> > +        kvm_arch_set_tsc_khz(cpu);
> > +    }
> > +
> >      ret = kvm_getput_regs(x86_cpu, 1);
> >      if (ret < 0) {
> >          return ret;
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index a18e16e..e560ca3 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -6,6 +6,8 @@
> >  #include "cpu.h"
> >  #include "sysemu/kvm.h"
> >  
> > +#include "qemu/error-report.h"
> > +
> >  static const VMStateDescription vmstate_segment = {
> >      .name = "segment",
> >      .version_id = 1,
> > @@ -331,6 +333,13 @@ static int cpu_post_load(void *opaque, int version_id)
> >      CPUX86State *env = &cpu->env;
> >      int i;
> >  
> > +    if (env->tsc_khz && env->user_tsc_khz &&
> > +        env->tsc_khz != env->user_tsc_khz) {
> > +        error_report("Mismatch between user-specified TSC frequency and "
> > +                     "migrated TSC frequency");
> > +        return -EINVAL;
> > +    }
> > +
> >      /*
> >       * Real mode guest segments register DPL should be zero.
> >       * Older KVM version were setting it wrongly.
> > @@ -775,6 +784,26 @@ static const VMStateDescription vmstate_xss = {
> >      }
> >  };
> >  
> > +static bool tsc_khz_needed(void *opaque)
> > +{
> > +    X86CPU *cpu = opaque;
> > +    CPUX86State *env = &cpu->env;
> > +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> > +    PCMachineClass *pcmc = PC_MACHINE_CLASS(mc);
> > +    return env->tsc_khz && pcmc->save_tsc_khz;
> > +}
> > +
> > +static const VMStateDescription vmstate_tsc_khz = {
> > +    .name = "cpu/tsc_khz",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = tsc_khz_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_INT64(env.tsc_khz, X86CPU),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >  VMStateDescription vmstate_x86_cpu = {
> >      .name = "cpu",
> >      .version_id = 12,
> > @@ -895,6 +924,7 @@ VMStateDescription vmstate_x86_cpu = {
> >          &vmstate_msr_hyperv_runtime,
> >          &vmstate_avx512,
> >          &vmstate_xss,
> > +        &vmstate_tsc_khz,
> >          NULL
> >      }
> >  };
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo

Re: [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Eduardo Habkost]

On Fri, Nov 27, 2015 at 08:16:42AM +0800, Haozhong Zhang wrote:
> On 11/26/15 12:19, Eduardo Habkost wrote:
> > On Tue, Nov 24, 2015 at 11:33:57AM +0800, Haozhong Zhang wrote:
> > > This patch enables migrating vcpu's TSC rate. If KVM on the destination
> > > machine supports TSC scaling, guest programs will observe a consistent
> > > TSC rate across the migration.
> > > 
> > > If TSC scaling is not supported on the destination machine, the
> > > migration will not be aborted and QEMU on the destination will not set
> > > vcpu's TSC rate to the migrated value.
> > > 
> > > If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
> > > machine is inconsistent with the migrated TSC rate, the migration will
> > > be aborted.
> > > 
> > > For backwards compatibility, the migration of vcpu's TSC rate is
> > > disabled on pc-*-2.4 and older machine types.
> > > 
> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > 
> > Assuming the PC compat code will be moved to
> > pc_*_2_5_machine_options(), because the patch will be included
> > after QEMU 2.5.0:
> >
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > 
> > One comment below:
> 
> Hi Eduardo,
> 
> Thank you for reviewing!
> 
> Besides the comment, should I submit a new version which updates the
> compat code after pc-*-2.6 machine types are added?

There's no need to resubmit. I have queued the patches for 2.6,
in the branch at:

  git://github.com/ehabkost/qemu.git x86-next

The pc-2.6 series is in that queue because it is a dependency.
But I plan to rebase and submit a pull request containing only
the x86-specific patches, after the pc-2.6 series is merged
through Michael's tree.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v6 3/3] target-i386: add support to migrate vcpu's TSC rate

[Eduardo Habkost]

On Fri, Nov 27, 2015 at 08:16:42AM +0800, Haozhong Zhang wrote:
> On 11/26/15 12:19, Eduardo Habkost wrote:
> > On Tue, Nov 24, 2015 at 11:33:57AM +0800, Haozhong Zhang wrote:
> > > This patch enables migrating vcpu's TSC rate. If KVM on the destination
> > > machine supports TSC scaling, guest programs will observe a consistent
> > > TSC rate across the migration.
> > > 
> > > If TSC scaling is not supported on the destination machine, the
> > > migration will not be aborted and QEMU on the destination will not set
> > > vcpu's TSC rate to the migrated value.
> > > 
> > > If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination
> > > machine is inconsistent with the migrated TSC rate, the migration will
> > > be aborted.
> > > 
> > > For backwards compatibility, the migration of vcpu's TSC rate is
> > > disabled on pc-*-2.4 and older machine types.
> > > 
> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > 
> > Assuming the PC compat code will be moved to
> > pc_*_2_5_machine_options(), because the patch will be included
> > after QEMU 2.5.0:
> >
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > 
> > One comment below:
> 
> Hi Eduardo,
> 
> Thank you for reviewing!
> 
> Besides the comment, should I submit a new version which updates the
> compat code after pc-*-2.6 machine types are added?

There's no need to resubmit. I have queued the patches for 2.6,
in the branch at:

  git://github.com/ehabkost/qemu.git x86-next

The pc-2.6 series is in that queue because it is a dependency.
But I plan to rebase and submit a pull request containing only
the x86-specific patches, after the pc-2.6 series is merged
through Michael's tree.

-- 
Eduardo