[PATCH 0/4] x86/fpu: Make AMX state ready for CPU idle

[PATCH 0/4] x86/fpu: Make AMX state ready for CPU idle

[Chang S. Bae]

AMX state is a large state (at least 8KB or more). Entering CPU idle with
this non-initialized large state may result in shallow states while a
deeper low-power state is available, [1].

This series ensures AMX states are initialized before entering CPU idle.

PATCH1 update the opcode table for TILERELEASE. PATCH2 introduces a helper
to make the state ready for CPU idle. But then this function can be located
in two different places:

  (a) Under arch_cpu_idle_enter(), which is called before 
      cpuidle_idle_call() or cpu_idle_poll() [2].
  (b) In drivers/idle/intel_idle.c, call this along with other idle code 
      such as intel_idle() or intel_idle_s2idle(), as per peterz's 
      suggestion [3].
 
The approach (a) may decouple the architecture-specific idle code from the
chip-specific code in the intel_idle driver. (b) tends to put them together
in one place.

Both approaches are included now -- PATCH3 has the approach (a) and so does
PATCH4 for (b). But it shouldn't be an issue to pick one of them
exclusively from this series.

The patchset is based on 30d02551ba4f ("x86/fpu: Optimize out sigframe
xfeatures when in init state") in the tip's x86/urgent branch, as calling
xgetbv(1) via xfeatures_in_use().

Thanks,
Chang

[1]: Intel Architecture Instruction Set Extension Programming Reference
     May 2021, https://software.intel.com/content/dam/develop/external/us/en/documents-tps/architecture-instruction-set-extensions-programming-reference.pdf
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/idle.c
[3]: https://lore.kernel.org/lkml/YK+I8YEEnhWjuVAd@hirez.programming.kicks-ass.net/

Chang S. Bae (4):
  x86/insn/amx: Add TILERELEASE instruction to the opcode map
  x86/fpu: Add a helper to prepare AMX registers for CPU idle
  x86/fpu: Prepare AMX state for CPU idle
  intel_idle/amx: Add SPR support with XSTATE idle

 arch/x86/include/asm/fpu/api.h        |  2 +
 arch/x86/include/asm/special_insns.h  |  9 ++++
 arch/x86/kernel/fpu/core.c            | 21 +++++++++
 arch/x86/kernel/process.c             |  1 +
 arch/x86/lib/x86-opcode-map.txt       |  8 +++-
 drivers/idle/intel_idle.c             | 62 +++++++++++++++++++++++++++
 tools/arch/x86/lib/x86-opcode-map.txt |  8 +++-
 7 files changed, 109 insertions(+), 2 deletions(-)


base-commit: 30d02551ba4f681cfa605cedacf231b8641169f0
-- 
2.17.1

[PATCH 1/4] x86/insn/amx: Add TILERELEASE instruction to the opcode map

[Chang S. Bae]

Include the opcode of TILERELEASE that returns all the AMX states to
INIT-state.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/x86/lib/x86-opcode-map.txt       | 8 +++++++-
 tools/arch/x86/lib/x86-opcode-map.txt | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index ec31f5b60323..dbc5078ccafe 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -690,7 +690,9 @@ AVXcode: 2
 45: vpsrlvd/q Vx,Hx,Wx (66),(v)
 46: vpsravd Vx,Hx,Wx (66),(v) | vpsravd/q Vx,Hx,Wx (66),(evo)
 47: vpsllvd/q Vx,Hx,Wx (66),(v)
-# Skip 0x48-0x4b
+# Skip 0x48
+49: Grp22 (1A)
+# Skip 0x4a-0x4b
 4c: vrcp14ps/d Vpd,Wpd (66),(ev)
 4d: vrcp14ss/d Vsd,Hpd,Wsd (66),(ev)
 4e: vrsqrt14ps/d Vpd,Wpd (66),(ev)
@@ -1082,6 +1084,10 @@ GrpTable: Grp21
 7: ENDBR64 (F3),(010),(11B) | ENDBR32 (F3),(011),(11B)
 EndTable
 
+GrpTable: Grp22
+0: TILERELEASE (!F3),(v1),(11B)
+EndTable
+
 # AMD's Prefetch Group
 GrpTable: GrpP
 0: PREFETCH
diff --git a/tools/arch/x86/lib/x86-opcode-map.txt b/tools/arch/x86/lib/x86-opcode-map.txt
index ec31f5b60323..dbc5078ccafe 100644
--- a/tools/arch/x86/lib/x86-opcode-map.txt
+++ b/tools/arch/x86/lib/x86-opcode-map.txt
@@ -690,7 +690,9 @@ AVXcode: 2
 45: vpsrlvd/q Vx,Hx,Wx (66),(v)
 46: vpsravd Vx,Hx,Wx (66),(v) | vpsravd/q Vx,Hx,Wx (66),(evo)
 47: vpsllvd/q Vx,Hx,Wx (66),(v)
-# Skip 0x48-0x4b
+# Skip 0x48
+49: Grp22 (1A)
+# Skip 0x4a-0x4b
 4c: vrcp14ps/d Vpd,Wpd (66),(ev)
 4d: vrcp14ss/d Vsd,Hpd,Wsd (66),(ev)
 4e: vrsqrt14ps/d Vpd,Wpd (66),(ev)
@@ -1082,6 +1084,10 @@ GrpTable: Grp21
 7: ENDBR64 (F3),(010),(11B) | ENDBR32 (F3),(011),(11B)
 EndTable
 
+GrpTable: Grp22
+0: TILERELEASE (!F3),(v1),(11B)
+EndTable
+
 # AMD's Prefetch Group
 GrpTable: GrpP
 0: PREFETCH
-- 
2.17.1

[PATCH 2/4] x86/fpu: Add a helper to prepare AMX state for CPU idle

[Chang S. Bae]

When a CPU enters an idle state, non-initialized states left in large
registers may be the cause of preventing deeper low-power states.

The new helper ensures the AMX state is initialized to make the CPU
ready for low-power states.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/x86/include/asm/fpu/api.h       |  2 ++
 arch/x86/include/asm/special_insns.h |  9 +++++++++
 arch/x86/kernel/fpu/core.c           | 21 +++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index b7267b9e452f..c4965d2d0bb6 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -162,4 +162,6 @@ static inline bool fpstate_is_confidential(struct fpu_guest *gfpu)
 struct task_struct;
 extern long fpu_xstate_prctl(struct task_struct *tsk, int option, unsigned long arg2);
 
+extern void fpu_idle_fpregs(void);
+
 #endif /* _ASM_X86_FPU_API_H */
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 68c257a3de0d..d434fbaeb3ff 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -294,6 +294,15 @@ static inline int enqcmds(void __iomem *dst, const void *src)
 	return 0;
 }
 
