Re: [PATCH 7/7] target/arm/cpu: spe: Enable spe to work with host cpu

[Andrew Jones]

On Fri, Aug 07, 2020 at 08:10:37AM +0000, Haibo Xu wrote:
> Turn on the spe cpu property by default when working with host
> cpu type in KVM mode, i.e. we can now do '-cpu host' to add the 
> vSPE, and '-cpu host,spe=off' to remove it. 

-cpu max with KVM should also enable it by default

> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  target/arm/cpu.c   | 4 ++++
>  target/arm/kvm64.c | 9 +++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 67ab0089fd..42fa99953c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1719,6 +1719,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>          cpu->pmceid1 = 0;
>      }   
>  
> +    if (!cpu->has_spe || !kvm_enabled()) {
> +        unset_feature(env, ARM_FEATURE_SPE);
> +    }

I don't think this should be necessary.

> +
>      if (!arm_feature(env, ARM_FEATURE_EL2)) {
>          /* Disable the hypervisor feature bits in the processor feature
>           * registers if we don't have EL2. These are id_pfr1[15:12] and
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index be045ccc5f..4ea58afc1d 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -679,6 +679,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures 
> *ahcf)
>      features |= 1ULL << ARM_FEATURE_AARCH64;
>      features |= 1ULL << ARM_FEATURE_PMU;
>      features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
> +    features |= 1ULL << ARM_FEATURE_SPE;

No, SPE is not a feature we assume is present in v8.0 CPUs.

> 
>      ahcf->features = features;
> 
> @@ -826,6 +827,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      } else {
>          env->features &= ~(1ULL << ARM_FEATURE_PMU);
>      }
> +    if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_SPE_V1)) {
> +        cpu->has_spe = false;
> +    }
> +    if (cpu->has_spe) {
> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SPE_V1;
> +    } else {
> +        env->features &= ~(1ULL << ARM_FEATURE_SPE);
> +    }

The PMU code above this isn't a good pattern to copy. The SVE code below
is better. SVE uses an ID bit and doesn't do the redundant KVM cap check.
It'd be nice to cleanup the PMU code (with a separate patch) and then add
SPE in a better way.

>      if (cpu_isar_feature(aa64_sve, cpu)) {
>          assert(kvm_arm_sve_supported());
>          cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
> -- 
> 2.17.1
> 

Thanks,
drew