[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <819f9f59-6d80-40c9-a7a7-1b8485fa45f2@amd.com>
Date: Mon, 24 Mar 2025 16:55:34 +0530
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
To: Mario Limonciello <superm1@...nel.org>,
"Gautham R . Shenoy" <gautham.shenoy@....com>,
Perry Yuan <perry.yuan@....com>
Cc: "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<linux-kernel@...r.kernel.org>,
"open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>,
Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH v4 5/5] cpufreq/amd-pstate-ut: Add a unit test for raw EPP
On 3/21/2025 7:58 AM, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@....com>
>
> Ensure that all supported raw EPP values work properly.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> drivers/cpufreq/amd-pstate-ut.c | 58 +++++++++++++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index e671bc7d15508..d0c5c0aa3cc94 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -26,6 +26,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> +#include <linux/mm.h>
> #include <linux/fs.h>
> #include <linux/cleanup.h>
>
> @@ -33,6 +34,7 @@
>
> #include "amd-pstate.h"
>
> +DEFINE_FREE(free_page, void *, if (_T) free_page((unsigned long)_T))
>
> struct amd_pstate_ut_struct {
> const char *name;
> @@ -46,6 +48,7 @@ static int amd_pstate_ut_acpi_cpc_valid(u32 index);
> static int amd_pstate_ut_check_enabled(u32 index);
> static int amd_pstate_ut_check_perf(u32 index);
> static int amd_pstate_ut_check_freq(u32 index);
> +static int amd_pstate_ut_epp(u32 index);
> static int amd_pstate_ut_check_driver(u32 index);
>
> static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
> @@ -53,6 +56,7 @@ static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
> {"amd_pstate_ut_check_enabled", amd_pstate_ut_check_enabled },
> {"amd_pstate_ut_check_perf", amd_pstate_ut_check_perf },
> {"amd_pstate_ut_check_freq", amd_pstate_ut_check_freq },
> + {"amd_pstate_ut_epp", amd_pstate_ut_epp },
> {"amd_pstate_ut_check_driver", amd_pstate_ut_check_driver }
> };
>
> @@ -239,6 +243,60 @@ static int amd_pstate_set_mode(enum amd_pstate_mode mode)
> return amd_pstate_update_status(mode_str, strlen(mode_str));
> }
>
> +static int amd_pstate_ut_epp(u32 index)
> +{
> + struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
> + void *buf __free(free_page) = NULL;
> + struct amd_cpudata *cpudata;
> + int ret, cpu = 0;
> + u16 epp;
> +
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy)
> + return -ENODEV;
> +
> + cpudata = policy->driver_data;
> +
> + /* disable dynamic EPP before running test */
> + if (cpudata->dynamic_epp) {
> + pr_debug("Dynamic EPP is enabled, disabling it\n");
> + amd_pstate_clear_dynamic_epp(policy);
> + }
> +
> + buf = (void *)__get_free_page(GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = amd_pstate_set_mode(AMD_PSTATE_ACTIVE);
This causes a deadlock, as we are holding cpufreq_policy reference and calling set_mode(), which
needs to unregister the cpufreq subsystem (i.e. needs all cpufreq_policy references freed).
We might need to drop and retake the reference around set_mode(). Also make cpudata variable NULL
to avoid anyone using a stale cpudata pointer later on by mistake.
OR
Directly call set_epp() instead of store_energy_performance_preference() and eliminate the need
of policy ref after set_mode().
> + if (ret)
> + return ret;
> +
> + for (epp = 0; epp <= U8_MAX; epp++) {
> + u8 val;
> +
> + /* write all EPP values */
> + memset(buf, 0, sizeof(*buf));
> + snprintf(buf, PAGE_SIZE, "%d", epp);
> + ret = store_energy_performance_preference(policy, buf, sizeof(*buf));
> + if (ret < 0)
> + return ret;
> +
> + /* check if the EPP value reads back correctly for raw numbers */
> + memset(buf, 0, sizeof(*buf));
> + ret = show_energy_performance_preference(policy, buf);
> + if (ret < 0)
> + return ret;
> + strreplace(buf, '\n', '\0');
> + ret = kstrtou8(buf, 0, &val);
> + if (!ret && epp != val) {
> + pr_err("Raw EPP value mismatch: %d != %d\n", epp, val);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int amd_pstate_ut_check_driver(u32 index)
> {
> enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE;
Powered by blists - more mailing lists