qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu

From: Ying Fang
Subject: Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Date: Mon, 15 Jun 2020 20:43:25 +0800
User-agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 


On 6/10/2020 3:40 PM, Andrew Jones wrote:

On Wed, Jun 10, 2020 at 09:32:06AM +0800, Ying Fang wrote:



On 6/8/2020 8:49 PM, Andrew Jones wrote:

On Mon, Jun 08, 2020 at 08:12:43PM +0800, Ying Fang wrote:

From: fangying <fangying1@huawei.com>

Virtual time adjustment was implemented for virt-5.0 machine type,
but the cpu property was enabled only for host-passthrough and
max cpu model. Let's add it for arm cpu which has the generic timer
feature enabled.

Suggested-by: Andrew Jones <drjones@redhat.com>


This isn't true. I did suggest the way to arrange the code, after
Peter suggested to move the kvm_arm_add_vcpu_properties() call to
arm_cpu_post_init(), but I didn't suggest making this change in general,
which is what this tag means. In fact, I've argued that it's pretty

I'm quite sorry for adding it here.


No problem.


pointless to do this, since KVM users should be using '-cpu host' or
'-cpu max' anyway. Since I don't need credit for the code arranging,

As discussed in thread [1], there is a situation where a 'custom' cpu mode
is needed for us to keep instruction set compatibility so that migration can
be done, just like x86 does.


I understand the motivation. But, as I've said, KVM doesn't work that way.


And we are planning to add support for it if
nobody is currently doing that.


Great! I'm looking forward to seeing the KVM patches. Especially since,
without the KVM patches, the 'custom' CPU model isn't a custom CPU model,
it's just a misleading way to use host passthrough. Indeed, I'm a bit
opposed to allowing anything other than '-cpu host' and '-cpu max' (with
features explicitly enabled/disabled, e.g. -cpu host,pmu=off) to work
until KVM actually works with CPU models. Otherwise, how do we know the
difference between a model that actually works and one that is just
misleadingly named?

Yes you are right here.
My colleague zhanghailiang and me are now working on it. We will post the patch set soon. 

Thanks,
drew



Thanks.
Ying

[1]: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00022.html

please just drop the tag. Peter can maybe do that on merge though. Also,
despite not agreeing that we need this change today, as there's nothing
wrong with it and it looks good to me

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew


Signed-off-by: Ying Fang <fangying1@huawei.com>

---
v3:
- set kvm-no-adjvtime property in kvm_arm_add_vcpu_properties

v2:
- move kvm_arm_add_vcpu_properties into arm_cpu_post_init

v1:
- initial commit
- https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg08518.html

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 32bec156f2..5b7a36b5d7 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1245,6 +1245,10 @@ void arm_cpu_post_init(Object *obj)
       if (arm_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER)) {
           qdev_property_add_static(DEVICE(cpu), &arm_cpu_gt_cntfrq_property);
       }
+
+    if (kvm_enabled()) {
+        kvm_arm_add_vcpu_properties(obj);
+    }
   }
   static void arm_cpu_finalizefn(Object *obj)
@@ -2029,7 +2033,6 @@ 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);
@@ -2183,7 +2186,6 @@ 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 cbc5c3868f..778cecc2e6 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -592,7 +592,6 @@ 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 4bdbe6dcac..eef3bbd1cc 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -194,17 +194,18 @@ static void kvm_no_adjvtime_set(Object *obj, bool value, 
Error **errp)
   /* KVM VCPU properties should be prefixed with "kvm-". */
   void kvm_arm_add_vcpu_properties(Object *obj)
   {
-    if (!kvm_enabled()) {
-        return;
-    }
+    ARMCPU *cpu = ARM_CPU(obj);
+    CPUARMState *env = &cpu->env;
-    ARM_CPU(obj)->kvm_adjvtime = true;
-    object_property_add_bool(obj, "kvm-no-adjvtime", kvm_no_adjvtime_get,
-                             kvm_no_adjvtime_set);
-    object_property_set_description(obj, "kvm-no-adjvtime",
-                                    "Set on to disable the adjustment of "
-                                    "the virtual counter. VM stopped time "
-                                    "will be counted.");
+    if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
+        cpu->kvm_adjvtime = true;
+        object_property_add_bool(obj, "kvm-no-adjvtime", kvm_no_adjvtime_get,
+                                 kvm_no_adjvtime_set);
+        object_property_set_description(obj, "kvm-no-adjvtime",
+                                        "Set on to disable the adjustment of "
+                                        "the virtual counter. VM stopped time "
+                                        "will be counted.");
+    }
   }
   bool kvm_arm_pmu_supported(CPUState *cpu)
--
2.23.0




.





.
reply via email to
[Prev in Thread] Current Thread [Next in Thread]