diff mbox

[RFCv2,01/19] drm/i915: Provide a hook for selftests

Message ID 20161220130814.10213-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Dec. 20, 2016, 1:07 p.m. UTC
Some pieces of code are independent of hardware but are very tricky to
exercise through the normal userspace ABI or via debugfs hooks. Being
able to create mock unit tests and execute them through CI is vital.
Start by adding a central point where we can execute unit tests and
a parameter to enable them. This is disabled by default as the
expectation is that these tests will occasionally explode.

To facilitate integration with igt, any parameter beginning with
i915.igt__ is interpreted as a subtest executable independently via
igt/drv_selftest.

Two classes of selftests are recognised: mock unit tests and integration
tests. Mock unit tests are run as soon as the module is loaded, before
the device is probed. At that point there is no driver instantiated and
all hw interactions must be "mocked". This is very useful for writing
universal tests to exercise code not typically run on a broad range of
architectures. Alternatively, you can hook into the late selftests and
run when the device has been instantiated - hw interactions are real.

v2: Add a macro for compiling conditional code for mock objects inside
real objects.
v3: Differentiate between mock unit tests and late integration test.
v4: List the tests in natural order, use igt to sort after modparam.
v5: s/late/live/

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v1
---
 drivers/gpu/drm/i915/Kconfig.debug                 |  15 ++
 drivers/gpu/drm/i915/Makefile                      |   3 +
 drivers/gpu/drm/i915/i915_pci.c                    |  19 +-
 drivers/gpu/drm/i915/i915_selftest.h               |  91 +++++++++
 .../gpu/drm/i915/selftests/i915_live_selftests.h   |  11 +
 .../gpu/drm/i915/selftests/i915_mock_selftests.h   |  11 +
 drivers/gpu/drm/i915/selftests/i915_selftest.c     | 222 +++++++++++++++++++++
 7 files changed, 371 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/i915_selftest.h
 create mode 100644 drivers/gpu/drm/i915/selftests/i915_live_selftests.h
 create mode 100644 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
 create mode 100644 drivers/gpu/drm/i915/selftests/i915_selftest.c

Comments

