[RFC PATCH 0/5] target/arm/kvm: Provide an option to adjust virtual time

[RFC PATCH 0/5] target/arm/kvm: Provide an option to adjust virtual time

[Andrew Jones]

This series is inspired by a series[1] posted by Bijan Mottahedeh about
a year ago.  The problem described in the cover letter of [1] is easily
reproducible and some users would like to have the option to avoid it.
However the solution, which is to adjust the virtual counter offset each
time the VM transitions to the running state, introduces a different
problem, which is that the virtual and physical counters diverge.  As
described in the cover letter of [1] this divergence is easily observed
when comparing the output of `date` and `hwclock` after suspending the
guest, waiting a while, and then resuming it.  Because this different
problem may actually be worse for some users, unlike [1], the series
posted here makes the virtual counter offset adjustment optional and not
even enabled by default.  Besides the adjustment being optional, this
series approaches the needed changes differently to apply them in more
appropriate locations.  Finally, unlike [1], this series doesn't attempt
to measure "pause time" itself.  Simply using QEMU_CLOCK_VIRTUAL, which
only ticks when the VM is not stopped, is sufficient.

I've based this series on the SVE series[2] because we're adding a new
CPU feature (kvm-adjvtime) and the SVE series introduces CPU feature
documentation, probing, and tests that we can then immediately apply
to kvm-adjvtime.

Additional notes
----------------

Note 1
------

As described above, when running a guest with kvm-adjtime enabled it
will be less likely the guest OS and guest applications get surprise
time jumps when they use the virtual counter.  However the counter will
no longer reflect real time.  It will lag behind.  If this is a problem
then the guest can resynchronize its time from an external source or
even from its physical counter.  If the suspend/resume is done with
libvirt's virsh, and the guest is running the guest agent, then it's
also possible to use a sequence like this

 $ virsh suspend $GUEST
 $ virsh resume $GUEST
 $ virsh domtime --sync $GUEST

in order to resynchronize a guest right after the resume.  Of course
there will still be time when the clock is not right, possibly creating
confusing timestamps in logs, for example, and the guest must still be
tolerant to the time synchronizations.

Note 2
------

Userspace that wants to set KVM_REG_ARM_TIMER_CNT should beware that
the KVM register ID is not correct.  This cannot be fixed because it's
UAPI and if the UAPI headers are used then it can't be a problem.
However, if a userspace attempts to create the ID themselves from the
register's specification, then they will get KVM_REG_ARM_TIMER_CVAL
instead, as the _CNT and _CVAL definitions have their register
parameters swapped.

Note 3
------

I didn't test this with a 32-bit KVM host, but the changes to kvm32.c
are the same as kvm64.c. So what could go wrong? Test results would be
appreciated.
 

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05713.html
[2] https://patchew.org/QEMU/20191001125845.8793-1-drjones@redhat.com/

Thanks,
drew


Andrew Jones (5):
  target/arm/kvm64: kvm64 cpus have timer registers
  timer: arm: Introduce functions to get the host cntfrq
  target/arm/kvm: Implement cpu feature kvm-adjvtime
  tests/arm-cpu-features: Check feature default values
  target/arm/cpu: Add the kvm-adjvtime CPU property

 docs/arm-cpu-features.rst | 27 +++++++++++++++++++++-
 include/qemu/timer.h      | 16 +++++++++++++
 target/arm/cpu.c          |  2 ++
 target/arm/cpu.h          |  3 +++
 target/arm/cpu64.c        |  1 +
 target/arm/kvm.c          | 47 ++++++++++++++++++++++++++++++++++++++
 target/arm/kvm32.c        | 15 ++++++++++++
 target/arm/kvm64.c        | 16 +++++++++++++
 target/arm/kvm_arm.h      | 23 +++++++++++++++++++
 target/arm/monitor.c      |  1 +
 tests/arm-cpu-features.c  | 48 +++++++++++++++++++++++++++++++--------
 11 files changed, 189 insertions(+), 10 deletions(-)

-- 
2.20.1

[RFC PATCH 1/5] target/arm/kvm64: kvm64 cpus have timer registers

[Andrew Jones]

Add the missing GENERIC_TIMER feature to kvm64 cpus.

We don't currently use these registers when KVM is enabled, but it's
probably best we add the feature flag for consistency and potential
future use. There's also precedent, as we add the PMU feature flag to
KVM enabled guests, even though we don't use those registers either.

This change was originally posted as a hunk of a different, never
merged patch from Bijan Mottahedeh.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target/arm/kvm64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 876184b8fe4d..5cafcb7d36dd 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -605,6 +605,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
     set_feature(&features, ARM_FEATURE_NEON);
     set_feature(&features, ARM_FEATURE_AARCH64);
     set_feature(&features, ARM_FEATURE_PMU);
+    set_feature(&features, ARM_FEATURE_GENERIC_TIMER);
 
     ahcf->features = features;
 
-- 
2.20.1

[RFC PATCH 2/5] timer: arm: Introduce functions to get the host cntfrq

[Andrew Jones]

When acceleration like KVM is in use it's necessary to use the host's
counter frequency when converting ticks to or from time units.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 include/qemu/timer.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 85bc6eb00b21..8941ddea8242 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -1006,6 +1006,22 @@ static inline int64_t cpu_get_host_ticks(void)
 }
 #endif
 
+#if defined(__aarch64__)
+static inline uint32_t cpu_get_host_tick_frequency(void)
+{
+    uint64_t frq;
+    asm volatile("mrs %0, cntfrq_el0" : "=r" (frq));
+    return frq;
+}
+#elif defined(__arm__)
+static inline uint32_t cpu_get_host_tick_frequency(void)
+{
+    uint32_t frq;
+    asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (frq));
+    return frq;
+}
+#endif
+
 #ifdef CONFIG_PROFILER
 static inline int64_t profile_getclock(void)
 {
-- 
2.20.1

[RFC PATCH 3/5] target/arm/kvm: Implement cpu feature kvm-adjvtime

[Andrew Jones]

When kvm-adjvtime is enabled the guest's cntvct[_el0] won't count
the time when the VM is stopped. That time is skipped by updating
cntvoff[_el2] on each transition to vm_running using the current
QEMU_CLOCK_VIRTUAL time. QEMU_CLOCK_VIRTUAL only ticks when the VM
is running.

This patch only provides the implementation. A subsequent patch
will provide the CPU property allowing the feature to be enabled.

Reported-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Suggested-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 target/arm/cpu.h     |  3 +++
 target/arm/kvm.c     | 20 ++++++++++++++++++++
 target/arm/kvm32.c   | 15 +++++++++++++++
 target/arm/kvm64.c   | 15 +++++++++++++++
 target/arm/kvm_arm.h | 12 ++++++++++++
 5 files changed, 65 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5b9c3e4cd73d..923bd5e6384d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -821,6 +821,9 @@ struct ARMCPU {
     /* KVM init features for this CPU */
     uint32_t kvm_init_features[7];
 
+    /* KVM CPU features */
+    bool kvm_adjvtime;
+
     /* Uniprocessor system with MP extensions */
     bool mp_is_up;
 
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 7a540d9591f9..f79b9b8ef57a 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -571,6 +571,26 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
     return 0;
 }
 
+void kvm_arm_set_virtual_time(CPUState *cs)
+{
+    uint64_t cnt;
+    struct kvm_one_reg reg = {
+        .id = KVM_REG_ARM_TIMER_CNT,
+        .addr = (uintptr_t)&cnt,
+    };
+    int ret;
+
+    cnt = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+                   cpu_get_host_tick_frequency(),
+                   NANOSECONDS_PER_SECOND);
+
+    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
+    if (ret) {
+        error_report("Failed to set KVM_REG_ARM_TIMER_CNT");
+        abort();
+    }
+}
+
 int kvm_put_vcpu_events(ARMCPU *cpu)
 {
     CPUARMState *env = &cpu->env;
diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
index 32bf8d6757c4..79c6013066a9 100644
--- a/target/arm/kvm32.c
+++ b/target/arm/kvm32.c
@@ -16,6 +16,7 @@
 #include "qemu-common.h"
 #include "cpu.h"
 #include "qemu/timer.h"
+#include "sysemu/runstate.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
 #include "internals.h"
@@ -183,6 +184,18 @@ int kvm_arm_cpreg_level(uint64_t regidx)
     return KVM_PUT_RUNTIME_STATE;
 }
 
+static void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
+{
+    CPUState *cs = opaque;
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    if (running) {
+        if (cpu->kvm_adjvtime) {
+            kvm_arm_set_virtual_time(cs);
+        }
+    }
+}
+
 #define ARM_CPU_ID_MPIDR       0, 0, 0, 5
 
 int kvm_arch_init_vcpu(CPUState *cs)
@@ -198,6 +211,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return -EINVAL;
     }
 
+    qemu_add_vm_change_state_handler(kvm_arm_vm_state_change, cs);
+
     /* Determine init features for this CPU */
     memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
     if (cpu->start_powered_off) {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 5cafcb7d36dd..494cf7f8a5cd 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -23,6 +23,7 @@
 #include "qemu/host-utils.h"
 #include "qemu/main-loop.h"
 #include "exec/gdbstub.h"
+#include "sysemu/runstate.h"
 #include "sysemu/kvm.h"
 #include "sysemu/kvm_int.h"
 #include "kvm_arm.h"
@@ -720,6 +721,18 @@ static int kvm_arm_sve_set_vls(CPUState *cs)
     return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
 }
 
+static void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
+{
+    CPUState *cs = opaque;
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    if (running) {
+        if (cpu->kvm_adjvtime) {
+            kvm_arm_set_virtual_time(cs);
+        }
+    }
+}
+
 #define ARM_CPU_ID_MPIDR       3, 0, 0, 0, 5
 
 int kvm_arch_init_vcpu(CPUState *cs)
@@ -735,6 +748,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
         return -EINVAL;
     }
 
+    qemu_add_vm_change_state_handler(kvm_arm_vm_state_change, cs);
+
     /* Determine init features for this CPU */
     memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
     if (cpu->start_powered_off) {
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index a1cc6513f72b..d506c4e84be6 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -232,6 +232,16 @@ void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map);
  */
 void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu);
 
+/**
+ * void kvm_arm_set_virtual_time:
+ * @cs: CPUState
+ *
+ * Sets the guest's virtual counter offset to the difference of the host's
+ * current time and QEMU's QEMU_CLOCK_VIRTUAL time. This allows the
+ * guest's virtual counter to only reflect VM running time.
+ */
+void kvm_arm_set_virtual_time(CPUState *cs);
+
 /**
  * kvm_arm_aarch32_supported:
  * @cs: CPUState
@@ -323,6 +333,8 @@ static inline int kvm_arm_vgic_probe(void)
     return 0;
 }
 
+static inline void kvm_arm_set_virtual_time(CPUState *cs) {}
+
 static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {}
 static inline void kvm_arm_pmu_init(CPUState *cs) {}
 
-- 
2.20.1

[RFC PATCH 4/5] tests/arm-cpu-features: Check feature default values

[Andrew Jones]

If we know what the default value should be then we can test for
that as well as the feature existence.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tests/arm-cpu-features.c | 44 ++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/tests/arm-cpu-features.c b/tests/arm-cpu-features.c
index 92668efb8f56..ee444b04010f 100644
--- a/tests/arm-cpu-features.c
+++ b/tests/arm-cpu-features.c
@@ -141,6 +141,32 @@ static bool resp_get_feature(QDict *resp, const char *feature)
     qobject_unref(_resp);                                              \
 })
 
+#define assert_has_feature_enabled(qts, cpu_type, feature)             \
+({                                                                     \
+    QDict *_resp, *_props;                                             \
+                                                                       \
+    _resp = do_query_no_props(qts, cpu_type);                          \
+    g_assert(_resp);                                                   \
+    g_assert(resp_has_props(_resp));                                   \
+    _props = resp_get_props(_resp);                                    \
+    g_assert(qdict_get(_props, feature));                              \
+    g_assert(qdict_get_bool(_props, feature));                         \
+    qobject_unref(_resp);                                              \
+})
+
+#define assert_has_feature_disabled(qts, cpu_type, feature)            \
+({                                                                     \
+    QDict *_resp, *_props;                                             \
+                                                                       \
+    _resp = do_query_no_props(qts, cpu_type);                          \
+    g_assert(_resp);                                                   \
+    g_assert(resp_has_props(_resp));                                   \
+    _props = resp_get_props(_resp);                                    \
+    g_assert(qdict_get(_props, feature));                              \
+    g_assert(!qdict_get_bool(_props, feature));                        \
+    qobject_unref(_resp);                                              \
+})
+
 static void assert_type_full(QTestState *qts)
 {
     const char *error;
@@ -387,16 +413,16 @@ static void test_query_cpu_model_expansion(const void *data)
     assert_error(qts, "host", "The CPU type 'host' requires KVM", NULL);
 
     /* Test expected feature presence/absence for some cpu types */
-    assert_has_feature(qts, "max", "pmu");
-    assert_has_feature(qts, "cortex-a15", "pmu");
+    assert_has_feature_enabled(qts, "max", "pmu");
+    assert_has_feature_enabled(qts, "cortex-a15", "pmu");
     assert_has_not_feature(qts, "cortex-a15", "aarch64");
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
-        assert_has_feature(qts, "max", "aarch64");
-        assert_has_feature(qts, "max", "sve");
-        assert_has_feature(qts, "max", "sve128");
-        assert_has_feature(qts, "cortex-a57", "pmu");
-        assert_has_feature(qts, "cortex-a57", "aarch64");
+        assert_has_feature_enabled(qts, "max", "aarch64");
+        assert_has_feature_enabled(qts, "max", "sve");
+        assert_has_feature_enabled(qts, "max", "sve128");
+        assert_has_feature_enabled(qts, "cortex-a57", "pmu");
+        assert_has_feature_enabled(qts, "cortex-a57", "aarch64");
 
         sve_tests_default(qts, "max");
 
@@ -417,7 +443,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
 
     qts = qtest_init(MACHINE "-accel kvm -cpu host");
 
-    assert_has_feature(qts, "host", "pmu");
+    assert_has_feature_enabled(qts, "host", "pmu");
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
         bool kvm_supports_sve;
@@ -427,7 +453,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         QDict *resp;
         char *error;
 
