Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD

From: Catalin Marinas <catalin.marinas@arm•com>
To: Will Deacon <will@kernel•org>
Cc: Mark Brown <broonie@kernel•org>,
	linux-arm-kernel@lists•infradead.org,
	Suzuki K Poulose <suzuki.poulose@arm•com>
Subject: Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
Date: Fri, 16 Aug 2019 11:24:24 +0100	[thread overview]
Message-ID: <20190816102424.GA28874@arrakis.emea.arm.com> (raw)
In-Reply-To: <20190815163541.yngqvjmehpuf74ye@willie-the-truck>

On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
> On Wed, Aug 14, 2019 at 07:31:03PM +0100, Mark Brown wrote:
> > diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> > index fd6161336653..85552f6fceda 100644
> > --- a/arch/arm64/include/asm/mmu.h
> > +++ b/arch/arm64/include/asm/mmu.h
> > @@ -38,6 +38,7 @@ static inline bool arm64_kernel_unmapped_at_el0(void)
> >  static inline bool arm64_kernel_use_ng_mappings(void)
> >  {
> >  	bool tx1_bug;
> > +	u64 ftr;
> >  
> >  	/* What's a kpti? Use global mappings if we don't know. */
> >  	if (!IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0))
> > @@ -59,7 +60,7 @@ static inline bool arm64_kernel_use_ng_mappings(void)
> >  	 * KASLR is enabled so we're going to be enabling kpti on non-broken
> >  	 * CPUs regardless of their susceptibility to Meltdown. Rather
> >  	 * than force everybody to go through the G -> nG dance later on,
> > -	 * just put down non-global mappings from the beginning.
> > +	 * just put down non-global mappings from the beginning...
> >  	 */
> >  	if (!IS_ENABLED(CONFIG_CAVIUM_ERRATUM_27456)) {
> >  		tx1_bug = false;
> > @@ -74,6 +75,16 @@ static inline bool arm64_kernel_use_ng_mappings(void)
> >  		tx1_bug = __cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_27456);
> >  	}
> >  
> > +	/*
> > +	 * ...unless we have E0PD in which case we may use that in
> > +	 * preference to unmapping the kernel.
> > +	 */
> > +	if (IS_ENABLED(CONFIG_ARM64_E0PD)) {
> > +		ftr = read_sysreg_s(SYS_ID_AA64MMFR2_EL1);
> > +		if ((ftr >> ID_AA64MMFR2_E0PD_SHIFT) & 0xf)
> > +			return false;
> > +	}

What I don't particularly like here is that on big.LITTLE this hunk may
have a different behaviour depending on which CPU you run it on. In
general, such CPUID access should only be done in a non-preemptible
context.

We probably get away with this during early boot (before CPU caps have
been set up) when arm64_kernel_unmapped_at_el0() is false since we only
have a single CPU running. Later on at run-time, we either have
arm64_kernel_unmapped_at_el0() true, meaning that some CPU is missing
E0PD with kaslr_offset() > 0, or the kernel is mapped at EL0 with all
CPUs having E0PD. But I find it hard to reason about.

Could we move the above hunk in this block:

	} else if (!static_branch_likely(&arm64_const_caps_ready)) {
		...
	}

and reshuffle the rest of the function to only rely on
arm64_kernel_unmapped_at_el0() when the caps are ready (at run-time)?

> > +
> >  	return !tx1_bug && kaslr_offset() > 0;
> 
> I'm still unsure as to how this works with the kaslr check in
> kpti_install_ng_mappings(). Imagine you have a big.LITTLE system using
> kaslr where the boot CPU has E0PD but the secondary CPU doesn't, and
> requires kpti.
> 
> In this case, I think we'll:
> 
> 	1. Start off with global mappings installed by the boot CPU
> 	2. Detect KPTI as being required on the secondary CPU
> 	3. Avoid rewriting the page tables because kaslr_offset > 0
> 
> At this point, we've got exposed global mappings on the secondary CPU.
> 
> Thinking about this further, I think we can simply move all of the
> 'kaslr_offset() > 0' checks used by the kpti code (i.e. in
> arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
> unmap_kernel_at_el0()) into a helper function which does the check for
> E0PD as well. Perhaps 'kaslr_requires_kpti()' ?

I agree, this needs some refactoring as we have this decision in three
separate places.

Trying to put my thoughts together. At run-time, with capabilities fully
enabled, we want:

  arm64_kernel_use_ng_mappings() == arm64_kernel_unmapped_at_el0()

  KPTI is equivalent to arm64_kernel_unmapped_at_el0()

At boot time, it's a best effort but we can only move from G to nG
mappings. We start with nG if the primary CPU requires it to avoid
unnecessary page table rewriting. For your above scenario,
kpti_install_ng_mappings() needs to know the boot CPU G/nG state and
skip the rewriting if already nG. If we have a kaslr_requires_kpti()
that only checks the current CPU, it wouldn't know whether kpti was
already applied at boot.

I think kaslr_requires_kpti() should access the raw CPUID registers (for
E0PD, TX1 bug) and be called only by unmap_kernel_at_el0() and
arm64_kernel_use_ng_mappings(), the latter if !arm64_const_caps_ready.
The boot CPU should store kaslr_requires_kpti() value somewhere and
kpti_install_ng_mappings() should check this variable before deciding to
skip the page table rewrite.

-- 
Catalin

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