Re: [PATCH 16/32] cpufreq: Stop using 32-bit MSR interfaces
From: Juergen Gross
Date: Tue Jun 30 2026 - 02:41:59 EST
On 29.06.26 16:55, Zhongqiu Han wrote:
On 6/29/2026 2:05 PM, Juergen Gross wrote:
The 32-bit MSR interfaces rdmsr() and wrmsr() are planned to be
removed. Use the related 64-bit variants instead.
Hello Juergen,
Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
drivers/cpufreq/acpi-cpufreq.c | 22 ++++----
drivers/cpufreq/e_powersaver.c | 48 ++++++++---------
drivers/cpufreq/longhaul.c | 15 +++---
drivers/cpufreq/longrun.c | 78 +++++++++++++++-------------
drivers/cpufreq/powernow-k6.c | 12 ++---
drivers/cpufreq/powernow-k8.c | 67 ++++++++++++------------
drivers/cpufreq/speedstep-centrino.c | 16 +++---
drivers/cpufreq/speedstep-lib.c | 63 +++++++++++-----------
8 files changed, 166 insertions(+), 155 deletions(-)
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 21639d9ac753..b40fa99b5ab2 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -246,32 +246,32 @@ static unsigned extract_freq(struct cpufreq_policy *policy, u32 val)
static u32 cpu_freq_read_intel(struct acpi_pct_register *not_used)
{
- u32 val, dummy __always_unused;
+ u64 val;
- rdmsr(MSR_IA32_PERF_CTL, val, dummy);
- return val;
+ rdmsrq(MSR_IA32_PERF_CTL, val);
+ return (u32)val;
}
static void cpu_freq_write_intel(struct acpi_pct_register *not_used, u32 val)
{
- u32 lo, hi;
+ struct msr msrval;
- rdmsr(MSR_IA32_PERF_CTL, lo, hi);
- lo = (lo & ~INTEL_MSR_RANGE) | (val & INTEL_MSR_RANGE);
- wrmsr(MSR_IA32_PERF_CTL, lo, hi);
+ rdmsrq(MSR_IA32_PERF_CTL, msrval.q);
+ msrval.h = (msrval.h & ~INTEL_MSR_RANGE) | (val & INTEL_MSR_RANGE);
msrval.l
Oh, indeed.
please feel free to correct me if there is any misunderstanding.
how about using u64 directly?
u64 msrval;
rdmsrq(MSR_IA32_PERF_CTL, msrval);
msrval = (msrval & ~(u64)INTEL_MSR_RANGE) | (val & INTEL_MSR_RANGE);
wrmsrq(MSR_IA32_PERF_CTL, msrval);
As per my understanding, we can use u64 instead of struct msr in many
other places in this patch as well.
I didn't look at all the MSR definitions to find out whether a plain u64 would
work.
Will look into this in more detail.
+ wrmsrq(MSR_IA32_PERF_CTL, msrval.q);
}
static u32 cpu_freq_read_amd(struct acpi_pct_register *not_used)
{
- u32 val, dummy __always_unused;
+ u64 val;
- rdmsr(MSR_AMD_PERF_CTL, val, dummy);
- return val;
+ rdmsrq(MSR_AMD_PERF_CTL, val);
+ return (u32)val;
}
static void cpu_freq_write_amd(struct acpi_pct_register *not_used, u32 val)
{
- wrmsr(MSR_AMD_PERF_CTL, val, 0);
+ wrmsrq(MSR_AMD_PERF_CTL, val);
}
static u32 cpu_freq_read_io(struct acpi_pct_register *reg)
diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c
index eb5a9209d828..13709f9667ea 100644
--- a/drivers/cpufreq/e_powersaver.c
+++ b/drivers/cpufreq/e_powersaver.c
@@ -90,7 +90,7 @@ static int eps_acpi_exit(struct cpufreq_policy *policy)
static unsigned int eps_get(unsigned int cpu)
{
struct eps_cpu_data *centaur;
- u32 lo, hi;
+ u64 val;
if (cpu)
return 0;
@@ -99,35 +99,35 @@ static unsigned int eps_get(unsigned int cpu)
return 0;
/* Return current frequency */
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
- return centaur->fsb * ((lo >> 8) & 0xff);
+ rdmsrq(MSR_IA32_PERF_STATUS, val);
+ return centaur->fsb * ((val >> 8) & 0xff);
}
static int eps_set_state(struct eps_cpu_data *centaur,
struct cpufreq_policy *policy,
u32 dest_state)
{
- u32 lo, hi;
+ u64 val;
int i;
/* Wait while CPU is busy */
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
+ rdmsrq(MSR_IA32_PERF_STATUS, val);
i = 0;
- while (lo & ((1 << 16) | (1 << 17))) {
+ while (val & ((1 << 16) | (1 << 17))) {
udelay(16);
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
+ rdmsrq(MSR_IA32_PERF_STATUS, val);
i++;
if (unlikely(i > 64)) {
return -ENODEV;
}
}
/* Set new multiplier and voltage */
- wrmsr(MSR_IA32_PERF_CTL, dest_state & 0xffff, 0);
+ wrmsrq(MSR_IA32_PERF_CTL, dest_state & 0xffff);
/* Wait until transition end */
i = 0;
do {
udelay(16);
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
+ rdmsrq(MSR_IA32_PERF_STATUS, val);
i++;
if (unlikely(i > 64)) {
return -ENODEV;
do {
udelay(16);
rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
i++;
if (unlikely(i > 64)) {
return -ENODEV;
}
} while (lo & ((1 << 16) | (1 << 17)));-->lo has been deleted?
Thanks for spotting that. Seems by 32-bit allyesconfig tree lost the config
entry needed for building this source.
Will fix this.
@@ -139,10 +139,10 @@ static int eps_set_state(struct eps_cpu_data *centaur,
u8 current_multiplier, current_voltage;
/* Print voltage and multiplier */
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
- current_voltage = lo & 0xff;
+ rdmsrq(MSR_IA32_PERF_STATUS, val);
+ current_voltage = val & 0xff;
pr_info("Current voltage = %dmV\n", current_voltage * 16 + 700);
- current_multiplier = (lo >> 8) & 0xff;
+ current_multiplier = (val >> 8) & 0xff;
pr_info("Current multiplier = %d\n", current_multiplier);
}
#endif
@@ -171,8 +171,8 @@ static int eps_target(struct cpufreq_policy *policy, unsigned int index)
static int eps_cpu_init(struct cpufreq_policy *policy)
{
unsigned int i;
- u32 lo, hi;
u64 val;
+ struct msr status;
u8 current_multiplier, current_voltage;
u8 max_multiplier, max_voltage;
u8 min_multiplier, min_voltage;
@@ -195,13 +195,13 @@ static int eps_cpu_init(struct cpufreq_policy *policy)
switch (c->x86_model) {
case 10:
- rdmsr(0x1153, lo, hi);
- brand = (((lo >> 2) ^ lo) >> 18) & 3;
+ rdmsrq(0x1153, val);
+ brand = (((val >> 2) ^ val) >> 18) & 3;
pr_cont("Model A ");
break;
case 13:
- rdmsr(0x1154, lo, hi);
- brand = (((lo >> 4) ^ (lo >> 2))) & 0x000000ff;
+ rdmsrq(0x1154, val);
+ brand = (((val >> 4) ^ (val >> 2))) & 0x000000ff;
pr_cont("Model D ");
break;
}
@@ -237,20 +237,20 @@ static int eps_cpu_init(struct cpufreq_policy *policy)
}
/* Print voltage and multiplier */
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
- current_voltage = lo & 0xff;
+ rdmsrq(MSR_IA32_PERF_STATUS, status.q);
+ current_voltage = status.l & 0xff;
pr_info("Current voltage = %dmV\n", current_voltage * 16 + 700);
- current_multiplier = (lo >> 8) & 0xff;
+ current_multiplier = (status.l >> 8) & 0xff;
pr_info("Current multiplier = %d\n", current_multiplier);
/* Print limits */
- max_voltage = hi & 0xff;
+ max_voltage = status.h & 0xff;
pr_info("Highest voltage = %dmV\n", max_voltage * 16 + 700);
- max_multiplier = (hi >> 8) & 0xff;
+ max_multiplier = (status.h >> 8) & 0xff;
pr_info("Highest multiplier = %d\n", max_multiplier);
- min_voltage = (hi >> 16) & 0xff;
+ min_voltage = (status.h >> 16) & 0xff;
pr_info("Lowest voltage = %dmV\n", min_voltage * 16 + 700);
- min_multiplier = (hi >> 24) & 0xff;
+ min_multiplier = (status.h >> 24) & 0xff;
pr_info("Lowest multiplier = %d\n", min_multiplier);
/* Sanity checks */
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
index a18d1d11725f..4c2599264333 100644
--- a/drivers/cpufreq/longhaul.c
+++ b/drivers/cpufreq/longhaul.c
@@ -118,13 +118,14 @@ static unsigned int calc_speed(int mult)
static int longhaul_get_cpu_mult(void)
{
- unsigned long invalue = 0, lo, hi;
+ unsigned long invalue = 0;
+ u64 val;
- rdmsr(MSR_IA32_EBL_CR_POWERON, lo, hi);
- invalue = (lo & (1<<22|1<<23|1<<24|1<<25))>>22;
+ rdmsrq(MSR_IA32_EBL_CR_POWERON, val);
+ invalue = (val & (1<<22|1<<23|1<<24|1<<25))>>22;
if (longhaul_version == TYPE_LONGHAUL_V2 ||
longhaul_version == TYPE_POWERSAVER) {
- if (lo & (1<<27))
+ if (val & (1<<27))
invalue += 16;
}
return eblcr[invalue];
@@ -761,7 +762,7 @@ static int longhaul_cpu_init(struct cpufreq_policy *policy)
struct cpuinfo_x86 *c = &cpu_data(0);
char *cpuname = NULL;
int ret;
- u32 lo, hi;
+ u64 val;
/* Check what we have on this motherboard */
switch (c->x86_model) {
@@ -835,8 +836,8 @@ static int longhaul_cpu_init(struct cpufreq_policy *policy)
}
/* Check Longhaul ver. 2 */
if (longhaul_version == TYPE_LONGHAUL_V2) {
- rdmsr(MSR_VIA_LONGHAUL, lo, hi);
- if (lo == 0 && hi == 0)
+ rdmsrq(MSR_VIA_LONGHAUL, val);
+ if (val == 0)
/* Looks like MSR isn't present */
longhaul_version = TYPE_LONGHAUL_V1;
}
diff --git a/drivers/cpufreq/longrun.c b/drivers/cpufreq/longrun.c
index f3aaca0496a4..99abef32e7e5 100644
--- a/drivers/cpufreq/longrun.c
+++ b/drivers/cpufreq/longrun.c
@@ -35,27 +35,27 @@ static unsigned int longrun_low_freq, longrun_high_freq;
*/
static void longrun_get_policy(struct cpufreq_policy *policy)
{
- u32 msr_lo, msr_hi;
+ struct msr msr;
- rdmsr(MSR_TMTA_LONGRUN_FLAGS, msr_lo, msr_hi);
- pr_debug("longrun flags are %x - %x\n", msr_lo, msr_hi);
- if (msr_lo & 0x01)
+ rdmsrq(MSR_TMTA_LONGRUN_FLAGS, msr.q);
+ pr_debug("longrun flags are %x - %x\n", msr.l, msr.h);
+ if (msr.l & 0x01)
policy->policy = CPUFREQ_POLICY_PERFORMANCE;
else
policy->policy = CPUFREQ_POLICY_POWERSAVE;
- rdmsr(MSR_TMTA_LONGRUN_CTRL, msr_lo, msr_hi);
- pr_debug("longrun ctrl is %x - %x\n", msr_lo, msr_hi);
- msr_lo &= 0x0000007F;
- msr_hi &= 0x0000007F;
+ rdmsrq(MSR_TMTA_LONGRUN_CTRL, msr.q);
+ pr_debug("longrun ctrl is %x - %x\n", msr.l, msr.h);
+ msr.l &= 0x0000007F;
+ msr.h &= 0x0000007F;
if (longrun_high_freq <= longrun_low_freq) {
/* Assume degenerate Longrun table */
policy->min = policy->max = longrun_high_freq;
} else {
- policy->min = longrun_low_freq + msr_lo *
+ policy->min = longrun_low_freq + msr.l *
((longrun_high_freq - longrun_low_freq) / 100);
- policy->max = longrun_low_freq + msr_hi *
+ policy->max = longrun_low_freq + msr.h *
((longrun_high_freq - longrun_low_freq) / 100);
}
policy->cpu = 0;
@@ -71,7 +71,7 @@ static void longrun_get_policy(struct cpufreq_policy *policy)
*/
static int longrun_set_policy(struct cpufreq_policy *policy)
{
- u32 msr_lo, msr_hi;
+ struct msr msr;
u32 pctg_lo, pctg_hi;
if (!policy)
@@ -93,24 +93,24 @@ static int longrun_set_policy(struct cpufreq_policy *policy)
pctg_lo = pctg_hi;
/* performance or economy mode */
- rdmsr(MSR_TMTA_LONGRUN_FLAGS, msr_lo, msr_hi);
- msr_lo &= 0xFFFFFFFE;
+ rdmsrq(MSR_TMTA_LONGRUN_FLAGS, msr.q);
+ msr.l &= 0xFFFFFFFE;
switch (policy->policy) {
case CPUFREQ_POLICY_PERFORMANCE:
- msr_lo |= 0x00000001;
+ msr.l |= 0x00000001;
break;
case CPUFREQ_POLICY_POWERSAVE:
break;
}
- wrmsr(MSR_TMTA_LONGRUN_FLAGS, msr_lo, msr_hi);
+ wrmsrq(MSR_TMTA_LONGRUN_FLAGS, msr.q);
/* lower and upper boundary */
- rdmsr(MSR_TMTA_LONGRUN_CTRL, msr_lo, msr_hi);
- msr_lo &= 0xFFFFFF80;
- msr_hi &= 0xFFFFFF80;
- msr_lo |= pctg_lo;
- msr_hi |= pctg_hi;
- wrmsr(MSR_TMTA_LONGRUN_CTRL, msr_lo, msr_hi);
+ rdmsrq(MSR_TMTA_LONGRUN_CTRL, msr.q);
+ msr.l &= 0xFFFFFF80;
+ msr.h &= 0xFFFFFF80;
+ msr.l |= pctg_lo;
+ msr.h |= pctg_hi;
+ wrmsrq(MSR_TMTA_LONGRUN_CTRL, msr.q);
return 0;
}
@@ -160,7 +160,7 @@ static unsigned int longrun_get(unsigned int cpu)
static int longrun_determine_freqs(unsigned int *low_freq,
unsigned int *high_freq)
{
- u32 msr_lo, msr_hi;
+ struct msr msr;
u32 save_lo, save_hi;
u32 eax, ebx, ecx, edx;
u32 try_hi;
@@ -178,15 +178,17 @@ static int longrun_determine_freqs(unsigned int *low_freq,
* For maximum frequency, read out level zero.
*/
/* minimum */
- rdmsr(MSR_TMTA_LRTI_READOUT, msr_lo, msr_hi);
- wrmsr(MSR_TMTA_LRTI_READOUT, msr_hi, msr_hi);
- rdmsr(MSR_TMTA_LRTI_VOLT_MHZ, msr_lo, msr_hi);
- *low_freq = msr_lo * 1000; /* to kHz */
+ rdmsrq(MSR_TMTA_LRTI_READOUT, msr.q);
+ msr.l = msr.h;
+ wrmsrq(MSR_TMTA_LRTI_READOUT, msr.q);
+ rdmsrq(MSR_TMTA_LRTI_VOLT_MHZ, msr.q);
+ *low_freq = msr.l * 1000; /* to kHz */
/* maximum */
- wrmsr(MSR_TMTA_LRTI_READOUT, 0, msr_hi);
- rdmsr(MSR_TMTA_LRTI_VOLT_MHZ, msr_lo, msr_hi);
- *high_freq = msr_lo * 1000; /* to kHz */
+ msr.l = 0;
+ wrmsrq(MSR_TMTA_LRTI_READOUT, msr.q);
+ rdmsrq(MSR_TMTA_LRTI_VOLT_MHZ, msr.q);
+ *high_freq = msr.l * 1000; /* to kHz */
pr_debug("longrun table interface told %u - %u kHz\n",
*low_freq, *high_freq);
@@ -202,9 +204,9 @@ static int longrun_determine_freqs(unsigned int *low_freq,
pr_debug("high frequency is %u kHz\n", *high_freq);
/* get current borders */
- rdmsr(MSR_TMTA_LONGRUN_CTRL, msr_lo, msr_hi);
- save_lo = msr_lo & 0x0000007F;
- save_hi = msr_hi & 0x0000007F;
+ rdmsrq(MSR_TMTA_LONGRUN_CTRL, msr.q);
+ save_lo = msr.l & 0x0000007F;
+ save_hi = msr.h & 0x0000007F;
/* if current perf_pctg is larger than 90%, we need to decrease the
* upper limit to make the calculation more accurate.
@@ -214,16 +216,18 @@ static int longrun_determine_freqs(unsigned int *low_freq,
* on some barrier values */
for (try_hi = 80; try_hi > 0 && ecx > 90; try_hi -= 10) {
/* set to 0 to try_hi perf_pctg */
- msr_lo &= 0xFFFFFF80;
- msr_hi &= 0xFFFFFF80;
- msr_hi |= try_hi;
- wrmsr(MSR_TMTA_LONGRUN_CTRL, msr_lo, msr_hi);
+ msr.l &= 0xFFFFFF80;
+ msr.h &= 0xFFFFFF80;
+ msr.h |= try_hi;
+ wrmsrq(MSR_TMTA_LONGRUN_CTRL, msr.q);
/* read out current core MHz and current perf_pctg */
cpuid(0x80860007, &eax, &ebx, &ecx, &edx);
/* restore values */
- wrmsr(MSR_TMTA_LONGRUN_CTRL, save_lo, save_hi);
+ msr.l = save_lo;
+ msr.h = save_hi;
+ wrmsrq(MSR_TMTA_LONGRUN_CTRL, msr.q);
}
As sashiko.dev pointed, from the second iter, this patch code
will clear upper 25 bits to zero.
https://sashiko.dev/#/patchset/20260629060526.3638272-1-jgross%40suse.com
for (try_hi = 80; ...; ...) {
- msr_lo &= 0xFFFFFF80; /* iter 1: X.lo[31:7]; iter 2+: same */
- msr_hi &= 0xFFFFFF80;
- msr_hi |= try_hi;
- wrmsr(reg, msr_lo, msr_hi);/* writes X.hi[31:7] | try_hi */
+ msr.l &= 0xFFFFFF80; /* iter 2+: locals=0 -> mask = 0 */
+ msr.h &= 0xFFFFFF80;
+ msr.h |= try_hi;
+ wrmsrq(reg, msr.q); /* <-- iter 2+ writes 0 (vs X.hi[31:7])*/
cpuid(...);
/* restore */
- wrmsr(reg, save_lo, save_hi); /* locals untouched */
+ msr.l = save_lo; /* poisons locals ->next iter bug */
+ msr.h = save_hi;
+ wrmsrq(reg, msr.q);
}
I'm not sure whether the upper 25 bits are reserved, but the original
code preserved them across iterations. It might be safer to keep that
behavior unchanged.
Will fix that.
Juergen
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature