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

Ying Fang posted 1 patch 3 years, 10 months ago
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200608121243.2076-1-fangying1@huawei.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>
[PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Posted by Ying Fang 3 years, 10 months ago
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>
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


Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Posted by Andrew Jones 3 years, 10 months ago
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
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,
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
> 
> 


Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Posted by Ying Fang 3 years, 10 months ago

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.
> 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. And we are planning to add 
support for it if nobody is currently doing that.

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
>>
>>
> 
> .
> 

Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Posted by Andrew Jones 3 years, 10 months ago
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?

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
> > > 
> > > 
> > 
> > .
> > 
> 


Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Posted by Ying Fang 3 years, 10 months ago

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
>>>>
>>>>
>>>
>>> .
>>>
>>
> 
> .
> 

Re: [PATCH v3] target/arm/cpu: adjust virtual time for arm cpu
Posted by Peter Maydell 3 years, 10 months ago
On Mon, 8 Jun 2020 at 13:12, Ying Fang <fangying1@huawei.com> 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>
> Signed-off-by: Ying Fang <fangying1@huawei.com>



Applied to target-arm.next, thanks (with a minor commit message
wording tweak, and the suggested-by tag removed).

-- PMM