linux-kernel - Re: [tip: x86/fpu] intel_idle: Add a new flag to initialize the AMX state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YtUimvkv+rDJJ9zu@zn.tnic>
Date:   Mon, 18 Jul 2022 11:06:34 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>
Cc:     linux-tip-commits@...r.kernel.org,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        "Chang S. Bae" <chang.seok.bae@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Zhang Rui <rui.zhang@...el.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [tip: x86/fpu] intel_idle: Add a new flag to initialize the AMX
 state

Hi,

this is conflicting with the retbleed changes which went upstream and
resolving those conflicts would practically mean rewriting your patches.

Can you please redo them ontop of -rc7 and send them again?

This is not something we normally do but retbleed is not something
normal so..

Thanks!

On Tue, Jun 14, 2022 at 10:53:58PM -0000, tip-bot2 for Chang S. Bae wrote:
> The following commit has been merged into the x86/fpu branch of tip:
> 
> Commit-ID:     f08ef9057b7b110f44cd364744ba6b5f0115390f
> Gitweb:        https://git.kernel.org/tip/f08ef9057b7b110f44cd364744ba6b5f0115390f
> Author:        Chang S. Bae <chang.seok.bae@...el.com>
> AuthorDate:    Tue, 14 Jun 2022 09:41:16 -07:00
> Committer:     Dave Hansen <dave.hansen@...ux.intel.com>
> CommitterDate: Tue, 14 Jun 2022 15:48:58 -07:00
> 
> intel_idle: Add a new flag to initialize the AMX state
> 
> The non-initialized AMX state can be the cause of C-state demotion from C6
> to C1E. This low-power idle state may improve power savings and thus result
> in a higher available turbo frequency budget.
> 
> This behavior is implementation-specific. Initialize the state for the C6
> entrance of Sapphire Rapids as needed.
> 
> Suggested-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Signed-off-by: Chang S. Bae <chang.seok.bae@...el.com>
> Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Tested-by: Zhang Rui <rui.zhang@...el.com>
> Link: https://lkml.kernel.org/r/20220608164748.11864-3-chang.seok.bae@intel.com
> Link: https://lkml.kernel.org/r/20220614164116.5196-1-chang.seok.bae@intel.com
> ---
>  drivers/idle/intel_idle.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 424ef47..8a19ba1 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"
>  
> @@ -106,6 +107,11 @@ static unsigned int mwait_substates __initdata;
>  #define CPUIDLE_FLAG_ALWAYS_ENABLE	BIT(15)
>  
>  /*
> + * Initialize large xstate for the C6-state entrance.
> + */
> +#define CPUIDLE_FLAG_INIT_XSTATE	BIT(16)
> +
> +/*
>   * MWAIT takes an 8-bit "hint" in EAX "suggesting"
>   * the C-state (top nibble) and sub-state (bottom nibble)
>   * 0x00 means "MWAIT(C1)", 0x10 means "MWAIT(C2)" etc.
> @@ -159,6 +165,13 @@ static __cpuidle int intel_idle_irq(struct cpuidle_device *dev,
>  	return ret;
>  }
>  
> +static __cpuidle int intel_idle_xstate(struct cpuidle_device *dev,
> +				       struct cpuidle_driver *drv, int index)
> +{
> +	fpu_idle_fpregs();
> +	return __intel_idle(dev, drv, index);
> +}
> +
>  /**
>   * intel_idle_s2idle - Ask the processor to enter the given idle state.
>   * @dev: cpuidle device of the target CPU.
> @@ -174,8 +187,12 @@ static __cpuidle int intel_idle_irq(struct cpuidle_device *dev,
>  static __cpuidle int intel_idle_s2idle(struct cpuidle_device *dev,
>  				       struct cpuidle_driver *drv, int index)
>  {
> -	unsigned long eax = flg2MWAIT(drv->states[index].flags);
>  	unsigned long ecx = 1; /* break on interrupt flag */
> +	struct cpuidle_state *state = &drv->states[index];
> +	unsigned long eax = flg2MWAIT(state->flags);
> +
> +	if (state->flags & CPUIDLE_FLAG_INIT_XSTATE)
> +		fpu_idle_fpregs();
>  
>  	mwait_idle_with_hints(eax, ecx);
>  
> @@ -910,7 +927,8 @@ static struct cpuidle_state spr_cstates[] __initdata = {
>  	{
>  		.name = "C6",
>  		.desc = "MWAIT 0x20",
> -		.flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED |
> +					   CPUIDLE_FLAG_INIT_XSTATE,
>  		.exit_latency = 290,
>  		.target_residency = 800,
>  		.enter = &intel_idle,
> @@ -1819,6 +1837,9 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
>  		if (cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_IRQ_ENABLE)
>  			drv->states[drv->state_count].enter = intel_idle_irq;
>  
> +		if (cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_INIT_XSTATE)
> +			drv->states[drv->state_count].enter = intel_idle_xstate;
> +
>  		if ((disabled_states_mask & BIT(drv->state_count)) ||
>  		    ((icpu->use_acpi || force_use_acpi) &&
>  		     intel_idle_off_by_default(mwait_hint) &&

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette