All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add unit test module for AMD P-State driver
@ 2022-03-23  7:14 Meng Li
  2022-03-23  7:15 ` [PATCH 1/3] cpufreq: amd-pstate: Expose struct amd_cpudata Meng Li
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Meng Li @ 2022-03-23  7:14 UTC (permalink / raw)
  To: Shuah Khan, Rafael J . Wysocki, Huang Rui, linux-pm
  Cc: Nathan Fontenot, Deepak Sharma, Alex Deucher, Mario Limonciello,
	Jinzhou Su, Perry Yuan, Xiaojian Du, Viresh Kumar,
	Borislav Petkov, linux-kernel, Meng Li

Hi all:

AMD P-State unit test(amd-pstate-ut) is a kernel module for testing the
functions of amd-pstate.
It could import as a module to launch some test tasks.

We upstream out AMD P-state driver into Linux kernel and use this unit
test module to verify the required conditions and basic functions of
amd-pstate before integration test.

When you test all the test cases, you will get the following test results.
The status "P" is pass, "F" is fail.
jasmine@jasmine:/sys/module/amd_pstate_ut/parameters$ cat
unit_test
Index    Test cases              Status
0        stop                    [ ]
1        all                     [P]
2        x86_vendor              [P]
3        acpi_cpc_valid          [P]
4        modprobed_driver        [P]
5        capability_check        [P]
6        enable                  [P]
7        init_perf               [P]
8        support_boost           [P]
9        clear_status            [ ]
------------------------------------------
begin_index = 1 end_index= 8

For exmaple: The test case acpi_cpc_valid is used to check whether the
_CPC object is exist in SBIOS.
The amd-pstate initialization will fail if the _CPC in ACPI SBIOS is
not existed at the detected processor, so it is a necessary condition.

At present, its test cases are very simple, and the corresponding test
cases will continue to be added later to improve the test coverage.

Thanks,
Jasmine

Meng Li (3):
  cpufreq: amd-pstate: Expose struct amd_cpudata
  cpupower: Introduce a new unit test module for AMD P-State driver
  Documentation: amd-pstate: Add unit test introduction

 Documentation/admin-guide/pm/amd-pstate.rst   | 221 +++++++
 drivers/cpufreq/amd-pstate.c                  |  60 +-
 include/linux/amd-pstate.h                    |  74 +++
 tools/power/cpupower/debug/kernel/Makefile    |  10 +-
 .../cpupower/debug/kernel/amd-pstate-ut.c     | 618 ++++++++++++++++++
 5 files changed, 923 insertions(+), 60 deletions(-)
 create mode 100644 include/linux/amd-pstate.h
 create mode 100644 tools/power/cpupower/debug/kernel/amd-pstate-ut.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] cpufreq: amd-pstate: Expose struct amd_cpudata
  2022-03-23  7:14 [PATCH 0/3] Add unit test module for AMD P-State driver Meng Li
@ 2022-03-23  7:15 ` Meng Li
  2022-03-23  7:15 ` [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver Meng Li
  2022-03-23  7:15 ` [PATCH 3/3] Documentation: amd-pstate: Add unit test introduction Meng Li
  2 siblings, 0 replies; 7+ messages in thread
From: Meng Li @ 2022-03-23  7:15 UTC (permalink / raw)
  To: Shuah Khan, Rafael J . Wysocki, Huang Rui, linux-pm
  Cc: Nathan Fontenot, Deepak Sharma, Alex Deucher, Mario Limonciello,
	Jinzhou Su, Perry Yuan, Xiaojian Du, Viresh Kumar,
	Borislav Petkov, linux-kernel, Meng Li

Expose struct amd_cpudata to AMD P-State unit test module.

This data struct will be used on the following AMD P-State unit test
(amd-pstate-ut) module. The amd-pstate-ut module can get some AMD P-State
infomations by this data struct. For example: highest perf, nominal
perf, boost supported etc.

Signed-off-by: Meng Li <li.meng@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 60 +----------------------------
 include/linux/amd-pstate.h   | 74 ++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 59 deletions(-)
 create mode 100644 include/linux/amd-pstate.h

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 7be38bc6a673..5f7a00a64f76 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -36,6 +36,7 @@
 #include <linux/delay.h>
 #include <linux/uaccess.h>
 #include <linux/static_call.h>
+#include <linux/amd-pstate.h>
 
 #include <acpi/processor.h>
 #include <acpi/cppc_acpi.h>
