[PATCH 0/4] x86/acpi/cppc: Minor clean up for x86 CPPC implementation

[PATCH 0/4] x86/acpi/cppc: Minor clean up for x86 CPPC implementation

[Huang Rui]

Hi all,

While we were fixing the legacy issue below, we found the dependencies
between smpboot and CPPC were not very good. But due to urgent fix for
5.17-rc1, I didn't have much time to provide a complete solution.

https://lore.kernel.org/lkml/YdeWDDCwBQAYnlKb@amd.com/

In these series, I expand the scope of acpi/cppc_msr to acpi/cppc to cover
the all the CPPC helper functions for x86 ACPI. And then clean up the
smpboot and move CPPC related functions into the acpi/cppc.c. This design
is more straightforward and more clear to handle the CPPC in x86 and
resolve dependency issues between CPPC and smpboot.c.

Thanks,
Ray

Huang Rui (4):
  x86/acpi: Expand the CPPC MSR file to cover the whole CPPC
    implementation
  x86, sched: Move AMD maximum frequency ratio setting function into x86
    CPPC
  x86, sched: Expose init_freq_invariance to topology header
  x86/acpi: Move init_freq_invariance_cppc into x86 CPPC

 arch/x86/include/asm/topology.h |  13 +++-
 arch/x86/kernel/acpi/Makefile   |   2 +-
 arch/x86/kernel/acpi/cppc.c     | 103 ++++++++++++++++++++++++++++++++
 arch/x86/kernel/acpi/cppc_msr.c |  49 ---------------
 arch/x86/kernel/smpboot.c       |  72 +---------------------
 5 files changed, 118 insertions(+), 121 deletions(-)
 create mode 100644 arch/x86/kernel/acpi/cppc.c
 delete mode 100644 arch/x86/kernel/acpi/cppc_msr.c

-- 
2.25.1

[PATCH 1/4] x86/acpi: Expand the CPPC MSR file to cover the whole CPPC implementation

[Huang Rui]

Rename the cppc_msr.c to cppc.c in x86 ACPI, that expects to use this file
to cover more function implementation for ACPI CPPC beside MSR helpers.
Naming as "cppc" is more straightforward as one of the functionalities
under ACPI subsystem.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 arch/x86/kernel/acpi/Makefile               | 2 +-
 arch/x86/kernel/acpi/{cppc_msr.c => cppc.c} | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/kernel/acpi/{cppc_msr.c => cppc.c} (96%)

diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index cf340d85946a..fc17b3f136fe 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -3,7 +3,7 @@
 obj-$(CONFIG_ACPI)		+= boot.o
 obj-$(CONFIG_ACPI_SLEEP)	+= sleep.o wakeup_$(BITS).o
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
-obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_msr.o
+obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc.o
 
 ifneq ($(CONFIG_ACPI_PROCESSOR),)
 obj-y				+= cstate.o
diff --git a/arch/x86/kernel/acpi/cppc_msr.c b/arch/x86/kernel/acpi/cppc.c
similarity index 96%
rename from arch/x86/kernel/acpi/cppc_msr.c
rename to arch/x86/kernel/acpi/cppc.c
index b961de569e7e..08d823d72586 100644
--- a/arch/x86/kernel/acpi/cppc_msr.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * cppc_msr.c:  MSR Interface for CPPC
+ * cppc.c: CPPC Interface for x86
  * Copyright (c) 2016, Intel Corporation.
  */
 
-- 
2.25.1

[PATCH 2/4] x86, sched: Move AMD maximum frequency ratio setting function into x86 CPPC

[Huang Rui]

The AMD maximum frequency ratio setting function depends on CPPC, so the
x86 CPPC implementation file is better space for this function.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 arch/x86/include/asm/topology.h |  9 +++++++
 arch/x86/kernel/acpi/cppc.c     | 40 ++++++++++++++++++++++++++++++
 arch/x86/kernel/smpboot.c       | 44 +--------------------------------
 3 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 2f0b6be8eaab..168ade7d4007 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -226,4 +226,13 @@ void init_freq_invariance_cppc(void);
 #define init_freq_invariance_cppc init_freq_invariance_cppc
 #endif
 
+#ifdef CONFIG_ACPI_CPPC_LIB
+bool amd_set_max_freq_ratio(u64 *ratio);
+#else
+static inline bool amd_set_max_freq_ratio(u64 *ratio)
+{
+	return false;
+}
+#endif
+
 #endif /* _ASM_X86_TOPOLOGY_H */
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
index 08d823d72586..f0be5058e3e3 100644
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -6,6 +6,8 @@
 
 #include <acpi/cppc_acpi.h>
 #include <asm/msr.h>
