[PATCH 1/2] x86/cpu: introduce x86_get_freq

[PATCH 1/2] x86/cpu: introduce x86_get_freq

[zhenwei pi]

Wrapper function x86_get_freq to get freq on a x86 platform, hide
detailed implementation for proc routine.
Also export this function for the further use, a typical case is that
kvm module gets the freq of the host and tell the guest side by
kvmclock.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 arch/x86/include/asm/processor.h |  2 ++
 arch/x86/kernel/cpu/common.c     | 18 ++++++++++++++++++
 arch/x86/kernel/cpu/proc.c       | 13 +++----------
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 355d38c0cf60..753ab64b7bf3 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -855,4 +855,6 @@ enum mds_mitigations {
 	MDS_MITIGATION_VMWERV,
 };
 
+unsigned int x86_get_freq(unsigned int cpu);
+
 #endif /* _ASM_X86_PROCESSOR_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0083464de5e3..fc6c8ba7a777 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -22,6 +22,7 @@
 #include <linux/io.h>
 #include <linux/syscore_ops.h>
 #include <linux/pgtable.h>
+#include <linux/cpufreq.h>
 
 #include <asm/cmdline.h>
 #include <asm/stackprotector.h>
@@ -2104,3 +2105,20 @@ void arch_smt_update(void)
 	/* Check whether IPI broadcasting can be enabled */
 	apic_smt_update();
 }
+
+unsigned int x86_get_freq(unsigned int cpu)
+{
+	unsigned int freq = 0;
+
+	if (cpu_has(&cpu_data(cpu), X86_FEATURE_TSC)) {
+		freq = aperfmperf_get_khz(cpu);
+		if (!freq)
+			freq = cpufreq_quick_get(cpu);
+
+		if (!freq)
+			freq = cpu_khz;
+	}
+
+	return freq;
+}
+EXPORT_SYMBOL_GPL(x86_get_freq);
diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index 4eec8889b0ff..5384b3a2258e 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -3,7 +3,6 @@
 #include <linux/timex.h>
 #include <linux/string.h>
 #include <linux/seq_file.h>
-#include <linux/cpufreq.h>
 
 #include "cpu.h"
 
@@ -61,7 +60,7 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
 static int show_cpuinfo(struct seq_file *m, void *v)
 {
 	struct cpuinfo_x86 *c = v;
-	unsigned int cpu;
+	unsigned int cpu, freq;
 	int i;
 
 	cpu = c->cpu_index;
@@ -83,16 +82,10 @@ static int show_cpuinfo(struct seq_file *m, void *v)
 	if (c->microcode)
 		seq_printf(m, "microcode\t: 0x%x\n", c->microcode);
 
-	if (cpu_has(c, X86_FEATURE_TSC)) {
-		unsigned int freq = aperfmperf_get_khz(cpu);
-
-		if (!freq)
-			freq = cpufreq_quick_get(cpu);
-		if (!freq)
-			freq = cpu_khz;
+	freq = x86_get_freq(cpu);
+	if (freq)
 		seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
 			   freq / 1000, (freq % 1000));
-	}
 
 	/* Cache size */
 	if (c->x86_cache_size)
-- 
2.25.1

[PATCH 2/2] KVM: x86: use x86_get_freq to get freq for kvmclock

[zhenwei pi]

If the host side supports APERF&MPERF feature, the guest side may get
mismatched freq.

KVM uses x86_get_freq to get the same freq for guest side.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 arch/x86/kvm/x86.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5a403d92833f..0c2a2188700f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8305,10 +8305,8 @@ static void tsc_khz_changed(void *data)
 
 	if (data)
 		khz = freq->new;
-	else if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
-		khz = cpufreq_quick_get(raw_smp_processor_id());
 	if (!khz)
-		khz = tsc_khz;
+		khz = x86_get_freq(raw_smp_processor_id());
 	__this_cpu_write(cpu_tsc_khz, khz);
 }
 
-- 
2.25.1

Re: [PATCH 1/2] x86/cpu: introduce x86_get_freq

[Thomas Gleixner]

On Mon, Nov 29 2021 at 13:20, zhenwei pi wrote:

> Wrapper function x86_get_freq to get freq on a x86 platform, hide
> detailed implementation for proc routine.

Please rename this to x86_get_cpufreq_khz() simply because
x86_get_freq() is way too unspecific.

Please mention function names with 'function()' which makes it obvious
what it is. Here and in the Subject.

Also spell out frequency all over the place in the change log. There is
no reason for using abbreviations in text.

> +
> +unsigned int x86_get_freq(unsigned int cpu)
> +{
> +	unsigned int freq = 0;
> +
> +	if (cpu_has(&cpu_data(cpu), X86_FEATURE_TSC)) {

Please use cpu_feature_enabled(X86....) and make this condition
negated:

     if (!cpu_feature_enabled(X86_FEATURE_TSC))
            return 0;

which spares an indentation level and the 0 initialization of the variable.

Thanks,

        tglx