+static inline void tile_release(void)
+{
+	/*
+	 * Instruction opcode for TILERELEASE; supported in binutils
+	 * version >= 2.36.
+	 */
+	asm volatile(".byte 0xc4, 0xe2, 0x78, 0x49, 0xc0");
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_SPECIAL_INSNS_H */
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 8ea306b1bf8e..15d7bd67715f 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -753,3 +753,24 @@ int fpu__exception_code(struct fpu *fpu, int trap_nr)
 	 */
 	return 0;
 }
+
+/*
+ * Leaving state in some large registers may prevent the processor from
+ * entering lower-power idle states. Initialize those states when needed.
+ *
+ * A caller needs to make sure fpregs are saved before this.
+ */
+void fpu_idle_fpregs(void)
+{
+	/*
+	 * Ensure AMX TILE registers in INIT-state before entering the idle
+	 * state.
+	 *
+	 * Dynamic states are enabled only when X86_FEATURE_XGETBV1 is
+	 * available.
+	 */
+	if (fpu_state_size_dynamic() && (xfeatures_in_use() & XFEATURE_MASK_XTILE)) {
+		tile_release();
+		fpregs_deactivate(&current->thread.fpu);
+	}
+}
-- 
2.17.1

[PATCH 3/4] x86/fpu: Prepare AMX state for CPU idle

[Chang S. Bae]

Before entering CPU idle, make sure large registers such as AMX TILE are
initialized. Otherwise, non-initialized register states may prevent a CPU
from entering a deeper low-power state.

Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index eb470be0e5ae..7044ba276064 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -707,6 +707,7 @@ static inline void play_dead(void)
 
 void arch_cpu_idle_enter(void)
 {
+	fpu_idle_fpregs();
 	tsc_verify_tsc_adjust(false);
 	local_touch_nmi();
 }
-- 
2.17.1

[PATCH 4/4] intel_idle: Add SPR support with AMX INIT-state

[Chang S. Bae]

Add a custom Sapphire Rapids C-state table to intel_idle driver. The
parameters in the table are preferred over those supplied by ACPI.