-        assert_has_feature(qts, "host", "aarch64");
+        assert_has_feature_enabled(qts, "host", "aarch64");
 
         assert_error(qts, "cortex-a15",
             "We cannot guarantee the CPU type 'cortex-a15' works "
-- 
2.20.1

[RFC PATCH 5/5] target/arm/cpu: Add the kvm-adjvtime CPU property

[Andrew Jones]

kvm-adjvtime is a KVM specific CPU property and a first of its kind.
To accommodate it we also add kvm_arm_add_vcpu_properties() and a
KVM specific CPU properties description to the CPU features document.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 docs/arm-cpu-features.rst | 27 ++++++++++++++++++++++++++-
 target/arm/cpu.c          |  2 ++
 target/arm/cpu64.c        |  1 +
 target/arm/kvm.c          | 27 +++++++++++++++++++++++++++
 target/arm/kvm_arm.h      | 11 +++++++++++
 target/arm/monitor.c      |  1 +
 tests/arm-cpu-features.c  |  4 ++++
 7 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/docs/arm-cpu-features.rst b/docs/arm-cpu-features.rst
index 1b367e22e16e..5c317296845f 100644
--- a/docs/arm-cpu-features.rst
+++ b/docs/arm-cpu-features.rst
@@ -31,7 +31,9 @@ supporting the feature or only supporting the feature under certain
 configurations.  For example, the `aarch64` CPU feature, which, when
 disabled, enables the optional AArch32 CPU feature, is only supported
 when using the KVM accelerator and when running on a host CPU type that
-supports the feature.
+supports the feature.  While `aarch64` currently only works with KVM,
+it could work with TCG.  CPU features that are specific to KVM are
+prefixed with "kvm-" and are described in "KVM VCPU Features".
 
 CPU Feature Probing
 ===================
@@ -171,6 +173,29 @@ disabling many SVE vector lengths would be quite verbose, the `sve<N>` CPU
 properties have special semantics (see "SVE CPU Property Parsing
 Semantics").
 
+KVM VCPU Features
+=================
+
+KVM VCPU features are CPU features that are specific to KVM, such as
+paravirt features or features that enable CPU virtualization extensions.
+The features' CPU properties are only available when KVM is enabled and are
+named with the prefix "kvm-".  KVM VCPU features may be probed, enabled, and
+disabled in the same way as other CPU features.  Below is the list of KVM
+VCPU features and their descriptions.
+
+  kvm-adjvtime             When enabled, each time the VM transitions back
+                           to running state the VCPU's vitual counter is
+                           updated to ensure stopped time is not counted.
+                           This avoids time jumps surprising guest OSes and
+                           applications, as long as they use the virtual
+                           counter for timekeeping, but has the side effect
+                           of the virtual and physical counters diverging.
+                           All timekeeping based on the virtual counter will
+                           appear to lag behind any timekeeping that does
+                           not subtract VM stopped time.  The guest may
+                           resynchronize its virtual counter with other time
+                           sources as needed.
+
 SVE CPU Properties
 ==================
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7695ae551218..d8ad26438f9f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2483,6 +2483,7 @@ static void arm_max_initfn(Object *obj)
 
     if (kvm_enabled()) {
         kvm_arm_set_cpu_features_from_host(cpu);
+        kvm_arm_add_vcpu_properties(obj);
     } else {
         cortex_a15_initfn(obj);
 
@@ -2674,6 +2675,7 @@ static void arm_host_initfn(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
         aarch64_add_sve_properties(obj);
     }
+    kvm_arm_add_vcpu_properties(obj);
     arm_cpu_post_init(obj);
 }
 
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 68baf0482ffa..c9a657a178ce 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -620,6 +620,7 @@ static void aarch64_max_initfn(Object *obj)
 
     if (kvm_enabled()) {
         kvm_arm_set_cpu_features_from_host(cpu);
+        kvm_arm_add_vcpu_properties(obj);
     } else {
         uint64_t t;
         uint32_t u;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index f79b9b8ef57a..1652e3febe51 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -17,6 +17,8 @@
 #include "qemu/timer.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
+#include "qom/object.h"
+#include "qapi/error.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/kvm_int.h"
@@ -179,6 +181,31 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
     env->features = arm_host_cpu_features.features;
 }
 
+static bool kvm_adjvtime_get(Object *obj, Error **errp)
+{
+    return ARM_CPU(obj)->kvm_adjvtime;
+}
+
+static void kvm_adjvtime_set(Object *obj, bool value, Error **errp)
+{
+    ARM_CPU(obj)->kvm_adjvtime = value;
+}
+
+/* KVM VCPU properties should be prefixed with "kvm-". */
+void kvm_arm_add_vcpu_properties(Object *obj)
+{
+    if (!kvm_enabled()) {
+        return;
+    }
+
+    object_property_add_bool(obj, "kvm-adjvtime", kvm_adjvtime_get,
+                             kvm_adjvtime_set, &error_abort);
+    object_property_set_description(obj, "kvm-adjvtime",
+                                    "Set on to enable the adjustment of "
+                                    "the virtual counter. VM stopped time "
+                                    "will not be counted.", &error_abort);
+}
+
 bool kvm_arm_pmu_supported(CPUState *cpu)
 {
     KVMState *s = KVM_STATE(current_machine->accelerator);
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index d506c4e84be6..5d8077df6a4a 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -232,6 +232,15 @@ void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map);
  */
 void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu);
 
+/**
+ * void kvm_arm_add_vcpu_properties:
+ * @obj: The CPU object to add the properties to
+ *
+ * Add all KVM specific CPU properties to the CPU object. These
+ * are the CPU properties with "kvm-" prefixed names.
+ */
+void kvm_arm_add_vcpu_properties(Object *obj);
+
 /**
  * void kvm_arm_set_virtual_time:
  * @cs: CPUState
@@ -308,6 +317,8 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
     cpu->host_cpu_probe_failed = true;
 }
 
+static inline void kvm_arm_add_vcpu_properties(Object *obj) {}
+
 static inline bool kvm_arm_aarch32_supported(CPUState *cs)
 {
     return false;
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index e912ed2cefa0..a89976fe7e4b 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -103,6 +103,7 @@ static const char *cpu_model_advertised_features[] = {
     "sve128", "sve256", "sve384", "sve512",
     "sve640", "sve768", "sve896", "sve1024", "sve1152", "sve1280",
     "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
+    "kvm-adjvtime",
     NULL
 };
 
diff --git a/tests/arm-cpu-features.c b/tests/arm-cpu-features.c
index ee444b04010f..c207a2bec9e9 100644
--- a/tests/arm-cpu-features.c
+++ b/tests/arm-cpu-features.c
@@ -417,6 +417,8 @@ static void test_query_cpu_model_expansion(const void *data)
     assert_has_feature_enabled(qts, "cortex-a15", "pmu");
     assert_has_not_feature(qts, "cortex-a15", "aarch64");
 
+    assert_has_not_feature(qts, "max", "kvm-adjvtime");
+
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
         assert_has_feature_enabled(qts, "max", "aarch64");
         assert_has_feature_enabled(qts, "max", "sve");
@@ -445,6 +447,8 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
 
     assert_has_feature_enabled(qts, "host", "pmu");
 
+    assert_has_feature_disabled(qts, "host", "kvm-adjvtime");
+
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
         bool kvm_supports_sve;
         char max_name[8], name[8];
-- 
2.20.1

Re: [RFC PATCH 2/5] timer: arm: Introduce functions to get the host cntfrq

[Richard Henderson]

On 10/7/19 1:06 PM, Andrew Jones wrote:
> When acceleration like KVM is in use it's necessary to use the host's
> counter frequency when converting ticks to or from time units.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  include/qemu/timer.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH 1/5] target/arm/kvm64: kvm64 cpus have timer registers

[Richard Henderson]

On 10/7/19 1:06 PM, Andrew Jones wrote:
> Add the missing GENERIC_TIMER feature to kvm64 cpus.
> 
> We don't currently use these registers when KVM is enabled, but it's
> probably best we add the feature flag for consistency and potential
> future use. There's also precedent, as we add the PMU feature flag to
> KVM enabled guests, even though we don't use those registers either.
> 
> This change was originally posted as a hunk of a different, never
> merged patch from Bijan Mottahedeh.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  target/arm/kvm64.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH 3/5] target/arm/kvm: Implement cpu feature kvm-adjvtime

[Richard Henderson]

On 10/7/19 1:06 PM, Andrew Jones wrote:
> +static void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
> +{
> +    CPUState *cs = opaque;
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    if (running) {
> +        if (cpu->kvm_adjvtime) {
> +            kvm_arm_set_virtual_time(cs);
> +        }
> +    }
> +}

Worth putting this in kvm.c too, so you don't have to duplicate it?  You can
always split it apart later if you ever do need a different hook for 32 vs 64.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH 4/5] tests/arm-cpu-features: Check feature default values

[Richard Henderson]

On 10/7/19 1:06 PM, Andrew Jones wrote:
> If we know what the default value should be then we can test for
> that as well as the feature existence.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  tests/arm-cpu-features.c | 44 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH 5/5] target/arm/cpu: Add the kvm-adjvtime CPU property

[Richard Henderson]

On 10/7/19 1:06 PM, Andrew Jones wrote:
> kvm-adjvtime is a KVM specific CPU property and a first of its kind.
> To accommodate it we also add kvm_arm_add_vcpu_properties() and a
> KVM specific CPU properties description to the CPU features document.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH 3/5] target/arm/kvm: Implement cpu feature kvm-adjvtime

[Andrew Jones]

On Wed, Oct 09, 2019 at 08:50:02PM -0400, Richard Henderson wrote:
> On 10/7/19 1:06 PM, Andrew Jones wrote:
> > +static void kvm_arm_vm_state_change(void *opaque, int running, RunState state)
> > +{
> > +    CPUState *cs = opaque;
> > +    ARMCPU *cpu = ARM_CPU(cs);
> > +
> > +    if (running) {
> > +        if (cpu->kvm_adjvtime) {
> > +            kvm_arm_set_virtual_time(cs);
> > +        }
> > +    }
> > +}
> 
> Worth putting this in kvm.c too, so you don't have to duplicate it?  You can
> always split it apart later if you ever do need a different hook for 32 vs 64.

True. I'll send a v1 without the premature duplication of this function
and your r-b's

Thanks,
drew