| Message ID | 20171213125816.18371-4-petri.latvala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Petri Latvala (2017-12-13 12:58:15) > From: Antonio Argenziano <antonio.argenziano@intel.com> > > 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. It barely scratch the surfaces of what could be done to feed in a bad address. > > v2 (Petri): Split the directory changes to separate commits, fix > indentation > > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com> > Signed-off-by: Petri Latvala <petri.latvala@intel.com> > --- > tests/hw-tests/gem_bad_address.c | 69 +++++++++++++++++++++++----------------- > 1 file changed, 39 insertions(+), 30 deletions(-) > > diff --git a/tests/hw-tests/gem_bad_address.c b/tests/hw-tests/gem_bad_address.c > index a970dfa4..2d6112bd 100644 > --- a/tests/hw-tests/gem_bad_address.c > +++ b/tests/hw-tests/gem_bad_address.c > @@ -23,37 +23,53 @@ > * Authors: > * Eric Anholt <eric@anholt.net> > * Jesse Barnes <jbarnes@virtuousgeek.org> (based on gem_bad_blit.c) > + * Antonio Argenziano <antonio.argenziano@intel.com> > * > */ > > #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. igt provides the facilities to detect GPU hangs. Wrap in igt_fork_hang_detector, finish with gem_sync. Actual machine death sadly requires an outside body. > + * > + * 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; That is asking to turn off the HW validator (for the most part), and requires drm_open_driver_master(). But since you are doing something illegal in a context that allows you to shoot yourself in the foot, what's the point? What does it prove? > - intel_batchbuffer_flush(batch); > + obj.handle = gem_create(fd, 4096); > + > + batch[i++] = MI_STORE_DWORD_IMM | MI_MEM_VIRTUAL; Note that MI_STORE_DWORD doesn't even need a bad address to take over the machine on some platforms. You should start with a validation that a known good store works, before testing whether a bad one is skipped. -Chris
diff --git a/tests/hw-tests/gem_bad_address.c b/tests/hw-tests/gem_bad_address.c index a970dfa4..2d6112bd 100644 --- a/tests/hw-tests/gem_bad_address.c +++ b/tests/hw-tests/gem_bad_address.c @@ -23,37 +23,53 @@ * Authors: * Eric Anholt <eric@anholt.net> * Jesse Barnes <jbarnes@virtuousgeek.org> (based on gem_bad_blit.c) + * Antonio Argenziano <antonio.argenziano@intel.com> * */ #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 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); } igt_simple_main @@ -62,14 +78,7 @@ igt_simple_main fd = drm_open_driver(DRIVER_INTEL); - bufmgr = drm_intel_bufmgr_gem_init(fd, 4096); - drm_intel_bufmgr_gem_enable_reuse(bufmgr); - batch = intel_batchbuffer_alloc(bufmgr, intel_get_drm_devid(fd)); - - bad_store(); - - intel_batchbuffer_free(batch); - drm_intel_bufmgr_destroy(bufmgr); + bad_store(fd, I915_EXEC_BLT); close(fd); }