[PATCH v2 0/3] Fix three racy updates to MSR_K7_HWCR

[PATCH v2 0/3] Fix three racy updates to MSR_K7_HWCR

[Jim Mattson]

I was backporting commit 65f55a301766 ("x86/CPU/AMD: Add CPUID faulting
support") to a local branch based on Linux v6.12, when our internal Sashiko
asked:

> Can this corrupt MSR_K7_HWCR? disable_cpuid()->set_cpuid_faulting() is
> called with preemption disabled, but interrupts are still enabled. Since
> msr_set_bit() performs a read-modify-write without disabling interrupts,
> if an IPI arrives between the read and write and modifies MSR_K7_HWCR
> (e.g. acpi-cpufreq toggling Core Performance Boost), the IPI's update
> will be lost.

To confirm that this wasn't just AI slop, I set up an empirical test on a
Turin system. First, I replaced the amd-pstate cpufreq driver with
acpi-cpufreq. Then I ran a test program, where one thread repeatedly reads
CPU0's HWCR, toggles /sys/devices/system/cpu/cpufreq/boost, reads CPU0's
HWCR again, and then verifies that the CPB_DIS bit has flipped. A second
thread, pinned to CPU0, repeatedly calls arch_prctl(ARCH_SET_CPUID, <val>),
where <val> alternates between 0 and 1. With the second thread running, the
first thread soon fails the verification step, indicating that the CPB_DIS
bit change is, in fact, lost.

Per Boris's review of v1, this version hoists the HWCR update logic out
into a new helper, amd_update_hwcr(), which performs the read-modify-write
with interrupts disabled, and converts the three runtime
(non-initialization) HWCR read-modify-write sites to use it:

  * set_cpuid_faulting(), fixing the race demonstrated above (patch 1);
  * toggle_hw_mce_inject(), which previously performed the
    read-modify-write as two independent crosscalls (patch 2);
  * boost_set_msr() in acpi-cpufreq, whose process-context invocation on
    the cpufreq policy teardown path can race with an HWCR update made by
    the MCE injector's crosscall (patch 3).

Initialization-time HWCR updates are left alone for now, to avoid excessive
churn.

v1 -> v2:
 - Add some of the cover letter details to the first patch
 - Hoist the HWCR update logic out into amd_update_hwcr() [Boris]
 - Use the helper in toggle_hw_mce_inject(), collapsing the split
   read/write crosscalls into one
 - Use the helper in boost_set_msr() [Boris]

v1: https://lore.kernel.org/all/20260609211611.466231-1-jmattson@google.com/

Jim Mattson (3):
  x86/CPU/AMD: Avoid racy updates to MSR_K7_HWCR in set_cpuid_faulting()
  x86/mce/inject: Avoid racy updates to MSR_K7_HWCR in
    toggle_hw_mce_inject()
  cpufreq: ACPI: Avoid racy updates to MSR_K7_HWCR in boost_set_msr()

 arch/x86/include/asm/msr-index.h |  3 +++
 arch/x86/include/asm/processor.h |  2 ++
 arch/x86/kernel/cpu/amd.c        | 42 ++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/mce/inject.c | 34 +++++++++++++++++++-------
 arch/x86/kernel/process.c        |  4 +--
 drivers/cpufreq/acpi-cpufreq.c   |  7 +++---
 6 files changed, 78 insertions(+), 14 deletions(-)


base-commit: 2b414a95b8f7307d42173ba9e580d6d3e2bcbfce
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH v2 1/3] x86/CPU/AMD: Avoid racy updates to MSR_K7_HWCR in set_cpuid_faulting()

[Jim Mattson]

Since msr_set_bit() and msr_clear_bit() perform a non-atomic update to an
MSR, they can race with a write to the same MSR from interrupt context.

On AMD CPUs, set_cpuid_faulting() uses these functions to modify
MSR_K7_HWCR from process context, with preemption disabled but interrupts
enabled. The acpi-cpufreq driver's boost_set_msr() modifies HWCR from
interrupt context. If a crosscall IPI arrives between
set_cpuid_faulting()'s read and write of MSR_K7_HWCR and toggles the Core
Performance Boost disable bit (CPB_DIS), the IPI's update is lost.

This race has been observed empirically on a Turin system running the
acpi-cpufreq driver with a synthetic test. One thread repeatedly toggles
/sys/devices/system/cpu/cpufreq/boost and verifies CPB_DIS on CPU0 after
each write. A second thread pinned to CPU0 calls arch_prctl(ARCH_SET_CPUID,
<val>), with alternating <val>s of 0 and 1. CPB_DIS bit changes are
sometimes lost.

Introduce amd_update_hwcr() to perform an interrupt-safe
read-modify-write of MSR_K7_HWCR, and use it in set_cpuid_faulting() to
prevent races with HWCR updates in interrupt context. Note that when
set_cpuid_faulting() is called from __switch_to_xtra(), interrupts are
already disabled, so the race is only possible on the arch_prctl()
paths.

Reported-by: Sashiko (gemini/gemini-3.1-pro-preview)
Closes: https://lore.kernel.org/all/20260609211611.466231-1-jmattson@google.com/
Suggested-by: Borislav Petkov <bp@alien8.de>
Fixes: 65f55a301766 ("x86/CPU/AMD: Add CPUID faulting support")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/msr-index.h |  1 +
 arch/x86/include/asm/processor.h |  2 ++
 arch/x86/kernel/cpu/amd.c        | 42 ++++++++++++++++++++++++++++++++
 arch/x86/kernel/process.c        |  4 +--
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 86554de9a3f5..18c4be75e927 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -899,6 +899,7 @@
 #define MSR_K7_HWCR_IRPERF_EN_BIT	30
 #define MSR_K7_HWCR_IRPERF_EN		BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)
 #define MSR_K7_HWCR_CPUID_USER_DIS_BIT	35
+#define MSR_K7_HWCR_CPUID_USER_DIS	BIT_ULL(MSR_K7_HWCR_CPUID_USER_DIS_BIT)
 #define MSR_K7_FID_VID_CTL		0xc0010041
 #define MSR_K7_FID_VID_STATUS		0xc0010042
 #define MSR_K7_HWCR_CPB_DIS_BIT		25
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 67dd932305db..d2ad7ac24e02 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -716,9 +716,11 @@ static __always_inline void amd_clear_divider(void)
 }
 
 extern void amd_check_microcode(void);
+extern int amd_update_hwcr(u64 clear, u64 set);
 #else
 static inline void amd_clear_divider(void)		{ }
 static inline void amd_check_microcode(void)		{ }
+static inline int amd_update_hwcr(u64 clear, u64 set) { return -ENODEV; }
 #endif
 
 extern unsigned long arch_align_stack(unsigned long sp);
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 31f01e9c7114..abb6f755be98 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1319,6 +1319,48 @@ void amd_check_microcode(void)
 		on_each_cpu(zenbleed_check_cpu, NULL, 1);
 }
 
