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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 16/08/2019 13:10, Mark Brown wrote:
> On Fri, Aug 16, 2019 at 11:24:24AM +0100, Catalin Marinas wrote:
>> On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
> 
>>>> +	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.
> 
> Yes, all this stuff is unfortunately hard to reason about since there's
> several environment changes during boot which have a material effect and
> also multiple different things that might trigger KPTI.  IIRC my thinking
> here was that if we turned on KPTI we're turning it on for all CPUs so
> by the time we could be prempted we'd be returning true from the earlier
> check for arm64_kernel_unmapped_at_el0() but it's possible I missed some
> case there.  I was trying to avoid disturbing the existing code too much
> unless I had a strong reason to on the basis that I might be missing
> something about the way it was done.
> 
>> 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)?
> 
> I've added the check, will look at the reshuffle.
> 
>>> 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()
> 
> Yes, this bit is simple - once we're up and running everything is clear.
> 
>> 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.
> 
> We definitely need some variable I think, and I think you're right that
> making the decision on the boot CPU would simplify things a lot.  The
> systems with very large memories that are most affected by the cost of
> moving from global to non-global mappings are most likely symmetric
> anyway so only looking at the boot CPU should be fine for that.
> 

With KASLR, we already rewrite the page table from __primary_switch() after
relocating the kernel. So, we may be able to perform "raw cpuid check" on
the boot CPU with MMU turned on, before we re-write the pagetables for KASLR
displacement and nG if that is needed (by maybe updating SWWAPPER_MMU_FLAGS) for
the boot CPU and store this information somewhere. Thus we may be able to
avoid another re-write of the pagetables after we have booted the secondaries.

We could continue to do the per-CPU check to see if we need nG mappings
and perform the transition later if needed, like we do now.

Discussing this with Catalin, he suggests to use a variable for the status
of "nG" flag for PTE/PMD_MAYBE_NG, to avoid calling the helper function
all the time. By using the per-CPU check we can make sure the flag is uptodate.

Also, we can continue to fail the hotplugged CPUs if we detect that the 
pagetables are Global and the new CPU requires nG (for heterogeneous systems).

Suzuki


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