| Message ID | 20200903135307.251331-5-ascull@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce separate nVHE hyp context | expand |
Hi Andrew, On Thu, 03 Sep 2020 14:52:53 +0100, Andrew Scull <ascull@google.com> wrote: > > nVHE symbols are prefixed but this is sometimes hidden from the host by > aliasing the non-prefixed symbol to the prefixed version with a macro. > This runs into problems if nVHE tries to use the symbol as it becomes > doubly prefixed. Avoid this by omitting the aliasing macro for nVHE. > > Cc: David Brazdil <dbrazdil@google.com> > Signed-off-by: Andrew Scull <ascull@google.com> > --- > arch/arm64/include/asm/kvm_asm.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > index 6f98fbd0ac81..6f9c4162a764 100644 > --- a/arch/arm64/include/asm/kvm_asm.h > +++ b/arch/arm64/include/asm/kvm_asm.h > @@ -99,8 +99,11 @@ struct kvm_s2_mmu; > > DECLARE_KVM_NVHE_SYM(__kvm_hyp_init); > DECLARE_KVM_HYP_SYM(__kvm_hyp_vector); > + > +#ifndef __KVM_NVHE_HYPERVISOR__ > #define __kvm_hyp_init CHOOSE_NVHE_SYM(__kvm_hyp_init) > #define __kvm_hyp_vector CHOOSE_HYP_SYM(__kvm_hyp_vector) > +#endif Hmmm. Why do we limit this to these two symbols instead of making it a property of the "CHOOSE_*" implementation? The use of CHOOSE_HYP_SYM is already forbidden in the EL2 code (see how any symbol results in __nvhe_undefined_symbol being emitted). Does anything break if we have: #define CHOOSE_NVHE_SYM(x) x when __KVM_NVHE_HYPERVISOR__ is defined? Thanks, M.
On Mon, Sep 07, 2020 at 11:38:38AM +0100, Marc Zyngier wrote: > Hi Andrew, > > On Thu, 03 Sep 2020 14:52:53 +0100, > Andrew Scull <ascull@google.com> wrote: > > > > nVHE symbols are prefixed but this is sometimes hidden from the host by > > aliasing the non-prefixed symbol to the prefixed version with a macro. > > This runs into problems if nVHE tries to use the symbol as it becomes > > doubly prefixed. Avoid this by omitting the aliasing macro for nVHE. > > > > Cc: David Brazdil <dbrazdil@google.com> > > Signed-off-by: Andrew Scull <ascull@google.com> > > --- > > arch/arm64/include/asm/kvm_asm.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h > > index 6f98fbd0ac81..6f9c4162a764 100644 > > --- a/arch/arm64/include/asm/kvm_asm.h > > +++ b/arch/arm64/include/asm/kvm_asm.h > > @@ -99,8 +99,11 @@ struct kvm_s2_mmu; > > > > DECLARE_KVM_NVHE_SYM(__kvm_hyp_init); > > DECLARE_KVM_HYP_SYM(__kvm_hyp_vector); > > + > > +#ifndef __KVM_NVHE_HYPERVISOR__ > > #define __kvm_hyp_init CHOOSE_NVHE_SYM(__kvm_hyp_init) > > #define __kvm_hyp_vector CHOOSE_HYP_SYM(__kvm_hyp_vector) > > +#endif > > Hmmm. Why do we limit this to these two symbols instead of making it a > property of the "CHOOSE_*" implementation? > > The use of CHOOSE_HYP_SYM is already forbidden in the EL2 code (see > how any symbol results in __nvhe_undefined_symbol being emitted). Does > anything break if we have: > > #define CHOOSE_NVHE_SYM(x) x > > when __KVM_NVHE_HYPERVISOR__ is defined? I've specialized the CHOOSE_* macros along the lines you suggested for each of the 3 relevant contexts: host, VHE and nVHE. If you think that's overkill, the host and VHE cases can be merged. diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 6f98fbd0ac81..a952859117b2 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -60,10 +60,24 @@ DECLARE_KVM_VHE_SYM(sym); \ DECLARE_KVM_NVHE_SYM(sym) +#if defined(__KVM_NVHE_HYPERVISOR__) + +#define CHOOSE_HYP_SYM(sym) CHOOSE_NVHE_SYM(sym) +#define CHOOSE_NVHE_SYM(sym) sym +/* The nVHE hypervisor shouldn't even try to access VHE symbols */ +extern void *__nvhe_undefined_symbol; +#define CHOOSE_VHE_SYM(sym) __nvhe_undefined_symbol + +#elif defined(__KVM_VHE_HYPERVISOR) + +#define CHOOSE_HYP_SYM(sym) CHOOSE_VHE_SYM(sym) #define CHOOSE_VHE_SYM(sym) sym -#define CHOOSE_NVHE_SYM(sym) kvm_nvhe_sym(sym) +/* The VHE hypervisor shouldn't even try to access nVHE symbols */ +extern void *__vhe_undefined_symbol; +#define CHOOSE_NVHE_SYM(sym) __vhe_undefined_symbol + +#else -#ifndef __KVM_NVHE_HYPERVISOR__ /* * BIG FAT WARNINGS: * @@ -77,10 +91,9 @@ */ #define CHOOSE_HYP_SYM(sym) (is_kernel_in_hyp_mode() ? CHOOSE_VHE_SYM(sym) \ : CHOOSE_NVHE_SYM(sym)) -#else -/* The nVHE hypervisor shouldn't even try to access anything */ -extern void *__nvhe_undefined_symbol; -#define CHOOSE_HYP_SYM(sym) __nvhe_undefined_symbol +#define CHOOSE_VHE_SYM(sym) sym +#define CHOOSE_NVHE_SYM(sym) kvm_nvhe_sym(sym) + #endif /* Translate a kernel address @ptr into its equivalent linear mapping */
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 6f98fbd0ac81..6f9c4162a764 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -99,8 +99,11 @@ struct kvm_s2_mmu; DECLARE_KVM_NVHE_SYM(__kvm_hyp_init); DECLARE_KVM_HYP_SYM(__kvm_hyp_vector); + +#ifndef __KVM_NVHE_HYPERVISOR__ #define __kvm_hyp_init CHOOSE_NVHE_SYM(__kvm_hyp_init) #define __kvm_hyp_vector CHOOSE_HYP_SYM(__kvm_hyp_vector) +#endif #ifdef CONFIG_KVM_INDIRECT_VECTORS extern atomic_t arm64_el2_vector_last_slot;
nVHE symbols are prefixed but this is sometimes hidden from the host by aliasing the non-prefixed symbol to the prefixed version with a macro. This runs into problems if nVHE tries to use the symbol as it becomes doubly prefixed. Avoid this by omitting the aliasing macro for nVHE. Cc: David Brazdil <dbrazdil@google.com> Signed-off-by: Andrew Scull <ascull@google.com> --- arch/arm64/include/asm/kvm_asm.h | 3 +++ 1 file changed, 3 insertions(+)