1. Patchew
  2. linux
  3. View series

[PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()

Jiaqing Zhao posted 1 patch 7 months, 1 week ago 
arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()
Posted by Jiaqing Zhao 7 months, 1 week ago 
When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
to save fixed-range MTRRs. On platforms without fixed-range MTRRs,
accessing these MSRs will trigger unchecked MSR access error. Make
sure fixed-range MTRRs are supported before access to prevent such
error.

Since mtrr_state.have_fixed is only set when MTRRs are present and
enabled, checking the CPU feature flag in mtrr_save_fixed_ranges()
is unnecessary.

Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
Cc: stable@vger.kernel.org
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
---
v2:
* Removed unnecessary boot_cpu_has(X86_FEATURE_MTRR) check.
* Updated commit message.
Link: https://lore.kernel.org/all/20250509085612.2236222-2-jiaqing.zhao@linux.intel.com

 arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index e2c6b471d230..8c18327eb10b 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -593,7 +593,7 @@ static void get_fixed_ranges(mtrr_type *frs)
 
 void mtrr_save_fixed_ranges(void *info)
 {
-	if (boot_cpu_has(X86_FEATURE_MTRR))
+	if (mtrr_state.have_fixed)
 		get_fixed_ranges(mtrr_state.fixed_ranges);
 }
 
-- 
2.43.0
Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()
Posted by Borislav Petkov 7 months, 1 week ago 
On Fri, May 09, 2025 at 05:06:33PM +0000, Jiaqing Zhao wrote:
> When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
> to save fixed-range MTRRs. On platforms without fixed-range MTRRs,
> accessing these MSRs will trigger unchecked MSR access error. Make
> sure fixed-range MTRRs are supported before access to prevent such
> error.
> 
> Since mtrr_state.have_fixed is only set when MTRRs are present and
> enabled, checking the CPU feature flag in mtrr_save_fixed_ranges()
> is unnecessary.
> 
> Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>

Next question: this is CC:stable, meaning it'll go to Linus now.

What exactly is it fixing?

Because the patch in Fixes: is from 2007. :-\

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()
Posted by Jiaqing Zhao 7 months, 1 week ago 
On 2025-05-10 01:32, Borislav Petkov wrote:
> On Fri, May 09, 2025 at 05:06:33PM +0000, Jiaqing Zhao wrote:
>> When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
>> to save fixed-range MTRRs. On platforms without fixed-range MTRRs,
>> accessing these MSRs will trigger unchecked MSR access error. Make
>> sure fixed-range MTRRs are supported before access to prevent such
>> error.
>>
>> Since mtrr_state.have_fixed is only set when MTRRs are present and
>> enabled, checking the CPU feature flag in mtrr_save_fixed_ranges()
>> is unnecessary.
>>
>> Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> 
> Next question: this is CC:stable, meaning it'll go to Linus now.
> 
> What exactly is it fixing?
> 
> Because the patch in Fixes: is from 2007. :-\

Hi, Boris

This fixes unchecked MSR access error on platform without fixed-range
MTRRs when doing ACPI S3 suspend. IMHO, though it is handled and won't
panic kernel, it is worth getting fixed, and it matches the stable rule
that

"It fixes a problem like an oops, a hang, data corruption, a real
security issue, a hardware quirk, a build error (but not for things
marked CONFIG_BROKEN), or some “oh, that’s not good” issue."

Kernel log is attached below.

Thanks,
Jiaqing

[ 173.115706] ACPI: PM: Saving platform NVS memory
[ 173.115818] Disabling non-boot CPUs ...
[ 173.126530] unchecked MSR access error: RDMSR from 0x250 at rIP: 0xffffffffa90a15ff (get_fixed_ranges+0x)
[ 173.126749] Call Trace:
[ 173.126806] <TASK>
[ 173.126858] ? show_stack_regs+0x23/0x30
[ 173.126946] ? fixup_exception+0x5a4/0x610
[ 173.127037] ? printk_get_next_message+0x105/0x350
[ 173.127141] ? gp_try_fixup_and_notify+0x37/0x100
[ 173.127244] ? exc_general_protection+0xe1/0x1f0
[ 173.127346] ? asm_exc_general_protection+0x27/0x30
[ 173.127452] ? __cfi_x86_acpi_suspend_lowlevel+0x10/0x10
[ 173.127567] ? get_fixed_ranges+0x5f/0x390
[ 173.127657] mtrr_save_fixed_ranges+0x1b/0x40
[ 173.127753] save_processor_state+0x111/0x220
[ 173.127849] do_suspend_lowlevel+0xf/0xb70
[ 173.127939] x86_acpi_suspend_lowlevel+0x14c/0x180
[ 173.128042] acpi_suspend_enter+0x17e/0x1e0
[ 173.128133] suspend_devices_and_enter+0x62d/0x950
[ 173.128236] pm_suspend+0x2cf/0x4c0
[ 173.128314] state_store+0x109/0x130
[ 173.128393] kobj_attr_store+0x1e/0x40
[ 173.128477] sysfs_kf_write+0x45/0x60
[ 173.128559] kernfs_fop_write_iter+0x113/0x1a0
[ 173.128698] vfs_write+0x38a/0x470
[ 173.128775] ksys_write+0x87/0x100
[ 173.128851] __x64_sys_write+0x1b/0x30
[ 173.128932] x64_sys_call+0x17f1/0x25e0
[ 173.129017] do_syscall_64+0x74/0x120
[ 173.129098] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 173.129206] RIP: 0033:0x7d6a19d82687
[ 173.129288] Code: 00 00 00 b8 00 00 00 00 0f 05 48 3d 01 f0 ff ff 72 09 f7 d8 89 c7 e8 e8 fa ff ff c3 0f0
[ 173.129661] RSP: 002b:00007d66c7a7f668 EFLAGS: 00000213 ORIG_RAX: 0000000000000001
[ 173.129819] RAX: ffffffffffffffda RBX: 00007d68a8858450 RCX: 00007d6a19d82687
[ 173.129967] RDX: 0000000000000003 RSI: 00007d680881a2a0 RDI: 00000000000000a8
[ 173.130115] RBP: 00007d66c7a7f6e0 R08: ffffffffffffffff R09: 0000000000000000
[ 173.130262] R10: 0000000000020000 R11: 0000000000000213 R12: 0000000000000000
[ 173.130410] R13: 0000000070ec9fb8 R14: 00000000000000a8 R15: 00007d66c7a7f75c
[ 173.130559] </TASK>
Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()
Posted by Borislav Petkov 7 months, 1 week ago 
On May 12, 2025 10:31:23 AM GMT+02:00, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>This fixes unchecked MSR access error on platform without fixed-range
>MTRRs when doing ACPI S3 suspend.

