[Intel-gfx] [PATCH i-g-t] tests/drm_hw_lock: Tests for hw_lock fixes.
Antoine, Peter
peter.antoine at intel.com
Tue Apr 28 01:38:44 PDT 2015
Thanks for the review, new patch inbound.
-----Original Message-----
From: Thomas Wood [mailto:thomas.wood at intel.com]
Sent: Monday, April 27, 2015 4:25 PM
To: Antoine, Peter
Cc: Intel Graphics Development; airlied at redhat.com; dri-devel at lists.freedesktop.org; Daniel Vetter
Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/drm_hw_lock: Tests for hw_lock fixes.
On 23 April 2015 at 15:07, Peter Antoine <peter.antoine at intel.com> wrote:
> There are several issues with the hardware locks functions that
> stretch from kernel crashes to priority escalations. This new test
> will test the the fixes for these features.
>
> This test will cause a driver/kernel crash on un-patched kernels, the
> following patches should be applied to stop the crashes:
>
> drm: Kernel Crash in drm_unlock
> drm: Fixes unsafe deference in locks.
>
> Issue: VIZ-5485
> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> ---
> lib/ioctl_wrappers.c | 19 +++++
> lib/ioctl_wrappers.h | 1 +
> tests/Makefile.sources | 1 +
> tests/drm_hw_lock.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 228 insertions(+)
> create mode 100644 tests/drm_hw_lock.c
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c index
> 000d394..ad8b3d3 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -964,6 +964,25 @@ bool gem_has_bsd2(int fd) {
> return gem_has_enable_ring(fd,LOCAL_I915_PARAM_HAS_BSD2);
> }
> +#define I915_PARAM_HAS_LEGACY_CONTEXT 35
Please add some API documentation for this new function here.
> +bool drm_has_legacy_context(int fd)
> +{
> + int tmp = 0;
> + drm_i915_getparam_t gp;
> +
> + memset(&gp, 0, sizeof(gp));
> + gp.value = &tmp;
> + gp.param = I915_PARAM_HAS_LEGACY_CONTEXT;
> +
> + /*
> + * if legacy context param is not supported, then it's old and we
> + * can assume that the HW_LOCKS are supported.
> + */
> + if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) != 0)
> + return true;
> +
> + return tmp == 1;
> +}
> /**
> * gem_available_aperture_size:
> * @fd: open i915 drm file descriptor diff --git
> a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h index ced7ef3..3adc700
> 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -120,6 +120,7 @@ bool gem_has_bsd(int fd); bool gem_has_blt(int
> fd); bool gem_has_vebox(int fd); bool gem_has_bsd2(int fd);
> +bool drm_has_legacy_context(int fd);
> bool gem_uses_aliasing_ppgtt(int fd); int gem_available_fences(int
> fd); uint64_t gem_available_aperture_size(int fd); diff --git
> a/tests/Makefile.sources b/tests/Makefile.sources index
> 71de6de..2f69afc 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -84,6 +84,7 @@ TESTS_progs_M = \
> pm_sseu \
> prime_self_import \
> template \
> + drm_hw_lock \
Please also add the binary name to .gitignore.
> $(NULL)
>
> TESTS_progs = \
> diff --git a/tests/drm_hw_lock.c b/tests/drm_hw_lock.c new file mode
> 100644 index 0000000..aad50ba
> --- /dev/null
> +++ b/tests/drm_hw_lock.c
> @@ -0,0 +1,207 @@
> +/*
> + * Copyright © 2015 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.
> + *
> + * Authors:
> + * Peter Antoine <peter.antoine at intel.com>
> + */
> +
> +/*
> + * Testcase: Test the HW_LOCKs for correct support and non-crashing.
This would be a good sentence to use in the IGT_TEST_DESCRIPTION macro, to add a description for the test.
> + *
> + * This test will check that he hw_locks are only g_supported for
> +drivers that
> + * require that support. If it is not g_supported then the functions
> +all return
> + * the correct error code.
> + *
> + * If g_supported it will check that the functions do not crash when
> +the crash
> + * tests are used, also that one of the tests is a security level
> +escalation
> + * that should be rejected.
> + */
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <sys/ioctl.h>
> +#include <drm.h>
> +#include "drmtest.h"
> +#include "igt_core.h"
> +#include "ioctl_wrappers.h"
> +
> +#ifndef DRM_KERNEL_CONTEXT
> +# define DRM_KERNEL_CONTEXT (0)
> +#endif
> +
> +#ifndef _DRM_LOCK_HELD
> +# define _DRM_LOCK_HELD 0x80000000U /**< Hardware lock is held */
> +#endif
> +
> +#ifndef _DRM_LOCK_CONT
> +# define _DRM_LOCK_CONT 0x40000000U /**< Hardware lock is contended */
> +#endif
> +
> +static bool g_sig_fired;
> +static bool g_supported;
> +static struct sigaction old_action;
> +
> +static void sig_term_handler(int value) {
> + g_sig_fired = true;
> +}
> +
> +static bool set_term_handler(void)
> +{
> + static struct sigaction new_action;
> +
> + new_action.sa_handler = sig_term_handler;
> + sigemptyset(&new_action.sa_mask);
> + new_action.sa_flags = 0;
> +
> + igt_assert(sigaction(SIGTERM, &new_action, &old_action) == 0);
> +
> + if (old_action.sa_handler != SIG_IGN)
> + return true;
> + else
> + return false;
> +}
> +
> +static void restore_term_handler(void) {
> + sigaction(SIGTERM, &old_action, NULL); }
> +
> +static void does_lock_crash(int fd)
> +{
> + struct drm_lock rubbish;
> + int ret;
> +
> + memset(&rubbish, 'A', sizeof(struct drm_lock));
> +
> + g_sig_fired = false;
> + igt_assert(set_term_handler());
Since set_term_handler and restore_term_handler are called at the start and end of every subtest, they could be placed in the respective igt_fixture blocks before and after the subtests in igt_main.
> +
> + ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> +
> + igt_assert(ret == -1 && (!g_supported || g_sig_fired));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + restore_term_handler();
> +}
> +
> +static void does_unlock_crash(int fd) {
> + struct drm_lock rubbish;
> + int ret;
> +
> + g_sig_fired = false;
> + igt_assert(set_term_handler());
> +
> + memset(&rubbish, 'A', sizeof(struct drm_lock));
> +
> + ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> +
> + igt_assert(ret == -1 && (!g_supported || g_sig_fired));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + restore_term_handler();
> +}
> +
> +static void priority_escalation(int fd) {
> + struct drm_lock rubbish;
> + int ret;
> +
> + g_sig_fired = false;
> + igt_assert(set_term_handler());
g_sig_fired is not checked in these tests, so presumably it doesn't need to be set before each test?
> +
> + /* this should be rejected */
> + rubbish.context = DRM_KERNEL_CONTEXT;
> + ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> + igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + /* this should also be rejected */
> + rubbish.context = _DRM_LOCK_HELD | DRM_KERNEL_CONTEXT;
> + g_sig_fired = false;
> + ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> + igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + /* this should also be rejected */
> + rubbish.context = _DRM_LOCK_CONT | DRM_KERNEL_CONTEXT;
> + g_sig_fired = false;
> + ret = ioctl(fd, DRM_IOCTL_LOCK, &rubbish);
> + igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + /* this should be rejected */
> + rubbish.context = DRM_KERNEL_CONTEXT;
> + g_sig_fired = false;
> + ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> + igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + /* this should also be rejected */
> + rubbish.context = _DRM_LOCK_HELD | DRM_KERNEL_CONTEXT;
> + g_sig_fired = false;
> + ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> + igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + /* this should also be rejected */
> + rubbish.context = _DRM_LOCK_CONT | DRM_KERNEL_CONTEXT;
> + g_sig_fired = false;
> + ret = ioctl(fd, DRM_IOCTL_UNLOCK, &rubbish);
> + igt_assert(ret == -1 && (!g_supported || errno == EPERM));
> + igt_assert(ret == -1 && (g_supported || errno == EINVAL));
> +
> + restore_term_handler();
> +}
> +
> +igt_main
> +{
> + int fd = -1;
> +
> + g_sig_fired = false;
> + g_supported = false;
> +
> + igt_fixture {
> + fd = drm_open_any();
> + igt_assert(fd >= 0);
drm_open_any will always return a valid file descriptor. If no device is found, it will call igt_skip.
> + }
> +
> + g_supported = drm_has_legacy_context(fd);
This needs to be inside the igt_fixture block above, otherwise it may interfere with subtest enumeration.
> +
> + igt_info("HW LOCK Supported: %s\n", g_supported?"Yes":"No");
> +
> + igt_subtest("lock-crash") {
> + does_lock_crash(fd);
> + }
> +
> + igt_subtest("unlock-crash") {
> + does_unlock_crash(fd);
> + }
> +
> + igt_subtest("priority-escalation") {
> + priority_escalation(fd);
> + }
> +
> + igt_fixture
> + close(fd);
> +}
> +
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the dri-devel
mailing list