| Message ID | 20241212155610.76522-7-steven.price@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Support for Arm CCA in KVM | expand |
Steven Price <steven.price@arm.com> writes: > +static int rmi_check_version(void) > +{ > + struct arm_smccc_res res; > + int version_major, version_minor; > + unsigned long host_version = RMI_ABI_VERSION(RMI_ABI_MAJOR_VERSION, > + RMI_ABI_MINOR_VERSION); > + > + arm_smccc_1_1_invoke(SMC_RMI_VERSION, host_version, &res); > + > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) > + return -ENXIO; > + > + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a1); > + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a1); > + > + if (res.a0 != RMI_SUCCESS) { > + kvm_err("Unsupported RMI ABI (v%d.%d) we want v%d.%d\n", > + version_major, version_minor, > + RMI_ABI_MAJOR_VERSION, > + RMI_ABI_MINOR_VERSION); > + return -ENXIO; > + } > + > + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); > + > + return 0; > +} > + Should we include both high and low version numbers in the kvm_err message on error? ie, high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2); high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2); -aneesh
On 12/19/24 3:44 PM, Aneesh Kumar K.V wrote: > Steven Price <steven.price@arm.com> writes: > >> +static int rmi_check_version(void) >> +{ >> + struct arm_smccc_res res; >> + int version_major, version_minor; >> + unsigned long host_version = RMI_ABI_VERSION(RMI_ABI_MAJOR_VERSION, >> + RMI_ABI_MINOR_VERSION); >> + >> + arm_smccc_1_1_invoke(SMC_RMI_VERSION, host_version, &res); >> + >> + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) >> + return -ENXIO; >> + >> + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a1); >> + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a1); >> + >> + if (res.a0 != RMI_SUCCESS) { >> + kvm_err("Unsupported RMI ABI (v%d.%d) we want v%d.%d\n", >> + version_major, version_minor, >> + RMI_ABI_MAJOR_VERSION, >> + RMI_ABI_MINOR_VERSION); >> + return -ENXIO; >> + } >> + >> + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); >> + >> + return 0; >> +} >> + > > Should we include both high and low version numbers in the kvm_err > message on error? ie, > > high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2); > high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2); > I think so since a range of supported versions are returned in the failing case. Besides, 'unsigned short' is more suitable for the local variable version_{major, minor} since both are 16-bits in width. 'unsigned short' explicitly indicates their width. Thanks, Gavin
On 12/13/24 1:55 AM, Steven Price wrote: > Query the RMI version number and check if it is a compatible version. A > static key is also provided to signal that a supported RMM is available. > > Functions are provided to query if a VM or VCPU is a realm (or rec) > which currently will always return false. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > Changes since v5: > * Reword "unsupported" message from "host supports" to "we want" to > clarify that 'we' are the 'host'. > Changes since v2: > * Drop return value from kvm_init_rme(), it was always 0. > * Rely on the RMM return value to identify whether the RSI ABI is > compatible. > --- > arch/arm64/include/asm/kvm_emulate.h | 18 +++++++++ > arch/arm64/include/asm/kvm_host.h | 4 ++ > arch/arm64/include/asm/kvm_rme.h | 56 ++++++++++++++++++++++++++++ > arch/arm64/include/asm/virt.h | 1 + > arch/arm64/kvm/Makefile | 3 +- > arch/arm64/kvm/arm.c | 6 +++ > arch/arm64/kvm/rme.c | 50 +++++++++++++++++++++++++ > 7 files changed, 137 insertions(+), 1 deletion(-) > create mode 100644 arch/arm64/include/asm/kvm_rme.h > create mode 100644 arch/arm64/kvm/rme.c > [...] > + > +static inline bool kvm_is_realm(struct kvm *kvm) > +{ > + if (static_branch_unlikely(&kvm_rme_is_available) && kvm) > + return kvm->arch.is_realm; > + return false; > +} > + kvm->arch.is_realm won't be true when kvm_rme_is_available is false. It's set in kvm_arch_init_vm() of PATCH[10]. So it's safe to be simplified as below if the changes to kvm_arch_init_vm() is slightly modified, more details can be found from the comments to PATCH[10] With the changes, we don't have to check kvm_rme_is_available every time. static inline bool kvm_is_realm(struct kvm *kvm) { return (kvm && kvm->arch.is_realm); } Thanks, Gavin
On 29/01/2025 03:57, Gavin Shan wrote: > On 12/13/24 1:55 AM, Steven Price wrote: >> Query the RMI version number and check if it is a compatible version. A >> static key is also provided to signal that a supported RMM is available. >> >> Functions are provided to query if a VM or VCPU is a realm (or rec) >> which currently will always return false. >> >> Signed-off-by: Steven Price <steven.price@arm.com> >> --- >> Changes since v5: >> * Reword "unsupported" message from "host supports" to "we want" to >> clarify that 'we' are the 'host'. >> Changes since v2: >> * Drop return value from kvm_init_rme(), it was always 0. >> * Rely on the RMM return value to identify whether the RSI ABI is >> compatible. >> --- >> arch/arm64/include/asm/kvm_emulate.h | 18 +++++++++ >> arch/arm64/include/asm/kvm_host.h | 4 ++ >> arch/arm64/include/asm/kvm_rme.h | 56 ++++++++++++++++++++++++++++ >> arch/arm64/include/asm/virt.h | 1 + >> arch/arm64/kvm/Makefile | 3 +- >> arch/arm64/kvm/arm.c | 6 +++ >> arch/arm64/kvm/rme.c | 50 +++++++++++++++++++++++++ >> 7 files changed, 137 insertions(+), 1 deletion(-) >> create mode 100644 arch/arm64/include/asm/kvm_rme.h >> create mode 100644 arch/arm64/kvm/rme.c >> > > [...] > >> + >> +static inline bool kvm_is_realm(struct kvm *kvm) >> +{ >> + if (static_branch_unlikely(&kvm_rme_is_available) && kvm) >> + return kvm->arch.is_realm; >> + return false; >> +} >> + > > kvm->arch.is_realm won't be true when kvm_rme_is_available is false. > It's set > in kvm_arch_init_vm() of PATCH[10]. So it's safe to be simplified as > below if > the changes to kvm_arch_init_vm() is slightly modified, more details can be > found from the comments to PATCH[10] > > With the changes, we don't have to check kvm_rme_is_available every time. This is true, however, kvm_rme_is_available is a static key, which means that the kernel uses runtime patching to change the value. So by checking kvm_rme_is_available this check is short-circuited and there's no (data) memory access. I have to admit I've no idea whether this actually makes any practical difference to any benchmark but the intention was to have the minimum possible overhead on systems which don't have the RME hardware. Steve > static inline bool kvm_is_realm(struct kvm *kvm) > { > return (kvm && kvm->arch.is_realm); > } > > Thanks, > Gavin >
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index cf811009a33c..4d417e39763e 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -702,4 +702,22 @@ static inline void kvm_vcpu_enable_ptrauth(struct kvm_vcpu *vcpu) { vcpu_set_flag(vcpu, GUEST_HAS_PTRAUTH); } + +static inline bool kvm_is_realm(struct kvm *kvm) +{ + if (static_branch_unlikely(&kvm_rme_is_available) && kvm) + return kvm->arch.is_realm; + return false; +} + +static inline enum realm_state kvm_realm_state(struct kvm *kvm) +{ + return READ_ONCE(kvm->arch.realm.state); +} + +static inline bool vcpu_is_rec(struct kvm_vcpu *vcpu) +{ + return false; +} + #endif /* __ARM64_KVM_EMULATE_H__ */ diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index e18e9244d17a..8482638dce3b 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -27,6 +27,7 @@ #include <asm/fpsimd.h> #include <asm/kvm.h> #include <asm/kvm_asm.h> +#include <asm/kvm_rme.h> #include <asm/vncr_mapping.h> #define __KVM_HAVE_ARCH_INTC_INITIALIZED @@ -380,6 +381,9 @@ struct kvm_arch { * the associated pKVM instance in the hypervisor. */ struct kvm_protected_vm pkvm; + + bool is_realm; + struct realm realm; }; struct kvm_vcpu_fault_info { diff --git a/arch/arm64/include/asm/kvm_rme.h b/arch/arm64/include/asm/kvm_rme.h new file mode 100644 index 000000000000..69af5c3a1e44 --- /dev/null +++ b/arch/arm64/include/asm/kvm_rme.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2023 ARM Ltd. + */ + +#ifndef __ASM_KVM_RME_H +#define __ASM_KVM_RME_H + +/** + * enum realm_state - State of a Realm + */ +enum realm_state { + /** + * @REALM_STATE_NONE: + * Realm has not yet been created. rmi_realm_create() may be + * called to create the realm. + */ + REALM_STATE_NONE, + /** + * @REALM_STATE_NEW: + * Realm is under construction, not eligible for execution. Pages + * may be populated with rmi_data_create(). + */ + REALM_STATE_NEW, + /** + * @REALM_STATE_ACTIVE: + * Realm has been created and is eligible for execution with + * rmi_rec_enter(). Pages may no longer be populated with + * rmi_data_create(). + */ + REALM_STATE_ACTIVE, + /** + * @REALM_STATE_DYING: + * Realm is in the process of being destroyed or has already been + * destroyed. + */ + REALM_STATE_DYING, + /** + * @REALM_STATE_DEAD: + * Realm has been destroyed. + */ + REALM_STATE_DEAD +}; + +/** + * struct realm - Additional per VM data for a Realm + * + * @state: The lifetime state machine for the realm + */ +struct realm { + enum realm_state state; +}; + +void kvm_init_rme(void); + +#endif diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h index ebf4a9f943ed..e45d47156dcf 100644 --- a/arch/arm64/include/asm/virt.h +++ b/arch/arm64/include/asm/virt.h @@ -81,6 +81,7 @@ void __hyp_reset_vectors(void); bool is_kvm_arm_initialised(void); DECLARE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); +DECLARE_STATIC_KEY_FALSE(kvm_rme_is_available); static inline bool is_pkvm_initialized(void) { diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile index 3cf7adb2b503..ce8a10d3161d 100644 --- a/arch/arm64/kvm/Makefile +++ b/arch/arm64/kvm/Makefile @@ -23,7 +23,8 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \ vgic/vgic-v3.o vgic/vgic-v4.o \ vgic/vgic-mmio.o vgic/vgic-mmio-v2.o \ vgic/vgic-mmio-v3.o vgic/vgic-kvm-device.o \ - vgic/vgic-its.o vgic/vgic-debug.o + vgic/vgic-its.o vgic/vgic-debug.o \ + rme.o kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o pmu.o kvm-$(CONFIG_ARM64_PTR_AUTH) += pauth.o diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index a102c3aebdbc..93087eca9c51 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -40,6 +40,7 @@ #include <asm/kvm_nested.h> #include <asm/kvm_pkvm.h> #include <asm/kvm_ptrauth.h> +#include <asm/kvm_rme.h> #include <asm/sections.h> #include <kvm/arm_hypercalls.h> @@ -59,6 +60,8 @@ enum kvm_wfx_trap_policy { static enum kvm_wfx_trap_policy kvm_wfi_trap_policy __read_mostly = KVM_WFX_NOTRAP_SINGLE_TASK; static enum kvm_wfx_trap_policy kvm_wfe_trap_policy __read_mostly = KVM_WFX_NOTRAP_SINGLE_TASK; +DEFINE_STATIC_KEY_FALSE(kvm_rme_is_available); + DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector); DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page); @@ -2769,6 +2772,9 @@ static __init int kvm_arm_init(void) in_hyp_mode = is_kernel_in_hyp_mode(); + if (in_hyp_mode) + kvm_init_rme(); + if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) || cpus_have_final_cap(ARM64_WORKAROUND_1508412)) kvm_info("Guests without required CPU erratum workarounds can deadlock system!\n" \ diff --git a/arch/arm64/kvm/rme.c b/arch/arm64/kvm/rme.c new file mode 100644 index 000000000000..b88269b80c11 --- /dev/null +++ b/arch/arm64/kvm/rme.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 ARM Ltd. + */ + +#include <linux/kvm_host.h> + +#include <asm/rmi_cmds.h> +#include <asm/virt.h> + +static int rmi_check_version(void) +{ + struct arm_smccc_res res; + int version_major, version_minor; + unsigned long host_version = RMI_ABI_VERSION(RMI_ABI_MAJOR_VERSION, + RMI_ABI_MINOR_VERSION); + + arm_smccc_1_1_invoke(SMC_RMI_VERSION, host_version, &res); + + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) + return -ENXIO; + + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a1); + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a1); + + if (res.a0 != RMI_SUCCESS) { + kvm_err("Unsupported RMI ABI (v%d.%d) we want v%d.%d\n", + version_major, version_minor, + RMI_ABI_MAJOR_VERSION, + RMI_ABI_MINOR_VERSION); + return -ENXIO; + } + + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); + + return 0; +} + +void kvm_init_rme(void) +{ + if (PAGE_SIZE != SZ_4K) + /* Only 4k page size on the host is supported */ + return; + + if (rmi_check_version()) + /* Continue without realm support */ + return; + + /* Future patch will enable static branch kvm_rme_is_available */ +}
Query the RMI version number and check if it is a compatible version. A static key is also provided to signal that a supported RMM is available. Functions are provided to query if a VM or VCPU is a realm (or rec) which currently will always return false. Signed-off-by: Steven Price <steven.price@arm.com> --- Changes since v5: * Reword "unsupported" message from "host supports" to "we want" to clarify that 'we' are the 'host'. Changes since v2: * Drop return value from kvm_init_rme(), it was always 0. * Rely on the RMM return value to identify whether the RSI ABI is compatible. --- arch/arm64/include/asm/kvm_emulate.h | 18 +++++++++ arch/arm64/include/asm/kvm_host.h | 4 ++ arch/arm64/include/asm/kvm_rme.h | 56 ++++++++++++++++++++++++++++ arch/arm64/include/asm/virt.h | 1 + arch/arm64/kvm/Makefile | 3 +- arch/arm64/kvm/arm.c | 6 +++ arch/arm64/kvm/rme.c | 50 +++++++++++++++++++++++++ 7 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/include/asm/kvm_rme.h create mode 100644 arch/arm64/kvm/rme.c