[PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Juergen Gross]

Drop the variants using 2 32-bit values instead of a single 64-bit one
of the *_on_cpu() MSR access functions.

Juergen Gross (8):
  x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
  x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
  x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
  x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
  x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
  x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use
    rdmsr_safe_on_cpu()
  x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
  x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use
    wrmsr_safe_on_cpu()

 arch/x86/events/intel/ds.c                    | 11 +--
 arch/x86/events/intel/pt.c                    |  2 +-
 arch/x86/events/intel/uncore_discovery.c      |  2 +-
 arch/x86/events/intel/uncore_snbep.c          |  2 +-
 arch/x86/include/asm/msr.h                    | 44 +++--------
 arch/x86/kernel/acpi/cppc.c                   |  8 +-
 arch/x86/kernel/cpu/intel_epb.c               |  8 +-
 arch/x86/kernel/cpu/mce/amd.c                 |  6 +-
 arch/x86/kernel/cpu/mce/inject.c              | 12 +--
 arch/x86/kernel/cpu/microcode/intel.c         |  2 +-
 arch/x86/kernel/msr.c                         |  8 +-
 arch/x86/lib/msr-smp.c                        | 79 ++-----------------
 drivers/cpufreq/acpi-cpufreq.c                |  4 +-
 drivers/cpufreq/amd-pstate-ut.c               |  2 +-
 drivers/cpufreq/amd-pstate.c                  | 21 +++--
 drivers/cpufreq/amd_freq_sensitivity.c        |  4 +-
 drivers/cpufreq/intel_pstate.c                | 64 +++++++--------
 drivers/cpufreq/p4-clockmod.c                 | 32 ++++----
 drivers/cpufreq/speedstep-centrino.c          | 27 ++++---
 drivers/hwmon/coretemp.c                      | 44 +++++------
 drivers/hwmon/via-cputemp.c                   | 16 ++--
 drivers/platform/x86/amd/hfi/hfi.c            |  4 +-
 .../intel/speed_select_if/isst_if_common.c    | 13 ++-
 .../intel/uncore-frequency/uncore-frequency.c | 12 +--
 drivers/powercap/intel_rapl_msr.c             |  2 +-
 drivers/thermal/intel/intel_tcc.c             | 43 +++++-----
 drivers/thermal/intel/x86_pkg_temp_thermal.c  | 22 +++---
 27 files changed, 203 insertions(+), 291 deletions(-)

-- 
2.54.0

[PATCH 2/8] x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()

[Juergen Gross]

Now that rdmsr_on_cpu() has the same interface as rdmsrq_on_cpu(), the
callers of rdmsrq_on_cpu() can be switched to rdmsr_on_cpu() and
rdmsrq_on_cpu() can be removed.

At the same time switch the only user of rdmsrl_on_cpu() to
rdmsr_on_cpu() and drop rdmsrl_on_cpu(), too.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/events/intel/uncore_snbep.c          |  2 +-
 arch/x86/include/asm/msr.h                    |  7 ------
 arch/x86/kernel/cpu/intel_epb.c               |  4 ++--
 arch/x86/kernel/cpu/mce/inject.c              |  4 ++--
 arch/x86/kernel/cpu/microcode/intel.c         |  2 +-
 arch/x86/lib/msr-smp.c                        | 15 -------------
 drivers/cpufreq/acpi-cpufreq.c                |  4 ++--
 drivers/cpufreq/amd-pstate.c                  |  8 +++----
 drivers/cpufreq/intel_pstate.c                | 22 +++++++++----------
 .../intel/uncore-frequency/uncore-frequency.c |  6 ++---
 10 files changed, 26 insertions(+), 48 deletions(-)

diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index 215d33e260ed..fee94698b611 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -3695,7 +3695,7 @@ static int skx_msr_cpu_bus_read(int cpu, u64 *topology)
 {
 	u64 msr_value;
 
-	if (rdmsrq_on_cpu(cpu, SKX_MSR_CPU_BUS_NUMBER, &msr_value) ||
+	if (rdmsr_on_cpu(cpu, SKX_MSR_CPU_BUS_NUMBER, &msr_value) ||
 			!(msr_value & SKX_MSR_CPU_BUS_VALID_BIT))
 		return -ENXIO;
 
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index fcdaeddf4337..8c96fc5c6169 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -258,7 +258,6 @@ int msr_clear_bit(u32 msr, u8 bit);
 #ifdef CONFIG_SMP
 int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u64 *q);
 int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h);
-int rdmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 *q);
 int wrmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 q);
 void rdmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *msrs);
 void wrmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *msrs);
@@ -279,11 +278,6 @@ static inline int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 	wrmsr(msr_no, l, h);
 	return 0;
 }
-static inline int rdmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
-{
-	rdmsrq(msr_no, *q);
-	return 0;
-}
 static inline int wrmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 {
 	wrmsrq(msr_no, q);
@@ -329,7 +323,6 @@ static inline int wrmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8])
 /* Compatibility wrappers: */
 #define rdmsrl(msr, val) rdmsrq(msr, val)
 #define wrmsrl(msr, val) wrmsrq(msr, val)
-#define rdmsrl_on_cpu(cpu, msr, q) rdmsrq_on_cpu(cpu, msr, q)
 
 #endif /* __ASSEMBLER__ */
 #endif /* _ASM_X86_MSR_H */
diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
index 2c56f8730f59..cb5a3c299f26 100644
--- a/arch/x86/kernel/cpu/intel_epb.c
+++ b/arch/x86/kernel/cpu/intel_epb.c
@@ -139,7 +139,7 @@ static ssize_t energy_perf_bias_show(struct device *dev,
 	u64 epb;
 	int ret;
 
-	ret = rdmsrq_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
+	ret = rdmsr_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
 	if (ret < 0)
 		return ret;
 
@@ -161,7 +161,7 @@ static ssize_t energy_perf_bias_store(struct device *dev,
 	else if (kstrtou64(buf, 0, &val) || val > MAX_EPB)
 		return -EINVAL;
 
-	ret = rdmsrq_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
+	ret = rdmsr_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
 	if (ret < 0)
 		return ret;
 
diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index fa13a8a4946b..78649651c987 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -590,7 +590,7 @@ static int inj_bank_set(void *data, u64 val)
 	u64 cap;
 
 	/* Get bank count on target CPU so we can handle non-uniform values. */
-	rdmsrq_on_cpu(m->extcpu, MSR_IA32_MCG_CAP, &cap);
+	rdmsr_on_cpu(m->extcpu, MSR_IA32_MCG_CAP, &cap);
 	n_banks = cap & MCG_BANKCNT_MASK;
 
 	if (val >= n_banks) {
@@ -614,7 +614,7 @@ static int inj_bank_set(void *data, u64 val)
 	if (cpu_feature_enabled(X86_FEATURE_SMCA)) {
 		u64 ipid;
 
-		if (rdmsrq_on_cpu(m->extcpu, MSR_AMD64_SMCA_MCx_IPID(val), &ipid)) {
+		if (rdmsr_on_cpu(m->extcpu, MSR_AMD64_SMCA_MCx_IPID(val), &ipid)) {
 			pr_err("Error reading IPID on CPU%d\n", m->extcpu);
 			return -EINVAL;
 		}
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index a4c0a0cf928b..0b49ee6ddd2e 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -663,7 +663,7 @@ static void stage_microcode(void)
 
 		pkg_id = topology_logical_package_id(cpu);
 
-		err = rdmsrq_on_cpu(cpu, MSR_IA32_MCU_STAGING_MBOX_ADDR, &mmio_pa);
+		err = rdmsr_on_cpu(cpu, MSR_IA32_MCU_STAGING_MBOX_ADDR, &mmio_pa);
 		if (WARN_ON_ONCE(err))
 			return;
 
diff --git a/arch/x86/lib/msr-smp.c b/arch/x86/lib/msr-smp.c
index 6e04aabda863..7c96f003bfe0 100644
--- a/arch/x86/lib/msr-smp.c
+++ b/arch/x86/lib/msr-smp.c
@@ -46,21 +46,6 @@ int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
 }
 EXPORT_SYMBOL(rdmsr_on_cpu);
 
-int rdmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
-{
-	int err;
-	struct msr_info rv;
-
-	memset(&rv, 0, sizeof(rv));
-
-	rv.msr_no = msr_no;
-	err = smp_call_function_single(cpu, __rdmsr_on_cpu, &rv, 1);
-	*q = rv.reg.q;
-
-	return err;
-}
-EXPORT_SYMBOL(rdmsrq_on_cpu);
-
 int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 {
 	int err;
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 21639d9ac753..43bf1c21c4ca 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -79,11 +79,11 @@ static bool boost_state(unsigned int cpu)
 	case X86_VENDOR_INTEL:
 	case X86_VENDOR_CENTAUR:
 	case X86_VENDOR_ZHAOXIN:
-		rdmsrq_on_cpu(cpu, MSR_IA32_MISC_ENABLE, &msr);
+		rdmsr_on_cpu(cpu, MSR_IA32_MISC_ENABLE, &msr);
 		return !(msr & MSR_IA32_MISC_ENABLE_TURBO_DISABLE);
 	case X86_VENDOR_HYGON:
 	case X86_VENDOR_AMD:
-		rdmsrq_on_cpu(cpu, MSR_K7_HWCR, &msr);
+		rdmsr_on_cpu(cpu, MSR_K7_HWCR, &msr);
 		return !(msr & MSR_K7_HWCR_CPB_DIS);
 	}
 	return false;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 62b5d995281d..8da4f0f034e2 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -204,7 +204,7 @@ static u8 msr_get_epp(struct amd_cpudata *cpudata)
 	u64 value;
 	int ret;
 
-	ret = rdmsrq_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &value);
+	ret = rdmsr_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &value);
 	if (ret < 0) {
 		pr_debug("Could not retrieve energy perf value (%d)\n", ret);
 		return ret;
@@ -378,7 +378,7 @@ static int amd_pstate_init_floor_perf(struct cpufreq_policy *policy)
 	if (!cpu_feature_enabled(X86_FEATURE_CPPC_PERF_PRIO))
 		return 0;
 
-	ret = rdmsrq_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ2, &value);
+	ret = rdmsr_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ2, &value);
 	if (ret) {
 		pr_err("failed to read CPPC REQ2 value. Error (%d)\n", ret);
 		return ret;
@@ -476,7 +476,7 @@ static int msr_init_perf(struct amd_cpudata *cpudata)
 	if (ret)
 		return ret;
 
-	ret = rdmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &cppc_req);
+	ret = rdmsr_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, &cppc_req);
 	if (ret)
 		return ret;
 
@@ -877,7 +877,7 @@ static int amd_pstate_init_boost_support(struct amd_cpudata *cpudata)
 		goto exit_err;
 	}
 
-	ret = rdmsrq_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
+	ret = rdmsr_on_cpu(cpudata->cpu, MSR_K7_HWCR, &boost_val);
 	if (ret) {
 		pr_err_once("failed to read initial CPU boost state!\n");
 		ret = -EIO;
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 1f093e346430..4cd3686f194a 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -632,8 +632,8 @@ static s16 intel_pstate_get_epp(struct cpudata *cpu_data, u64 hwp_req_data)
 		 * MSR_HWP_REQUEST, so need to read and get EPP.
 		 */
 		if (!hwp_req_data) {
-			epp = rdmsrq_on_cpu(cpu_data->cpu, MSR_HWP_REQUEST,
-					    &hwp_req_data);
+			epp = rdmsr_on_cpu(cpu_data->cpu, MSR_HWP_REQUEST,
+					   &hwp_req_data);
 			if (epp)
 				return epp;
 		}
@@ -886,7 +886,7 @@ static ssize_t show_base_frequency(struct cpufreq_policy *policy, char *buf)
 	if (ratio <= 0) {
 		u64 cap;
 
-		rdmsrq_on_cpu(policy->cpu, MSR_HWP_CAPABILITIES, &cap);
+		rdmsr_on_cpu(policy->cpu, MSR_HWP_CAPABILITIES, &cap);
 		ratio = HWP_GUARANTEED_PERF(cap);
 	}
 
@@ -1187,7 +1187,7 @@ static void __intel_pstate_get_hwp_cap(struct cpudata *cpu)
 {
 	u64 cap;
 
-	rdmsrq_on_cpu(cpu->cpu, MSR_HWP_CAPABILITIES, &cap);
+	rdmsr_on_cpu(cpu->cpu, MSR_HWP_CAPABILITIES, &cap);
 	WRITE_ONCE(cpu->hwp_cap_cached, cap);
 	cpu->pstate.max_pstate = HWP_GUARANTEED_PERF(cap);
 	cpu->pstate.turbo_pstate = HWP_HIGHEST_PERF(cap);
@@ -1269,7 +1269,7 @@ static void intel_pstate_hwp_set(unsigned int cpu)
 	if (cpu_data->policy == CPUFREQ_POLICY_PERFORMANCE)
 		min = max;
 
-	rdmsrq_on_cpu(cpu, MSR_HWP_REQUEST, &value);
+	rdmsr_on_cpu(cpu, MSR_HWP_REQUEST, &value);
 
 	value &= ~HWP_MIN_PERF(~0L);
 	value |= HWP_MIN_PERF(min);
@@ -2156,7 +2156,7 @@ static int core_get_min_pstate(int cpu)
 {
 	u64 value;
 
-	rdmsrq_on_cpu(cpu, MSR_PLATFORM_INFO, &value);
+	rdmsr_on_cpu(cpu, MSR_PLATFORM_INFO, &value);
 	return (value >> 40) & 0xFF;
 }
 
@@ -2164,7 +2164,7 @@ static int core_get_max_pstate_physical(int cpu)
 {
 	u64 value;
 
-	rdmsrq_on_cpu(cpu, MSR_PLATFORM_INFO, &value);
+	rdmsr_on_cpu(cpu, MSR_PLATFORM_INFO, &value);
 	return (value >> 8) & 0xFF;
 }
 
@@ -2209,7 +2209,7 @@ static int core_get_max_pstate(int cpu)
 	int tdp_ratio;
 	int err;
 
-	rdmsrq_on_cpu(cpu, MSR_PLATFORM_INFO, &plat_info);
+	rdmsr_on_cpu(cpu, MSR_PLATFORM_INFO, &plat_info);
 	max_pstate = (plat_info >> 8) & 0xFF;
 
 	tdp_ratio = core_get_tdp_ratio(cpu, plat_info);
@@ -2241,7 +2241,7 @@ static int core_get_turbo_pstate(int cpu)
 	u64 value;
 	int nont, ret;
 
-	rdmsrq_on_cpu(cpu, MSR_TURBO_RATIO_LIMIT, &value);
+	rdmsr_on_cpu(cpu, MSR_TURBO_RATIO_LIMIT, &value);
 	nont = core_get_max_pstate(cpu);
 	ret = (value) & 255;
 	if (ret <= nont)
@@ -2264,7 +2264,7 @@ static int knl_get_turbo_pstate(int cpu)
 	u64 value;
 	int nont, ret;
 
-	rdmsrq_on_cpu(cpu, MSR_TURBO_RATIO_LIMIT, &value);
+	rdmsr_on_cpu(cpu, MSR_TURBO_RATIO_LIMIT, &value);
 	nont = core_get_max_pstate(cpu);
 	ret = (((value) >> 8) & 0xFF);
 	if (ret <= nont)
@@ -3318,7 +3318,7 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
 
 		intel_pstate_get_hwp_cap(cpu);
 
-		rdmsrq_on_cpu(cpu->cpu, MSR_HWP_REQUEST, &value);
+		rdmsr_on_cpu(cpu->cpu, MSR_HWP_REQUEST, &value);
 		WRITE_ONCE(cpu->hwp_req_cached, value);
 
 		cpu->epp_cached = intel_pstate_get_epp(cpu, value);
diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
index 667f2c8b9594..b9878a4d391b 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
@@ -52,7 +52,7 @@ static int uncore_read_control_freq(struct uncore_data *data, unsigned int *valu
 	if (data->control_cpu < 0)
 		return -ENXIO;
 
-	ret = rdmsrq_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, &cap);
+	ret = rdmsr_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, &cap);
 	if (ret)
 		return ret;
 
@@ -77,7 +77,7 @@ static int uncore_write_control_freq(struct uncore_data *data, unsigned int inpu
 	if (data->control_cpu < 0)
 		return -ENXIO;
 
-	ret = rdmsrq_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, &cap);
+	ret = rdmsr_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, &cap);
 	if (ret)
 		return ret;
 
@@ -106,7 +106,7 @@ static int uncore_read_freq(struct uncore_data *data, unsigned int *freq)
 	if (data->control_cpu < 0)
 		return -ENXIO;
 
