Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220214184104.1710107-1-dlatypov@google.com> <CAGS_qxomM_mGzvFokZH5dnf7L3kCitB3dWD8JH56fPcDSb6PeA@mail.gmail.com>
 <YgtLFwORwGggQ7M2@lahna>
In-Reply-To: <YgtLFwORwGggQ7M2@lahna>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Tue, 15 Feb 2022 10:03:01 -0800
Message-ID: <CAGS_qxrHUaCWNZ_sbTkhZvM9=BMN-ZH1LpMXCzxz=FbEeSx+Pg@mail.gmail.com>
Subject: Re: [PATCH] thunderbolt: test: get running under UML, add kunitconfig
To:     Mika Westerberg <mika.westerberg@linux.intel.com>
Cc:     brendanhiggins@google.com, davidgow@google.com,
        linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
        linux-usb@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Feb 14, 2022 at 10:41 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi Daniel,
>
> On Mon, Feb 14, 2022 at 06:39:25PM -0800, Daniel Latypov wrote:
> > On Mon, Feb 14, 2022 at 10:41 AM Daniel Latypov <dlatypov@google.com> wrote:
> > >
> > > These tests didn't work under the normal `kunit.py run` command since
> > > they require CONFIG_PCI=y, which could not be set on ARCH=um.
> > >
> > > Commit 68f5d3f3b654 ("um: add PCI over virtio emulation driver") lets us
> > > do so. To make it so people don't have to figure out how to do so, we
> > > add a drivers/thunderbolt/.kunitconfig.
> > >
> > > Can now run these tests using
> > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/thunderbolt
> > >
> > > Potentially controversial bits:
> > > 1. this .kunitconfig is UML-specific, can't do this for example
> > > $ ./tools/testing/kunit/kunit.py run --arch=x86_64 --kunitconfig=drivers/thunderbolt
> > > 2. this removes the manual call to __kunit_test_suites_init(), which
> > >    allowed us to control exactly when the tests got run.
> >
> > kernel-test-robot points out something I had forgotten.
> > Doing this prevents us from being able to build this test as a module.
> >
> > kunit_test_suites() defines an init_module() which conflicts with the
> > existing ones.
> >
> > There's some relevant discussion about reworking how kunit modules
> > work here, https://lore.kernel.org/linux-kselftest/e5fa413ed59083ca63f3479d507b972380da0dcf.camel@codeconstruct.com.au/
> >
> > So I think we have two options for this patch:
> > a) proceed, but disable building the test as a module for now (tristate => bool)
> > b) wait on this patch until kunit module support is refactored
> >
> > Basically the question is: does this slightly easier way of running
> > the test seem worth losing the ability to test as a module in the
> > short-term?
>
> I would like to keep the module option available.
>
> For me, I can just continue running this under QEMU for now so let's
> wait until the reworking has been done. Thanks for looking into this,
> though! :)

Sounds good.
We can treat this patch as just an example of what people can manually
do if they want to run tests under UML.

And I'll also look to this when I inevitably forget how to enable
CONFIG_PCI=y on UML again.

Thanks!
Daniel