Re: [PATCH 2/6] arm64: add sysfs vulnerability show for meltdown

[Julien Thierry <julien.thierry@arm.com>]

Hi Jeremy,

On 06/12/2018 23:44, Jeremy Linton wrote:
> Add a simple state machine which will track whether
> all the online cores in a machine are vulnerable.
> 
> Once that is done we have a fairly authoritative view
> of the machine vulnerability, which allows us to make a
> judgment about machine safety if it hasn't been mitigated.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  arch/arm64/kernel/cpufeature.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 242898395f68..bea9adfef7fa 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -905,6 +905,8 @@ has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
>  	return has_cpuid_feature(entry, scope);
>  }
>  
> +static enum { A64_MELT_UNSET, A64_MELT_SAFE, A64_MELT_UNKN } __meltdown_safe = A64_MELT_UNSET;
> +

I'm wondering, do we really need that tri state?

Can't we consider that we are safe an move to unsafe/unkown if any cpu
during bring up is not in the safe list?

The only user of this is cpu_show_meltdown, but I don't imagine it'll
get called before unmap_kernel_at_el0() is called for the boot CPU which
should initialise that state.

Or is there another reason for having that UNSET state?

Thanks,

>  #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>  static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
>  
> @@ -928,6 +930,15 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
>  {
>  	char const *str = "command line option";
>  
> +	bool meltdown_safe = is_cpu_meltdown_safe() ||
> +		has_cpuid_feature(entry, scope);
> +
> +	/* Only safe if all booted cores are known safe */
> +	if (meltdown_safe && __meltdown_safe == A64_MELT_UNSET)
> +		__meltdown_safe = A64_MELT_SAFE;
> +	else if (!meltdown_safe)
> +		__meltdown_safe = A64_MELT_UNKN;
> +
>  	/*
>  	 * For reasons that aren't entirely clear, enabling KPTI on Cavium
>  	 * ThunderX leads to apparent I-cache corruption of kernel text, which
> @@ -949,11 +960,7 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
>  	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
>  		return true;
>  
> -	if (is_cpu_meltdown_safe())
> -		return false;
> -
> -	/* Defer to CPU feature registers */
> -	return !has_cpuid_feature(entry, scope);
> +	return !meltdown_safe;
>  }
>  
>  static void
> @@ -1920,3 +1927,17 @@ static int __init enable_mrs_emulation(void)
>  }
>  
>  core_initcall(enable_mrs_emulation);
> +
> +#ifdef CONFIG_GENERIC_CPU_VULNERABILITIES
> +ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr,
> +		char *buf)
> +{
> +	if (arm64_kernel_unmapped_at_el0())
> +		return sprintf(buf, "Mitigation: KPTI\n");
> +
> +	if (__meltdown_safe == A64_MELT_SAFE)
> +		return sprintf(buf, "Not affected\n");
> +
> +	return sprintf(buf, "Unknown\n");
> +}
> +#endif
> 

-- 
Julien Thierry