[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is
From: |
Miguel Luis |
Subject: |
Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported |
Date: |
Tue, 28 Feb 2023 12:23:21 +0000 |
Hi Richard,
> On 27 Feb 2023, at 18:24, Richard Henderson <richard.henderson@linaro.org>
> wrote:
>
> On 2/27/23 06:37, Miguel Luis wrote:
>> From: Haibo Xu <haibo.xu@linaro.org>
>> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
>> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b0000.
>> Ref:
>> https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo.xu@linaro.org/
>> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>> ---
>> target/arm/cpu.h | 2 +-
>> target/arm/kvm64.c | 16 ++++++++++++++++
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 9aeed3c848..de2a88b43e 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const
>> ARMISARegisters *id)
>> static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>> {
>> - return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
>> + return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>> }
>
> No, this predicate is testing if EL2 supports AArch32 more.
>
>> @@ -714,6 +723,10 @@ 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;
>> + }
>
> This is the test you want...
>
>> @@ -881,6 +894,9 @@ 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_aa32_el2, cpu)) {
>> + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> + }
>
> ... here.
>
> While you could add a new isar_feature predicate for EL2 supported in AArch64
> mode, the feature test is equivalent and good enough, and is more obviously
> tied to the required KVM support.
>
>
Thank you for the explanation. I will take it into consideration on the next
version.
Miguel
> r~
- [RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support, Miguel Luis, 2023/02/27
- [RFC PATCH 1/5] linux-headers: [kvm, arm64] add the necessary definitions to match host kernel, Miguel Luis, 2023/02/27
- [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ, Miguel Luis, 2023/02/27
- [RFC PATCH 3/5] target/arm/kvm: add helper to detect EL2 when using KVM, Miguel Luis, 2023/02/27
- [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported, Miguel Luis, 2023/02/27
- [RFC PATCH 5/5] arm/virt: provide virtualization extensions to the guest, Miguel Luis, 2023/02/27