All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Ander Conselvan De Oliveira <conselvan2@gmail.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	Thomas Wood <thomas.wood@intel.com>,
	Martin Peres <martin.peres@free.fr>,
	Ben Skeggs <bskeggs@redhat.com>,
	Nouveau Dev <nouveau@lists.freedesktop.org>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Shared scaffolding to unit-test kernel code in userspace (was Re: [Intel-gfx] [PATCH] drm/i915: Add link training test)
Date: Wed, 23 Sep 2015 11:38:17 +0200	[thread overview]
Message-ID: <20150923093817.GG3383@phenom.ffwll.local> (raw)
In-Reply-To: <1442999931.2745.21.camel@gmail.com>

On Wed, Sep 23, 2015 at 11:18 AM, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> On Wed, 2015-09-23 at 10:24 +0200, Daniel Vetter wrote:
>> On Tue, Sep 15, 2015 at 04:08:53PM +0300, Ander Conselvan De Oliveira wrote:
>> > On Mon, 2015-09-14 at 16:38 +0300, Ander Conselvan De Oliveira wrote:
>> > > On Mon, 2015-09-14 at 15:11 +0200, Daniel Vetter wrote:
>> > > > On Mon, Sep 14, 2015 at 02:51:51PM +0300, Ander Conselvan de Oliveira wrote:
>> > > > > ---
>> > > > >
>> > > > > On Fri, 2015-09-11 at 17:11 +0300, Ander Conselvan de Oliveira wrote:
>> > > > > > On Wed, 2015-09-09 at 11:33 +0100, Thomas Wood wrote:
>> > > > > > > On 8 September 2015 at 13:28, Ander Conselvan de Oliveira
>> > > > > > > <ander.conselvan.de.oliveira@intel.com> wrote:
>> > > > > > > >
>> > > > > >
>> > > > > > > > diff --git a/link-training-test/Makefile b/link-training-test/Makefile
>> > > > > > >
>> > > > > > > If this is meant to be part of the test suite, then it needs to be in
>> > > > > > > the tests directory and use the igt test infrastructure. Otherwise it
>> > > > > > > should be placed in tools or tools/link-training-test.
>> > > > > >
>> > > > > > I made the test use the igt infrastructure, but I'm not sure if this is
>> > > > > > a good fit for it. The dependency on the kernel is on build time, but
>> > > > > > once compiled this can be run on any machine. This can also introduce
>> > > > > > build failures if the test is not kept in sync with the driver source.
>> > > > > > Ideally that a failure to build this would be reported as the test
>> > > > > > failing, but I have no idea of how to achieve that.
>> > > > >
>> > > > > Alternatively, this could be in the kernel source tree directly. This
>> > > > > patch adds a test subdir to the i915 source dir, containing the link
>> > > > > training test. The test is compiled as part of the normal build using
>> > > > > the extra-y variable so that it doesn't get linked to the final kernel.
>> > > > >
>> > > > > When make is run from the tests directory, a thin wrapper around the
>> > > > > tests is built and linked to the object file compiled as part of the
>> > > > > kernel build. Running make run_tests from the test dir runs the test
>> > > > > and reports success or failure.
>> > > > >
>> > > > > Any thoughts?
>> > > >
>> > > > I think there's some precedence in other subsystems to integrate unit
>> > > > tests directly in the kernel, e.g. locking selftest or similar things.
>> > > > Usual approach is to either have a special module (but that often means
>> > > > piles of EXPORT_SYMBOL only for that selftest module). Or just a y/n
>> > > > Kconfig option which enables that code and runs all the self/unit tests
>> > > > when the module loads.
>> > > >
>> > > > I'd go with that approach since it's simpler. And we'd only need to tell
>> > > > QA to enable that Kconfig option for more testing.
>> > >
>> > > I'll have a look into that Kconfig approach, but there's a couple of things
>> > > I like about having the unit test as user space binaries:
>> > >
>> > >   - there's no need to boot the newly compiled kernel, so doing a test run
>> > >     is super fast;
>> > >   - the binaries can be debugged with gdb just like other user space stuff.
>> >
>> > I implemented the test using the Kconfig approach, and it seems to work well
>> > without impacting the points above. I added the call to run the test as the
>> > first thing in i915_init(), and with the driver compiled built-in, running
>> > the kernel under qemu will run the tests. And qemu can also provide a gdb
>> > remote target.
>> >
>> > One thing might be a problem though. With the previous approach, the
>> > functions overriden by the test where simply reimplemented in the new binary.
>> > But now the test is linked to the entire driver, so that's not possible. To
>> > work around that, I had to add function pointers to all the functions called
>> > by the link training state machine to intel_dp. I don't think that method
>> > scales well.
>> >
>> > I'll send update patches for reference as replies to this mail.
>>
>> I had a few discussions about this at XDC and I know think doing this is
>> userspace is better:
>> - Faster to run tests (since no module reloading required).
>> - Nouveau is developed in userspace and would like to reuse shared link
>>   training code too if possible from the dp helpers.
>>
>> So I'm leaning towards scaffolding in userspace now. Might be good to
>> check out how the nouveau userspace runtime works just to steal a few
>> tricks.
>
> I chatted with Martin Peres about this and had a look at the code. There are a few interesting
> tricks in Nouveau indeed. They have user space implementation for some kernel internals such as
> mutexes, ioremap on top of libpciaccess, etc. and an extensive set of stubs.
>
> The one thing that wouldn't be easy to take advantage of is the build integration. Their upstream
> repository is not actually a kernel tree. It contains their drm code, the same that ends up in the
> kernel plus all the user space stuff. There's a script that converts the commits from that
> repository for inclusion in the kernel tree.

Could we perhaps move at least some of the scafffolding for kernel
functions to upstream? I guess nouveau has some means to pull changes from
upstream too, so maybe we could push that some place nice. And I'm pretty
sure we're not the only ones thinking about unit-testing specific
kernelcode in a userspace environment.

Adding noveau and kselftest folks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org>
To: Ander Conselvan De Oliveira
	<conselvan2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Nouveau Dev
	<nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	intel-gfx
	<intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Ben Skeggs <bskeggs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Greg KH
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Thomas Wood <thomas.wood-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: Shared scaffolding to unit-test kernel code in userspace (was Re: [Intel-gfx] [PATCH] drm/i915: Add link training test)
Date: Wed, 23 Sep 2015 11:38:17 +0200	[thread overview]
Message-ID: <20150923093817.GG3383@phenom.ffwll.local> (raw)
In-Reply-To: <1442999931.2745.21.camel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Wed, Sep 23, 2015 at 11:18 AM, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> On Wed, 2015-09-23 at 10:24 +0200, Daniel Vetter wrote:
>> On Tue, Sep 15, 2015 at 04:08:53PM +0300, Ander Conselvan De Oliveira wrote:
>> > On Mon, 2015-09-14 at 16:38 +0300, Ander Conselvan De Oliveira wrote:
>> > > On Mon, 2015-09-14 at 15:11 +0200, Daniel Vetter wrote:
>> > > > On Mon, Sep 14, 2015 at 02:51:51PM +0300, Ander Conselvan de Oliveira wrote:
>> > > > > ---
>> > > > >
>> > > > > On Fri, 2015-09-11 at 17:11 +0300, Ander Conselvan de Oliveira wrote:
>> > > > > > On Wed, 2015-09-09 at 11:33 +0100, Thomas Wood wrote:
>> > > > > > > On 8 September 2015 at 13:28, Ander Conselvan de Oliveira
>> > > > > > > <ander.conselvan.de.oliveira@intel.com> wrote:
>> > > > > > > >
>> > > > > >
>> > > > > > > > diff --git a/link-training-test/Makefile b/link-training-test/Makefile
>> > > > > > >
>> > > > > > > If this is meant to be part of the test suite, then it needs to be in
>> > > > > > > the tests directory and use the igt test infrastructure. Otherwise it
>> > > > > > > should be placed in tools or tools/link-training-test.
>> > > > > >
>> > > > > > I made the test use the igt infrastructure, but I'm not sure if this is
>> > > > > > a good fit for it. The dependency on the kernel is on build time, but
>> > > > > > once compiled this can be run on any machine. This can also introduce
>> > > > > > build failures if the test is not kept in sync with the driver source.
>> > > > > > Ideally that a failure to build this would be reported as the test
>> > > > > > failing, but I have no idea of how to achieve that.
>> > > > >
>> > > > > Alternatively, this could be in the kernel source tree directly. This
>> > > > > patch adds a test subdir to the i915 source dir, containing the link
>> > > > > training test. The test is compiled as part of the normal build using
>> > > > > the extra-y variable so that it doesn't get linked to the final kernel.
>> > > > >
>> > > > > When make is run from the tests directory, a thin wrapper around the
>> > > > > tests is built and linked to the object file compiled as part of the
>> > > > > kernel build. Running make run_tests from the test dir runs the test
>> > > > > and reports success or failure.
>> > > > >
>> > > > > Any thoughts?
>> > > >
>> > > > I think there's some precedence in other subsystems to integrate unit
>> > > > tests directly in the kernel, e.g. locking selftest or similar things.
>> > > > Usual approach is to either have a special module (but that often means
>> > > > piles of EXPORT_SYMBOL only for that selftest module). Or just a y/n
>> > > > Kconfig option which enables that code and runs all the self/unit tests
>> > > > when the module loads.
>> > > >
>> > > > I'd go with that approach since it's simpler. And we'd only need to tell
>> > > > QA to enable that Kconfig option for more testing.
>> > >
>> > > I'll have a look into that Kconfig approach, but there's a couple of things
>> > > I like about having the unit test as user space binaries:
>> > >
>> > >   - there's no need to boot the newly compiled kernel, so doing a test run
>> > >     is super fast;
>> > >   - the binaries can be debugged with gdb just like other user space stuff.
>> >
>> > I implemented the test using the Kconfig approach, and it seems to work well
>> > without impacting the points above. I added the call to run the test as the
>> > first thing in i915_init(), and with the driver compiled built-in, running
>> > the kernel under qemu will run the tests. And qemu can also provide a gdb
>> > remote target.
>> >
>> > One thing might be a problem though. With the previous approach, the
>> > functions overriden by the test where simply reimplemented in the new binary.
>> > But now the test is linked to the entire driver, so that's not possible. To
>> > work around that, I had to add function pointers to all the functions called
>> > by the link training state machine to intel_dp. I don't think that method
>> > scales well.
>> >
>> > I'll send update patches for reference as replies to this mail.
>>
>> I had a few discussions about this at XDC and I know think doing this is
>> userspace is better:
>> - Faster to run tests (since no module reloading required).
>> - Nouveau is developed in userspace and would like to reuse shared link
>>   training code too if possible from the dp helpers.
>>
>> So I'm leaning towards scaffolding in userspace now. Might be good to
>> check out how the nouveau userspace runtime works just to steal a few
>> tricks.
>
> I chatted with Martin Peres about this and had a look at the code. There are a few interesting
> tricks in Nouveau indeed. They have user space implementation for some kernel internals such as
> mutexes, ioremap on top of libpciaccess, etc. and an extensive set of stubs.
>
> The one thing that wouldn't be easy to take advantage of is the build integration. Their upstream
> repository is not actually a kernel tree. It contains their drm code, the same that ends up in the
> kernel plus all the user space stuff. There's a script that converts the commits from that
> repository for inclusion in the kernel tree.

