linux-kernel - Re: [PATCH] kunit: cs_dsp: Depend on FW_CS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA1CXcDPg7Na9biCMOx4i_xwXZ5Y_qq-7SiYEU82v2a6TFpMJA@mail.gmail.com>
Date: Fri, 21 Mar 2025 05:37:50 -0600
From: Nico Pache <npache@...hat.com>
To: Mark Brown <broonie@...nel.org>
Cc: rf@...nsource.cirrus.com, patches@...nsource.cirrus.com, 
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	kunit-dev@...glegroups.com, simont@...nsource.cirrus.com, 
	ckeepax@...nsource.cirrus.com, brendan.higgins@...ux.dev, davidgow@...gle.com, 
	rmoar@...gle.com, johannes.berg@...el.com, sj@...nel.org
Subject: Re: [PATCH] kunit: cs_dsp: Depend on FW_CS_DSP rather then enabling it

On Thu, Mar 20, 2025 at 4:49 PM Mark Brown <broonie@...nel.org> wrote:
>
> On Thu, Mar 20, 2025 at 04:21:16PM -0600, Nico Pache wrote:
> > On Thu, Mar 20, 2025 at 1:14 PM Mark Brown <broonie@...nel.org> wrote:
> > > On Wed, Mar 19, 2025 at 05:05:39PM -0600, Nico Pache wrote:
>
> > > >  config FW_CS_DSP_KUNIT_TEST
> > > >       tristate "KUnit tests for Cirrus Logic cs_dsp" if !KUNIT_ALL_TESTS
> > > > -     depends on KUNIT && REGMAP
> > > > +     depends on KUNIT && REGMAP && FW_CS_DSP
> > > >       default KUNIT_ALL_TESTS
> > > > -     select FW_CS_DSP
>
> > > This makes no sense to me, the select statement is forcing on the code
> > > it's testing which is a library and so is selected by it's users, this
>
> > Similarly to eb5c79828cfa ("firmware: cs_dsp: FW_CS_DSP_KUNIT_TEST
> > should not select REGMAP"), We shouldnt force a feature on when using
> > KUNIT_ALL_TESTS.
>
> This feature is not user selectable, at an absolute minimum you would
> need to make the library available in KUnit test builds.
>
> > > change will just stop the tests being run unless someone does the dance
> > > to enable a driver which relies on the library.  That is something that
>
> > My config also sets the UML wrapper to enable this FW_CS_DSP config so
> > it will continue to work in that environment.
>
> Simply adding it to the all_tests.config will just result in it getting
> turned off by Kconfig during the build since it's not a visible option
> so that's not accomplishing anything.  all_tests.config is not UML
> specific, it's for enabling all the KUnit tests that could run in UML no
> matter how you're running them.
>
> > > seems unlikely to change the outcome of the tests when run from KUnit
> > > which is independent of any hardware.
>
> > KUNIT is supported outside the UML environment, and some distros (like
> > fedora, and downstream flavors), use KUNIT as modules, with
> > KUNIT_ALL_TESTS=m. We only want the tests that are supported by our
> > config to be available, we dont want KUNIT going and enabling other
> > features so the test works.
>
> The point is not that KUnit is frequently run in UML (personally I
> mostly run it with emulated hardware instead) but rather that this is a
> library which can be tested independently of having a relevant DSP.

Ok, thank you for the carifying message!

So would the correct approach be allowing users to select FW_CS_DSP,
then apply these changes?

It also looks like FW_CS_DSP_KUNIT_TEST_UTILS and FW_CS_DSP_KUNIT_TEST
are redundant.