[v3,3/7] arm64: kpti: move check for non-vulnerable CPUs to a function

Message ID 20190109235544.2992426-4-jeremy.linton@arm.com
State New
Headers show
Series
  • arm64: add system vulnerability sysfs entries
Related show

Commit Message

Jeremy Linton Jan. 9, 2019, 11:55 p.m.
From: Mian Yousaf Kaukab <ykaukab@suse.de>


Add is_meltdown_safe() which is a whitelist of known safe cores.

Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>

[Moved location of function]
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

---
 arch/arm64/kernel/cpufeature.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

-- 
2.17.2

Comments

Stefan Wahren Jan. 12, 2019, 10:41 a.m. | #1
Hi Jeremy,

> Jeremy Linton <jeremy.linton@arm.com> hat am 10. Januar 2019 um 00:55 geschrieben:

> 

> 

> From: Mian Yousaf Kaukab <ykaukab@suse.de>

> 

> Add is_meltdown_safe() which is a whitelist of known safe cores.

> 

> Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>

> [Moved location of function]

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

> ---

>  arch/arm64/kernel/cpufeature.c | 15 +++++++++++----

>  1 file changed, 11 insertions(+), 4 deletions(-)


i only want to inform you that this patch doesn't cleanly apply against linux-next on Friday.

Best regards
Stefan
Suzuki K Poulose Jan. 14, 2019, 11:32 a.m. | #2
Hi Jeremy,

On 09/01/2019 23:55, Jeremy Linton wrote:
> From: Mian Yousaf Kaukab <ykaukab@suse.de>

> 

> Add is_meltdown_safe() which is a whitelist of known safe cores.

> 

> Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>

> [Moved location of function]

> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

> ---

>   arch/arm64/kernel/cpufeature.c | 15 +++++++++++----

>   1 file changed, 11 insertions(+), 4 deletions(-)

> 

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c

> index 4f272399de89..ab784d7a0083 100644

> --- a/arch/arm64/kernel/cpufeature.c

> +++ b/arch/arm64/kernel/cpufeature.c

> @@ -947,8 +947,7 @@ has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)

>   #ifdef CONFIG_UNMAP_KERNEL_AT_EL0

>   static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */

>   

> -static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

> -				int scope)

> +static bool is_cpu_meltdown_safe(void)

>   {

>   	/* List of CPUs that are not vulnerable and don't need KPTI */

>   	static const struct midr_range kpti_safe_list[] = {

> @@ -962,6 +961,15 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

>   		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),

>   		{ /* sentinel */ }

>   	};

> +	if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))


nit: Does it make sense to rename the list to "meltdown_safe_list", to match the
function name ?

Also also, you may do :

	return is_midr_in_range_list(read_cpuid_id(), kpti_safe_list);

Either way

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Jeremy Linton Jan. 18, 2019, 4:35 p.m. | #3
Hi,

On 01/14/2019 05:32 AM, Suzuki K Poulose wrote:
> Hi Jeremy,

> 

> On 09/01/2019 23:55, Jeremy Linton wrote:

>> From: Mian Yousaf Kaukab <ykaukab@suse.de>

>>

>> Add is_meltdown_safe() which is a whitelist of known safe cores.

>>

>> Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>

>> [Moved location of function]

>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

>> ---

>>   arch/arm64/kernel/cpufeature.c | 15 +++++++++++----

>>   1 file changed, 11 insertions(+), 4 deletions(-)

>>

>> diff --git a/arch/arm64/kernel/cpufeature.c 

>> b/arch/arm64/kernel/cpufeature.c

>> index 4f272399de89..ab784d7a0083 100644

>> --- a/arch/arm64/kernel/cpufeature.c

>> +++ b/arch/arm64/kernel/cpufeature.c

>> @@ -947,8 +947,7 @@ has_useable_cnp(const struct 

>> arm64_cpu_capabilities *entry, int scope)

>>   #ifdef CONFIG_UNMAP_KERNEL_AT_EL0

>>   static int __kpti_forced; /* 0: not forced, >0: forced on, <0: 

>> forced off */

>> -static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities 

>> *entry,

>> -                int scope)

>> +static bool is_cpu_meltdown_safe(void)

>>   {

>>       /* List of CPUs that are not vulnerable and don't need KPTI */

>>       static const struct midr_range kpti_safe_list[] = {

>> @@ -962,6 +961,15 @@ static bool unmap_kernel_at_el0(const struct 

>> arm64_cpu_capabilities *entry,

>>           MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),

>>           { /* sentinel */ }

>>       };

>> +    if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))

> 

> nit: Does it make sense to rename the list to "meltdown_safe_list", to 

> match the

> function name ?

> 

> Also also, you may do :

> 

>      return is_midr_in_range_list(read_cpuid_id(), kpti_safe_list);

> 

> Either way

> 

> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Hi, again.

Part of the delay in responding to this one, has been the fact that 
originally meltodwn_safe() was being used in two places (which is why it 
was broken out). But that isn't true anymore, and this patch is 
effectively just fluff, so it seemed appropriate for the chopping block 
too, which is what i'm planning.

Patch

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4f272399de89..ab784d7a0083 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -947,8 +947,7 @@  has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope)
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */
 
-static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
-				int scope)
+static bool is_cpu_meltdown_safe(void)
 {
 	/* List of CPUs that are not vulnerable and don't need KPTI */
 	static const struct midr_range kpti_safe_list[] = {
@@ -962,6 +961,15 @@  static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
 		{ /* sentinel */ }
 	};
+	if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
+		return true;
+
+	return false;
+}
+
+static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
+				int scope)
+{
 	char const *str = "command line option";
 
 	/*
@@ -985,8 +993,7 @@  static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
 		return true;
 
-	/* Don't force KPTI for CPUs that are not vulnerable */
-	if (is_midr_in_range_list(read_cpuid_id(), kpti_safe_list))
+	if (is_cpu_meltdown_safe())
 		return false;
 
 	/* Defer to CPU feature registers */