| Message ID | 20210319100146.1149909-14-qperret@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Stage-2 for the host | expand |
On Fri, Mar 19, 2021 at 10:01:21AM +0000, Quentin Perret wrote: > Introduce the infrastructure in KVM enabling to copy CPU feature > registers into EL2-owned data-structures, to allow reading sanitised > values directly at EL2 in nVHE. > > Given that only a subset of these features are being read by the > hypervisor, the ones that need to be copied are to be listed under > <asm/kvm_cpufeature.h> together with the name of the nVHE variable that > will hold the copy. This introduces only the infrastructure enabling > this copy. The first users will follow shortly. > > Signed-off-by: Quentin Perret <qperret@google.com> > --- > arch/arm64/include/asm/cpufeature.h | 1 + > arch/arm64/include/asm/kvm_cpufeature.h | 22 ++++++++++++++++++++++ > arch/arm64/include/asm/kvm_host.h | 4 ++++ > arch/arm64/kernel/cpufeature.c | 13 +++++++++++++ > arch/arm64/kvm/sys_regs.c | 19 +++++++++++++++++++ > 5 files changed, 59 insertions(+) > create mode 100644 arch/arm64/include/asm/kvm_cpufeature.h Acked-by: Will Deacon <will@kernel.org> Will
Hi Quentin, On Fri, 19 Mar 2021 10:01:21 +0000, Quentin Perret <qperret@google.com> wrote: > > Introduce the infrastructure in KVM enabling to copy CPU feature > registers into EL2-owned data-structures, to allow reading sanitised > values directly at EL2 in nVHE. > > Given that only a subset of these features are being read by the > hypervisor, the ones that need to be copied are to be listed under > <asm/kvm_cpufeature.h> together with the name of the nVHE variable that > will hold the copy. This introduces only the infrastructure enabling > this copy. The first users will follow shortly. > > Signed-off-by: Quentin Perret <qperret@google.com> > --- > arch/arm64/include/asm/cpufeature.h | 1 + > arch/arm64/include/asm/kvm_cpufeature.h | 22 ++++++++++++++++++++++ > arch/arm64/include/asm/kvm_host.h | 4 ++++ > arch/arm64/kernel/cpufeature.c | 13 +++++++++++++ > arch/arm64/kvm/sys_regs.c | 19 +++++++++++++++++++ > 5 files changed, 59 insertions(+) > create mode 100644 arch/arm64/include/asm/kvm_cpufeature.h > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index 61177bac49fa..a85cea2cac57 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -607,6 +607,7 @@ void check_local_cpu_capabilities(void); > > u64 read_sanitised_ftr_reg(u32 id); > u64 __read_sysreg_by_encoding(u32 sys_id); > +int copy_ftr_reg(u32 id, struct arm64_ftr_reg *dst); > > static inline bool cpu_supports_mixed_endian_el0(void) > { > diff --git a/arch/arm64/include/asm/kvm_cpufeature.h b/arch/arm64/include/asm/kvm_cpufeature.h > new file mode 100644 > index 000000000000..3d245f96a9fe > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_cpufeature.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2020 - Google LLC > + * Author: Quentin Perret <qperret@google.com> > + */ > + > +#ifndef __ARM64_KVM_CPUFEATURE_H__ > +#define __ARM64_KVM_CPUFEATURE_H__ > + > +#include <asm/cpufeature.h> > + > +#include <linux/build_bug.h> > + > +#if defined(__KVM_NVHE_HYPERVISOR__) > +#define DECLARE_KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg name > +#define DEFINE_KVM_HYP_CPU_FTR_REG(name) struct arm64_ftr_reg name > +#else > +#define DECLARE_KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg kvm_nvhe_sym(name) > +#define DEFINE_KVM_HYP_CPU_FTR_REG(name) BUILD_BUG() > +#endif > + > +#endif > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 6a2031af9562..02e172dc5087 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -740,9 +740,13 @@ void kvm_clr_pmu_events(u32 clr); > > void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu); > void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu); > + > +void setup_kvm_el2_caps(void); > #else > static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {} > static inline void kvm_clr_pmu_events(u32 clr) {} > + > +static inline void setup_kvm_el2_caps(void) {} > #endif > > void kvm_vcpu_load_sysregs_vhe(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 066030717a4c..6252476e4e73 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1154,6 +1154,18 @@ u64 read_sanitised_ftr_reg(u32 id) > } > EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg); > > +int copy_ftr_reg(u32 id, struct arm64_ftr_reg *dst) > +{ > + struct arm64_ftr_reg *regp = get_arm64_ftr_reg(id); > + > + if (!regp) > + return -EINVAL; > + > + *dst = *regp; > + > + return 0; > +} > + > #define read_sysreg_case(r) \ > case r: val = read_sysreg_s(r); break; > > @@ -2773,6 +2785,7 @@ void __init setup_cpu_features(void) > > setup_system_capabilities(); > setup_elf_hwcaps(arm64_elf_hwcaps); > + setup_kvm_el2_caps(); > > if (system_supports_32bit_el0()) > setup_elf_hwcaps(compat_elf_hwcaps); > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 4f2f1e3145de..6c5d133689ae 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -21,6 +21,7 @@ > #include <asm/debug-monitors.h> > #include <asm/esr.h> > #include <asm/kvm_arm.h> > +#include <asm/kvm_cpufeature.h> > #include <asm/kvm_emulate.h> > #include <asm/kvm_hyp.h> > #include <asm/kvm_mmu.h> > @@ -2775,3 +2776,21 @@ void kvm_sys_reg_table_init(void) > /* Clear all higher bits. */ > cache_levels &= (1 << (i*3))-1; > } > + > +#define CPU_FTR_REG_HYP_COPY(id, name) \ > + { .sys_id = id, .dst = (struct arm64_ftr_reg *)&kvm_nvhe_sym(name) } > +struct __ftr_reg_copy_entry { > + u32 sys_id; > + struct arm64_ftr_reg *dst; > +} hyp_ftr_regs[] __initdata = { > +}; > + > +void __init setup_kvm_el2_caps(void) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(hyp_ftr_regs); i++) { > + WARN(copy_ftr_reg(hyp_ftr_regs[i].sys_id, hyp_ftr_regs[i].dst), > + "%u feature register not found\n", hyp_ftr_regs[i].sys_id); > + } > +} > -- > 2.31.0.rc2.261.g7f71774620-goog > > I can't say I'm thrilled with this. Actually, it is fair to say that I don't like it at all! ;-) Copying whole structures with pointers that make no sense at EL2 feels... wrong. As we discussed offline, the main reason for this infrastructure is that the read_ctr macro directly uses arm64_ftr_reg_ctrel0.sys_val when ARM64_MISMATCHED_CACHE_TYPE is set. One thing to realise is that with the protected mode, we can rely on patching as there is no such thing as a "late" CPU. So by specialising read_ctr when compiled for nVHE, we can just make it give us the final value, provided that KVM's own __flush_dcache_area() is limited to protected mode. Once this problem is solved, this whole patch can mostly go, as we are left with exactly *two* u64 quantities to be populated, something that we can probably do in kvm_sys_reg_table_init(). I'll post some patches later today to try and explain what I have in mind. Thanks, M.
Hey Marc, On Monday 22 Mar 2021 at 13:44:38 (+0000), Marc Zyngier wrote: > I can't say I'm thrilled with this. Actually, it is fair to say that I > don't like it at all! ;-) :-) > Copying whole structures with pointers that > make no sense at EL2 feels... wrong. And I don't disagree at all. I tried to keep this as small as possible as the series is already quite intrusive, but I certainly understand the concern. > As we discussed offline, the main reason for this infrastructure is > that the read_ctr macro directly uses arm64_ftr_reg_ctrel0.sys_val > when ARM64_MISMATCHED_CACHE_TYPE is set. Indeed that is the only reason. > One thing to realise is that with the protected mode, we can rely on > patching as there is no such thing as a "late" CPU. So by specialising > read_ctr when compiled for nVHE, we can just make it give us the final > value, provided that KVM's own __flush_dcache_area() is limited to > protected mode. > > Once this problem is solved, this whole patch can mostly go, as we are > left with exactly *two* u64 quantities to be populated, something that > we can probably do in kvm_sys_reg_table_init(). > > I'll post some patches later today to try and explain what I have in > mind. Sounds great, thank you very much for the help! Quentin
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 61177bac49fa..a85cea2cac57 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -607,6 +607,7 @@ void check_local_cpu_capabilities(void); u64 read_sanitised_ftr_reg(u32 id); u64 __read_sysreg_by_encoding(u32 sys_id); +int copy_ftr_reg(u32 id, struct arm64_ftr_reg *dst); static inline bool cpu_supports_mixed_endian_el0(void) { diff --git a/arch/arm64/include/asm/kvm_cpufeature.h b/arch/arm64/include/asm/kvm_cpufeature.h new file mode 100644 index 000000000000..3d245f96a9fe --- /dev/null +++ b/arch/arm64/include/asm/kvm_cpufeature.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2020 - Google LLC + * Author: Quentin Perret <qperret@google.com> + */ + +#ifndef __ARM64_KVM_CPUFEATURE_H__ +#define __ARM64_KVM_CPUFEATURE_H__ + +#include <asm/cpufeature.h> + +#include <linux/build_bug.h> + +#if defined(__KVM_NVHE_HYPERVISOR__) +#define DECLARE_KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg name +#define DEFINE_KVM_HYP_CPU_FTR_REG(name) struct arm64_ftr_reg name +#else +#define DECLARE_KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg kvm_nvhe_sym(name) +#define DEFINE_KVM_HYP_CPU_FTR_REG(name) BUILD_BUG() +#endif + +#endif diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 6a2031af9562..02e172dc5087 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -740,9 +740,13 @@ void kvm_clr_pmu_events(u32 clr); void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu); void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu); + +void setup_kvm_el2_caps(void); #else static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {} static inline void kvm_clr_pmu_events(u32 clr) {} + +static inline void setup_kvm_el2_caps(void) {} #endif void kvm_vcpu_load_sysregs_vhe(struct kvm_vcpu *vcpu); diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 066030717a4c..6252476e4e73 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1154,6 +1154,18 @@ u64 read_sanitised_ftr_reg(u32 id) } EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg); +int copy_ftr_reg(u32 id, struct arm64_ftr_reg *dst) +{ + struct arm64_ftr_reg *regp = get_arm64_ftr_reg(id); + + if (!regp) + return -EINVAL; + + *dst = *regp; + + return 0; +} + #define read_sysreg_case(r) \ case r: val = read_sysreg_s(r); break; @@ -2773,6 +2785,7 @@ void __init setup_cpu_features(void) setup_system_capabilities(); setup_elf_hwcaps(arm64_elf_hwcaps); + setup_kvm_el2_caps(); if (system_supports_32bit_el0()) setup_elf_hwcaps(compat_elf_hwcaps); diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 4f2f1e3145de..6c5d133689ae 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -21,6 +21,7 @@ #include <asm/debug-monitors.h> #include <asm/esr.h> #include <asm/kvm_arm.h> +#include <asm/kvm_cpufeature.h> #include <asm/kvm_emulate.h> #include <asm/kvm_hyp.h> #include <asm/kvm_mmu.h> @@ -2775,3 +2776,21 @@ void kvm_sys_reg_table_init(void) /* Clear all higher bits. */ cache_levels &= (1 << (i*3))-1; } + +#define CPU_FTR_REG_HYP_COPY(id, name) \ + { .sys_id = id, .dst = (struct arm64_ftr_reg *)&kvm_nvhe_sym(name) } +struct __ftr_reg_copy_entry { + u32 sys_id; + struct arm64_ftr_reg *dst; +} hyp_ftr_regs[] __initdata = { +}; + +void __init setup_kvm_el2_caps(void) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(hyp_ftr_regs); i++) { + WARN(copy_ftr_reg(hyp_ftr_regs[i].sys_id, hyp_ftr_regs[i].dst), + "%u feature register not found\n", hyp_ftr_regs[i].sys_id); + } +}
Introduce the infrastructure in KVM enabling to copy CPU feature registers into EL2-owned data-structures, to allow reading sanitised values directly at EL2 in nVHE. Given that only a subset of these features are being read by the hypervisor, the ones that need to be copied are to be listed under <asm/kvm_cpufeature.h> together with the name of the nVHE variable that will hold the copy. This introduces only the infrastructure enabling this copy. The first users will follow shortly. Signed-off-by: Quentin Perret <qperret@google.com> --- arch/arm64/include/asm/cpufeature.h | 1 + arch/arm64/include/asm/kvm_cpufeature.h | 22 ++++++++++++++++++++++ arch/arm64/include/asm/kvm_host.h | 4 ++++ arch/arm64/kernel/cpufeature.c | 13 +++++++++++++ arch/arm64/kvm/sys_regs.c | 19 +++++++++++++++++++ 5 files changed, 59 insertions(+) create mode 100644 arch/arm64/include/asm/kvm_cpufeature.h