From: Huang Rui <ray.huang@amd.com>
To: "Meng, Li (Jassmine)" <Li.Meng@amd.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
"Fontenot, Nathan" <Nathan.Fontenot@amd.com>,
"Sharma, Deepak" <Deepak.Sharma@amd.com>,
"Deucher, Alexander" <Alexander.Deucher@amd.com>,
"Limonciello, Mario" <Mario.Limonciello@amd.com>,
"Su, Jinzhou (Joe)" <Jinzhou.Su@amd.com>,
"Yuan, Perry" <Perry.Yuan@amd.com>,
"Du, Xiaojian" <Xiaojian.Du@amd.com>,
Viresh Kumar <viresh.kumar@linaro.org>,
Borislav Petkov <bp@alien8.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V6 2/4] cpufreq: amd-pstate: Add test module for amd-pstate driver
Date: Sun, 22 May 2022 10:27:02 +0800 [thread overview]
Message-ID: <Yomfdmc+bRM0E/ZW@amd.com> (raw)
In-Reply-To: <20220519134737.359290-3-li.meng@amd.com>
On Thu, May 19, 2022 at 09:47:35PM +0800, Meng, Li (Jassmine) wrote:
> Add amd-pstate-ut module, which is conceptually out-of-tree module
> and provides ways for selftests/amd-pstate driver to test various
> kernel module-related functionality. This module will be expected by
> some of selftests to be present and loaded.
>
> Signed-off-by: Meng Li <li.meng@amd.com>
> Acked-by: Huang Rui <ray.huang@amd.com>
> ---
> drivers/cpufreq/Kconfig.x86 | 6 +
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/amd-pstate-ut.c | 278 ++++++++++++++++++++++++++++++++
> 3 files changed, 285 insertions(+)
> create mode 100644 drivers/cpufreq/amd-pstate-ut.c
>
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 55516043b656..37ba282cd156 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -51,6 +51,12 @@ config X86_AMD_PSTATE
>
> If in doubt, say N.
>
> +config X86_AMD_PSTATE_UT
> + tristate "selftest for AMD Processor P-State driver"
> + depends on X86_AMD_PSTATE
I think we won't set X86_AMD_PSTATE as the depends of this module, because
even if the acpi-cpufreq is loaded, this module can test it out and tell
user the result.
ACPI and ACPI_PROCESSOR should the dependencies.
Thanks,
Ray
> + help
> + This kernel module is used for testing. It's safe to say M here.
> +
> config X86_ACPI_CPUFREQ
> tristate "ACPI Processor P-States driver"
> depends on ACPI_PROCESSOR
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 285de70af877..49b98c62c5af 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -30,6 +30,7 @@ amd_pstate-y := amd-pstate.o amd-pstate-trace.o
>
> obj-$(CONFIG_X86_ACPI_CPUFREQ) += acpi-cpufreq.o
> obj-$(CONFIG_X86_AMD_PSTATE) += amd_pstate.o
> +obj-$(CONFIG_X86_AMD_PSTATE_UT) += amd-pstate-ut.o
> obj-$(CONFIG_X86_POWERNOW_K8) += powernow-k8.o
> obj-$(CONFIG_X86_PCC_CPUFREQ) += pcc-cpufreq.o
> obj-$(CONFIG_X86_POWERNOW_K6) += powernow-k6.o
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> new file mode 100644
> index 000000000000..a510355b804e
> --- /dev/null
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-1.0-or-later
> +/*
> + * AMD Processor P-state Frequency Driver Unit Test
> + *
> + * Copyright (C) 2022 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + * Author: Meng Li <li.meng@amd.com>
> + *
> + * The AMD P-State Unit Test is a test module for testing the amd-pstate
> + * driver. 1) It can help all users to verify their processor support
> + * (SBIOS/Firmware or Hardware). 2) Kernel can have a basic function
> + * test to avoid the kernel regression during the update. 3) We can
> + * introduce more functional or performance tests to align the result
> + * together, it will benefit power and performance scale optimization.
> + *
> + * At present, it only implements the basic framework and some simple
> + * test cases. Next, 1) we will add a rst document. 2) we will add more
> + * test cases to improve the depth and coverage of the test.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include "../tools/testing/selftests/kselftest_module.h"
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/fs.h>
> +#include <linux/amd-pstate.h>
> +
> +#include <acpi/cppc_acpi.h>
> +
> +/*
> + * Abbreviations:
> + * aput: used as a shortform for AMD P-State unit test.
> + * It helps to keep variable names smaller, simpler
> + */
> +
> +KSTM_MODULE_GLOBALS();
> +
> +/*
> + * Kernel module for testing the AMD P-State unit test
> + */
> +enum aput_result {
> + APUT_RESULT_PASS,
> + APUT_RESULT_FAIL,
> + MAX_APUT_RESULT,
> +};
> +
> +struct aput_struct {
> + const char *name;
> + void (*func)(u32 index);
> + enum aput_result result;
> +};
> +
> +static void aput_acpi_cpc(u32 index);
> +static void aput_check_enabled(u32 index);
> +static void aput_check_perf(u32 index);
> +static void aput_check_freq(u32 index);
> +
> +static struct aput_struct aput_cases[] = {
> + {"acpi_cpc_valid", aput_acpi_cpc },
> + {"check_enabled", aput_check_enabled },
> + {"check_perf", aput_check_perf },
> + {"check_freq", aput_check_freq }
> +};
> +
> +static bool get_shared_mem(void)
> +{
> + bool result = false;
> + char buf[5] = {0};
> + struct file *filp = NULL;
> + loff_t pos = 0;
> + ssize_t ret;
> +
> + if (!boot_cpu_has(X86_FEATURE_CPPC)) {
> + filp = filp_open("/sys/module/amd_pstate/parameters/shared_mem", FMODE_PREAD, 0);
> + if (IS_ERR(filp))
> + pr_err("%s Open param file fail!\n", __func__);
> + else {
> + ret = kernel_read(filp, &buf, sizeof(buf), &pos);
> + if (ret < 0)
> + pr_err("%s ret=%ld unable to read from param file!\n",
> + __func__, ret);
> + filp_close(filp, NULL);
> + }
> +
> + if ('Y' == *buf)
> + result = true;
> + }
> +
> + return result;
> +}
> +
> +static void aput_acpi_cpc(u32 index)
> +{
> + if (acpi_cpc_valid())
> + aput_cases[index].result = APUT_RESULT_PASS;
> + else
> + aput_cases[index].result = APUT_RESULT_FAIL;
> +}
> +
> +static void aput_pstate_enable(u32 index)
> +{
> + int ret = 0;
> + u64 cppc_enable = 0;
> +
> + ret = rdmsrl_safe(MSR_AMD_CPPC_ENABLE, &cppc_enable);
> + if (ret) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s rdmsrl_safe MSR_AMD_CPPC_ENABLE ret=%d is error!\n", __func__, ret);
> + return;
> + }
> + if (cppc_enable)
> + aput_cases[index].result = APUT_RESULT_PASS;
> + else
> + aput_cases[index].result = APUT_RESULT_FAIL;
> +}
> +
> +/*
> + *Check if enabled amd pstate
> + */
> +static void aput_check_enabled(u32 index)
> +{
> + if (get_shared_mem())
> + aput_cases[index].result = APUT_RESULT_PASS;
> + else
> + aput_pstate_enable(index);
> +}
> +
> +/*
> + * Check if the each performance values are reasonable.
> + * highest_perf >= nominal_perf > lowest_nonlinear_perf > lowest_perf > 0
> + */
> +static void aput_check_perf(u32 index)
> +{
> + int cpu = 0, ret = 0;
> + u32 highest_perf = 0, nominal_perf = 0, lowest_nonlinear_perf = 0, lowest_perf = 0;
> + u64 cap1 = 0;
> + struct cppc_perf_caps cppc_perf;
> + struct cpufreq_policy *policy = NULL;
> + struct amd_cpudata *cpudata = NULL;
> +
> + highest_perf = amd_get_highest_perf();
> +
> + for_each_possible_cpu(cpu) {
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy)
> + break;
> + cpudata = policy->driver_data;
> +
> + if (get_shared_mem()) {
> + ret = cppc_get_perf_caps(cpu, &cppc_perf);
> + if (ret) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cppc_get_perf_caps ret=%d is error!\n", __func__, ret);
> + return;
> + }
> +
> + nominal_perf = cppc_perf.nominal_perf;
> + lowest_nonlinear_perf = cppc_perf.lowest_nonlinear_perf;
> + lowest_perf = cppc_perf.lowest_perf;
> + } else {
> + ret = rdmsrl_safe_on_cpu(cpu, MSR_AMD_CPPC_CAP1, &cap1);
> + if (ret) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s read CPPC_CAP1 ret=%d is error!\n", __func__, ret);
> + return;
> + }
> +
> + nominal_perf = AMD_CPPC_NOMINAL_PERF(cap1);
> + lowest_nonlinear_perf = AMD_CPPC_LOWNONLIN_PERF(cap1);
> + lowest_perf = AMD_CPPC_LOWEST_PERF(cap1);
> + }
> +
> + if ((highest_perf != READ_ONCE(cpudata->highest_perf)) ||
> + (nominal_perf != READ_ONCE(cpudata->nominal_perf)) ||
> + (lowest_nonlinear_perf != READ_ONCE(cpudata->lowest_nonlinear_perf)) ||
> + (lowest_perf != READ_ONCE(cpudata->lowest_perf))) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cpu%d highest=%d %d nominal=%d %d lowest_nonlinear=%d %d lowest=%d %d are not equal!\n",
> + __func__, cpu, highest_perf, cpudata->highest_perf,
> + nominal_perf, cpudata->nominal_perf,
> + lowest_nonlinear_perf, cpudata->lowest_nonlinear_perf,
> + lowest_perf, cpudata->lowest_perf);
> + return;
> + }
> +
> + if (!((highest_perf >= nominal_perf) &&
> + (nominal_perf > lowest_nonlinear_perf) &&
> + (lowest_nonlinear_perf > lowest_perf) &&
> + (lowest_perf > 0))) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cpu%d highest=%d nominal=%d lowest_nonlinear=%d lowest=%d have error!\n",
> + __func__, cpu, highest_perf, nominal_perf,
> + lowest_nonlinear_perf, lowest_perf);
> + return;
> + }
> + }
> +
> + aput_cases[index].result = APUT_RESULT_PASS;
> +}
> +
> +/*
> + * Check if the each frequency values are reasonable.
> + * max_freq >= nominal_freq > lowest_nonlinear_freq > min_freq > 0
> + * check max freq when set support boost mode.
> + */
> +static void aput_check_freq(u32 index)
> +{
> + int cpu = 0;
> + struct cpufreq_policy *policy = NULL;
> + struct amd_cpudata *cpudata = NULL;
> +
> + for_each_possible_cpu(cpu) {
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy)
> + break;
> + cpudata = policy->driver_data;
> +
> + if (!((cpudata->max_freq >= cpudata->nominal_freq) &&
> + (cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
> + (cpudata->lowest_nonlinear_freq > cpudata->min_freq) &&
> + (cpudata->min_freq > 0))) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cpu%d max=%d nominal=%d lowest_nonlinear=%d min=%d have error!\n",
> + __func__, cpu, cpudata->max_freq, cpudata->nominal_freq,
> + cpudata->lowest_nonlinear_freq, cpudata->min_freq);
> + return;
> + }
> +
> + if (cpudata->min_freq != policy->min) {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cpu%d cpudata_min_freq=%d policy_min=%d have error!\n",
> + __func__, cpu, cpudata->min_freq, policy->min);
> + return;
> + }
> +
> + if (cpudata->boost_supported) {
> + if ((policy->max == cpudata->max_freq) ||
> + (policy->max == cpudata->nominal_freq))
> + aput_cases[index].result = APUT_RESULT_PASS;
> + else {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cpu%d policy_max=%d cpu_max=%d cpu_nominal=%d have error!\n",
> + __func__, cpu, policy->max, cpudata->max_freq,
> + cpudata->nominal_freq);
> + return;
> + }
> + } else {
> + aput_cases[index].result = APUT_RESULT_FAIL;
> + pr_err("%s cpu%d not support boost!\n", __func__, cpu);
> + return;
> + }
> + }
> +}
> +
> +static void aput_do_test_case(void)
> +{
> + u32 i = 0, arr_size = ARRAY_SIZE(aput_cases);
> +
> + for (i = 0; i < arr_size; i++) {
> + pr_info("****** Begin %-5d\t %-20s\t ******\n", i+1, aput_cases[i].name);
> + aput_cases[i].func(i);
> + KSTM_CHECK_ZERO(aput_cases[i].result);
> + pr_info("****** End %-5d\t %-20s\t ******\n", i+1, aput_cases[i].name);
> + }
> +}
> +
> +static void __init selftest(void)
> +{
> + aput_do_test_case();
> +}
> +
> +KSTM_MODULE_LOADERS(amd_pstate_ut);
> +MODULE_AUTHOR("Meng Li <li.meng@amd.com>");
> +MODULE_DESCRIPTION("Unit test for AMD P-state driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-05-22 2:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-19 13:47 [PATCH V6 0/4] Add unit test module for AMD P-State driver Meng Li
2022-05-19 13:47 ` [PATCH V6 1/4] cpufreq: amd-pstate: Expose struct amd_cpudata Meng Li
2022-05-19 13:47 ` [PATCH V6 2/4] cpufreq: amd-pstate: Add test module for amd-pstate driver Meng Li
2022-05-19 19:21 ` Shuah Khan
2022-05-20 0:20 ` kernel test robot
2022-05-22 2:27 ` Huang Rui [this message]
2022-05-19 13:47 ` [PATCH V6 3/4] selftests: amd-pstate: Add test trigger " Meng Li
2022-05-19 19:50 ` Shuah Khan
2022-05-19 13:47 ` [PATCH V6 4/4] Documentation: amd-pstate: Add unit test introduction Meng Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yomfdmc+bRM0E/ZW@amd.com \
--to=ray.huang@amd.com \
--cc=Alexander.Deucher@amd.com \
--cc=Deepak.Sharma@amd.com \
--cc=Jinzhou.Su@amd.com \
--cc=Li.Meng@amd.com \
--cc=Mario.Limonciello@amd.com \
--cc=Nathan.Fontenot@amd.com \
--cc=Perry.Yuan@amd.com \
--cc=Xiaojian.Du@amd.com \
--cc=bp@alien8.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael.j.wysocki@intel.com \
--cc=skhan@linuxfoundation.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.