Re: [RFC v2 4/5] target/arm: Enable feature ARM_FEATURE_EL2 if EL2 is supported

[Eric Auger <eric.auger@redhat.com>]

Hi Peter,

On 3/5/24 17:49, Peter Maydell wrote:
> On Fri, 9 Feb 2024 at 16:00, Eric Auger <eric.auger@redhat.com> wrote:
>> From: Haibo Xu <haibo.xu@linaro.org>
>>
>> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
>> In case the host does support NV, expose the feature.
>>
>> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - remove isar_feature_aa64_aa32_el2 modif in target/arm/cpu.h
>>   [Richard] and use el2_supported in kvm_arch_init_vcpu
>> ---
>>  target/arm/kvm.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index 0996866afe..a08bc68a3f 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -238,6 +238,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>       */
>>      int fdarray[3];
>>      bool sve_supported;
>> +    bool el2_supported;
>>      bool pmu_supported = false;
>>      uint64_t features = 0;
>>      int err;
>> @@ -268,6 +269,14 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>          init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
>>      }
>>
>> +    /*
>> +     * Ask for EL2 if supported.
>> +     */
>> +    el2_supported = kvm_arm_el2_supported();
>> +    if (el2_supported) {
>> +        init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> +    }
>> +
>>      /*
>>       * Ask for Pointer Authentication if supported, so that we get
>>       * the unsanitized field values for AA64ISAR1_EL1.
>> @@ -449,6 +458,10 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>      features |= 1ULL << ARM_FEATURE_PMU;
>>      features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>>
>> +    if (el2_supported) {
>> +        features |= 1ULL << ARM_FEATURE_EL2;
>> +    }
>> +
>>      ahcf->features = features;
>>
>>      return true;
>> @@ -1912,6 +1925,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>          cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
>>                                        1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>>      }
>> +    if (kvm_arm_el2_supported()) {
>> +        cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> +    }
>>
>>      /* Do KVM_ARM_VCPU_INIT ioctl */
>>      ret = kvm_arm_vcpu_init(cpu);
> Am I reading this right that if the kernel supports FEAT_NV
> then we will always ask for a vCPU with that feature?
> Is that a good idea, or should we arrange to only do it if
> the user uses the 'virtualization=on' option to -M virt ?
> (Or does that happen already in some way I'm not seeing?)
yes you're right, if the host supports it, the feature is currently set
on the vcpu. I am not totaly clear under which conditions the features
shall be instantiated in the scratch VM and when the host passthrough
model shall be altered by machine option.  

Thanks

Eric 
>
> thanks
> -- PMM
>