[PATCH v4 09/12] amd-pstate-ut: Add a testcase to validate the visibility of driver attributes

Gautham R. Shenoy posted 12 patches 1 week ago
[PATCH v4 09/12] amd-pstate-ut: Add a testcase to validate the visibility of driver attributes
Posted by Gautham R. Shenoy 1 week ago
amd-pstate driver has per-attribute visibility functions to
dynamically control which sysfs freq_attrs are exposed based on the
platform capabilities and the current amd_pstate mode. However, there
is no test coverage to validate that the driver's live attribute list
matches the expected visibility for each mode.

Add amd_pstate_ut_check_freq_attrs() to the amd-pstate unit test
module. For each enabled mode (passive, active, guided), the test
independently derives the expected visibility of each attribute:
  - Core attributes (max_freq, lowest_nonlinear_freq, highest_perf)
    are always expected.
  - Prefcore attributes (prefcore_ranking, hw_prefcore) are expected
    only when cpudata->hw_prefcore indicates platform support.
  - EPP attributes (energy_performance_preference,
    energy_performance_available_preferences) are expected only in
    active mode.
  - Floor frequency attributes (floor_freq, floor_count) are expected
    only when X86_FEATURE_CPPC_PERF_PRIO is present.

Compare these independent expectations against the live driver's attr
array, catching bugs such as attributes leaking into wrong modes or
visibility functions checking incorrect conditions.

Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate-ut.c | 139 ++++++++++++++++++++++++++++++--
 drivers/cpufreq/amd-pstate.c    |   8 ++
 drivers/cpufreq/amd-pstate.h    |   4 +
 3 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 3dcdf56883a6..1f62ab6438b4 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -23,6 +23,8 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/bitfield.h>
+#include <linux/cpufeature.h>
+#include <linux/cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -53,13 +55,15 @@ 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_check_driver(u32 index);
+static int amd_pstate_ut_check_freq_attrs(u32 index);
 
 static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
-	{"amd_pstate_ut_acpi_cpc_valid",   amd_pstate_ut_acpi_cpc_valid   },
-	{"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_check_driver",	   amd_pstate_ut_check_driver     }
+	{"amd_pstate_ut_acpi_cpc_valid",    amd_pstate_ut_acpi_cpc_valid   },
+	{"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_check_driver",      amd_pstate_ut_check_driver     },
+	{"amd_pstate_ut_check_freq_attrs",  amd_pstate_ut_check_freq_attrs },
 };
 
 static bool test_in_list(const char *list, const char *name)
@@ -293,6 +297,131 @@ static int amd_pstate_ut_check_driver(u32 index)
 	return ret;
 }
 
+enum attr_category {
+	ATTR_ALWAYS,
+	ATTR_PREFCORE,
+	ATTR_EPP,
+	ATTR_FLOOR_FREQ,
+};
+
+static const struct {
+	const char	*name;
+	enum attr_category category;
+} expected_freq_attrs[] = {
+	{"amd_pstate_max_freq",				ATTR_ALWAYS},
+	{"amd_pstate_lowest_nonlinear_freq",		ATTR_ALWAYS},
+	{"amd_pstate_highest_perf",			ATTR_ALWAYS},
+	{"amd_pstate_prefcore_ranking",			ATTR_PREFCORE},
+	{"amd_pstate_hw_prefcore",			ATTR_PREFCORE},
+	{"energy_performance_preference",		ATTR_EPP},
+	{"energy_performance_available_preferences",	ATTR_EPP},
+	{"amd_pstate_floor_freq",			ATTR_FLOOR_FREQ},
+	{"amd_pstate_floor_count",			ATTR_FLOOR_FREQ},
+};
+
+static bool attr_in_driver(struct freq_attr **driver_attrs, const char *name)
+{
+	int j;
+
+	for (j = 0; driver_attrs[j]; j++) {
+		if (!strcmp(driver_attrs[j]->attr.name, name))
+			return true;
+	}
+	return false;
+}
+
+/*
+ * Verify that for each mode the driver's live ->attr array contains exactly
+ * the attributes that should be visible.  Expected visibility is derived
+ * independently from hw_prefcore, cpu features, and the current mode —
+ * not from the driver's own visibility functions.
+ */
+static int amd_pstate_ut_check_freq_attrs(u32 index)
+{
+	enum amd_pstate_mode orig_mode = amd_pstate_get_status();
+	static const enum amd_pstate_mode modes[] = {
+		AMD_PSTATE_PASSIVE, AMD_PSTATE_ACTIVE, AMD_PSTATE_GUIDED,
+	};
+	bool has_prefcore, has_floor_freq;
+	int m, i, ret;
+
+	has_floor_freq = cpu_feature_enabled(X86_FEATURE_CPPC_PERF_PRIO);
+
+	/*
+	 * Determine prefcore support from any online CPU's cpudata.
+	 * hw_prefcore reflects the platform-wide decision made at init.
+	 */
+	has_prefcore = false;
+	for_each_online_cpu(i) {
+		struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
+		struct amd_cpudata *cpudata;
+
+		policy = cpufreq_cpu_get(i);
+		if (!policy)
+			continue;
+		cpudata = policy->driver_data;
+		has_prefcore = cpudata->hw_prefcore;
+		break;
+	}
+
+	for (m = 0; m < ARRAY_SIZE(modes); m++) {
+		struct freq_attr **driver_attrs;
+
+		ret = amd_pstate_set_mode(modes[m]);
+		if (ret)
+			goto out;
+
+		driver_attrs = amd_pstate_get_current_attrs();
+		if (!driver_attrs) {
+			pr_err("%s: no driver attrs in mode %s\n",
+			       __func__, amd_pstate_get_mode_string(modes[m]));
+			ret = -EINVAL;
+			goto out;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(expected_freq_attrs); i++) {
+			bool expected, found;
+
+			switch (expected_freq_attrs[i].category) {
+			case ATTR_ALWAYS:
+				expected = true;
+				break;
+			case ATTR_PREFCORE:
+				expected = has_prefcore;
+				break;
+			case ATTR_EPP:
+				expected = (modes[m] == AMD_PSTATE_ACTIVE);
+				break;
+			case ATTR_FLOOR_FREQ:
+				expected = has_floor_freq;
+				break;
+			default:
+				expected = false;
+				break;
+			}
+
+			found = attr_in_driver(driver_attrs,
+					       expected_freq_attrs[i].name);
+
+			if (expected != found) {
+				pr_err("%s: mode %s: attr %s expected %s but is %s\n",
+				       __func__,
+				       amd_pstate_get_mode_string(modes[m]),
+				       expected_freq_attrs[i].name,
+				       expected ? "visible" : "hidden",
+				       found ? "visible" : "hidden");
+				ret = -EINVAL;
+				goto out;
+			}
+		}
+	}
+
+	ret = 0;
+out:
+	amd_pstate_set_mode(orig_mode);
+	return ret;
+}
+
 static int __init amd_pstate_ut_init(void)
 {
 	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 5eae74a67aeb..ed9fd4155a25 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1390,6 +1390,14 @@ static struct freq_attr_visibility amd_pstate_attr_visibility[] = {
 	{&amd_pstate_floor_count, floor_freq_visibility},
 };
 
+struct freq_attr **amd_pstate_get_current_attrs(void)
+{
+	if (!current_pstate_driver)
+		return NULL;
+	return current_pstate_driver->attr;
+}
+EXPORT_SYMBOL_GPL(amd_pstate_get_current_attrs);
+
 static struct freq_attr **get_freq_attrs(void)
 {
 	bool attr_visible[ARRAY_SIZE(amd_pstate_attr_visibility)];
diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
index 453adfb445f8..faead0b19a8a 100644
--- a/drivers/cpufreq/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -134,4 +134,8 @@ const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
 int amd_pstate_get_status(void);
 int amd_pstate_update_status(const char *buf, size_t size);
 
+struct freq_attr;
+
+struct freq_attr **amd_pstate_get_current_attrs(void);
+
 #endif /* _LINUX_AMD_PSTATE_H */
-- 
2.34.1

Re: [PATCH v4 09/12] amd-pstate-ut: Add a testcase to validate the visibility of driver attributes
Posted by Mario Limonciello 1 week ago

On 3/26/26 06:47, Gautham R. Shenoy wrote:
> amd-pstate driver has per-attribute visibility functions to
> dynamically control which sysfs freq_attrs are exposed based on the
> platform capabilities and the current amd_pstate mode. However, there
> is no test coverage to validate that the driver's live attribute list
> matches the expected visibility for each mode.
> 
> Add amd_pstate_ut_check_freq_attrs() to the amd-pstate unit test
> module. For each enabled mode (passive, active, guided), the test
> independently derives the expected visibility of each attribute:
>    - Core attributes (max_freq, lowest_nonlinear_freq, highest_perf)
>      are always expected.
>    - Prefcore attributes (prefcore_ranking, hw_prefcore) are expected
>      only when cpudata->hw_prefcore indicates platform support.
>    - EPP attributes (energy_performance_preference,
>      energy_performance_available_preferences) are expected only in
>      active mode.
>    - Floor frequency attributes (floor_freq, floor_count) are expected
>      only when X86_FEATURE_CPPC_PERF_PRIO is present.
> 
> Compare these independent expectations against the live driver's attr
> array, catching bugs such as attributes leaking into wrong modes or
> visibility functions checking incorrect conditions.
> 
> Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/cpufreq/amd-pstate-ut.c | 139 ++++++++++++++++++++++++++++++--
>   drivers/cpufreq/amd-pstate.c    |   8 ++
>   drivers/cpufreq/amd-pstate.h    |   4 +
>   3 files changed, 146 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index 3dcdf56883a6..1f62ab6438b4 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -23,6 +23,8 @@
>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>   
>   #include <linux/bitfield.h>
> +#include <linux/cpufeature.h>
> +#include <linux/cpufreq.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/moduleparam.h>
> @@ -53,13 +55,15 @@ 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_check_driver(u32 index);
> +static int amd_pstate_ut_check_freq_attrs(u32 index);
>   
>   static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
> -	{"amd_pstate_ut_acpi_cpc_valid",   amd_pstate_ut_acpi_cpc_valid   },
> -	{"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_check_driver",	   amd_pstate_ut_check_driver     }
> +	{"amd_pstate_ut_acpi_cpc_valid",    amd_pstate_ut_acpi_cpc_valid   },
> +	{"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_check_driver",      amd_pstate_ut_check_driver     },
> +	{"amd_pstate_ut_check_freq_attrs",  amd_pstate_ut_check_freq_attrs },
>   };
>   
>   static bool test_in_list(const char *list, const char *name)
> @@ -293,6 +297,131 @@ static int amd_pstate_ut_check_driver(u32 index)
>   	return ret;
>   }
>   
> +enum attr_category {
> +	ATTR_ALWAYS,
> +	ATTR_PREFCORE,
> +	ATTR_EPP,
> +	ATTR_FLOOR_FREQ,
> +};
> +
> +static const struct {
> +	const char	*name;
> +	enum attr_category category;
> +} expected_freq_attrs[] = {
> +	{"amd_pstate_max_freq",				ATTR_ALWAYS},
> +	{"amd_pstate_lowest_nonlinear_freq",		ATTR_ALWAYS},
> +	{"amd_pstate_highest_perf",			ATTR_ALWAYS},
> +	{"amd_pstate_prefcore_ranking",			ATTR_PREFCORE},
> +	{"amd_pstate_hw_prefcore",			ATTR_PREFCORE},
> +	{"energy_performance_preference",		ATTR_EPP},
> +	{"energy_performance_available_preferences",	ATTR_EPP},
> +	{"amd_pstate_floor_freq",			ATTR_FLOOR_FREQ},
> +	{"amd_pstate_floor_count",			ATTR_FLOOR_FREQ},
> +};
> +
> +static bool attr_in_driver(struct freq_attr **driver_attrs, const char *name)
> +{
> +	int j;
> +
> +	for (j = 0; driver_attrs[j]; j++) {
> +		if (!strcmp(driver_attrs[j]->attr.name, name))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/*
> + * Verify that for each mode the driver's live ->attr array contains exactly
> + * the attributes that should be visible.  Expected visibility is derived
> + * independently from hw_prefcore, cpu features, and the current mode —
> + * not from the driver's own visibility functions.
> + */
> +static int amd_pstate_ut_check_freq_attrs(u32 index)
> +{
> +	enum amd_pstate_mode orig_mode = amd_pstate_get_status();
> +	static const enum amd_pstate_mode modes[] = {
> +		AMD_PSTATE_PASSIVE, AMD_PSTATE_ACTIVE, AMD_PSTATE_GUIDED,
> +	};
> +	bool has_prefcore, has_floor_freq;
> +	int m, i, ret;
> +
> +	has_floor_freq = cpu_feature_enabled(X86_FEATURE_CPPC_PERF_PRIO);
> +
> +	/*
> +	 * Determine prefcore support from any online CPU's cpudata.
> +	 * hw_prefcore reflects the platform-wide decision made at init.
> +	 */
> +	has_prefcore = false;
> +	for_each_online_cpu(i) {
> +		struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
> +		struct amd_cpudata *cpudata;
> +
> +		policy = cpufreq_cpu_get(i);
> +		if (!policy)
> +			continue;
> +		cpudata = policy->driver_data;
> +		has_prefcore = cpudata->hw_prefcore;
> +		break;
> +	}
> +
> +	for (m = 0; m < ARRAY_SIZE(modes); m++) {
> +		struct freq_attr **driver_attrs;
> +
> +		ret = amd_pstate_set_mode(modes[m]);
> +		if (ret)
> +			goto out;
> +
> +		driver_attrs = amd_pstate_get_current_attrs();
> +		if (!driver_attrs) {
> +			pr_err("%s: no driver attrs in mode %s\n",
> +			       __func__, amd_pstate_get_mode_string(modes[m]));
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		for (i = 0; i < ARRAY_SIZE(expected_freq_attrs); i++) {
> +			bool expected, found;
> +
> +			switch (expected_freq_attrs[i].category) {
> +			case ATTR_ALWAYS:
> +				expected = true;
> +				break;
> +			case ATTR_PREFCORE:
> +				expected = has_prefcore;
> +				break;
> +			case ATTR_EPP:
> +				expected = (modes[m] == AMD_PSTATE_ACTIVE);
> +				break;
> +			case ATTR_FLOOR_FREQ:
> +				expected = has_floor_freq;
> +				break;
> +			default:
> +				expected = false;
> +				break;
> +			}
> +
> +			found = attr_in_driver(driver_attrs,
> +					       expected_freq_attrs[i].name);
> +
> +			if (expected != found) {
> +				pr_err("%s: mode %s: attr %s expected %s but is %s\n",
> +				       __func__,
> +				       amd_pstate_get_mode_string(modes[m]),
> +				       expected_freq_attrs[i].name,
> +				       expected ? "visible" : "hidden",
> +				       found ? "visible" : "hidden");
> +				ret = -EINVAL;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	ret = 0;
> +out:
> +	amd_pstate_set_mode(orig_mode);
> +	return ret;
> +}
> +
>   static int __init amd_pstate_ut_init(void)
>   {
>   	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5eae74a67aeb..ed9fd4155a25 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1390,6 +1390,14 @@ static struct freq_attr_visibility amd_pstate_attr_visibility[] = {
>   	{&amd_pstate_floor_count, floor_freq_visibility},
>   };
>   
> +struct freq_attr **amd_pstate_get_current_attrs(void)
> +{
> +	if (!current_pstate_driver)
> +		return NULL;
> +	return current_pstate_driver->attr;
> +}
> +EXPORT_SYMBOL_GPL(amd_pstate_get_current_attrs);
> +
>   static struct freq_attr **get_freq_attrs(void)
>   {
>   	bool attr_visible[ARRAY_SIZE(amd_pstate_attr_visibility)];
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index 453adfb445f8..faead0b19a8a 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -134,4 +134,8 @@ const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
>   int amd_pstate_get_status(void);
>   int amd_pstate_update_status(const char *buf, size_t size);
>   
> +struct freq_attr;
> +
> +struct freq_attr **amd_pstate_get_current_attrs(void);
> +
>   #endif /* _LINUX_AMD_PSTATE_H */