From 4d6e4976606279c8784646de006a4b6affb8ec3c Mon Sep 17 00:00:00 2001 From: Fuad Tabba Date: Wed, 3 Jul 2024 13:59:37 +0100 Subject: [PATCH] ANDROID: KVM: arm64: Consolidate allowed and restricted guest cpu feature checks The definitions for allowed and restricted cpu features for protected guests, which are based on feature registers were defined and checked for separately. This resulted in an inconsistency when checking if pointer authentication is allowed, which resulted in trapping it for protected guests despite it being allowed. To avoid this happening in the future, consolidate the definitions into one. Use that new definition to construct the guest view of the feature registers for consistency. Bug: 357781595 Bug: 278749606 Change-Id: Ia290b98a44ca533578add5ab4ca06888eb62e507 Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers") Reported-by: Mostafa Saleh Signed-off-by: Fuad Tabba --- arch/arm64/include/asm/kvm_pkvm.h | 57 +++++++++++------------------- arch/arm64/kvm/arm.c | 2 +- arch/arm64/kvm/hyp/nvhe/pkvm.c | 8 ++--- arch/arm64/kvm/hyp/nvhe/sys_regs.c | 55 +++++++++++++--------------- 4 files changed, 50 insertions(+), 72 deletions(-) diff --git a/arch/arm64/include/asm/kvm_pkvm.h b/arch/arm64/include/asm/kvm_pkvm.h index 14afc1211347..a61711f3eabb 100644 --- a/arch/arm64/include/asm/kvm_pkvm.h +++ b/arch/arm64/include/asm/kvm_pkvm.h @@ -28,11 +28,8 @@ bool pkvm_is_hyp_created(struct kvm *kvm); * machines, depending on the mode KVM is running in and on the type of guest * that is running. * - * The ALLOW masks represent a bitmask of feature fields that are allowed - * without any restrictions as long as they are supported by the system. - * - * The RESTRICT_UNSIGNED masks, if present, represent unsigned fields for - * features that are restricted to support at most the specified feature. + * Each field in the masks represents the highest supported *unsigned* value for + * the feature, if supported by the system. * * If a feature field is not present in either, than it is not supported. * @@ -49,17 +46,7 @@ bool pkvm_is_hyp_created(struct kvm *kvm); * - GICv3(+) system register interface * - Data Independent Timing * - Spectre/Meltdown Mitigation - */ -#define PVM_ID_AA64PFR0_ALLOW (\ - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_FP) | \ - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AdvSIMD) | \ - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC) | \ - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_DIT) | \ - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2) | \ - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3) \ - ) - -/* + * * Restrict to the following *unsigned* features for protected VMs: * - AArch64 guests only (no support for AArch32 guests): * AArch32 adds complexity in trap handling, emulation, condition codes, @@ -67,7 +54,13 @@ bool pkvm_is_hyp_created(struct kvm *kvm); * - RAS (v1) * Supported by KVM */ -#define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\ +#define PVM_ID_AA64PFR0_ALLOW (\ + ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_FP) | \ + ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AdvSIMD) | \ + ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC) | \ + ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_DIT) | \ + ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2) | \ + ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3) | \ SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL0, IMP) | \ SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL1, IMP) | \ SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL2, IMP) | \ @@ -93,20 +86,16 @@ bool pkvm_is_hyp_created(struct kvm *kvm); * - Distinction between Secure and Non-secure Memory * - Mixed-endian at EL0 only * - Non-context synchronizing exception entry and exit + * + * Restrict to the following *unsigned* features for protected VMs: + * - 40-bit IPA + * - 16-bit ASID */ #define PVM_ID_AA64MMFR0_ALLOW (\ ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_BIGEND) | \ ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_SNSMEM) | \ ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_BIGENDEL0) | \ - ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_EXS) \ - ) - -/* - * Restrict to the following *unsigned* features for protected VMs: - * - 40-bit IPA - * - 16-bit ASID - */ -#define PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED (\ + ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_EXS) | \ FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_PARANGE), ID_AA64MMFR0_EL1_PARANGE_40) | \ FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_ASIDBITS), ID_AA64MMFR0_EL1_ASIDBITS_16) \ ) @@ -201,15 +190,6 @@ bool pkvm_is_hyp_created(struct kvm *kvm); ) /* Restrict pointer authentication to the basic version. */ -#define PVM_ID_AA64ISAR1_RESTRICT_UNSIGNED (\ - FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA), ID_AA64ISAR1_EL1_APA_PAuth) | \ - FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API), ID_AA64ISAR1_EL1_API_PAuth) \ - ) - -#define PVM_ID_AA64ISAR2_RESTRICT_UNSIGNED (\ - FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_APA3), ID_AA64ISAR2_EL1_APA3_PAuth) \ - ) - #define PVM_ID_AA64ISAR1_ALLOW (\ ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_DPB) | \ ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_JSCVT) | \ @@ -222,13 +202,16 @@ bool pkvm_is_hyp_created(struct kvm *kvm); ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_SPECRES) | \ ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_BF16) | \ ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_DGH) | \ - ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_I8MM) \ + ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_I8MM) | \ + FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA), ID_AA64ISAR1_EL1_APA_PAuth) | \ + FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API), ID_AA64ISAR1_EL1_API_PAuth) \ ) #define PVM_ID_AA64ISAR2_ALLOW (\ ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_ATS1A)| \ ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_GPA3) | \ - ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_MOPS) \ + ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_MOPS) | \ + FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_APA3), ID_AA64ISAR2_EL1_APA3_PAuth) \ ) /* diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 907a60cc53b8..430695c87655 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -103,7 +103,7 @@ static bool pkvm_ext_allowed(struct kvm *kvm, long ext) PVM_ID_AA64DFR0_ALLOW); case KVM_CAP_ARM_SVE: return FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE), - PVM_ID_AA64PFR0_RESTRICT_UNSIGNED); + PVM_ID_AA64PFR0_ALLOW); case KVM_CAP_ARM_PTRAUTH_ADDRESS: return FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API), PVM_ID_AA64ISAR1_ALLOW) && diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c index 809c119f78b0..1b93913e8ba4 100644 --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c @@ -45,9 +45,9 @@ static void pvm_init_traps_aa64pfr0(struct kvm_vcpu *vcpu) /* Protected KVM does not support AArch32 guests. */ BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), - PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) != ID_AA64PFR0_EL1_EL0_IMP); + PVM_ID_AA64PFR0_ALLOW) != ID_AA64PFR0_EL1_EL0_IMP); BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), - PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) != ID_AA64PFR0_EL1_EL1_IMP); + PVM_ID_AA64PFR0_ALLOW) != ID_AA64PFR0_EL1_EL1_IMP); /* * Linux guests assume support for floating-point and Advanced SIMD. Do @@ -478,8 +478,8 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE), PVM_ID_AA64PFR0_ALLOW)) set_bit(KVM_ARM_VCPU_SVE, allowed_features); - if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API), PVM_ID_AA64ISAR1_RESTRICT_UNSIGNED) && - FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA), PVM_ID_AA64ISAR1_RESTRICT_UNSIGNED)) + if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API), PVM_ID_AA64ISAR1_ALLOW) && + FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA), PVM_ID_AA64ISAR1_ALLOW)) set_bit(KVM_ARM_VCPU_PTRAUTH_ADDRESS, allowed_features); if (FIELD_GET(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPI), PVM_ID_AA64ISAR1_ALLOW) && diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c index 1e05d3d06810..5d93f61c523c 100644 --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c @@ -52,7 +52,6 @@ static void inject_undef64(struct kvm_vcpu *vcpu) /* * Returns the restricted features values of the feature register based on the * limitations in restrict_fields. - * A feature id field value of 0b0000 does not impose any restrictions. * Note: Use only for unsigned feature field values. */ static u64 get_restricted_features_unsigned(u64 sys_reg_val, @@ -85,24 +84,20 @@ static u64 get_restricted_features_unsigned(u64 sys_reg_val, static u64 get_pvm_id_aa64pfr0(const struct kvm_vcpu *vcpu) { - u64 set_mask = 0; - u64 allow_mask = PVM_ID_AA64PFR0_ALLOW; - - set_mask |= get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val, - PVM_ID_AA64PFR0_RESTRICT_UNSIGNED); - - return (id_aa64pfr0_el1_sys_val & allow_mask) | set_mask; + return get_restricted_features_unsigned(id_aa64pfr0_el1_sys_val, + PVM_ID_AA64PFR0_ALLOW); } static u64 get_pvm_id_aa64pfr1(const struct kvm_vcpu *vcpu) { const struct kvm *kvm = (const struct kvm *)kern_hyp_va(vcpu->kvm); - u64 allow_mask = PVM_ID_AA64PFR1_ALLOW; + u64 value = get_restricted_features_unsigned(id_aa64pfr1_el1_sys_val, + PVM_ID_AA64PFR1_ALLOW); if (!kvm_has_mte(kvm)) - allow_mask &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_MTE); + value &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_MTE); - return id_aa64pfr1_el1_sys_val & allow_mask; + return value; } static u64 get_pvm_id_aa64zfr0(const struct kvm_vcpu *vcpu) @@ -162,46 +157,46 @@ static u64 get_pvm_id_aa64isar0(const struct kvm_vcpu *vcpu) static u64 get_pvm_id_aa64isar1(const struct kvm_vcpu *vcpu) { - u64 allow_mask = PVM_ID_AA64ISAR1_ALLOW; + u64 value = get_restricted_features_unsigned(id_aa64isar1_el1_sys_val, + PVM_ID_AA64ISAR1_ALLOW); if (!vcpu_has_ptrauth(vcpu)) - allow_mask &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA) | - ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API) | - ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPA) | - ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPI)); + value &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA) | + ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_API) | + ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPA) | + ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_GPI)); - return id_aa64isar1_el1_sys_val & allow_mask; + return value; } static u64 get_pvm_id_aa64isar2(const struct kvm_vcpu *vcpu) { - u64 allow_mask = PVM_ID_AA64ISAR2_ALLOW; + u64 value = get_restricted_features_unsigned(id_aa64isar2_el1_sys_val, + PVM_ID_AA64ISAR2_ALLOW); if (!vcpu_has_ptrauth(vcpu)) - allow_mask &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_APA3) | - ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_GPA3)); + value &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_APA3) | + ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_GPA3)); - return id_aa64isar2_el1_sys_val & allow_mask; + return id_aa64isar2_el1_sys_val & value; } static u64 get_pvm_id_aa64mmfr0(const struct kvm_vcpu *vcpu) { - u64 set_mask; - - set_mask = get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val, - PVM_ID_AA64MMFR0_RESTRICT_UNSIGNED); - - return (id_aa64mmfr0_el1_sys_val & PVM_ID_AA64MMFR0_ALLOW) | set_mask; + return get_restricted_features_unsigned(id_aa64mmfr0_el1_sys_val, + PVM_ID_AA64MMFR0_ALLOW); } static u64 get_pvm_id_aa64mmfr1(const struct kvm_vcpu *vcpu) { - return id_aa64mmfr1_el1_sys_val & PVM_ID_AA64MMFR1_ALLOW; + return get_restricted_features_unsigned(id_aa64mmfr1_el1_sys_val, + PVM_ID_AA64MMFR1_ALLOW); } static u64 get_pvm_id_aa64mmfr2(const struct kvm_vcpu *vcpu) { - return id_aa64mmfr2_el1_sys_val & PVM_ID_AA64MMFR2_ALLOW; + return get_restricted_features_unsigned(id_aa64mmfr2_el1_sys_val, + PVM_ID_AA64MMFR2_ALLOW); } /* Read a sanitized cpufeature ID register by its encoding */ @@ -276,7 +271,7 @@ static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu, * of AArch32 feature id registers. */ BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), - PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_EL1_EL1_IMP); + PVM_ID_AA64PFR0_ALLOW) > ID_AA64PFR0_EL1_EL1_IMP); return pvm_access_raz_wi(vcpu, p, r); }