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.
Later patches make use of struct realm and the states as the ioctls
interfaces are added to support realm and REC creation and destruction.
Reviewed-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
Changes since v8:
* No need to guard kvm_init_rme() behind 'in_hyp_mode'.
Changes since v6:
* Improved message for an unsupported RMI ABI version.
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 | 2 +-
arch/arm64/kvm/arm.c | 5 +++
arch/arm64/kvm/rme.c | 56 ++++++++++++++++++++++++++++
7 files changed, 141 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_rme.h
create mode 100644 arch/arm64/kvm/rme.c
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index fa8a08a1ccd5..ab4093e41c4b 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -674,4 +674,22 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
vcpu->arch.hcrx_el2 |= HCRX_EL2_SCTLR2En;
}
}
+
+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 2f2394cce24e..d1511ce26191 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
@@ -404,6 +405,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..9c8a0b23e0e4
--- /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 /* __ASM_KVM_RME_H */
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index aa280f356b96..db73c9bfd8c9 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -82,6 +82,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 3ebc0570345c..70fa017831b3 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -16,7 +16,7 @@ CFLAGS_handle_exit.o += -Wno-override-init
kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
inject_fault.o va_layout.o handle_exit.o config.o \
guest.o debug.o reset.o sys_regs.o stacktrace.o \
- vgic-sys-reg-v3.o fpsimd.o pkvm.o \
+ vgic-sys-reg-v3.o fpsimd.o pkvm.o rme.o \
arch_timer.o trng.o vmid.o emulate-nested.o nested.o at.o \
vgic/vgic.o vgic/vgic-init.o \
vgic/vgic-irqfd.o vgic/vgic-v2.o \
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 888f7c7abf54..76177c56f1ef 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_base);
@@ -2836,6 +2839,8 @@ static __init int kvm_arm_init(void)
in_hyp_mode = is_kernel_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..67cf2d94cb2d
--- /dev/null
+++ b/arch/arm64/kvm/rme.c
@@ -0,0 +1,56 @@
+// 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;
+ unsigned short 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) {
+ unsigned short high_version_major, high_version_minor;
+
+ high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
+ high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
+
+ kvm_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d.%d\n",
+ version_major, version_minor,
+ high_version_major, high_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 */
+}
--
2.43.0
On Wed, 20 Aug 2025 15:55:25 +0100,
Steven Price <steven.price@arm.com> 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.
>
> Later patches make use of struct realm and the states as the ioctls
> interfaces are added to support realm and REC creation and destruction.
>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v8:
> * No need to guard kvm_init_rme() behind 'in_hyp_mode'.
> Changes since v6:
> * Improved message for an unsupported RMI ABI version.
> 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 | 2 +-
> arch/arm64/kvm/arm.c | 5 +++
> arch/arm64/kvm/rme.c | 56 ++++++++++++++++++++++++++++
> 7 files changed, 141 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/kvm_rme.h
> create mode 100644 arch/arm64/kvm/rme.c
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index fa8a08a1ccd5..ab4093e41c4b 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -674,4 +674,22 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
> vcpu->arch.hcrx_el2 |= HCRX_EL2_SCTLR2En;
> }
> }
> +
> +static inline bool kvm_is_realm(struct kvm *kvm)
> +{
> + if (static_branch_unlikely(&kvm_rme_is_available) && kvm)
Under what circumstances would you call this with a NULL pointer?
> + 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 2f2394cce24e..d1511ce26191 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
> @@ -404,6 +405,9 @@ struct kvm_arch {
> * the associated pKVM instance in the hypervisor.
> */
> struct kvm_protected_vm pkvm;
> +
> + bool is_realm;
> + struct realm realm;
Given that pkvm and CCA are pretty much exclusive, I don't think we
need to store both states separately. Make those a union.
> };
>
> 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..9c8a0b23e0e4
> --- /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
None of that is about RME. This is about CCA, which is purely a SW
construct, and not a CPU architecture feature.
So 's/rme/cca/' everywhere that describe something that is not a
direct effect of FEAT_RME being implemented on the CPU, but instead
something that is CCA-specific.
> +
> +/**
> + * 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 /* __ASM_KVM_RME_H */
> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
> index aa280f356b96..db73c9bfd8c9 100644
> --- a/arch/arm64/include/asm/virt.h
> +++ b/arch/arm64/include/asm/virt.h
> @@ -82,6 +82,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);
Same thing about RME.
>
> static inline bool is_pkvm_initialized(void)
> {
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 3ebc0570345c..70fa017831b3 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -16,7 +16,7 @@ CFLAGS_handle_exit.o += -Wno-override-init
> kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
> inject_fault.o va_layout.o handle_exit.o config.o \
> guest.o debug.o reset.o sys_regs.o stacktrace.o \
> - vgic-sys-reg-v3.o fpsimd.o pkvm.o \
> + vgic-sys-reg-v3.o fpsimd.o pkvm.o rme.o \
> arch_timer.o trng.o vmid.o emulate-nested.o nested.o at.o \
> vgic/vgic.o vgic/vgic-init.o \
> vgic/vgic-irqfd.o vgic/vgic-v2.o \
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 888f7c7abf54..76177c56f1ef 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_base);
> @@ -2836,6 +2839,8 @@ static __init int kvm_arm_init(void)
>
> in_hyp_mode = is_kernel_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..67cf2d94cb2d
> --- /dev/null
> +++ b/arch/arm64/kvm/rme.c
> @@ -0,0 +1,56 @@
> +// 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;
> + unsigned short 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);
Shouldn't you first check that RME is actually available, by looking
at ID_AA64PFR0_EL1.RME?
> +
> + 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) {
> + unsigned short high_version_major, high_version_minor;
> +
> + high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
> + high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
> +
> + kvm_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d.%d\n",
> + version_major, version_minor,
> + high_version_major, high_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;
Move the comment above the check (same thing below).
M.
--
Without deviation from the norm, progress is not possible.
On 01/10/2025 12:05, Marc Zyngier wrote:
> On Wed, 20 Aug 2025 15:55:25 +0100,
> Steven Price <steven.price@arm.com> 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.
>>
>> Later patches make use of struct realm and the states as the ioctls
>> interfaces are added to support realm and REC creation and destruction.
>>
>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Changes since v8:
>> * No need to guard kvm_init_rme() behind 'in_hyp_mode'.
>> Changes since v6:
>> * Improved message for an unsupported RMI ABI version.
>> 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 | 2 +-
>> arch/arm64/kvm/arm.c | 5 +++
>> arch/arm64/kvm/rme.c | 56 ++++++++++++++++++++++++++++
>> 7 files changed, 141 insertions(+), 1 deletion(-)
>> create mode 100644 arch/arm64/include/asm/kvm_rme.h
>> create mode 100644 arch/arm64/kvm/rme.c
>>
>> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
>> index fa8a08a1ccd5..ab4093e41c4b 100644
>> --- a/arch/arm64/include/asm/kvm_emulate.h
>> +++ b/arch/arm64/include/asm/kvm_emulate.h
>> @@ -674,4 +674,22 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
>> vcpu->arch.hcrx_el2 |= HCRX_EL2_SCTLR2En;
>> }
>> }
>> +
>> +static inline bool kvm_is_realm(struct kvm *kvm)
>> +{
>> + if (static_branch_unlikely(&kvm_rme_is_available) && kvm)
>
> Under what circumstances would you call this with a NULL pointer?
kvm_vm_ioctl_check_extension() is the culprit. I guess this could be
handled with an equivalent to kvm_pvm_ext_allowed().
>> + 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 2f2394cce24e..d1511ce26191 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
>> @@ -404,6 +405,9 @@ struct kvm_arch {
>> * the associated pKVM instance in the hypervisor.
>> */
>> struct kvm_protected_vm pkvm;
>> +
>> + bool is_realm;
>> + struct realm realm;
>
> Given that pkvm and CCA are pretty much exclusive, I don't think we
> need to store both states separately. Make those a union.
Ack
>> };
>>
>> 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..9c8a0b23e0e4
>> --- /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
>
> None of that is about RME. This is about CCA, which is purely a SW
> construct, and not a CPU architecture feature.
>
> So 's/rme/cca/' everywhere that describe something that is not a
> direct effect of FEAT_RME being implemented on the CPU, but instead
> something that is CCA-specific.
Ok, it's a lot of churn but you've got a good point.
>> +
>> +/**
>> + * 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 /* __ASM_KVM_RME_H */
>> diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
>> index aa280f356b96..db73c9bfd8c9 100644
>> --- a/arch/arm64/include/asm/virt.h
>> +++ b/arch/arm64/include/asm/virt.h
>> @@ -82,6 +82,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);
>
> Same thing about RME.
>
>>
>> static inline bool is_pkvm_initialized(void)
>> {
>> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
>> index 3ebc0570345c..70fa017831b3 100644
>> --- a/arch/arm64/kvm/Makefile
>> +++ b/arch/arm64/kvm/Makefile
>> @@ -16,7 +16,7 @@ CFLAGS_handle_exit.o += -Wno-override-init
>> kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
>> inject_fault.o va_layout.o handle_exit.o config.o \
>> guest.o debug.o reset.o sys_regs.o stacktrace.o \
>> - vgic-sys-reg-v3.o fpsimd.o pkvm.o \
>> + vgic-sys-reg-v3.o fpsimd.o pkvm.o rme.o \
>> arch_timer.o trng.o vmid.o emulate-nested.o nested.o at.o \
>> vgic/vgic.o vgic/vgic-init.o \
>> vgic/vgic-irqfd.o vgic/vgic-v2.o \
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index 888f7c7abf54..76177c56f1ef 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_base);
>> @@ -2836,6 +2839,8 @@ static __init int kvm_arm_init(void)
>>
>> in_hyp_mode = is_kernel_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..67cf2d94cb2d
>> --- /dev/null
>> +++ b/arch/arm64/kvm/rme.c
>> @@ -0,0 +1,56 @@
>> +// 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;
>> + unsigned short 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);
>
> Shouldn't you first check that RME is actually available, by looking
> at ID_AA64PFR0_EL1.RME?
Well, you made a good point above that this isn't RME, it's CCA. And I
guess there's a possible world where the CCA interface could be
supported with something other than FEAT_RME (FEAT_RME2 maybe?) so I'm
not sure it necessarily a good idea to pin this on a CPU feature bit.
Ultimately what we want to know is whether the firmware thinks it can
supply us with the CCA interface and we don't really care how it
achieves it.
>> +
>> + 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) {
>> + unsigned short high_version_major, high_version_minor;
>> +
>> + high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
>> + high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
>> +
>> + kvm_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d.%d\n",
>> + version_major, version_minor,
>> + high_version_major, high_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;
>
> Move the comment above the check (same thing below).
Ack.
Thanks,
Steve
On Wed, 01 Oct 2025 14:20:13 +0100,
Steven Price <steven.price@arm.com> wrote:
>
> >> +static int rmi_check_version(void)
> >> +{
> >> + struct arm_smccc_res res;
> >> + unsigned short 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);
> >
> > Shouldn't you first check that RME is actually available, by looking
> > at ID_AA64PFR0_EL1.RME?
>
> Well, you made a good point above that this isn't RME, it's CCA. And I
> guess there's a possible world where the CCA interface could be
> supported with something other than FEAT_RME (FEAT_RME2 maybe?) so I'm
> not sure it necessarily a good idea to pin this on a CPU feature
> bit.
But you cannot have CCA without RME. You cannot have CCA with
GICv3. And my point was more that RME could be used by something other
than CCA - I certainly don't anticipate someone else adopting the CCA
interface for anything...
> Ultimately what we want to know is whether the firmware thinks it can
> supply us with the CCA interface and we don't really care how it
> achieves it.
I disagree. You rely on specific feature sets to be available (hell,
everything is baked around GICv3... GICv5 anyone?).
For this sort of stuff, you absolutely need to know what you are
running on, not what some broken firmware tries to pretend it is.
M.
--
Without deviation from the norm, progress is not possible.
On 01/10/2025 14:35, Marc Zyngier wrote:
> On Wed, 01 Oct 2025 14:20:13 +0100,
> Steven Price <steven.price@arm.com> wrote:
>>
>>>> +static int rmi_check_version(void)
>>>> +{
>>>> + struct arm_smccc_res res;
>>>> + unsigned short 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);
>>>
>>> Shouldn't you first check that RME is actually available, by looking
>>> at ID_AA64PFR0_EL1.RME?
>>
>> Well, you made a good point above that this isn't RME, it's CCA. And I
>> guess there's a possible world where the CCA interface could be
>> supported with something other than FEAT_RME (FEAT_RME2 maybe?) so I'm
>> not sure it necessarily a good idea to pin this on a CPU feature
>> bit.
>
> But you cannot have CCA without RME. You cannot have CCA with
> GICv3. And my point was more that RME could be used by something other
> than CCA - I certainly don't anticipate someone else adopting the CCA
> interface for anything...
>
>> Ultimately what we want to know is whether the firmware thinks it can
>> supply us with the CCA interface and we don't really care how it
>> achieves it.
>
> I disagree. You rely on specific feature sets to be available (hell,
> everything is baked around GICv3... GICv5 anyone?).
>
> For this sort of stuff, you absolutely need to know what you are
> running on, not what some broken firmware tries to pretend it is.
Well I don't agree, but equally I think the chances of the RMM interface
existing without RME is nil, so I guess the extra check doesn't really
matter. So I'll add it. In the (highly) unlikely event that it causes a
problem then it would be easy enough to remove.
Thanks,
Steve
On 8/21/25 12: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.
>
> Later patches make use of struct realm and the states as the ioctls
> interfaces are added to support realm and REC creation and destruction.
>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v8:
> * No need to guard kvm_init_rme() behind 'in_hyp_mode'.
> Changes since v6:
> * Improved message for an unsupported RMI ABI version.
> 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 | 2 +-
> arch/arm64/kvm/arm.c | 5 +++
> arch/arm64/kvm/rme.c | 56 ++++++++++++++++++++++++++++
> 7 files changed, 141 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/kvm_rme.h
> create mode 100644 arch/arm64/kvm/rme.c
>
[...]
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 888f7c7abf54..76177c56f1ef 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>
>
Nit: The header file <asm/kvm_rme.h> has been included to <asm/kvm_host.h> and
<linux/kvm_host.h>, which has been included to arm.c. So this explicit inclusion
can be dropped.
> #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_base);
> @@ -2836,6 +2839,8 @@ static __init int kvm_arm_init(void)
>
> in_hyp_mode = is_kernel_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..67cf2d94cb2d
> --- /dev/null
> +++ b/arch/arm64/kvm/rme.c
> @@ -0,0 +1,56 @@
> +// 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;
> + unsigned short 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) {
> + unsigned short high_version_major, high_version_minor;
> +
> + high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
> + high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
> +
> + kvm_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d.%d\n",
> + version_major, version_minor,
> + high_version_major, high_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;
Nit: The comment can be moved before the check, something like below. Otherwise,
{} is needed here.
/* Only 4kB page size is supported */
if (PAGE_SIZE != SZ_4K)
return;
> +
> + if (rmi_check_version())
> + /* Continue without realm support */
> + return;
Nit: same as above.
> +
> + /* Future patch will enable static branch kvm_rme_is_available */
> +}
Thanks,
Gavin
On 03/09/2025 12:15, Gavin Shan wrote:
> On 8/21/25 12: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.
>>
>> Later patches make use of struct realm and the states as the ioctls
>> interfaces are added to support realm and REC creation and destruction.
>>
>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Changes since v8:
>> * No need to guard kvm_init_rme() behind 'in_hyp_mode'.
>> Changes since v6:
>> * Improved message for an unsupported RMI ABI version.
>> 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 | 2 +-
>> arch/arm64/kvm/arm.c | 5 +++
>> arch/arm64/kvm/rme.c | 56 ++++++++++++++++++++++++++++
>> 7 files changed, 141 insertions(+), 1 deletion(-)
>> create mode 100644 arch/arm64/include/asm/kvm_rme.h
>> create mode 100644 arch/arm64/kvm/rme.c
>>
>
> [...]
>
>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>> index 888f7c7abf54..76177c56f1ef 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>
>>
>
> Nit: The header file <asm/kvm_rme.h> has been included to <asm/
> kvm_host.h> and
> <linux/kvm_host.h>, which has been included to arm.c. So this explicit
> inclusion
> can be dropped.
While it's true that it could be dropped because of the indirect
include, generally it's better to explicitly include a header file when
you are using the definitions from it. That way the code can be
refactored (e.g. if asm/kvm_host.h is changed to no longer needs the
header it can then safely drop the header include).
We have a similar situation with asm/kvm_asm.h being included both here
and in asm/kvm_host.h (and probably many others, this was just the first
I spotted).
>> #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_base);
>> @@ -2836,6 +2839,8 @@ static __init int kvm_arm_init(void)
>> in_hyp_mode = is_kernel_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..67cf2d94cb2d
>> --- /dev/null
>> +++ b/arch/arm64/kvm/rme.c
>> @@ -0,0 +1,56 @@
>> +// 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;
>> + unsigned short 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) {
>> + unsigned short high_version_major, high_version_minor;
>> +
>> + high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
>> + high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
>> +
>> + kvm_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d.
>> %d\n",
>> + version_major, version_minor,
>> + high_version_major, high_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;
>
> Nit: The comment can be moved before the check, something like below.
> Otherwise,
> {} is needed here.
>
> /* Only 4kB page size is supported */
> if (PAGE_SIZE != SZ_4K)
> return;
>
>> +
>> + if (rmi_check_version())
>> + /* Continue without realm support */
>> + return;
>
> Nit: same as above.
Fair enough, I'm never sure what's best here - the comments are for the
error path "Continue without realm support [if we can't agree on a
version]", but it's a single statement so braces aren't needed. Still
the first instance can easily be hoisted above without losing its
meaning, and the second can be extended to clarify.
Thanks,
Steve
>> +
>> + /* Future patch will enable static branch kvm_rme_is_available */
>> +}
>
> Thanks,
> Gavin
>
© 2016 - 2026 Red Hat, Inc.