linux-kernel - Re: [PATCH v2] cpufreq/amd-pstate: Enable ITMT support after initializing core rankings

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] [day] [month] [year] [list]

Message-ID: <e0f66414-e960-41a7-a8d2-06437405a3d5@amd.com>
Date: Fri, 11 Apr 2025 12:25:12 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: K Prateek Nayak <kprateek.nayak@....com>,
 "Gautham R. Shenoy" <gautham.shenoy@....com>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Viresh Kumar <viresh.kumar@...aro.org>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>,
 Huang Rui <ray.huang@....com>, Perry Yuan <perry.yuan@....com>,
 Meng Li <li.meng@....com>
Subject: Re: [PATCH v2] cpufreq/amd-pstate: Enable ITMT support after
 initializing core rankings

On 4/11/2025 3:14 AM, K Prateek Nayak wrote:
> When working on dynamic ITMT priority support, it was observed that
> "asym_prefer_cpu" on AMD systems supporting Preferred Core ranking
> was always set to the first CPU in the sched group when the system boots
> up despite another CPU in the group having a higher ITMT ranking.
> 
> "asym_prefer_cpu" is cached when the sched domain hierarchy is
> constructed. On AMD systems that support Preferred Core rankings, sched
> domains are rebuilt when ITMT support is enabled for the first time from
> amd_pstate*_cpu_init().
> 
> Since amd_pstate*_cpu_init() is called to initialize the cpudata for
> each CPU, the ITMT support is enabled after the first CPU initializes
> its asym priority but this is too early since other CPUs have not yet
> initialized their asym priorities and the sched domain is rebuilt only
> once when the support is toggled on for the first time.
> 
> Initialize the asym priorities first in amd_pstate*_cpu_init() and then
> enable ITMT support later in amd_pstate_register_driver() to ensure all
> CPUs have correctly initialized their asym priorities before sched
> domain hierarchy is rebuilt.
> 
> Clear the ITMT support when the amd-pstate driver unregisters since core
> rankings cannot be trusted unless the update_limits() callback is
> operational.
> 
> Remove the delayed work mechanism now that ITMT support is only toggled
> from the driver init path which is outside the cpuhp critical section.
> 
> Fixes: f3a052391822 ("cpufreq: amd-pstate: Enable amd-pstate preferred core support")
> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>

Reviewed-by: Mario Limonciello <mario.limonciello@....com>

Will queue it up for testing.

> ---
> v1..v2:
> 
> o Retained the name amd_pstate_init_prefcore() (Mario)
> 
> o Moved sched_set_itmt_support() towards the end of
>    amd_pstate_register_driver() to address mode switch scenarios.
> 
> o Disable ITMT support when driver unregisters to prevent incorrect ITMT
>    behavior in absence of update_limits() callback.
> 
> v1: https://lore.kernel.org/lkml/20250409030004.23008-1-kprateek.nayak@amd.com/
> ---
>   drivers/cpufreq/amd-pstate.c | 25 ++++++++-----------------
>   1 file changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index c54c031939c8..b961f3a3b580 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -794,16 +794,6 @@ static void amd_perf_ctl_reset(unsigned int cpu)
>   	wrmsrl_on_cpu(cpu, MSR_AMD_PERF_CTL, 0);
>   }
>   
> -/*
> - * Set amd-pstate preferred core enable can't be done directly from cpufreq callbacks
> - * due to locking, so queue the work for later.
> - */
> -static void amd_pstste_sched_prefcore_workfn(struct work_struct *work)
> -{
> -	sched_set_itmt_support();
> -}
> -static DECLARE_WORK(sched_prefcore_work, amd_pstste_sched_prefcore_workfn);
> -
>   #define CPPC_MAX_PERF	U8_MAX
>   
>   static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata)
> @@ -814,14 +804,8 @@ static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata)
>   
>   	cpudata->hw_prefcore = true;
>   
> -	/*
> -	 * The priorities can be set regardless of whether or not
> -	 * sched_set_itmt_support(true) has been called and it is valid to
> -	 * update them at any time after it has been called.
> -	 */
> +	/* Priorities must be initialized before ITMT support can be toggled on. */
>   	sched_set_itmt_core_prio((int)READ_ONCE(cpudata->prefcore_ranking), cpudata->cpu);
> -
> -	schedule_work(&sched_prefcore_work);
>   }
>   
>   static void amd_pstate_update_limits(unsigned int cpu)
> @@ -1196,6 +1180,9 @@ static ssize_t show_energy_performance_preference(
>   
>   static void amd_pstate_driver_cleanup(void)
>   {
> +	if (amd_pstate_prefcore)
> +		sched_clear_itmt_support();
> +
>   	cppc_state = AMD_PSTATE_DISABLE;
>   	current_pstate_driver = NULL;
>   }
> @@ -1238,6 +1225,10 @@ static int amd_pstate_register_driver(int mode)
>   		return ret;
>   	}
>   
> +	/* Enable ITMT support once all CPUs have initialized their asym priorities. */
> +	if (amd_pstate_prefcore)
> +		sched_set_itmt_support();
> +
>   	return 0;
>   }
>   
> 
> base-commit: 56a49e19e1aea1374e9ba58cfd40260587bb7355