Re: [PATCH v2 1/3] arm64: Avoid enabling KPTI unnecessarily

From: Will Deacon <will@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Ard Biesheuvel <ardb@google.com>,
	linux-arm-kernel@lists.infradead.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH v2 1/3] arm64: Avoid enabling KPTI unnecessarily
Date: Mon, 27 Nov 2023 16:31:03 +0000	[thread overview]
Message-ID: <20231127163103.GA8627@willie-the-truck> (raw)
In-Reply-To: <CAMj1kXFpN8NsS6a9XmBOQ+JybpJtbBb87gyvD5f7Ng4cFs7Sxg@mail.gmail.com>

On Mon, Nov 27, 2023 at 04:52:11PM +0100, Ard Biesheuvel wrote:
> On Mon, 27 Nov 2023 at 16:48, Will Deacon <will@kernel.org> wrote:
> >
> > On Mon, Nov 27, 2023 at 01:00:51PM +0100, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Commit 42c5a3b04bf6 refactored the KPTI init code in a way that results
> > > in the use of non-global kernel mappings even on systems that have no
> > > need for it, and even when KPTI has been disabled explicitly via the
> > > command line.
> > >
> > > Ensure that this only happens when we have decided (based on the
> > > detected system-wide CPU features) that KPTI should be enabled.
> > >
> > > Fixes: 42c5a3b04bf6 ("arm64: Split kpti_install_ng_mappings()")
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  arch/arm64/kernel/cpufeature.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index 646591c67e7a..91d2d6714969 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -1839,6 +1839,10 @@ static int __init __kpti_install_ng_mappings(void *__unused)
> > >
> > >  static void __init kpti_install_ng_mappings(void)
> > >  {
> > > +     /* Check whether KPTI is going to be used */
> > > +     if (!cpus_have_cap(ARM64_UNMAP_KERNEL_AT_EL0))
> > > +             return;
> >
> > Why can't you use arm64_kernel_unmapped_at_el0() here?
> >
> 
> Because it relies on alternatives patching, which hasn't occurred yet
> at this point.

Hmm. Keeping the determination of the capabilities separate from the
alternatives patching feels like it's asking for trouble given how
many of the boolean system_*() helpers in asm/cpufeature.h are using
the alternative_has_cap*() code.

Could we move the call to apply_alternatives_all() into
setup_system_features() and then you could do the kpti stuff after that?
I think sve_setup() and sme_setup() are ok, but I'd be more comfortable
moving those later too, given how things like system_supports_sve() rely
on the alternatives as well.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel