Re: [PATCH v2 2/9] cpufreq/amd-pstate: Remove the defensive check for bios_min_perf

[Mario Limonciello <superm1@kernel.org>]

On 7/23/26 15:12, K Prateek Nayak wrote:
> Initialization of bios_min_perf (BIOS Requested CPU Min Freq.) only
> succeeds when the driver init finds the CPPC_REQ MSRs to have all 0s
> except for MIN_PERF bits.
> 
> A kexec puts the driver through the suspend path which, although resets
> the min_perf back to bios_min_perf, keeps the rest of the CPPR_REQ
> intact with the last value at the time of suspend.
> 
> The defensive check for bios_min_perf exists to prevent the min perf
> from last CPPC_REQ being incorrectly considered as bios_min_perf when
> a kexec switches from an older kernel running the version of driver
> which is not aware of bios_min_perf to a newer one.
> 
> This scenario is extremely unlikely and Mario suggested it is better to
> simplify the initialization rather than complicating the suspend resume
> paths.
> 
> Drop the defensive check for bios_min_perf initialization and add a
> debug message to dump the BIOS Requested Min Freq. to console leaving
> enough breadcrumbs for debug if a situation so arises.
> 
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
> Changelog rfc v1..v2:
> 
> o Reworked based on review comments from Mario.
> ---
>   drivers/cpufreq/amd-pstate.c | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1157f05df10f..02308e95636e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -462,7 +462,6 @@ static int msr_init_perf(struct amd_cpudata *cpudata)
>   {
>   	union perf_cached perf = READ_ONCE(cpudata->perf);
>   	u64 cap1, numerator, cppc_req;
> -	u8 min_perf;
>   
>   	int ret = rdmsrq_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
>   				     &cap1);
> @@ -478,16 +477,6 @@ static int msr_init_perf(struct amd_cpudata *cpudata)
>   		return ret;
>   
>   	WRITE_ONCE(cpudata->cppc_req_cached, cppc_req);
> -	min_perf = FIELD_GET(AMD_CPPC_MIN_PERF_MASK, cppc_req);
> -
> -	/*
> -	 * Clear out the min_perf part to check if the rest of the MSR is 0, if yes, this is an
> -	 * indication that the min_perf value is the one specified through the BIOS option
> -	 */
> -	cppc_req &= ~(AMD_CPPC_MIN_PERF_MASK);
> -
> -	if (!cppc_req)
> -		perf.bios_min_perf = min_perf;
>   
>   	perf.highest_perf = numerator;
>   	perf.max_limit_perf = numerator;
> @@ -495,6 +484,7 @@ static int msr_init_perf(struct amd_cpudata *cpudata)
>   	perf.nominal_perf = FIELD_GET(AMD_CPPC_NOMINAL_PERF_MASK, cap1);
>   	perf.lowest_nonlinear_perf = FIELD_GET(AMD_CPPC_LOWNONLIN_PERF_MASK, cap1);
>   	perf.lowest_perf = FIELD_GET(AMD_CPPC_LOWEST_PERF_MASK, cap1);
> +	perf.bios_min_perf = FIELD_GET(AMD_CPPC_MIN_PERF_MASK, cppc_req);
>   	WRITE_ONCE(cpudata->perf, perf);
>   	WRITE_ONCE(cpudata->prefcore_ranking, FIELD_GET(AMD_CPPC_HIGHEST_PERF_MASK, cap1));
>   	WRITE_ONCE(cpudata->floor_perf_cnt, FIELD_GET(AMD_CPPC_FLOOR_PERF_CNT_MASK, cap1));
> @@ -1043,6 +1033,13 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
>   		return -EINVAL;
>   	}
>   
> +	if (perf.bios_min_perf) {
> +		u32 bios_min_freq = perf_to_freq(perf, cpudata->nominal_freq, perf.bios_min_perf);
> +
> +		pr_debug("Found Requested CPU Min Frequency of %uMHz on CPU%d\n",
> +			 bios_min_freq, cpudata->cpu);
> +	}
> +
>   	return 0;
>   }
>