2024-02-29 00:24:46

by Andi Kleen

[permalink] [raw]
Subject: [PATCH v3] x86/mtrr: Check if fixed MTRRs exist before saving them

MTRRs have a obsolete fixed variant for fine grained
caching control of the 640K-1MB region. This fixed variant has a
separate capability bit in the MTRR capability MSR. The MTRR code
checks this capability bit before trying to access the fixed MTRR MSRs,
except in one place. This patch fixes this place to also
check the capability.

Otherwise there will be a WARN_ON when the respective MSRS don't exist.

Fixes: 2b1f6278d77c ("[PATCH] x86: Save the MTRRs of the BSP before booting an AP")
Signed-off-by: Andi Kleen <[email protected]>

---

v2: Add Fixes tag and expand description.
v3: Expand description
---
arch/x86/kernel/cpu/mtrr/mtrr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
index 767bf1c71aad..2a2fc14955cd 100644
--- a/arch/x86/kernel/cpu/mtrr/mtrr.c
+++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
@@ -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);
--
2.43.0



2024-04-01 22:26:54

by Andi Kleen

[permalink] [raw]
Subject: [PING] Re: [PATCH v3] x86/mtrr: Check if fixed MTRRs exist before saving them

On Wed, Feb 28, 2024 at 04:19:52PM -0800, Andi Kleen wrote:
> MTRRs have a obsolete fixed variant for fine grained
> caching control of the 640K-1MB region. This fixed variant has a
> separate capability bit in the MTRR capability MSR. The MTRR code
> checks this capability bit before trying to access the fixed MTRR MSRs,
> except in one place. This patch fixes this place to also
> check the capability.
>
> Otherwise there will be a WARN_ON when the respective MSRS don't exist.
>
> Fixes: 2b1f6278d77c ("[PATCH] x86: Save the MTRRs of the BSP before booting an AP")
> Signed-off-by: Andi Kleen <[email protected]>

Ping!

>
> ---
>
> v2: Add Fixes tag and expand description.
> v3: Expand description
> ---
> arch/x86/kernel/cpu/mtrr/mtrr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
> index 767bf1c71aad..2a2fc14955cd 100644
> --- a/arch/x86/kernel/cpu/mtrr/mtrr.c
> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
> @@ -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);
> --
> 2.43.0
>