| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d499c3e7-cc0f-467a-8401-bc53b70259f7@arm.com>
Date: Mon, 3 Feb 2025 11:18:06 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Steven Price <steven.price@....com>, Gavin Shan <gshan@...hat.com>,
kvm@...r.kernel.org, kvmarm@...ts.linux.dev
Cc: Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
Will Deacon <will@...nel.org>, James Morse <james.morse@....com>,
Oliver Upton <oliver.upton@...ux.dev>, Zenghui Yu <yuzenghui@...wei.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Joey Gouly <joey.gouly@....com>, Alexandru Elisei
<alexandru.elisei@....com>, Christoffer Dall <christoffer.dall@....com>,
Fuad Tabba <tabba@...gle.com>, linux-coco@...ts.linux.dev,
Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>,
Shanker Donthineni <sdonthineni@...dia.com>, Alper Gun
<alpergun@...gle.com>, "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>
Subject: Re: [PATCH v6 12/43] arm64: RME: Allocate/free RECs to match vCPUs
On 30/01/2025 16:40, Steven Price wrote:
> 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@....com>
>>> ---
>>> 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.
nit: kvm is available from the VCPU via vcpu->kvm
Cheers
Suzuki
Powered by blists - more mailing lists