[i-g-t,v3,3/3] Convert shell script tests to C version
diff mbox

Message ID 1496739254-18898-3-git-send-email-abdiel.janulgue@linux.intel.com
State New
Headers show

Commit Message

Abdiel Janulgue June 6, 2017, 8:54 a.m. UTC
v3: Drop redundant test covered by drv_hangman/basic. Descend thru
    debugfs path when reading sysfs entries (Chris).

v2: Use internal igt_debugfs functions instead of cat and document
    debugfs tests.
    Convert sysfs_l3_parity properly.
    Rename redundant names in tests.

Converted:
 - check_drm_clients (ensures no other clients are running.
   functionality provided by drm_open_driver_master).
 - debugfs_emon_crash
 - debugfs_wedged
 - drv_debugfs_reader
 - sysfs_l3_parity
 - test_rte_check  (same as check_drm_clients)
 - tools_test
 - ZZ_check_dmesg

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 tests/Makefile.sources   |   9 +---
 tests/ZZ_check_dmesg     |  11 -----
 tests/check_drm_clients  |   6 ---
 tests/debugfs.c          |  96 +++++++++++++++++++++++++++++++++++++
 tests/debugfs_emon_crash |  16 -------
 tests/debugfs_wedged     |  10 ----
 tests/drv_debugfs_reader |   9 ----
 tests/sysfs_l3_parity    |  22 ---------
 tests/test_rte_check     |   6 ---
 tests/tools.c            | 122 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/tools_test         |  16 -------
 11 files changed, 220 insertions(+), 103 deletions(-)
 delete mode 100755 tests/ZZ_check_dmesg
 delete mode 100755 tests/check_drm_clients
 create mode 100644 tests/debugfs.c
 delete mode 100755 tests/debugfs_emon_crash
 delete mode 100755 tests/debugfs_wedged
 delete mode 100755 tests/drv_debugfs_reader
 delete mode 100755 tests/sysfs_l3_parity
 delete mode 100755 tests/test_rte_check
 create mode 100644 tests/tools.c
 delete mode 100755 tests/tools_test

Comments

Arkadiusz Hiler June 12, 2017, 11:14 a.m. UTC | #1
On Tue, Jun 06, 2017 at 11:54:14AM +0300, Abdiel Janulgue wrote:
> v3: Drop redundant test covered by drv_hangman/basic. Descend thru
>     debugfs path when reading sysfs entries (Chris).
> 
> v2: Use internal igt_debugfs functions instead of cat and document
>     debugfs tests.
>     Convert sysfs_l3_parity properly.
>     Rename redundant names in tests.
> 
> Converted:
>  - check_drm_clients (ensures no other clients are running.
>    functionality provided by drm_open_driver_master).
>  - debugfs_emon_crash
>  - debugfs_wedged
>  - drv_debugfs_reader
>  - sysfs_l3_parity
>  - test_rte_check  (same as check_drm_clients)
>  - tools_test
>  - ZZ_check_dmesg

Hey,

Doing each tool in a separate commit would help with readability.

> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> ---
>  tests/Makefile.sources   |   9 +---
>  tests/ZZ_check_dmesg     |  11 -----
>  tests/check_drm_clients  |   6 ---
>  tests/debugfs.c          |  96 +++++++++++++++++++++++++++++++++++++
>  tests/debugfs_emon_crash |  16 -------
>  tests/debugfs_wedged     |  10 ----
>  tests/drv_debugfs_reader |   9 ----
>  tests/sysfs_l3_parity    |  22 ---------
>  tests/test_rte_check     |   6 ---
>  tests/tools.c            | 122 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/tools_test         |  16 -------
>  11 files changed, 220 insertions(+), 103 deletions(-)
>  delete mode 100755 tests/ZZ_check_dmesg
>  delete mode 100755 tests/check_drm_clients
>  create mode 100644 tests/debugfs.c
>  delete mode 100755 tests/debugfs_emon_crash
>  delete mode 100755 tests/debugfs_wedged
>  delete mode 100755 tests/drv_debugfs_reader
>  delete mode 100755 tests/sysfs_l3_parity
>  delete mode 100755 tests/test_rte_check
>  create mode 100644 tests/tools.c
>  delete mode 100755 tests/tools_test
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 9553e4d..c4a78a9 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -243,6 +243,8 @@ TESTS_progs = \
>  	drv_module_reload \
>  	kms_sysfs_edid_timing \
>  	perf \
> +	debugfs \

