| Message ID | 20230127112932.38045-5-steven.price@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,01/28] arm64: RME: Handle Granule Protection Faults (GPFs) | expand |
On Fri, 27 Jan 2023 11:29:08 +0000 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. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ > arch/arm64/include/asm/kvm_host.h | 4 +++ > arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ > arch/arm64/include/asm/virt.h | 1 + > arch/arm64/kvm/Makefile | 3 +- > arch/arm64/kvm/arm.c | 8 +++++ > arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ > 7 files changed, 103 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 9bdba47f7e14..5a2b7229e83f 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -490,4 +490,21 @@ static inline bool vcpu_has_feature(struct kvm_vcpu *vcpu, int feature) > return test_bit(feature, vcpu->arch.features); > } > > +static inline bool kvm_is_realm(struct kvm *kvm) > +{ > + if (static_branch_unlikely(&kvm_rme_is_available)) > + 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 35a159d131b5..04347c3a8c6b 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -26,6 +26,7 @@ > #include <asm/fpsimd.h> > #include <asm/kvm.h> > #include <asm/kvm_asm.h> > +#include <asm/kvm_rme.h> > > #define __KVM_HAVE_ARCH_INTC_INITIALIZED > > @@ -240,6 +241,9 @@ struct kvm_arch { > * the associated pKVM instance in the hypervisor. > */ > struct kvm_protected_vm pkvm; > + > + bool is_realm; ^ It would be better to put more comments which really helps on the review. I was looking for the user of this memeber to see when it is set. It seems it is not in this patch. It would have been nice to have a quick answer from the comments. > + 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..c26bc2c6770d > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_rme.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2023 ARM Ltd. > + */ > + > +#ifndef __ASM_KVM_RME_H > +#define __ASM_KVM_RME_H > + > +enum realm_state { > + REALM_STATE_NONE, > + REALM_STATE_NEW, > + REALM_STATE_ACTIVE, > + REALM_STATE_DYING > +}; > + > +struct realm { > + enum realm_state state; > +}; > + > +int kvm_init_rme(void); > + > +#endif > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h > index 4eb601e7de50..be1383e26626 100644 > --- a/arch/arm64/include/asm/virt.h > +++ b/arch/arm64/include/asm/virt.h > @@ -80,6 +80,7 @@ void __hyp_set_vectors(phys_addr_t phys_vector_base); > void __hyp_reset_vectors(void); > > DECLARE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); > +DECLARE_STATIC_KEY_FALSE(kvm_rme_is_available); > > /* Reports the availability of HYP mode */ > static inline bool is_hyp_mode_available(void) > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile > index 5e33c2d4645a..d2f0400c50da 100644 > --- a/arch/arm64/kvm/Makefile > +++ b/arch/arm64/kvm/Makefile > @@ -20,7 +20,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 > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 9c5573bc4614..d97b39d042ab 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -38,6 +38,7 @@ > #include <asm/kvm_asm.h> > #include <asm/kvm_mmu.h> > #include <asm/kvm_pkvm.h> > +#include <asm/kvm_rme.h> > #include <asm/kvm_emulate.h> > #include <asm/sections.h> > > @@ -47,6 +48,7 @@ > > static enum kvm_mode kvm_mode = KVM_MODE_DEFAULT; > DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); > +DEFINE_STATIC_KEY_FALSE(kvm_rme_is_available); > > DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector); > > @@ -2213,6 +2215,12 @@ int kvm_arch_init(void *opaque) > > in_hyp_mode = is_kernel_in_hyp_mode(); > > + if (in_hyp_mode) { > + err = kvm_init_rme(); > + if (err) > + return err; > + } > + > 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..f6b587bc116e > --- /dev/null > +++ b/arch/arm64/kvm/rme.c > @@ -0,0 +1,49 @@ > +// 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; > + > + arm_smccc_1_1_invoke(SMC_RMI_VERSION, &res); > + > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) > + return -ENXIO; > + > + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a0); > + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a0); > + > + if (version_major != RMI_ABI_MAJOR_VERSION) { > + kvm_err("Unsupported RMI ABI (version %d.%d) we support %d\n", > + version_major, version_minor, > + RMI_ABI_MAJOR_VERSION); > + return -ENXIO; > + } > + > + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); > + > + return 0; > +} > + > +int kvm_init_rme(void) > +{ > + if (PAGE_SIZE != SZ_4K) > + /* Only 4k page size on the host is supported */ > + return 0; > + > + if (rmi_check_version()) > + /* Continue without realm support */ > + return 0; > + > + /* Future patch will enable static branch kvm_rme_is_available */ > + > + return 0; > +}
On Fri, 27 Jan 2023 11:29:08 +0000 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. > > Signed-off-by: Steven Price <steven.price@arm.com> > --- > arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ > arch/arm64/include/asm/kvm_host.h | 4 +++ > arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ > arch/arm64/include/asm/virt.h | 1 + > arch/arm64/kvm/Makefile | 3 +- > arch/arm64/kvm/arm.c | 8 +++++ > arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ > 7 files changed, 103 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 9bdba47f7e14..5a2b7229e83f 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -490,4 +490,21 @@ static inline bool vcpu_has_feature(struct kvm_vcpu *vcpu, int feature) > return test_bit(feature, vcpu->arch.features); > } > > +static inline bool kvm_is_realm(struct kvm *kvm) > +{ > + if (static_branch_unlikely(&kvm_rme_is_available)) > + 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 35a159d131b5..04347c3a8c6b 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -26,6 +26,7 @@ > #include <asm/fpsimd.h> > #include <asm/kvm.h> > #include <asm/kvm_asm.h> > +#include <asm/kvm_rme.h> > > #define __KVM_HAVE_ARCH_INTC_INITIALIZED > > @@ -240,6 +241,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..c26bc2c6770d > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_rme.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2023 ARM Ltd. > + */ > + > +#ifndef __ASM_KVM_RME_H > +#define __ASM_KVM_RME_H > + > +enum realm_state { > + REALM_STATE_NONE, > + REALM_STATE_NEW, > + REALM_STATE_ACTIVE, > + REALM_STATE_DYING > +}; > + By the way, it is better to add more comments to introduce about the states here. > +struct realm { > + enum realm_state state; > +}; > + > +int kvm_init_rme(void); > + > +#endif > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h > index 4eb601e7de50..be1383e26626 100644 > --- a/arch/arm64/include/asm/virt.h > +++ b/arch/arm64/include/asm/virt.h > @@ -80,6 +80,7 @@ void __hyp_set_vectors(phys_addr_t phys_vector_base); > void __hyp_reset_vectors(void); > > DECLARE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); > +DECLARE_STATIC_KEY_FALSE(kvm_rme_is_available); > > /* Reports the availability of HYP mode */ > static inline bool is_hyp_mode_available(void) > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile > index 5e33c2d4645a..d2f0400c50da 100644 > --- a/arch/arm64/kvm/Makefile > +++ b/arch/arm64/kvm/Makefile > @@ -20,7 +20,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 > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 9c5573bc4614..d97b39d042ab 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -38,6 +38,7 @@ > #include <asm/kvm_asm.h> > #include <asm/kvm_mmu.h> > #include <asm/kvm_pkvm.h> > +#include <asm/kvm_rme.h> > #include <asm/kvm_emulate.h> > #include <asm/sections.h> > > @@ -47,6 +48,7 @@ > > static enum kvm_mode kvm_mode = KVM_MODE_DEFAULT; > DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); > +DEFINE_STATIC_KEY_FALSE(kvm_rme_is_available); > > DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector); > > @@ -2213,6 +2215,12 @@ int kvm_arch_init(void *opaque) > > in_hyp_mode = is_kernel_in_hyp_mode(); > > + if (in_hyp_mode) { > + err = kvm_init_rme(); > + if (err) > + return err; > + } > + > 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..f6b587bc116e > --- /dev/null > +++ b/arch/arm64/kvm/rme.c > @@ -0,0 +1,49 @@ > +// 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; > + > + arm_smccc_1_1_invoke(SMC_RMI_VERSION, &res); > + > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) > + return -ENXIO; > + > + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a0); > + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a0); > + > + if (version_major != RMI_ABI_MAJOR_VERSION) { > + kvm_err("Unsupported RMI ABI (version %d.%d) we support %d\n", > + version_major, version_minor, > + RMI_ABI_MAJOR_VERSION); > + return -ENXIO; > + } > + > + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); > + > + return 0; > +} > + > +int kvm_init_rme(void) > +{ > + if (PAGE_SIZE != SZ_4K) > + /* Only 4k page size on the host is supported */ > + return 0; > + > + if (rmi_check_version()) > + /* Continue without realm support */ > + return 0; > + > + /* Future patch will enable static branch kvm_rme_is_available */ > + > + return 0; > +}
On 13/02/2023 15:48, Zhi Wang wrote: > On Fri, 27 Jan 2023 11:29:08 +0000 > 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. >> >> Signed-off-by: Steven Price <steven.price@arm.com> >> --- >> arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ >> arch/arm64/include/asm/kvm_host.h | 4 +++ >> arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ >> arch/arm64/include/asm/virt.h | 1 + >> arch/arm64/kvm/Makefile | 3 +- >> arch/arm64/kvm/arm.c | 8 +++++ >> arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ >> 7 files changed, 103 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 9bdba47f7e14..5a2b7229e83f 100644 >> --- a/arch/arm64/include/asm/kvm_emulate.h >> +++ b/arch/arm64/include/asm/kvm_emulate.h >> @@ -490,4 +490,21 @@ static inline bool vcpu_has_feature(struct kvm_vcpu *vcpu, int feature) >> return test_bit(feature, vcpu->arch.features); >> } >> >> +static inline bool kvm_is_realm(struct kvm *kvm) >> +{ >> + if (static_branch_unlikely(&kvm_rme_is_available)) >> + 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 35a159d131b5..04347c3a8c6b 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -26,6 +26,7 @@ >> #include <asm/fpsimd.h> >> #include <asm/kvm.h> >> #include <asm/kvm_asm.h> >> +#include <asm/kvm_rme.h> >> >> #define __KVM_HAVE_ARCH_INTC_INITIALIZED >> >> @@ -240,6 +241,9 @@ struct kvm_arch { >> * the associated pKVM instance in the hypervisor. >> */ >> struct kvm_protected_vm pkvm; >> + >> + bool is_realm; > ^ > It would be better to put more comments which really helps on the review. Thanks for the feedback - I had thought "is realm" was fairly self-documenting, but perhaps I've just spent too much time with this code. > I was looking for the user of this memeber to see when it is set. It seems > it is not in this patch. It would have been nice to have a quick answer from the > comments. The usage is in the kvm_is_realm() function which is used in several of the later patches as a way to detect this kvm guest is a realm guest. I think the main issue is that I've got the patches in the wrong other. Patch 7 "arm64: kvm: Allow passing machine type in KVM creation" should probably be before this one, then I could add the assignment of is_realm into this patch (potentially splitting out the is_realm parts into another patch). Thanks, Steve
On Mon, 13 Feb 2023 15:59:05 +0000 Steven Price <steven.price@arm.com> wrote: > On 13/02/2023 15:48, Zhi Wang wrote: > > On Fri, 27 Jan 2023 11:29:08 +0000 > > 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. > >> > >> Signed-off-by: Steven Price <steven.price@arm.com> > >> --- > >> arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ > >> arch/arm64/include/asm/kvm_host.h | 4 +++ > >> arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ > >> arch/arm64/include/asm/virt.h | 1 + > >> arch/arm64/kvm/Makefile | 3 +- > >> arch/arm64/kvm/arm.c | 8 +++++ > >> arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ > >> 7 files changed, 103 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 9bdba47f7e14..5a2b7229e83f 100644 > >> --- a/arch/arm64/include/asm/kvm_emulate.h > >> +++ b/arch/arm64/include/asm/kvm_emulate.h > >> @@ -490,4 +490,21 @@ static inline bool vcpu_has_feature(struct kvm_vcpu *vcpu, int feature) > >> return test_bit(feature, vcpu->arch.features); > >> } > >> > >> +static inline bool kvm_is_realm(struct kvm *kvm) > >> +{ > >> + if (static_branch_unlikely(&kvm_rme_is_available)) > >> + 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 35a159d131b5..04347c3a8c6b 100644 > >> --- a/arch/arm64/include/asm/kvm_host.h > >> +++ b/arch/arm64/include/asm/kvm_host.h > >> @@ -26,6 +26,7 @@ > >> #include <asm/fpsimd.h> > >> #include <asm/kvm.h> > >> #include <asm/kvm_asm.h> > >> +#include <asm/kvm_rme.h> > >> > >> #define __KVM_HAVE_ARCH_INTC_INITIALIZED > >> > >> @@ -240,6 +241,9 @@ struct kvm_arch { > >> * the associated pKVM instance in the hypervisor. > >> */ > >> struct kvm_protected_vm pkvm; > >> + > >> + bool is_realm; > > ^ > > It would be better to put more comments which really helps on the review. > > Thanks for the feedback - I had thought "is realm" was fairly > self-documenting, but perhaps I've just spent too much time with this code. > > > I was looking for the user of this memeber to see when it is set. It seems > > it is not in this patch. It would have been nice to have a quick answer from the > > comments. > > The usage is in the kvm_is_realm() function which is used in several of > the later patches as a way to detect this kvm guest is a realm guest. > > I think the main issue is that I've got the patches in the wrong other. > Patch 7 "arm64: kvm: Allow passing machine type in KVM creation" should > probably be before this one, then I could add the assignment of is_realm > into this patch (potentially splitting out the is_realm parts into > another patch). > I agree the patch order seems a problem here. The name is self-documenting but if the user of the variable is not in this patch, still needs to jump to the related patch to confirm if the variable is used as expected. In that situation, a comment would help to avoid jumping between patches (sometimes finding the the user of a variable from a patch bundle really slows down the review progress and eventually you have to open a terminal and check it in the git tree). > Thanks, > > Steve >
On 27-01-2023 04:59 pm, 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> > --- > arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ > arch/arm64/include/asm/kvm_host.h | 4 +++ > arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ > arch/arm64/include/asm/virt.h | 1 + > arch/arm64/kvm/Makefile | 3 +- > arch/arm64/kvm/arm.c | 8 +++++ > arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ > 7 files changed, 103 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 9bdba47f7e14..5a2b7229e83f 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -490,4 +490,21 @@ static inline bool vcpu_has_feature(struct kvm_vcpu *vcpu, int feature) > return test_bit(feature, vcpu->arch.features); > } > > +static inline bool kvm_is_realm(struct kvm *kvm) > +{ > + if (static_branch_unlikely(&kvm_rme_is_available)) > + 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 35a159d131b5..04347c3a8c6b 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -26,6 +26,7 @@ > #include <asm/fpsimd.h> > #include <asm/kvm.h> > #include <asm/kvm_asm.h> > +#include <asm/kvm_rme.h> > > #define __KVM_HAVE_ARCH_INTC_INITIALIZED > > @@ -240,6 +241,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..c26bc2c6770d > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_rme.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2023 ARM Ltd. > + */ > + > +#ifndef __ASM_KVM_RME_H > +#define __ASM_KVM_RME_H > + > +enum realm_state { > + REALM_STATE_NONE, > + REALM_STATE_NEW, > + REALM_STATE_ACTIVE, > + REALM_STATE_DYING > +}; > + > +struct realm { > + enum realm_state state; > +}; > + > +int kvm_init_rme(void); > + > +#endif > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h > index 4eb601e7de50..be1383e26626 100644 > --- a/arch/arm64/include/asm/virt.h > +++ b/arch/arm64/include/asm/virt.h > @@ -80,6 +80,7 @@ void __hyp_set_vectors(phys_addr_t phys_vector_base); > void __hyp_reset_vectors(void); > > DECLARE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); > +DECLARE_STATIC_KEY_FALSE(kvm_rme_is_available); > > /* Reports the availability of HYP mode */ > static inline bool is_hyp_mode_available(void) > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile > index 5e33c2d4645a..d2f0400c50da 100644 > --- a/arch/arm64/kvm/Makefile > +++ b/arch/arm64/kvm/Makefile > @@ -20,7 +20,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 > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 9c5573bc4614..d97b39d042ab 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -38,6 +38,7 @@ > #include <asm/kvm_asm.h> > #include <asm/kvm_mmu.h> > #include <asm/kvm_pkvm.h> > +#include <asm/kvm_rme.h> > #include <asm/kvm_emulate.h> > #include <asm/sections.h> > > @@ -47,6 +48,7 @@ > > static enum kvm_mode kvm_mode = KVM_MODE_DEFAULT; > DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); > +DEFINE_STATIC_KEY_FALSE(kvm_rme_is_available); > > DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector); > > @@ -2213,6 +2215,12 @@ int kvm_arch_init(void *opaque) > > in_hyp_mode = is_kernel_in_hyp_mode(); > > + if (in_hyp_mode) { > + err = kvm_init_rme(); > + if (err) > + return err; > + } > + > 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..f6b587bc116e > --- /dev/null > +++ b/arch/arm64/kvm/rme.c > @@ -0,0 +1,49 @@ > +// 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; > + > + arm_smccc_1_1_invoke(SMC_RMI_VERSION, &res); > + > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) > + return -ENXIO; > + > + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a0); > + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a0); > + > + if (version_major != RMI_ABI_MAJOR_VERSION) { > + kvm_err("Unsupported RMI ABI (version %d.%d) we support %d\n", Can we please replace "we support" to host supports. Also in the patch present in the repo, you are using variable our_version, can this be changed to host_version? > + version_major, version_minor, > + RMI_ABI_MAJOR_VERSION); > + return -ENXIO; > + } > + > + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); > + > + return 0; > +} > + > +int kvm_init_rme(void) > +{ > + if (PAGE_SIZE != SZ_4K) > + /* Only 4k page size on the host is supported */ > + return 0; > + > + if (rmi_check_version()) > + /* Continue without realm support */ > + return 0; > + > + /* Future patch will enable static branch kvm_rme_is_available */ > + > + return 0; > +} Thanks, Ganapat
On 18/03/2024 07:17, Ganapatrao Kulkarni wrote: > > > On 27-01-2023 04:59 pm, 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> >> --- >> arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ >> arch/arm64/include/asm/kvm_host.h | 4 +++ >> arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ >> arch/arm64/include/asm/virt.h | 1 + >> arch/arm64/kvm/Makefile | 3 +- >> arch/arm64/kvm/arm.c | 8 +++++ >> arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ >> 7 files changed, 103 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/rme.c b/arch/arm64/kvm/rme.c >> new file mode 100644 >> index 000000000000..f6b587bc116e >> --- /dev/null >> +++ b/arch/arm64/kvm/rme.c >> @@ -0,0 +1,49 @@ >> +// 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; >> + >> + arm_smccc_1_1_invoke(SMC_RMI_VERSION, &res); >> + >> + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) >> + return -ENXIO; >> + >> + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a0); >> + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a0); >> + >> + if (version_major != RMI_ABI_MAJOR_VERSION) { >> + kvm_err("Unsupported RMI ABI (version %d.%d) we support %d\n", > > Can we please replace "we support" to host supports. > Also in the patch present in the repo, you are using variable > our_version, can this be changed to host_version? Sure, I do have a bad habit using "we" - thanks for point it out. Steve >> + version_major, version_minor, >> + RMI_ABI_MAJOR_VERSION); >> + return -ENXIO; >> + } >> + >> + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); >> + >> + return 0; >> +} >> + >> +int kvm_init_rme(void) >> +{ >> + if (PAGE_SIZE != SZ_4K) >> + /* Only 4k page size on the host is supported */ >> + return 0; >> + >> + if (rmi_check_version()) >> + /* Continue without realm support */ >> + return 0; >> + >> + /* Future patch will enable static branch kvm_rme_is_available */ >> + >> + return 0; >> +} > > Thanks, > Ganapat
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 9bdba47f7e14..5a2b7229e83f 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -490,4 +490,21 @@ static inline bool vcpu_has_feature(struct kvm_vcpu *vcpu, int feature) return test_bit(feature, vcpu->arch.features); } +static inline bool kvm_is_realm(struct kvm *kvm) +{ + if (static_branch_unlikely(&kvm_rme_is_available)) + 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 35a159d131b5..04347c3a8c6b 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -26,6 +26,7 @@ #include <asm/fpsimd.h> #include <asm/kvm.h> #include <asm/kvm_asm.h> +#include <asm/kvm_rme.h> #define __KVM_HAVE_ARCH_INTC_INITIALIZED @@ -240,6 +241,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..c26bc2c6770d --- /dev/null +++ b/arch/arm64/include/asm/kvm_rme.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2023 ARM Ltd. + */ + +#ifndef __ASM_KVM_RME_H +#define __ASM_KVM_RME_H + +enum realm_state { + REALM_STATE_NONE, + REALM_STATE_NEW, + REALM_STATE_ACTIVE, + REALM_STATE_DYING +}; + +struct realm { + enum realm_state state; +}; + +int kvm_init_rme(void); + +#endif diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h index 4eb601e7de50..be1383e26626 100644 --- a/arch/arm64/include/asm/virt.h +++ b/arch/arm64/include/asm/virt.h @@ -80,6 +80,7 @@ void __hyp_set_vectors(phys_addr_t phys_vector_base); void __hyp_reset_vectors(void); DECLARE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); +DECLARE_STATIC_KEY_FALSE(kvm_rme_is_available); /* Reports the availability of HYP mode */ static inline bool is_hyp_mode_available(void) diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile index 5e33c2d4645a..d2f0400c50da 100644 --- a/arch/arm64/kvm/Makefile +++ b/arch/arm64/kvm/Makefile @@ -20,7 +20,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 diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 9c5573bc4614..d97b39d042ab 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -38,6 +38,7 @@ #include <asm/kvm_asm.h> #include <asm/kvm_mmu.h> #include <asm/kvm_pkvm.h> +#include <asm/kvm_rme.h> #include <asm/kvm_emulate.h> #include <asm/sections.h> @@ -47,6 +48,7 @@ static enum kvm_mode kvm_mode = KVM_MODE_DEFAULT; DEFINE_STATIC_KEY_FALSE(kvm_protected_mode_initialized); +DEFINE_STATIC_KEY_FALSE(kvm_rme_is_available); DECLARE_KVM_HYP_PER_CPU(unsigned long, kvm_hyp_vector); @@ -2213,6 +2215,12 @@ int kvm_arch_init(void *opaque) in_hyp_mode = is_kernel_in_hyp_mode(); + if (in_hyp_mode) { + err = kvm_init_rme(); + if (err) + return err; + } + 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..f6b587bc116e --- /dev/null +++ b/arch/arm64/kvm/rme.c @@ -0,0 +1,49 @@ +// 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; + + arm_smccc_1_1_invoke(SMC_RMI_VERSION, &res); + + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) + return -ENXIO; + + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a0); + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a0); + + if (version_major != RMI_ABI_MAJOR_VERSION) { + kvm_err("Unsupported RMI ABI (version %d.%d) we support %d\n", + version_major, version_minor, + RMI_ABI_MAJOR_VERSION); + return -ENXIO; + } + + kvm_info("RMI ABI version %d.%d\n", version_major, version_minor); + + return 0; +} + +int kvm_init_rme(void) +{ + if (PAGE_SIZE != SZ_4K) + /* Only 4k page size on the host is supported */ + return 0; + + if (rmi_check_version()) + /* Continue without realm support */ + return 0; + + /* Future patch will enable static branch kvm_rme_is_available */ + + return 0; +}
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> --- arch/arm64/include/asm/kvm_emulate.h | 17 ++++++++++ arch/arm64/include/asm/kvm_host.h | 4 +++ arch/arm64/include/asm/kvm_rme.h | 22 +++++++++++++ arch/arm64/include/asm/virt.h | 1 + arch/arm64/kvm/Makefile | 3 +- arch/arm64/kvm/arm.c | 8 +++++ arch/arm64/kvm/rme.c | 49 ++++++++++++++++++++++++++++ 7 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/include/asm/kvm_rme.h create mode 100644 arch/arm64/kvm/rme.c