| Message ID | 20230526143348.4072074-3-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Allow using VHE in the nVHE hypervisor | expand |
Hi Marc, On Fri, May 26, 2023 at 03:33:33PM +0100, Marc Zyngier wrote: > Using is_kernel_in_hyp_mode() in hypervisor code is a pretty bad > mistake. This helper only checks for CurrentEL being EL2, which > is always true. > > Make the link fail if using the helper in hypervisor context > by referencing a non-existent function. Whilst we're at it, > flag the helper as __always_inline, which it really should be. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/include/asm/virt.h | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h > index 4eb601e7de50..91029709d133 100644 > --- a/arch/arm64/include/asm/virt.h > +++ b/arch/arm64/include/asm/virt.h > @@ -110,8 +110,13 @@ static inline bool is_hyp_mode_mismatched(void) > return __boot_cpu_mode[0] != __boot_cpu_mode[1]; > } > > -static inline bool is_kernel_in_hyp_mode(void) > +extern void gotcha_is_kernel_in_hyp_mode(void); > + > +static __always_inline bool is_kernel_in_hyp_mode(void) > { > +#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__) > + gotcha_is_kernel_in_hyp_mode(); > +#endif > return read_sysreg(CurrentEL) == CurrentEL_EL2; > } Would BUILD_BUG() work in this context, or have I missed something?
On Tue, 30 May 2023 20:54:51 +0100, Oliver Upton <oliver.upton@linux.dev> wrote: > > Hi Marc, > > On Fri, May 26, 2023 at 03:33:33PM +0100, Marc Zyngier wrote: > > Using is_kernel_in_hyp_mode() in hypervisor code is a pretty bad > > mistake. This helper only checks for CurrentEL being EL2, which > > is always true. > > > > Make the link fail if using the helper in hypervisor context > > by referencing a non-existent function. Whilst we're at it, > > flag the helper as __always_inline, which it really should be. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/include/asm/virt.h | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h > > index 4eb601e7de50..91029709d133 100644 > > --- a/arch/arm64/include/asm/virt.h > > +++ b/arch/arm64/include/asm/virt.h > > @@ -110,8 +110,13 @@ static inline bool is_hyp_mode_mismatched(void) > > return __boot_cpu_mode[0] != __boot_cpu_mode[1]; > > } > > > > -static inline bool is_kernel_in_hyp_mode(void) > > +extern void gotcha_is_kernel_in_hyp_mode(void); > > + > > +static __always_inline bool is_kernel_in_hyp_mode(void) > > { > > +#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__) > > + gotcha_is_kernel_in_hyp_mode(); > > +#endif > > return read_sysreg(CurrentEL) == CurrentEL_EL2; > > } > > Would BUILD_BUG() work in this context, or have I missed something? Too obvious? :-) I'll fix that. Thanks, M.
diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h index 4eb601e7de50..91029709d133 100644 --- a/arch/arm64/include/asm/virt.h +++ b/arch/arm64/include/asm/virt.h @@ -110,8 +110,13 @@ static inline bool is_hyp_mode_mismatched(void) return __boot_cpu_mode[0] != __boot_cpu_mode[1]; } -static inline bool is_kernel_in_hyp_mode(void) +extern void gotcha_is_kernel_in_hyp_mode(void); + +static __always_inline bool is_kernel_in_hyp_mode(void) { +#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__) + gotcha_is_kernel_in_hyp_mode(); +#endif return read_sysreg(CurrentEL) == CurrentEL_EL2; }
Using is_kernel_in_hyp_mode() in hypervisor code is a pretty bad mistake. This helper only checks for CurrentEL being EL2, which is always true. Make the link fail if using the helper in hypervisor context by referencing a non-existent function. Whilst we're at it, flag the helper as __always_inline, which it really should be. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/include/asm/virt.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)