On Wed, Aug 10, 2022 at 8:20 PM David Gow [off-list ref] wrote:

On Thu, Aug 11, 2022 at 7:41 AM Nico Pache [off-list ref] wrote:

quoted

Both the USB4 and Nitro Enclaves KUNIT tests are now able to be compiled
if KUNIT is compiled as a module. This leads to issues if KUNIT is being
packaged separately from the core kernel and when KUNIT is run baremetal
without the required driver compiled into the kernel.

Fixes: 635dcd16844b ("thunderbolt: test: Use kunit_test_suite() macro")
Fixes: fe5be808fa6c ("nitro_enclaves: test: Use kunit_test_suite() macro")
Signed-off-by: Nico Pache <npache@redhat.com>
---

Hmm... I'm not quite sure I understand the case that's broken here. Is it:
- KUnit is built as a module (CONFIG_KUNIT=m)
- USB4/nitro_enclaves are also built as modules, with the test enabled.
- The kunit module is not available at runtime, so neither driver
module can load (due to missing kunit dependencies)

Exactly, except the issue is also when the USB/NE=y not just when they
are modules. This is currently creating an issue with our build system
during the depmod stage and has been preventing us from generating
Fedora builds.

If so, that's not a case (i.e., the kunit.ko module being unavailable
if it was built) we've tried to support thus far. I guess a de-facto
rule for supporting it would be to depend on KUNIT=y for any KUnit
tests which are built into the same module as the driver they're
testing.

Yeah, although it's not been a case you've been trying to support, it
has been working so far :) This has been the case (built-in tests
utilizing 'depends on KUNIT=y') since we began supporting KUNIT in our
testing infrastructure and it would be nice to keep that as a de-facto
rule :)

Alternatively, maybe we could do some horrible hacks to compile stub
versions of various KUnit assertion symbols in unconditionally, which
forward to the real ones if KUnit is available.

(Personally, I'd love it if we could get rid of CONFIG_KUNIT=m
altogether, and it's actually broken right at the moment[1]. There are
still some cases (unloading / reloading KUnit with different filter
options) which require it, though.)

Personally I'd hate to see KUNIT=m go as that is how we have been able
to support running Kunit tests so far.

A little background on how we utilize Kunit. We build with KUNIT=m and
KUNIT_ALL_TESTS=m and run the tests baremetal.
Our build system creates 3 packages (kernel, kernel-modules, and
kernel-modules-internal), this allows us to ship the kernel and its
modules, while also isolating packages that we dont want to ship
outside of QE and developers. We then have our own infrastructure in
place to run and collect the output of these tests in our pipelined
environments. We dont utilize UML because we dont support that feature
in RHEL.

Fedora uses this same methodology for running KUNIT, so we are
frequently running kunit on an 'upstream' variant.

I'm not sure how many organizations are supporting continuous KUNIT
testing, or how they are achieving it, but dropping module support
would prevent us from doing the CI testing we have in place.

Cheers!
-- Nico

Cheers,
-- David

[1]: https://patchwork.kernel.org/project/linux-kselftest/patch/20220713005221.1926290-1-davidgow@google.com/

quoted

 drivers/thunderbolt/Kconfig         | 3 +--
 drivers/virt/nitro_enclaves/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index e76a6c173637..f12d0a3ee3e2 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig

@@ -29,8 +29,7 @@ config USB4_DEBUGFS_WRITE

 config USB4_KUNIT_TEST
        bool "KUnit tests" if !KUNIT_ALL_TESTS
-       depends on (USB4=m || KUNIT=y)
-       depends on KUNIT
+       depends on USB4 && KUNIT=y

This can probably just:
depends on KUNIT=y

quoted

        default KUNIT_ALL_TESTS

 config USB4_DMA_TEST

diff --git a/drivers/virt/nitro_enclaves/Kconfig b/drivers/virt/nitro_enclaves/Kconfig
index ce91add81401..dc4d25c26256 100644
--- a/drivers/virt/nitro_enclaves/Kconfig
+++ b/drivers/virt/nitro_enclaves/Kconfig

@@ -17,7 +17,7 @@ config NITRO_ENCLAVES

 config NITRO_ENCLAVES_MISC_DEV_TEST
        bool "Tests for the misc device functionality of the Nitro Enclaves" if !KUNIT_ALL_TESTS
-       depends on NITRO_ENCLAVES && KUNIT
+       depends on NITRO_ENCLAVES && KUNIT=y
        default KUNIT_ALL_TESTS
        help
          Enable KUnit tests for the misc device functionality of the Nitro
--

2.36.1