On Tue, 15 Sep 2020 at 14:11, Andrew Jones <drjo...@redhat.com> wrote:
>
> On Tue, Sep 15, 2020 at 03:11:39AM +0000, Haibo Xu wrote:
> > Adds a spe=[on/off] option to enable/disable vSPE support in
> > guest vCPU.
> >
> > Reviewed-by: Andrew Jones <drjo...@redhat.com>
> > Signed-off-by: Haibo Xu <haibo...@linaro.org>
> > ---
> > target/arm/cpu.c | 6 ++++++
> > target/arm/cpu.h | 13 ++++++++++++
> > target/arm/cpu64.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 71 insertions(+)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index c179e0752d..f211958eaa 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1310,6 +1310,12 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error
> > **errp)
> > error_propagate(errp, local_err);
> > return;
> > }
> > +
> > + arm_cpu_spe_finalize(cpu, &local_err);
> > + if (local_err != NULL) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > }
> > }
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index a1c7d8ebae..baf2bbcee8 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -24,6 +24,7 @@
> > #include "hw/registerfields.h"
> > #include "cpu-qom.h"
> > #include "exec/cpu-defs.h"
> > +#include "qapi/qapi-types-common.h"
> >
> > /* ARM processors have a weak memory model */
> > #define TCG_GUEST_DEFAULT_MO (0)
> > @@ -196,9 +197,11 @@ typedef struct {
> > #ifdef TARGET_AARCH64
> > # define ARM_MAX_VQ 16
> > void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
> > +void arm_cpu_spe_finalize(ARMCPU *cpu, Error **errp);
> > #else
> > # define ARM_MAX_VQ 1
> > static inline void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp) { }
> > +static inline void arm_cpu_spe_finalize(ARMCPU *cpu, Error **errp) { }
> > #endif
> >
> > typedef struct ARMVectorReg {
> > @@ -829,6 +832,8 @@ struct ARMCPU {
> > bool has_el3;
> > /* CPU has PMU (Performance Monitor Unit) */
> > bool has_pmu;
> > + /* CPU has SPE (Statistical Profiling Extension) */
> > + OnOffAuto has_spe;
> > /* CPU has VFP */
> > bool has_vfp;
> > /* CPU has Neon */
> > @@ -3869,6 +3874,14 @@ static inline bool isar_feature_aa64_pmu_8_4(const
> > ARMISARegisters *id)
> > FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMUVER) != 0xf;
> > }
> >
> > +/*
> > + * Currently we don't differentiate between the ARMv8.2-SPE and
> > ARMv8.3-SPE.
> > + */
> > +static inline bool isar_feature_aa64_spe(const ARMISARegisters *id)
> > +{
> > + return FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMSVER) != 0;
> > +}
> > +
> > static inline bool isar_feature_aa64_rcpc_8_3(const ARMISARegisters *id)
> > {
> > return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, LRCPC) != 0;
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 3c2b3d9599..4997c4a3c0 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -572,6 +572,55 @@ void aarch64_add_sve_properties(Object *obj)
> > }
> > }
> >
> > +void arm_cpu_spe_finalize(ARMCPU *cpu, Error **errp)
> > +{
> > + uint64_t t;
> > + uint32_t value = 0;
> > +
> > + if (cpu->has_spe == ON_OFF_AUTO_AUTO) {
> > + if (kvm_enabled() && kvm_arm_spe_supported()) {
> > + cpu->has_spe = ON_OFF_AUTO_ON;
> > + } else {
> > + cpu->has_spe = ON_OFF_AUTO_OFF;
> > + }
> > + } else if (cpu->has_spe == ON_OFF_AUTO_ON) {
> > + if (!kvm_enabled() || !kvm_arm_spe_supported()) {
> > + error_setg(errp, "'spe' cannot be enabled on this host");
> > + return;
> > + }
> > + }
> > +
> > + /*
> > + * According to the ARM ARM, the ID_AA64DFR0[PMSVER] currently
> > + * support 3 values:
> > + *
> > + * 0b0000: SPE not implemented
> > + * 0b0001: ARMv8.2-SPE implemented
> > + * 0b0010: ARMv8.3-SPE implemented
> > + *
> > + * But the kernel KVM API didn't expose all these 3 values, and
> > + * we can only get whether the SPE feature is supported or not.
> > + * So here we just set the PMSVER to 1 if this feature was supported.
> > + */
> > + if (cpu->has_spe == ON_OFF_AUTO_ON) {
> > + value = 1;
> > + }
> > +
> > + t = cpu->isar.id_aa64dfr0;
> > + t = FIELD_DP64(t, ID_AA64DFR0, PMSVER, value);
> > + cpu->isar.id_aa64dfr0 = t;
> > +}
> > +
> > +static bool arm_spe_get(Object *obj, Error **errp)
> > +{
> > + return ARM_CPU(obj)->has_spe != ON_OFF_AUTO_OFF;
> > +}
> > +
> > +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;
> > +}
> > +
> > /* -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;
> > @@ -721,6 +770,9 @@ static void aarch64_max_initfn(Object *obj)
> > aarch64_add_sve_properties(obj);
> > object_property_add(obj, "sve-max-vq", "uint32",
> > cpu_max_get_sve_max_vq,
> > 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);
> > }
>
> The cover-letter says we're only providing the 'spe' property for KVM
> enabled guests, but we're adding it here in the common part of the
> max init function. It should be added in the kvm_enabled() part only.
>
> Thanks,
> drew
>
Yes, It should be in the kvm_enabled() part.
Will fix it in the next version.
Thanks,
Haibo
> >
> > static const ARMCPUInfo aarch64_cpus[] = {
> > --
> > 2.17.1
> >
> >
>