@@ -65,65 +66,6 @@ MODULE_PARM_DESC(shared_mem,
 
 static struct cpufreq_driver amd_pstate_driver;
 
-/**
- * struct  amd_aperf_mperf
- * @aperf: actual performance frequency clock count
- * @mperf: maximum performance frequency clock count
- * @tsc:   time stamp counter
- */
-struct amd_aperf_mperf {
-	u64 aperf;
-	u64 mperf;
-	u64 tsc;
-};
-
-/**
- * struct amd_cpudata - private CPU data for AMD P-State
- * @cpu: CPU number
- * @req: constraint request to apply
- * @cppc_req_cached: cached performance request hints
- * @highest_perf: the maximum performance an individual processor may reach,
- *		  assuming ideal conditions
- * @nominal_perf: the maximum sustained performance level of the processor,
- *		  assuming ideal operating conditions
- * @lowest_nonlinear_perf: the lowest performance level at which nonlinear power
- *			   savings are achieved
- * @lowest_perf: the absolute lowest performance level of the processor
- * @max_freq: the frequency that mapped to highest_perf
- * @min_freq: the frequency that mapped to lowest_perf
- * @nominal_freq: the frequency that mapped to nominal_perf
- * @lowest_nonlinear_freq: the frequency that mapped to lowest_nonlinear_perf
- * @cur: Difference of Aperf/Mperf/tsc count between last and current sample
- * @prev: Last Aperf/Mperf/tsc count value read from register
- * @freq: current cpu frequency value
- * @boost_supported: check whether the Processor or SBIOS supports boost mode
- *
- * The amd_cpudata is key private data for each CPU thread in AMD P-State, and
- * represents all the attributes and goals that AMD P-State requests at runtime.
- */
-struct amd_cpudata {
-	int	cpu;
-
-	struct	freq_qos_request req[2];
-	u64	cppc_req_cached;
-
-	u32	highest_perf;
-	u32	nominal_perf;
-	u32	lowest_nonlinear_perf;
-	u32	lowest_perf;
-
-	u32	max_freq;
-	u32	min_freq;
-	u32	nominal_freq;
-	u32	lowest_nonlinear_freq;
-
-	struct amd_aperf_mperf cur;
-	struct amd_aperf_mperf prev;
-
-	u64 freq;
-	bool	boost_supported;
-};
-
 static inline int pstate_enable(bool enable)
 {
 	return wrmsrl_safe(MSR_AMD_CPPC_ENABLE, enable);
diff --git a/include/linux/amd-pstate.h b/include/linux/amd-pstate.h
new file mode 100644
index 000000000000..790b04c9000b
--- /dev/null
+++ b/include/linux/amd-pstate.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * linux/include/linux/amd-pstate.h
+ *
+ * Copyright (C) 2007-2010 Advanced Micro Devices, Inc.
+ * Author: Meng Li <li.meng@amd.com>
+ */
+#ifndef _LINUX_AMD_PSTATE_H
+#define _LINUX_AMD_PSTATE_H
+
+#include <linux/pm_qos.h>
+/*********************************************************************
+ *                        AMD P-state INTERFACE                       *
+ *********************************************************************/
+/**
+ * struct  amd_aperf_mperf
+ * @aperf: actual performance frequency clock count
+ * @mperf: maximum performance frequency clock count
+ * @tsc:   time stamp counter
+ */
+struct amd_aperf_mperf {
+	u64 aperf;
+	u64 mperf;
+	u64 tsc;
+};
+
+/**
+ * struct amd_cpudata - private CPU data for AMD P-State
+ * @cpu: CPU number
+ * @req: constraint request to apply
+ * @cppc_req_cached: cached performance request hints
+ * @highest_perf: the maximum performance an individual processor may reach,
+ *		  assuming ideal conditions
+ * @nominal_perf: the maximum sustained performance level of the processor,
+ *		  assuming ideal operating conditions
+ * @lowest_nonlinear_perf: the lowest performance level at which nonlinear power
+ *			   savings are achieved
+ * @lowest_perf: the absolute lowest performance level of the processor
+ * @max_freq: the frequency that mapped to highest_perf
+ * @min_freq: the frequency that mapped to lowest_perf
+ * @nominal_freq: the frequency that mapped to nominal_perf
+ * @lowest_nonlinear_freq: the frequency that mapped to lowest_nonlinear_perf
+ * @cur: Difference of Aperf/Mperf/tsc count between last and current sample
+ * @prev: Last Aperf/Mperf/tsc count value read from register
+ * @freq: current cpu frequency value
+ * @boost_supported: check whether the Processor or SBIOS supports boost mode
+ *
+ * The amd_cpudata is key private data for each CPU thread in AMD P-State, and
+ * represents all the attributes and goals that AMD P-State requests at runtime.
+ */
+struct amd_cpudata {
+	int	cpu;
+
+	struct	freq_qos_request req[2];
+	u64	cppc_req_cached;
+
+	u32	highest_perf;
+	u32	nominal_perf;
+	u32	lowest_nonlinear_perf;
+	u32	lowest_perf;
+
+	u32	max_freq;
+	u32	min_freq;
+	u32	nominal_freq;
+	u32	lowest_nonlinear_freq;
+
+	struct amd_aperf_mperf cur;
+	struct amd_aperf_mperf prev;
+
+	u64	freq;
+	bool	boost_supported;
+};
+
+#endif /* _LINUX_AMD_PSTATE_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver
  2022-03-23  7:14 [PATCH 0/3] Add unit test module for AMD P-State driver Meng Li
  2022-03-23  7:15 ` [PATCH 1/3] cpufreq: amd-pstate: Expose struct amd_cpudata Meng Li
@ 2022-03-23  7:15 ` Meng Li
  2022-03-23 14:15   ` Shuah Khan
  2022-03-23  7:15 ` [PATCH 3/3] Documentation: amd-pstate: Add unit test introduction Meng Li
  2 siblings, 1 reply; 7+ messages in thread
From: Meng Li @ 2022-03-23  7:15 UTC (permalink / raw)
  To: Shuah Khan, Rafael J . Wysocki, Huang Rui, linux-pm
  Cc: Nathan Fontenot, Deepak Sharma, Alex Deucher, Mario Limonciello,
	Jinzhou Su, Perry Yuan, Xiaojian Du, Viresh Kumar,
	Borislav Petkov, linux-kernel, Meng Li

amd-pstate-ut is a kernel module for testing the functions of AMD P-State driver.

It can verify the required conditions and basic functions of AMD P-State
driver before integration test.

Signed-off-by: Meng Li <li.meng@amd.com>
---
 tools/power/cpupower/debug/kernel/Makefile    |  10 +-
 .../cpupower/debug/kernel/amd-pstate-ut.c     | 618 ++++++++++++++++++
 2 files changed, 627 insertions(+), 1 deletion(-)
 create mode 100644 tools/power/cpupower/debug/kernel/amd-pstate-ut.c

diff --git a/tools/power/cpupower/debug/kernel/Makefile b/tools/power/cpupower/debug/kernel/Makefile
index 7b5c43684be1..314827f106b4 100644
--- a/tools/power/cpupower/debug/kernel/Makefile
+++ b/tools/power/cpupower/debug/kernel/Makefile
@@ -8,11 +8,19 @@ ifeq ("$(CONFIG_X86_TSC)", "y")
   obj-m	 += cpufreq-test_tsc.o
 endif
 
+amd_pstate_ut-y := amd-pstate-ut.o
+
+ifeq ("$(CONFIG_X86_AMD_PSTATE)", "y")
+  obj-m	 += amd_pstate_ut.o
+else ifeq ("$(CONFIG_X86_AMD_PSTATE)", "m")
+  obj-m	 += amd_pstate_ut.o
+endif
+
 default:
 	$(MAKE) -C $(KDIR) M=$(CURDIR)
 
 clean:
-	- rm -rf *.o *.ko .*.cmd .*.mod.* *.mod.c
+	- rm -rf *.o *.ko .*.cmd *.mod .*.mod.* *.mod.c
 	- rm -rf Module.symvers modules.order
 
 install: default
diff --git a/tools/power/cpupower/debug/kernel/amd-pstate-ut.c b/tools/power/cpupower/debug/kernel/amd-pstate-ut.c
new file mode 100644
index 000000000000..0cb08e50947b
--- /dev/null
+++ b/tools/power/cpupower/debug/kernel/amd-pstate-ut.c
@@ -0,0 +1,618 @@
+// 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>
+ *
+ */
+
+#define pr_fmt(fmt) "AMD P-state UT: " fmt
+
+#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>
+
+#define AMD_PSTATE_UT_ERR(...) pr_err(__VA_ARGS__)
+#define AMD_PSTATE_UT_DEBUG(...) pr_debug(__VA_ARGS__)
+#define AMD_PSTATE_UT_INFO(...) pr_info(__VA_ARGS__)
+
+#define UT_LOG_BEGIN(index) AMD_PSTATE_UT_DEBUG("****** Begin %-5d\t %-20s\t ******\n", index, ut_cases[index].name)
+#define UT_LOG_END(index) AMD_PSTATE_UT_DEBUG("****** End   %-5d\t %-20s\t ******\n", index, ut_cases[index].name)
+
+enum ut_index {
+	UT_INDEX_BEGIN=0,
+	UT_INDEX_END,
+	MAX_UT_INDEX,
+};
+
+enum ut_result {
+	UT_RESULT_NOT_TEST=0,	//' '
+	UT_RESULT_PASS,		//'P'
+	UT_RESULT_FAIL,		//'F'
+	MAX_UT_RESULT,
+};
+
+struct ut_struct {
+	const char *name;
+	void (*func)(u32 index);
+	u32 index;
+	enum ut_result result;
+};
+
+static enum ut_result ut_special(u32 begin_index,
+		u32 end_index);
+
+static void ut_stop(u32 index);
+static void ut_all(u32 index);
+static void ut_x86_vendor(u32 index);
+static void ut_acpi_cpc(u32 index);
+static void ut_modprobed_driver(u32 index);
+static void ut_capability_check(u32 index);
+static void ut_enable(u32 index);
+static void ut_init_perf(u32 index);
+static void ut_support_boost(u32 index);
+static void ut_clear_status(u32 index);
+
+/* Amd-pstate unit test cases
+ * 0 : stop
+ * 1 : all
+ * n : from n to start
+ */
+static u32 ut_array_index[MAX_UT_INDEX] = {0};
+
+static struct ut_struct ut_cases[] = {
+	{"stop",                ut_stop                }, //0
+	{"all",                 ut_all                 }, //1
+	{"x86_vendor",          ut_x86_vendor          },
+	{"acpi_cpc_valid",      ut_acpi_cpc            },
+	{"modprobed_driver",    ut_modprobed_driver    },
+	{"capability_check",    ut_capability_check    },
+	{"enable",              ut_enable              },
+	{"init_perf",           ut_init_perf           },
+	{"support_boost",       ut_support_boost       },
+	{"clear_status",        ut_clear_status        } //max
+};
+
+static bool get_shared_mem(void)
+{
+	bool result = false;
+	char buf[5] = {0};
+	struct file *filp = NULL;
+	loff_t pos = 0;
+	ssize_t ret;
+
+	filp = filp_open("/sys/module/amd_pstate/parameters/shared_mem", FMODE_PREAD, 0);
+	if (IS_ERR(filp))
+	{
+		AMD_PSTATE_UT_ERR("%s Open param file fail!", __func__);
+	}
+	else
+	{
+		ret = kernel_read(filp, &buf, sizeof(buf), &pos);
+		if (ret < 0)
+		{
+			AMD_PSTATE_UT_ERR("%s ret=%ld unable to read from param file!", __func__, ret);
+		}
+	}
+	filp_close(filp, NULL);
+
+	if ('Y' == *buf)
+	{
+		result = true;
+	}
+
+	return (result);
+}
+
+static bool ut_check_index(u32 index,
+		const char *func_name)
+{
+	bool ret = true;
+
+	if (index != ut_cases[index].index)
+	{
+		AMD_PSTATE_UT_ERR("%s index=%d is error!", func_name, index);
+		ret = false;
+	}
+
+	return (ret);
+}
+
+static void ut_stop(u32 index)
+{
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	AMD_PSTATE_UT_DEBUG("!====! Stop unit test! !====! \n");
+
+	//memset begin/end index
+	memset(ut_array_index, 0, sizeof(ut_array_index));
+}
+
+static void ut_all(u32 index)
+{
+	u32 arr_size = ARRAY_SIZE(ut_cases);
+	enum ut_result result = UT_RESULT_PASS;
+
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	if (index+1 < arr_size)
+	{
+		AMD_PSTATE_UT_DEBUG("!====! Begin all unit test! !====!\n");
+
+		//Not include ut_clear_status
+		result = ut_special(index+1, arr_size-1-1);
+
+		AMD_PSTATE_UT_DEBUG("!====! End   all unit test! !====!\n");
+	}
+	else
+	{
+		result = UT_RESULT_FAIL;
+		AMD_PSTATE_UT_ERR("%s arr_size=%d is error!", __func__, arr_size);
+	}
+	ut_cases[index].result = result;
+}
+
+static enum ut_result ut_special(u32 begin_index,
+		u32 end_index)
+{
+	u32 i = 0;
+	enum ut_result result = UT_RESULT_PASS;
+
+	for (i=begin_index; i<=end_index; i++)
+	{
+		if ((0 == ut_array_index[UT_INDEX_BEGIN]) &&
+			(0 == ut_array_index[UT_INDEX_END]))
+		{
+			AMD_PSTATE_UT_DEBUG("!====! Stop unit test : End %-5d\t %-20s\t! !====! \n", i, ut_cases[i].name);
+			break;
+		}
+		else
+		{
+			ut_cases[i].func(i);
+			if (UT_RESULT_PASS != ut_cases[i].result)
+			{
+				result = ut_cases[i].result;
+			}
+		}
+	}
+
+	return (result);
+}
+
+static void ut_x86_vendor(u32 index)
+{
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+	{
+		ut_cases[index].result = UT_RESULT_PASS;
+	}
+	else
+	{
+		ut_cases[index].result = UT_RESULT_FAIL;
+	}
+
+	UT_LOG_END(index);
+}
+
+static void ut_acpi_cpc(u32 index)
+{
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	if (acpi_cpc_valid())
+	{
+		ut_cases[index].result = UT_RESULT_PASS;
+	}
+	else
+	{
+		ut_cases[index].result = UT_RESULT_FAIL;
+	}
+
+	UT_LOG_END(index);
+}
+
+static void ut_modprobed_driver(u32 index)
+{
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	if (cpufreq_get_current_driver())
+	{
+		ut_cases[index].result = UT_RESULT_PASS;
+	}
+	else
+	{
+		ut_cases[index].result = UT_RESULT_FAIL;
+	}
+
+	UT_LOG_END(index);
+}
+
+static void ut_capability_check(u32 index)
+{
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	if (boot_cpu_has(X86_FEATURE_CPPC))
+	{
+		ut_cases[index].result = UT_RESULT_PASS;
+	}
+	else
+	{
+		//shared memory
+		if (get_shared_mem())
+		{
+			ut_cases[index].result = UT_RESULT_PASS;
+		}
+		else
+		{
+			ut_cases[index].result = UT_RESULT_FAIL;
+		}
+	}
+
+	UT_LOG_END(index);
+}
+
+static void ut_pstate_enable(u32 index)
+{
+	int ret = 0;
+	u64 cppc_enable = 0;
+
+	ret = rdmsrl_safe(MSR_AMD_CPPC_ENABLE, &cppc_enable);
+	if (ret)
+	{
+		ut_cases[index].result = UT_RESULT_FAIL;
+		AMD_PSTATE_UT_ERR("%s rdmsrl_safe MSR_AMD_CPPC_ENABLE ret=%d is error!", __func__, ret);
+		return;
+	}
+	if (cppc_enable)
+	{
+		ut_cases[index].result = UT_RESULT_PASS;
+	}
+	else
+	{
+		ut_cases[index].result = UT_RESULT_FAIL;
+	}
+}
+
+static void ut_enable(u32 index)
+{
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	if (get_shared_mem())
+	{
+		//not check
+		ut_cases[index].result = UT_RESULT_PASS;
+	}
+	else
+	{
+		ut_pstate_enable(index);
+	}
+
+	UT_LOG_END(index);
+}
+
+static void ut_init_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;
+
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	//get perf
+	highest_perf = amd_get_highest_perf();
+
+	for_each_possible_cpu(cpu)
+	{
+		//get amd cpudata
+		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)
+			{
+				ut_cases[index].result = UT_RESULT_FAIL;
+				AMD_PSTATE_UT_ERR("%s cppc_get_perf_caps ret=%d is error!", __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)
+			{
+				ut_cases[index].result = UT_RESULT_FAIL;
+				AMD_PSTATE_UT_ERR("%s rdmsrl_safe_on_cpu MSR_AMD_CPPC_CAP1 ret=%d is error!", __func__, ret);
+				return;
+			}
+
+			//get perf from MSR
+			nominal_perf = AMD_CPPC_NOMINAL_PERF(cap1);
+			lowest_nonlinear_perf = AMD_CPPC_LOWNONLIN_PERF(cap1);
+			lowest_perf = AMD_CPPC_LOWEST_PERF(cap1);
+		}
+
+		//check highest_perf,nominal_perf,lowest_nonlinear_perf
+		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)))
+		{
+			ut_cases[index].result = UT_RESULT_FAIL;
+			AMD_PSTATE_UT_ERR("%s cpu%d highest_perf=%d %d nominal_perf=%d %d lowest_nonlinear_perf=%d %d lowest_perf=%d %d are not equal!",
+					__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;
+		}
+	}
+
+	ut_cases[index].result = UT_RESULT_PASS;
+
+	UT_LOG_END(index);
+}
+
+static void ut_support_boost(u32 index)
+{
+	int cpu = 0;
+	struct cpufreq_policy *policy = NULL;
+        struct amd_cpudata *cpudata = NULL;
+
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	for_each_possible_cpu(cpu)
+	{
+		//get amd cpudata
+		policy = cpufreq_cpu_get(cpu);
+		if (!policy)
+			break;
+		cpudata = policy->driver_data;
+
+		if (READ_ONCE(cpudata->boost_supported))
+		{
+			ut_cases[index].result = UT_RESULT_PASS;
+		}
+		else
+		{
+			ut_cases[index].result = UT_RESULT_FAIL;
+			break;
+		}
+	}
+
+	UT_LOG_END(index);
+}
+
+static void ut_clear_status(u32 index)
+{
+	int i = 0;
+
+	if (!ut_check_index(index, __func__))
+	{
+		return;
+	}
+
+	UT_LOG_BEGIN(index);
+
+	for (i = 0; i < ARRAY_SIZE(ut_cases); i++)
+	{
+		ut_cases[i].result = UT_RESULT_NOT_TEST;
+	}
+
+	//memset begin/end index
+	memset(ut_array_index, 0, sizeof(ut_array_index));
+
+	UT_LOG_END(index);
+}
+
+static int param_set_index(const char *str)
+{
+	int i = 0, result = 0;
+
+	//memset begin/end index
+	memset(ut_array_index, 0, sizeof(ut_array_index));
+
+	//get begin/end index
+	while ((NULL != str) && ('\0' != *str) && ('\n' != *str))
+	{
+		ut_array_index[i] = ut_array_index[i]*10 + *str - '0';
+		str++;
+		if (' ' == *str)
+		{
+			str++;
+			i++;
+			if (MAX_UT_INDEX <= i)
+			{
+				result = -EINVAL;
+				AMD_PSTATE_UT_ERR("%s arg_num=%d, Invalid argument! \n", __func__, i);
+				break;
+			}
+		}
+	}
+
+	AMD_PSTATE_UT_DEBUG("%s %d %d result=%d! \n", __func__, ut_array_index[0], ut_array_index[1], result);
+
+	return (result);
+}
+
+static void ut_init_index(void)
+{
+	int i = 0;
+
+	if (1 != ut_cases[1].index)
+	{
+		//init all unit test cases index
+		for (i = 0; i < ARRAY_SIZE(ut_cases); i++)
+		{
+			ut_cases[i].index = i;
+		}
+	}
+}
+
+/* Parameters */
+static int param_set_unit_test(const char *buffer,
+		const struct kernel_param *kp)
+{
+	int result = 0;
+	u32 begin_index = 0, end_index = 0;
+
+	//init test case index
+	ut_init_index();
+
+	//get begin/end index
+	result = param_set_index(buffer);
+	if (result < 0)
+		return result;
+
+	begin_index  = ut_array_index[UT_INDEX_BEGIN];
+	end_index  = ut_array_index[UT_INDEX_END];
+	if (0 == begin_index)
+	{
+		ut_array_index[UT_INDEX_END] = 0;
+		ut_stop(0);
+	}
+	else if (1 == begin_index)
+	{
+		//Not include ut_clear_status
+		ut_array_index[UT_INDEX_END] = ARRAY_SIZE(ut_cases) - 1 - 1;
+		ut_all(1);
+	}
+	else
+	{
+		if (begin_index > end_index)
+		{
+			end_index = begin_index;
+			ut_array_index[UT_INDEX_END] = end_index;
+		}
+		ut_special(begin_index, end_index);
+	}
+
+	return result;
+};
+
+static int param_get_unit_test(char *buffer,
+		const struct kernel_param *kp)
+{
+	char ret = 'N';
+	int i = 0, result = 0;
+
+	result = sprintf(buffer, "%-5s\t %-20s\t Status\n", "Index", "Test cases");
+
+	for (i = 0; i < ARRAY_SIZE(ut_cases); i++)
+	{
+		switch (ut_cases[i].result)
+		{
+		case UT_RESULT_NOT_TEST:
+			ret = ' ';
+			break;
+
+		case UT_RESULT_PASS:
+			ret = 'P';
+			break;
+
+		case UT_RESULT_FAIL:
+			ret = 'F';
+			break;
+
+		default:
+			ret = ' ';
+			break;
+		}
+
+		result += sprintf(buffer + result, "%-5d\t %-20s\t [%c]\n",
+				ut_cases[i].index,
+				ut_cases[i].name,
+				ret);
+	}
+
+	result += sprintf(buffer + result, "------------------------------------------\nbegin_index = %d end_index= %d\n",
+			ut_array_index[UT_INDEX_BEGIN], ut_array_index[UT_INDEX_END]);
+
+	return result;
+};
+
+static const struct kernel_param_ops param_ops_unittest = {
+	.set = param_set_unit_test,
+	.get = param_get_unit_test,
+};
+
+module_param_cb(unit_test, &param_ops_unittest, &ut_array_index, 0664);
+MODULE_PARM_DESC(unit_test,
+		" AMD P-state unit test 0 : stop 1 : start all x: start from x");
+
+/* Module */
+static int __init amd_pstate_ut_init(void)
+{
+	//init test case index
+	ut_init_index();
+
+	return 0;
+}
+
+static void __exit amd_pstate_ut_exit(void)
+{
+}
+
+module_init(amd_pstate_ut_init);
+module_exit(amd_pstate_ut_exit);
+
+MODULE_AUTHOR("Meng Li <li.meng@amd.com>");
+MODULE_DESCRIPTION("Unit test for AMD P-state driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] Documentation: amd-pstate: Add unit test introduction
  2022-03-23  7:14 [PATCH 0/3] Add unit test module for AMD P-State driver Meng Li
  2022-03-23  7:15 ` [PATCH 1/3] cpufreq: amd-pstate: Expose struct amd_cpudata Meng Li
  2022-03-23  7:15 ` [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver Meng Li
@ 2022-03-23  7:15 ` Meng Li
  2 siblings, 0 replies; 7+ messages in thread
From: Meng Li @ 2022-03-23  7:15 UTC (permalink / raw)
  To: Shuah Khan, Rafael J . Wysocki, Huang Rui, linux-pm
  Cc: Nathan Fontenot, Deepak Sharma, Alex Deucher, Mario Limonciello,
	Jinzhou Su, Perry Yuan, Xiaojian Du, Viresh Kumar,
	Borislav Petkov, linux-kernel, Meng Li

Introduce the AMD P-State unit test module design and implementation.

Signed-off-by: Meng Li <li.meng@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 221 ++++++++++++++++++++
 1 file changed, 221 insertions(+)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index 1923cb25073b..47ba6eeb2e38 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -182,6 +182,7 @@ according to the ``struct sugov_cpu`` that utilization update belongs to.
 Then ``amd-pstate`` updates the desired performance according to the CPU
 scheduler assigned.
 
+.. _processor_support:
 
 Processor Support
 =======================
@@ -283,6 +284,8 @@ efficiency frequency management method on AMD processors.
 Kernel Module Options for ``amd-pstate``
 =========================================
 
+.. _shared_mem:
+
 ``shared_mem``
 Use a module param (shared_mem) to enable related processors manually with
 **amd_pstate.shared_mem=1**.
@@ -395,6 +398,224 @@ about part of the output. ::
  CPU_006     712          116408        39        49        166       0.6769  8950227 1839034 37192089  24.06  11.272       470         2.496         kworker/6:0-1264
 
 
+Unit Tests for amd-pstate
+-------------------------
+
+``amd-pstate-ut`` is a kernel module for testing the functions of ``amd-pstate``.
+It can verify the required conditions and basic functions of ``amd-pstate`` before integration test.
+
+1. Test case decriptions
+
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | Index   | Functions                      | Description                                                                        |
+        +=========+================================+====================================================================================+
+        | 0       | stop                           || Stop the test currently in progress.                                              |
+        |         |                                ||                                                                                   |
+        |         |                                || For example: if the n test case is being executed, it needs to be completed and   |
+        |         |                                | the next test case will not be executed.                                           |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 1       | all                            || Start all test cases.                                                             |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : all cases pass                                                           |
+        |         |                                || - Fail : only one case fail                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 2       | x86_vendor                     || Check whether the boot CPU x86_vendor is AMD.                                     |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : no                                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 3       | acpi_cpc_valid                 || Check whether the _CPC object is present in SBIOS.                                |
+        |         |                                ||                                                                                   |
+        |         |                                || The detail refer to `Processor Support <processor_support_>`_.                    |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : no                                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 4       | modprobed_driver               || Check whether cpufreq_driver module is load.                                      |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : no                                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 5       | capability_check               || Full MSR Support: check whether cpu set feature flag :c:macro:`X86_FEATURE_CPPC`. |
+        |         |                                || The detail refer to `Processor Support <processor_support_>`_.                    |
+        |         |                                ||                                                                                   |
+        |         |                                || Shared Memory Support: check whether module param shared_mem enable               |
+        |         |                                || The detail refer to `Shared Memory <shared_mem_>`_.                               |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : no                                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 6       | enable                         || Check whether AMD P-State is enabled.                                             |
+        |         |                                ||                                                                                   |
+        |         |                                || AMD P-States and ACPI hardware P-States always can be supported in one processor. |
+        |         |                                | But AMD P-States has the higher priority and if it is enabled with                 |
+        |         |                                | :c:macro:`MSR_AMD_CPPC_ENABLE` or ``cppc_set_enable``, it will respond to the      |
+        |         |                                | request from AMD P-States.                                                         |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : no                                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 7       | init_perf                      || Check whether AMD P-State init perf sucessfully.                                  |
+        |         |                                | Include highest_perf, nominal_perf, lowest_nonlinear_perf and lowest_perf values.  |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : no                                                                       |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 8       | support_boost                  || Check whether all cpus support boost.                                             |
+        |         |                                ||                                                                                   |
+        |         |                                || If boost is not active but supported, this maximum frequency will be larger than  |
+        |         |                                | the one in ``cpuinfo``.                                                            |
+        |         |                                ||                                                                                   |
+        |         |                                || - Pass : yes                                                                      |
+        |         |                                || - Fail : only one cpu not suppport boost                                          |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+        | 9       | clear_status                   || Clear all test results.                                                           |
+        |         |                                ||                                                                                   |
+        |         |                                || **This test case is always at the end and cannot be changed.**                    |
+        +---------+--------------------------------+------------------------------------------------------------------------------------+
+
+#. How to execute the tests
+
+    a. Build ::
+
+        jasminemeng@jasmine-meng:~/amd-brahma$ cd linux
+        jasminemeng@jasmine-meng:~/amd-brahma/linux$ make modules M=tools/power/cpupower/debug/kernel/
+          CC [M]  tools/power/cpupower/debug/kernel/amd_pstate_ut.o
+          MODPOST tools/power/cpupower/debug/kernel/Module.symvers
+          CC [M]  tools/power/cpupower/debug/kernel/amd_pstate_ut.mod.o
+          LD [M]  tools/power/cpupower/debug/kernel/amd_pstate_ut.ko
+
+
+    #. Installation & Steps
+
+       Test all cases ::
+
+        jasmine@jasmine-MayanDAP-RMB:~$ sudo insmod amd_pstate_ut.ko
+        jasmine@jasmine-MayanDAP-RMB:~$ cd /sys/module/amd_pstate_ut/parameters/
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ sudo chmod 666 unit_test
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ cat unit_test
+        Index	 Test cases          	 Status
+        0    	 stop                	 [ ]
+        1    	 all                 	 [ ]
+        2    	 x86_vendor          	 [ ]
+        3    	 acpi_cpc_valid      	 [ ]
+        4    	 modprobed_driver    	 [ ]
+        5    	 capability_check    	 [ ]
+        6    	 enable              	 [ ]
+        7    	 init_perf           	 [ ]
+        8    	 support_boost       	 [ ]
+        9    	 clear_status        	 [ ]
+        ------------------------------------------
+        begin_index = 0 end_index= 0
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ echo 1 > unit_test
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ cat unit_test
+        Index	 Test cases          	 Status
+        0    	 stop                	 [ ]
+        1    	 all                 	 [P]
+        2    	 x86_vendor          	 [P]
+        3    	 acpi_cpc_valid      	 [P]
+        4    	 modprobed_driver    	 [P]
+        5    	 capability_check    	 [P]
+        6    	 enable              	 [P]
+        7    	 init_perf           	 [P]
+        8    	 support_boost       	 [P]
+        9    	 clear_status        	 [ ]
+        ------------------------------------------
+        begin_index = 1 end_index= 8
+
+       Clear test resut ::
+
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ echo 9 > unit_test
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ cat unit_test
+        Index	 Test cases          	 Status
+        0    	 stop                	 [ ]
+        1    	 all                 	 [ ]
+        2    	 x86_vendor          	 [ ]
+        3    	 acpi_cpc_valid      	 [ ]
+        4    	 modprobed_driver    	 [ ]
+        5    	 capability_check    	 [ ]
+        6    	 enable              	 [ ]
+        7    	 init_perf           	 [ ]
+        8    	 support_boost       	 [ ]
+        ------------------------------------------
+        begin_index = 0 end_index= 0
+
+       Test specific case ::
+
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ echo 3 > unit_test
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ cat unit_test
+        Index	 Test cases          	 Status
+        0    	 stop                	 [ ]
+        1    	 all                 	 [ ]
+        2    	 x86_vendor          	 [ ]
+        3    	 acpi_cpc_valid      	 [P]
+        4    	 modprobed_driver    	 [ ]
+        5    	 capability_check    	 [ ]
+        6    	 enable              	 [ ]
+        7    	 init_perf           	 [ ]
+        8    	 support_boost       	 [ ]
+        9    	 clear_status        	 [ ]
+        ------------------------------------------
+        begin_index = 3 end_index= 3
+
+       Test some cases ::
+
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ echo 5 8 > unit_test
+        jasmine@jasmine-MayanDAP-RMB:/sys/module/amd_pstate_ut/parameters$ cat unit_test
+        Index	 Test cases          	 Status
+        0    	 stop                	 [ ]
+        1    	 all                 	 [ ]
+        2    	 x86_vendor          	 [ ]
+        3    	 acpi_cpc_valid      	 [P]
+        4    	 modprobed_driver    	 [ ]
+        5    	 capability_check    	 [P]
+        6    	 enable              	 [P]
+        7    	 init_perf           	 [P]
+        8    	 support_boost       	 [P]
+        9    	 clear_status        	 [ ]
+        ------------------------------------------
+        begin_index = 5 end_index= 8
+
+
+    #. Results
+
+        +-----------+--------------------------------+
+        |  Status   | Description                    |
+        +===========+================================+
+        |   [ ]     | Not test.                      |
+        +-----------+--------------------------------+
+        |   [P]     | Test pass.                     |
+        +-----------+--------------------------------+
+        |   [F]     | Test fail.                     |
+        +-----------+--------------------------------+
+
+        Enable all the messages in the ``amd-pstate-ut`` module. ::
+
+         jasmine@jasmine-MayanDAP-RMB:~/3_Cr$ su
+         root@jasmine-MayanDAP-RMB:# echo -n "module amd_pstate_ut +p" > /sys/kernel/debug/dynamic_debug/control
+
+        When you start to test all cases, you will get the follow logs. ::
+
+         jasmine@jasmine-MayanDAP-RMB:~$ dmesg | grep "AMD P-state UT" | tee log.txt
+         [  732.084122] AMD P-state UT:param_set_index 1 0 result=0!
+         [  732.084140] AMD P-state UT:!====! Begin all unit test! !====!
+         [  732.084146] AMD P-state UT:****** Begin 2    	 x86_vendor          	 ******
+         [  732.084152] AMD P-state UT:****** End   2    	 x86_vendor          	 ******
+         [  732.084156] AMD P-state UT:****** Begin 3    	 acpi_cpc_valid      	 ******
+         [  732.084162] AMD P-state UT:****** End   3    	 acpi_cpc_valid      	 ******
+         [  732.084165] AMD P-state UT:****** Begin 4    	 modprobed_driver    	 ******
+         [  732.084168] AMD P-state UT:****** End   4    	 modprobed_driver    	 ******
+         [  732.084172] AMD P-state UT:****** Begin 5    	 capability_check    	 ******
+         [  732.084174] AMD P-state UT:****** End   5    	 capability_check    	 ******
+         [  732.084176] AMD P-state UT:****** Begin 6    	 enable              	 ******
+         [  732.084222] AMD P-state UT:****** End   6    	 enable              	 ******
+         [  732.084225] AMD P-state UT:****** Begin 7    	 init_perf           	 ******
+         [  732.085016] AMD P-state UT:****** End   7    	 init_perf           	 ******
+         [  732.085026] AMD P-state UT:****** Begin 8    	 support_boost       	 ******
+         [  732.085033] AMD P-state UT:****** End   8    	 support_boost       	 ******
+         [  732.085036] AMD P-state UT:!====! End   all unit test! !====!
+
+
 Reference
 ===========
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver
  2022-03-23  7:15 ` [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver Meng Li
@ 2022-03-23 14:15   ` Shuah Khan
  2022-03-28 13:57     ` Meng, Li (Jassmine)
  0 siblings, 1 reply; 7+ messages in thread
From: Shuah Khan @ 2022-03-23 14:15 UTC (permalink / raw)
  To: Meng Li, Rafael J . Wysocki, Huang Rui, linux-pm
  Cc: Nathan Fontenot, Deepak Sharma, Alex Deucher, Mario Limonciello,
	Jinzhou Su, Perry Yuan, Xiaojian Du, Viresh Kumar,
	Borislav Petkov, linux-kernel, Shuah Khan

On 3/23/22 1:15 AM, Meng Li wrote:
> amd-pstate-ut is a kernel module for testing the functions of AMD P-State driver.
> 
> It can verify the required conditions and basic functions of AMD P-State
> driver before integration test.
> 

Can you elaborate on the need for a kernel module? It would be helpful
to know tne value the mdoule adds and why it is necessary. Include
details on why it can't be done as part of the user-space program.

I am not saying it isn't necssary, I would like to know the reasons
before I review the patch.

Also if this is a driver test, why not use other test frameworks
such as kunit or kselftest. cpupower is user-space utility and
driver debug code would not belong in here.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver
  2022-03-23 14:15   ` Shuah Khan
@ 2022-03-28 13:57     ` Meng, Li (Jassmine)
  2022-03-28 14:56       ` Shuah Khan
  0 siblings, 1 reply; 7+ messages in thread
From: Meng, Li (Jassmine) @ 2022-03-28 13:57 UTC (permalink / raw)
  To: Shuah Khan, Rafael J . Wysocki, Huang, Ray, linux-pm
  Cc: Fontenot, Nathan, Sharma, Deepak, Deucher, Alexander,
	Limonciello, Mario, Su, Jinzhou (Joe),
	Yuan, Perry, Du, Xiaojian, Viresh Kumar, Borislav Petkov,
	linux-kernel

[AMD Official Use Only]

Hi Shuah:

Thank you very much for your valuable suggestions. 
We will adapt it to test our AMD P-State driver. But we haven't decided which one to adapt, kunit or kselftest.

Requirements for our unit test module:
 - It can access kernel internal structures and functions which aren't exposed to user space.
 - It is implemented through the script trigger CPU benchmark app in conjunction with the kernel module.

Therefore, next, we will study which method can meet the above requirements.
Can you give us some suggestions?

Thanks,
Jasmine

-----Original Message-----
From: Shuah Khan <skhan@linuxfoundation.org> 
Sent: Wednesday, March 23, 2022 10:15 PM
To: Meng, Li (Jassmine) <Li.Meng@amd.com>; Rafael J . Wysocki <rafael.j.wysocki@intel.com>; Huang, Ray <Ray.Huang@amd.com>; linux-pm@vger.kernel.org
Cc: 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; Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver

[CAUTION: External Email]

On 3/23/22 1:15 AM, Meng Li wrote:
> amd-pstate-ut is a kernel module for testing the functions of AMD P-State driver.
>
> It can verify the required conditions and basic functions of AMD 
> P-State driver before integration test.
>

Can you elaborate on the need for a kernel module? It would be helpful to know tne value the mdoule adds and why it is necessary. Include details on why it can't be done as part of the user-space program.

I am not saying it isn't necssary, I would like to know the reasons before I review the patch.

Also if this is a driver test, why not use other test frameworks such as kunit or kselftest. cpupower is user-space utility and driver debug code would not belong in here.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver
  2022-03-28 13:57     ` Meng, Li (Jassmine)
@ 2022-03-28 14:56       ` Shuah Khan
  0 siblings, 0 replies; 7+ messages in thread
From: Shuah Khan @ 2022-03-28 14:56 UTC (permalink / raw)
  To: Meng, Li (Jassmine), Rafael J . Wysocki, Huang, Ray, linux-pm
  Cc: Fontenot, Nathan, Sharma, Deepak, Deucher, Alexander,
	Limonciello, Mario, Su, Jinzhou (Joe),
	Yuan, Perry, Du, Xiaojian, Viresh Kumar, Borislav Petkov,
	linux-kernel, Shuah Khan

On 3/28/22 7:57 AM, Meng, Li (Jassmine) wrote:
> [AMD Official Use Only]
> 
> Hi Shuah:
> 
> Thank you very much for your valuable suggestions.
> We will adapt it to test our AMD P-State driver. But we haven't decided which one to adapt, kunit or kselftest.
> 
> Requirements for our unit test module:
>   - It can access kernel internal structures and functions which aren't exposed to user space.
>   - It is implemented through the script trigger CPU benchmark app in conjunction with the kernel module.
> 
> Therefore, next, we will study which method can meet the above requirements.
> Can you give us some suggestions?
> 

No top posting please. If you need to access kernel internal structures
and functions KUNit is a better choice for you as long as all of the
testing can be done in the kernel driver.

If kernel internal structures and functions need to be tested from user-space,
then a kernel driver test module and a user-space kselftest shell or C program
is the approach to take.

> -----Original Message-----
> From: Shuah Khan <skhan@linuxfoundation.org>
> Sent: Wednesday, March 23, 2022 10:15 PM
> To: Meng, Li (Jassmine) <Li.Meng@amd.com>; Rafael J . Wysocki <rafael.j.wysocki@intel.com>; Huang, Ray <Ray.Huang@amd.com>; linux-pm@vger.kernel.org
> Cc: 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; Shuah Khan <skhan@linuxfoundation.org>
> Subject: Re: [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver
> 
> [CAUTION: External Email]
> 
> On 3/23/22 1:15 AM, Meng Li wrote:
>> amd-pstate-ut is a kernel module for testing the functions of AMD P-State driver.
>>
>> It can verify the required conditions and basic functions of AMD
>> P-State driver before integration test.
>>
> 
> Can you elaborate on the need for a kernel module? It would be helpful to know tne value the mdoule adds and why it is necessary. Include details on why it can't be done as part of the user-space program.
> 
> I am not saying it isn't necssary, I would like to know the reasons before I review the patch.
> 
> Also if this is a driver test, why not use other test frameworks such as kunit or kselftest. cpupower is user-space utility and driver debug code would not belong in here.
> 

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-03-28 14:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-23  7:14 [PATCH 0/3] Add unit test module for AMD P-State driver Meng Li
2022-03-23  7:15 ` [PATCH 1/3] cpufreq: amd-pstate: Expose struct amd_cpudata Meng Li
2022-03-23  7:15 ` [PATCH 2/3] cpupower: Introduce a new unit test module for AMD P-State driver Meng Li
2022-03-23 14:15   ` Shuah Khan
2022-03-28 13:57     ` Meng, Li (Jassmine)
2022-03-28 14:56       ` Shuah Khan
2022-03-23  7:15 ` [PATCH 3/3] Documentation: amd-pstate: Add unit test introduction Meng Li

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.