Skip to contentSkip to conversation

x86/mtrr: Check if fixed MTRRs exist before saving them

PATCH Superseded Andi Kleen 2 participants 3 messages Jan 10, 2024 – Feb 28, 2024 x86

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH v2 1/1 x86/mtrr: Check if fixed MTRRs exist before saving them

This one place forgot to check the fixed capability. Otherwise there will a harmless but ugly WARN_ON for the failed WRMSR when the MSR doesn't exist. Noticed when testing X86S support.

Fixes 2b1f6278d77c ("[PATCH] x86: Save the MTRRs of the BSP before")

---

v2: Add Fixes tag and expand description.

---
 arch/x86/kernel/cpu/mtrr/mtrr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/cpu/mtrr/mtrr.c +1 -1
index 767bf1c71aad..2a2fc14955cd 100644
@@ -609,7 +609,7 @@ void mtrr_save_state(void)
 {
 	int first_cpu;
 
-	if (!mtrr_enabled())
+	if (!mtrr_enabled() || !mtrr_state.have_fixed)
 		return;
 
 	first_cpu = cpumask_first(cpu_online_mask);
Signature · 1 line
2.43.0
author
#1 lore raw
Quoted 10 lines · Andi Kleen

This one place forgot to check the fixed capability. Otherwise there will a harmless but ugly WARN_ON for the failed WRMSR when the MSR doesn't exist. Noticed when testing X86S support.

Fixes 2b1f6278d77c ("[PATCH] x86: Save the MTRRs of the BSP before")

---

v2: Add Fixes tag and expand description.

Just wanted to ping the patch. Thanks.

-Andi

Quoted 20 lines
---
 arch/x86/kernel/cpu/mtrr/mtrr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/cpu/mtrr/mtrr.c +1 -1
index 767bf1c71aad..2a2fc14955cd 100644
@@ -609,7 +609,7 @@ void mtrr_save_state(void)
 {
 	int first_cpu;
 
-	if (!mtrr_enabled())
+	if (!mtrr_enabled() || !mtrr_state.have_fixed)
 		return;
 
 	first_cpu = cpumask_first(cpu_online_mask);
Signature · 1 line
2.43.0
#2 lore raw
In reply to Andi Kleen
Quoted 1 line · Andi Kleen

This one place forgot to check the fixed capability.

This one place forgot? And what the heck is a fixed capability?

Quoted 2 lines

Otherwise there will a harmless but ugly WARN_ON for the failed WRMSR when the MSR doesn't exist.

This is not a sentence as it lacks a verb,

Also it does not matter whether you think that the WARN_ON() is ugly or not. Can you please write a changelog in technical terms?

Quoted 1 line

Noticed when testing X86S support.

Who cares?

Quoted 1 line
Fixes 2b1f6278d77c ("[PATCH] x86: Save the MTRRs of the BSP before")

This fixes tag is pointlessly truncated. The correct one is:

Fixes 2b1f6278d77c ("[PATCH] x86: Save the MTRRs of the BSP before booting an AP")

It's not rocket science to copy & pasta correctly.

Thanks,

tglx