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 <fangyi...@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 <drjo...@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 <drjo...@redhat.com> > > > > Thanks, > > drew > > > > > Signed-off-by: Ying Fang <fangyi...@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 > > > > > > > > > > . > > >