Re: [PATCH] RFC: Make igts for cross-driver stuff mandatory?

[Sean Paul <sean@poorly.run>]

On Fri, Oct 19, 2018 at 10:50:49AM +0200, Daniel Vetter wrote:
> Hi all,
> 
> This is just to collect feedback on this idea, and see whether the
> overall dri-devel community stands on all this. I think the past few
> cross-vendor uapi extensions all came with igts attached, and
> personally I think there's lots of value in having them: A
> cross-vendor interface isn't useful if every driver implements it
> slightly differently.
> 
> I think there's 2 questions here:
> 
> - Do we want to make such testcases mandatory?
> 

Yes, more testing == better code.


> - If yes, are we there yet, or is there something crucially missing
>   still?

In my experience, no. Last week while trying to replicate an intel-gfx CI
failure, I tried compiling igt for one of my (intel) chromebooks. It seems like
cross-compilation (or, in my case, just specifying
prefix/ld_library_path/sbin_path) is broken on igt. If we want to impose
restrictions across the entire subsystem, we need to make sure that everyone can
build and deploy igt easily.

I managed to hack around everything and get it working, but I still haven't
tried switching out the toolchain. Once we have some GitLab CI to validate
cross-compilation, then we can consider making IGT mandatory.

It's possible that I'm just a meson n00b and didn't use the right incantation,
so maybe it already works, but then we need better documentation.

I've pasted my horrible hacks below, I also didn't have libunwind, so removed
its usage.

Sean


/snip


From ab8c7d274c32559295b38d6ceeaabded14b207d4 Mon Sep 17 00:00:00 2001
From: Sean Paul <seanpaul@chromium.org>
Date: Thu, 25 Oct 2018 08:40:28 -0400
Subject: [PATCH] igt: Hacks to compile in CrOS chroot

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 lib/igt_core.c            | 78 ---------------------------------------
 lib/meson.build           |  1 -
 meson.build               |  4 +-
 tests/gem_userptr_blits.c |  2 +
 tools/meson.build         |  7 ----
 5 files changed, 5 insertions(+), 87 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 23bb858f..ca65d7cc 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -71,8 +71,6 @@
 #include "igt_sysrq.h"
 #include "igt_rc.h"
 
-#define UNW_LOCAL_ONLY
-#include <libunwind.h>
 #include <elfutils/libdwfl.h>
 
 #ifdef HAVE_LIBGEN_H
@@ -1209,63 +1207,6 @@ static void write_stderr(const char *str)
 
 static void print_backtrace(void)
 {
-	unw_cursor_t cursor;
-	unw_context_t uc;
-	int stack_num = 0;
-
-	Dwfl_Callbacks cbs = {
-		.find_elf = dwfl_linux_proc_find_elf,
-		.find_debuginfo = dwfl_standard_find_debuginfo,
-	};
-
-	Dwfl *dwfl = dwfl_begin(&cbs);
-
-	if (dwfl_linux_proc_report(dwfl, getpid())) {
-		dwfl_end(dwfl);
-		dwfl = NULL;
-	} else
-		dwfl_report_end(dwfl, NULL, NULL);
-
-	igt_info("Stack trace:\n");
-
-	unw_getcontext(&uc);
-	unw_init_local(&cursor, &uc);
-	while (unw_step(&cursor) > 0) {
-		char name[255];
-		unw_word_t off, ip;
-		Dwfl_Module *mod = NULL;
-
-		unw_get_reg(&cursor, UNW_REG_IP, &ip);
-
-		if (dwfl)
-			mod = dwfl_addrmodule(dwfl, ip);
-
-		if (mod) {
-			const char *src, *dwfl_name;
-			Dwfl_Line *line;
-			int lineno;
-			GElf_Sym sym;
-
-			line = dwfl_module_getsrc(mod, ip);
-			dwfl_name = dwfl_module_addrsym(mod, ip, &sym, NULL);
-
-			if (line && dwfl_name) {
-				src = dwfl_lineinfo(line, NULL, &lineno, NULL, NULL, NULL);
-				igt_info("  #%d %s:%d %s()\n", stack_num++, src, lineno, dwfl_name);
-				continue;
-			}
-		}
-
-		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
-			igt_info("  #%d [<unknown>+0x%x]\n", stack_num++,
-				 (unsigned int) ip);
-		else
-			igt_info("  #%d [%s+0x%x]\n", stack_num++, name,
-				 (unsigned int) off);
-	}
-
-	if (dwfl)
-		dwfl_end(dwfl);
 }
 
 static const char hex[] = "0123456789abcdef";
@@ -1420,25 +1361,6 @@ xprintf(const char *fmt, ...)
 
 static void print_backtrace_sig_safe(void)
 {
-	unw_cursor_t cursor;
-	unw_context_t uc;
-	int stack_num = 0;
-
-	write_stderr("Stack trace: \n");
-
-	unw_getcontext(&uc);
-	unw_init_local(&cursor, &uc);
-	while (unw_step(&cursor) > 0) {
-		char name[255];
-		unw_word_t off;
-
-		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
-			xstrlcpy(name, "<unknown>", 10);
-
-		xprintf(" #%d [%s+0x%x]\n", stack_num++, name,
-				(unsigned int) off);
-
-	}
 }
 
 void __igt_fail_assert(const char *domain, const char *file, const int line,
diff --git a/lib/meson.build b/lib/meson.build
index 7e2c9b7a..26cef0c6 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -59,7 +59,6 @@ lib_deps = [
 	libkmod,
 	libprocps,
 	libudev,
-	libunwind,
 	libdw,
 	pciaccess,
 	pthreads,
diff --git a/meson.build b/meson.build
index eff35585..86ad6602 100644
--- a/meson.build
+++ b/meson.build
@@ -102,7 +102,6 @@ build_info += 'With libdrm: ' + ','.join(libdrm_info)
 pciaccess = dependency('pciaccess', version : '>=0.10')
 libkmod = dependency('libkmod')
 libprocps = dependency('libprocps', required : true)
-libunwind = dependency('libunwind', required : true)
 libdw = dependency('libdw', required : true)
 ssl = dependency('openssl', required : true)
 pixman = dependency('pixman-1', required : true)
@@ -217,12 +216,15 @@ prefix = get_option('prefix')
 bindir = get_option('bindir')
 datadir = join_paths(get_option('datadir'), 'igt-gpu-tools')
 includedir = get_option('includedir')
+fullincdir = join_paths(prefix, includedir)
 libdir = get_option('libdir')
 libexecdir = join_paths(get_option('libexecdir'), 'igt-gpu-tools')
 amdgpudir = join_paths(libexecdir, 'amdgpu')
 mandir = get_option('mandir')
 pkgconfigdir = join_paths(libdir, 'pkgconfig')
 
+inc = [ include_directories(fullincdir, join_paths( fullincdir, 'libdrm')), inc ]
+
 if get_option('use_rpath')
 	# Set up runpath for the test executables towards libigt.so.
 	# The path should be relative to $ORIGIN so the library is
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
index 909dd19d..92d5a6e1 100644
--- a/tests/gem_userptr_blits.c
+++ b/tests/gem_userptr_blits.c
@@ -1069,6 +1069,7 @@ static void store_dword_rand(int i915, unsigned int engine,
 
 static void test_readonly(int i915)
 {
+#if 0
 	unsigned char orig[SHA_DIGEST_LENGTH];
 	uint64_t aperture_size;
 	uint32_t whandle, rhandle;
@@ -1178,6 +1179,7 @@ static void test_readonly(int i915)
 
 	munmap(space, total);
 	munmap(pages, sz);
+#endif
 }
 
 static jmp_buf sigjmp;
diff --git a/tools/meson.build b/tools/meson.build
index 79f36aa9..5e990cd7 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -90,11 +90,6 @@ install_subdir('registers', install_dir : datadir,
 		 'Makefile', 'Makefile.in', 'Makefile.am',
 	       ])
 
-shared_library('intel_aubdump', 'aubdump.c',
-	       dependencies : [ lib_igt_chipset, dlsym ],
-	       name_prefix : '',
-	       install : true)
-
 executable('intel_gpu_top', 'intel_gpu_top.c',
 	   install : true,
 	   install_rpath : bindir_rpathdir,
@@ -104,7 +99,5 @@ conf_data = configuration_data()
 conf_data.set('prefix', prefix)
 conf_data.set('exec_prefix', '${prefix}')
 conf_data.set('libdir', join_paths('${prefix}', libdir))
-configure_file(input : 'intel_aubdump.in', output : 'intel_aubdump',
-               configuration : conf_data, install_dir : bindir)
 
 subdir('null_state_gen')
-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx