[PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Hi list,

This series is based on Paolo's guest_phys_bits patchset [1].

Currently, the old and new kvmclocks have the same feature name
"kvmclock" in FeatureWordInfo[FEAT_KVM].

When I tried to dig into the history of this unusual naming and fix it,
I realized that Tim was already trying to rename it, so I picked up his
renaming patch [2] (with a new commit message and other minor changes).

13 years age, the same name was introduced in [3], and its main purpose
is to make it easy for users to enable/disable 2 kvmclocks. Then, in
2012, Don tried to rename the new kvmclock, but the follow-up did not
address Igor and Eduardo's comments about compatibility.

Tim [2], not long ago, and I just now, were both puzzled by the naming
one after the other.

So, this series is to push for renaming the new kvmclock feature to
"kvmclock2" and adding compatibility support for older machines (PC 9.0
and older).

Finally, let's put an end to decades of doubt about this name.


Next Step
=========

This series just separates the two kvmclocks from the naming, and in
subsequent patches I plan to stop setting kvmclock(old kcmclock) by
default as long as KVM supports kvmclock2 (new kvmclock).

Also, try to deprecate the old kvmclock in KVM side.

[1]: https://lore.kernel.org/qemu-devel/20240325141422.1380087-1-pbonzini@redhat.com/
[2]: https://lore.kernel.org/qemu-devel/20230908124534.25027-4-twiederh@redhat.com/
[3]: https://lore.kernel.org/qemu-devel/1300401727-5235-3-git-send-email-glommer@redhat.com/
[4]: https://lore.kernel.org/qemu-devel/1348171412-23669-3-git-send-email-Don@CloudSwitch.com/

Thanks and Best Regards,
Zhao

---
Tim Wiederhake (1):
  target/i386: Fix duplicated kvmclock name in FEAT_KVM

Zhao Liu (6):
  target/i386/kvm: Add feature bit definitions for KVM CPUID
  target/i386/kvm: Remove local MSR_KVM_WALL_CLOCK and
    MSR_KVM_SYSTEM_TIME definitions
  target/i386/kvm: Only Save/load kvmclock MSRs when kvmclock enabled
  target/i386/kvm: Save/load MSRs of new kvmclock
    (KVM_FEATURE_CLOCKSOURCE2)
  target/i386/kvm: Add legacy_kvmclock cpu property
  target/i386/kvm: Update comment in kvm_cpu_realizefn()

 hw/i386/kvm/clock.c       |  5 ++--
 hw/i386/pc.c              |  1 +
 target/i386/cpu.c         |  3 +-
 target/i386/cpu.h         | 32 +++++++++++++++++++++
 target/i386/kvm/kvm-cpu.c | 25 ++++++++++++++++-
 target/i386/kvm/kvm.c     | 59 +++++++++++++++++++++++++--------------
 6 files changed, 99 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH for-9.1 1/7] target/i386/kvm: Add feature bit definitions for KVM CPUID

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Add feature definiations for KVM_CPUID_FEATURES in CPUID (
CPUID[4000_0001].EAX and CPUID[4000_0001].EDX), to get rid of lots of
offset calculations.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/kvm/clock.c   |  5 ++---
 target/i386/cpu.h     | 23 +++++++++++++++++++++++
 target/i386/kvm/kvm.c | 28 ++++++++++++++--------------
 3 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 40aa9a32c32c..7c9752d5036f 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -27,7 +27,6 @@
 #include "qapi/error.h"
 
 #include <linux/kvm.h>
-#include "standard-headers/asm-x86/kvm_para.h"
 #include "qom/object.h"
 
 #define TYPE_KVM_CLOCK "kvmclock"
@@ -334,8 +333,8 @@ void kvmclock_create(bool create_always)
 
     assert(kvm_enabled());
     if (create_always ||
-        cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
-                                       (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
+        cpu->env.features[FEAT_KVM] & (CPUID_FEAT_KVM_CLOCK |
+                                       CPUID_FEAT_KVM_CLOCK2)) {
         sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
     }
 }
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 83e473584517..b1b8d11cb0fe 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -27,6 +27,7 @@
 #include "qapi/qapi-types-common.h"
 #include "qemu/cpu-float.h"
 #include "qemu/timer.h"
+#include "standard-headers/asm-x86/kvm_para.h"
 
 #define XEN_NR_VIRQS 24
 
@@ -951,6 +952,28 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 /* Packets which contain IP payload have LIP values */
 #define CPUID_14_0_ECX_LIP              (1U << 31)
 
+/* (Old) KVM paravirtualized clocksource */
+#define CPUID_FEAT_KVM_CLOCK            (1U << KVM_FEATURE_CLOCKSOURCE)
+/* (New) KVM specific paravirtualized clocksource */
+#define CPUID_FEAT_KVM_CLOCK2           (1U << KVM_FEATURE_CLOCKSOURCE2)
+/* KVM asynchronous page fault */
+#define CPUID_FEAT_KVM_ASYNCPF          (1U << KVM_FEATURE_ASYNC_PF)
+/* KVM stolen (when guest vCPU is not running) time accounting */
+#define CPUID_FEAT_KVM_STEAL_TIME       (1U << KVM_FEATURE_STEAL_TIME)
+/* KVM paravirtualized end-of-interrupt signaling */
+#define CPUID_FEAT_KVM_PV_EOI           (1U << KVM_FEATURE_PV_EOI)
+/* KVM paravirtualized spinlocks support */
+#define CPUID_FEAT_KVM_PV_UNHALT        (1U << KVM_FEATURE_PV_UNHALT)
+/* KVM host-side polling on HLT control from the guest */
+#define CPUID_FEAT_KVM_POLL_CONTROL     (1U << KVM_FEATURE_POLL_CONTROL)
+/* KVM interrupt based asynchronous page fault*/
+#define CPUID_FEAT_KVM_ASYNCPF_INT      (1U << KVM_FEATURE_ASYNC_PF_INT)
+/* KVM 'Extended Destination ID' support for external interrupts */
+#define CPUID_FEAT_KVM_MSI_EXT_DEST_ID  (1U << KVM_FEATURE_MSI_EXT_DEST_ID)
+
+/* Hint to KVM that vCPUs expect never preempted for an unlimited time */
+#define CPUID_FEAT_KVM_HINTS_REALTIME    (1U << KVM_HINTS_REALTIME)
+
 /* CLZERO instruction */
 #define CPUID_8000_0008_EBX_CLZERO      (1U << 0)
 /* Always save/restore FP error pointers */
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index e68cbe929302..2f3c8bc3a4ed 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -481,13 +481,13 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
          * be enabled without the in-kernel irqchip
          */
         if (!kvm_irqchip_in_kernel()) {
-            ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
+            ret &= ~CPUID_FEAT_KVM_PV_UNHALT;
         }
         if (kvm_irqchip_is_split()) {
-            ret |= 1U << KVM_FEATURE_MSI_EXT_DEST_ID;
+            ret |= CPUID_FEAT_KVM_MSI_EXT_DEST_ID;
         }
     } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
-        ret |= 1U << KVM_HINTS_REALTIME;
+        ret |= CPUID_FEAT_KVM_HINTS_REALTIME;
     }
 
     return ret;
@@ -3324,20 +3324,20 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
         kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
         kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
-        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, env->async_pf_int_msr);
         }
-        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
         }
-        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_PV_EOI) {
             kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, env->pv_eoi_en_msr);
         }
-        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_STEAL_TIME) {
             kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
         }
 
-        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_POLL_CONTROL) {
             kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, env->poll_control_msr);
         }
 
@@ -3789,19 +3789,19 @@ static int kvm_get_msrs(X86CPU *cpu)
 #endif
     kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
     kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
-    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
     }
-    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, 0);
     }
-    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_PV_EOI) {
         kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, 0);
     }
-    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_STEAL_TIME) {
         kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0);
     }
-    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_POLL_CONTROL) {
         kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, 1);
     }
     if (has_architectural_pmu_version > 0) {
@@ -5434,7 +5434,7 @@ uint64_t kvm_swizzle_msi_ext_dest_id(uint64_t address)
         return address;
     }
     env = &X86_CPU(first_cpu)->env;
-    if (!(env->features[FEAT_KVM] & (1 << KVM_FEATURE_MSI_EXT_DEST_ID))) {
+    if (!(env->features[FEAT_KVM] & CPUID_FEAT_KVM_MSI_EXT_DEST_ID)) {
         return address;
     }
 
-- 
2.34.1

[PATCH for-9.1 2/7] target/i386/kvm: Remove local MSR_KVM_WALL_CLOCK and MSR_KVM_SYSTEM_TIME definitions

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

These 2 MSRs have been already defined in the kvm_para header
(standard-headers/asm-x86/kvm_para.h).

Remove QEMU local definitions to avoid duplication.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/kvm/kvm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 2f3c8bc3a4ed..be88339fb8bd 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -78,9 +78,6 @@
 #define KVM_APIC_BUS_CYCLE_NS       1
 #define KVM_APIC_BUS_FREQUENCY      (1000000000ULL / KVM_APIC_BUS_CYCLE_NS)
 
-#define MSR_KVM_WALL_CLOCK  0x11
-#define MSR_KVM_SYSTEM_TIME 0x12
-
 /* A 4096-byte buffer can hold the 8-byte kvm_msrs header, plus
  * 255 kvm_msr_entry structs */
 #define MSR_BUF_SIZE 4096
-- 
2.34.1

[PATCH for-9.1 3/7] target/i386/kvm: Only Save/load kvmclock MSRs when kvmclock enabled

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

MSR_KVM_SYSTEM_TIME and MSR_KVM_WALL_CLOCK are attached with the (old)
kvmclock feature (KVM_FEATURE_CLOCKSOURCE).

So, just save/load them only when kvmclock (KVM_FEATURE_CLOCKSOURCE) is
enabled.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/kvm/kvm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index be88339fb8bd..498580d60c3b 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3319,8 +3319,10 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
      */
     if (level >= KVM_PUT_RESET_STATE) {
         kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
-        kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
-        kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_CLOCK) {
+            kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
+            kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+        }
         if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, env->async_pf_int_msr);
         }
@@ -3784,8 +3786,10 @@ static int kvm_get_msrs(X86CPU *cpu)
         kvm_msr_entry_add(cpu, MSR_LSTAR, 0);
     }
 #endif
-    kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
-    kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_CLOCK) {
+        kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
+        kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
+    }
     if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
     }
-- 
2.34.1

[PATCH for-9.1 4/7] target/i386/kvm: Save/load MSRs of new kvmclock (KVM_FEATURE_CLOCKSOURCE2)

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

MSR_KVM_SYSTEM_TIME_NEW and MSR_KVM_WALL_CLOCK_NEW are bound to new
kvmclock (KVM_FEATURE_CLOCKSOURCE2).

Add the save/load support for these 2 MSRs.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.h     |  2 ++
 target/i386/kvm/kvm.c | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b1b8d11cb0fe..b339f9ce454f 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1741,6 +1741,8 @@ typedef struct CPUArchState {
 
     uint64_t system_time_msr;
     uint64_t wall_clock_msr;
+    uint64_t system_time_new_msr;
+    uint64_t wall_clock_new_msr;
     uint64_t steal_time_msr;
     uint64_t async_pf_en_msr;
     uint64_t async_pf_int_msr;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 498580d60c3b..4a6bf0581859 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3323,6 +3323,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
             kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
             kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
         }
+        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_CLOCK2) {
+            kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME_NEW,
+                              env->system_time_new_msr);
+            kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK_NEW,
+                              env->wall_clock_new_msr);
+        }
         if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, env->async_pf_int_msr);
         }
@@ -3790,6 +3796,10 @@ static int kvm_get_msrs(X86CPU *cpu)
         kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
         kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
     }
+    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_CLOCK2) {
+        kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME_NEW, 0);
+        kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK_NEW, 0);
+    }
     if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
     }
@@ -4029,6 +4039,12 @@ static int kvm_get_msrs(X86CPU *cpu)
         case MSR_KVM_WALL_CLOCK:
             env->wall_clock_msr = msrs[i].data;
             break;
+        case MSR_KVM_SYSTEM_TIME_NEW:
+            env->system_time_new_msr = msrs[i].data;
+            break;
+        case MSR_KVM_WALL_CLOCK_NEW:
+            env->wall_clock_new_msr = msrs[i].data;
+            break;
         case MSR_MCG_STATUS:
             env->mcg_status = msrs[i].data;
             break;
-- 
2.34.1

[PATCH for-9.1 5/7] target/i386/kvm: Add legacy_kvmclock cpu property

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Currently, the old kvmclock (KVM_FEATURE_CLOCKSOURCE) and the new
(KVM_FEATURE_CLOCKSOURCE2) are always set/unset together since they have
the same feature name "kvmclock" since the commit 642258c6c7 ("kvm: add
kvmclock to its second bit").

Before renaming the new kvmclock, introduce legacy_kvmclock to inherit
the behavior of both old and new kvmclocks being enabled/disabled at the
same time.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/pc.c              |  1 +
 target/i386/cpu.c         |  1 +
 target/i386/cpu.h         |  7 +++++++
 target/i386/kvm/kvm-cpu.c | 19 +++++++++++++++++++
 4 files changed, 28 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9c4b3969cc8a..a452da0a45a1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -80,6 +80,7 @@
 
 GlobalProperty pc_compat_9_0[] = {
     { TYPE_X86_CPU, "guest-phys-bits", "0" },
+    { TYPE_X86_CPU, "legacy-kvmclock", "true" },
 };
 const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0);
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index eef3d08473ed..1b6caf071a6d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7941,6 +7941,7 @@ static Property x86_cpu_properties[] = {
      */
     DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true),
     DEFINE_PROP_BOOL("xen-vapic", X86CPU, xen_vapic, false),
+    DEFINE_PROP_BOOL("legacy-kvmclock", X86CPU, legacy_kvmclock, false),
 
     /*
      * From "Requirements for Implementing the Microsoft
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b339f9ce454f..b3ee2a35f2c1 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2070,6 +2070,13 @@ struct ArchCPU {
     int32_t hv_max_vps;
 
     bool xen_vapic;
+
+    /*
+     * Compatibility bits for old machine types.
+     * If true, always set/unset KVM_FEATURE_CLOCKSOURCE and
+     * KVM_FEATURE_CLOCKSOURCE2 at the same time.
+     */
+    bool legacy_kvmclock;
 };
 
 typedef struct X86CPUModel X86CPUModel;
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index e6b7a46743b5..ae3cb27c8aa8 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -18,6 +18,8 @@
 #include "kvm_i386.h"
 #include "hw/core/accel-cpu.h"
 
+#include "standard-headers/asm-x86/kvm_para.h"
+
 static void kvm_set_guest_phys_bits(CPUState *cs)
 {
     X86CPU *cpu = X86_CPU(cs);
@@ -72,6 +74,23 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
                                                    MSR_IA32_UCODE_REV);
         }
     }
+
+    if (cpu->legacy_kvmclock) {
+        /*
+         * The old and new kvmclock are both set by default from the
+         * oldest KVM supported (v4.5, see "OS requirements" section at
+         * docs/system/target-i386.rst). So when one of them is missing,
+         * it is only possible that the user is actively masking it.
+         * Then, mask both at the same time for compatibility with v9.0
+         * and older QEMU's kvmclock behavior.
+         */
+        if (!(env->features[FEAT_KVM] & CPUID_FEAT_KVM_CLOCK) ||
+            !(env->features[FEAT_KVM] & CPUID_FEAT_KVM_CLOCK2)) {
+            env->features[FEAT_KVM] &= ~(CPUID_FEAT_KVM_CLOCK |
+                                         CPUID_FEAT_KVM_CLOCK2);
+        }
+    }
+
     ret = host_cpu_realizefn(cs, errp);
     if (!ret) {
         return ret;
-- 
2.34.1

[PATCH for-9.1 6/7] target/i386: Fix duplicated kvmclock name in FEAT_KVM

[Zhao Liu]

From: Tim Wiederhake <twiederh@redhat.com>

The commit 642258c6c7 ("kvm: add kvmclock to its second bit") gave the
old and new kvmclocks with the same name "kvmclock", to facilitate user
to set/unset the feature bits for both 2 kvmclock features together.

This could work because:
* QEMU side:
  - x86_cpu_register_bit_prop() supports "the same property name can be
    registered multiple times to make it affect multiple bits in the
    same FeatureWord".
* KVM side:
  - The only difference between 2 version kvmclocks is their MSRs have
    different addresses.
  - When 2 kvmclocks are both enabled, KVM will prioritize the use of
    new kvmclock's MSRs.

However, there're reasons we need give the second kvmclock a new name:
* Based on the KVM mechanism, it doesn't make sense to bind two
  kvmclocks together. Since kvmclock is enabled by default in most cases
  as a KVM PV feature, the benefit of the same naming is reflected in
  the fact that -kvmclock can disable both. But, following the KVM
  interface style (i.e., separating the two kvmclocks) is clearly
  clearer to the user.
* For developers, identical names have been creating confusion along
  with persistent doubts about the naming.
* FeatureWordInfo should define names based on hardware/Host CPUID bit,
  and the name is used to distinguish the bit.
* User actions based on +/- feature names should only work on
  independent feature bits. The common effect of multiple features
  should be controlled by an additional CPU property or additional code
  logic to show the association between different feature bits.
* The old kvmclock will eventually be removed. Different naming can ease
  the burden of future cleanups.

Therefore, rename the new kvmclock feature as "kvmclock2".

Additionally, add "kvmclock2" entry in kvm_default_props[] since the
oldest kernel supported by QEMU (v4.5) has supported the new kvm clock.

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Based on Tim's original patch, rewrote the commit message and added the
tiny fix for compatibility.
---
 target/i386/cpu.c         | 2 +-
 target/i386/kvm/kvm-cpu.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1b6caf071a6d..0a1dac60f5de 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -855,7 +855,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     [FEAT_KVM] = {
         .type = CPUID_FEATURE_WORD,
         .feat_names = {
-            "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
+            "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock2",
             "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
             NULL, "kvm-pv-tlb-flush", "kvm-asyncpf-vmexit", "kvm-pv-ipi",
             "kvm-poll-control", "kvm-pv-sched-yield", "kvm-asyncpf-int", "kvm-msi-ext-dest-id",
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index ae3cb27c8aa8..753f90c18bd6 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -77,7 +77,7 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
 
     if (cpu->legacy_kvmclock) {
         /*
-         * The old and new kvmclock are both set by default from the
+         * The kvmclock and kvmclock2 are both set by default from the
          * oldest KVM supported (v4.5, see "OS requirements" section at
          * docs/system/target-i386.rst). So when one of them is missing,
          * it is only possible that the user is actively masking it.
@@ -179,6 +179,7 @@ static void kvm_cpu_xsave_init(void)
  */
 static PropValue kvm_default_props[] = {
     { "kvmclock", "on" },
+    { "kvmclock2", "on" },
     { "kvm-nopiodelay", "on" },
     { "kvm-asyncpf", "on" },
     { "kvm-steal-time", "on" },
-- 
2.34.1

[PATCH for-9.1 7/7] target/i386/kvm: Update comment in kvm_cpu_realizefn()

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

With the guest_phys_bits and legacy_kvmclock change, update the comment
about function call flow.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/kvm/kvm-cpu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 753f90c18bd6..5b48b023c33b 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -60,7 +60,10 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
      *  -> x86_cpu_expand_features()
      *  -> cpu_exec_realizefn():
      *            -> accel_cpu_common_realize()
-     *               kvm_cpu_realizefn() -> host_cpu_realizefn()
+     *               kvm_cpu_realizefn()
+     *                       -> update cpu_pm, ucode_rev, kvmclock
+     *                          host_cpu_realizefn()
+     *                          update guest_phys_bits on Host support
      *  -> cpu_common_realizefn()
      *  -> check/update ucode_rev, phys_bits, mwait
      */
-- 
2.34.1

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Zhao Liu]

Hi maintainers,

Ping. Do you like this diea?

Thanks,
Zhao

On Fri, Mar 29, 2024 at 06:19:47PM +0800, Zhao Liu wrote:
> Date: Fri, 29 Mar 2024 18:19:47 +0800
> From: Zhao Liu <zhao1.liu@linux.intel.com>
> Subject: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature
>  name
> X-Mailer: git-send-email 2.34.1
> 
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Hi list,
> 
> This series is based on Paolo's guest_phys_bits patchset [1].
> 
> Currently, the old and new kvmclocks have the same feature name
> "kvmclock" in FeatureWordInfo[FEAT_KVM].
> 
> When I tried to dig into the history of this unusual naming and fix it,
> I realized that Tim was already trying to rename it, so I picked up his
> renaming patch [2] (with a new commit message and other minor changes).
> 
> 13 years age, the same name was introduced in [3], and its main purpose
> is to make it easy for users to enable/disable 2 kvmclocks. Then, in
> 2012, Don tried to rename the new kvmclock, but the follow-up did not
> address Igor and Eduardo's comments about compatibility.
> 
> Tim [2], not long ago, and I just now, were both puzzled by the naming
> one after the other.
> 
> So, this series is to push for renaming the new kvmclock feature to
> "kvmclock2" and adding compatibility support for older machines (PC 9.0
> and older).
> 
> Finally, let's put an end to decades of doubt about this name.
> 
> 
> Next Step
> =========
> 
> This series just separates the two kvmclocks from the naming, and in
> subsequent patches I plan to stop setting kvmclock(old kcmclock) by
> default as long as KVM supports kvmclock2 (new kvmclock).
> 
> Also, try to deprecate the old kvmclock in KVM side.
> 
> [1]: https://lore.kernel.org/qemu-devel/20240325141422.1380087-1-pbonzini@redhat.com/
> [2]: https://lore.kernel.org/qemu-devel/20230908124534.25027-4-twiederh@redhat.com/
> [3]: https://lore.kernel.org/qemu-devel/1300401727-5235-3-git-send-email-glommer@redhat.com/
> [4]: https://lore.kernel.org/qemu-devel/1348171412-23669-3-git-send-email-Don@CloudSwitch.com/
> 
> Thanks and Best Regards,
> Zhao
> 
> ---
> Tim Wiederhake (1):
>   target/i386: Fix duplicated kvmclock name in FEAT_KVM
> 
> Zhao Liu (6):
>   target/i386/kvm: Add feature bit definitions for KVM CPUID
>   target/i386/kvm: Remove local MSR_KVM_WALL_CLOCK and
>     MSR_KVM_SYSTEM_TIME definitions
>   target/i386/kvm: Only Save/load kvmclock MSRs when kvmclock enabled
>   target/i386/kvm: Save/load MSRs of new kvmclock
>     (KVM_FEATURE_CLOCKSOURCE2)
>   target/i386/kvm: Add legacy_kvmclock cpu property
>   target/i386/kvm: Update comment in kvm_cpu_realizefn()
> 
>  hw/i386/kvm/clock.c       |  5 ++--
>  hw/i386/pc.c              |  1 +
>  target/i386/cpu.c         |  3 +-
>  target/i386/cpu.h         | 32 +++++++++++++++++++++
>  target/i386/kvm/kvm-cpu.c | 25 ++++++++++++++++-
>  target/i386/kvm/kvm.c     | 59 +++++++++++++++++++++++++--------------
>  6 files changed, 99 insertions(+), 26 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [PATCH for-9.1 1/7] target/i386/kvm: Add feature bit definitions for KVM CPUID

[Xiaoyao Li]

On 3/29/2024 6:19 PM, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Add feature definiations for KVM_CPUID_FEATURES in CPUID (
> CPUID[4000_0001].EAX and CPUID[4000_0001].EDX), to get rid of lots of
> offset calculations.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   hw/i386/kvm/clock.c   |  5 ++---
>   target/i386/cpu.h     | 23 +++++++++++++++++++++++
>   target/i386/kvm/kvm.c | 28 ++++++++++++++--------------
>   3 files changed, 39 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> index 40aa9a32c32c..7c9752d5036f 100644
> --- a/hw/i386/kvm/clock.c
> +++ b/hw/i386/kvm/clock.c
> @@ -27,7 +27,6 @@
>   #include "qapi/error.h"
>   
>   #include <linux/kvm.h>
> -#include "standard-headers/asm-x86/kvm_para.h"
>   #include "qom/object.h"
>   
>   #define TYPE_KVM_CLOCK "kvmclock"
> @@ -334,8 +333,8 @@ void kvmclock_create(bool create_always)
>   
>       assert(kvm_enabled());
>       if (create_always ||
> -        cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
> -                                       (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
> +        cpu->env.features[FEAT_KVM] & (CPUID_FEAT_KVM_CLOCK |
> +                                       CPUID_FEAT_KVM_CLOCK2)) {
>           sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
>       }
>   }
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 83e473584517..b1b8d11cb0fe 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -27,6 +27,7 @@
>   #include "qapi/qapi-types-common.h"
>   #include "qemu/cpu-float.h"
>   #include "qemu/timer.h"
> +#include "standard-headers/asm-x86/kvm_para.h"
>   
>   #define XEN_NR_VIRQS 24
>   
> @@ -951,6 +952,28 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
>   /* Packets which contain IP payload have LIP values */
>   #define CPUID_14_0_ECX_LIP              (1U << 31)
>   
> +/* (Old) KVM paravirtualized clocksource */
> +#define CPUID_FEAT_KVM_CLOCK            (1U << KVM_FEATURE_CLOCKSOURCE)

we can drop the _FEAT_, just name it as

CPUID_KVM_CLOCK

> +/* (New) KVM specific paravirtualized clocksource */
> +#define CPUID_FEAT_KVM_CLOCK2           (1U << KVM_FEATURE_CLOCKSOURCE2)
> +/* KVM asynchronous page fault */
> +#define CPUID_FEAT_KVM_ASYNCPF          (1U << KVM_FEATURE_ASYNC_PF)
> +/* KVM stolen (when guest vCPU is not running) time accounting */
> +#define CPUID_FEAT_KVM_STEAL_TIME       (1U << KVM_FEATURE_STEAL_TIME)
> +/* KVM paravirtualized end-of-interrupt signaling */
> +#define CPUID_FEAT_KVM_PV_EOI           (1U << KVM_FEATURE_PV_EOI)
> +/* KVM paravirtualized spinlocks support */
> +#define CPUID_FEAT_KVM_PV_UNHALT        (1U << KVM_FEATURE_PV_UNHALT)
> +/* KVM host-side polling on HLT control from the guest */
> +#define CPUID_FEAT_KVM_POLL_CONTROL     (1U << KVM_FEATURE_POLL_CONTROL)
> +/* KVM interrupt based asynchronous page fault*/
> +#define CPUID_FEAT_KVM_ASYNCPF_INT      (1U << KVM_FEATURE_ASYNC_PF_INT)
> +/* KVM 'Extended Destination ID' support for external interrupts */
> +#define CPUID_FEAT_KVM_MSI_EXT_DEST_ID  (1U << KVM_FEATURE_MSI_EXT_DEST_ID)
> +
> +/* Hint to KVM that vCPUs expect never preempted for an unlimited time */
> +#define CPUID_FEAT_KVM_HINTS_REALTIME    (1U << KVM_HINTS_REALTIME)
> +
>   /* CLZERO instruction */
>   #define CPUID_8000_0008_EBX_CLZERO      (1U << 0)
>   /* Always save/restore FP error pointers */
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index e68cbe929302..2f3c8bc3a4ed 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -481,13 +481,13 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>            * be enabled without the in-kernel irqchip
>            */
>           if (!kvm_irqchip_in_kernel()) {
> -            ret &= ~(1U << KVM_FEATURE_PV_UNHALT);
> +            ret &= ~CPUID_FEAT_KVM_PV_UNHALT;
>           }
>           if (kvm_irqchip_is_split()) {
> -            ret |= 1U << KVM_FEATURE_MSI_EXT_DEST_ID;
> +            ret |= CPUID_FEAT_KVM_MSI_EXT_DEST_ID;
>           }
>       } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
> -        ret |= 1U << KVM_HINTS_REALTIME;
> +        ret |= CPUID_FEAT_KVM_HINTS_REALTIME;
>       }
>   
>       return ret;
> @@ -3324,20 +3324,20 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>           kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
>           kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
>           kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
> -        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
> +        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
>               kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, env->async_pf_int_msr);
>           }
> -        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
> +        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF) {
>               kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
>           }
> -        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
> +        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_PV_EOI) {
>               kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, env->pv_eoi_en_msr);
>           }
> -        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
> +        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_STEAL_TIME) {
>               kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, env->steal_time_msr);
>           }
>   
> -        if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
> +        if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_POLL_CONTROL) {
>               kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, env->poll_control_msr);
>           }
>   
> @@ -3789,19 +3789,19 @@ static int kvm_get_msrs(X86CPU *cpu)
>   #endif
>       kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
>       kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
> -    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF_INT)) {
> +    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF_INT) {
>           kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
>       }
> -    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
> +    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_ASYNCPF) {
>           kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, 0);
>       }
> -    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
> +    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_PV_EOI) {
>           kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, 0);
>       }
> -    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_STEAL_TIME)) {
> +    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_STEAL_TIME) {
>           kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 0);
>       }
> -    if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_POLL_CONTROL)) {
> +    if (env->features[FEAT_KVM] & CPUID_FEAT_KVM_POLL_CONTROL) {
>           kvm_msr_entry_add(cpu, MSR_KVM_POLL_CONTROL, 1);
>       }
>       if (has_architectural_pmu_version > 0) {
> @@ -5434,7 +5434,7 @@ uint64_t kvm_swizzle_msi_ext_dest_id(uint64_t address)
>           return address;
>       }
>       env = &X86_CPU(first_cpu)->env;
> -    if (!(env->features[FEAT_KVM] & (1 << KVM_FEATURE_MSI_EXT_DEST_ID))) {
> +    if (!(env->features[FEAT_KVM] & CPUID_FEAT_KVM_MSI_EXT_DEST_ID)) {
>           return address;
>       }
>   

Re: [PATCH for-9.1 2/7] target/i386/kvm: Remove local MSR_KVM_WALL_CLOCK and MSR_KVM_SYSTEM_TIME definitions

[Xiaoyao Li]

On 3/29/2024 6:19 PM, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> These 2 MSRs have been already defined in the kvm_para header
> (standard-headers/asm-x86/kvm_para.h).
> 
> Remove QEMU local definitions to avoid duplication.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   target/i386/kvm/kvm.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 2f3c8bc3a4ed..be88339fb8bd 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -78,9 +78,6 @@
>   #define KVM_APIC_BUS_CYCLE_NS       1
>   #define KVM_APIC_BUS_FREQUENCY      (1000000000ULL / KVM_APIC_BUS_CYCLE_NS)
>   
> -#define MSR_KVM_WALL_CLOCK  0x11
> -#define MSR_KVM_SYSTEM_TIME 0x12
> -
>   /* A 4096-byte buffer can hold the 8-byte kvm_msrs header, plus
>    * 255 kvm_msr_entry structs */
>   #define MSR_BUF_SIZE 4096

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Xiaoyao Li]

On 3/29/2024 6:19 PM, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Hi list,
> 
> This series is based on Paolo's guest_phys_bits patchset [1].
> 
> Currently, the old and new kvmclocks have the same feature name
> "kvmclock" in FeatureWordInfo[FEAT_KVM].
> 
> When I tried to dig into the history of this unusual naming and fix it,
> I realized that Tim was already trying to rename it, so I picked up his
> renaming patch [2] (with a new commit message and other minor changes).
> 
> 13 years age, the same name was introduced in [3], and its main purpose
> is to make it easy for users to enable/disable 2 kvmclocks. Then, in
> 2012, Don tried to rename the new kvmclock, but the follow-up did not
> address Igor and Eduardo's comments about compatibility.
> 
> Tim [2], not long ago, and I just now, were both puzzled by the naming
> one after the other.

The commit message of [3] said the reason clearly:

   When we tweak flags involving this value - specially when we use "-",
   we have to act on both.

So you are trying to change it to "when people want to disable kvmclock, 
they need to use '-kvmclock,-kvmclock2' instead of '-kvmclock'"

IMHO, I prefer existing code and I don't see much value of 
differentiating them. If the current code puzzles you, then we can add 
comment to explain.

> So, this series is to push for renaming the new kvmclock feature to
> "kvmclock2" and adding compatibility support for older machines (PC 9.0
> and older).
> 
> Finally, let's put an end to decades of doubt about this name.
> 
> 
> Next Step
> =========
> 
> This series just separates the two kvmclocks from the naming, and in
> subsequent patches I plan to stop setting kvmclock(old kcmclock) by
> default as long as KVM supports kvmclock2 (new kvmclock).

No. It will break existing guests that use KVM_FEATURE_CLOCKSOURCE.

> Also, try to deprecate the old kvmclock in KVM side.
> 
> [1]: https://lore.kernel.org/qemu-devel/20240325141422.1380087-1-pbonzini@redhat.com/
> [2]: https://lore.kernel.org/qemu-devel/20230908124534.25027-4-twiederh@redhat.com/
> [3]: https://lore.kernel.org/qemu-devel/1300401727-5235-3-git-send-email-glommer@redhat.com/
> [4]: https://lore.kernel.org/qemu-devel/1348171412-23669-3-git-send-email-Don@CloudSwitch.com/
> 
> Thanks and Best Regards,
> Zhao
> 
> ---
> Tim Wiederhake (1):
>    target/i386: Fix duplicated kvmclock name in FEAT_KVM
> 
> Zhao Liu (6):
>    target/i386/kvm: Add feature bit definitions for KVM CPUID
>    target/i386/kvm: Remove local MSR_KVM_WALL_CLOCK and
>      MSR_KVM_SYSTEM_TIME definitions
>    target/i386/kvm: Only Save/load kvmclock MSRs when kvmclock enabled
>    target/i386/kvm: Save/load MSRs of new kvmclock
>      (KVM_FEATURE_CLOCKSOURCE2)
>    target/i386/kvm: Add legacy_kvmclock cpu property
>    target/i386/kvm: Update comment in kvm_cpu_realizefn()
> 
>   hw/i386/kvm/clock.c       |  5 ++--
>   hw/i386/pc.c              |  1 +
>   target/i386/cpu.c         |  3 +-
>   target/i386/cpu.h         | 32 +++++++++++++++++++++
>   target/i386/kvm/kvm-cpu.c | 25 ++++++++++++++++-
>   target/i386/kvm/kvm.c     | 59 +++++++++++++++++++++++++--------------
>   6 files changed, 99 insertions(+), 26 deletions(-)
> 

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Zhao Liu]

Hi Xiaoyao,

On Wed, Apr 24, 2024 at 11:57:11PM +0800, Xiaoyao Li wrote:
> Date: Wed, 24 Apr 2024 23:57:11 +0800
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
>  feature name
> 
> On 3/29/2024 6:19 PM, Zhao Liu wrote:
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > Hi list,
> > 
> > This series is based on Paolo's guest_phys_bits patchset [1].
> > 
> > Currently, the old and new kvmclocks have the same feature name
> > "kvmclock" in FeatureWordInfo[FEAT_KVM].
> > 
> > When I tried to dig into the history of this unusual naming and fix it,
> > I realized that Tim was already trying to rename it, so I picked up his
> > renaming patch [2] (with a new commit message and other minor changes).
> > 
> > 13 years age, the same name was introduced in [3], and its main purpose
> > is to make it easy for users to enable/disable 2 kvmclocks. Then, in
> > 2012, Don tried to rename the new kvmclock, but the follow-up did not
> > address Igor and Eduardo's comments about compatibility.
> > 
> > Tim [2], not long ago, and I just now, were both puzzled by the naming
> > one after the other.
> 
> The commit message of [3] said the reason clearly:
> 
>   When we tweak flags involving this value - specially when we use "-",
>   we have to act on both.
> 
> So you are trying to change it to "when people want to disable kvmclock,
> they need to use '-kvmclock,-kvmclock2' instead of '-kvmclock'"
>
> IMHO, I prefer existing code and I don't see much value of differentiating
> them. If the current code puzzles you, then we can add comment to explain.

It's enough to just enable kvmclock2 for Guest; kvmclock (old) is
redundant in the presence of kvmclock2.

So operating both feature bits at the same time is not a reasonable
choice, we should only keep kvmclock2 for Guest. It's possible because
the oldest linux (v4.5) which QEMU i386 supports has kvmclock2.

Pls see:
https://elixir.bootlin.com/linux/v4.5/source/arch/x86/include/uapi/asm/kvm_para.h#L22

With this in mind, I'm trying to implement a silky smooth transition to
"only kcmclock2 support".

This series is now separating kvmclock and kvmclock2, and then I plan to
go to the KVM side and deprecate kvmclock2, and then finally remove
kvmclock (old) in QEMU. Separating the two features makes the process
clearer.

Continuing to use the same name equally would have silently caused the
CPUID to change on the Guest side, which would have hurt compatibility
as well.

> > So, this series is to push for renaming the new kvmclock feature to
> > "kvmclock2" and adding compatibility support for older machines (PC 9.0
> > and older).
> > 
> > Finally, let's put an end to decades of doubt about this name.
> > 
> > 
> > Next Step
> > =========
> > 
> > This series just separates the two kvmclocks from the naming, and in
> > subsequent patches I plan to stop setting kvmclock(old kcmclock) by
> > default as long as KVM supports kvmclock2 (new kvmclock).
> 
> No. It will break existing guests that use KVM_FEATURE_CLOCKSOURCE.

Please refer to my elaboration on v4.5 above, where kvmclock2 is
available, it should be used in priority.

> > Also, try to deprecate the old kvmclock in KVM side.
> > 
> > [1]: https://lore.kernel.org/qemu-devel/20240325141422.1380087-1-pbonzini@redhat.com/
> > [2]: https://lore.kernel.org/qemu-devel/20230908124534.25027-4-twiederh@redhat.com/
> > [3]: https://lore.kernel.org/qemu-devel/1300401727-5235-3-git-send-email-glommer@redhat.com/
> > [4]: https://lore.kernel.org/qemu-devel/1348171412-23669-3-git-send-email-Don@CloudSwitch.com/

Thanks,
Zhao

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Xiaoyao Li]

On 4/25/2024 3:17 PM, Zhao Liu wrote:
> Hi Xiaoyao,
> 
> On Wed, Apr 24, 2024 at 11:57:11PM +0800, Xiaoyao Li wrote:
>> Date: Wed, 24 Apr 2024 23:57:11 +0800
>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>> Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
>>   feature name
>>
>> On 3/29/2024 6:19 PM, Zhao Liu wrote:
>>> From: Zhao Liu <zhao1.liu@intel.com>
>>>
>>> Hi list,
>>>
>>> This series is based on Paolo's guest_phys_bits patchset [1].
>>>
>>> Currently, the old and new kvmclocks have the same feature name
>>> "kvmclock" in FeatureWordInfo[FEAT_KVM].
>>>
>>> When I tried to dig into the history of this unusual naming and fix it,
>>> I realized that Tim was already trying to rename it, so I picked up his
>>> renaming patch [2] (with a new commit message and other minor changes).
>>>
>>> 13 years age, the same name was introduced in [3], and its main purpose
>>> is to make it easy for users to enable/disable 2 kvmclocks. Then, in
>>> 2012, Don tried to rename the new kvmclock, but the follow-up did not
>>> address Igor and Eduardo's comments about compatibility.
>>>
>>> Tim [2], not long ago, and I just now, were both puzzled by the naming
>>> one after the other.
>>
>> The commit message of [3] said the reason clearly:
>>
>>    When we tweak flags involving this value - specially when we use "-",
>>    we have to act on both.
>>
>> So you are trying to change it to "when people want to disable kvmclock,
>> they need to use '-kvmclock,-kvmclock2' instead of '-kvmclock'"
>>
>> IMHO, I prefer existing code and I don't see much value of differentiating
>> them. If the current code puzzles you, then we can add comment to explain.
> 
> It's enough to just enable kvmclock2 for Guest; kvmclock (old) is
> redundant in the presence of kvmclock2.
> 
> So operating both feature bits at the same time is not a reasonable
> choice, we should only keep kvmclock2 for Guest. It's possible because
> the oldest linux (v4.5) which QEMU i386 supports has kvmclock2.

who said the oldest Linux QEMU supports is from 4.5? what about 2.x kernel?

Besides, not only the Linux guest, whatever guest OS is, it will be 
broken if the guest is using kvmclock and QEMU starts to drop support of 
kvmclock.

So, again, hard NAK to drop the support of kvmclock. It breaks existing 
guests that use kvmclock. You cannot force people to upgrade their 
existing VMs to use kvmclock2 instead of kvmclock.

> Pls see:
> https://elixir.bootlin.com/linux/v4.5/source/arch/x86/include/uapi/asm/kvm_para.h#L22
> 
> With this in mind, I'm trying to implement a silky smooth transition to
> "only kcmclock2 support".
> 
> This series is now separating kvmclock and kvmclock2, and then I plan to
> go to the KVM side and deprecate kvmclock2, and then finally remove
> kvmclock (old) in QEMU. Separating the two features makes the process
> clearer.
> 
> Continuing to use the same name equally would have silently caused the
> CPUID to change on the Guest side, which would have hurt compatibility
> as well.
> 
>>> So, this series is to push for renaming the new kvmclock feature to
>>> "kvmclock2" and adding compatibility support for older machines (PC 9.0
>>> and older).
>>>
>>> Finally, let's put an end to decades of doubt about this name.
>>>
>>>
>>> Next Step
>>> =========
>>>
>>> This series just separates the two kvmclocks from the naming, and in
>>> subsequent patches I plan to stop setting kvmclock(old kcmclock) by
>>> default as long as KVM supports kvmclock2 (new kvmclock).
>>
>> No. It will break existing guests that use KVM_FEATURE_CLOCKSOURCE.
> 
> Please refer to my elaboration on v4.5 above, where kvmclock2 is
> available, it should be used in priority.
> 
>>> Also, try to deprecate the old kvmclock in KVM side.
>>>
>>> [1]: https://lore.kernel.org/qemu-devel/20240325141422.1380087-1-pbonzini@redhat.com/
>>> [2]: https://lore.kernel.org/qemu-devel/20230908124534.25027-4-twiederh@redhat.com/
>>> [3]: https://lore.kernel.org/qemu-devel/1300401727-5235-3-git-send-email-glommer@redhat.com/
>>> [4]: https://lore.kernel.org/qemu-devel/1348171412-23669-3-git-send-email-Don@CloudSwitch.com/
> 
> Thanks,
> Zhao
> 
> 

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Zhao Liu]

On Thu, Apr 25, 2024 at 04:40:10PM +0800, Xiaoyao Li wrote:
> Date: Thu, 25 Apr 2024 16:40:10 +0800
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
>  feature name
> 
> On 4/25/2024 3:17 PM, Zhao Liu wrote:
> > Hi Xiaoyao,
> > 
> > On Wed, Apr 24, 2024 at 11:57:11PM +0800, Xiaoyao Li wrote:
> > > Date: Wed, 24 Apr 2024 23:57:11 +0800
> > > From: Xiaoyao Li <xiaoyao.li@intel.com>
> > > Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
> > >   feature name
> > > 
> > > On 3/29/2024 6:19 PM, Zhao Liu wrote:
> > > > From: Zhao Liu <zhao1.liu@intel.com>
> > > > 
> > > > Hi list,
> > > > 
> > > > This series is based on Paolo's guest_phys_bits patchset [1].
> > > > 
> > > > Currently, the old and new kvmclocks have the same feature name
> > > > "kvmclock" in FeatureWordInfo[FEAT_KVM].
> > > > 
> > > > When I tried to dig into the history of this unusual naming and fix it,
> > > > I realized that Tim was already trying to rename it, so I picked up his
> > > > renaming patch [2] (with a new commit message and other minor changes).
> > > > 
> > > > 13 years age, the same name was introduced in [3], and its main purpose
> > > > is to make it easy for users to enable/disable 2 kvmclocks. Then, in
> > > > 2012, Don tried to rename the new kvmclock, but the follow-up did not
> > > > address Igor and Eduardo's comments about compatibility.
> > > > 
> > > > Tim [2], not long ago, and I just now, were both puzzled by the naming
> > > > one after the other.
> > > 
> > > The commit message of [3] said the reason clearly:
> > > 
> > >    When we tweak flags involving this value - specially when we use "-",
> > >    we have to act on both.
> > > 
> > > So you are trying to change it to "when people want to disable kvmclock,
> > > they need to use '-kvmclock,-kvmclock2' instead of '-kvmclock'"
> > > 
> > > IMHO, I prefer existing code and I don't see much value of differentiating
> > > them. If the current code puzzles you, then we can add comment to explain.
> > 
> > It's enough to just enable kvmclock2 for Guest; kvmclock (old) is
> > redundant in the presence of kvmclock2.
> > 
> > So operating both feature bits at the same time is not a reasonable
> > choice, we should only keep kvmclock2 for Guest. It's possible because
> > the oldest linux (v4.5) which QEMU i386 supports has kvmclock2.
> 
> who said the oldest Linux QEMU supports is from 4.5? what about 2.x kernel?

For Host (docs/system/target-i386.rst).

> Besides, not only the Linux guest, whatever guest OS is, it will be broken
> if the guest is using kvmclock and QEMU starts to drop support of kvmclock.

I'm not aware of any minimum version requirements for Guest supported
by KVM, but there are no commitment. 

> So, again, hard NAK to drop the support of kvmclock. It breaks existing
> guests that use kvmclock. You cannot force people to upgrade their existing
> VMs to use kvmclock2 instead of kvmclock.

I agree, legacy kvmclock can be left out, if the old kernel does not
support kvmclock2 and strongly requires kvmclock, it can be enabled
using 9.1 and earlier machines or legacy-kvmclock, as long as Host still
supports it.

What's the gap in handling it this way? especially considering that
kvmclock2 was introduced in v2.6.35, and earlier kernels are no longer
maintained. The availability of the PV feature requires compatibility
for both Host and Guest.

Anyway, the above discussion here is about future plans, and this series
does not prevent any Guest from ignoring kvmclock2 in favor of kvmclock.

What this series is doing, i.e. separating the current two kvmclock and
ensuring CPUID compatibility via legacy-kvmclock, could balance the
compatibility requirements of the ancient (unmaintained kernel) with
the need for future feature changes.

Thanks,
Zhao

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Xiaoyao Li]

On 4/25/2024 6:29 PM, Zhao Liu wrote:
> On Thu, Apr 25, 2024 at 04:40:10PM +0800, Xiaoyao Li wrote:
>> Date: Thu, 25 Apr 2024 16:40:10 +0800
>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>> Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
>>   feature name
>>
>> On 4/25/2024 3:17 PM, Zhao Liu wrote:
>>> Hi Xiaoyao,
>>>
>>> On Wed, Apr 24, 2024 at 11:57:11PM +0800, Xiaoyao Li wrote:
>>>> Date: Wed, 24 Apr 2024 23:57:11 +0800
>>>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>>>> Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
>>>>    feature name
>>>>
>>>> On 3/29/2024 6:19 PM, Zhao Liu wrote:
>>>>> From: Zhao Liu <zhao1.liu@intel.com>
>>>>>
>>>>> Hi list,
>>>>>
>>>>> This series is based on Paolo's guest_phys_bits patchset [1].
>>>>>
>>>>> Currently, the old and new kvmclocks have the same feature name
>>>>> "kvmclock" in FeatureWordInfo[FEAT_KVM].
>>>>>
>>>>> When I tried to dig into the history of this unusual naming and fix it,
>>>>> I realized that Tim was already trying to rename it, so I picked up his
>>>>> renaming patch [2] (with a new commit message and other minor changes).
>>>>>
>>>>> 13 years age, the same name was introduced in [3], and its main purpose
>>>>> is to make it easy for users to enable/disable 2 kvmclocks. Then, in
>>>>> 2012, Don tried to rename the new kvmclock, but the follow-up did not
>>>>> address Igor and Eduardo's comments about compatibility.
>>>>>
>>>>> Tim [2], not long ago, and I just now, were both puzzled by the naming
>>>>> one after the other.
>>>>
>>>> The commit message of [3] said the reason clearly:
>>>>
>>>>     When we tweak flags involving this value - specially when we use "-",
>>>>     we have to act on both.
>>>>
>>>> So you are trying to change it to "when people want to disable kvmclock,
>>>> they need to use '-kvmclock,-kvmclock2' instead of '-kvmclock'"
>>>>
>>>> IMHO, I prefer existing code and I don't see much value of differentiating
>>>> them. If the current code puzzles you, then we can add comment to explain.
>>>
>>> It's enough to just enable kvmclock2 for Guest; kvmclock (old) is
>>> redundant in the presence of kvmclock2.
>>>
>>> So operating both feature bits at the same time is not a reasonable
>>> choice, we should only keep kvmclock2 for Guest. It's possible because
>>> the oldest linux (v4.5) which QEMU i386 supports has kvmclock2.
>>
>> who said the oldest Linux QEMU supports is from 4.5? what about 2.x kernel?
> 
> For Host (docs/system/target-i386.rst).
> 
>> Besides, not only the Linux guest, whatever guest OS is, it will be broken
>> if the guest is using kvmclock and QEMU starts to drop support of kvmclock.
> 
> I'm not aware of any minimum version requirements for Guest supported
> by KVM, but there are no commitment.

the common commitment is at least keeping backwards compatibility.

>> So, again, hard NAK to drop the support of kvmclock. It breaks existing
>> guests that use kvmclock. You cannot force people to upgrade their existing
>> VMs to use kvmclock2 instead of kvmclock.
> 
> I agree, legacy kvmclock can be left out, if the old kernel does not
> support kvmclock2 and strongly requires kvmclock, it can be enabled
> using 9.1 and earlier machines or legacy-kvmclock, as long as Host still
> supports it.
> 
> What's the gap in handling it this way? especially considering that
> kvmclock2 was introduced in v2.6.35, and earlier kernels are no longer
> maintained. The availability of the PV feature requires compatibility
> for both Host and Guest.
> 
> Anyway, the above discussion here is about future plans, and this series
> does not prevent any Guest from ignoring kvmclock2 in favor of kvmclock.
> 
> What this series is doing, i.e. separating the current two kvmclock and
> ensuring CPUID compatibility via legacy-kvmclock, could balance the
> compatibility requirements of the ancient (unmaintained kernel) with
> the need for future feature changes.

You introduce a user-visible change that people need to use 
"-kvmclock,-kvmclock2" to totally disable kvmclock.

The only difference between kvmclock and kvmclock2 is the MSR index. And 
from users' perspective, they don't care this difference. The existing 
usage is simple. When I want kvmclock, use "+kvmclock". When I don't 
want it, use "-kvmclock".

You are complicating things and I don't see a strong reason that we have 
to do it.

> Thanks,
> Zhao
> 

Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock feature name

[Zhao Liu]

On Thu, Apr 25, 2024 at 08:04:46PM +0800, Xiaoyao Li wrote:
> Date: Thu, 25 Apr 2024 20:04:46 +0800
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
>  feature name
> 
> On 4/25/2024 6:29 PM, Zhao Liu wrote:
> > On Thu, Apr 25, 2024 at 04:40:10PM +0800, Xiaoyao Li wrote:
> > > Date: Thu, 25 Apr 2024 16:40:10 +0800
> > > From: Xiaoyao Li <xiaoyao.li@intel.com>
> > > Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
> > >   feature name
> > > 
> > > On 4/25/2024 3:17 PM, Zhao Liu wrote:
> > > > Hi Xiaoyao,
> > > > 
> > > > On Wed, Apr 24, 2024 at 11:57:11PM +0800, Xiaoyao Li wrote:
> > > > > Date: Wed, 24 Apr 2024 23:57:11 +0800
> > > > > From: Xiaoyao Li <xiaoyao.li@intel.com>
> > > > > Subject: Re: [PATCH for-9.1 0/7] target/i386/kvm: Cleanup the kvmclock
> > > > >    feature name
> > > > > 
> > > > > On 3/29/2024 6:19 PM, Zhao Liu wrote:
> > > > > > From: Zhao Liu <zhao1.liu@intel.com>
> > > > > > 
> > > > > > Hi list,
> > > > > > 
> > > > > > This series is based on Paolo's guest_phys_bits patchset [1].
> > > > > > 
> > > > > > Currently, the old and new kvmclocks have the same feature name
> > > > > > "kvmclock" in FeatureWordInfo[FEAT_KVM].
> > > > > > 
> > > > > > When I tried to dig into the history of this unusual naming and fix it,
> > > > > > I realized that Tim was already trying to rename it, so I picked up his
> > > > > > renaming patch [2] (with a new commit message and other minor changes).
> > > > > > 
> > > > > > 13 years age, the same name was introduced in [3], and its main purpose
> > > > > > is to make it easy for users to enable/disable 2 kvmclocks. Then, in
> > > > > > 2012, Don tried to rename the new kvmclock, but the follow-up did not
> > > > > > address Igor and Eduardo's comments about compatibility.
> > > > > > 
> > > > > > Tim [2], not long ago, and I just now, were both puzzled by the naming
> > > > > > one after the other.
> > > > > 
> > > > > The commit message of [3] said the reason clearly:
> > > > > 
> > > > >     When we tweak flags involving this value - specially when we use "-",
> > > > >     we have to act on both.
> > > > > 
> > > > > So you are trying to change it to "when people want to disable kvmclock,
> > > > > they need to use '-kvmclock,-kvmclock2' instead of '-kvmclock'"
> > > > > 
> > > > > IMHO, I prefer existing code and I don't see much value of differentiating
> > > > > them. If the current code puzzles you, then we can add comment to explain.
> > > > 
> > > > It's enough to just enable kvmclock2 for Guest; kvmclock (old) is
> > > > redundant in the presence of kvmclock2.
> > > > 
> > > > So operating both feature bits at the same time is not a reasonable
> > > > choice, we should only keep kvmclock2 for Guest. It's possible because
> > > > the oldest linux (v4.5) which QEMU i386 supports has kvmclock2.
> > > 
> > > who said the oldest Linux QEMU supports is from 4.5? what about 2.x kernel?
> > 
> > For Host (docs/system/target-i386.rst).
> > 
> > > Besides, not only the Linux guest, whatever guest OS is, it will be broken
> > > if the guest is using kvmclock and QEMU starts to drop support of kvmclock.
> > 
> > I'm not aware of any minimum version requirements for Guest supported
> > by KVM, but there are no commitment.
> 
> the common commitment is at least keeping backwards compatibility.
> 
> > > So, again, hard NAK to drop the support of kvmclock. It breaks existing
> > > guests that use kvmclock. You cannot force people to upgrade their existing
> > > VMs to use kvmclock2 instead of kvmclock.
> > 
> > I agree, legacy kvmclock can be left out, if the old kernel does not
> > support kvmclock2 and strongly requires kvmclock, it can be enabled
> > using 9.1 and earlier machines or legacy-kvmclock, as long as Host still
> > supports it.
> > 
> > What's the gap in handling it this way? especially considering that
> > kvmclock2 was introduced in v2.6.35, and earlier kernels are no longer
> > maintained. The availability of the PV feature requires compatibility
> > for both Host and Guest.
> > 
> > Anyway, the above discussion here is about future plans, and this series
> > does not prevent any Guest from ignoring kvmclock2 in favor of kvmclock.
> > 
> > What this series is doing, i.e. separating the current two kvmclock and
> > ensuring CPUID compatibility via legacy-kvmclock, could balance the
> > compatibility requirements of the ancient (unmaintained kernel) with
> > the need for future feature changes.
> 
> You introduce a user-visible change that people need to use
> "-kvmclock,-kvmclock2" to totally disable kvmclock.
> 
> The only difference between kvmclock and kvmclock2 is the MSR index. And
> from users' perspective, they don't care this difference. The existing usage
> is simple. When I want kvmclock, use "+kvmclock". When I don't want it, use
> "-kvmclock".
> 
> You are complicating things and I don't see a strong reason that we have to
> do it.
>

Okay, only when old kvmclock deprecation occurs can the values (of
renaming) be recognized, then I'll hold the renamed patches for now and
only keep some of the other cleanups.

Thanks for your review,

-Zhao