| Message ID | 20210315143536.214621-15-qperret@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: A stage 2 for the host | expand |
On Mon, Mar 15, 2021 at 02:35:14PM +0000, Quentin Perret wrote: > We will need to do cache maintenance at EL2 soon, so compile a copy of > __flush_dcache_area at EL2, and provide a copy of arm64_ftr_reg_ctrel0 > as it is needed by the read_ctr macro. > > Signed-off-by: Quentin Perret <qperret@google.com> > --- > arch/arm64/include/asm/kvm_cpufeature.h | 2 ++ > arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++- > arch/arm64/kvm/hyp/nvhe/cache.S | 13 +++++++++++++ > arch/arm64/kvm/sys_regs.c | 1 + > 4 files changed, 18 insertions(+), 1 deletion(-) > create mode 100644 arch/arm64/kvm/hyp/nvhe/cache.S > > diff --git a/arch/arm64/include/asm/kvm_cpufeature.h b/arch/arm64/include/asm/kvm_cpufeature.h > index 3fd9f60d2180..efba1b89b8a4 100644 > --- a/arch/arm64/include/asm/kvm_cpufeature.h > +++ b/arch/arm64/include/asm/kvm_cpufeature.h > @@ -13,3 +13,5 @@ > #define KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg kvm_nvhe_sym(name) > #endif > #endif > + > +KVM_HYP_CPU_FTR_REG(arm64_ftr_reg_ctrel0); I still think this is a bit weird. If you really want to macro-ise stuff, then why not follow the sort of thing we do for e.g. per-cpu variables and have separate DECLARE_HYP_CPU_FTR_REG and DEFINE_HYP_CPU_FTR_REG macros. That way kvm_cpufeature.h can have header guards like a normal header and we can drop the '#ifndef KVM_HYP_CPU_FTR_REG' altogether. I don't think the duplication of the symbol name really matters -- it should fail at build time if something is missing. Will
On Monday 15 Mar 2021 at 16:33:23 (+0000), Will Deacon wrote: > On Mon, Mar 15, 2021 at 02:35:14PM +0000, Quentin Perret wrote: > > We will need to do cache maintenance at EL2 soon, so compile a copy of > > __flush_dcache_area at EL2, and provide a copy of arm64_ftr_reg_ctrel0 > > as it is needed by the read_ctr macro. > > > > Signed-off-by: Quentin Perret <qperret@google.com> > > --- > > arch/arm64/include/asm/kvm_cpufeature.h | 2 ++ > > arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++- > > arch/arm64/kvm/hyp/nvhe/cache.S | 13 +++++++++++++ > > arch/arm64/kvm/sys_regs.c | 1 + > > 4 files changed, 18 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm64/kvm/hyp/nvhe/cache.S > > > > diff --git a/arch/arm64/include/asm/kvm_cpufeature.h b/arch/arm64/include/asm/kvm_cpufeature.h > > index 3fd9f60d2180..efba1b89b8a4 100644 > > --- a/arch/arm64/include/asm/kvm_cpufeature.h > > +++ b/arch/arm64/include/asm/kvm_cpufeature.h > > @@ -13,3 +13,5 @@ > > #define KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg kvm_nvhe_sym(name) > > #endif > > #endif > > + > > +KVM_HYP_CPU_FTR_REG(arm64_ftr_reg_ctrel0); > > I still think this is a bit weird. If you really want to macro-ise stuff, > then why not follow the sort of thing we do for e.g. per-cpu variables and > have separate DECLARE_HYP_CPU_FTR_REG and DEFINE_HYP_CPU_FTR_REG macros. > > That way kvm_cpufeature.h can have header guards like a normal header and > we can drop the '#ifndef KVM_HYP_CPU_FTR_REG' altogether. I don't think > the duplication of the symbol name really matters -- it should fail at > build time if something is missing. I just tend to hate unnecessary boilerplate, but if you feel strongly about it, happy to change :) Cheers, Quentin
On Mon, Mar 15, 2021 at 04:56:21PM +0000, Quentin Perret wrote: > On Monday 15 Mar 2021 at 16:33:23 (+0000), Will Deacon wrote: > > On Mon, Mar 15, 2021 at 02:35:14PM +0000, Quentin Perret wrote: > > > We will need to do cache maintenance at EL2 soon, so compile a copy of > > > __flush_dcache_area at EL2, and provide a copy of arm64_ftr_reg_ctrel0 > > > as it is needed by the read_ctr macro. > > > > > > Signed-off-by: Quentin Perret <qperret@google.com> > > > --- > > > arch/arm64/include/asm/kvm_cpufeature.h | 2 ++ > > > arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++- > > > arch/arm64/kvm/hyp/nvhe/cache.S | 13 +++++++++++++ > > > arch/arm64/kvm/sys_regs.c | 1 + > > > 4 files changed, 18 insertions(+), 1 deletion(-) > > > create mode 100644 arch/arm64/kvm/hyp/nvhe/cache.S > > > > > > diff --git a/arch/arm64/include/asm/kvm_cpufeature.h b/arch/arm64/include/asm/kvm_cpufeature.h > > > index 3fd9f60d2180..efba1b89b8a4 100644 > > > --- a/arch/arm64/include/asm/kvm_cpufeature.h > > > +++ b/arch/arm64/include/asm/kvm_cpufeature.h > > > @@ -13,3 +13,5 @@ > > > #define KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg kvm_nvhe_sym(name) > > > #endif > > > #endif > > > + > > > +KVM_HYP_CPU_FTR_REG(arm64_ftr_reg_ctrel0); > > > > I still think this is a bit weird. If you really want to macro-ise stuff, > > then why not follow the sort of thing we do for e.g. per-cpu variables and > > have separate DECLARE_HYP_CPU_FTR_REG and DEFINE_HYP_CPU_FTR_REG macros. > > > > That way kvm_cpufeature.h can have header guards like a normal header and > > we can drop the '#ifndef KVM_HYP_CPU_FTR_REG' altogether. I don't think > > the duplication of the symbol name really matters -- it should fail at > > build time if something is missing. > > I just tend to hate unnecessary boilerplate, but if you feel strongly > about it, happy to change :) I don't like it either, but I prefer it to overriding macros like this! I think having the "boilerplate" is a better starting point should we decide to consolidate the definitions somehow. Will
diff --git a/arch/arm64/include/asm/kvm_cpufeature.h b/arch/arm64/include/asm/kvm_cpufeature.h index 3fd9f60d2180..efba1b89b8a4 100644 --- a/arch/arm64/include/asm/kvm_cpufeature.h +++ b/arch/arm64/include/asm/kvm_cpufeature.h @@ -13,3 +13,5 @@ #define KVM_HYP_CPU_FTR_REG(name) extern struct arm64_ftr_reg kvm_nvhe_sym(name) #endif #endif + +KVM_HYP_CPU_FTR_REG(arm64_ftr_reg_ctrel0); diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile index 6894a917f290..42dde4bb80b1 100644 --- a/arch/arm64/kvm/hyp/nvhe/Makefile +++ b/arch/arm64/kvm/hyp/nvhe/Makefile @@ -13,7 +13,8 @@ lib-objs := clear_page.o copy_page.o memcpy.o memset.o lib-objs := $(addprefix ../../../lib/, $(lib-objs)) obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \ - hyp-main.o hyp-smp.o psci-relay.o early_alloc.o stub.o page_alloc.o + hyp-main.o hyp-smp.o psci-relay.o early_alloc.o stub.o page_alloc.o \ + cache.o obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \ ../fpsimd.o ../hyp-entry.o ../exception.o obj-y += $(lib-objs) diff --git a/arch/arm64/kvm/hyp/nvhe/cache.S b/arch/arm64/kvm/hyp/nvhe/cache.S new file mode 100644 index 000000000000..36cef6915428 --- /dev/null +++ b/arch/arm64/kvm/hyp/nvhe/cache.S @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Code copied from arch/arm64/mm/cache.S. + */ + +#include <linux/linkage.h> +#include <asm/assembler.h> +#include <asm/alternative.h> + +SYM_FUNC_START_PI(__flush_dcache_area) + dcache_by_line_op civac, sy, x0, x1, x2, x3 + ret +SYM_FUNC_END_PI(__flush_dcache_area) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 6c5d133689ae..3ec34c25e877 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2783,6 +2783,7 @@ struct __ftr_reg_copy_entry { u32 sys_id; struct arm64_ftr_reg *dst; } hyp_ftr_regs[] __initdata = { + CPU_FTR_REG_HYP_COPY(SYS_CTR_EL0, arm64_ftr_reg_ctrel0), }; void __init setup_kvm_el2_caps(void)
We will need to do cache maintenance at EL2 soon, so compile a copy of __flush_dcache_area at EL2, and provide a copy of arm64_ftr_reg_ctrel0 as it is needed by the read_ctr macro. Signed-off-by: Quentin Perret <qperret@google.com> --- arch/arm64/include/asm/kvm_cpufeature.h | 2 ++ arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++- arch/arm64/kvm/hyp/nvhe/cache.S | 13 +++++++++++++ arch/arm64/kvm/sys_regs.c | 1 + 4 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/kvm/hyp/nvhe/cache.S