Message-ID: <Yomfdmc+bRM0E/ZW@amd.com>
Date:   Sun, 22 May 2022 10:27:02 +0800
From:   Huang Rui <ray.huang@....com>
To:     "Meng, Li (Jassmine)" <Li.Meng@....com>
Cc:     Shuah Khan <skhan@...uxfoundation.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        "Fontenot, Nathan" <Nathan.Fontenot@....com>,
        "Sharma, Deepak" <Deepak.Sharma@....com>,
        "Deucher, Alexander" <Alexander.Deucher@....com>,
        "Limonciello, Mario" <Mario.Limonciello@....com>,
        "Su, Jinzhou (Joe)" <Jinzhou.Su@....com>,
        "Yuan, Perry" <Perry.Yuan@....com>,
        "Du, Xiaojian" <Xiaojian.Du@....com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Borislav Petkov <bp@...en8.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V6 2/4] cpufreq: amd-pstate: Add test module for
 amd-pstate driver

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@....com>
> Acked-by: Huang Rui <ray.huang@....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@....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@....com>");
> +MODULE_DESCRIPTION("Unit test for AMD P-state driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 

Powered by blists - more mailing lists