The name clashes on AOSP build:
build/core/base_rules.mk:238: error: external/igt/tests: MODULE.TARGET.EXECUTABLES.debugfs already defined by external/e2fsprogs/debugfs.

debugfs is a name of a binary that debugs ext{2,3,4} family of FSes.

I have to look into the build system and our Android.mks more deeply as
IGTs output binaries should be stored elsewhere and the clash is rather
enigmatic.

Also are we changing names of the test? I thought it was only a rewrite.

> +	tools \
>  	$(NULL)
>  
>  # IMPORTANT: The ZZ_ tests need to be run last!
> @@ -251,11 +253,6 @@ TESTS_scripts_M = \
>  	$(NULL)
>  
>  TESTS_scripts = \
> -	debugfs_emon_crash \
> -	drv_debugfs_reader \
> -	sysfs_l3_parity \
> -	test_rte_check \
> -	tools_test \
>  	$(NULL)

TESTS_scripts now is only $(NULL), so you may as well just remove it
from the `kernel_tests` variable and get rid of it completely.

>  
>  # This target contains testcases which support automagic subtest enumeration
> @@ -317,9 +314,7 @@ HANG = \
>  	$(NULL)
>  
>  scripts = \
> -	check_drm_clients \
>  	ddx_intel_after_fbdev \
> -	debugfs_wedged \
>  	drm_lib.sh \
>  	drm_getopt.sh \
>  	$(NULL)

<SNIP>

> diff --git a/tests/debugfs.c b/tests/debugfs.c
> new file mode 100644
> index 0000000..b9ae86c
> --- /dev/null
> +++ b/tests/debugfs.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +
> +static void get_sysfs_entry(int path_fd)

I was a little bit confused by the function - we are not getting
anything from it. Just reading and discarding.

Maybe read_and_discard_sysfs_entry? A comment would also do.

> +{
> +	struct dirent *dirent;
> +	DIR *dir;
> +
> +	dir = fdopendir(path_fd);
> +	if (!dir)
> +		return;
> +
> +	while ((dirent = readdir(dir))) {
> +		if (!strcmp(dirent->d_name, ".") ||
> +		    !strcmp(dirent->d_name, ".."))
> +			continue;
> +		if (dirent->d_type == DT_DIR) {
> +			int sub_fd = -1;
> +			igt_assert((sub_fd =
> +				    openat(path_fd, dirent->d_name, O_RDONLY |
> +					   O_DIRECTORY)) > 0);
> +			get_sysfs_entry(sub_fd);
> +			close(sub_fd);
> +		} else {
> +			char *buf = igt_sysfs_get(path_fd, dirent->d_name);

igt_sysfs_get may get us NULL.
Shouldn't we assert on that? It's an error-worthy.

Also we may end up trying to free it in with the free below.

> +			free(buf);
> +		}
> +	}
> +	closedir(dir);
> +}
> +
> +igt_main
> +{
> +	int fd = -1, debugfs;
> +	igt_skip_on_simulation();
> +
> +	igt_fixture {
> +		fd = drm_open_driver_master(DRIVER_INTEL);
> +		igt_require_gem(fd);
> +		debugfs = igt_debugfs_dir(fd);
> +	}
> +
> +	igt_subtest("drv_reader") {
> +		get_sysfs_entry(debugfs);
> +	}
> +
> +	igt_subtest("emon_crash") {
> +		int i;
> +		/*
> +		 * This check if we can crash the kernel with
> +		 * segmentation-fault by reading
> +		 * /sys/kernel/debug/dri/0/i915_emon_status too quickly
> +		 */
> +		for (i = 0; i < 1000; i++) {
> +			char *buf = igt_sysfs_get(debugfs,
> +						  "i915_emon_status");
> +			free(buf);
> +		}
> +
> +		/* If we got here, we haven't crashed */
> +		igt_success();
> +	}
> +
> +	igt_fixture {
> +		close(debugfs);
> +		close(fd);
> +	}
> +}

<SNIP>

> diff --git a/tests/tools.c b/tests/tools.c
> new file mode 100644
> index 0000000..2ec951a
> --- /dev/null
> +++ b/tests/tools.c
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +#include "igt.h"
> +#include <stdio.h>
> +
> +/**
> + * Parse the r-value of a [cmd] string.
> + */
> +static bool check_cmd_return_value(const char *s, void *data)
> +{
> +	int *val = data;
> +	char *cmd, *found;
> +	const char *delim = "[cmd]";
> +	const int delim_len = strlen(delim);
> +
> +	if (!(cmd = strstr(s, delim)))
> +		return false;
> +
> +	found = cmd + delim_len + 1;
> +	igt_assert(delim_len + strlen(found) < strlen(cmd));
> +
> +	*val = atoi(found);
> +	return true;
> +}
> +
> +igt_main
> +{
> +	int fd = -1;
> +	igt_skip_on_simulation();
> +
> +	igt_subtest("sysfs_l3_parity") {
> +		int exec_return;
> +
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -r 0 -b 0 "
> +			       "-s 0 -e");
> +		igt_assert(exec_return == IGT_EXIT_SUCCESS);
> +
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -l | "
> +			       "grep -c 'Row 0, Bank 0, Subbank 0 "
> +			       "is disabled'");
> +		if (exec_return == IGT_EXIT_SUCCESS) {
> +			int val = -1;
> +			igt_log_buffer_inspect(check_cmd_return_value,
> +					       &val);
> +			igt_assert(val == 1);
> +		} else {
> +			igt_fail(IGT_EXIT_FAILURE);
> +		}
> +
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -r 0 -b 0 "
> +			       "-s 0 -e");
> +		igt_assert(exec_return == IGT_EXIT_SUCCESS);
> +
> +		/* Check that we can clear remaps */
> +		igt_system_cmd(exec_return,
> +			       "../tools/intel_l3_parity -l | "
> +			       "wc -l");
> +		if (exec_return == IGT_EXIT_SUCCESS) {
> +			int val = -1;
> +			igt_log_buffer_inspect(check_cmd_return_value,
> +					       &val);
> +			igt_assert(val == 1);
> +		} else {
> +			igt_fail(IGT_EXIT_FAILURE);
> +		}
> +	}
> +
> +	igt_subtest("tools_test") {
> +		char *cmd;
> +
> +		igt_assert(asprintf(&cmd,
> +				    "../tools/intel_reg read 0x4030")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
> +		free(cmd);
> +
> +		igt_assert(asprintf(&cmd, "../tools/intel_reg dump")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
> +		free(cmd);
> +	}
> +
> +	igt_subtest("check_dmesg") {
> +		char *cmd;
> +		igt_assert(asprintf(&cmd, "dmesg | grep '\\*ERROR\\*'")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
> +		free(cmd);
> +
> +		igt_assert(asprintf(&cmd, "dmesg | grep "
> +				    "-- '------\\[ cut here \\]----'")
> +			   != -1);
> +		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
> +		free(cmd);

Are we testing the dmesg here?

If not they why systeming out "dmesg | grep" instead of going through
/dev/kmsg?

> +	}
> +}
Abdiel Janulgue June 13, 2017, 10:22 a.m. UTC | #2
On 12.06.2017 14:14, Arkadiusz Hiler wrote:
> On Tue, Jun 06, 2017 at 11:54:14AM +0300, Abdiel Janulgue wrote:
>> v3: Drop redundant test covered by drv_hangman/basic. Descend thru
>>     debugfs path when reading sysfs entries (Chris).
>>
>> v2: Use internal igt_debugfs functions instead of cat and document
>>     debugfs tests.
>>     Convert sysfs_l3_parity properly.
>>     Rename redundant names in tests.
>>
>> Converted:
>>  - check_drm_clients (ensures no other clients are running.
>>    functionality provided by drm_open_driver_master).
>>  - debugfs_emon_crash
>>  - debugfs_wedged
>>  - drv_debugfs_reader
>>  - sysfs_l3_parity
>>  - test_rte_check  (same as check_drm_clients)
>>  - tools_test
>>  - ZZ_check_dmesg
> 
> Hey,
> 
> Doing each tool in a separate commit would help with readability.

Will do.

> 
>>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>> ---
>>  tests/Makefile.sources   |   9 +---
>>  tests/ZZ_check_dmesg     |  11 -----
>>  tests/check_drm_clients  |   6 ---
>>  tests/debugfs.c          |  96 +++++++++++++++++++++++++++++++++++++
>>  tests/debugfs_emon_crash |  16 -------
>>  tests/debugfs_wedged     |  10 ----
>>  tests/drv_debugfs_reader |   9 ----
>>  tests/sysfs_l3_parity    |  22 ---------
>>  tests/test_rte_check     |   6 ---
>>  tests/tools.c            | 122 +++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/tools_test         |  16 -------
>>  11 files changed, 220 insertions(+), 103 deletions(-)
>>  delete mode 100755 tests/ZZ_check_dmesg
>>  delete mode 100755 tests/check_drm_clients
>>  create mode 100644 tests/debugfs.c
>>  delete mode 100755 tests/debugfs_emon_crash
>>  delete mode 100755 tests/debugfs_wedged
>>  delete mode 100755 tests/drv_debugfs_reader
>>  delete mode 100755 tests/sysfs_l3_parity
>>  delete mode 100755 tests/test_rte_check
>>  create mode 100644 tests/tools.c
>>  delete mode 100755 tests/tools_test
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 9553e4d..c4a78a9 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -243,6 +243,8 @@ TESTS_progs = \
>>  	drv_module_reload \
>>  	kms_sysfs_edid_timing \
>>  	perf \
>> +	debugfs \
> 
> The name clashes on AOSP build:
> build/core/base_rules.mk:238: error: external/igt/tests: MODULE.TARGET.EXECUTABLES.debugfs already defined by external/e2fsprogs/debugfs.
> 
> debugfs is a name of a binary that debugs ext{2,3,4} family of FSes.
> 
> I have to look into the build system and our Android.mks more deeply as
> IGTs output binaries should be stored elsewhere and the clash is rather
> enigmatic.
> 
> Also are we changing names of the test? I thought it was only a rewrite.

It's just a rewrite. Now that we combine those debugfs_* scripts into
one executable, maybe just the prefix would suffice? How about
debugfs_tests.c?

> 
>> +	tools \
>>  	$(NULL)
>>  
>>  # IMPORTANT: The ZZ_ tests need to be run last!
>> @@ -251,11 +253,6 @@ TESTS_scripts_M = \
>>  	$(NULL)
>>  
>>  TESTS_scripts = \
>> -	debugfs_emon_crash \
>> -	drv_debugfs_reader \
>> -	sysfs_l3_parity \
>> -	test_rte_check \
>> -	tools_test \
>>  	$(NULL)
> 
> TESTS_scripts now is only $(NULL), so you may as well just remove it
> from the `kernel_tests` variable and get rid of it completely.
> 
>>  
>>  # This target contains testcases which support automagic subtest enumeration
>> @@ -317,9 +314,7 @@ HANG = \
>>  	$(NULL)
>>  
>>  scripts = \
>> -	check_drm_clients \
>>  	ddx_intel_after_fbdev \
>> -	debugfs_wedged \
>>  	drm_lib.sh \
>>  	drm_getopt.sh \
>>  	$(NULL)
> 
> <SNIP>
> 
>> diff --git a/tests/debugfs.c b/tests/debugfs.c
>> new file mode 100644
>> index 0000000..b9ae86c
>> --- /dev/null
>> +++ b/tests/debugfs.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +#ifdef HAVE_CONFIG_H
>> +#include "config.h"
>> +#endif
>> +#include "igt.h"
>> +#include "igt_sysfs.h"
>> +#include <fcntl.h>
>> +#include <sys/types.h>
>> +#include <dirent.h>
>> +
>> +static void get_sysfs_entry(int path_fd)
> 
> I was a little bit confused by the function - we are not getting
> anything from it. Just reading and discarding.
> 
> Maybe read_and_discard_sysfs_entry? A comment would also do.

read_and_discard_sysfs_entry() sounds better.

> 
>> +{
>> +	struct dirent *dirent;
>> +	DIR *dir;
>> +
>> +	dir = fdopendir(path_fd);
>> +	if (!dir)
>> +		return;
>> +
>> +	while ((dirent = readdir(dir))) {
>> +		if (!strcmp(dirent->d_name, ".") ||
>> +		    !strcmp(dirent->d_name, ".."))
>> +			continue;
>> +		if (dirent->d_type == DT_DIR) {
>> +			int sub_fd = -1;
>> +			igt_assert((sub_fd =
>> +				    openat(path_fd, dirent->d_name, O_RDONLY |
>> +					   O_DIRECTORY)) > 0);
>> +			get_sysfs_entry(sub_fd);
>> +			close(sub_fd);
>> +		} else {
>> +			char *buf = igt_sysfs_get(path_fd, dirent->d_name);
> 
> igt_sysfs_get may get us NULL.
> Shouldn't we assert on that? It's an error-worthy.

Yep.

> <SNIP>

>> +		igt_assert(asprintf(&cmd, "dmesg | grep "
>> +				    "-- '------\\[ cut here \\]----'")
>> +			   != -1);
>> +		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
>> +		free(cmd);
> 
> Are we testing the dmesg here?
> 
> If not they why systeming out "dmesg | grep" instead of going through
> /dev/kmsg?

Yes. This could be improved. Thanks for the review!

Patch
diff mbox

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 9553e4d..c4a78a9 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -243,6 +243,8 @@  TESTS_progs = \
 	drv_module_reload \
 	kms_sysfs_edid_timing \
 	perf \
+	debugfs \
+	tools \
 	$(NULL)
 
 # IMPORTANT: The ZZ_ tests need to be run last!
@@ -251,11 +253,6 @@  TESTS_scripts_M = \
 	$(NULL)
 
 TESTS_scripts = \
-	debugfs_emon_crash \
-	drv_debugfs_reader \
-	sysfs_l3_parity \
-	test_rte_check \
-	tools_test \
 	$(NULL)
 
 # This target contains testcases which support automagic subtest enumeration
@@ -317,9 +314,7 @@  HANG = \
 	$(NULL)
 
 scripts = \
-	check_drm_clients \
 	ddx_intel_after_fbdev \
-	debugfs_wedged \
 	drm_lib.sh \
 	drm_getopt.sh \
 	$(NULL)
diff --git a/tests/ZZ_check_dmesg b/tests/ZZ_check_dmesg
deleted file mode 100755
index e28ba35..0000000
--- a/tests/ZZ_check_dmesg
+++ /dev/null
@@ -1,11 +0,0 @@ 
-#!/bin/sh
-
-if dmesg | grep '\*ERROR\*'  > /dev/null ; then
-	echo "DRM_ERROR dirt in dmesg"
-	exit 1
-fi
-
-if dmesg | grep -- '------\[ cut here \]----' > /dev/null  ; then
-	echo "found a backtrace in dmesg"
-	exit 1
-fi
diff --git a/tests/check_drm_clients b/tests/check_drm_clients
deleted file mode 100755
index 2a891b8..0000000
--- a/tests/check_drm_clients
+++ /dev/null
@@ -1,6 +0,0 @@ 
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/debugfs.c b/tests/debugfs.c
new file mode 100644
index 0000000..b9ae86c
--- /dev/null
+++ b/tests/debugfs.c
@@ -0,0 +1,96 @@ 
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include "igt.h"
+#include "igt_sysfs.h"
+#include <fcntl.h>
+#include <sys/types.h>
+#include <dirent.h>
+
+static void get_sysfs_entry(int path_fd)
+{
+	struct dirent *dirent;
+	DIR *dir;
+
+	dir = fdopendir(path_fd);
+	if (!dir)
+		return;
+
+	while ((dirent = readdir(dir))) {
+		if (!strcmp(dirent->d_name, ".") ||
+		    !strcmp(dirent->d_name, ".."))
+			continue;
+		if (dirent->d_type == DT_DIR) {
+			int sub_fd = -1;
+			igt_assert((sub_fd =
+				    openat(path_fd, dirent->d_name, O_RDONLY |
+					   O_DIRECTORY)) > 0);
+			get_sysfs_entry(sub_fd);
+			close(sub_fd);
+		} else {
+			char *buf = igt_sysfs_get(path_fd, dirent->d_name);
+			free(buf);
+		}
+	}
+	closedir(dir);
+}
+
+igt_main
+{
+	int fd = -1, debugfs;
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		fd = drm_open_driver_master(DRIVER_INTEL);
+		igt_require_gem(fd);
+		debugfs = igt_debugfs_dir(fd);
+	}
+
+	igt_subtest("drv_reader") {
+		get_sysfs_entry(debugfs);
+	}
+
+	igt_subtest("emon_crash") {
+		int i;
+		/*
+		 * This check if we can crash the kernel with
+		 * segmentation-fault by reading
+		 * /sys/kernel/debug/dri/0/i915_emon_status too quickly
+		 */
+		for (i = 0; i < 1000; i++) {
+			char *buf = igt_sysfs_get(debugfs,
+						  "i915_emon_status");
+			free(buf);
+		}
+
+		/* If we got here, we haven't crashed */
+		igt_success();
+	}
+
+	igt_fixture {
+		close(debugfs);
+		close(fd);
+	}
+}
diff --git a/tests/debugfs_emon_crash b/tests/debugfs_emon_crash
deleted file mode 100755
index 1dbfcb2..0000000
--- a/tests/debugfs_emon_crash
+++ /dev/null
@@ -1,16 +0,0 @@ 
-#!/bin/bash
-#
-# This check if we can crash the kernel with segmentation-fault
-# by reading /sys/kernel/debug/dri/0/i915_emon_status too quickly
-#
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-for z in $(seq 1 1000); do
-	cat $i915_dfs_path/i915_emon_status > /dev/null 2&>1
-done
-
-# If we got here, we haven't crashed
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/debugfs_wedged b/tests/debugfs_wedged
deleted file mode 100755
index f15ac46..0000000
--- a/tests/debugfs_wedged
+++ /dev/null
@@ -1,10 +0,0 @@ 
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# Testcase: wedge the hw to check the error_state reading
-# 
-# Unfortunately wedged is permanent, so this test is not run by default
-echo 1 > ${i915_dfs_path}/i915_wedged
-cat $i915_dfs_path/i915_error_state > /dev/null 2>&1
diff --git a/tests/drv_debugfs_reader b/tests/drv_debugfs_reader
deleted file mode 100755
index 6ea4e64..0000000
--- a/tests/drv_debugfs_reader
+++ /dev/null
@@ -1,9 +0,0 @@ 
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# read everything we can
-cat $i915_dfs_path/* > /dev/null 2>&1
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/sysfs_l3_parity b/tests/sysfs_l3_parity
deleted file mode 100755
index d5f3284..0000000
--- a/tests/sysfs_l3_parity
+++ /dev/null
@@ -1,22 +0,0 @@ 
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e || exit $?
-
-#Check that we can remap a row
-$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -d
-disabled=`$SOURCE_DIR/../tools/intel_l3_parity -l | grep -c 'Row 0, Bank 0, Subbank 0 is disabled'`
-if [ "$disabled" != "1" ] ; then
-	echo "Fail"
-	exit $IGT_EXIT_FAILURE
-fi
-
-$SOURCE_DIR/../tools/intel_l3_parity -r 0 -b 0 -s 0 -e
-
-#Check that we can clear remaps
-if [ `$SOURCE_DIR/../tools/intel_l3_parity -l | wc -l` != 1 ] ; then
-	echo "Fail 2"
-	exit $IGT_EXIT_FAILURE
-fi
diff --git a/tests/test_rte_check b/tests/test_rte_check
deleted file mode 100755
index 2a891b8..0000000
--- a/tests/test_rte_check
+++ /dev/null
@@ -1,6 +0,0 @@ 
-#!/bin/bash
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/tools.c b/tests/tools.c
new file mode 100644
index 0000000..2ec951a
--- /dev/null
+++ b/tests/tools.c
@@ -0,0 +1,122 @@ 
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include "igt.h"
+#include <stdio.h>
+
+/**
+ * Parse the r-value of a [cmd] string.
+ */
+static bool check_cmd_return_value(const char *s, void *data)
+{
+	int *val = data;
+	char *cmd, *found;
+	const char *delim = "[cmd]";
+	const int delim_len = strlen(delim);
+
+	if (!(cmd = strstr(s, delim)))
+		return false;
+
+	found = cmd + delim_len + 1;
+	igt_assert(delim_len + strlen(found) < strlen(cmd));
+
+	*val = atoi(found);
+	return true;
+}
+
+igt_main
+{
+	int fd = -1;
+	igt_skip_on_simulation();
+
+	igt_subtest("sysfs_l3_parity") {
+		int exec_return;
+
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -r 0 -b 0 "
+			       "-s 0 -e");
+		igt_assert(exec_return == IGT_EXIT_SUCCESS);
+
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -l | "
+			       "grep -c 'Row 0, Bank 0, Subbank 0 "
+			       "is disabled'");
+		if (exec_return == IGT_EXIT_SUCCESS) {
+			int val = -1;
+			igt_log_buffer_inspect(check_cmd_return_value,
+					       &val);
+			igt_assert(val == 1);
+		} else {
+			igt_fail(IGT_EXIT_FAILURE);
+		}
+
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -r 0 -b 0 "
+			       "-s 0 -e");
+		igt_assert(exec_return == IGT_EXIT_SUCCESS);
+
+		/* Check that we can clear remaps */
+		igt_system_cmd(exec_return,
+			       "../tools/intel_l3_parity -l | "
+			       "wc -l");
+		if (exec_return == IGT_EXIT_SUCCESS) {
+			int val = -1;
+			igt_log_buffer_inspect(check_cmd_return_value,
+					       &val);
+			igt_assert(val == 1);
+		} else {
+			igt_fail(IGT_EXIT_FAILURE);
+		}
+	}
+
+	igt_subtest("tools_test") {
+		char *cmd;
+
+		igt_assert(asprintf(&cmd,
+				    "../tools/intel_reg read 0x4030")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
+		free(cmd);
+
+		igt_assert(asprintf(&cmd, "../tools/intel_reg dump")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) == IGT_EXIT_SUCCESS);
+		free(cmd);
+	}
+
+	igt_subtest("check_dmesg") {
+		char *cmd;
+		igt_assert(asprintf(&cmd, "dmesg | grep '\\*ERROR\\*'")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
+		free(cmd);
+
+		igt_assert(asprintf(&cmd, "dmesg | grep "
+				    "-- '------\\[ cut here \\]----'")
+			   != -1);
+		igt_assert(igt_system_quiet(cmd) != IGT_EXIT_SUCCESS);
+		free(cmd);
+	}
+}
diff --git a/tests/tools_test b/tests/tools_test
deleted file mode 100755
index a27fb87..0000000
--- a/tests/tools_test
+++ /dev/null
@@ -1,16 +0,0 @@ 
-#!/bin/bash
-# Test some of the most critical tools we have accidentally broken before.
-# TODO: Possibly make tests parse output
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# ARB_MODE has existed for many gens
-PATH=$SOURCE_DIR/../tools:$PATH
-do_or_die "intel_reg read 0x4030"
-do_or_die "intel_reg dump"
-
-# TODO: Add more tests
-
-exit $IGT_EXIT_SUCCESS
-