| Message ID | 26b4ace9ea3c5b43d14802d6fc5ceea90befbcc8.1599549462.git.haibo.xu@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Add vSPE support to KVM guest | expand |
On Tue, Sep 08, 2020 at 08:13:28AM +0000, Haibo Xu wrote: > Turn on the spe cpu property by default if host cpu > support it, i.e. we can now do '-cpu max|host' to add > the vSPE, and '-cpu max|host,spe=off' to remove it. > > Signed-off-by: Haibo Xu <haibo.xu@linaro.org> > --- > target/arm/cpu.c | 3 +++ > target/arm/cpu.h | 2 ++ > target/arm/cpu64.c | 7 ++++++- > target/arm/kvm64.c | 12 ++++++++++++ > 4 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 786cc6134c..58f12d6eb5 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -2271,6 +2271,9 @@ static void arm_host_initfn(Object *obj) > kvm_arm_set_cpu_features_from_host(cpu); > if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > aarch64_add_sve_properties(obj); > + > + cpu->has_spe = ON_OFF_AUTO_AUTO; > + aarch64_add_spe_properties(obj); Why not put the assignment of has_spe into aarch64_add_spe_properties()? > } > arm_cpu_post_init(obj); > } > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index 395a1e5df8..5a3ea876c8 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1040,6 +1040,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); > void aarch64_sve_change_el(CPUARMState *env, int old_el, > int new_el, bool el0_a64); > void aarch64_add_sve_properties(Object *obj); > +void aarch64_add_spe_properties(Object *obj); > > /* > * SVE registers are encoded in KVM's memory in an endianness-invariant format. > @@ -1071,6 +1072,7 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o, > int n, bool a) > { } > static inline void aarch64_add_sve_properties(Object *obj) { } > +static inline void aarch64_add_spe_properties(Object *obj) { } > #endif > > #if !defined(CONFIG_TCG) > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 4997c4a3c0..d38c55e2ca 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -621,6 +621,11 @@ static void arm_spe_set(Object *obj, bool value, Error **errp) > ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > } > > +void aarch64_add_spe_properties(Object *obj) > +{ > + object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set); > +} > + > /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host); > * otherwise, a CPU with as many features enabled as our emulation supports. > * The version of '-cpu max' for qemu-system-arm is defined in cpu.c; > @@ -772,7 +777,7 @@ static void aarch64_max_initfn(Object *obj) > cpu_max_set_sve_max_vq, NULL, NULL); > > cpu->has_spe = ON_OFF_AUTO_AUTO; > - object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set); > + aarch64_add_spe_properties(obj); If TCG doesn't support this cpu feature then this should be in the kvm_enabled() part of this function. > } > > static const ARMCPUInfo aarch64_cpus[] = { > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index 5a2032fc9e..3f0a09c05b 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -515,6 +515,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > */ > int fdarray[3]; > bool sve_supported; > + bool spe_supported; > uint64_t features = 0; > uint64_t t; > int err; > @@ -655,6 +656,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > } > > sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0; > + spe_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, > + KVM_CAP_ARM_SPE_V1) > 0; > > kvm_arm_destroy_scratch_host_vcpu(fdarray); > > @@ -668,6 +671,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); > ahcf->isar.id_aa64pfr0 = t; > } > + if (!spe_supported) { > + t = ahcf->isar.id_aa64dfr0; > + t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, 0); > + ahcf->isar.id_aa64dfr0 = t; > + } > If this works then there's a bug with the KVM implementation. get-one-reg shouldn't expose SPE unless userspace has enabled it, which we're not doing on the scratch vcpu. That's why we set, not clear, the ID bits for SVE. > /* > * We can assume any KVM supporting CPU is at least a v8 > @@ -830,6 +838,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > assert(kvm_arm_sve_supported()); > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; > } > + if (cpu_isar_feature(aa64_spe, cpu)) { > + assert(kvm_arm_spe_supported()); > + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1; > + } > > /* Do KVM_ARM_VCPU_INIT ioctl */ > ret = kvm_arm_vcpu_init(cs); > -- > 2.17.1 > Thanks, drew
On Tue, 8 Sep 2020 at 19:33, Andrew Jones <drjones@redhat.com> wrote: > > On Tue, Sep 08, 2020 at 08:13:28AM +0000, Haibo Xu wrote: > > Turn on the spe cpu property by default if host cpu > > support it, i.e. we can now do '-cpu max|host' to add > > the vSPE, and '-cpu max|host,spe=off' to remove it. > > > > Signed-off-by: Haibo Xu <haibo.xu@linaro.org> > > --- > > target/arm/cpu.c | 3 +++ > > target/arm/cpu.h | 2 ++ > > target/arm/cpu64.c | 7 ++++++- > > target/arm/kvm64.c | 12 ++++++++++++ > > 4 files changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > > index 786cc6134c..58f12d6eb5 100644 > > --- a/target/arm/cpu.c > > +++ b/target/arm/cpu.c > > @@ -2271,6 +2271,9 @@ static void arm_host_initfn(Object *obj) > > kvm_arm_set_cpu_features_from_host(cpu); > > if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > > aarch64_add_sve_properties(obj); > > + > > + cpu->has_spe = ON_OFF_AUTO_AUTO; > > + aarch64_add_spe_properties(obj); > > Why not put the assignment of has_spe into aarch64_add_spe_properties()? > Yes, it could be. Will fix it in v3. > > } > > arm_cpu_post_init(obj); > > } > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > > index 395a1e5df8..5a3ea876c8 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -1040,6 +1040,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); > > void aarch64_sve_change_el(CPUARMState *env, int old_el, > > int new_el, bool el0_a64); > > void aarch64_add_sve_properties(Object *obj); > > +void aarch64_add_spe_properties(Object *obj); > > > > /* > > * SVE registers are encoded in KVM's memory in an endianness-invariant format. > > @@ -1071,6 +1072,7 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o, > > int n, bool a) > > { } > > static inline void aarch64_add_sve_properties(Object *obj) { } > > +static inline void aarch64_add_spe_properties(Object *obj) { } > > #endif > > > > #if !defined(CONFIG_TCG) > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > > index 4997c4a3c0..d38c55e2ca 100644 > > --- a/target/arm/cpu64.c > > +++ b/target/arm/cpu64.c > > @@ -621,6 +621,11 @@ static void arm_spe_set(Object *obj, bool value, Error **errp) > > ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; > > } > > > > +void aarch64_add_spe_properties(Object *obj) > > +{ > > + object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set); > > +} > > + > > /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host); > > * otherwise, a CPU with as many features enabled as our emulation supports. > > * The version of '-cpu max' for qemu-system-arm is defined in cpu.c; > > @@ -772,7 +777,7 @@ static void aarch64_max_initfn(Object *obj) > > cpu_max_set_sve_max_vq, NULL, NULL); > > > > cpu->has_spe = ON_OFF_AUTO_AUTO; > > - object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set); > > + aarch64_add_spe_properties(obj); > > If TCG doesn't support this cpu feature then this should be in the > kvm_enabled() part of this function. > > Will fix it in v3. > > } > > > > static const ARMCPUInfo aarch64_cpus[] = { > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > > index 5a2032fc9e..3f0a09c05b 100644 > > --- a/target/arm/kvm64.c > > +++ b/target/arm/kvm64.c > > @@ -515,6 +515,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > > */ > > int fdarray[3]; > > bool sve_supported; > > + bool spe_supported; > > uint64_t features = 0; > > uint64_t t; > > int err; > > @@ -655,6 +656,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > > } > > > > sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0; > > + spe_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, > > + KVM_CAP_ARM_SPE_V1) > 0; > > > > kvm_arm_destroy_scratch_host_vcpu(fdarray); > > > > @@ -668,6 +671,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > > t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); > > ahcf->isar.id_aa64pfr0 = t; > > } > > + if (!spe_supported) { > > + t = ahcf->isar.id_aa64dfr0; > > + t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, 0); > > + ahcf->isar.id_aa64dfr0 = t; > > + } > > > > If this works then there's a bug with the KVM implementation. get-one-reg > shouldn't expose SPE unless userspace has enabled it, which we're not > doing on the scratch vcpu. That's why we set, not clear, the ID bits for > SVE. > > You are right. I have printed the returned field, and it's 0. Thanks for pointing this out, I will fix it in v3. Regards, Haibo > > /* > > * We can assume any KVM supporting CPU is at least a v8 > > @@ -830,6 +838,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > > assert(kvm_arm_sve_supported()); > > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; > > } > > + if (cpu_isar_feature(aa64_spe, cpu)) { > > + assert(kvm_arm_spe_supported()); > > + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1; > > + } > > > > /* Do KVM_ARM_VCPU_INIT ioctl */ > > ret = kvm_arm_vcpu_init(cs); > > -- > > 2.17.1 > > > > Thanks, > drew >
diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 786cc6134c..58f12d6eb5 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -2271,6 +2271,9 @@ static void arm_host_initfn(Object *obj) kvm_arm_set_cpu_features_from_host(cpu); if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { aarch64_add_sve_properties(obj); + + cpu->has_spe = ON_OFF_AUTO_AUTO; + aarch64_add_spe_properties(obj); } arm_cpu_post_init(obj); } diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 395a1e5df8..5a3ea876c8 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1040,6 +1040,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); void aarch64_sve_change_el(CPUARMState *env, int old_el, int new_el, bool el0_a64); void aarch64_add_sve_properties(Object *obj); +void aarch64_add_spe_properties(Object *obj); /* * SVE registers are encoded in KVM's memory in an endianness-invariant format. @@ -1071,6 +1072,7 @@ static inline void aarch64_sve_change_el(CPUARMState *env, int o, int n, bool a) { } static inline void aarch64_add_sve_properties(Object *obj) { } +static inline void aarch64_add_spe_properties(Object *obj) { } #endif #if !defined(CONFIG_TCG) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 4997c4a3c0..d38c55e2ca 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -621,6 +621,11 @@ static void arm_spe_set(Object *obj, bool value, Error **errp) ARM_CPU(obj)->has_spe = value ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF; } +void aarch64_add_spe_properties(Object *obj) +{ + object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set); +} + /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host); * otherwise, a CPU with as many features enabled as our emulation supports. * The version of '-cpu max' for qemu-system-arm is defined in cpu.c; @@ -772,7 +777,7 @@ static void aarch64_max_initfn(Object *obj) cpu_max_set_sve_max_vq, NULL, NULL); cpu->has_spe = ON_OFF_AUTO_AUTO; - object_property_add_bool(obj, "spe", arm_spe_get, arm_spe_set); + aarch64_add_spe_properties(obj); } static const ARMCPUInfo aarch64_cpus[] = { diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 5a2032fc9e..3f0a09c05b 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -515,6 +515,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) */ int fdarray[3]; bool sve_supported; + bool spe_supported; uint64_t features = 0; uint64_t t; int err; @@ -655,6 +656,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) } sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0; + spe_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, + KVM_CAP_ARM_SPE_V1) > 0; kvm_arm_destroy_scratch_host_vcpu(fdarray); @@ -668,6 +671,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); ahcf->isar.id_aa64pfr0 = t; } + if (!spe_supported) { + t = ahcf->isar.id_aa64dfr0; + t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, 0); + ahcf->isar.id_aa64dfr0 = t; + } /* * We can assume any KVM supporting CPU is at least a v8 @@ -830,6 +838,10 @@ int kvm_arch_init_vcpu(CPUState *cs) assert(kvm_arm_sve_supported()); cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; } + if (cpu_isar_feature(aa64_spe, cpu)) { + assert(kvm_arm_spe_supported()); + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1; + } /* Do KVM_ARM_VCPU_INIT ioctl */ ret = kvm_arm_vcpu_init(cs);
Turn on the spe cpu property by default if host cpu support it, i.e. we can now do '-cpu max|host' to add the vSPE, and '-cpu max|host,spe=off' to remove it. Signed-off-by: Haibo Xu <haibo.xu@linaro.org> --- target/arm/cpu.c | 3 +++ target/arm/cpu.h | 2 ++ target/arm/cpu64.c | 7 ++++++- target/arm/kvm64.c | 12 ++++++++++++ 4 files changed, 23 insertions(+), 1 deletion(-)