Re: [PATCH] arm64: cpufeature: Trim feature reporting and include PAN emulation

[Dave Martin]

On Tue, Feb 20, 2018 at 02:46:24PM -0800, Kees Cook wrote:
> The PAN emulation notification was only happening for non-boot CPUs
> if CPU capabilities had already been configured. This seems to be the
> wrong place, as it's system-wide and isn't attached to capabilities,
> so its reporting didn't normally happen. Instead, report it once from
> the boot CPU. Additionally removes the redundant "feature" word from the
> "CPU features:" line.
> 
> Before (redundant "feature", and missing PAN emulation report):
> 
>  SMP: Total of 4 processors activated.
>  CPU features: detected feature: 32-bit EL0 Support
>  CPU features: detected feature: Kernel page table isolation (KPTI)
>  CPU: All CPU(s) started at EL2
> 
> After:
> 
>  SMP: Total of 4 processors activated.
>  CPU features: detected: 32-bit EL0 Support
>  CPU features: detected: Kernel page table isolation (KPTI)
>  CPU features: emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching
>  CPU: All CPU(s) started at EL2
> 
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>  arch/arm64/kernel/cpufeature.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 29b1f873e337..6c799ca58b53 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1333,9 +1333,6 @@ static void verify_local_cpu_capabilities(void)
>  
>  	if (system_supports_sve())
>  		verify_sve_features();
> -
> -	if (system_uses_ttbr0_pan())
> -		pr_info("Emulating Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
>  }
>  
>  void check_local_cpu_capabilities(void)
> @@ -1360,7 +1357,7 @@ void check_local_cpu_capabilities(void)
>  
>  static void __init setup_feature_capabilities(void)
>  {
> -	update_cpu_capabilities(arm64_features, "detected feature:");
> +	update_cpu_capabilities(arm64_features, "detected:");

Although I get what you're saying about redundant use of the word
"features", this feels like cosmetic churn that is unrelated to the
problem this patch is addressing.

It could be worth reviewing the CPU errata messages and other
miscellaneous printks together to make them less verbose and more
consistent all in one go, but that would be a separate patch...

>  	enable_cpu_capabilities(arm64_features);
>  }
>  
> @@ -1394,6 +1391,9 @@ void __init setup_cpu_features(void)
>  	if (system_supports_32bit_el0())
>  		setup_elf_hwcaps(compat_elf_hwcaps);
>  
> +	if (system_uses_ttbr0_pan())
> +		pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
> +

Moving this seems sensible.  The other option would be to paste it into
update_cpu_capabilities(), but the message would still potentially get
printed multiple times, so that doesn't feel like the right approach.

[...]

Cheers
---Dave