Re: [PATCH] kunit: fix Kconfig for build-in tests USB4 and Nitro Enclaves

From: David Gow <davidgow@google.com>
To: Nico Pache <npache@redhat.com>
Cc: KUnit Development <kunit-dev@googlegroups.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-usb@vger.kernel.org, Shuah Khan <skhan@linuxfoundation.org>,
	Daniel Latypov <dlatypov@google.com>,
	Brendan Higgins <brendan.higgins@linux.dev>,
	alcioa@amazon.com, lexnv@amazon.com,
	Andra Paraschiv <andraprs@amazon.com>,
	YehezkelShB@gmail.com,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	michael.jamet@intel.com, andreas.noever@gmail.com
Subject: Re: [PATCH] kunit: fix Kconfig for build-in tests USB4 and Nitro Enclaves
Date: Thu, 11 Aug 2022 08:19:59 +0800	[thread overview]
Message-ID: <CABVgOSmUgkeuKKS_UYMOTUE4vARLpw--j77J9=zAkk5Zr30N9g@mail.gmail.com> (raw)
In-Reply-To: <20220810234056.2494993-1-npache@redhat.com>

On Thu, Aug 11, 2022 at 7:41 AM Nico Pache <npache@redhat.com> wrote:
>
> 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)

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.

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.)

Cheers,
-- David

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

>  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

>         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
>