[LTP] [PATCH] rfim: add new test for verifying RFIM sysfs interface

[LTP] [PATCH] rfim: add new test for verifying RFIM sysfs interface

[Piotr Kubaj]

Validate presence and permissions of RFIM attributes.
The test checks first validity of general RFIM attributes,
and then checks either DLVR or FIVR, depending on hardware.

The test works only on Intel platforms with RFIM.

Signed-off-by: Piotr Kubaj <piotr.kubaj@intel.com>
---
 runtest/power_management_tests               |   1 +
 testcases/kernel/power_management/.gitignore |   1 +
 testcases/kernel/power_management/rfim.c     | 165 +++++++++++++++++++
 3 files changed, 167 insertions(+)
 create mode 100644 testcases/kernel/power_management/rfim.c

diff --git a/runtest/power_management_tests b/runtest/power_management_tests
index 4da57ee72..8ebbcff84 100644
--- a/runtest/power_management_tests
+++ b/runtest/power_management_tests
@@ -1,5 +1,6 @@
 #POWER_MANAGEMENT
 high_freq_hwp_cap_cppc high_freq_hwp_cap_cppc
+rfim rfim
 runpwtests03 runpwtests03.sh
 runpwtests04 runpwtests04.sh
 runpwtests06 runpwtests06.sh
diff --git a/testcases/kernel/power_management/.gitignore b/testcases/kernel/power_management/.gitignore
index 03f0c83e4..ecc2931fa 100644
--- a/testcases/kernel/power_management/.gitignore
+++ b/testcases/kernel/power_management/.gitignore
@@ -1 +1,2 @@
 high_freq_hwp_cap_cppc
+rfim
diff --git a/testcases/kernel/power_management/rfim.c b/testcases/kernel/power_management/rfim.c
new file mode 100644
index 000000000..06bc144a2
--- /dev/null
+++ b/testcases/kernel/power_management/rfim.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2026 Piotr Kubaj <piotr.kubaj@intel.com>
+ */
+
+/*\
+ * Validate presence and permissions of RFIM attributes.
+ * The test checks first validity of general RFIM attributes,
+ * and then checks either DLVR or FIVR, depending on hardware.
+ */
+
+#include "tst_test.h"
+
+#define RFIM_ROOT "/sys/bus/pci/devices/0000:00:04.0"
+
+enum rfim_variant {
+	RFIM_FIVR,
+	RFIM_DLVR,
+};
+
+static enum rfim_variant variant;
+
+static void setup(void)
+{
+	struct stat stats;
+
+	if (!stat(RFIM_ROOT"/dlvr", &stats)) {
+		if (S_ISDIR(stats.st_mode))
+			variant = RFIM_DLVR;
+		else
+			tst_brk(TBROK, "%s exists but is not a directory", RFIM_ROOT"/dlvr");
+	} else if (!stat(RFIM_ROOT"/fivr", &stats)) {
+		if (S_ISDIR(stats.st_mode))
+			variant = RFIM_FIVR;
+		else
+			tst_brk(TBROK, "%s exists but is not a directory", RFIM_ROOT"/fivr");
+	} else
+		tst_brk(TCONF, "Neither %s nor %s exists", RFIM_ROOT"/dlvr", RFIM_ROOT"/fivr");
+}
+
+static void check_read_only(const char *path)
+{
+	tst_res(TDEBUG, "Checking whether %s is read-only", path);
+
+	if (access(path, F_OK)) {
+		tst_res(TFAIL | TERRNO, "%s does not exist", path);
+		return;
+	}
+
+	int fd = open(path, O_RDONLY);
+
+	if (fd == -1) {
+		tst_res(TFAIL | TERRNO, "%s can't be read", path);
+		return;
+	}
+	close(fd);
+
+	fd = open(path, O_WRONLY);
+	if (fd != -1) {
+		close(fd);
+		tst_res(TFAIL, "%s is writable", path);
+		return;
+	}
+
+	tst_res(TPASS, "%s is read-only", path);
+}
+
+static void check_read_write(const char *path)
+{
+	tst_res(TDEBUG, "Checking whether %s is read-write", path);
+
+	if (access(path, F_OK)) {
+		tst_res(TFAIL | TERRNO, "%s does not exist", path);
+		return;
+	}
+
+	int fd = open(path, O_RDWR);
+
+	if (fd != -1) {
+		close(fd);
+		tst_res(TPASS, "%s is read-write", path);
+	} else
+		tst_res(TFAIL | TERRNO, "%s is not read-write", path);
+}
+
+static void run(void)
+{
+	const char * const fivr_nodes[] = {
+		RFIM_ROOT"/fivr/vco_ref_code_lo",
+		RFIM_ROOT"/fivr/vco_ref_code_hi",
+		RFIM_ROOT"/fivr/spread_spectrum_pct",
+		RFIM_ROOT"/fivr/spread_spectrum_clk_enable",
+		RFIM_ROOT"/fivr/rfi_vco_ref_code",
+		RFIM_ROOT"/fivr/fivr_fffc_rev",
+		NULL
+	};
+
+	const char * const ro_general_nodes[] = {
+		RFIM_ROOT"/dvfs/ddr_data_rate_point_0",
+		RFIM_ROOT"/dvfs/ddr_data_rate_point_1",
+		RFIM_ROOT"/dvfs/ddr_data_rate_point_2",
+		RFIM_ROOT"/dvfs/ddr_data_rate_point_3",
+		NULL
+	};
+
+	const char * const ro_dlvr_nodes[] = {
+		RFIM_ROOT"/dlvr/dlvr_hardware_rev",
+		RFIM_ROOT"/dlvr/dlvr_freq_mhz",
+		RFIM_ROOT"/dlvr/dlvr_pll_busy",
+		NULL
+	};
+
+	const char * const rw_dlvr_nodes[] = {
+		RFIM_ROOT"/dlvr/dlvr_freq_select",
+		RFIM_ROOT"/dlvr/dlvr_rfim_enable",
+		RFIM_ROOT"/dlvr/dlvr_spread_spectrum_pct",
+		RFIM_ROOT"/dlvr/dlvr_control_mode",
+		RFIM_ROOT"/dlvr/dlvr_control_lock",
+		NULL
+	};
+
+	const char * const rw_general_nodes[] = {
+		RFIM_ROOT"/dvfs/rfi_restriction_run_busy",
+		RFIM_ROOT"/dvfs/rfi_restriction_err_code",
+		RFIM_ROOT"/dvfs/rfi_restriction_data_rate_base",
+		RFIM_ROOT"/dvfs/rfi_restriction_data_rate",
+		NULL
+	};
+
+	for (int i = 0; ro_general_nodes[i]; i++)
+		check_read_only(ro_general_nodes[i]);
+
+	for (int i = 0; rw_general_nodes[i]; i++)
+		check_read_write(rw_general_nodes[i]);
+
+	if (variant == RFIM_DLVR) {
+		tst_res(TINFO, "Checking DLVR");
+		for (int i = 0; ro_dlvr_nodes[i]; i++)
+			check_read_only(ro_dlvr_nodes[i]);
+
+		for (int i = 0; rw_dlvr_nodes[i]; i++)
+			check_read_write(rw_dlvr_nodes[i]);
+	} else {
+		tst_res(TINFO, "Checking FIVR");
+		for (int i = 0; fivr_nodes[i]; i++)
+			check_read_write(fivr_nodes[i]);
+	}
+}
+
+static struct tst_test test = {
+	.min_kver = "6.4",
+	.needs_cpu_vendor = "GenuineIntel",
+	.needs_kconfigs = (const char *const []) {
+		"CONFIG_INT340X_THERMAL",
+		NULL
+	},
+	.needs_root = 1,
+	.supported_archs = (const char *const []) {
+		"x86",
+		"x86_64",
+		NULL
+	},
+	.setup = setup,
+	.test_all = run
+};
-- 
2.47.3

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] rfim: add new test for verifying RFIM sysfs interface

[linuxtestproject.agent]

Hi Piotr,

On Tue, 16 Jun 2026, Piotr Kubaj wrote:
> rfim: add new test for verifying RFIM sysfs interface

> /*\
>  * Validate presence and permissions of RFIM attributes.
>  * The test checks first validity of general RFIM attributes,
>  * and then checks either DLVR or FIVR, depending on hardware.
>  */

The test sets .needs_root = 1 but the doc comment does not explain
why root is required. Other tests in the same directory document
this (e.g. "The test needs root because reading /dev/cpu/N/msr
needs CAP_SYS_RAWIO / root."). Could a line be added explaining
that root is needed for write-access checks on sysfs attributes?

> } else if (!stat(RFIM_ROOT"/fivr", &stats)) {
> 	if (S_ISDIR(stats.st_mode))
> 		variant = RFIM_FIVR;
> 	else
> 		tst_brk(TBROK, "%s exists but is not a directory", RFIM_ROOT"/fivr");
> } else
> 	tst_brk(TCONF, "Neither %s nor %s exists", RFIM_ROOT"/dlvr", RFIM_ROOT"/fivr");

The first two branches of this if/else-if/else chain use braces but
the final else does not. Kernel coding style requires braces on all
branches when any branch needs them.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] rfim: add new test for verifying RFIM sysfs interface

[Andrea Cervesato via ltp]

Hi Piotr,

> diff --git a/runtest/power_management_tests b/runtest/power_management_tests
> index 4da57ee72..8ebbcff84 100644
> --- a/runtest/power_management_tests
> +++ b/runtest/power_management_tests
> @@ -1,5 +1,6 @@
>  #POWER_MANAGEMENT
>  high_freq_hwp_cap_cppc high_freq_hwp_cap_cppc
> +rfim rfim

Use rfim01 instead. We might want to add more tests in the future.

>  runpwtests03 runpwtests03.sh
>  runpwtests04 runpwtests04.sh
>  runpwtests06 runpwtests06.sh
> diff --git a/testcases/kernel/power_management/.gitignore b/testcases/kernel/power_management/.gitignore
> index 03f0c83e4..ecc2931fa 100644
> --- a/testcases/kernel/power_management/.gitignore
> +++ b/testcases/kernel/power_management/.gitignore
> @@ -1 +1,2 @@
>  high_freq_hwp_cap_cppc
> +rfim
> diff --git a/testcases/kernel/power_management/rfim.c b/testcases/kernel/power_management/rfim.c
> new file mode 100644
> index 000000000..06bc144a2
> --- /dev/null
> +++ b/testcases/kernel/power_management/rfim.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2026 Piotr Kubaj <piotr.kubaj@intel.com>
> + */
> +
> +/*\
> + * Validate presence and permissions of RFIM attributes.
> + * The test checks first validity of general RFIM attributes,
> + * and then checks either DLVR or FIVR, depending on hardware.
> + */
> +
> +#include "tst_test.h"
> +
> +#define RFIM_ROOT "/sys/bus/pci/devices/0000:00:04.0"
> +
> +enum rfim_variant {
> +	RFIM_FIVR,
> +	RFIM_DLVR,
> +};
> +
> +static enum rfim_variant variant;
> +
> +static void setup(void)
> +{
> +	struct stat stats;
> +
> +	if (!stat(RFIM_ROOT"/dlvr", &stats)) {
> +		if (S_ISDIR(stats.st_mode))
> +			variant = RFIM_DLVR;
> +		else
> +			tst_brk(TBROK, "%s exists but is not a directory", RFIM_ROOT"/dlvr");
> +	} else if (!stat(RFIM_ROOT"/fivr", &stats)) {
> +		if (S_ISDIR(stats.st_mode))
> +			variant = RFIM_FIVR;
> +		else
> +			tst_brk(TBROK, "%s exists but is not a directory", RFIM_ROOT"/fivr");
> +	} else
> +		tst_brk(TCONF, "Neither %s nor %s exists", RFIM_ROOT"/dlvr", RFIM_ROOT"/fivr");

Avoid too many nesting. Use if() -> TBROK or TCONF, instead of having else
everywhere. First, handle the error, then proceed with the test.

> +}
> +
> +static void check_read_only(const char *path)
> +{
> +	tst_res(TDEBUG, "Checking whether %s is read-only", path);
> +
> +	if (access(path, F_OK)) {
> +		tst_res(TFAIL | TERRNO, "%s does not exist", path);
> +		return;
> +	}
> +
> +	int fd = open(path, O_RDONLY);
> +
> +	if (fd == -1) {
> +		tst_res(TFAIL | TERRNO, "%s can't be read", path);
> +		return;
> +	}
> +	close(fd);
> +
> +	fd = open(path, O_WRONLY);
> +	if (fd != -1) {
> +		close(fd);
> +		tst_res(TFAIL, "%s is writable", path);
> +		return;
> +	}
> +
> +	tst_res(TPASS, "%s is read-only", path);
> +}

This function can be easily replaced by access().

TST_EXP_PASS(access(path, R_OK));
TST_EXP_FAIL(access(path, W_OK), ..);

> +
> +static void check_read_write(const char *path)
> +{
> +	tst_res(TDEBUG, "Checking whether %s is read-write", path);
> +
> +	if (access(path, F_OK)) {
> +		tst_res(TFAIL | TERRNO, "%s does not exist", path);

TERRNO already show the message. Either you use TERRNO or you specify
a message error by hand, not both. This is valid in general for all
other messages as well.

> +		return;
> +	}
> +
> +	int fd = open(path, O_RDWR);
> +
> +	if (fd != -1) {
> +		close(fd);
> +		tst_res(TPASS, "%s is read-write", path);
> +	} else
> +		tst_res(TFAIL | TERRNO, "%s is not read-write", path);
> +}

This one as well can be replaced with access().

> +
> +static void run(void)
> +{
> +	const char * const fivr_nodes[] = {
> +		RFIM_ROOT"/fivr/vco_ref_code_lo",
> +		RFIM_ROOT"/fivr/vco_ref_code_hi",
> +		RFIM_ROOT"/fivr/spread_spectrum_pct",
> +		RFIM_ROOT"/fivr/spread_spectrum_clk_enable",
> +		RFIM_ROOT"/fivr/rfi_vco_ref_code",
> +		RFIM_ROOT"/fivr/fivr_fffc_rev",
> +		NULL
> +	};
> +
> +	const char * const ro_general_nodes[] = {
> +		RFIM_ROOT"/dvfs/ddr_data_rate_point_0",
> +		RFIM_ROOT"/dvfs/ddr_data_rate_point_1",
> +		RFIM_ROOT"/dvfs/ddr_data_rate_point_2",
> +		RFIM_ROOT"/dvfs/ddr_data_rate_point_3",
> +		NULL
> +	};
> +
> +	const char * const ro_dlvr_nodes[] = {
> +		RFIM_ROOT"/dlvr/dlvr_hardware_rev",
> +		RFIM_ROOT"/dlvr/dlvr_freq_mhz",
> +		RFIM_ROOT"/dlvr/dlvr_pll_busy",
> +		NULL
> +	};
> +
> +	const char * const rw_dlvr_nodes[] = {
> +		RFIM_ROOT"/dlvr/dlvr_freq_select",
> +		RFIM_ROOT"/dlvr/dlvr_rfim_enable",
> +		RFIM_ROOT"/dlvr/dlvr_spread_spectrum_pct",
> +		RFIM_ROOT"/dlvr/dlvr_control_mode",
> +		RFIM_ROOT"/dlvr/dlvr_control_lock",
> +		NULL
> +	};
> +
> +	const char * const rw_general_nodes[] = {
> +		RFIM_ROOT"/dvfs/rfi_restriction_run_busy",
> +		RFIM_ROOT"/dvfs/rfi_restriction_err_code",
> +		RFIM_ROOT"/dvfs/rfi_restriction_data_rate_base",
> +		RFIM_ROOT"/dvfs/rfi_restriction_data_rate",
> +		NULL
> +	};

Please define them out as static arrays.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp