If this ends up having further revisions, please split it into two
(three?) separate changes:
1) Introduction of tests_hw directory
2) Moving (and changing) gem_bad_address to tests_hw
[3) Changing gem_bad_address]
First target of bikeshedding is the directory name. I kind of like
hw_tests more, it flows nicely with BUILD_HW_TESTS and pals, but I'm
not too fuzzed about that.
More comments inline.
On Wed, May 31, 2017 at 10:00:37AM -0700, Antonio Argenziano wrote:
> When writing to an invalid memory location, the HW should be clever
> enough to silently discard the write without disrupting execution.
> gem_bad_address aim at just that. The test has been updated to move away
> from the libDrm wrappers and use the IOCTL wrappers instead. Also the
> invalid address has been updated to be just outside of the GTT space.
>
> Since the test is considered to be HW focused, meaning that it doesn't
> really exercise the deriver but rather an HW feature, a new folder has
> been created to host such tests. The commit moves the test in the newly
> created folder for HW focused tests.
>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@xxxxxxxxx>
> ---
> Makefile.am | 4 ++
> configure.ac | 11 ++++++
> tests_hw/Makefile.am | 36 ++++++++++++++++++
> tests_hw/Makefile.sources | 8 ++++
> {tests => tests_hw}/gem_bad_address.c | 69 ++++++++++++++++++++---------------
> 5 files changed, 98 insertions(+), 30 deletions(-)
> create mode 100644 tests_hw/Makefile.am
> create mode 100644 tests_hw/Makefile.sources
> rename {tests => tests_hw}/gem_bad_address.c (50%)
>
> diff --git a/Makefile.am b/Makefile.am
> index 60168628..dca76bf0 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,6 +23,10 @@ ACLOCAL_AMFLAGS = ${ACLOCAL_FLAGS} -I m4
>
> SUBDIRS = lib man tools scripts benchmarks
>
> +if BUILD_HW_TESTS
> +SUBDIRS += tests_hw
> +endif
> +
> if BUILD_TESTS
> SUBDIRS += tests
> endif
> diff --git a/configure.ac b/configure.ac
> index 5342e33c..85885f25 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -333,6 +333,15 @@ fi
> AM_CONDITIONAL(BUILD_TESTS, [test "x$BUILD_TESTS" = xyes])
> AC_DEFINE_UNQUOTED(TARGET_CPU_PLATFORM, ["$host_cpu"], [Target platform])
>
> +AC_ARG_ENABLE(tests_hw,
> + AS_HELP_STRING([--disable-hw-tests],
> + [Disable HW tests build (default: enabled)]),
> + [BUILD_HW_TESTS=$enableval], [BUILD_HW_TESTS="yes"])
> +if test "x$BUILD_TESTS" = xyes; then
> + AC_DEFINE(BUILD_HW_TESTS, 1, [Build hw tests])
> +fi
> +AM_CONDITIONAL(BUILD_HW_TESTS, [test "x$BUILD_HW_TESTS" = xyes])
> +
> files="broadwell cherryview haswell ivybridge sandybridge valleyview skylake"
> for file in $files; do
> REGISTER_FILES="$REGISTER_FILES $file `cat $srcdir/tools/registers/$file`"
> @@ -353,6 +362,7 @@ AC_CONFIG_FILES([
> man/Makefile
> scripts/Makefile
> tests/Makefile
> + tests_hw/Makefile
> tools/Makefile
> tools/null_state_gen/Makefile
> tools/registers/Makefile
> @@ -376,6 +386,7 @@ echo "Intel GPU tools"
> echo ""
> echo " • Tests:"
> echo " Build tests : ${BUILD_TESTS}"
> +echo " Build hw tests : ${BUILD_HW_TESTS}"
> echo " Compile prime tests: ${NOUVEAU}"
> echo " Print stack traces : ${with_libunwind}"
> echo " Debug flags : ${DEBUG_CFLAGS}"
> diff --git a/tests_hw/Makefile.am b/tests_hw/Makefile.am
> new file mode 100644
> index 00000000..36ed9eb9
> --- /dev/null
> +++ b/tests_hw/Makefile.am
> @@ -0,0 +1,36 @@
> +include Makefile.sources
> +
> +if BUILD_HW_TESTS
> +
> +hw-test-list.txt: Makefile.sources
> + @echo TESTLIST > $@
> + @echo ${hw_tests_all} >> $@
> + @echo TESTLIST >> $@
> +
> +all-local: .gitignore
> +.gitignore: Makefile.sources
> + @echo "$(HW_TESTS_PROGS) hw-test-list.txt .gitignore" | sed 's/\s\+/\n/g' | sort > $@
> +
> +pkglibexec_PROGRAMS = \
> + $(HW_TESTS_PROGS) \
> + $(NULL)
> +
> +pkgdata_DATA = hw-test-list.txt
> +
> +CLEANFILES = hw-test-list.txt .gitignore
> +
> +AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) -Wno-unused-result $(DEBUG_CFLAGS)\
> + -I$(srcdir)/.. \
> + -I$(srcdir)/../lib \
> + -include "$(srcdir)/../lib/check-ndebug.h" \
> + -DIGT_SRCDIR=\""$(abs_srcdir)"\" \
> + -DIGT_DATADIR=\""$(pkgdatadir)"\" \
> + $(LIBUNWIND_CFLAGS) $(WERROR_CFLAGS) \
> + $(NULL)
> +
> +LDADD = ../lib/libintel_tools.la $(GLIB_LIBS) $(XMLRPC_LIBS) $(DLOPEN_LIBS)
> +
> +AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS) $(GLIB_CFLAGS)
> +AM_LDFLAGS = -Wl,--as-needed
> +
> +endif
> diff --git a/tests_hw/Makefile.sources b/tests_hw/Makefile.sources
> new file mode 100644
> index 00000000..25d181c0
> --- /dev/null
> +++ b/tests_hw/Makefile.sources
> @@ -0,0 +1,8 @@
> +HW_TESTS_PROGS = \
> + gem_bad_address \
> + $(NULL)
> +
> +#This target contains all testcases
> +hw_tests_all = \
> + $(HW_TESTS_PROGS) \
> + $(NULL)
> diff --git a/tests/gem_bad_address.c b/tests_hw/gem_bad_address.c
> similarity index 50%
> rename from tests/gem_bad_address.c
> rename to tests_hw/gem_bad_address.c
> index a970dfa4..c254f894 100644
> --- a/tests/gem_bad_address.c
> +++ b/tests_hw/gem_bad_address.c
> @@ -23,37 +23,53 @@
> * Authors:
> * Eric Anholt <eric@xxxxxxxxxx>
> * Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> (based on gem_bad_blit.c)
> + * Antonio Argenziano <antonio.argenziano@xxxxxxxxx>
> *
> */
>
> #include "igt.h"
> -#include <stdlib.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <fcntl.h>
> -#include <inttypes.h>
> -#include <errno.h>
> -#include <sys/stat.h>
> -#include <sys/time.h>
> -#include "drm.h"
> -#include "intel_bufmgr.h"
>
> -static drm_intel_bufmgr *bufmgr;
> -struct intel_batchbuffer *batch;
> -
> -#define BAD_GTT_DEST ((512*1024*1024)) /* past end of aperture */
> +/*
> + * This test aims at verifying that writing to an invalid location in memory,
> + * doesn't cause hangs. The store command should be ignored completely by the
> + * HW and the whole process should be transparent to the user. Therefore,
> + * the test doesn't perform any validation check but expects the wrapping
> + * execution environment to check no hangs have occurred.
The situation today is that there is no execution environment to even
run this test when it's in tests_hw. Should probably mark the lack of
such an environment as a TODO here.
When this test fails, what actually happens? Hard system hang?
Recoverable gpu hang? Unrecoverable gpu hang?
> + *
> + * The test needs to send a privileged batch to be able to write to the GTT.
> + */
>
> static void
> -bad_store(void)
> +bad_store(uint32_t fd, uint32_t engine)
> {
> - BEGIN_BATCH(4, 0);
> - OUT_BATCH(MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL | 1 << 21);
> - OUT_BATCH(0);
> - OUT_BATCH(BAD_GTT_DEST);
> - OUT_BATCH(0xdeadbeef);
> - ADVANCE_BATCH();
> + struct drm_i915_gem_exec_object2 obj;
> + struct drm_i915_gem_execbuffer2 execbuf;
> +
> + uint32_t batch[16];
> + int i = 0;
> +
> + memset(&obj, 0, sizeof(obj));
> + memset(&execbuf, 0, sizeof(execbuf));
> +
> + execbuf.buffers_ptr = to_user_pointer(&obj);
> + execbuf.buffer_count = 1;
> + execbuf.flags = engine;
> + execbuf.flags |= I915_EXEC_SECURE;
>
> - intel_batchbuffer_flush(batch);
> + obj.handle = gem_create(fd, 4096);
> +
> + batch[i++] = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL;
> + batch[i++] = 0x0; //Low part of the GTT address = 4GByte
> + batch[i++] = 0x1; //High part of the GTT address > GTT size
> + batch[i++] = 0xdeadbeef;
> +
> + batch[i++] = MI_BATCH_BUFFER_END;
> + batch[i++] = 0x0;
> +
> + gem_write(fd, obj.handle, 0, batch, sizeof(batch));
> + gem_execbuf(fd, &execbuf);
> +
> + gem_close(fd, obj.handle);
> }
Use tabs for indentation here.
--
Petri Latvala
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
