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

[PATCH v4 0/2] 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 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 (2):
  target-i386: fallback vcpu's TSC rate to value returned by KVM
  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     | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/machine.c | 28 ++++++++++++++++++++++++++++
 8 files changed, 81 insertions(+), 1 deletion(-)

-- 
2.6.3

[Qemu-devel] [PATCH v4 0/2] 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 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 (2):
  target-i386: fallback vcpu's TSC rate to value returned by KVM
  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     | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/machine.c | 28 ++++++++++++++++++++++++++++
 8 files changed, 81 insertions(+), 1 deletion(-)

-- 
2.6.3

[PATCH v4 1/2] 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.

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

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2a9953b..9084b29 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
     return 0;
 }
 
+static void kvm_arch_set_tsc_khz(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    int r;
+
+    /*
+     * If no user-specified TSC frequency is present, we will try to
+     * set env->tsc_khz to the value used by KVM.
+     */
+    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) {
+            error_report("warning: KVM_GET_TSC_KHZ failed");
+        } else {
+            env->tsc_khz = r;
+        }
+    }
+}
+
 int kvm_arch_put_registers(CPUState *cpu, int level)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
@@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
         }
     }
 
+    if (level == KVM_PUT_FULL_STATE) {
+        kvm_arch_set_tsc_khz(cpu);
+    }
+
     ret = kvm_getput_regs(x86_cpu, 1);
     if (ret < 0) {
         return ret;
-- 
2.4.8

[Qemu-devel] [PATCH v4 1/2] 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.

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

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2a9953b..9084b29 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
     return 0;
 }
 
+static void kvm_arch_set_tsc_khz(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    int r;
+
+    /*
+     * If no user-specified TSC frequency is present, we will try to
+     * set env->tsc_khz to the value used by KVM.
+     */
+    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) {
+            error_report("warning: KVM_GET_TSC_KHZ failed");
+        } else {
+            env->tsc_khz = r;
+        }
+    }
+}
+
 int kvm_arch_put_registers(CPUState *cpu, int level)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
@@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
         }
     }
 
+    if (level == KVM_PUT_FULL_STATE) {
+        kvm_arch_set_tsc_khz(cpu);
+    }
+
     ret = kvm_getput_regs(x86_cpu, 1);
     if (ret < 0) {
         return ret;
-- 
2.4.8

[PATCH v4 2/2] 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     | 26 ++++++++++++++++++++++++--
 target-i386/machine.c | 28 ++++++++++++++++++++++++++++
 8 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0cb8afd..2f2fc93 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 4bbc0ff..fea0f28 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 e5f1c5b..98c6a4c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1724,7 +1724,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..1ad1da8 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;
     void *kvm_xsave_buf;
 
     uint64_t mcg_cap;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9084b29..8448248 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
     int r;
 
     /*
-     * If no user-specified TSC frequency is present, we will try to
-     * set env->tsc_khz to the value used by KVM.
+     * For other cases of env->tsc_khz and env->user_tsc_khz:
+     *
+     * - We have eliminated all cases that satisfy
+     *       env->tsc_khz && env->user_tsc_khz &&
+     *       env->tsc_khz != env->user_tsc_khz
+     *   in cpu_post_load().
+     *
+     * - If env->user_tsc_khz is not zero, then it must be equal to
+     *   env->tsc_khz (if the latter is not zero) and has been set in
+     *   kvm_arch_init_vcpu().
+     */
+    if (env->tsc_khz && !env->user_tsc_khz) {
+        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) {
+            error_report("warning: TSC frequency mismatch between VM and host, "
+                         "and TSC scaling unavailable");
+        }
+    }
+
+    /*
+     * If neither the user-specified TSC frequency nor the migrated
+     * TSC frequency is present, we will try to set env->tsc_khz
+     * to the value used by KVM.
      */
     if (!env->tsc_khz) {
         r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a18e16e..eb062d2 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -331,6 +331,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) {
+        fprintf(stderr, "Mismatch between user-specified TSC frequency and "
+                "migrated TSC frequency");
+        return -1;
+    }
+
     /*
      * Real mode guest segments register DPL should be zero.
      * Older KVM version were setting it wrongly.
@@ -775,6 +782,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 +922,7 @@ VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_hyperv_runtime,
         &vmstate_avx512,
         &vmstate_xss,
+        &vmstate_tsc_khz,
         NULL
     }
 };
-- 
2.4.8

[Qemu-devel] [PATCH v4 2/2] 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     | 26 ++++++++++++++++++++++++--
 target-i386/machine.c | 28 ++++++++++++++++++++++++++++
 8 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0cb8afd..2f2fc93 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 4bbc0ff..fea0f28 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 e5f1c5b..98c6a4c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1724,7 +1724,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..1ad1da8 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;
     void *kvm_xsave_buf;
 
     uint64_t mcg_cap;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9084b29..8448248 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
     int r;
 
     /*
-     * If no user-specified TSC frequency is present, we will try to
-     * set env->tsc_khz to the value used by KVM.
+     * For other cases of env->tsc_khz and env->user_tsc_khz:
+     *
+     * - We have eliminated all cases that satisfy
+     *       env->tsc_khz && env->user_tsc_khz &&
+     *       env->tsc_khz != env->user_tsc_khz
+     *   in cpu_post_load().
+     *
+     * - If env->user_tsc_khz is not zero, then it must be equal to
+     *   env->tsc_khz (if the latter is not zero) and has been set in
+     *   kvm_arch_init_vcpu().
+     */
+    if (env->tsc_khz && !env->user_tsc_khz) {
+        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) {
+            error_report("warning: TSC frequency mismatch between VM and host, "
+                         "and TSC scaling unavailable");
+        }
+    }
+
+    /*
+     * If neither the user-specified TSC frequency nor the migrated
+     * TSC frequency is present, we will try to set env->tsc_khz
+     * to the value used by KVM.
      */
     if (!env->tsc_khz) {
         r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a18e16e..eb062d2 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -331,6 +331,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) {
+        fprintf(stderr, "Mismatch between user-specified TSC frequency and "
+                "migrated TSC frequency");
+        return -1;
+    }
+
     /*
      * Real mode guest segments register DPL should be zero.
      * Older KVM version were setting it wrongly.
@@ -775,6 +782,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 +922,7 @@ VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_hyperv_runtime,
         &vmstate_avx512,
         &vmstate_xss,
+        &vmstate_tsc_khz,
         NULL
     }
 };
-- 
2.4.8

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

[Eduardo Habkost]

On Mon, Nov 16, 2015 at 04:04:07PM +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.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  target-i386/kvm.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 2a9953b..9084b29 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
>      return 0;
>  }
>  
> +static void kvm_arch_set_tsc_khz(CPUState *cs)

> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +    int r;
> +
> +    /*
> +     * If no user-specified TSC frequency is present, we will try to
> +     * set env->tsc_khz to the value used by KVM.
> +     */
> +    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;

Can't we do this on kvm_arch_init_vcpu()? kvm_arch_put_registers()'s purpose is
to just copy data from QEMU to KVM, not the other way around.


> +        if (r < 0) {
> +            error_report("warning: KVM_GET_TSC_KHZ failed");

Having a kernel that doesn't support KVM_CAP_GET_TSC_KHZ shouldn't trigger a
warning every time we run QEMU, unless the user is explicitly asking for a
feature that requires KVM_GET_TSC_KHZ.

> +        } else {
> +            env->tsc_khz = r;
> +        }
> +    }
> +}
> +
>  int kvm_arch_put_registers(CPUState *cpu, int level)
>  {
>      X86CPU *x86_cpu = X86_CPU(cpu);
> @@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
>          }
>      }
>  
> +    if (level == KVM_PUT_FULL_STATE) {
> +        kvm_arch_set_tsc_khz(cpu);
> +    }

I see that kvm_arch_set_tsc_khz() will be extended to call
KVM_SET_TSC_KHZ in the next patch, so the kvm_arch_set_tsc_khz()
seems to belong here. But the KVM_GET_TSC_KHZ call doesn't seem
to belong in kvm_arch_set_tsc_khz()

kvm_arch_put_registers() callers don't expect any QEMU-side data
to change, but just that KVM data is updated according to the
QEMU-side data.

> +
>      ret = kvm_getput_regs(x86_cpu, 1);
>      if (ret < 0) {
>          return ret;
> -- 
> 2.4.8
> 

-- 
Eduardo

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

[Eduardo Habkost]

On Mon, Nov 16, 2015 at 04:04:07PM +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.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  target-i386/kvm.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 2a9953b..9084b29 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
>      return 0;
>  }
>  
> +static void kvm_arch_set_tsc_khz(CPUState *cs)

> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +    int r;
> +
> +    /*
> +     * If no user-specified TSC frequency is present, we will try to
> +     * set env->tsc_khz to the value used by KVM.
> +     */
> +    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;

Can't we do this on kvm_arch_init_vcpu()? kvm_arch_put_registers()'s purpose is
to just copy data from QEMU to KVM, not the other way around.


