On Fri, 7 Aug 2020 at 16:28, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>
> Hi Haibo,
>
> On 8/7/20 10:10 AM, Haibo Xu wrote:
> > Adds a spe=[on/off] option to enable/disable vSPE support in
> > guest vCPU. Note this option is only available for "-cpu host"
> > with KVM mode, and default value is on.
> >
> > Signed-off-by: Haibo Xu <haibo...@linaro.org>
> > ---
> > target/arm/cpu.c | 28 ++++++++++++++++++++++++++++
> > target/arm/cpu.h | 3 +++
> > 2 files changed, 31 insertions(+)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 111579554f..40768b4d19 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1122,6 +1122,29 @@ static void arm_set_pmu(Object *obj, bool value,
> > Error **errp)
> > cpu->has_pmu = value;
> > }
> >
> > +static bool arm_get_spe(Object *obj, Error **errp)
> > +{
> > + ARMCPU *cpu = ARM_CPU(obj);
> > +
> > + return cpu->has_spe;
> > +}
> > +
> > +static void arm_set_spe(Object *obj, bool value, Error **errp)
> > +{
> > + ARMCPU *cpu = ARM_CPU(obj);
> > +
> > + if (value) {
> > + if (kvm_enabled() && !kvm_arm_spe_supported()) {
> > + error_setg(errp, "'spe' feature not supported by KVM on this
> > host");
> > + return;
> > + }
> > + set_feature(&cpu->env, ARM_FEATURE_SPE);
> > + } else {
> > + unset_feature(&cpu->env, ARM_FEATURE_SPE);
> > + }
> > + cpu->has_spe = value;
> > +}
> > +
> > unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
> > {
> > /*
> > @@ -1195,6 +1218,11 @@ void arm_cpu_post_init(Object *obj)
> > object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
> > }
> >
> > + if (arm_feature(&cpu->env, ARM_FEATURE_SPE)) {
> > + cpu->has_spe = true;
>
> Being a profiling feature, are you sure you want it to be ON by default?
>
> I'd expect the opposite, either being turned on via command line at
> startup or by a management layer at runtime, when someone is ready
> to record the perf events and analyze them.
>
I'm not sure whether it's proper to follow the 'pmu' setting here
which has it on by default.
To be honest, I also prefer to turn it off by default(Please refer to
the comments in the cover letter).
> > + object_property_add_bool(obj, "spe", arm_get_spe, arm_set_spe);
> > + }
> > +
> > /*
> > * Allow user to turn off VFP and Neon support, but only for TCG --
> > * KVM does not currently allow us to lie to the guest about its
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 9e8ed423ea..fe0ac14386 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -822,6 +822,8 @@ struct ARMCPU {
> > bool has_el3;
> > /* CPU has PMU (Performance Monitor Unit) */
> > bool has_pmu;
> > + /* CPU has SPE (Statistical Profiling Extension) */
> > + bool has_spe;
> > /* CPU has VFP */
> > bool has_vfp;
> > /* CPU has Neon */
> > @@ -1959,6 +1961,7 @@ enum arm_features {
> > ARM_FEATURE_VBAR, /* has cp15 VBAR */
> > ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
> > ARM_FEATURE_M_MAIN, /* M profile Main Extension */
> > + ARM_FEATURE_SPE, /* has SPE support */
> > };
> >
> > static inline int arm_feature(CPUARMState *env, int feature)
> >
>