-	ret = rdmsrq_on_cpu(data->control_cpu, MSR_UNCORE_PERF_STATUS, &ratio);
+	ret = rdmsr_on_cpu(data->control_cpu, MSR_UNCORE_PERF_STATUS, &ratio);
 	if (ret)
 		return ret;
 
-- 
2.54.0

Re: [PATCH 2/8] x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Now that rdmsr_on_cpu() has the same interface as rdmsrq_on_cpu(), the
> callers of rdmsrq_on_cpu() can be switched to rdmsr_on_cpu() and
> rdmsrq_on_cpu() can be removed.
> 
> At the same time switch the only user of rdmsrl_on_cpu() to
> rdmsr_on_cpu() and drop rdmsrl_on_cpu(), too.

So instead of:

  x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
  x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()

We want something like this:

  x86/msr: Convert rdmsrl_on_cpu() users to rdmsrq_on_cpu()
  x86/msr: Drop the rdmsrl_on_cpu() alias to rdmsrq_on_cpu()
  x86/msr: Switch all callers of rdmsr_on_cpu() to use rdmsrq_on_cpu()
  x86/msr: Drop the obsolete rdmsr_on_cpu() API

Patch #1-#2 can be a single patch I guess, since there's only
a single user.

Thanks,

	Ingo

[PATCH 4/8] x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()

[Juergen Gross]

Now that wrmsr_on_cpu() has the same interface as wrmsrq_on_cpu(), the
callers of wrmsrq_on_cpu() can be switched to wrmsr_on_cpu() and
wrmsrq_on_cpu() can be removed.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/msr.h                    |  8 +----
 arch/x86/kernel/cpu/intel_epb.c               |  4 +--
 arch/x86/lib/msr-smp.c                        | 16 ---------
 drivers/cpufreq/amd-pstate.c                  |  8 ++---
 drivers/cpufreq/intel_pstate.c                | 36 +++++++++----------
 drivers/platform/x86/amd/hfi/hfi.c            |  4 +--
 .../intel/uncore-frequency/uncore-frequency.c |  6 ++--
 7 files changed, 30 insertions(+), 52 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index a004440b4c0a..c0a3bfba6b56 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -258,7 +258,6 @@ int msr_clear_bit(u32 msr, u8 bit);
 #ifdef CONFIG_SMP
 int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u64 *q);
 int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u64 q);
-int wrmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 q);
 void rdmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *msrs);
 void wrmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *msrs);
 int rdmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h);
@@ -278,11 +277,6 @@ static inline int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 	wrmsrq(msr_no, q);
 	return 0;
 }
-static inline int wrmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
-{
-	wrmsrq(msr_no, q);
-	return 0;
-}
 static inline void rdmsr_on_cpus(const struct cpumask *m, u32 msr_no,
 				struct msr __percpu *msrs)
 {
@@ -291,7 +285,7 @@ static inline void rdmsr_on_cpus(const struct cpumask *m, u32 msr_no,
 static inline void wrmsr_on_cpus(const struct cpumask *m, u32 msr_no,
 				struct msr __percpu *msrs)
 {
-	wrmsrq_on_cpu(0, msr_no, raw_cpu_read(msrs->q));
+	wrmsr_on_cpu(0, msr_no, raw_cpu_read(msrs->q));
 }
 static inline int rdmsr_safe_on_cpu(unsigned int cpu, u32 msr_no,
 				    u32 *l, u32 *h)
diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
index cb5a3c299f26..7533f47bf63d 100644
--- a/arch/x86/kernel/cpu/intel_epb.c
+++ b/arch/x86/kernel/cpu/intel_epb.c
@@ -165,8 +165,8 @@ static ssize_t energy_perf_bias_store(struct device *dev,
 	if (ret < 0)
 		return ret;
 
-	ret = wrmsrq_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS,
-			    (epb & ~EPB_MASK) | val);
+	ret = wrmsr_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS,
+			   (epb & ~EPB_MASK) | val);
 	if (ret < 0)
 		return ret;
 
diff --git a/arch/x86/lib/msr-smp.c b/arch/x86/lib/msr-smp.c
index 0b4f3c4e4f82..42d42641f2aa 100644
--- a/arch/x86/lib/msr-smp.c
+++ b/arch/x86/lib/msr-smp.c
@@ -61,22 +61,6 @@ int wrmsr_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 }
 EXPORT_SYMBOL(wrmsr_on_cpu);
 
-int wrmsrq_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
-{
-	int err;
-	struct msr_info rv;
-
-	memset(&rv, 0, sizeof(rv));
-
-	rv.msr_no = msr_no;
-	rv.reg.q = q;
-
-	err = smp_call_function_single(cpu, __wrmsr_on_cpu, &rv, 1);
-
-	return err;
-}
-EXPORT_SYMBOL(wrmsrq_on_cpu);
-
 static void __rwmsr_on_cpus(const struct cpumask *mask, u32 msr_no,
 			    struct msr __percpu *msrs,
 			    void (*msr_func) (void *info))
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8da4f0f034e2..c98ead520224 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -267,7 +267,7 @@ static int msr_update_perf(struct cpufreq_policy *policy, u8 min_perf,
 	if (fast_switch) {
 		wrmsrq(MSR_AMD_CPPC_REQ, value);
 	} else {
-		int ret = wrmsrq_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+		int ret = wrmsr_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
 
 		if (ret)
 			return ret;
@@ -315,7 +315,7 @@ static int msr_set_epp(struct cpufreq_policy *policy, u8 epp)
 	if (value == prev)
 		return 0;
 
-	ret = wrmsrq_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
+	ret = wrmsr_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value);
 	if (ret) {
 		pr_err("failed to set energy perf value (%d)\n", ret);
 		return ret;
@@ -353,7 +353,7 @@ static int amd_pstate_set_floor_perf(struct cpufreq_policy *policy, u8 perf)
 		goto out_trace;
 	}
 
-	ret = wrmsrq_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ2, value);
+	ret = wrmsr_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ2, value);
 	if (ret) {
 		changed = false;
 		pr_err("failed to set CPPC REQ2 value. Error (%d)\n", ret);
@@ -896,7 +896,7 @@ static int amd_pstate_init_boost_support(struct amd_cpudata *cpudata)
 
 static void amd_perf_ctl_reset(unsigned int cpu)
 {
-	wrmsrq_on_cpu(cpu, MSR_AMD_PERF_CTL, 0);
+	wrmsr_on_cpu(cpu, MSR_AMD_PERF_CTL, 0);
 }
 
 #define CPPC_MAX_PERF	U8_MAX
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 4cd3686f194a..3adc033a3492 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -736,7 +736,7 @@ static int intel_pstate_set_epp(struct cpudata *cpu, u32 epp)
 	 * function, so it cannot run in parallel with the update below.
 	 */
 	WRITE_ONCE(cpu->hwp_req_cached, value);
-	ret = wrmsrq_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
+	ret = wrmsr_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
 	if (!ret)
 		cpu->epp_cached = epp;
 
@@ -1315,7 +1315,7 @@ static void intel_pstate_hwp_set(unsigned int cpu)
 
 skip_epp:
 	WRITE_ONCE(cpu_data->hwp_req_cached, value);
-	wrmsrq_on_cpu(cpu, MSR_HWP_REQUEST, value);
+	wrmsr_on_cpu(cpu, MSR_HWP_REQUEST, value);
 }
 
 static void intel_pstate_disable_hwp_interrupt(struct cpudata *cpudata);
@@ -1362,7 +1362,7 @@ static void intel_pstate_hwp_offline(struct cpudata *cpu)
 	if (boot_cpu_has(X86_FEATURE_HWP_EPP))
 		value |= HWP_ENERGY_PERF_PREFERENCE(HWP_EPP_POWERSAVE);
 
-	wrmsrq_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
+	wrmsr_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
 
 	mutex_lock(&hybrid_capacity_lock);
 
@@ -1411,7 +1411,7 @@ static void intel_pstate_hwp_enable(struct cpudata *cpudata);
 static void intel_pstate_hwp_reenable(struct cpudata *cpu)
 {
 	intel_pstate_hwp_enable(cpu);
-	wrmsrq_on_cpu(cpu->cpu, MSR_HWP_REQUEST, READ_ONCE(cpu->hwp_req_cached));
+	wrmsr_on_cpu(cpu->cpu, MSR_HWP_REQUEST, READ_ONCE(cpu->hwp_req_cached));
 }
 
 static int intel_pstate_suspend(struct cpufreq_policy *policy)
@@ -1919,7 +1919,7 @@ static void intel_pstate_notify_work(struct work_struct *work)
 		hybrid_update_capacity(cpudata);
 	}
 
-	wrmsrq_on_cpu(cpudata->cpu, MSR_HWP_STATUS, 0);
+	wrmsr_on_cpu(cpudata->cpu, MSR_HWP_STATUS, 0);
 }
 
 static DEFINE_RAW_SPINLOCK(hwp_notify_lock);
@@ -1969,8 +1969,8 @@ static void intel_pstate_disable_hwp_interrupt(struct cpudata *cpudata)
 	if (!cpu_feature_enabled(X86_FEATURE_HWP_NOTIFY))
 		return;
 
-	/* wrmsrq_on_cpu has to be outside spinlock as this can result in IPC */
-	wrmsrq_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
+	/* wrmsr_on_cpu has to be outside spinlock as this can result in IPC */
+	wrmsr_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
 
 	raw_spin_lock_irq(&hwp_notify_lock);
 	cancel_work = cpumask_test_and_clear_cpu(cpudata->cpu, &hwp_intr_enable_mask);
@@ -1997,9 +1997,9 @@ static void intel_pstate_enable_hwp_interrupt(struct cpudata *cpudata)
 		if (cpu_feature_enabled(X86_FEATURE_HWP_HIGHEST_PERF_CHANGE))
 			interrupt_mask |= HWP_HIGHEST_PERF_CHANGE_REQ;
 
-		/* wrmsrq_on_cpu has to be outside spinlock as this can result in IPC */
-		wrmsrq_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, interrupt_mask);
-		wrmsrq_on_cpu(cpudata->cpu, MSR_HWP_STATUS, 0);
+		/* wrmsr_on_cpu has to be outside spinlock as this can result in IPC */
+		wrmsr_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, interrupt_mask);
+		wrmsr_on_cpu(cpudata->cpu, MSR_HWP_STATUS, 0);
 	}
 }
 
@@ -2038,9 +2038,9 @@ static void intel_pstate_hwp_enable(struct cpudata *cpudata)
 {
 	/* First disable HWP notification interrupt till we activate again */
 	if (boot_cpu_has(X86_FEATURE_HWP_NOTIFY))
-		wrmsrq_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
+		wrmsr_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
 
-	wrmsrq_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
+	wrmsr_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
 
 	intel_pstate_enable_hwp_interrupt(cpudata);
 
@@ -2306,8 +2306,8 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
 	 * the CPU being updated, so force the register update to run on the
 	 * right CPU.
 	 */
-	wrmsrq_on_cpu(cpu->cpu, MSR_IA32_PERF_CTL,
-		      pstate_funcs.get_val(cpu, pstate));
+	wrmsr_on_cpu(cpu->cpu, MSR_IA32_PERF_CTL,
+		     pstate_funcs.get_val(cpu, pstate));
 }
 
 static void intel_pstate_set_min_pstate(struct cpudata *cpu)
@@ -3164,7 +3164,7 @@ static void intel_cpufreq_hwp_update(struct cpudata *cpu, u32 min, u32 max,
 	if (fast_switch)
 		wrmsrq(MSR_HWP_REQUEST, value);
 	else
-		wrmsrq_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
+		wrmsr_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
 }
 
 static void intel_cpufreq_perf_ctl_update(struct cpudata *cpu,
@@ -3174,8 +3174,8 @@ static void intel_cpufreq_perf_ctl_update(struct cpudata *cpu,
 		wrmsrq(MSR_IA32_PERF_CTL,
 		       pstate_funcs.get_val(cpu, target_pstate));
 	else
-		wrmsrq_on_cpu(cpu->cpu, MSR_IA32_PERF_CTL,
-			      pstate_funcs.get_val(cpu, target_pstate));
+		wrmsr_on_cpu(cpu->cpu, MSR_IA32_PERF_CTL,
+			     pstate_funcs.get_val(cpu, target_pstate));
 }
 
 static int intel_cpufreq_update_pstate(struct cpufreq_policy *policy,
@@ -3385,7 +3385,7 @@ static int intel_cpufreq_suspend(struct cpufreq_policy *policy)
 		 * written by it may not be suitable.
 		 */
 		value &= ~HWP_DESIRED_PERF(~0L);
-		wrmsrq_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
+		wrmsr_on_cpu(cpu->cpu, MSR_HWP_REQUEST, value);
 		WRITE_ONCE(cpu->hwp_req_cached, value);
 	}
 
diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
index 83863a5e0fbc..580fbc3648bf 100644
--- a/drivers/platform/x86/amd/hfi/hfi.c
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -260,11 +260,11 @@ static int amd_hfi_set_state(unsigned int cpu, bool state)
 {
 	int ret;
 
-	ret = wrmsrq_on_cpu(cpu, MSR_AMD_WORKLOAD_CLASS_CONFIG, state ? 1 : 0);
+	ret = wrmsr_on_cpu(cpu, MSR_AMD_WORKLOAD_CLASS_CONFIG, state ? 1 : 0);
 	if (ret)
 		return ret;
 
-	return wrmsrq_on_cpu(cpu, MSR_AMD_WORKLOAD_HRST, 0x1);
+	return wrmsr_on_cpu(cpu, MSR_AMD_WORKLOAD_HRST, 0x1);
 }
 
 /**
diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
index b9878a4d391b..c4c24a355854 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c
@@ -89,7 +89,7 @@ static int uncore_write_control_freq(struct uncore_data *data, unsigned int inpu
 		cap |= FIELD_PREP(UNCORE_MIN_RATIO_MASK, input);
 	}
 
-	ret = wrmsrq_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, cap);
+	ret = wrmsr_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT, cap);
 	if (ret)
 		return ret;
 
@@ -213,8 +213,8 @@ static int uncore_pm_notify(struct notifier_block *nb, unsigned long mode,
 			if (!data || !data->valid || !data->stored_uncore_data)
 				return 0;
 
-			wrmsrq_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT,
-				      data->stored_uncore_data);
+			wrmsr_on_cpu(data->control_cpu, MSR_UNCORE_RATIO_LIMIT,
+				     data->stored_uncore_data);
 		}
 		break;
 	default:
-- 
2.54.0

Re: [PATCH 4/8] x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Now that wrmsr_on_cpu() has the same interface as wrmsrq_on_cpu(), the
> callers of wrmsrq_on_cpu() can be switched to wrmsr_on_cpu() and
> wrmsrq_on_cpu() can be removed.

Similarly to the other patches, this should be split into two
as well.

Thanks,

	Ingo

Re: [PATCH 4/8] x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
> > Now that wrmsr_on_cpu() has the same interface as wrmsrq_on_cpu(), the
> > callers of wrmsrq_on_cpu() can be switched to wrmsr_on_cpu() and
> > wrmsrq_on_cpu() can be removed.
> 
> Similarly to the other patches, this should be split into two
> as well.

Specifically, instead of:

  x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
  x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()

We want something like this:

  x86/msr: Switch all callers of wrmsr_on_cpu() to use wrmsrq_on_cpu()
  x86/msr: Remove unused wrmsr_on_cpu() API

Thanks,

	Ingo

[PATCH 6/8] x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()

[Juergen Gross]

Now that rdmsr_safe_on_cpu() has the same interface as
rdmsrq_safe_on_cpu(), the callers of rdmsrq_safe_on_cpu() can be
switched to rdmsr_safe_on_cpu() and rdmsrq_safe_on_cpu() can be
removed.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/events/intel/pt.c                             |  2 +-
 arch/x86/events/intel/uncore_discovery.c               |  2 +-
 arch/x86/include/asm/msr.h                             |  5 -----
 arch/x86/kernel/acpi/cppc.c                            |  6 +++---
 arch/x86/lib/msr-smp.c                                 | 10 ----------
 drivers/cpufreq/amd-pstate-ut.c                        |  2 +-
 drivers/cpufreq/amd-pstate.c                           |  3 +--
 drivers/cpufreq/intel_pstate.c                         |  6 +++---
 .../x86/intel/speed_select_if/isst_if_common.c         |  4 ++--
 drivers/powercap/intel_rapl_msr.c                      |  2 +-
 10 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index b5726b50e77d..7c92146b06ea 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1840,7 +1840,7 @@ static __init int pt_init(void)
 	for_each_online_cpu(cpu) {
 		u64 ctl;
 
-		ret = rdmsrq_safe_on_cpu(cpu, MSR_IA32_RTIT_CTL, &ctl);
+		ret = rdmsr_safe_on_cpu(cpu, MSR_IA32_RTIT_CTL, &ctl);
 		if (!ret && (ctl & RTIT_CTL_TRACEEN))
 			prior_warn++;
 	}
diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
index 583cbd06b9b8..0853a9e02fda 100644
--- a/arch/x86/events/intel/uncore_discovery.c
+++ b/arch/x86/events/intel/uncore_discovery.c
@@ -405,7 +405,7 @@ static bool uncore_discovery_msr(struct uncore_discovery_domain *domain)
 		if (__test_and_set_bit(die, die_mask))
 			continue;
 
-		if (rdmsrq_safe_on_cpu(cpu, domain->discovery_base, &base))
+		if (rdmsr_safe_on_cpu(cpu, domain->discovery_base, &base))
 			continue;
 
 		if (!base)
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index b3b43bc04b69..f2d14c670140 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -262,7 +262,6 @@ void rdmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *
 void wrmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *msrs);
 int rdmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q);
 int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h);
-int rdmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q);
 int wrmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q);
 int rdmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8]);
 int wrmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8]);
@@ -295,10 +294,6 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u32 l, u32 h)
 {
 	return wrmsr_safe(msr_no, l, h);
 }
-static inline int rdmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
-{
-	return rdmsrq_safe(msr_no, q);
-}
 static inline int wrmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 {
 	return wrmsrq_safe(msr_no, q);
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
index be4c5e9e5ff6..86bf2dafdf72 100644
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -49,7 +49,7 @@ int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val)
 {
 	int err;
 
-	err = rdmsrq_safe_on_cpu(cpunum, reg->address, val);
+	err = rdmsr_safe_on_cpu(cpunum, reg->address, val);
 	if (!err) {
 		u64 mask = GENMASK_ULL(reg->bit_offset + reg->bit_width - 1,
 				       reg->bit_offset);
@@ -65,7 +65,7 @@ int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
 	u64 rd_val;
 	int err;
 
-	err = rdmsrq_safe_on_cpu(cpunum, reg->address, &rd_val);
+	err = rdmsr_safe_on_cpu(cpunum, reg->address, &rd_val);
 	if (!err) {
 		u64 mask = GENMASK_ULL(reg->bit_offset + reg->bit_width - 1,
 				       reg->bit_offset);
@@ -147,7 +147,7 @@ int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf)
 	int ret;
 
 	if (cpu_feature_enabled(X86_FEATURE_CPPC)) {
-		ret = rdmsrq_safe_on_cpu(cpu, MSR_AMD_CPPC_CAP1, &val);
+		ret = rdmsr_safe_on_cpu(cpu, MSR_AMD_CPPC_CAP1, &val);
 		if (ret)
 			goto out;
 
diff --git a/arch/x86/lib/msr-smp.c b/arch/x86/lib/msr-smp.c
index 0dc3921e0259..fa22ac662c1d 100644
--- a/arch/x86/lib/msr-smp.c
+++ b/arch/x86/lib/msr-smp.c
@@ -186,16 +186,6 @@ int wrmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 }
 EXPORT_SYMBOL(wrmsrq_safe_on_cpu);
 
-int rdmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q)
-{
-	int err;
-
-	err = rdmsr_safe_on_cpu(cpu, msr_no, q);
-
-	return err;
-}
-EXPORT_SYMBOL(rdmsrq_safe_on_cpu);
-
 /*
  * These variants are significantly slower, but allows control over
  * the entire 32-bit GPR set.
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 735b29f76438..b59cb8d95f55 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -170,7 +170,7 @@ static int amd_pstate_ut_check_perf(u32 index)
 			lowest_nonlinear_perf = cppc_perf.lowest_nonlinear_perf;
 			lowest_perf = cppc_perf.lowest_perf;
 		} else {
-			ret = rdmsrq_safe_on_cpu(cpu, MSR_AMD_CPPC_CAP1, &cap1);
+			ret = rdmsr_safe_on_cpu(cpu, MSR_AMD_CPPC_CAP1, &cap1);
 			if (ret) {
 				pr_err("%s read CPPC_CAP1 ret=%d error!\n", __func__, ret);
 				return ret;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index c98ead520224..36c9c1e98115 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -467,8 +467,7 @@ static int msr_init_perf(struct amd_cpudata *cpudata)
 	u64 cap1, numerator, cppc_req;
 	u8 min_perf;
 
-	int ret = rdmsrq_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
-				     &cap1);
+	int ret = rdmsr_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1, &cap1);
 	if (ret)
 		return ret;
 
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 3adc033a3492..f91c6b264f93 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2178,13 +2178,13 @@ static int core_get_tdp_ratio(int cpu, u64 plat_info)
 		int err;
 
 		/* Get the TDP level (0, 1, 2) to get ratios */
-		err = rdmsrq_safe_on_cpu(cpu, MSR_CONFIG_TDP_CONTROL, &tdp_ctrl);
+		err = rdmsr_safe_on_cpu(cpu, MSR_CONFIG_TDP_CONTROL, &tdp_ctrl);
 		if (err)
 			return err;
 
 		/* TDP MSR are continuous starting at 0x648 */
 		tdp_msr = MSR_CONFIG_TDP_NOMINAL + (tdp_ctrl & 0x03);
-		err = rdmsrq_safe_on_cpu(cpu, tdp_msr, &tdp_ratio);
+		err = rdmsr_safe_on_cpu(cpu, tdp_msr, &tdp_ratio);
 		if (err)
 			return err;
 
@@ -2221,7 +2221,7 @@ static int core_get_max_pstate(int cpu)
 		return tdp_ratio;
 	}
 
-	err = rdmsrq_safe_on_cpu(cpu, MSR_TURBO_ACTIVATION_RATIO, &tar);
+	err = rdmsr_safe_on_cpu(cpu, MSR_TURBO_ACTIVATION_RATIO, &tar);
 	if (!err) {
 		int tar_levels;
 
diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
index 1c48bf6d5457..b15a798454dc 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
@@ -511,8 +511,8 @@ static long isst_if_msr_cmd_req(u8 *cmd_ptr, int *write_only, int resume)
 	} else {
 		u64 data;
 
-		ret = rdmsrq_safe_on_cpu(msr_cmd->logical_cpu,
-					 msr_cmd->msr, &data);
+		ret = rdmsr_safe_on_cpu(msr_cmd->logical_cpu,
+					msr_cmd->msr, &data);
 		if (!ret) {
 			msr_cmd->data = data;
 			*write_only = 0;
diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c
index a34543e66446..a6bdbe44c8dd 100644
--- a/drivers/powercap/intel_rapl_msr.c
+++ b/drivers/powercap/intel_rapl_msr.c
@@ -180,7 +180,7 @@ static int rapl_msr_read_raw(int cpu, struct reg_action *ra, bool pmu_ctx)
 		goto out;
 	}
 
-	if (rdmsrq_safe_on_cpu(cpu, ra->reg.msr, &ra->value)) {
+	if (rdmsr_safe_on_cpu(cpu, ra->reg.msr, &ra->value)) {
 		pr_debug("failed to read msr 0x%x on cpu %d\n", ra->reg.msr, cpu);
 		return -EIO;
 	}
-- 
2.54.0

Re: [PATCH 6/8] x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Now that rdmsr_safe_on_cpu() has the same interface as
> rdmsrq_safe_on_cpu(), the callers of rdmsrq_safe_on_cpu() can be
> switched to rdmsr_safe_on_cpu() and rdmsrq_safe_on_cpu() can be
> removed.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Similarly to the other patches, this should be split into two
patches as well.

Thanks,

	Ingo

[PATCH 8/8] x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use wrmsr_safe_on_cpu()

[Juergen Gross]

Now that wrmsr_safe_on_cpu() has the same interface as
wrmsrq_safe_on_cpu(), the callers of wrmsrq_safe_on_cpu() can be
switched to wrmsr_safe_on_cpu() and wrmsrq_safe_on_cpu() can be
removed.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/msr.h                       |  5 -----
 arch/x86/kernel/acpi/cppc.c                      |  2 +-
 arch/x86/lib/msr-smp.c                           | 16 ----------------
 drivers/cpufreq/amd-pstate.c                     |  2 +-
 .../x86/intel/speed_select_if/isst_if_common.c   |  9 ++++-----
 5 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index cb14ede8f587..a5596d268053 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -262,7 +262,6 @@ void rdmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *
 void wrmsr_on_cpus(const struct cpumask *mask, u32 msr_no, struct msr __percpu *msrs);
 int rdmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 *q);
 int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q);