> +        if (r < 0) {
> +            error_report("warning: KVM_GET_TSC_KHZ failed");

Having a kernel that doesn't support KVM_CAP_GET_TSC_KHZ shouldn't trigger a
warning every time we run QEMU, unless the user is explicitly asking for a
feature that requires KVM_GET_TSC_KHZ.

> +        } else {
> +            env->tsc_khz = r;
> +        }
> +    }
> +}
> +
>  int kvm_arch_put_registers(CPUState *cpu, int level)
>  {
>      X86CPU *x86_cpu = X86_CPU(cpu);
> @@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
>          }
>      }
>  
> +    if (level == KVM_PUT_FULL_STATE) {
> +        kvm_arch_set_tsc_khz(cpu);
> +    }

I see that kvm_arch_set_tsc_khz() will be extended to call
KVM_SET_TSC_KHZ in the next patch, so the kvm_arch_set_tsc_khz()
seems to belong here. But the KVM_GET_TSC_KHZ call doesn't seem
to belong in kvm_arch_set_tsc_khz()

kvm_arch_put_registers() callers don't expect any QEMU-side data
to change, but just that KVM data is updated according to the
QEMU-side data.

> +
>      ret = kvm_getput_regs(x86_cpu, 1);
>      if (ret < 0) {
>          return ret;
> -- 
> 2.4.8
> 

-- 
Eduardo

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

[Eduardo Habkost]

On Mon, Nov 16, 2015 at 04:04:08PM +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>
> ---
>  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     | 26 ++++++++++++++++++++++++--
>  target-i386/machine.c | 28 ++++++++++++++++++++++++++++
>  8 files changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0cb8afd..2f2fc93 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 4bbc0ff..fea0f28 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 e5f1c5b..98c6a4c 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1724,7 +1724,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..1ad1da8 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;
>      void *kvm_xsave_buf;
>  
>      uint64_t mcg_cap;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 9084b29..8448248 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
>      int r;
>  
>      /*
> -     * If no user-specified TSC frequency is present, we will try to
> -     * set env->tsc_khz to the value used by KVM.
> +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> +     *
> +     * - We have eliminated all cases that satisfy
> +     *       env->tsc_khz && env->user_tsc_khz &&
> +     *       env->tsc_khz != env->user_tsc_khz
> +     *   in cpu_post_load().
> +     *
> +     * - If env->user_tsc_khz is not zero, then it must be equal to
> +     *   env->tsc_khz (if the latter is not zero) and has been set in
> +     *   kvm_arch_init_vcpu().
> +     */
> +    if (env->tsc_khz && !env->user_tsc_khz) {
> +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;

Please don't duplicate the code from kvm_arch_init_vcpu(). We can
handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
can't we?

> +        if (r < 0) {
> +            error_report("warning: TSC frequency mismatch between VM and host, "
> +                         "and TSC scaling unavailable");
> +        }
> +    }
> +
> +    /*
> +     * If neither the user-specified TSC frequency nor the migrated
> +     * TSC frequency is present, we will try to set env->tsc_khz
> +     * to the value used by KVM.
>       */
>      if (!env->tsc_khz) {
>          r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index a18e16e..eb062d2 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -331,6 +331,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) {
> +        fprintf(stderr, "Mismatch between user-specified TSC frequency and "
> +                "migrated TSC frequency");
> +        return -1;
> +    }
> +
>      /*
>       * Real mode guest segments register DPL should be zero.
>       * Older KVM version were setting it wrongly.
> @@ -775,6 +782,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 +922,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 v4 2/2] target-i386: add support to migrate vcpu's TSC rate

[Eduardo Habkost]

On Mon, Nov 16, 2015 at 04:04:08PM +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>
> ---
>  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     | 26 ++++++++++++++++++++++++--
>  target-i386/machine.c | 28 ++++++++++++++++++++++++++++
>  8 files changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0cb8afd..2f2fc93 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 4bbc0ff..fea0f28 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 e5f1c5b..98c6a4c 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1724,7 +1724,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..1ad1da8 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;
>      void *kvm_xsave_buf;
>  
>      uint64_t mcg_cap;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 9084b29..8448248 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
>      int r;
>  
>      /*
> -     * If no user-specified TSC frequency is present, we will try to
> -     * set env->tsc_khz to the value used by KVM.
> +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> +     *
> +     * - We have eliminated all cases that satisfy
> +     *       env->tsc_khz && env->user_tsc_khz &&
> +     *       env->tsc_khz != env->user_tsc_khz
> +     *   in cpu_post_load().
> +     *
> +     * - If env->user_tsc_khz is not zero, then it must be equal to
> +     *   env->tsc_khz (if the latter is not zero) and has been set in
> +     *   kvm_arch_init_vcpu().
> +     */
> +    if (env->tsc_khz && !env->user_tsc_khz) {
> +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;

Please don't duplicate the code from kvm_arch_init_vcpu(). We can
handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
can't we?

> +        if (r < 0) {
> +            error_report("warning: TSC frequency mismatch between VM and host, "
> +                         "and TSC scaling unavailable");
> +        }
> +    }
> +
> +    /*
> +     * If neither the user-specified TSC frequency nor the migrated
> +     * TSC frequency is present, we will try to set env->tsc_khz
> +     * to the value used by KVM.
>       */
>      if (!env->tsc_khz) {
>          r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index a18e16e..eb062d2 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -331,6 +331,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) {
> +        fprintf(stderr, "Mismatch between user-specified TSC frequency and "
> +                "migrated TSC frequency");
> +        return -1;
> +    }
> +
>      /*
>       * Real mode guest segments register DPL should be zero.
>       * Older KVM version were setting it wrongly.
> @@ -775,6 +782,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 +922,7 @@ VMStateDescription vmstate_x86_cpu = {
>          &vmstate_msr_hyperv_runtime,
>          &vmstate_avx512,
>          &vmstate_xss,
> +        &vmstate_tsc_khz,
>          NULL
>      }
>  };
> -- 
> 2.4.8
> 

-- 
Eduardo

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

[Haozhong Zhang]

On 11/16/15 11:43, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 04:04:08PM +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>
> > ---
> >  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     | 26 ++++++++++++++++++++++++--
> >  target-i386/machine.c | 28 ++++++++++++++++++++++++++++
> >  8 files changed, 58 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 0cb8afd..2f2fc93 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 4bbc0ff..fea0f28 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 e5f1c5b..98c6a4c 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1724,7 +1724,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..1ad1da8 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;
> >      void *kvm_xsave_buf;
> >  
> >      uint64_t mcg_cap;
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 9084b29..8448248 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
> >      int r;
> >  
> >      /*
> > -     * If no user-specified TSC frequency is present, we will try to
> > -     * set env->tsc_khz to the value used by KVM.
> > +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> > +     *
> > +     * - We have eliminated all cases that satisfy
> > +     *       env->tsc_khz && env->user_tsc_khz &&
> > +     *       env->tsc_khz != env->user_tsc_khz
> > +     *   in cpu_post_load().
> > +     *
> > +     * - If env->user_tsc_khz is not zero, then it must be equal to
> > +     *   env->tsc_khz (if the latter is not zero) and has been set in
> > +     *   kvm_arch_init_vcpu().
> > +     */
> > +    if (env->tsc_khz && !env->user_tsc_khz) {
> > +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> > +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;
> 
> Please don't duplicate the code from kvm_arch_init_vcpu(). We can
> handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
> can't we?
>

No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will
exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not
abort. And because the return value of
kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its
caller do_kvm_cpu_synchronize_post_init(), I have to handle them in
different ways.

Haozhong

> > +        if (r < 0) {
> > +            error_report("warning: TSC frequency mismatch between VM and host, "
> > +                         "and TSC scaling unavailable");
> > +        }
> > +    }
> > +
> > +    /*
> > +     * If neither the user-specified TSC frequency nor the migrated
> > +     * TSC frequency is present, we will try to set env->tsc_khz
> > +     * to the value used by KVM.
> >       */
> >      if (!env->tsc_khz) {
> >          r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index a18e16e..eb062d2 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -331,6 +331,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) {
> > +        fprintf(stderr, "Mismatch between user-specified TSC frequency and "
> > +                "migrated TSC frequency");
> > +        return -1;
> > +    }
> > +
> >      /*
> >       * Real mode guest segments register DPL should be zero.
> >       * Older KVM version were setting it wrongly.
> > @@ -775,6 +782,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 +922,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 v4 2/2] target-i386: add support to migrate vcpu's TSC rate

[Haozhong Zhang]

On 11/16/15 11:43, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 04:04:08PM +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>
> > ---
> >  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     | 26 ++++++++++++++++++++++++--
> >  target-i386/machine.c | 28 ++++++++++++++++++++++++++++
> >  8 files changed, 58 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 0cb8afd..2f2fc93 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 4bbc0ff..fea0f28 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 e5f1c5b..98c6a4c 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1724,7 +1724,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..1ad1da8 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;
> >      void *kvm_xsave_buf;
> >  
> >      uint64_t mcg_cap;
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 9084b29..8448248 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
> >      int r;
> >  
> >      /*
> > -     * If no user-specified TSC frequency is present, we will try to
> > -     * set env->tsc_khz to the value used by KVM.
> > +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> > +     *
> > +     * - We have eliminated all cases that satisfy
> > +     *       env->tsc_khz && env->user_tsc_khz &&
> > +     *       env->tsc_khz != env->user_tsc_khz
> > +     *   in cpu_post_load().
> > +     *
> > +     * - If env->user_tsc_khz is not zero, then it must be equal to
> > +     *   env->tsc_khz (if the latter is not zero) and has been set in
> > +     *   kvm_arch_init_vcpu().
> > +     */
> > +    if (env->tsc_khz && !env->user_tsc_khz) {
> > +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> > +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;
> 
> Please don't duplicate the code from kvm_arch_init_vcpu(). We can
> handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
> can't we?
>

No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will
exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not
abort. And because the return value of
kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its
caller do_kvm_cpu_synchronize_post_init(), I have to handle them in
different ways.

Haozhong

> > +        if (r < 0) {
> > +            error_report("warning: TSC frequency mismatch between VM and host, "
> > +                         "and TSC scaling unavailable");
> > +        }
> > +    }
> > +
> > +    /*
> > +     * If neither the user-specified TSC frequency nor the migrated
> > +     * TSC frequency is present, we will try to set env->tsc_khz
> > +     * to the value used by KVM.
> >       */
> >      if (!env->tsc_khz) {
> >          r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ?
> > diff --git a/target-i386/machine.c b/target-i386/machine.c
> > index a18e16e..eb062d2 100644
> > --- a/target-i386/machine.c
> > +++ b/target-i386/machine.c
> > @@ -331,6 +331,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) {
> > +        fprintf(stderr, "Mismatch between user-specified TSC frequency and "
> > +                "migrated TSC frequency");
> > +        return -1;
> > +    }
> > +
> >      /*
> >       * Real mode guest segments register DPL should be zero.
> >       * Older KVM version were setting it wrongly.
> > @@ -775,6 +782,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 +922,7 @@ VMStateDescription vmstate_x86_cpu = {
> >          &vmstate_msr_hyperv_runtime,
> >          &vmstate_avx512,
> >          &vmstate_xss,
> > +        &vmstate_tsc_khz,
> >          NULL
> >      }
> >  };
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo

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

[Haozhong Zhang]

On 11/16/15 11:39, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 04:04:07PM +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.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > ---
> >  target-i386/kvm.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 2a9953b..9084b29 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
> >      return 0;
> >  }
> >  
> > +static void kvm_arch_set_tsc_khz(CPUState *cs)
> 
> > +{
> > +    X86CPU *cpu = X86_CPU(cs);
> > +    CPUX86State *env = &cpu->env;
> > +    int r;
> > +
> > +    /*
> > +     * If no user-specified TSC frequency is present, we will try to
> > +     * set env->tsc_khz to the value used by KVM.
> > +     */
> > +    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;
> 
> Can't we do this on kvm_arch_init_vcpu()? kvm_arch_put_registers()'s purpose is
> to just copy data from QEMU to KVM, not the other way around.
>

I'll move this to kvm_arch_init_vcpu().

> 
> > +        if (r < 0) {
> > +            error_report("warning: KVM_GET_TSC_KHZ failed");
> 
> Having a kernel that doesn't support KVM_CAP_GET_TSC_KHZ shouldn't trigger a
> warning every time we run QEMU, unless the user is explicitly asking for a
> feature that requires KVM_GET_TSC_KHZ.
>

I'll remove the warning.

> > +        } else {
> > +            env->tsc_khz = r;
> > +        }
> > +    }
> > +}
> > +
> >  int kvm_arch_put_registers(CPUState *cpu, int level)
> >  {
> >      X86CPU *x86_cpu = X86_CPU(cpu);
> > @@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
> >          }
> >      }
> >  
> > +    if (level == KVM_PUT_FULL_STATE) {
> > +        kvm_arch_set_tsc_khz(cpu);
> > +    }
> 
> I see that kvm_arch_set_tsc_khz() will be extended to call
> KVM_SET_TSC_KHZ in the next patch, so the kvm_arch_set_tsc_khz()
> seems to belong here. But the KVM_GET_TSC_KHZ call doesn't seem
> to belong in kvm_arch_set_tsc_khz()
> 
> kvm_arch_put_registers() callers don't expect any QEMU-side data
> to change, but just that KVM data is updated according to the
> QEMU-side data.
>

