On 05/07/2020 03:50 PM, Marc Zyngier wrote:
> On Thu, 7 May 2020 11:49:47 +0530
> Anshuman Khandual <anshuman.khandual@xxxxxxx> wrote:
>
> Hi Anshuman,
Hi Marc,
>
>> This validates KVM capabilities like VMID width, IPA range for hotplug CPU
>> against system finalized values. While here, it factors out get_vmid_bits()
>> for general use and also defines ID_AA64MMFR0_PARANGE_MASK.
>
> nit: these are not KVM-specific capabilities, but general
> virtualization features.
Sure, will change as (s/kvm/hyp) instead and update the commit
message here.
>
>>
>> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
>> Cc: Will Deacon <will@xxxxxxxxxx>
>> Cc: Marc Zyngier <maz@xxxxxxxxxx>
>> Cc: Mark Rutland <mark.rutland@xxxxxxx>
>> Cc: James Morse <james.morse@xxxxxxx>
>> Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> Cc: kvmarm@xxxxxxxxxxxxxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>
>> Suggested-by: Suzuki Poulose <suzuki.poulose@xxxxxxx>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
>> ---
>> arch/arm64/include/asm/cpufeature.h | 22 +++++++++++++++++++
>> arch/arm64/include/asm/kvm_mmu.h | 2 +-
>> arch/arm64/include/asm/sysreg.h | 1 +
>> arch/arm64/kernel/cpufeature.c | 2 ++
>> arch/arm64/kvm/reset.c | 33 +++++++++++++++++++++++++++--
>> 5 files changed, 57 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>> index afe08251ff95..6808a2091de4 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -745,6 +745,28 @@ static inline bool cpu_has_hw_af(void)
>> extern bool cpu_has_amu_feat(int cpu);
>> #endif
>>
>> +static inline unsigned int get_vmid_bits(u64 mmfr1)
>> +{
>> + int vmid_bits;
>> +
>> + vmid_bits = cpuid_feature_extract_unsigned_field(mmfr1,
>> + ID_AA64MMFR1_VMIDBITS_SHIFT);
>> + if (vmid_bits == ID_AA64MMFR1_VMIDBITS_16)
>> + return 16;
>> +
>> + /*
>> + * Return the default here even if any reserved
>> + * value is fetched from the system register.
>> + */
>> + return 8;
>> +}
>> +
>> +#ifdef CONFIG_KVM_ARM_HOST
>> +void verify_kvm_capabilities(void);
>> +#else
>> +static inline void verify_kvm_capabilities(void) { }
>> +#endif
>> +
>> #endif /* __ASSEMBLY__ */
>>
>> #endif
>> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
>> index 30b0e8d6b895..a7137e144b97 100644
>> --- a/arch/arm64/include/asm/kvm_mmu.h
>> +++ b/arch/arm64/include/asm/kvm_mmu.h
>> @@ -416,7 +416,7 @@ static inline unsigned int kvm_get_vmid_bits(void)
>> {
>> int reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
>>
>> - return (cpuid_feature_extract_unsigned_field(reg,
>> ID_AA64MMFR1_VMIDBITS_SHIFT) == 2) ? 16 : 8;
>> + return get_vmid_bits(reg);
>> }
>>
>> /*
>> diff --git a/arch/arm64/include/asm/sysreg.h
>> b/arch/arm64/include/asm/sysreg.h index c4ac0ac25a00..3510a4668970
>> 100644 --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -705,6 +705,7 @@
>> #define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1
>> #define ID_AA64MMFR0_PARANGE_48 0x5
>> #define ID_AA64MMFR0_PARANGE_52 0x6
>> +#define ID_AA64MMFR0_PARANGE_MASK 0x7
>>
>> #ifdef CONFIG_ARM64_PA_BITS_52
>> #define ID_AA64MMFR0_PARANGE_MAX ID_AA64MMFR0_PARANGE_52
>> diff --git a/arch/arm64/kernel/cpufeature.c
>> b/arch/arm64/kernel/cpufeature.c index 9fac745aa7bb..041dd610b0f8
>> 100644 --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -2206,6 +2206,8 @@ static void verify_local_cpu_capabilities(void)
>>
>> if (system_supports_sve())
>> verify_sve_features();
>> +
>> + verify_kvm_capabilities();
>
> You should only check this if booted at EL2. Otherwise, it doesn't
> really matter.
Sure, will first check on is_hyp_mode_available() before calling into
verify_kvm_capabilities().
>
>> }
>>
>> void check_local_cpu_capabilities(void)
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index 30b7ea680f66..1eebcc2a8396 100644
>> --- a/arch/arm64/kvm/reset.c
>> +++ b/arch/arm64/kvm/reset.c
>> @@ -340,11 +340,39 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
>> return ret;
>> }
>>
>> +void verify_kvm_capabilities(void)
>
> This is really in the wrong file. reset.c is supposed to contain things
> that are meaningful to the guest reset state. This clearly isn't. I'd
> suggest you add an accessor for the kvm_ipa_limit variable, and keep
> the function next to the other verify_* functions in cpufeature.c.
Sure, will do.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