-int wrmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q);
 int rdmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8]);
 int wrmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8]);
 #else  /*  CONFIG_SMP  */
@@ -294,10 +293,6 @@ static inline int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 {
 	return wrmsrq_safe(msr_no, q);
 }
-static inline int wrmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
-{
-	return wrmsrq_safe(msr_no, q);
-}
 static inline int rdmsr_safe_regs_on_cpu(unsigned int cpu, u32 regs[8])
 {
 	return rdmsr_safe_regs(regs);
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
index 86bf2dafdf72..327fdcecfa3b 100644
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -74,7 +74,7 @@ int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
 		val &= mask;
 		rd_val &= ~mask;
 		rd_val |= val;
-		err = wrmsrq_safe_on_cpu(cpunum, reg->address, rd_val);
+		err = wrmsr_safe_on_cpu(cpunum, reg->address, rd_val);
 	}
 	return err;
 }
diff --git a/arch/x86/lib/msr-smp.c b/arch/x86/lib/msr-smp.c
index b2859435f4af..9ae9ff11f1f1 100644
--- a/arch/x86/lib/msr-smp.c
+++ b/arch/x86/lib/msr-smp.c
@@ -169,22 +169,6 @@ int wrmsr_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
 }
 EXPORT_SYMBOL(wrmsr_safe_on_cpu);
 
-int wrmsrq_safe_on_cpu(unsigned int cpu, u32 msr_no, u64 q)
-{
-	int err;
-	struct msr_info rv;
-
-	memset(&rv, 0, sizeof(rv));
-
-	rv.msr_no = msr_no;
-	rv.reg.q = q;
-
-	err = smp_call_function_single(cpu, __wrmsr_safe_on_cpu, &rv, 1);
-
-	return err ? err : rv.err;
-}
-EXPORT_SYMBOL(wrmsrq_safe_on_cpu);
-
 /*
  * These variants are significantly slower, but allows control over
  * the entire 32-bit GPR set.
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 36c9c1e98115..355c5fb9313a 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -446,7 +446,7 @@ static int shmem_set_epp(struct cpufreq_policy *policy, u8 epp)
 
 static inline int msr_cppc_enable(struct cpufreq_policy *policy)
 {
-	return wrmsrq_safe_on_cpu(policy->cpu, MSR_AMD_CPPC_ENABLE, 1);
+	return wrmsr_safe_on_cpu(policy->cpu, MSR_AMD_CPPC_ENABLE, 1);
 }
 
 static int shmem_cppc_enable(struct cpufreq_policy *policy)
diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
index b15a798454dc..9d730e6f155d 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
@@ -192,8 +192,8 @@ void isst_resume_common(void)
 			if (cb->registered)
 				isst_mbox_resume_command(cb, sst_cmd);
 		} else {
-			wrmsrq_safe_on_cpu(sst_cmd->cpu, sst_cmd->cmd,
-					   sst_cmd->data);
+			wrmsr_safe_on_cpu(sst_cmd->cpu, sst_cmd->cmd,
+					  sst_cmd->data);
 		}
 	}
 }
@@ -500,9 +500,8 @@ static long isst_if_msr_cmd_req(u8 *cmd_ptr, int *write_only, int resume)
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
 
-		ret = wrmsrq_safe_on_cpu(msr_cmd->logical_cpu,
-					 msr_cmd->msr,
-					 msr_cmd->data);
+		ret = wrmsr_safe_on_cpu(msr_cmd->logical_cpu,
+					msr_cmd->msr, msr_cmd->data);
 		*write_only = 1;
 		if (!ret && !resume)
 			ret = isst_store_cmd(0, msr_cmd->msr,
-- 
2.54.0

Re: [PATCH 8/8] x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use wrmsr_safe_on_cpu()

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Now that wrmsr_safe_on_cpu() has the same interface as
> wrmsrq_safe_on_cpu(), the callers of wrmsrq_safe_on_cpu() can be
> switched to wrmsr_safe_on_cpu() and wrmsrq_safe_on_cpu() can be
> removed.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/include/asm/msr.h                       |  5 -----
>  arch/x86/kernel/acpi/cppc.c                      |  2 +-
>  arch/x86/lib/msr-smp.c                           | 16 ----------------
>  drivers/cpufreq/amd-pstate.c                     |  2 +-
>  .../x86/intel/speed_select_if/isst_if_common.c   |  9 ++++-----
>  5 files changed, 6 insertions(+), 28 deletions(-)

Please split this into two patches: one that does the API
change only, and one that removes the now obsolete definitions.

Thanks,

	Ingo

Re: [PATCH 8/8] x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use wrmsr_safe_on_cpu()

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
> > Now that wrmsr_safe_on_cpu() has the same interface as
> > wrmsrq_safe_on_cpu(), the callers of wrmsrq_safe_on_cpu() can be
> > switched to wrmsr_safe_on_cpu() and wrmsrq_safe_on_cpu() can be
> > removed.
> > 
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> > ---
> >  arch/x86/include/asm/msr.h                       |  5 -----
> >  arch/x86/kernel/acpi/cppc.c                      |  2 +-
> >  arch/x86/lib/msr-smp.c                           | 16 ----------------
> >  drivers/cpufreq/amd-pstate.c                     |  2 +-
> >  .../x86/intel/speed_select_if/isst_if_common.c   |  9 ++++-----
> >  5 files changed, 6 insertions(+), 28 deletions(-)
> 
> Please split this into two patches: one that does the API
> change only, and one that removes the now obsolete definitions.

Followup, instead of:

  x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
  x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use wrmsr_safe_on_cpu()

Please just transition the users to the 64-bit API and remove the
unused 32-bit API:

  x86/msr: Switch all callers of wrmsr_safe_on_cpu() to use wrmsrq_safe_on_cpu()
  x86/mrs: Remove unused wrmsrq_safe_on_cpu() API

Thanks,

	Ingo

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Ingo Molnar]

* Juergen Gross <jgross@suse.com> wrote:

> Drop the variants using 2 32-bit values instead of a single 64-bit one
> of the *_on_cpu() MSR access functions.
> 
> Juergen Gross (8):
>   x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
>   x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
>   x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
>   x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
>   x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
>   x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()
>   x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
>   x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use  wrmsr_safe_on_cpu()

To sum up my review feedback for the invididual patches, we want
to do this instead:

  x86/msr: Convert rdmsrl_on_cpu() users to rdmsrq_on_cpu()
  x86/msr: Drop the rdmsrl_on_cpu() alias to rdmsrq_on_cpu()

  x86/msr: Switch all callers of rdmsr_on_cpu() to use rdmsrq_on_cpu()
  x86/msr: Remove the unused rdmsr_on_cpu() API

  x86/msr: Switch all callers of wrmsr_on_cpu() to use wrmsrq_on_cpu()
  x86/msr: Remove unused wrmsr_on_cpu() API

  x86/msr: Switch all callers of rdmsr_safe_on_cpu() to use rdmsrq_safe_on_cpu()
  x86/msr: Remove unused rdmsr_safe_on_cpu() API

  x86/msr: Switch all callers of wrmsr_safe_on_cpu() to use wrmsrq_safe_on_cpu()
  x86/mrs: Remove unused wrmsrq_safe_on_cpu() API

Note how there's no "conversion" of the 32-bit API itself in this
approach, we just do a straightforward migration of the users to
the already existing 64-bit APIs, then remove any unused APIs.

Thanks,

	Ingo

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2037 bytes --]

On 05.06.26 11:05, Ingo Molnar wrote:
> 
> * Juergen Gross <jgross@suse.com> wrote:
> 
>> Drop the variants using 2 32-bit values instead of a single 64-bit one
>> of the *_on_cpu() MSR access functions.
>>
>> Juergen Gross (8):
>>    x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
>>    x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
>>    x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
>>    x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
>>    x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
>>    x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()
>>    x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
>>    x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use  wrmsr_safe_on_cpu()
> 
> To sum up my review feedback for the invididual patches, we want
> to do this instead:
> 
>    x86/msr: Convert rdmsrl_on_cpu() users to rdmsrq_on_cpu()
>    x86/msr: Drop the rdmsrl_on_cpu() alias to rdmsrq_on_cpu()
> 
>    x86/msr: Switch all callers of rdmsr_on_cpu() to use rdmsrq_on_cpu()
>    x86/msr: Remove the unused rdmsr_on_cpu() API
> 
>    x86/msr: Switch all callers of wrmsr_on_cpu() to use wrmsrq_on_cpu()
>    x86/msr: Remove unused wrmsr_on_cpu() API
> 
>    x86/msr: Switch all callers of rdmsr_safe_on_cpu() to use rdmsrq_safe_on_cpu()
>    x86/msr: Remove unused rdmsr_safe_on_cpu() API
> 
>    x86/msr: Switch all callers of wrmsr_safe_on_cpu() to use wrmsrq_safe_on_cpu()
>    x86/mrs: Remove unused wrmsrq_safe_on_cpu() API
> 
> Note how there's no "conversion" of the 32-bit API itself in this
> approach, we just do a straightforward migration of the users to
> the already existing 64-bit APIs, then remove any unused APIs.

Fine with me, but I just wanted to get rid of the "q" and "l" suffices
completely, as they serve no special purpose after dropping all other
variants.

OTOH if wanted such a switch could be done later easily.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Ingo Molnar]

* Jürgen Groß <jgross@suse.com> wrote:

> On 05.06.26 11:05, Ingo Molnar wrote:
> > 
> > * Juergen Gross <jgross@suse.com> wrote:
> > 
> > > Drop the variants using 2 32-bit values instead of a single 64-bit one
> > > of the *_on_cpu() MSR access functions.
> > > 
> > > Juergen Gross (8):
> > >    x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
> > >    x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
> > >    x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
> > >    x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
> > >    x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
> > >    x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()
> > >    x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
> > >    x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use  wrmsr_safe_on_cpu()
> > 
> > To sum up my review feedback for the invididual patches, we want
> > to do this instead:
> > 
> >    x86/msr: Convert rdmsrl_on_cpu() users to rdmsrq_on_cpu()
> >    x86/msr: Drop the rdmsrl_on_cpu() alias to rdmsrq_on_cpu()
> > 
> >    x86/msr: Switch all callers of rdmsr_on_cpu() to use rdmsrq_on_cpu()
> >    x86/msr: Remove the unused rdmsr_on_cpu() API
> > 
> >    x86/msr: Switch all callers of wrmsr_on_cpu() to use wrmsrq_on_cpu()
> >    x86/msr: Remove unused wrmsr_on_cpu() API
> > 
> >    x86/msr: Switch all callers of rdmsr_safe_on_cpu() to use rdmsrq_safe_on_cpu()
> >    x86/msr: Remove unused rdmsr_safe_on_cpu() API
> > 
> >    x86/msr: Switch all callers of wrmsr_safe_on_cpu() to use wrmsrq_safe_on_cpu()
> >    x86/mrs: Remove unused wrmsrq_safe_on_cpu() API
> > 
> > Note how there's no "conversion" of the 32-bit API itself in this
> > approach, we just do a straightforward migration of the users to
> > the already existing 64-bit APIs, then remove any unused APIs.
> 
> Fine with me, but I just wanted to get rid of the "q" and "l" suffices
> completely, as they serve no special purpose after dropping all other
> variants.
> 
> OTOH if wanted such a switch could be done later easily.

Well, we had a similar discussion back when we standardized on
rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
MSR handling APIs. Why have a different pattern in any of the
derived APIs? It should really use the same conceptual namespace,
not some confusing mixture of two naming schemes.

Thanks,

	Ingo

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2883 bytes --]

On 05.06.26 11:18, Ingo Molnar wrote:
> 
> * Jürgen Groß <jgross@suse.com> wrote:
> 
>> On 05.06.26 11:05, Ingo Molnar wrote:
>>>
>>> * Juergen Gross <jgross@suse.com> wrote:
>>>
>>>> Drop the variants using 2 32-bit values instead of a single 64-bit one
>>>> of the *_on_cpu() MSR access functions.
>>>>
>>>> Juergen Gross (8):
>>>>     x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
>>>>     x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
>>>>     x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
>>>>     x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
>>>>     x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
>>>>     x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()
>>>>     x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
>>>>     x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use  wrmsr_safe_on_cpu()
>>>
>>> To sum up my review feedback for the invididual patches, we want
>>> to do this instead:
>>>
>>>     x86/msr: Convert rdmsrl_on_cpu() users to rdmsrq_on_cpu()
>>>     x86/msr: Drop the rdmsrl_on_cpu() alias to rdmsrq_on_cpu()
>>>
>>>     x86/msr: Switch all callers of rdmsr_on_cpu() to use rdmsrq_on_cpu()
>>>     x86/msr: Remove the unused rdmsr_on_cpu() API
>>>
>>>     x86/msr: Switch all callers of wrmsr_on_cpu() to use wrmsrq_on_cpu()
>>>     x86/msr: Remove unused wrmsr_on_cpu() API
>>>
>>>     x86/msr: Switch all callers of rdmsr_safe_on_cpu() to use rdmsrq_safe_on_cpu()
>>>     x86/msr: Remove unused rdmsr_safe_on_cpu() API
>>>
>>>     x86/msr: Switch all callers of wrmsr_safe_on_cpu() to use wrmsrq_safe_on_cpu()
>>>     x86/mrs: Remove unused wrmsrq_safe_on_cpu() API
>>>
>>> Note how there's no "conversion" of the 32-bit API itself in this
>>> approach, we just do a straightforward migration of the users to
>>> the already existing 64-bit APIs, then remove any unused APIs.
>>
>> Fine with me, but I just wanted to get rid of the "q" and "l" suffices
>> completely, as they serve no special purpose after dropping all other
>> variants.
>>
>> OTOH if wanted such a switch could be done later easily.
> 
> Well, we had a similar discussion back when we standardized on
> rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
> MSR handling APIs. Why have a different pattern in any of the
> derived APIs? It should really use the same conceptual namespace,
> not some confusing mixture of two naming schemes.

In the long run I'd like to do the same conversion for the rdmsr*() and
wrmsr*() interfaces, too (so only offering and using the 64-bit variants).

I understand that this is not guaranteed to be accepted immediately after
this series, so I agree that it is better to keep the "q" suffix for now
in order to avoid confusion.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Ingo Molnar]

* Jürgen Groß <jgross@suse.com> wrote:

> > Well, we had a similar discussion back when we standardized on
> > rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
> > MSR handling APIs. Why have a different pattern in any of the
> > derived APIs? It should really use the same conceptual namespace,
> > not some confusing mixture of two naming schemes.
> 
> In the long run I'd like to do the same conversion for the rdmsr*() and
> wrmsr*() interfaces, too (so only offering and using the 64-bit variants).

Why? We had this discussion for the original MSR API namespace
cleanup a year ago, and decided to standardize on the rdmsrq()/wrmsrq()
namespace:

  c435e608cf59 x86/msr: Rename 'rdmsrl()' to 'rdmsrq()'
  78255eb23973 x86/msr: Rename 'wrmsrl()' to 'wrmsrq()'
  6fe22abacd40 x86/msr: Rename 'rdmsrl_safe()' to 'rdmsrq_safe()'
  6fa17efe4544 x86/msr: Rename 'wrmsrl_safe()' to 'wrmsrq_safe()'
  5e404cb7ac4c x86/msr: Rename 'rdmsrl_safe_on_cpu()' to 'rdmsrq_safe_on_cpu()'
  27a23a544a55 x86/msr: Rename 'wrmsrl_safe_on_cpu()' to 'wrmsrq_safe_on_cpu()'
  d7484babd2c4 x86/msr: Rename 'rdmsrl_on_cpu()' to 'rdmsrq_on_cpu()'
  c895ecdab2e4 x86/msr: Rename 'wrmsrl_on_cpu()' to 'wrmsrq_on_cpu()'
  ebe29309c4d2 x86/msr: Rename 'mce_rdmsrl()' to 'mce_rdmsrq()'
  8e44e83f57c3 x86/msr: Rename 'mce_wrmsrl()' to 'mce_wrmsrq()'
  e2b8af0c6939 x86/msr: Rename 'rdmsrl_amd_safe()' to 'rdmsrq_amd_safe()'
  604d15d15ebd x86/msr: Rename 'wrmsrl_amd_safe()' to 'wrmsrq_amd_safe()'
  7cbc2ba7c107 x86/msr: Rename 'native_wrmsrl()' to 'native_wrmsrq()'
  eef476f15c83 x86/msr: Rename 'wrmsrl_cstar()' to 'wrmsrq_cstar()'

There's several good reasons to use the 'q' suffix in the API names,
why relitigate this? :-)

Thanks,

	Ingo

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Jürgen Groß <jgross@suse.com> wrote:
> 
> > > Well, we had a similar discussion back when we standardized on
> > > rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
> > > MSR handling APIs. Why have a different pattern in any of the
> > > derived APIs? It should really use the same conceptual namespace,
> > > not some confusing mixture of two naming schemes.
> > 
> > In the long run I'd like to do the same conversion for the rdmsr*() and
> > wrmsr*() interfaces, too (so only offering and using the 64-bit variants).
> 
> Why? We had this discussion for the original MSR API namespace
> cleanup a year ago, and decided to standardize on the rdmsrq()/wrmsrq()
> namespace:
> 
>   c435e608cf59 x86/msr: Rename 'rdmsrl()' to 'rdmsrq()'
>   78255eb23973 x86/msr: Rename 'wrmsrl()' to 'wrmsrq()'
>   6fe22abacd40 x86/msr: Rename 'rdmsrl_safe()' to 'rdmsrq_safe()'
>   6fa17efe4544 x86/msr: Rename 'wrmsrl_safe()' to 'wrmsrq_safe()'
>   5e404cb7ac4c x86/msr: Rename 'rdmsrl_safe_on_cpu()' to 'rdmsrq_safe_on_cpu()'
>   27a23a544a55 x86/msr: Rename 'wrmsrl_safe_on_cpu()' to 'wrmsrq_safe_on_cpu()'
>   d7484babd2c4 x86/msr: Rename 'rdmsrl_on_cpu()' to 'rdmsrq_on_cpu()'
>   c895ecdab2e4 x86/msr: Rename 'wrmsrl_on_cpu()' to 'wrmsrq_on_cpu()'
>   ebe29309c4d2 x86/msr: Rename 'mce_rdmsrl()' to 'mce_rdmsrq()'
>   8e44e83f57c3 x86/msr: Rename 'mce_wrmsrl()' to 'mce_wrmsrq()'
>   e2b8af0c6939 x86/msr: Rename 'rdmsrl_amd_safe()' to 'rdmsrq_amd_safe()'
>   604d15d15ebd x86/msr: Rename 'wrmsrl_amd_safe()' to 'wrmsrq_amd_safe()'
>   7cbc2ba7c107 x86/msr: Rename 'native_wrmsrl()' to 'native_wrmsrq()'
>   eef476f15c83 x86/msr: Rename 'wrmsrl_cstar()' to 'wrmsrq_cstar()'
> 
> There's several good reasons to use the 'q' suffix in the API names,
> why relitigate this? :-)

And just to be clear: I have no objections whatsoever to
phasing out all the old 32-bit APIs, like your series does.

Thanks,

	Ingo

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2308 bytes --]

On 05.06.26 11:56, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
>>
>> * Jürgen Groß <jgross@suse.com> wrote:
>>
>>>> Well, we had a similar discussion back when we standardized on
>>>> rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
>>>> MSR handling APIs. Why have a different pattern in any of the
>>>> derived APIs? It should really use the same conceptual namespace,
>>>> not some confusing mixture of two naming schemes.
>>>
>>> In the long run I'd like to do the same conversion for the rdmsr*() and
>>> wrmsr*() interfaces, too (so only offering and using the 64-bit variants).
>>
>> Why? We had this discussion for the original MSR API namespace
>> cleanup a year ago, and decided to standardize on the rdmsrq()/wrmsrq()
>> namespace:
>>
>>    c435e608cf59 x86/msr: Rename 'rdmsrl()' to 'rdmsrq()'
>>    78255eb23973 x86/msr: Rename 'wrmsrl()' to 'wrmsrq()'
>>    6fe22abacd40 x86/msr: Rename 'rdmsrl_safe()' to 'rdmsrq_safe()'
>>    6fa17efe4544 x86/msr: Rename 'wrmsrl_safe()' to 'wrmsrq_safe()'
>>    5e404cb7ac4c x86/msr: Rename 'rdmsrl_safe_on_cpu()' to 'rdmsrq_safe_on_cpu()'
>>    27a23a544a55 x86/msr: Rename 'wrmsrl_safe_on_cpu()' to 'wrmsrq_safe_on_cpu()'
>>    d7484babd2c4 x86/msr: Rename 'rdmsrl_on_cpu()' to 'rdmsrq_on_cpu()'
>>    c895ecdab2e4 x86/msr: Rename 'wrmsrl_on_cpu()' to 'wrmsrq_on_cpu()'
>>    ebe29309c4d2 x86/msr: Rename 'mce_rdmsrl()' to 'mce_rdmsrq()'
>>    8e44e83f57c3 x86/msr: Rename 'mce_wrmsrl()' to 'mce_wrmsrq()'
>>    e2b8af0c6939 x86/msr: Rename 'rdmsrl_amd_safe()' to 'rdmsrq_amd_safe()'
>>    604d15d15ebd x86/msr: Rename 'wrmsrl_amd_safe()' to 'wrmsrq_amd_safe()'
>>    7cbc2ba7c107 x86/msr: Rename 'native_wrmsrl()' to 'native_wrmsrq()'
>>    eef476f15c83 x86/msr: Rename 'wrmsrl_cstar()' to 'wrmsrq_cstar()'
>>
>> There's several good reasons to use the 'q' suffix in the API names,
>> why relitigate this? :-)
> 
> And just to be clear: I have no objections whatsoever to
> phasing out all the old 32-bit APIs, like your series does.

Thanks for the confirmation. :-)

And regarding the "q" suffix: I'm not insisting to drop it, I just felt it would
no longer be needed when the variants without suffix no longer exist. If you
like to keep it, then be it so.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2308 bytes --]

On 05.06.26 11:56, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@kernel.org> wrote:
> 
>>
>> * Jürgen Groß <jgross@suse.com> wrote:
>>
>>>> Well, we had a similar discussion back when we standardized on
>>>> rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
>>>> MSR handling APIs. Why have a different pattern in any of the
>>>> derived APIs? It should really use the same conceptual namespace,
>>>> not some confusing mixture of two naming schemes.
>>>
>>> In the long run I'd like to do the same conversion for the rdmsr*() and
>>> wrmsr*() interfaces, too (so only offering and using the 64-bit variants).
>>
>> Why? We had this discussion for the original MSR API namespace
>> cleanup a year ago, and decided to standardize on the rdmsrq()/wrmsrq()
>> namespace:
>>
>>    c435e608cf59 x86/msr: Rename 'rdmsrl()' to 'rdmsrq()'
>>    78255eb23973 x86/msr: Rename 'wrmsrl()' to 'wrmsrq()'
>>    6fe22abacd40 x86/msr: Rename 'rdmsrl_safe()' to 'rdmsrq_safe()'
>>    6fa17efe4544 x86/msr: Rename 'wrmsrl_safe()' to 'wrmsrq_safe()'
>>    5e404cb7ac4c x86/msr: Rename 'rdmsrl_safe_on_cpu()' to 'rdmsrq_safe_on_cpu()'
>>    27a23a544a55 x86/msr: Rename 'wrmsrl_safe_on_cpu()' to 'wrmsrq_safe_on_cpu()'
>>    d7484babd2c4 x86/msr: Rename 'rdmsrl_on_cpu()' to 'rdmsrq_on_cpu()'
>>    c895ecdab2e4 x86/msr: Rename 'wrmsrl_on_cpu()' to 'wrmsrq_on_cpu()'
>>    ebe29309c4d2 x86/msr: Rename 'mce_rdmsrl()' to 'mce_rdmsrq()'
>>    8e44e83f57c3 x86/msr: Rename 'mce_wrmsrl()' to 'mce_wrmsrq()'
>>    e2b8af0c6939 x86/msr: Rename 'rdmsrl_amd_safe()' to 'rdmsrq_amd_safe()'
>>    604d15d15ebd x86/msr: Rename 'wrmsrl_amd_safe()' to 'wrmsrq_amd_safe()'
>>    7cbc2ba7c107 x86/msr: Rename 'native_wrmsrl()' to 'native_wrmsrq()'
>>    eef476f15c83 x86/msr: Rename 'wrmsrl_cstar()' to 'wrmsrq_cstar()'
>>
>> There's several good reasons to use the 'q' suffix in the API names,
>> why relitigate this? :-)
> 
> And just to be clear: I have no objections whatsoever to
> phasing out all the old 32-bit APIs, like your series does.

Thanks for the confirmation. :-)

And regarding the "q" suffix: I'm not insisting to drop it, I just felt it would
no longer be needed when the variants without suffix no longer exist. If you
like to keep it, then be it so.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]