On 29/01/2025 04:50, Gavin Shan wrote: > On 12/13/24 1:55 AM, Steven Price wrote: >> The RMM maintains a data structure known as the Realm Execution Context >> (or REC). It is similar to struct kvm_vcpu and tracks the state of the >> virtual CPUs. KVM must delegate memory and request the structures are >> created when vCPUs are created, and suitably tear down on destruction. >> >> RECs must also be supplied with addition pages - auxiliary (or AUX) >> granules - for storing the larger registers state (e.g. for SVE). The >> number of AUX granules for a REC depends on the parameters with which >> the Realm was created - the RMM makes this information available via the >> RMI_REC_AUX_COUNT call performed after creating the Realm Descriptor >> (RD). >> >> Note that only some of register state for the REC can be set by KVM, the >> rest is defined by the RMM (zeroed). The register state then cannot be >> changed by KVM after the REC is created (except when the guest >> explicitly requests this e.g. by performing a PSCI call). The RMM also >> requires that the VMM creates RECs in ascending order of the MPIDR. >> >> See Realm Management Monitor specification (DEN0137) for more >> information: >> https://developer.arm.com/documentation/den0137/ >> >> Signed-off-by: Steven Price <steven.price@xxxxxxx> >> --- >> Changes since v5: >> * Separate the concept of vcpu_is_rec() and >> kvm_arm_vcpu_rec_finalized() by using the KVM_ARM_VCPU_REC feature as >> the indication that the VCPU is a REC. >> Changes since v2: >> * Free rec->run earlier in kvm_destroy_realm() and adapt to previous >> patches. >> --- >> arch/arm64/include/asm/kvm_emulate.h | 7 ++ >> arch/arm64/include/asm/kvm_host.h | 3 + >> arch/arm64/include/asm/kvm_rme.h | 18 ++++ >> arch/arm64/kvm/arm.c | 9 ++ >> arch/arm64/kvm/reset.c | 11 ++ >> arch/arm64/kvm/rme.c | 144 +++++++++++++++++++++++++++ >> 6 files changed, 192 insertions(+) >> >> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/ >> include/asm/kvm_emulate.h >> index 27f54a7778aa..ec2b6d9c9c07 100644 >> --- a/arch/arm64/include/asm/kvm_emulate.h >> +++ b/arch/arm64/include/asm/kvm_emulate.h >> @@ -722,7 +722,14 @@ static inline bool kvm_realm_is_created(struct >> kvm *kvm) >> static inline bool vcpu_is_rec(struct kvm_vcpu *vcpu) >> { >> + if (static_branch_unlikely(&kvm_rme_is_available)) >> + return vcpu_has_feature(vcpu, KVM_ARM_VCPU_REC); >> return false; >> } >> > > It seems the check on kvm_rme_is_available is unnecessary because > KVM_ARM_VCPU_REC > is possible to be true only when kvm_rme_is_available is true. Similar to a previous patch - the check of the static key is to avoid overhead on systems without RME. >> +static inline bool kvm_arm_vcpu_rec_finalized(struct kvm_vcpu *vcpu) >> +{ >> + return vcpu->arch.rec.mpidr != INVALID_HWID; >> +} >> + >> #endif /* __ARM64_KVM_EMULATE_H__ */ > > I would suggest to rename to kvm_arm_rec_finalized() since vCPU and REC are > similar objects at the same level. It'd better to avoid duplicate object > name reference in the function name. Ack >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/ >> asm/kvm_host.h >> index 8482638dce3b..220195c727ef 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -789,6 +789,9 @@ struct kvm_vcpu_arch { >> /* Per-vcpu CCSIDR override or NULL */ >> u32 *ccsidr; >> + >> + /* Realm meta data */ >> + struct realm_rec rec; >> }; >> /* >> diff --git a/arch/arm64/include/asm/kvm_rme.h b/arch/arm64/include/ >> asm/kvm_rme.h >> index 32bdedf1d866..62d4a63d3035 100644 >> --- a/arch/arm64/include/asm/kvm_rme.h >> +++ b/arch/arm64/include/asm/kvm_rme.h >> @@ -6,6 +6,7 @@ >> #ifndef __ASM_KVM_RME_H >> #define __ASM_KVM_RME_H >> +#include <asm/rmi_smc.h> >> #include <uapi/linux/kvm.h> >> /** >> @@ -65,6 +66,21 @@ struct realm { >> unsigned int ia_bits; >> }; >> +/** >> + * struct realm_rec - Additional per VCPU data for a Realm >> + * >> + * @mpidr: MPIDR (Multiprocessor Affinity Register) value to identify >> this VCPU >> + * @rec_page: Kernel VA of the RMM's private page for this REC >> + * @aux_pages: Additional pages private to the RMM for this REC >> + * @run: Kernel VA of the RmiRecRun structure shared with the RMM >> + */ >> +struct realm_rec { >> + unsigned long mpidr; >> + void *rec_page; >> + struct page *aux_pages[REC_PARAMS_AUX_GRANULES]; >> + struct rec_run *run; >> +}; >> + >> void kvm_init_rme(void); >> u32 kvm_realm_ipa_limit(void); >> @@ -72,6 +88,8 @@ int kvm_realm_enable_cap(struct kvm *kvm, struct >> kvm_enable_cap *cap); >> int kvm_init_realm_vm(struct kvm *kvm); >> void kvm_destroy_realm(struct kvm *kvm); >> void kvm_realm_destroy_rtts(struct kvm *kvm, u32 ia_bits); >> +int kvm_create_rec(struct kvm_vcpu *vcpu); >> +void kvm_destroy_rec(struct kvm_vcpu *vcpu); >> #define RMM_RTT_BLOCK_LEVEL 2 >> #define RMM_RTT_MAX_LEVEL 3 >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 73016e1e0067..2d97147651be 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -525,6 +525,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) >> /* Force users to call KVM_ARM_VCPU_INIT */ >> vcpu_clear_flag(vcpu, VCPU_INITIALIZED); >> + vcpu->arch.rec.mpidr = INVALID_HWID; >> + >> vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO; >> /* Set up the timer */ >> @@ -1467,6 +1469,9 @@ static unsigned long >> system_supported_vcpu_features(void) >> if (!cpus_have_final_cap(ARM64_HAS_NESTED_VIRT)) >> clear_bit(KVM_ARM_VCPU_HAS_EL2, &features); >> + if (!static_branch_unlikely(&kvm_rme_is_available)) >> + clear_bit(KVM_ARM_VCPU_REC, &features); >> + >> return features; >> } >> @@ -1506,6 +1511,10 @@ static int >> kvm_vcpu_init_check_features(struct kvm_vcpu *vcpu, >> if (test_bit(KVM_ARM_VCPU_HAS_EL2, &features)) >> return -EINVAL; >> + /* RME is incompatible with AArch32 */ >> + if (test_bit(KVM_ARM_VCPU_REC, &features)) >> + return -EINVAL; >> + >> return 0; >> } >> > > One corner case seems missed in kvm_vcpu_init_check_features(). It's > allowed to > initialize a vCPU with REC feature even kvm_is_realm(kvm) is false. > Hopefully, > I didn't miss something. Ah, yes good point. I'll pass a kvm pointer to kvm_vcpu_init_check_features() and use kvm_is_realm() in there. Thanks, Steve > [...] > > Thanks, > Gavin >
