Re: [PATCH v2 02/17] arm64: Prevent the use of is_kernel_in_hyp_mode() in hypervisor code

From: Marc Zyngier <maz@kernel.org>
To: Oliver Upton <oliver.upton@linux.dev>
Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Quentin Perret <qperret@google.com>,
	Will Deacon <will@kernel.org>, Fuad Tabba <tabba@google.com>
Subject: Re: [PATCH v2 02/17] arm64: Prevent the use of is_kernel_in_hyp_mode() in hypervisor code
Date: Wed, 31 May 2023 08:17:04 +0100	[thread overview]
Message-ID: <865y89c6mn.wl-maz@kernel.org> (raw)
In-Reply-To: <ZHZUi/4kXxRmCa7a@linux.dev>

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.

-- 
Without deviation from the norm, progress is not possible.