* [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu @ 2020-06-03 2:02 Ying Fang 2020-06-03 2:09 ` Richard Henderson 2020-06-03 8:26 ` Andrew Jones 0 siblings, 2 replies; 5+ messages in thread From: Ying Fang @ 2020-06-03 2:02 UTC (permalink / raw) To: drjones, qemu-devel, qemu-arm Cc: peter.maydell, zhang.zhanghailiang, wu.wubin 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 gernic timer feature enabled. Signed-off-by: Ying Fang <fangying1@huawei.com> --- 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 --- target/arm/cpu.c | 3 +-- target/arm/cpu64.c | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 32bec156f2..1e9b7a51f2 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1244,6 +1244,7 @@ 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); + kvm_arm_add_vcpu_properties(obj); } } @@ -2029,7 +2030,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 +2183,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; -- 2.23.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu 2020-06-03 2:02 [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu Ying Fang @ 2020-06-03 2:09 ` Richard Henderson 2020-06-03 8:26 ` Andrew Jones 1 sibling, 0 replies; 5+ messages in thread From: Richard Henderson @ 2020-06-03 2:09 UTC (permalink / raw) To: Ying Fang, drjones, qemu-devel, qemu-arm Cc: peter.maydell, zhang.zhanghailiang, wu.wubin On 6/2/20 7:02 PM, Ying Fang wrote: > 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 gernic "generic" > timer feature enabled. > > > Signed-off-by: Ying Fang <fangying1@huawei.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu 2020-06-03 2:02 [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu Ying Fang 2020-06-03 2:09 ` Richard Henderson @ 2020-06-03 8:26 ` Andrew Jones 2020-06-03 14:23 ` Peter Maydell 2020-06-04 7:16 ` Ying Fang 1 sibling, 2 replies; 5+ messages in thread From: Andrew Jones @ 2020-06-03 8:26 UTC (permalink / raw) To: Ying Fang Cc: peter.maydell, qemu-arm, qemu-devel, wu.wubin, zhang.zhanghailiang On Wed, Jun 03, 2020 at 10:02:08AM +0800, Ying Fang wrote: > 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 gernic > timer feature enabled. > > > Signed-off-by: Ying Fang <fangying1@huawei.com> > > --- > 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 > > --- > target/arm/cpu.c | 3 +-- > target/arm/cpu64.c | 1 - > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 32bec156f2..1e9b7a51f2 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1244,6 +1244,7 @@ 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); > + kvm_arm_add_vcpu_properties(obj); The name 'kvm_arm_add_vcpu_properties' says nothing about being specific to the timer. So this is either the wrong place for this function, or the function is named wrong. I'd say it's the wrong place, because, as the comment above kvm_arm_add_vcpu_properties() implies, the function is for all 'kvm-*' prefixed properties, not just timer related ones. It's true we don't have any others yet, but we will. I have plans to post kvm-steal-time soon, and despite it also having 'time' in its name, it shouldn't depend on ARM_FEATURE_GENERIC_TIMER. How about the below [untested] patch instead? Thanks, drew diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 32bec156f2d4..e9084f98ef10 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(); + } } 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 cbc5c3868fce..778cecc2e6ca 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 4bdbe6dcac07..eef3bbd1cc2c 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) > } > } > > @@ -2029,7 +2030,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 +2183,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; > -- > 2.23.0 > > > ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu 2020-06-03 8:26 ` Andrew Jones @ 2020-06-03 14:23 ` Peter Maydell 2020-06-04 7:16 ` Ying Fang 1 sibling, 0 replies; 5+ messages in thread From: Peter Maydell @ 2020-06-03 14:23 UTC (permalink / raw) To: Andrew Jones Cc: Ying Fang, qemu-arm, QEMU Developers, wu.wubin, zhanghailiang On Wed, 3 Jun 2020 at 09:26, Andrew Jones <drjones@redhat.com> wrote: > The name 'kvm_arm_add_vcpu_properties' says nothing about being specific > to the timer. So this is either the wrong place for this function, or the > function is named wrong. I'd say it's the wrong place, because, as the > comment above kvm_arm_add_vcpu_properties() implies, the function is > for all 'kvm-*' prefixed properties, not just timer related ones. It's > true we don't have any others yet, but we will. I have plans to post > kvm-steal-time soon, and despite it also having 'time' in its name, it > shouldn't depend on ARM_FEATURE_GENERIC_TIMER. > > How about the below [untested] patch instead? Your patch is the code arrangement I had in mind, yes. -- PMM ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu 2020-06-03 8:26 ` Andrew Jones 2020-06-03 14:23 ` Peter Maydell @ 2020-06-04 7:16 ` Ying Fang 1 sibling, 0 replies; 5+ messages in thread From: Ying Fang @ 2020-06-04 7:16 UTC (permalink / raw) To: Andrew Jones Cc: peter.maydell, qemu-arm, qemu-devel, wu.wubin, zhang.zhanghailiang On 6/3/2020 4:26 PM, Andrew Jones wrote: > On Wed, Jun 03, 2020 at 10:02:08AM +0800, Ying Fang wrote: >> 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 gernic >> timer feature enabled. >> >> >> Signed-off-by: Ying Fang <fangying1@huawei.com> >> >> --- >> 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 >> >> --- >> target/arm/cpu.c | 3 +-- >> target/arm/cpu64.c | 1 - >> 2 files changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/target/arm/cpu.c b/target/arm/cpu.c >> index 32bec156f2..1e9b7a51f2 100644 >> --- a/target/arm/cpu.c >> +++ b/target/arm/cpu.c >> @@ -1244,6 +1244,7 @@ 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); >> + kvm_arm_add_vcpu_properties(obj); > > The name 'kvm_arm_add_vcpu_properties' says nothing about being specific > to the timer. So this is either the wrong place for this function, or the > function is named wrong. I'd say it's the wrong place, because, as the > comment above kvm_arm_add_vcpu_properties() implies, the function is > for all 'kvm-*' prefixed properties, not just timer related ones. It's > true we don't have any others yet, but we will. I have plans to post can't agree more. > kvm-steal-time soon, and despite it also having 'time' in its name, it > shouldn't depend on ARM_FEATURE_GENERIC_TIMER. yes, that would be nice. > > How about the below [untested] patch instead? this patch is much more reasonable for me, thanks for your help. I've tested it and it works fine. > > Thanks, > drew > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 32bec156f2d4..e9084f98ef10 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() should be 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 cbc5c3868fce..778cecc2e6ca 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 4bdbe6dcac07..eef3bbd1cc2c 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) > > >> } >> } >> >> @@ -2029,7 +2030,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 +2183,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; >> -- >> 2.23.0 >> >> >> > > > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-04 7:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-03 2:02 [PATCH v2] target/arm/cpu: adjust virtual time for arm cpu Ying Fang 2020-06-03 2:09 ` Richard Henderson 2020-06-03 8:26 ` Andrew Jones 2020-06-03 14:23 ` Peter Maydell 2020-06-04 7:16 ` Ying Fang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).