Good to know this. As above, I'll move the KVM_GET_TSC_KHZ call to
kvm_arch_init_vcpu().

Haozhong

> > +
> >      ret = kvm_getput_regs(x86_cpu, 1);
> >      if (ret < 0) {
> >          return ret;
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo

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

[Haozhong Zhang]

On 11/16/15 11:39, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 04:04:07PM +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.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > ---
> >  target-i386/kvm.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 2a9953b..9084b29 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2327,6 +2327,27 @@ static int kvm_get_debugregs(X86CPU *cpu)
> >      return 0;
> >  }
> >  
> > +static void kvm_arch_set_tsc_khz(CPUState *cs)
> 
> > +{
> > +    X86CPU *cpu = X86_CPU(cs);
> > +    CPUX86State *env = &cpu->env;
> > +    int r;
> > +
> > +    /*
> > +     * If no user-specified TSC frequency is present, we will try to
> > +     * set env->tsc_khz to the value used by KVM.
> > +     */
> > +    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;
> 
> Can't we do this on kvm_arch_init_vcpu()? kvm_arch_put_registers()'s purpose is
> to just copy data from QEMU to KVM, not the other way around.
>

I'll move this to kvm_arch_init_vcpu().

> 
> > +        if (r < 0) {
> > +            error_report("warning: KVM_GET_TSC_KHZ failed");
> 
> Having a kernel that doesn't support KVM_CAP_GET_TSC_KHZ shouldn't trigger a
> warning every time we run QEMU, unless the user is explicitly asking for a
> feature that requires KVM_GET_TSC_KHZ.
>

I'll remove the warning.

> > +        } else {
> > +            env->tsc_khz = r;
> > +        }
> > +    }
> > +}
> > +
> >  int kvm_arch_put_registers(CPUState *cpu, int level)
> >  {
> >      X86CPU *x86_cpu = X86_CPU(cpu);
> > @@ -2341,6 +2362,10 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
> >          }
> >      }
> >  
> > +    if (level == KVM_PUT_FULL_STATE) {
> > +        kvm_arch_set_tsc_khz(cpu);
> > +    }
> 
> I see that kvm_arch_set_tsc_khz() will be extended to call
> KVM_SET_TSC_KHZ in the next patch, so the kvm_arch_set_tsc_khz()
> seems to belong here. But the KVM_GET_TSC_KHZ call doesn't seem
> to belong in kvm_arch_set_tsc_khz()
> 
> kvm_arch_put_registers() callers don't expect any QEMU-side data
> to change, but just that KVM data is updated according to the
> QEMU-side data.
>