+/**
+ * amd_update_hwcr - Update MSR_K7_HWCR on the executing CPU
+ * @clear: Bits to clear
+ * @set: Bits to set
+ *
+ * MSR_K7_HWCR is written from both process context (e.g. CPUID faulting
+ * updates via arch_prctl(ARCH_SET_CPUID)) and interrupt context (e.g.
+ * Core Performance Boost updates IPI'd by the acpi-cpufreq driver), so
+ * a read-modify-write of the MSR must be performed with interrupts
+ * disabled to avoid losing an update made by an intervening interrupt.
+ * All runtime (non-initialization) updates of MSR_K7_HWCR should go
+ * through this helper.
+ *
+ * Bits in @set take precedence over bits in @clear.
+ *
+ * Context: Any context except NMI. Disabling interrupts does not
+ *          serialize against an NMI, so NMI handlers must not write
+ *          MSR_K7_HWCR.
+ *
+ * Return: 0 on success, negative error code if an MSR access faults.
+ */
+int amd_update_hwcr(u64 clear, u64 set)
+{
+	unsigned long flags;
+	u64 oldval, newval;
+	int ret;
+
+	local_irq_save(flags);
+	ret = rdmsrq_safe(MSR_K7_HWCR, &oldval);
+	if (ret)
+		goto out;
+
+	newval = (oldval & ~clear) | set;
+
+	if (newval != oldval)
+		ret = wrmsrq_safe(MSR_K7_HWCR, newval);
+out:
+	local_irq_restore(flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(amd_update_hwcr);
+
 static const char * const s5_reset_reason_txt[] = {
 	[0]  = "thermal pin BP_THERMTRIP_L was tripped",
 	[1]  = "power button was pressed for 4 seconds",
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 4c718f8adc59..08ef205f6b7f 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -355,9 +355,9 @@ static void set_cpuid_faulting(bool on)
 		wrmsrq(MSR_MISC_FEATURES_ENABLES, msrval);
 	} else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
 		if (on)
-			msr_set_bit(MSR_K7_HWCR, MSR_K7_HWCR_CPUID_USER_DIS_BIT);
+			amd_update_hwcr(0, MSR_K7_HWCR_CPUID_USER_DIS);
 		else
-			msr_clear_bit(MSR_K7_HWCR, MSR_K7_HWCR_CPUID_USER_DIS_BIT);
+			amd_update_hwcr(MSR_K7_HWCR_CPUID_USER_DIS, 0);
 	}
 }
 
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH v2 2/3] x86/mce/inject: Avoid racy updates to MSR_K7_HWCR in toggle_hw_mce_inject()

[Jim Mattson]

toggle_hw_mce_inject() performs a read-modify-write of MSR_K7_HWCR as two
independent crosscalls: one to read the MSR and one to write it.  Another
HWCR update on the target CPU can be lost if it occurs CPU between the two
crosscalls.

Replace the two crosscalls with a single crosscall that performs the
read-modify-write using the amd_update_hwcr() helper.

Opportunistically, replace the open-coded BIT(18) with a new
MSR_K7_HWCR_MCSTATUSWREN macro.

Fixes: 21690934d934 ("EDAC, mce_amd_inj: Enable direct writes to MCE MSRs")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/include/asm/msr-index.h |  2 ++
 arch/x86/kernel/cpu/mce/inject.c | 34 +++++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 18c4be75e927..e24a0a6b5d17 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -900,6 +900,8 @@
 #define MSR_K7_HWCR_IRPERF_EN		BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)
 #define MSR_K7_HWCR_CPUID_USER_DIS_BIT	35
 #define MSR_K7_HWCR_CPUID_USER_DIS	BIT_ULL(MSR_K7_HWCR_CPUID_USER_DIS_BIT)
+#define MSR_K7_HWCR_MCSTATUSWREN_BIT	18
+#define MSR_K7_HWCR_MCSTATUSWREN	BIT_ULL(MSR_K7_HWCR_MCSTATUSWREN_BIT)
 #define MSR_K7_FID_VID_CTL		0xc0010041
 #define MSR_K7_FID_VID_STATUS		0xc0010042
 #define MSR_K7_HWCR_CPB_DIS_BIT		25
diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index d02c4f556cd0..9d6e330fec61 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -30,6 +30,7 @@
 #include <asm/mce.h>
 #include <asm/msr.h>
 #include <asm/nmi.h>
+#include <asm/processor.h>
 #include <asm/smp.h>
 
 #include "internal.h"
@@ -310,28 +311,43 @@ static struct notifier_block inject_nb = {
 	.notifier_call  = mce_inject_raise,
 };
 
+struct hwcr_update_info {
+	u64 clear;
+	u64 set;
+	int err;
+};
+
+static void ipi_update_hwcr(void *info)
+{
+	struct hwcr_update_info *ui = info;
+
+	ui->err = amd_update_hwcr(ui->clear, ui->set);
+}
+
 /*
  * Caller needs to be make sure this cpu doesn't disappear
  * from under us, i.e.: get_cpu/put_cpu.
  */
 static int toggle_hw_mce_inject(unsigned int cpu, bool enable)
 {
-	u32 l, h;
+	struct hwcr_update_info ui = {
+		.clear = enable ? 0 : MSR_K7_HWCR_MCSTATUSWREN,
+		.set = enable ? MSR_K7_HWCR_MCSTATUSWREN : 0,
+	};
 	int err;
 
-	err = rdmsr_on_cpu(cpu, MSR_K7_HWCR, &l, &h);
+	err = smp_call_function_single(cpu, ipi_update_hwcr, &ui, 1);
 	if (err) {
-		pr_err("%s: error reading HWCR\n", __func__);
+		pr_err("%s: error calling ipi_update_hwcr on CPU %d\n", __func__, cpu);
 		return err;
 	}
 
-	enable ? (l |= BIT(18)) : (l &= ~BIT(18));
-
-	err = wrmsr_on_cpu(cpu, MSR_K7_HWCR, l, h);
-	if (err)
-		pr_err("%s: error writing HWCR\n", __func__);
+	if (ui.err) {
+		pr_err("%s: error updating HWCR on CPU %d\n", __func__, cpu);
+		return ui.err;
+	}
 
-	return err;
+	return 0;
 }
 
 static int __set_inj(const char *buf)
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH v2 3/3] cpufreq: ACPI: Avoid racy updates to MSR_K7_HWCR in boost_set_msr()

[Jim Mattson]

On AMD and Hygon CPUs, use amd_update_hwcr() in boost_set_msr() so that all
runtime updates of MSR_K7_HWCR go through the common interrupt-safe helper.

boost_set_msr() usually runs with interrupts disabled, but the call via
cpufreq_boost_down_prep() on the cpufreq policy teardown path runs in
process context with interrupts enabled. The MCE injector's
toggle_hw_mce_inject() modifies the same MSR via a crosscall that can
target any CPU. If such a crosscall lands between the open-coded read and
write of MSR_K7_HWCR, the injector's McStatusWrEn update is lost.

Suggested-by: Borislav Petkov <bp@alien8.de>
Fixes: 21690934d934 ("EDAC, mce_amd_inj: Enable direct writes to MCE MSRs")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 21639d9ac753..8fc4cfd045c4 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -103,9 +103,10 @@ static int boost_set_msr(bool enable)
 		break;
 	case X86_VENDOR_HYGON:
 	case X86_VENDOR_AMD:
-		msr_addr = MSR_K7_HWCR;
-		msr_mask = MSR_K7_HWCR_CPB_DIS;
-		break;
+		if (enable)
+			return amd_update_hwcr(MSR_K7_HWCR_CPB_DIS, 0);
+		else
+			return amd_update_hwcr(0, MSR_K7_HWCR_CPB_DIS);
 	default:
 		return -EINVAL;
 	}
-- 
2.54.0.1136.gdb2ca164c4-goog

Re: [PATCH v2 1/3] x86/CPU/AMD: Avoid racy updates to MSR_K7_HWCR in set_cpuid_faulting()

[Yosry Ahmed]

On Fri, Jun 12, 2026 at 2:57 PM Jim Mattson <jmattson@google.com> wrote:
>
> Since msr_set_bit() and msr_clear_bit() perform a non-atomic update to an
> MSR, they can race with a write to the same MSR from interrupt context.
>
> On AMD CPUs, set_cpuid_faulting() uses these functions to modify
> MSR_K7_HWCR from process context, with preemption disabled but interrupts
> enabled. The acpi-cpufreq driver's boost_set_msr() modifies HWCR from
> interrupt context. If a crosscall IPI arrives between
> set_cpuid_faulting()'s read and write of MSR_K7_HWCR and toggles the Core
> Performance Boost disable bit (CPB_DIS), the IPI's update is lost.
>
> This race has been observed empirically on a Turin system running the
> acpi-cpufreq driver with a synthetic test. One thread repeatedly toggles
> /sys/devices/system/cpu/cpufreq/boost and verifies CPB_DIS on CPU0 after
> each write. A second thread pinned to CPU0 calls arch_prctl(ARCH_SET_CPUID,
> <val>), with alternating <val>s of 0 and 1. CPB_DIS bit changes are
> sometimes lost.
>
> Introduce amd_update_hwcr() to perform an interrupt-safe
> read-modify-write of MSR_K7_HWCR, and use it in set_cpuid_faulting() to
> prevent races with HWCR updates in interrupt context. Note that when
> set_cpuid_faulting() is called from __switch_to_xtra(), interrupts are
> already disabled, so the race is only possible on the arch_prctl()
> paths.
>
> Reported-by: Sashiko (gemini/gemini-3.1-pro-preview)
> Closes: https://lore.kernel.org/all/20260609211611.466231-1-jmattson@google.com/
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Fixes: 65f55a301766 ("x86/CPU/AMD: Add CPUID faulting support")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/include/asm/msr-index.h |  1 +
>  arch/x86/include/asm/processor.h |  2 ++
>  arch/x86/kernel/cpu/amd.c        | 42 ++++++++++++++++++++++++++++++++
>  arch/x86/kernel/process.c        |  4 +--
>  4 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 86554de9a3f5..18c4be75e927 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -899,6 +899,7 @@
>  #define MSR_K7_HWCR_IRPERF_EN_BIT      30
>  #define MSR_K7_HWCR_IRPERF_EN          BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)
>  #define MSR_K7_HWCR_CPUID_USER_DIS_BIT 35
> +#define MSR_K7_HWCR_CPUID_USER_DIS     BIT_ULL(MSR_K7_HWCR_CPUID_USER_DIS_BIT)
>  #define MSR_K7_FID_VID_CTL             0xc0010041
>  #define MSR_K7_FID_VID_STATUS          0xc0010042
>  #define MSR_K7_HWCR_CPB_DIS_BIT                25
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 67dd932305db..d2ad7ac24e02 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -716,9 +716,11 @@ static __always_inline void amd_clear_divider(void)
>  }
>
>  extern void amd_check_microcode(void);
> +extern int amd_update_hwcr(u64 clear, u64 set);
>  #else
>  static inline void amd_clear_divider(void)             { }
>  static inline void amd_check_microcode(void)           { }
> +static inline int amd_update_hwcr(u64 clear, u64 set) { return -ENODEV; }
>  #endif
>
>  extern unsigned long arch_align_stack(unsigned long sp);
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 31f01e9c7114..abb6f755be98 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1319,6 +1319,48 @@ void amd_check_microcode(void)
>                 on_each_cpu(zenbleed_check_cpu, NULL, 1);
>  }
>
> +/**
> + * amd_update_hwcr - Update MSR_K7_HWCR on the executing CPU
> + * @clear: Bits to clear
> + * @set: Bits to set
> + *
> + * MSR_K7_HWCR is written from both process context (e.g. CPUID faulting
> + * updates via arch_prctl(ARCH_SET_CPUID)) and interrupt context (e.g.
> + * Core Performance Boost updates IPI'd by the acpi-cpufreq driver), so
> + * a read-modify-write of the MSR must be performed with interrupts
> + * disabled to avoid losing an update made by an intervening interrupt.
> + * All runtime (non-initialization) updates of MSR_K7_HWCR should go
> + * through this helper.
> + *
> + * Bits in @set take precedence over bits in @clear.
> + *
> + * Context: Any context except NMI. Disabling interrupts does not
> + *          serialize against an NMI, so NMI handlers must not write
> + *          MSR_K7_HWCR.

Might be more useful to WARN if in_nmi()?

> + *
> + * Return: 0 on success, negative error code if an MSR access faults.
> + */
> +int amd_update_hwcr(u64 clear, u64 set)
> +{
> +       unsigned long flags;
> +       u64 oldval, newval;
> +       int ret;
> +
> +       local_irq_save(flags);
> +       ret = rdmsrq_safe(MSR_K7_HWCR, &oldval);
> +       if (ret)
> +               goto out;
> +
> +       newval = (oldval & ~clear) | set;
> +
> +       if (newval != oldval)
> +               ret = wrmsrq_safe(MSR_K7_HWCR, newval);

Can we reuse msr_set_bit() and msr_clear_bit() here?

> +out:
> +       local_irq_restore(flags);
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(amd_update_hwcr);
> +
>  static const char * const s5_reset_reason_txt[] = {
>         [0]  = "thermal pin BP_THERMTRIP_L was tripped",
>         [1]  = "power button was pressed for 4 seconds",
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 4c718f8adc59..08ef205f6b7f 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -355,9 +355,9 @@ static void set_cpuid_faulting(bool on)
>                 wrmsrq(MSR_MISC_FEATURES_ENABLES, msrval);
>         } else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
>                 if (on)
> -                       msr_set_bit(MSR_K7_HWCR, MSR_K7_HWCR_CPUID_USER_DIS_BIT);
> +                       amd_update_hwcr(0, MSR_K7_HWCR_CPUID_USER_DIS);
>                 else
> -                       msr_clear_bit(MSR_K7_HWCR, MSR_K7_HWCR_CPUID_USER_DIS_BIT);
> +                       amd_update_hwcr(MSR_K7_HWCR_CPUID_USER_DIS, 0);
>         }
>  }
>
> --
> 2.54.0.1136.gdb2ca164c4-goog
>

Re: [PATCH v2 0/3] Fix three racy updates to MSR_K7_HWCR

[Andrew Cooper]

> hoists the HWCR update logic out
> into a new helper, amd_update_hwcr(), which performs the read-modify-write
> with interrupts disabled

Add some `perf` to your test scenario and these bugs will reappear, this
time caused by NMIs.

I don't have any good suggestions. Core scope MSRs are horrible because
you can't update them atomically.

At boot time you can fix it by ensuring the RMW will cause both threads
to write the same value (i.e. the modify is not dependent on the read
value, and that there is exactly one WRMSR in the entire kernel for this
MSR).

For runtime with multiple agendas going on, you need some kind of
spinlock because it's a plain concurrency problem.

~Andrew

Re: [PATCH v2 0/3] Fix three racy updates to MSR_K7_HWCR

[Jim Mattson]

On Fri, Jun 12, 2026 at 4:01 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> > hoists the HWCR update logic out
> > into a new helper, amd_update_hwcr(), which performs the read-modify-write
> > with interrupts disabled
>
> Add some `perf` to your test scenario and these bugs will reappear, this
> time caused by NMIs.
>
> I don't have any good suggestions. Core scope MSRs are horrible because
> you can't update them atomically.

I agree, but I spot-checked a couple of (Zen1+) PPRs, and I thought
they said that HWCR has thread scope. For example, in
https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/56255_OSRR.pdf
has:

> _lthree[1:0]_core[3:0]_thread[1:0]; MSRC0010015

And, earlier:

> The absence of the instance parameter _thread[1:0] signifies that there is not a specific instance of said register per thread and thus the register is shared between thread 1 and thread 0.

Was it core-shared on older platforms?

Re: [PATCH v2 1/3] x86/CPU/AMD: Avoid racy updates to MSR_K7_HWCR in set_cpuid_faulting()

[Jim Mattson]

On Fri, Jun 12, 2026 at 3:30 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Fri, Jun 12, 2026 at 2:57 PM Jim Mattson <jmattson@google.com> wrote:
...
> > + * Context: Any context except NMI. Disabling interrupts does not
> > + *          serialize against an NMI, so NMI handlers must not write
> > + *          MSR_K7_HWCR.
>
> Might be more useful to WARN if in_nmi()?

I concur.

> > + *
> > + * Return: 0 on success, negative error code if an MSR access faults.
> > + */
> > +int amd_update_hwcr(u64 clear, u64 set)
> > +{
> > +       unsigned long flags;
> > +       u64 oldval, newval;
> > +       int ret;
> > +
> > +       local_irq_save(flags);
> > +       ret = rdmsrq_safe(MSR_K7_HWCR, &oldval);
> > +       if (ret)
> > +               goto out;
> > +
> > +       newval = (oldval & ~clear) | set;
> > +
> > +       if (newval != oldval)
> > +               ret = wrmsrq_safe(MSR_K7_HWCR, newval);
>
> Can we reuse msr_set_bit() and msr_clear_bit() here?

Yes. I tried to make it more general, but none of the current users
want to modify more than one bit.

I'll try to get v3 out this week.