+#include <asm/processor.h>
+#include <asm/topology.h>
 
 /* Refer to drivers/acpi/cppc_acpi.c for the description of functions */
 
@@ -47,3 +49,41 @@ int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
 	}
 	return err;
 }
+
+bool amd_set_max_freq_ratio(u64 *ratio)
+{
+	struct cppc_perf_caps perf_caps;
+	u64 highest_perf, nominal_perf;
+	u64 perf_ratio;
+	int rc;
+
+	if (!ratio)
+		return false;
+
+	rc = cppc_get_perf_caps(0, &perf_caps);
+	if (rc) {
+		pr_debug("Could not retrieve perf counters (%d)\n", rc);
+		return false;
+	}
+
+	highest_perf = amd_get_highest_perf();
+	nominal_perf = perf_caps.nominal_perf;
+
+	if (!highest_perf || !nominal_perf) {
+		pr_debug("Could not retrieve highest or nominal performance\n");
+		return false;
+	}
+
+	perf_ratio = div_u64(highest_perf * SCHED_CAPACITY_SCALE, nominal_perf);
+	/* midpoint between max_boost and max_P */
+	perf_ratio = (perf_ratio + SCHED_CAPACITY_SCALE) >> 1;
+	if (!perf_ratio) {
+		pr_debug("Non-zero highest/nominal perf values led to a 0 ratio\n");
+		return false;
+	}
+
+	*ratio = perf_ratio;
+	arch_set_max_freq_ratio(false);
+
+	return true;
+}
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 617012f4619f..0718cc7649a4 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -2097,48 +2097,6 @@ static bool intel_set_max_freq_ratio(void)
 	return true;
 }
 
-#ifdef CONFIG_ACPI_CPPC_LIB
-static bool amd_set_max_freq_ratio(void)
-{
-	struct cppc_perf_caps perf_caps;
-	u64 highest_perf, nominal_perf;
-	u64 perf_ratio;
-	int rc;
-
-	rc = cppc_get_perf_caps(0, &perf_caps);
-	if (rc) {
-		pr_debug("Could not retrieve perf counters (%d)\n", rc);
-		return false;
-	}
-
-	highest_perf = amd_get_highest_perf();
-	nominal_perf = perf_caps.nominal_perf;
-
-	if (!highest_perf || !nominal_perf) {
-		pr_debug("Could not retrieve highest or nominal performance\n");
-		return false;
-	}
-
-	perf_ratio = div_u64(highest_perf * SCHED_CAPACITY_SCALE, nominal_perf);
-	/* midpoint between max_boost and max_P */
-	perf_ratio = (perf_ratio + SCHED_CAPACITY_SCALE) >> 1;
-	if (!perf_ratio) {
-		pr_debug("Non-zero highest/nominal perf values led to a 0 ratio\n");
-		return false;
-	}
-
-	arch_turbo_freq_ratio = perf_ratio;
-	arch_set_max_freq_ratio(false);
-
-	return true;
-}
-#else
-static bool amd_set_max_freq_ratio(void)
-{
-	return false;
-}
-#endif
-
 static void init_counter_refs(void)
 {
 	u64 aperf, mperf;
@@ -2187,7 +2145,7 @@ static void init_freq_invariance(bool secondary, bool cppc_ready)
 		if (!cppc_ready) {
 			return;
 		}
-		ret = amd_set_max_freq_ratio();
+		ret = amd_set_max_freq_ratio(&arch_turbo_freq_ratio);
 	}
 
 	if (ret) {
-- 
2.25.1

[PATCH 3/4] x86, sched: Expose init_freq_invariance to topology header

[Huang Rui]

The function init_freq_invariance will be used on x86 CPPC, so expose it in
the topology header.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 arch/x86/include/asm/topology.h | 4 ++++
 arch/x86/kernel/smpboot.c       | 8 +-------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 168ade7d4007..c35005a03768 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -215,10 +215,14 @@ extern void arch_scale_freq_tick(void);
 #define arch_scale_freq_tick arch_scale_freq_tick
 
 extern void arch_set_max_freq_ratio(bool turbo_disabled);
+void init_freq_invariance(bool secondary, bool cppc_ready);
 #else
 static inline void arch_set_max_freq_ratio(bool turbo_disabled)
 {
 }
+static inline void init_freq_invariance(bool secondary, bool cppc_ready)
+{
+}
 #endif
 
 #if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 0718cc7649a4..8f2ff9be0fcc 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -155,8 +155,6 @@ static inline void smpboot_restore_warm_reset_vector(void)
 	*((volatile u32 *)phys_to_virt(TRAMPOLINE_PHYS_LOW)) = 0;
 }
 