Tvrtko Ursulin Jan. 11, 2017, 6:17 p.m. UTC | #1
On 20/12/2016 13:07, Chris Wilson wrote:
> Some pieces of code are independent of hardware but are very tricky to
> exercise through the normal userspace ABI or via debugfs hooks. Being
> able to create mock unit tests and execute them through CI is vital.
> Start by adding a central point where we can execute unit tests and
> a parameter to enable them. This is disabled by default as the
> expectation is that these tests will occasionally explode.
>
> To facilitate integration with igt, any parameter beginning with
> i915.igt__ is interpreted as a subtest executable independently via
> igt/drv_selftest.
>
> Two classes of selftests are recognised: mock unit tests and integration
> tests. Mock unit tests are run as soon as the module is loaded, before
> the device is probed. At that point there is no driver instantiated and
> all hw interactions must be "mocked". This is very useful for writing
> universal tests to exercise code not typically run on a broad range of
> architectures. Alternatively, you can hook into the late selftests and
> run when the device has been instantiated - hw interactions are real.
>
> v2: Add a macro for compiling conditional code for mock objects inside
> real objects.
> v3: Differentiate between mock unit tests and late integration test.
> v4: List the tests in natural order, use igt to sort after modparam.
> v5: s/late/live/
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v1
> ---
>  drivers/gpu/drm/i915/Kconfig.debug                 |  15 ++
>  drivers/gpu/drm/i915/Makefile                      |   3 +
>  drivers/gpu/drm/i915/i915_pci.c                    |  19 +-
>  drivers/gpu/drm/i915/i915_selftest.h               |  91 +++++++++
>  .../gpu/drm/i915/selftests/i915_live_selftests.h   |  11 +
>  .../gpu/drm/i915/selftests/i915_mock_selftests.h   |  11 +
>  drivers/gpu/drm/i915/selftests/i915_selftest.c     | 222 +++++++++++++++++++++
>  7 files changed, 371 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_selftest.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_live_selftests.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
>  create mode 100644 drivers/gpu/drm/i915/selftests/i915_selftest.c
>
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index 598551dbf62c..de051502e891 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -26,6 +26,7 @@ config DRM_I915_DEBUG
>          select DRM_DEBUG_MM if DRM=y
>  	select DRM_DEBUG_MM_SELFTEST
>  	select DRM_I915_SW_FENCE_DEBUG_OBJECTS
> +	select DRM_I915_SELFTEST
>          default n
>          help
>            Choose this option to turn on extra driver debugging that may affect
> @@ -59,3 +60,17 @@ config DRM_I915_SW_FENCE_DEBUG_OBJECTS
>            Recommended for driver developers only.
>
>            If in doubt, say "N".
> +
> +config DRM_I915_SELFTEST
> +	bool "Enable selftests upon driver load"
> +	depends on DRM_I915
> +	default n
> +	help
> +	  Choose this option to allow the driver to perform selftests upon
> +	  loading; also requires the i915.selftest=1 module parameter. To
> +	  exit the module after running the selftests (i.e. to prevent normal
> +	  module initialisation afterwards) use i915.selftest=-1.
> +
> +	  Recommended for driver developers only.
> +
> +	  If in doubt, say "N".
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 5196509e71cf..461aeb44a9ad 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -3,6 +3,7 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>
>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> +subdir-ccflags-$(CONFIG_DRM_I915_SELFTEST) := -I$(src) -I$(src)/selftests
>  subdir-ccflags-y += \
>  	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
>
> @@ -114,6 +115,8 @@ i915-y += dvo_ch7017.o \
>
>  # Post-mortem debug and GPU hang state capture
>  i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
> +i915-$(CONFIG_DRM_I915_SELFTEST) += \
> +	selftests/i915_selftest.o
>
>  # virtual gpu code
>  i915-y += i915_vgpu.o
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 9885458b0fb8..3d416d142573 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -27,6 +27,7 @@
>  #include <linux/vga_switcheroo.h>
>
>  #include "i915_drv.h"
> +#include "i915_selftest.h"
>
>  #define GEN_DEFAULT_PIPEOFFSETS \
>  	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
> @@ -477,6 +478,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	struct intel_device_info *intel_info =
>  		(struct intel_device_info *) ent->driver_data;
> +	int err;
>
>  	if (IS_ALPHA_SUPPORT(intel_info) && !i915.alpha_support) {
>  		DRM_INFO("The driver support for your hardware in this kernel version is alpha quality\n"
> @@ -500,7 +502,17 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (vga_switcheroo_client_probe_defer(pdev))
>  		return -EPROBE_DEFER;
>
> -	return i915_driver_load(pdev, ent);
> +	err = i915_driver_load(pdev, ent);
> +	if (err)
> +		return err;
> +
> +	err = i915_live_selftests(pdev);
> +	if (err) {
> +		i915_driver_unload(pci_get_drvdata(pdev));
> +		return err > 0 ? -ENOTTY : err;
> +	}
> +
> +	return 0;
>  }
>
>  static void i915_pci_remove(struct pci_dev *pdev)
> @@ -522,6 +534,11 @@ static struct pci_driver i915_pci_driver = {
>  static int __init i915_init(void)
>  {
>  	bool use_kms = true;
> +	int err;
> +
> +	err = i915_mock_selftests();
> +	if (err)
> +		return err > 0 ? 0 : err;

Am I again confused by the return codes? :) Module param of -1 will 
result in i915_mock_selftests returning 1, which here translates to 0 so 
it won't abort the load like it should.

>
>  	/*
>  	 * Enable KMS by default, unless explicitly overriden by
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> new file mode 100644
> index 000000000000..6b3d1192e092
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright © 2016 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.
> + */
> +
> +#ifndef __I915_SELFTEST_H__
> +#define __I915_SELFTEST_H__
> +
> +struct pci_dev;
> +struct drm_i915_private;
> +
> +struct i915_selftest {
> +	unsigned long timeout_jiffies;
> +	unsigned int timeout_ms;
> +	unsigned int random_seed;
> +	int mock;
> +	int live;
> +};
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +extern struct i915_selftest i915_selftest;
> +
> +int i915_mock_selftests(void);
> +int i915_live_selftests(struct pci_dev *pdev);
> +
> +/* We extract the function declarations from i915_mock_selftests.h and
> + * i915_live_selftests.h Add your unit test declarations there!
> + *
> + * Mock unit tests are run very early upon module load, before the driver
> + * is probed. All hardware interactions, as well as other subsystems, must
> + * be "mocked".
> + *
> + * Late unit tests are run after the driver is loaded - all hardware
> + * interactions are real.
> + */
> +#define selftest(name, func) int func(void);
> +#include "i915_mock_selftests.h"
> +#undef selftest
> +#define selftest(name, func) int func(struct drm_i915_private *i915);
> +#include "i915_live_selftests.h"
> +#undef selftest
> +
> +struct i915_subtest {
> +	int (*func)(void *data);
> +	const char *name;
> +};
> +
> +int __i915_subtests(const char *caller,
> +		    const struct i915_subtest *st,
> +		    int count,
> +		    void *data);
> +#define i915_subtests(T, data) \
> +	__i915_subtests(__func__, T, ARRAY_SIZE(T), data)
> +
> +#define SUBTEST(x) { x, #x }
> +
> +#define I915_SELFTEST_DECLARE(x) x
> +#define I915_SELFTEST_ONLY(x) unlikely(x)
> +
> +#else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
> +
> +static inline int i915_mock_selftests(void) { return 0; }
> +static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
> +
> +#define I915_SELFTEST_DECLARE(x)
> +#define I915_SELFTEST_ONLY(x) 0
> +
> +#endif
> +
> +#define I915_SELFTEST_TIMEOUT(name__) \
> +	unsigned long name__ = jiffies + i915_selftest.timeout_jiffies
> +
> +#endif /* !__I915_SELFTEST_H__ */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> new file mode 100644
> index 000000000000..f3e17cb10e05
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> @@ -0,0 +1,11 @@
> +/* List each unit test as selftest(name, function)
> + *
> + * The name is used as both an enum and expanded as subtest__name to create
> + * a module parameter. It must be unique and legal for a C identifier.
> + *
> + * The function should be of type int function(void). It may be conditionally
> + * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
> + *
> + * Tests are executed in order by igt/drv_selftest
> + */
> +selftest(sanitycheck, i915_live_sanitycheck) /* keep first (igt selfcheck) */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> new file mode 100644
> index 000000000000..69e97a2ba4a6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -0,0 +1,11 @@
> +/* List each unit test as selftest(name, function)
> + *
> + * The name is used as both an enum and expanded as subtest__name to create
> + * a module parameter. It must be unique and legal for a C identifier.
> + *
> + * The function should be of type int function(void). It may be conditionally
> + * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
> + *
> + * Tests are executed in order by igt/drv_selftest
> + */
> +selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt selfcheck) */
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> new file mode 100644
> index 000000000000..4876f974edb8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -0,0 +1,222 @@
> +/*
> + * Copyright © 2016 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.
> + */
> +
> +#include <linux/random.h>
> +
> +#include "i915_drv.h"
> +#include "i915_selftest.h"
> +
> +struct i915_selftest i915_selftest __read_mostly = {
> +	.timeout_ms = 1000,
> +};
> +
> +int i915_mock_sanitycheck(void)
> +{
> +	pr_info("i915: %s() - ok!\n", __func__);
> +	return 0;
> +}
> +
> +int i915_live_sanitycheck(struct drm_i915_private *i915)
> +{
> +	pr_info("%s: %s() - ok!\n", i915->drm.driver->name, __func__);
> +	return 0;
> +}
> +
> +enum {
> +#define selftest(name, func) mock_##name,
> +#include "i915_mock_selftests.h"
> +#undef selftest
> +};
> +enum {
> +#define selftest(name, func) live_##name,
> +#include "i915_live_selftests.h"
> +#undef selftest
> +};
> +
> +struct selftest {
> +	bool enabled;
> +	const char *name;
> +	union {
> +		int (*mock)(void);
> +		int (*live)(struct drm_i915_private *);
> +	};
> +};
> +
> +#define selftest(n, f) [mock_##n] = { .name = #n, .mock = f },
> +static struct selftest mock_selftests[] = {
> +#include "i915_mock_selftests.h"
> +};
> +#undef selftest
> +
> +#define selftest(n, f) [live_##n] = { .name = #n, .live = f },
> +static struct selftest live_selftests[] = {
> +#include "i915_live_selftests.h"
> +};
> +#undef selftest
> +
> +/* Embed the line number into the parameter name so that we can order tests */
> +#define selftest(n, func) selftest_0(n, func, param(n))
> +#define param(n) __PASTE(igt__, __PASTE(__PASTE(__LINE__, __), mock_##n))
> +#define selftest_0(n, func, id) \
> +module_param_named(id, mock_selftests[mock_##n].enabled, bool, 0400);
> +#include "i915_mock_selftests.h"
> +#undef selftest_0
> +#undef param
> +
> +#define param(n) __PASTE(igt__, __PASTE(__PASTE(__LINE__, __), live_##n))
> +#define selftest_0(n, func, id) \
> +module_param_named(id, live_selftests[live_##n].enabled, bool, 0400);
> +#include "i915_live_selftests.h"
> +#undef selftest_0
> +#undef param
> +#undef selftest
> +
> +static void set_default_test_all(struct selftest *st, unsigned long count)
> +{
> +	unsigned long i;
> +
> +	for (i = 0; i < count; i++)
> +		if (st[i].enabled)
> +			return;
> +
> +	for (i = 0; i < count; i++)
> +		st[i].enabled = true;
> +}

unsigned int should be enough for everyone! :) (i & count)

> +
> +static int run_selftests(const char *name,
> +			 struct selftest *st,
> +			 unsigned long count,
> +			 void *data)
> +{
> +	int err = 0;
> +

If I got it right:

/* Make sure both live and mock run with the same seed if ran one after 
another. */

? just not sure what happens if user sets zero.

> +	while (!i915_selftest.random_seed)
> +		i915_selftest.random_seed = get_random_int();
> +
> +	i915_selftest.timeout_jiffies =
> +		i915_selftest.timeout_ms ?
> +		msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
> +		MAX_SCHEDULE_TIMEOUT;
> +
> +	set_default_test_all(st, count);
> +
> +	pr_info("i915: Performing %s selftests with st_random_seed=%x and st_timeout=%u\n",
> +		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
> +
> +	/* Tests are listed in order in i915_*_selftests.h */
> +	for (; count--; st++) {
> +		if (!st->enabled)
> +			continue;
> +
> +		cond_resched();
> +		if (signal_pending(current))
> +			return -EINTR;
> +
> +		pr_debug("i915: Running %s\n", st->name);
> +		if (data)
> +			err = st->live(data);
> +		else
> +			err = st->mock();
> +		if (err)
> +			break;
> +	}
> +
> +	if (WARN(err > 0 || err == -ENOTTY,
> +		 "%s returned %d, conflicting with selftest's magic values!\n",
> +		 st->name, err))
> +		err = -1;
> +
> +	rcu_barrier();

Why this?

> +	return err;
> +}
> +
> +#define run_selftests(x, data) \
> +	(run_selftests)(#x, x##_selftests, ARRAY_SIZE(x##_selftests), data)
> +
> +int i915_mock_selftests(void)
> +{
> +	int err;
> +
> +	if (!i915_selftest.mock)
> +		return 0;
> +
> +	err = run_selftests(mock, NULL);
> +	if (err)
> +		return err;
> +
> +	if (i915_selftest.mock < 0)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +int i915_live_selftests(struct pci_dev *pdev)
> +{
> +	int err;
> +
> +	if (!i915_selftest.live)
> +		return 0;
> +
> +	err = run_selftests(live, to_i915(pci_get_drvdata(pdev)));
> +	if (err)
> +		return err;
> +
> +	if (i915_selftest.live < 0)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +int __i915_subtests(const char *caller,
> +		    const struct i915_subtest *st,
> +		    int count,
> +		    void *data)
> +{
> +	int err;
> +
> +	for (; count--; st++) {
> +		cond_resched();
> +		if (signal_pending(current))
> +			return -EINTR;
> +
> +		pr_debug("i915: Running %s/%s\n", caller, st->name);
> +		err = st->func(data);
> +		if (err) {
> +			if (err != -EINTR)
> +				pr_err("i915/%s: %s failed with error %d\n",
> +				       caller, st->name, err);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
> +module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
> +
> +module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
> +MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then exit module)");
> +
> +module_param_named_unsafe(live_selftests, i915_selftest.live, int, 0400);
> +MODULE_PARM_DESC(live_selftests, "Run selftests after driver initialisation on the live system (0:disabled [default], 1:run tests then continue, -1:run tests then exit module)");
>

Regards,

Tvrtko
Chris Wilson Jan. 11, 2017, 6:34 p.m. UTC | #2
On Wed, Jan 11, 2017 at 06:17:48PM +0000, Tvrtko Ursulin wrote:
> On 20/12/2016 13:07, Chris Wilson wrote:
> >@@ -522,6 +534,11 @@ static struct pci_driver i915_pci_driver = {
> > static int __init i915_init(void)
> > {
> > 	bool use_kms = true;
> >+	int err;
> >+
> >+	err = i915_mock_selftests();
> >+	if (err)
> >+		return err > 0 ? 0 : err;
> 
> Am I again confused by the return codes? :) Module param of -1 will
> result in i915_mock_selftests returning 1, which here translates to
> 0 so it won't abort the load like it should.

I had to give up on that for silent passing and do the remove from
userspace on success instead. Returning anything other than 0 causes noise
in dmesg. That I can live with after an error during the selftest, since
dmesg should also contain more details on the test failure.

If i915.mock_selftests=-1 then we run the tests and stop. We just leave
the module loaded even though it hasn't bound to any pci devices. :|
igt/drv_selftest and kselftests/gpu/i915.sh then unload the module.

> >+static void set_default_test_all(struct selftest *st, unsigned long count)
> >+{
> >+	unsigned long i;
> >+
> >+	for (i = 0; i < count; i++)
> >+		if (st[i].enabled)
> >+			return;
> >+
> >+	for (i = 0; i < count; i++)
> >+		st[i].enabled = true;
> >+}
> 
> unsigned int should be enough for everyone! :) (i & count)

Such shortsightedness!

> >+static int run_selftests(const char *name,
> >+			 struct selftest *st,
> >+			 unsigned long count,
> >+			 void *data)
> >+{
> >+	int err = 0;
> >+
> 
> If I got it right:
> 
> /* Make sure both live and mock run with the same seed if ran one
> after another. */

Yes, choose the seed once, run every selected test with the same seed.
 
> ? just not sure what happens if user sets zero.

I wasn't such if 0 was a valid seed, so I wasn't caring too much if the
user did i915.st_random_seed=0. They will see the pr_info() and go
wtf, and hopefully don't do that again.

> >+	while (!i915_selftest.random_seed)
> >+		i915_selftest.random_seed = get_random_int();
> >+
> >+	i915_selftest.timeout_jiffies =
> >+		i915_selftest.timeout_ms ?
> >+		msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
> >+		MAX_SCHEDULE_TIMEOUT;
> >+
> >+	set_default_test_all(st, count);
> >+
> >+	pr_info("i915: Performing %s selftests with st_random_seed=%x and st_timeout=%u\n",
> >+		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
> >+
> >+	/* Tests are listed in order in i915_*_selftests.h */
> >+	for (; count--; st++) {
> >+		if (!st->enabled)
> >+			continue;
> >+
> >+		cond_resched();
> >+		if (signal_pending(current))
> >+			return -EINTR;
> >+
> >+		pr_debug("i915: Running %s\n", st->name);
> >+		if (data)
> >+			err = st->live(data);
> >+		else
> >+			err = st->mock();
> >+		if (err)
> >+			break;
> >+	}
> >+
> >+	if (WARN(err > 0 || err == -ENOTTY,
> >+		 "%s returned %d, conflicting with selftest's magic values!\n",
> >+		 st->name, err))
> >+		err = -1;
> >+
> >+	rcu_barrier();
> 
> Why this?

Paranoia for the tests aborting without the barrier, as we can't rely on
module_unload providing it since we may go on to load the driver as
normal.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 598551dbf62c..de051502e891 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -26,6 +26,7 @@  config DRM_I915_DEBUG
         select DRM_DEBUG_MM if DRM=y
 	select DRM_DEBUG_MM_SELFTEST
 	select DRM_I915_SW_FENCE_DEBUG_OBJECTS
+	select DRM_I915_SELFTEST
         default n
         help
           Choose this option to turn on extra driver debugging that may affect
@@ -59,3 +60,17 @@  config DRM_I915_SW_FENCE_DEBUG_OBJECTS
           Recommended for driver developers only.
 
           If in doubt, say "N".
+
+config DRM_I915_SELFTEST
+	bool "Enable selftests upon driver load"
+	depends on DRM_I915
+	default n
+	help
+	  Choose this option to allow the driver to perform selftests upon
+	  loading; also requires the i915.selftest=1 module parameter. To
+	  exit the module after running the selftests (i.e. to prevent normal
+	  module initialisation afterwards) use i915.selftest=-1.
+
+	  Recommended for driver developers only.
+
+	  If in doubt, say "N".
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5196509e71cf..461aeb44a9ad 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -3,6 +3,7 @@ 
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
+subdir-ccflags-$(CONFIG_DRM_I915_SELFTEST) := -I$(src) -I$(src)/selftests
 subdir-ccflags-y += \
 	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
 
@@ -114,6 +115,8 @@  i915-y += dvo_ch7017.o \
 
 # Post-mortem debug and GPU hang state capture
 i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
+i915-$(CONFIG_DRM_I915_SELFTEST) += \
+	selftests/i915_selftest.o
 
 # virtual gpu code
 i915-y += i915_vgpu.o
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 9885458b0fb8..3d416d142573 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -27,6 +27,7 @@ 
 #include <linux/vga_switcheroo.h>
 
 #include "i915_drv.h"
+#include "i915_selftest.h"
 
 #define GEN_DEFAULT_PIPEOFFSETS \
 	.pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \
@@ -477,6 +478,7 @@  static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct intel_device_info *intel_info =
 		(struct intel_device_info *) ent->driver_data;
+	int err;
 
 	if (IS_ALPHA_SUPPORT(intel_info) && !i915.alpha_support) {
 		DRM_INFO("The driver support for your hardware in this kernel version is alpha quality\n"
@@ -500,7 +502,17 @@  static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (vga_switcheroo_client_probe_defer(pdev))
 		return -EPROBE_DEFER;
 
-	return i915_driver_load(pdev, ent);
+	err = i915_driver_load(pdev, ent);
+	if (err)
+		return err;
+
+	err = i915_live_selftests(pdev);
+	if (err) {
+		i915_driver_unload(pci_get_drvdata(pdev));
+		return err > 0 ? -ENOTTY : err;
+	}
+
+	return 0;
 }
 
 static void i915_pci_remove(struct pci_dev *pdev)
@@ -522,6 +534,11 @@  static struct pci_driver i915_pci_driver = {
 static int __init i915_init(void)
 {
 	bool use_kms = true;
+	int err;
+
+	err = i915_mock_selftests();
+	if (err)
+		return err > 0 ? 0 : err;
 
 	/*
 	 * Enable KMS by default, unless explicitly overriden by
diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
new file mode 100644
index 000000000000..6b3d1192e092
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -0,0 +1,91 @@ 
+/*
+ * Copyright © 2016 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.
+ */
+
+#ifndef __I915_SELFTEST_H__
+#define __I915_SELFTEST_H__
+
+struct pci_dev;
+struct drm_i915_private;
+
+struct i915_selftest {
+	unsigned long timeout_jiffies;
+	unsigned int timeout_ms;
+	unsigned int random_seed;
+	int mock;
+	int live;
+};
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+extern struct i915_selftest i915_selftest;
+
+int i915_mock_selftests(void);
+int i915_live_selftests(struct pci_dev *pdev);
+
+/* We extract the function declarations from i915_mock_selftests.h and
+ * i915_live_selftests.h Add your unit test declarations there!
+ *
+ * Mock unit tests are run very early upon module load, before the driver
+ * is probed. All hardware interactions, as well as other subsystems, must
+ * be "mocked".
+ *
+ * Late unit tests are run after the driver is loaded - all hardware
+ * interactions are real.
+ */
+#define selftest(name, func) int func(void);
+#include "i915_mock_selftests.h"
+#undef selftest
+#define selftest(name, func) int func(struct drm_i915_private *i915);
+#include "i915_live_selftests.h"
+#undef selftest
+
+struct i915_subtest {
+	int (*func)(void *data);
+	const char *name;
+};
+
+int __i915_subtests(const char *caller,
+		    const struct i915_subtest *st,
+		    int count,
+		    void *data);
+#define i915_subtests(T, data) \
+	__i915_subtests(__func__, T, ARRAY_SIZE(T), data)
+
+#define SUBTEST(x) { x, #x }
+
+#define I915_SELFTEST_DECLARE(x) x
+#define I915_SELFTEST_ONLY(x) unlikely(x)
+
+#else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */
+
+static inline int i915_mock_selftests(void) { return 0; }
+static inline int i915_live_selftests(struct pci_dev *pdev) { return 0; }
+
+#define I915_SELFTEST_DECLARE(x)
+#define I915_SELFTEST_ONLY(x) 0
+
+#endif
+
+#define I915_SELFTEST_TIMEOUT(name__) \
+	unsigned long name__ = jiffies + i915_selftest.timeout_jiffies
+
+#endif /* !__I915_SELFTEST_H__ */
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
new file mode 100644
index 000000000000..f3e17cb10e05
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -0,0 +1,11 @@ 
+/* List each unit test as selftest(name, function)
+ *
+ * The name is used as both an enum and expanded as subtest__name to create
+ * a module parameter. It must be unique and legal for a C identifier.
+ *
+ * The function should be of type int function(void). It may be conditionally
+ * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
+ *
+ * Tests are executed in order by igt/drv_selftest
+ */
+selftest(sanitycheck, i915_live_sanitycheck) /* keep first (igt selfcheck) */
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
new file mode 100644
index 000000000000..69e97a2ba4a6
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -0,0 +1,11 @@ 
+/* List each unit test as selftest(name, function)
+ *
+ * The name is used as both an enum and expanded as subtest__name to create
+ * a module parameter. It must be unique and legal for a C identifier.
+ *
+ * The function should be of type int function(void). It may be conditionally
+ * compiled using #if IS_ENABLED(DRM_I915_SELFTEST).
+ *
+ * Tests are executed in order by igt/drv_selftest
+ */
+selftest(sanitycheck, i915_mock_sanitycheck) /* keep first (igt selfcheck) */
diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
new file mode 100644
index 000000000000..4876f974edb8
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
@@ -0,0 +1,222 @@ 
+/*
+ * Copyright © 2016 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.
+ */
+
+#include <linux/random.h>
+
+#include "i915_drv.h"
+#include "i915_selftest.h"
+
+struct i915_selftest i915_selftest __read_mostly = {
+	.timeout_ms = 1000,
+};
+
+int i915_mock_sanitycheck(void)
+{
+	pr_info("i915: %s() - ok!\n", __func__);
+	return 0;
+}
+
+int i915_live_sanitycheck(struct drm_i915_private *i915)
+{
+	pr_info("%s: %s() - ok!\n", i915->drm.driver->name, __func__);
+	return 0;
+}
+
+enum {
+#define selftest(name, func) mock_##name,
+#include "i915_mock_selftests.h"
+#undef selftest
+};
+enum {
+#define selftest(name, func) live_##name,
+#include "i915_live_selftests.h"
+#undef selftest
+};
+
+struct selftest {
+	bool enabled;
+	const char *name;
+	union {
+		int (*mock)(void);
+		int (*live)(struct drm_i915_private *);
+	};
+};
+
+#define selftest(n, f) [mock_##n] = { .name = #n, .mock = f },
+static struct selftest mock_selftests[] = {
+#include "i915_mock_selftests.h"
+};
+#undef selftest
+
+#define selftest(n, f) [live_##n] = { .name = #n, .live = f },
+static struct selftest live_selftests[] = {
+#include "i915_live_selftests.h"
+};
+#undef selftest
+
+/* Embed the line number into the parameter name so that we can order tests */
+#define selftest(n, func) selftest_0(n, func, param(n))
+#define param(n) __PASTE(igt__, __PASTE(__PASTE(__LINE__, __), mock_##n))
+#define selftest_0(n, func, id) \
+module_param_named(id, mock_selftests[mock_##n].enabled, bool, 0400);
+#include "i915_mock_selftests.h"
+#undef selftest_0
+#undef param
+
+#define param(n) __PASTE(igt__, __PASTE(__PASTE(__LINE__, __), live_##n))
+#define selftest_0(n, func, id) \
+module_param_named(id, live_selftests[live_##n].enabled, bool, 0400);
+#include "i915_live_selftests.h"
+#undef selftest_0
+#undef param
+#undef selftest
+
+static void set_default_test_all(struct selftest *st, unsigned long count)
+{
+	unsigned long i;
+
+	for (i = 0; i < count; i++)
+		if (st[i].enabled)
+			return;
+
+	for (i = 0; i < count; i++)
+		st[i].enabled = true;
+}
+
+static int run_selftests(const char *name,
+			 struct selftest *st,
+			 unsigned long count,
+			 void *data)
+{
+	int err = 0;
+
+	while (!i915_selftest.random_seed)
+		i915_selftest.random_seed = get_random_int();
+
+	i915_selftest.timeout_jiffies =
+		i915_selftest.timeout_ms ?
+		msecs_to_jiffies_timeout(i915_selftest.timeout_ms) :
+		MAX_SCHEDULE_TIMEOUT;
+
+	set_default_test_all(st, count);
+
+	pr_info("i915: Performing %s selftests with st_random_seed=%x and st_timeout=%u\n",
+		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
+
+	/* Tests are listed in order in i915_*_selftests.h */
+	for (; count--; st++) {
+		if (!st->enabled)
+			continue;
+
+		cond_resched();
+		if (signal_pending(current))
+			return -EINTR;
+
+		pr_debug("i915: Running %s\n", st->name);
+		if (data)
+			err = st->live(data);
+		else
+			err = st->mock();
+		if (err)
+			break;
+	}
+
+	if (WARN(err > 0 || err == -ENOTTY,
+		 "%s returned %d, conflicting with selftest's magic values!\n",
+		 st->name, err))
+		err = -1;
+
+	rcu_barrier();
+	return err;
+}
+
+#define run_selftests(x, data) \
+	(run_selftests)(#x, x##_selftests, ARRAY_SIZE(x##_selftests), data)
+
+int i915_mock_selftests(void)
+{
+	int err;
+
+	if (!i915_selftest.mock)
+		return 0;
+
+	err = run_selftests(mock, NULL);
+	if (err)
+		return err;
+
+	if (i915_selftest.mock < 0)
+		return 1;
+
+	return 0;
+}
+
+int i915_live_selftests(struct pci_dev *pdev)
+{
+	int err;
+
+	if (!i915_selftest.live)
+		return 0;
+
+	err = run_selftests(live, to_i915(pci_get_drvdata(pdev)));
+	if (err)
+		return err;
+
+	if (i915_selftest.live < 0)
+		return 1;
+
+	return 0;
+}
+
+int __i915_subtests(const char *caller,
+		    const struct i915_subtest *st,
+		    int count,
+		    void *data)
+{
+	int err;
+
+	for (; count--; st++) {
+		cond_resched();
+		if (signal_pending(current))
+			return -EINTR;
+
+		pr_debug("i915: Running %s/%s\n", caller, st->name);
+		err = st->func(data);
+		if (err) {
+			if (err != -EINTR)
+				pr_err("i915/%s: %s failed with error %d\n",
+				       caller, st->name, err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
+module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
+
+module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
+MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then exit module)");
+
+module_param_named_unsafe(live_selftests, i915_selftest.live, int, 0400);
+MODULE_PARM_DESC(live_selftests, "Run selftests after driver initialisation on the live system (0:disabled [default], 1:run tests then continue, -1:run tests then exit module)");