Good to know this. As above, I'll move the KVM_GET_TSC_KHZ call to
kvm_arch_init_vcpu().

Haozhong

> > +
> >      ret = kvm_getput_regs(x86_cpu, 1);
> >      if (ret < 0) {
> >          return ret;
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo

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

[Eduardo Habkost]

On Mon, Nov 16, 2015 at 10:30:08PM +0800, Haozhong Zhang wrote:
> On 11/16/15 11:43, Eduardo Habkost wrote:
> > On Mon, Nov 16, 2015 at 04:04:08PM +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>
[...]
> > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > > index 9084b29..8448248 100644
> > > --- a/target-i386/kvm.c
> > > +++ b/target-i386/kvm.c
> > > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
> > >      int r;
> > >  
> > >      /*
> > > -     * If no user-specified TSC frequency is present, we will try to
> > > -     * set env->tsc_khz to the value used by KVM.
> > > +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> > > +     *
> > > +     * - We have eliminated all cases that satisfy
> > > +     *       env->tsc_khz && env->user_tsc_khz &&
> > > +     *       env->tsc_khz != env->user_tsc_khz
> > > +     *   in cpu_post_load().
> > > +     *
> > > +     * - If env->user_tsc_khz is not zero, then it must be equal to
> > > +     *   env->tsc_khz (if the latter is not zero) and has been set in
> > > +     *   kvm_arch_init_vcpu().
> > > +     */
> > > +    if (env->tsc_khz && !env->user_tsc_khz) {
> > > +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> > > +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;
> > 
> > Please don't duplicate the code from kvm_arch_init_vcpu(). We can
> > handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
> > can't we?
> >
> 
> No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will
> exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not
> abort. And because the return value of
> kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its
> caller do_kvm_cpu_synchronize_post_init(), I have to handle them in
> different ways.

Reporting errors back in kvm_put_registers() may be difficult, I
see, so handling user-provided TSC frequency in
kvm_arch_init_vcpu() makes sense. But you can still avoid code
duplication. Just reuse the same function in kvm_arch_init_vcpu()
and kvm_put_registers(), but return errors back to the caller in
kvm_arch_init_vcpu() in case env->user_tsc_khz is set.

kvm_put_registers() can ignore the error, and just print a
warning. But (on both cases) we should print a warning only if
env->tsc_khz doesn't match KVM_GET_TSC_KHZ, because we don't want
to print spurious warnings on every migration when TSC scaling
isn't supported. (You even suggested changes to the example code
that does that, at Message-ID:
<20151106023244.GB24388@hzzhang-OptiPlex-9020.sh.intel.com>).

Also, I believe it won't be a problem if we call KVM_SET_TSC_KHZ
twice in the case of incoming migration, so there's no need to
check user_tsc_khz in the kvm_arch_put_registers() path. Keeping
the code simple is more important than avoiding one extra ioctl()
on incoming migration, IMO.

-- 
Eduardo

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

[Eduardo Habkost]

On Mon, Nov 16, 2015 at 10:30:08PM +0800, Haozhong Zhang wrote:
> On 11/16/15 11:43, Eduardo Habkost wrote:
> > On Mon, Nov 16, 2015 at 04:04:08PM +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>
[...]
> > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > > index 9084b29..8448248 100644
> > > --- a/target-i386/kvm.c
> > > +++ b/target-i386/kvm.c
> > > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
> > >      int r;
> > >  
> > >      /*
> > > -     * If no user-specified TSC frequency is present, we will try to
> > > -     * set env->tsc_khz to the value used by KVM.
> > > +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> > > +     *
> > > +     * - We have eliminated all cases that satisfy
> > > +     *       env->tsc_khz && env->user_tsc_khz &&
> > > +     *       env->tsc_khz != env->user_tsc_khz
> > > +     *   in cpu_post_load().
> > > +     *
> > > +     * - If env->user_tsc_khz is not zero, then it must be equal to
> > > +     *   env->tsc_khz (if the latter is not zero) and has been set in
> > > +     *   kvm_arch_init_vcpu().
> > > +     */
> > > +    if (env->tsc_khz && !env->user_tsc_khz) {
> > > +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> > > +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;
> > 
> > Please don't duplicate the code from kvm_arch_init_vcpu(). We can
> > handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
> > can't we?
> >
> 
> No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will
> exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not
> abort. And because the return value of
> kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its
> caller do_kvm_cpu_synchronize_post_init(), I have to handle them in
> different ways.

Reporting errors back in kvm_put_registers() may be difficult, I
see, so handling user-provided TSC frequency in
kvm_arch_init_vcpu() makes sense. But you can still avoid code
duplication. Just reuse the same function in kvm_arch_init_vcpu()
and kvm_put_registers(), but return errors back to the caller in
kvm_arch_init_vcpu() in case env->user_tsc_khz is set.

kvm_put_registers() can ignore the error, and just print a
warning. But (on both cases) we should print a warning only if
env->tsc_khz doesn't match KVM_GET_TSC_KHZ, because we don't want
to print spurious warnings on every migration when TSC scaling
isn't supported. (You even suggested changes to the example code
that does that, at Message-ID:
<20151106023244.GB24388@hzzhang-OptiPlex-9020.sh.intel.com>).

Also, I believe it won't be a problem if we call KVM_SET_TSC_KHZ
twice in the case of incoming migration, so there's no need to
check user_tsc_khz in the kvm_arch_put_registers() path. Keeping
the code simple is more important than avoiding one extra ioctl()
on incoming migration, IMO.

-- 
Eduardo

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

[Haozhong Zhang]

On 11/16/15 13:35, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 10:30:08PM +0800, Haozhong Zhang wrote:
> > On 11/16/15 11:43, Eduardo Habkost wrote:
> > > On Mon, Nov 16, 2015 at 04:04:08PM +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>
> [...]
> > > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > > > index 9084b29..8448248 100644
> > > > --- a/target-i386/kvm.c
> > > > +++ b/target-i386/kvm.c
> > > > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
> > > >      int r;
> > > >  
> > > >      /*
> > > > -     * If no user-specified TSC frequency is present, we will try to
> > > > -     * set env->tsc_khz to the value used by KVM.
> > > > +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> > > > +     *
> > > > +     * - We have eliminated all cases that satisfy
> > > > +     *       env->tsc_khz && env->user_tsc_khz &&
> > > > +     *       env->tsc_khz != env->user_tsc_khz
> > > > +     *   in cpu_post_load().
> > > > +     *
> > > > +     * - If env->user_tsc_khz is not zero, then it must be equal to
> > > > +     *   env->tsc_khz (if the latter is not zero) and has been set in
> > > > +     *   kvm_arch_init_vcpu().
> > > > +     */
> > > > +    if (env->tsc_khz && !env->user_tsc_khz) {
> > > > +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> > > > +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;
> > > 
> > > Please don't duplicate the code from kvm_arch_init_vcpu(). We can
> > > handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
> > > can't we?
> > >
> > 
> > No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will
> > exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not
> > abort. And because the return value of
> > kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its
> > caller do_kvm_cpu_synchronize_post_init(), I have to handle them in
> > different ways.
> 
> Reporting errors back in kvm_put_registers() may be difficult, I
> see, so handling user-provided TSC frequency in
> kvm_arch_init_vcpu() makes sense. But you can still avoid code
> duplication. Just reuse the same function in kvm_arch_init_vcpu()
> and kvm_put_registers(), but return errors back to the caller in
> kvm_arch_init_vcpu() in case env->user_tsc_khz is set.
>

Agree on using the same function to set TSC rate. I'll change in the
next version.

> kvm_put_registers() can ignore the error, and just print a
> warning. But (on both cases) we should print a warning only if
> env->tsc_khz doesn't match KVM_GET_TSC_KHZ, because we don't want
> to print spurious warnings on every migration when TSC scaling
> isn't supported. (You even suggested changes to the example code
> that does that, at Message-ID:
> <20151106023244.GB24388@hzzhang-OptiPlex-9020.sh.intel.com>).
>

I'll check whether env->tsc_khz == KVM_GET_TSC_KHZ in the next
version.

> Also, I believe it won't be a problem if we call KVM_SET_TSC_KHZ
> twice in the case of incoming migration, so there's no need to
> check user_tsc_khz in the kvm_arch_put_registers() path. Keeping
> the code simple is more important than avoiding one extra ioctl()
> on incoming migration, IMO.
>

I'll use tsc_khz only in the next version.

Thanks,
Haozhong

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

[Haozhong Zhang]

On 11/16/15 13:35, Eduardo Habkost wrote:
> On Mon, Nov 16, 2015 at 10:30:08PM +0800, Haozhong Zhang wrote:
> > On 11/16/15 11:43, Eduardo Habkost wrote:
> > > On Mon, Nov 16, 2015 at 04:04:08PM +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>
> [...]
> > > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > > > index 9084b29..8448248 100644
> > > > --- a/target-i386/kvm.c
> > > > +++ b/target-i386/kvm.c
> > > > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs)
> > > >      int r;
> > > >  
> > > >      /*
> > > > -     * If no user-specified TSC frequency is present, we will try to
> > > > -     * set env->tsc_khz to the value used by KVM.
> > > > +     * For other cases of env->tsc_khz and env->user_tsc_khz:
> > > > +     *
> > > > +     * - We have eliminated all cases that satisfy
> > > > +     *       env->tsc_khz && env->user_tsc_khz &&
> > > > +     *       env->tsc_khz != env->user_tsc_khz
> > > > +     *   in cpu_post_load().
> > > > +     *
> > > > +     * - If env->user_tsc_khz is not zero, then it must be equal to
> > > > +     *   env->tsc_khz (if the latter is not zero) and has been set in
> > > > +     *   kvm_arch_init_vcpu().
> > > > +     */
> > > > +    if (env->tsc_khz && !env->user_tsc_khz) {
> > > > +        r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ?
> > > > +            kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP;
> > > 
> > > Please don't duplicate the code from kvm_arch_init_vcpu(). We can
> > > handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE),
> > > can't we?
> > >
> > 
> > No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will
> > exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not
> > abort. And because the return value of
> > kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its
> > caller do_kvm_cpu_synchronize_post_init(), I have to handle them in
> > different ways.
> 
> Reporting errors back in kvm_put_registers() may be difficult, I
> see, so handling user-provided TSC frequency in
> kvm_arch_init_vcpu() makes sense. But you can still avoid code
> duplication. Just reuse the same function in kvm_arch_init_vcpu()
> and kvm_put_registers(), but return errors back to the caller in
> kvm_arch_init_vcpu() in case env->user_tsc_khz is set.
>

Agree on using the same function to set TSC rate. I'll change in the
next version.

> kvm_put_registers() can ignore the error, and just print a
> warning. But (on both cases) we should print a warning only if
> env->tsc_khz doesn't match KVM_GET_TSC_KHZ, because we don't want
> to print spurious warnings on every migration when TSC scaling
> isn't supported. (You even suggested changes to the example code
> that does that, at Message-ID:
> <20151106023244.GB24388@hzzhang-OptiPlex-9020.sh.intel.com>).
>

I'll check whether env->tsc_khz == KVM_GET_TSC_KHZ in the next
version.

> Also, I believe it won't be a problem if we call KVM_SET_TSC_KHZ
> twice in the case of incoming migration, so there's no need to
> check user_tsc_khz in the kvm_arch_put_registers() path. Keeping
> the code simple is more important than avoiding one extra ioctl()
> on incoming migration, IMO.
>

I'll use tsc_khz only in the next version.

Thanks,
Haozhong