SPR supports AMX, and so this custom table uses idle entry points that know
how to initialize AMX state if necessary.

This guarantees that AMX state will never be the cause of hardware C-state
demotion from C6 to C1E. Under some conditions, this may result in improved
power savings and thus a higher available turbo frequency budget.

[ Based on patch by Artem Bityutskiy <artem.bityutskiy@linux.intel.com>. ]

Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
 drivers/idle/intel_idle.c | 62 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index e6c543b5ee1d..0ac7dc9e6d51 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -54,6 +54,7 @@
 #include <asm/intel-family.h>
 #include <asm/mwait.h>
 #include <asm/msr.h>
+#include <asm/fpu/api.h>
 
 #define INTEL_IDLE_VERSION "0.5.1"
 
@@ -155,6 +156,39 @@ static __cpuidle int intel_idle_s2idle(struct cpuidle_device *dev,
 	return 0;
 }
 
+/**
+ * intel_idle_tile - Ask the processor to enter the given idle state.
+ * @dev: cpuidle device of the target CPU.
+ * @drv: cpuidle driver (assumed to point to intel_idle_driver).
+ *
+ * Ensure TILE registers in INIT-state before using intel_idle() to
+ * enter the idle state.
+ */
+static __cpuidle int intel_idle_tile(struct cpuidle_device *dev,
+				     struct cpuidle_driver *drv, int index)
+{
+	fpu_idle_fpregs();
+
+	return intel_idle(dev, drv, index);
+}
+
+/**
+ * intel_idle_s2idle_tile - Ask the processor to enter the given idle state.
+ * @dev: cpuidle device of the target CPU.
+ * @drv: cpuidle driver (assumed to point to intel_idle_driver).
+ * @index: Target idle state index.
+ *
+ * Ensure TILE registers in INIT-state before using intel_idle_s2idle() to
+ * enter the idle state.
+ */
+static __cpuidle int intel_idle_s2idle_tile(struct cpuidle_device *dev,
+					    struct cpuidle_driver *drv, int index)
+{
+	fpu_idle_fpregs();
+
+	return intel_idle_s2idle(dev, drv, index);
+}
+
 /*
  * States are indexed by the cstate number,
  * which is also the index into the MWAIT hint array.
@@ -752,6 +786,27 @@ static struct cpuidle_state icx_cstates[] __initdata = {
 		.enter = NULL }
 };
 
+static struct cpuidle_state spr_cstates[] __initdata = {
+	{
+		.name = "C1",
+		.desc = "MWAIT 0x00",
+		.flags = MWAIT2flg(0x00),
+		.exit_latency = 1,
+		.target_residency = 1,
+		.enter = &intel_idle,
+		.enter_s2idle = intel_idle_s2idle, },
+	{
+		.name = "C6",
+		.desc = "MWAIT 0x20",
+		.flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED,
+		.exit_latency = 128,
+		.target_residency = 384,
+		.enter = &intel_idle_tile,
+		.enter_s2idle = intel_idle_s2idle_tile, },
+	{
+		.enter = NULL }
+};
+
 static struct cpuidle_state atom_cstates[] __initdata = {
 	{
 		.name = "C1E",
@@ -1095,6 +1150,12 @@ static const struct idle_cpu idle_cpu_icx __initconst = {
 	.use_acpi = true,
 };
 
+static const struct idle_cpu idle_cpu_spr __initconst = {
+	.state_table = spr_cstates,
+	.disable_promotion_to_c1e = true,
+	.use_acpi = true,
+};
+
 static const struct idle_cpu idle_cpu_avn __initconst = {
 	.state_table = avn_cstates,
 	.disable_promotion_to_c1e = true,
@@ -1157,6 +1218,7 @@ static const struct x86_cpu_id intel_idle_ids[] __initconst = {
 	X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X,		&idle_cpu_skx),
 	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X,		&idle_cpu_icx),
 	X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D,		&idle_cpu_icx),
+	X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X,	&idle_cpu_spr),
 	X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL,	&idle_cpu_knl),
 	X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM,	&idle_cpu_knl),
 	X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT,	&idle_cpu_bxt),
-- 
2.17.1

Re: [PATCH 4/4] intel_idle: Add SPR support with AMX INIT-state

[Thomas Gleixner]

Chang,

On Thu, Nov 04 2021 at 15:52, Chang S. Bae wrote:
> +/**
> + * intel_idle_tile - Ask the processor to enter the given idle state.
> + * @dev: cpuidle device of the target CPU.
> + * @drv: cpuidle driver (assumed to point to intel_idle_driver).
> + *
> + * Ensure TILE registers in INIT-state before using intel_idle() to
> + * enter the idle state.
> + */
> +static __cpuidle int intel_idle_tile(struct cpuidle_device *dev,
> +				     struct cpuidle_driver *drv, int index)
> +{
> +	fpu_idle_fpregs();

That's redundant because arch_cpu_idle_enter() is invoked before the
actual idle mechanism. 

> +/**
> + * intel_idle_s2idle_tile - Ask the processor to enter the given idle state.
> + * @dev: cpuidle device of the target CPU.
> + * @drv: cpuidle driver (assumed to point to intel_idle_driver).
> + * @index: Target idle state index.
> + *
> + * Ensure TILE registers in INIT-state before using intel_idle_s2idle() to
> + * enter the idle state.
> + */
> +static __cpuidle int intel_idle_s2idle_tile(struct cpuidle_device *dev,
> +					    struct cpuidle_driver *drv, int index)
> +{
> +	fpu_idle_fpregs();

Ditto

Thanks,

        tglx

Re: [PATCH 4/4] intel_idle: Add SPR support with AMX INIT-state

[Bae, Chang Seok]

On Nov 5, 2021, at 07:33, Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> Chang,
> 
> On Thu, Nov 04 2021 at 15:52, Chang S. Bae wrote:
>> +/**
>> + * intel_idle_tile - Ask the processor to enter the given idle state.
>> + * @dev: cpuidle device of the target CPU.
>> + * @drv: cpuidle driver (assumed to point to intel_idle_driver).
>> + *
>> + * Ensure TILE registers in INIT-state before using intel_idle() to
>> + * enter the idle state.
>> + */
>> +static __cpuidle int intel_idle_tile(struct cpuidle_device *dev,
>> +				     struct cpuidle_driver *drv, int index)
>> +{
>> +	fpu_idle_fpregs();
> 
> That's redundant because arch_cpu_idle_enter() is invoked before the
> actual idle mechanism. 

I think the way this series is shaped makes confusion, sorry.

Since PATCH3 and PATCH4 are in debate -- which approach should be chosen, it
was decided to post both and let just one of them be selected. E.g., if PATCH3
is right, then PATCH4 should be abandoned.

I think PATCH3 is better. Maybe PATCH4 should not be sent together to avoid
such confusion.

Thanks,
Chang

Re: [PATCH 4/4] intel_idle: Add SPR support with AMX INIT-state

[Thomas Gleixner]

On Fri, Nov 05 2021 at 16:03, Bae, Chang Seok wrote:
> On Nov 5, 2021, at 07:33, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Thu, Nov 04 2021 at 15:52, Chang S. Bae wrote:
>>> +static __cpuidle int intel_idle_tile(struct cpuidle_device *dev,
>>> +				     struct cpuidle_driver *drv, int index)
>>> +{
>>> +	fpu_idle_fpregs();
>> 
>> That's redundant because arch_cpu_idle_enter() is invoked before the
>> actual idle mechanism. 
>
> I think the way this series is shaped makes confusion, sorry.
>
> Since PATCH3 and PATCH4 are in debate -- which approach should be chosen, it
> was decided to post both and let just one of them be selected. E.g., if PATCH3
> is right, then PATCH4 should be abandoned.

My bad. I should have read the cover letter before complaining.

> I think PATCH3 is better. Maybe PATCH4 should not be sent together to avoid
> such confusion.

Yes. patch 3 is way better than patch 4.

Thanks,

        tglx

Re: [PATCH 4/4] intel_idle: Add SPR support with AMX INIT-state

[Chang S. Bae]

On 11/5/2021 10:26 AM, Thomas Gleixner wrote:
> On Fri, Nov 05 2021 at 16:03, Bae, Chang Seok wrote:
> 
> My bad. I should have read the cover letter before complaining.
> 
>> I think PATCH3 is better. Maybe PATCH4 should not be sent together to avoid
>> such confusion.
> 
> Yes. patch 3 is way better than patch 4.

At the time of this post, it was clueless whether this TILERELEASE need 
is architectural or implementation-specific. But the latter is the case 
as it turns out. So PATCH4 only appears in V2 here -- 
https://lore.kernel.org/lkml/20220309223431.26560-1-chang.seok.bae@intel.com/

Thanks,
Chang