Is this happening on hw which is shipping now and users will see it or is this some new platform which is yet to see the light of day in the future?

-- 
Sent from a small device: formatting sucks and brevity is inevitable.
Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()
Posted by Jiaqing Zhao 7 months, 1 week ago 

On 2025-05-12 16:46, Borislav Petkov wrote:
> On May 12, 2025 10:31:23 AM GMT+02:00, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>> This fixes unchecked MSR access error on platform without fixed-range
>> MTRRs when doing ACPI S3 suspend.
> 
> Is this happening on hw which is shipping now and users will see it or is this some new platform which is yet to see the light of day in the future?
> 
Actually it is happening on virtualized platform. A recent version of
ACRN hypervisor has removed emulated fixed-range MTRRs.

Thanks,
Jiaqing
Re: [PATCH v2] x86/mtrr: Check if fixed-range MTRR exists in mtrr_save_fixed_ranges()
Posted by Borislav Petkov 7 months, 1 week ago 
On Mon, May 12, 2025 at 05:24:21PM +0800, Jiaqing Zhao wrote:
> Actually it is happening on virtualized platform. A recent version of
> ACRN hypervisor has removed emulated fixed-range MTRRs.

Oh ok, nothing urgent then.

Will queue it for the next merge window.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
[tip: x86/mtrr] x86/mtrr: Check if fixed-range MTRRs exist in mtrr_save_fixed_ranges()
Posted by tip-bot2 for Jiaqing Zhao 7 months, 1 week ago 
The following commit has been merged into the x86/mtrr branch of tip:

Commit-ID:     824c6384e8d9275d4ec7204f3f79a4ac6bc10379
Gitweb:        https://git.kernel.org/tip/824c6384e8d9275d4ec7204f3f79a4ac6bc10379
Author:        Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
AuthorDate:    Fri, 09 May 2025 17:06:33 
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Mon, 12 May 2025 13:04:40 +02:00

x86/mtrr: Check if fixed-range MTRRs exist in mtrr_save_fixed_ranges()

When suspending, save_processor_state() calls mtrr_save_fixed_ranges()
to save fixed-range MTRRs.

On platforms without fixed-range MTRRs like the ACRN hypervisor which
has removed fixed-range MTRR emulation, accessing these MSRs will
trigger an unchecked MSR access error. Make sure fixed-range MTRRs are
supported before access to prevent such error.

Since mtrr_state.have_fixed is only set when MTRRs are present and
enabled, checking the CPU feature flag in mtrr_save_fixed_ranges() is
unnecessary.

Fixes: 3ebad5905609 ("[PATCH] x86: Save and restore the fixed-range MTRRs of the BSP when suspending")
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/20250509170633.3411169-2-jiaqing.zhao@linux.intel.com
---
 arch/x86/kernel/cpu/mtrr/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
index e2c6b47..8c18327 100644
--- a/arch/x86/kernel/cpu/mtrr/generic.c
+++ b/arch/x86/kernel/cpu/mtrr/generic.c
@@ -593,7 +593,7 @@ static void get_fixed_ranges(mtrr_type *frs)
 
 void mtrr_save_fixed_ranges(void *info)
 {
-	if (boot_cpu_has(X86_FEATURE_MTRR))
+	if (mtrr_state.have_fixed)
 		get_fixed_ranges(mtrr_state.fixed_ranges);
 }

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.