-static void init_freq_invariance(bool secondary, bool cppc_ready);
-
 /*
  * Report back to the Boot Processor during boot time or to the caller processor
  * during CPU online.
@@ -2125,7 +2123,7 @@ static void register_freq_invariance_syscore_ops(void)
 static inline void register_freq_invariance_syscore_ops(void) {}
 #endif
 
-static void init_freq_invariance(bool secondary, bool cppc_ready)
+void init_freq_invariance(bool secondary, bool cppc_ready)
 {
 	bool ret = false;
 
@@ -2222,8 +2220,4 @@ void arch_scale_freq_tick(void)
 	pr_warn("Scheduler frequency invariance went wobbly, disabling!\n");
 	schedule_work(&disable_freq_invariance_work);
 }
-#else
-static inline void init_freq_invariance(bool secondary, bool cppc_ready)
-{
-}
 #endif /* CONFIG_X86_64 */
-- 
2.25.1

[PATCH 4/4] x86/acpi: Move init_freq_invariance_cppc into x86 CPPC

[Huang Rui]

The init_freq_invariance_cppc code actually doesn't need the SMP
functionality. So setting the CONFIG_SMP as the check condition for
init_freq_invariance_cppc may cause the confusion to misunderstand the
CPPC. And the x86 CPPC file is better space to store the CPPC related
functions, while the init_freq_invariance_cppc is out of smpboot, that
means, the CONFIG_SMP won't be mandatory condition any more. And It's more
clear than before.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 arch/x86/include/asm/topology.h |  4 +---
 arch/x86/kernel/acpi/cppc.c     | 14 ++++++++++++++
 arch/x86/kernel/smpboot.c       | 20 --------------------
 3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index c35005a03768..9c73d62f7276 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -225,12 +225,10 @@ static inline void init_freq_invariance(bool secondary, bool cppc_ready)
 }
 #endif
 
-#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
+#ifdef CONFIG_ACPI_CPPC_LIB
 void init_freq_invariance_cppc(void);
 #define init_freq_invariance_cppc init_freq_invariance_cppc
-#endif
 
-#ifdef CONFIG_ACPI_CPPC_LIB
 bool amd_set_max_freq_ratio(u64 *ratio);
 #else
 static inline bool amd_set_max_freq_ratio(u64 *ratio)
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
index f0be5058e3e3..df1644d9b3b6 100644
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -87,3 +87,17 @@ bool amd_set_max_freq_ratio(u64 *ratio)
 
 	return true;
 }
+
+static DEFINE_MUTEX(freq_invariance_lock);
+
+void init_freq_invariance_cppc(void)
+{
+	static bool secondary;
+
+	mutex_lock(&freq_invariance_lock);
+
+	init_freq_invariance(secondary, true);
+	secondary = true;
+
+	mutex_unlock(&freq_invariance_lock);
+}
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 8f2ff9be0fcc..2ef14772dc04 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -83,10 +83,6 @@
 #include <asm/hw_irq.h>
 #include <asm/stackprotector.h>
 
-#ifdef CONFIG_ACPI_CPPC_LIB
-#include <acpi/cppc_acpi.h>
-#endif
-
 /* representing HT siblings of each logical CPU */
 DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
 EXPORT_PER_CPU_SYMBOL(cpu_sibling_map);
@@ -2156,22 +2152,6 @@ void init_freq_invariance(bool secondary, bool cppc_ready)
 	}
 }
 
-#ifdef CONFIG_ACPI_CPPC_LIB
-static DEFINE_MUTEX(freq_invariance_lock);
-
-void init_freq_invariance_cppc(void)
-{
-	static bool secondary;
-
-	mutex_lock(&freq_invariance_lock);
-
-	init_freq_invariance(secondary, true);
-	secondary = true;
-
-	mutex_unlock(&freq_invariance_lock);
-}
-#endif
-
 static void disable_freq_invariance_workfn(struct work_struct *work)
 {
 	static_branch_disable(&arch_scale_freq_key);
-- 
2.25.1

Re: [PATCH 0/4] x86/acpi/cppc: Minor clean up for x86 CPPC implementation

[Rafael J. Wysocki]

On Mon, Feb 14, 2022 at 11:51 AM Huang Rui <ray.huang@amd.com> wrote:
>
> Hi all,
>
> While we were fixing the legacy issue below, we found the dependencies
> between smpboot and CPPC were not very good. But due to urgent fix for
> 5.17-rc1, I didn't have much time to provide a complete solution.
>
> https://lore.kernel.org/lkml/YdeWDDCwBQAYnlKb@amd.com/
>
> In these series, I expand the scope of acpi/cppc_msr to acpi/cppc to cover
> the all the CPPC helper functions for x86 ACPI. And then clean up the
> smpboot and move CPPC related functions into the acpi/cppc.c. This design
> is more straightforward and more clear to handle the CPPC in x86 and
> resolve dependency issues between CPPC and smpboot.c.
>
> Thanks,
> Ray
>
> Huang Rui (4):
>   x86/acpi: Expand the CPPC MSR file to cover the whole CPPC
>     implementation
>   x86, sched: Move AMD maximum frequency ratio setting function into x86
>     CPPC
>   x86, sched: Expose init_freq_invariance to topology header
>   x86/acpi: Move init_freq_invariance_cppc into x86 CPPC
>
>  arch/x86/include/asm/topology.h |  13 +++-
>  arch/x86/kernel/acpi/Makefile   |   2 +-
>  arch/x86/kernel/acpi/cppc.c     | 103 ++++++++++++++++++++++++++++++++
>  arch/x86/kernel/acpi/cppc_msr.c |  49 ---------------
>  arch/x86/kernel/smpboot.c       |  72 +---------------------
>  5 files changed, 118 insertions(+), 121 deletions(-)
>  create mode 100644 arch/x86/kernel/acpi/cppc.c
>  delete mode 100644 arch/x86/kernel/acpi/cppc_msr.c
>
> --

This series makes sense to me and I'm inclined to take it if there are
no objections, so if there are any, please let me know.

Re: [PATCH 0/4] x86/acpi/cppc: Minor clean up for x86 CPPC implementation

[Rafael J. Wysocki]

On Tue, Mar 1, 2022 at 8:20 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Feb 14, 2022 at 11:51 AM Huang Rui <ray.huang@amd.com> wrote:
> >
> > Hi all,
> >
> > While we were fixing the legacy issue below, we found the dependencies
> > between smpboot and CPPC were not very good. But due to urgent fix for
> > 5.17-rc1, I didn't have much time to provide a complete solution.
> >
> > https://lore.kernel.org/lkml/YdeWDDCwBQAYnlKb@amd.com/
> >
> > In these series, I expand the scope of acpi/cppc_msr to acpi/cppc to cover
> > the all the CPPC helper functions for x86 ACPI. And then clean up the
> > smpboot and move CPPC related functions into the acpi/cppc.c. This design
> > is more straightforward and more clear to handle the CPPC in x86 and
> > resolve dependency issues between CPPC and smpboot.c.
> >
> > Thanks,
> > Ray
> >
> > Huang Rui (4):
> >   x86/acpi: Expand the CPPC MSR file to cover the whole CPPC
> >     implementation
> >   x86, sched: Move AMD maximum frequency ratio setting function into x86
> >     CPPC
> >   x86, sched: Expose init_freq_invariance to topology header
> >   x86/acpi: Move init_freq_invariance_cppc into x86 CPPC
> >
> >  arch/x86/include/asm/topology.h |  13 +++-
> >  arch/x86/kernel/acpi/Makefile   |   2 +-
> >  arch/x86/kernel/acpi/cppc.c     | 103 ++++++++++++++++++++++++++++++++
> >  arch/x86/kernel/acpi/cppc_msr.c |  49 ---------------
> >  arch/x86/kernel/smpboot.c       |  72 +---------------------
> >  5 files changed, 118 insertions(+), 121 deletions(-)
> >  create mode 100644 arch/x86/kernel/acpi/cppc.c
> >  delete mode 100644 arch/x86/kernel/acpi/cppc_msr.c
> >
> > --
>
> This series makes sense to me and I'm inclined to take it if there are
> no objections, so if there are any, please let me know.

And so applied as 5.18 material now with slightly adjusted subjects.

Thanks!