Could we perhaps move at least some of the scafffolding for kernel
functions to upstream? I guess nouveau has some means to pull changes from
upstream too, so maybe we could push that some place nice. And I'm pretty
sure we're not the only ones thinking about unit-testing specific
kernelcode in a userspace environment.

Adding noveau and kselftest folks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

  reply	other threads:[~2015-09-23  9:35 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-08 12:27 [PATCH 0/8] [RFC] Link training test Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 1/8] drm/i915: Rename DP link training functions Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 2/8] drm/i915: Don't pass *DP around to " Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 3/8] drm/i915: Split intel_dp_update_link_train() Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 4/8] drm/i915: Split write of pattern to DP reg from intel_dp_set_link_train Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 5/8] drm/i915: Don't call intel_dp_set_signal_levels() on link train reset Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 6/8] drm/i915: Move generic link training code to a separate file Ander Conselvan de Oliveira
2015-09-08 12:27 ` [PATCH 7/8] drm/i915: Move max voltage and pre emphasis to intel_dp_link_training.c Ander Conselvan de Oliveira
2015-09-08 13:01   ` Ville Syrjälä
2015-10-19  5:14     ` Thulasimani, Sivakumar
2015-10-19  5:59       ` Ander Conselvan De Oliveira
2015-10-19  6:13         ` Thulasimani, Sivakumar
2015-09-08 12:27 ` [PATCH 8/8] drm/i915: Extract intel_dev_info.[ch] Ander Conselvan de Oliveira
2015-09-08 12:28 ` [PATCH i-g-t] Add a link training test Ander Conselvan de Oliveira
2015-09-08 13:11   ` Ville Syrjälä
2015-09-08 13:26     ` Ander Conselvan De Oliveira
2015-09-09 10:33   ` Thomas Wood
2015-09-11 14:11     ` [PATCH] Add a link training test (v2) Ander Conselvan de Oliveira
2015-09-14 11:51       ` [PATCH] drm/i915: Add link training test Ander Conselvan de Oliveira
2015-09-14 13:11         ` Daniel Vetter
2015-09-14 13:38           ` Ander Conselvan De Oliveira
2015-09-15 13:08             ` Ander Conselvan De Oliveira
2015-09-23  8:24               ` Daniel Vetter
2015-09-23  9:18                 ` Ander Conselvan De Oliveira
2015-09-23  9:38                   ` Daniel Vetter [this message]
2015-09-23  9:38                     ` Shared scaffolding to unit-test kernel code in userspace (was Re: [Intel-gfx] [PATCH] drm/i915: Add link training test) Daniel Vetter
2015-09-29  8:47                     ` Ander Conselvan De Oliveira
2015-09-29  8:47                       ` Shared scaffolding to unit-test kernel code in userspace (was " Ander Conselvan De Oliveira
2015-09-15 13:11             ` [PATCH 1/2] drm/i915: Make link training state machine code use function pointers Ander Conselvan de Oliveira
2015-09-15 13:11               ` [PATCH 2/2] drm/i915: Add a link training test Ander Conselvan de Oliveira
2015-09-22 15:12 ` [PATCH 0/8] [RFC] Link " Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150923093817.GG3383@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=bskeggs@redhat.com \
    --cc=conselvan2@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.peres@free.fr \
    --cc=nouveau@lists.freedesktop.org \
    --cc=shuahkh@osg.samsung.com \
    --cc=thomas.wood@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.