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

[Philippe Mathieu-Daudé]

Hi Drew,

On Thu, Dec 12, 2019 at 7:27 PM Andrew Jones <address@hidden> wrote:
> kvm-no-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 <address@hidden>
> ---
>  docs/arm-cpu-features.rst | 31 ++++++++++++++++++++++++++++++-
>  hw/arm/virt.c             |  8 ++++++++
>  include/hw/arm/virt.h     |  1 +
>  target/arm/cpu.c          |  2 ++
>  target/arm/cpu64.c        |  1 +
>  target/arm/kvm.c          | 28 ++++++++++++++++++++++++++++
>  target/arm/kvm_arm.h      | 11 +++++++++++
>  target/arm/monitor.c      |  1 +
>  tests/arm-cpu-features.c  |  4 ++++
>  9 files changed, 86 insertions(+), 1 deletion(-)
>
[...]
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index cb7041e9677a..e9a08eb883bf 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1664,6 +1664,11 @@ static void machvirt_init(MachineState *machine)
>              }
>          }
>
> +        if (vmc->kvm_no_adjvtime &&
> +            object_property_find(cpuobj, "kvm-no-adjvtime", NULL)) {
> +            object_property_set_bool(cpuobj, true, "kvm-no-adjvtime", NULL);
> +        }
> +
>          if (vmc->no_pmu && object_property_find(cpuobj, "pmu", NULL)) {
>              object_property_set_bool(cpuobj, false, "pmu", NULL);
>          }
> @@ -2154,8 +2159,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(5, 0)
>
>  static void virt_machine_4_2_options(MachineClass *mc)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>      virt_machine_5_0_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
> +    vmc->kvm_no_adjvtime = true;
>  }
>  DEFINE_VIRT_MACHINE(4, 2)
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 0b41083e9d5c..80e00161b8f2 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -109,6 +109,7 @@ typedef struct {
>      bool smbios_old_sys_ver;
>      bool no_highmem_ecam;
>      bool no_ged;   /* Machines < 4.2 has no support for ACPI GED device */
> +    bool kvm_no_adjvtime;
>  } VirtMachineClass;
>
>  typedef struct {
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 7a4ac9339bf9..53c73c25a67f 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2482,6 +2482,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);
>
> @@ -2673,6 +2674,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 a39d6fcea34f..3cd416db8089 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -605,6 +605,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 a55fe7d7aefd..7666b250ab96 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,32 @@ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
>      env->features = arm_host_cpu_features.features;
>  }
>
> +static bool kvm_no_adjvtime_get(Object *obj, Error **errp)
> +{
> +    return !ARM_CPU(obj)->kvm_adjvtime;
> +}
> +
> +static void kvm_no_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()) {

This can't happen, right? Can we turn that into an assertion, or
remove the check?

> +        return;
> +    }
> +
> +    ARM_CPU(obj)->kvm_adjvtime = true;
> +    object_property_add_bool(obj, "kvm-no-adjvtime", kvm_no_adjvtime_get,
> +                             kvm_no_adjvtime_set, &error_abort);
> +    object_property_set_description(obj, "kvm-no-adjvtime",
> +                                    "Set on to disable the adjustment of "
> +                                    "the virtual counter. VM stopped time "
> +                                    "will 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 16b53e45377d..7d76f26beaca 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_get_virtual_time:
>   * @cs: CPUState
> @@ -319,6 +328,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 fa054f8a369c..9725dfff16d4 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-no-adjvtime",
>      NULL
>  };
>
> diff --git a/tests/arm-cpu-features.c b/tests/arm-cpu-features.c
> index 06ed05e934e6..738e49447377 100644
> --- a/tests/arm-cpu-features.c
> +++ b/tests/arm-cpu-features.c
> @@ -435,6 +435,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-no-adjvtime");
> +
>      if (g_str_equal(qtest_get_arch(), "aarch64")) {
>          assert_has_feature_enabled(qts, "max", "aarch64");
>          assert_has_feature_enabled(qts, "max", "sve");
> @@ -469,6 +471,8 @@ static void test_query_cpu_model_expansion_kvm(const void 
> *data)
>          return;
>      }
>
> +    assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime");
> +
>      if (g_str_equal(qtest_get_arch(), "aarch64")) {
>          bool kvm_supports_sve;
>          char max_name[8], name[8];
> --
> 2.21.0
>
>