| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87a72jj1sh.wl-maz@kernel.org>
Date: Thu, 07 May 2020 14:46:22 +0100
From: Marc Zyngier <maz@...nel.org>
To: David Brazdil <dbrazdil@...gle.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
James Morse <james.morse@....com>,
Julien Thierry <julien.thierry.kdev@...il.com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Will Deacon <will@...nel.org>, kvmarm@...ts.cs.columbia.edu,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/15] arm64: kvm: Add build rules for separate nVHE object files
On Thu, 30 Apr 2020 15:48:20 +0100,
David Brazdil <dbrazdil@...gle.com> wrote:
>
> Add new folder arch/arm64/kvm/hyp/nvhe and a Makefile for building code that
s/folder/directory/
> runs in EL2 under nVHE KVM.
>
> Compile each source file into a `.hyp.tmp.o` object first, then prefix all
> its symbols with "__hyp_text_" using `objcopy` and produce a `.hyp.o`.
Madness. ;-)
> Suffixes were chosen so that it would be possible for VHE and nVHE to share
> some source files, but compiled with different CFLAGS. nVHE build rules add
> -D__HYPERVISOR__.
>
> Macros are added for prefixing a nVHE symbol name when referenced from kernel
> proper.
>
> Signed-off-by: David Brazdil <dbrazdil@...gle.com>
> ---
> arch/arm64/include/asm/kvm_asm.h | 13 ++++++++++++
> arch/arm64/kernel/image-vars.h | 12 +++++++++++
> arch/arm64/kvm/hyp/Makefile | 4 ++--
> arch/arm64/kvm/hyp/nvhe/Makefile | 35 ++++++++++++++++++++++++++++++++
> scripts/kallsyms.c | 1 +
> 5 files changed, 63 insertions(+), 2 deletions(-)
> create mode 100644 arch/arm64/kvm/hyp/nvhe/Makefile
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 7c7eeeaab9fa..99ab204519ca 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -42,6 +42,12 @@
>
> #include <linux/mm.h>
>
> +/* Translate the name of @sym to its nVHE equivalent. */
> +#define kvm_nvhe_sym(sym) __hyp_text_##sym
> +
> +/* Choose between VHE and nVHE variants of a symbol. */
> +#define kvm_hyp_sym(sym) (has_vhe() ? sym : kvm_nvhe_sym(sym))
> +
> /* Translate a kernel address of @sym into its equivalent linear mapping */
> #define kvm_ksym_ref(sym) \
> ({ \
> @@ -51,6 +57,13 @@
> val; \
> })
>
> +/*
> + * Translate a kernel address of @sym into its equivalent linear mapping,
> + * choosing between VHE and nVHE variant of @sym accordingly.
> + */
> +#define kvm_hyp_ref(sym) \
> + (has_vhe() ? kvm_ksym_ref(sym) : kvm_ksym_ref(kvm_nvhe_sym(sym)))
> +
Things are becoming a bit redundant here: you have a has_vhe() test,
followed by a is_kernel_in_hyp_mode() test in kvm_ksym_ref(). Certainly
this could do with some simplification. Another thing to think about is
whether has_vhe() is the right thing to use as it isn't valid until you
have discovered the capabilities. I think it is fine, but you should
check it.
> struct kvm;
> struct kvm_vcpu;
>
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 7f06ad93fc95..13850134fc28 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -51,4 +51,16 @@ __efistub__ctype = _ctype;
>
> #endif
>
> +#ifdef CONFIG_KVM
> +
> +/*
> + * KVM nVHE code has its own symbol namespace prefixed by __hyp_text_, to
> + * isolate it from the kernel proper. The following symbols are legally
> + * accessed by it, therefore provide aliases to make them linkable.
> + * Do not include symbols which may not be safely accessed under hypervisor
> + * memory mappings.
> + */
> +
> +#endif /* CONFIG_KVM */
> +
> #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> index 29e2b2cd2fbc..79bf822a484b 100644
> --- a/arch/arm64/kvm/hyp/Makefile
> +++ b/arch/arm64/kvm/hyp/Makefile
> @@ -6,9 +6,9 @@
> ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
> $(DISABLE_STACKLEAK_PLUGIN)
>
> -obj-$(CONFIG_KVM) += hyp.o
> +obj-$(CONFIG_KVM) += vhe.o nvhe/
>
> -hyp-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o sysreg-sr.o \
> +vhe-y := vgic-v3-sr.o timer-sr.o aarch32.o vgic-v2-cpuif-proxy.o sysreg-sr.o \
> debug-sr.o entry.o switch.o fpsimd.o tlb.o host_hypercall.o hyp-entry.o
>
> # KVM code is run at a different exception code with a different map, so
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> new file mode 100644
> index 000000000000..873d3ab0fd68
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for Kernel-based Virtual Machine module, HYP/nVHE part
> +#
> +
> +asflags-y := -D__HYPERVISOR__
> +ccflags-y := -D__HYPERVISOR__ -fno-stack-protector -DDISABLE_BRANCH_PROFILING \
> + $(DISABLE_STACKLEAK_PLUGIN)
> +
> +obj-y :=
> +
> +obj-y := $(patsubst %.o,%.hyp.o,$(obj-y))
> +extra-y := $(patsubst %.hyp.o,%.hyp.tmp.o,$(obj-y))
> +
> +$(obj)/%.hyp.tmp.o: $(src)/%.c FORCE
> + $(call if_changed_rule,cc_o_c)
> +$(obj)/%.hyp.tmp.o: $(src)/%.S FORCE
> + $(call if_changed_rule,as_o_S)
> +$(obj)/%.hyp.o: $(obj)/%.hyp.tmp.o FORCE
> + $(call if_changed,hypcopy)
> +
> +quiet_cmd_hypcopy = HYPCOPY $@
> + cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__hyp_text_ $< $@
> +
> +# KVM nVHE code is run at a different exception code with a different map, so
s/exception code/exception level/
s/map/memory map/
> +# compiler instrumentation that inserts callbacks or checks into the code may
> +# cause crashes. Just disable it.
> +GCOV_PROFILE := n
> +KASAN_SANITIZE := n
> +UBSAN_SANITIZE := n
> +KCOV_INSTRUMENT := n
> +
> +# Skip objtool checking for this directory because nVHE code is compiled with
> +# non-standard build rules.
> +OBJECT_FILES_NON_STANDARD := y
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 3e8dea6e0a95..b5c9dc6de38d 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -109,6 +109,7 @@ static bool is_ignored_symbol(const char *name, char type)
> ".LASANPC", /* s390 kasan local symbols */
> "__crc_", /* modversions */
> "__efistub_", /* arm64 EFI stub namespace */
> + "__hyp_text_", /* arm64 non-VHE KVM namespace */
> NULL
> };
Could you add a small comment to the commit message as to what this is
for? It certainly piqued my curiosity, and keeping a record of it would
be nice.
>
> --
> 2.26.1
>
>
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists