| 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: <a31b7ee9ad1edaa38aa122ac90cc605c@kernel.org>
Date: Thu, 18 Jun 2020 17:51:27 +0100
From: Marc Zyngier <maz@...nel.org>
To: David Brazdil <dbrazdil@...gle.com>
Cc: Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
James Morse <james.morse@....com>,
Julien Thierry <julien.thierry.kdev@...il.com>,
Suzuki K Poulose <suzuki.poulose@....com>,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, android-kvm@...gle.com,
kernel-team@...roid.com, Andrew Scull <ascull@...gle.com>
Subject: Re: [PATCH v3 04/15] arm64: kvm: Handle calls to prefixed hyp
functions
Hi David,
On 2020-06-18 13:25, David Brazdil wrote:
> From: Andrew Scull <ascull@...gle.com>
>
> This patch is part of a series which builds KVM's non-VHE hyp code
> separately
> from VHE and the rest of the kernel.
>
> Once hyp functions are moved to a hyp object, they will have prefixed
> symbols.
> This change declares and gets the address of the prefixed version for
> calls to
> the hyp functions.
>
> To aid migration, the hyp functions that have not yet moved have their
> prefixed
> versions aliased to their non-prefixed version. This begins with all
> the hyp
> functions being listed and will reduce to none of them once the
> migration is
> complete.
>
> Signed-off-by: Andrew Scull <ascull@...gle.com>
>
> Extracted kvm_call_hyp nVHE branches into own helper macros.
> Signed-off-by: David Brazdil <dbrazdil@...gle.com>
nit: if you want to add this kind of comment, try to write it between
square brackets, without blank lines in between:
Signed-off-by: Andrew Scull <ascull@...gle.com>
[David: Extracted kvm_call_hyp nVHE branches into own helper macros.]
Signed-off-by: David Brazdil <dbrazdil@...gle.com>
> ---
> arch/arm64/include/asm/kvm_asm.h | 19 +++++++++++++++++++
> arch/arm64/include/asm/kvm_host.h | 19 ++++++++++++++++---
> arch/arm64/kernel/image-vars.h | 15 +++++++++++++++
> 3 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h
> b/arch/arm64/include/asm/kvm_asm.h
> index 352aaebf4198..6a682d66a640 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -42,6 +42,24 @@
>
> #include <linux/mm.h>
>
> +/*
> + * Translate name of a symbol defined in nVHE hyp to the name seen
> + * by kernel proper. All nVHE symbols are prefixed by the build system
> + * to avoid clashes with the VHE variants.
> + */
> +#define kvm_nvhe_sym(sym) __kvm_nvhe_##sym
> +
> +#define DECLARE_KVM_VHE_SYM(sym) extern char sym[]
> +#define DECLARE_KVM_NVHE_SYM(sym) extern char kvm_nvhe_sym(sym)[]
> +
> +/*
> + * Define a pair of symbols sharing the same name but one defined in
> + * VHE and the other in nVHE hyp implementations.
> + */
> +#define DECLARE_KVM_HYP_SYM(sym) \
> + DECLARE_KVM_VHE_SYM(sym); \
> + DECLARE_KVM_NVHE_SYM(sym)
> +
> /* Translate a kernel address of @sym into its equivalent linear
> mapping */
> #define kvm_ksym_ref(sym) \
> ({ \
> @@ -50,6 +68,7 @@
> val = lm_alias(&sym); \
> val; \
> })
> +#define kvm_ksym_ref_nvhe(sym) kvm_ksym_ref(kvm_nvhe_sym(sym))
>
> struct kvm;
> struct kvm_vcpu;
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index c3e6fcc664b1..e782f98243d3 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -448,6 +448,20 @@ void kvm_arm_resume_guest(struct kvm *kvm);
>
> u64 __kvm_call_hyp(void *hypfn, ...);
>
> +#define kvm_call_hyp_nvhe(f, ...) \
> + do { \
> + DECLARE_KVM_NVHE_SYM(f); \
I wanted to move this out to __kvm_call_hyp, but the nVHE ssbs code
got in the way... Oh well.
> + __kvm_call_hyp(kvm_ksym_ref_nvhe(f), ##__VA_ARGS__); \
> + } while(0)
> +
> +#define kvm_call_hyp_nvhe_ret(f, ...) \
> + ({ \
> + DECLARE_KVM_NVHE_SYM(f); \
> + typeof(f(__VA_ARGS__)) ret; \
> + ret = __kvm_call_hyp(kvm_ksym_ref_nvhe(f), \
> + ##__VA_ARGS__); \
You don't need to redefine ret here. actually, you can just evaluate
the expression and let C do its magic:
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index e782f98243d3..49d1a5cd8f8f 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -457,9 +457,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
#define kvm_call_hyp_nvhe_ret(f, ...) \
({ \
DECLARE_KVM_NVHE_SYM(f); \
- typeof(f(__VA_ARGS__)) ret; \
- ret = __kvm_call_hyp(kvm_ksym_ref_nvhe(f), \
- ##__VA_ARGS__); \
+ __kvm_call_hyp(kvm_ksym_ref_nvhe(f), ##__VA_ARGS__); \
})
/*
> + })
> +
> /*
> * The couple of isb() below are there to guarantee the same behaviour
> * on VHE as on !VHE, where the eret to EL1 acts as a context
> @@ -459,7 +473,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
> f(__VA_ARGS__); \
> isb(); \
> } else { \
> - __kvm_call_hyp(kvm_ksym_ref(f), ##__VA_ARGS__); \
> + kvm_call_hyp_nvhe(f, ##__VA_ARGS__); \
> } \
> } while(0)
>
> @@ -471,8 +485,7 @@ u64 __kvm_call_hyp(void *hypfn, ...);
> ret = f(__VA_ARGS__); \
> isb(); \
> } else { \
> - ret = __kvm_call_hyp(kvm_ksym_ref(f), \
> - ##__VA_ARGS__); \
> + ret = kvm_call_hyp_nvhe_ret(f, ##__VA_ARGS__); \
> } \
> \
> ret; \
> diff --git a/arch/arm64/kernel/image-vars.h
> b/arch/arm64/kernel/image-vars.h
> index f32b406e90c0..89affa38b143 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -61,6 +61,21 @@ __efistub__ctype = _ctype;
> * memory mappings.
> */
>
> +__kvm_nvhe___kvm_enable_ssbs = __kvm_enable_ssbs;
> +__kvm_nvhe___kvm_flush_vm_context = __kvm_flush_vm_context;
> +__kvm_nvhe___kvm_get_mdcr_el2 = __kvm_get_mdcr_el2;
> +__kvm_nvhe___kvm_timer_set_cntvoff = __kvm_timer_set_cntvoff;
> +__kvm_nvhe___kvm_tlb_flush_local_vmid = __kvm_tlb_flush_local_vmid;
> +__kvm_nvhe___kvm_tlb_flush_vmid = __kvm_tlb_flush_vmid;
> +__kvm_nvhe___kvm_tlb_flush_vmid_ipa = __kvm_tlb_flush_vmid_ipa;
> +__kvm_nvhe___kvm_vcpu_run_nvhe = __kvm_vcpu_run_nvhe;
> +__kvm_nvhe___vgic_v3_get_ich_vtr_el2 = __vgic_v3_get_ich_vtr_el2;
> +__kvm_nvhe___vgic_v3_init_lrs = __vgic_v3_init_lrs;
> +__kvm_nvhe___vgic_v3_read_vmcr = __vgic_v3_read_vmcr;
> +__kvm_nvhe___vgic_v3_restore_aprs = __vgic_v3_restore_aprs;
> +__kvm_nvhe___vgic_v3_save_aprs = __vgic_v3_save_aprs;
> +__kvm_nvhe___vgic_v3_write_vmcr = __vgic_v3_write_vmcr;
> +
> #endif /* CONFIG_KVM */
>
> #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
Otherwise looks good.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Powered by blists - more mailing lists