Hi David,
On 2020-06-22 11:20, David Brazdil wrote:
Hi Marc,
quoted
quoted
- void *dst = lm_alias(__bp_harden_hyp_vecs + slot * SZ_2K);
+ char *vec = has_vhe() ? __bp_harden_hyp_vecs
+ : kvm_nvhe_sym(__bp_harden_hyp_vecs);
If we get this construct often, then something that abstracts
the uggliness of the symbol duality would be nice...
Agreed, I do hope that this will end up being limited to finding the address of
the hyp-init vector once EL2 becomes self-contained. Even this vector selection
can be done in EL2 where the symbol duality does not exist.
If we were to hide it, there is a trade off between code "elegance" and clarity
of what's happening under the hood. I was thinking we could extract this
`has_vhe() ? foo : __kvm_nvhe_foo` as a `#define foo` but I do worry that
anybody debugging this code would be cursing my name.
I would say that whoever is debugging this code better have an understanding
of how things are supposed to work. Given that this is only a handful of
people so far, I think your name is safe! ;-)
It would also not work
with other macros that take symbol names, notably kvm_ksym_ref. But that can be
rewritten to accept a pointer instead. The more verbose but less magic approach
is to have a bunch of different helpers for various situations, eg.
__pa_symbol_nvhe. What would be your preference?
I'd be happy with the (maybe temporary) magic approach. It helps reasoning
about things, and makes the transition smoother. Yes, bugs could crop up
there, but given the static nature of obtaining a symbol's address, I'm
fairly confident we'll get it right. The same cannot be said about pointers
though.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hey Marc,
I'd be happy with the (maybe temporary) magic approach. It helps reasoning
about things, and makes the transition smoother. Yes, bugs could crop up
there, but given the static nature of obtaining a symbol's address, I'm
fairly confident we'll get it right. The same cannot be said about pointers
though.
Ok, so this is what it would look like:
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 6a682d66a640..0d1b3b1946f0 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -59,13 +59,14 @@
#define DECLARE_KVM_HYP_SYM(sym) \
DECLARE_KVM_VHE_SYM(sym); \
DECLARE_KVM_NVHE_SYM(sym)
+#define CHOOSE_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) \
+#define kvm_ksym_ref(ptr) \
({ \
- void *val = &sym; \
+ void *val = (ptr); \
if (!is_kernel_in_hyp_mode()) \
- val = lm_alias(&sym); \
+ val = lm_alias((ptr)); \
val; \
})
#define kvm_ksym_ref_nvhe(sym) kvm_ksym_ref(kvm_nvhe_sym(sym))@@ -76,7 +77,14 @@ struct kvm_vcpu;
extern char __kvm_hyp_init[];
extern char __kvm_hyp_init_end[];
-extern char __kvm_hyp_vector[];
+DECLARE_KVM_HYP_SYM(__kvm_hyp_vector);
+#define __kvm_hyp_vector CHOOSE_HYP_SYM(__kvm_hyp_vector)
+
+#ifdef CONFIG_KVM_INDIRECT_VECTORS
+DECLARE_KVM_HYP_SYM(__bp_harden_hyp_vecs);
+#define __bp_harden_hyp_vecs CHOOSE_HYP_SYM(__bp_harden_hyp_vecs)
+extern atomic_t arm64_el2_vector_last_slot;
+#endif
extern void __kvm_flush_vm_context(void);
extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
Everything compiles and boots. Only existing code I had to change was:
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 90cb90561446..34b551385153 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1285,7 +1285,7 @@ static void cpu_init_hyp_mode(void)
* so that we can use adr_l to access per-cpu variables in EL2.
*/
tpidr_el2 = ((unsigned long)this_cpu_ptr(&kvm_host_data) -
- (unsigned long)kvm_ksym_ref(kvm_host_data));
+ (unsigned long)kvm_ksym_ref(&kvm_host_data));
pgd_ptr = kvm_mmu_get_httbr();
hyp_stack_ptr = __this_cpu_read(kvm_arm_hyp_stack_page) + PAGE_SIZE;
WDYT?
Thanks,
-David
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel