[PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n

Stephen Boyd posted 1 patch 1 month, 1 week ago
drivers/of/.kunitconfig   | 1 +
drivers/of/Kconfig        | 1 -
drivers/of/overlay_test.c | 2 ++
3 files changed, 3 insertions(+), 1 deletion(-)
[PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n
Posted by Stephen Boyd 1 month, 1 week ago
Some configurations want to enable CONFIG_KUNIT without enabling
CONFIG_OF_OVERLAY. The kunit overlay code already skips if
CONFIG_OF_OVERLAY isn't enabled, so this select here isn't really doing
anything besides making it easier to run the tests without them
skipping. Remove the select and move the config setting to the
drivers/of/.kunitconfig file so that the overlay tests can be run with
or without CONFIG_OF_OVERLAY set to test either behavior.

Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/of/.kunitconfig   | 1 +
 drivers/of/Kconfig        | 1 -
 drivers/of/overlay_test.c | 2 ++
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/of/.kunitconfig b/drivers/of/.kunitconfig
index 4c53d2c7a275..7d570cb922a1 100644
--- a/drivers/of/.kunitconfig
+++ b/drivers/of/.kunitconfig
@@ -1,4 +1,5 @@
 CONFIG_KUNIT=y
 CONFIG_OF=y
 CONFIG_OF_KUNIT_TEST=y
+CONFIG_OF_OVERLAY=y
 CONFIG_OF_OVERLAY_KUNIT_TEST=y
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 0e2d608c3e20..0efa2d1b687a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -111,7 +111,6 @@ config OF_OVERLAY_KUNIT_TEST
 	tristate "Device Tree overlay KUnit tests" if !KUNIT_ALL_TESTS
 	depends on KUNIT
 	default KUNIT_ALL_TESTS
-	select OF_OVERLAY
 	help
 	  This option builds KUnit unit tests for the device tree overlay code.
 
diff --git a/drivers/of/overlay_test.c b/drivers/of/overlay_test.c
index 1f76d50fb16a..c787524c5a88 100644
--- a/drivers/of/overlay_test.c
+++ b/drivers/of/overlay_test.c
@@ -65,6 +65,8 @@ static void of_overlay_apply_kunit_cleanup(struct kunit *test)
 	struct device_node *np;
 
 	of_root_kunit_skip(test);
+	if (!IS_ENABLED(CONFIG_OF_OVERLAY))
+		kunit_skip(test, "requires CONFIG_OF_OVERLAY to apply overlay");
 	if (!IS_ENABLED(CONFIG_OF_EARLY_FLATTREE))
 		kunit_skip(test, "requires CONFIG_OF_EARLY_FLATTREE for root node");
 

base-commit: 6e0391e48cf9fb8b1b5e27c0cbbaf2e4639f2c33
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
Re: [PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n
Posted by Rob Herring 1 month, 1 week ago
On Wed, Oct 16, 2024 at 02:20:15PM -0700, Stephen Boyd wrote:
> Some configurations want to enable CONFIG_KUNIT without enabling
> CONFIG_OF_OVERLAY. The kunit overlay code already skips if
> CONFIG_OF_OVERLAY isn't enabled, so this select here isn't really doing
> anything besides making it easier to run the tests without them
> skipping. Remove the select and move the config setting to the
> drivers/of/.kunitconfig file so that the overlay tests can be run with
> or without CONFIG_OF_OVERLAY set to test either behavior.
> 
> Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")

Doesn't really seem like a fix. Does this need to go into 6.12?

> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>  drivers/of/.kunitconfig   | 1 +
>  drivers/of/Kconfig        | 1 -
>  drivers/of/overlay_test.c | 2 ++
>  3 files changed, 3 insertions(+), 1 deletion(-)
Re: [PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n
Posted by Stephen Boyd 1 month, 1 week ago
Quoting Rob Herring (2024-10-17 13:38:10)
> On Wed, Oct 16, 2024 at 02:20:15PM -0700, Stephen Boyd wrote:
> > Some configurations want to enable CONFIG_KUNIT without enabling
> > CONFIG_OF_OVERLAY. The kunit overlay code already skips if
> > CONFIG_OF_OVERLAY isn't enabled, so this select here isn't really doing
> > anything besides making it easier to run the tests without them
> > skipping. Remove the select and move the config setting to the
> > drivers/of/.kunitconfig file so that the overlay tests can be run with
> > or without CONFIG_OF_OVERLAY set to test either behavior.
> > 
> > Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
> 
> Doesn't really seem like a fix.

Ok. Feel free to drop the tag.

> Does this need to go into 6.12?

It's only important for 6.12 if kernel configurators want to build the
kernel with OF_OVERLAY_KUNIT_TEST enabled and not be forced to enable
CONFIG_OF_OVERLAY. I don't mind if it waits a while.
Re: [PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n
Posted by Geert Uytterhoeven 2 weeks, 4 days ago
Hi Stephen,

On Sat, Oct 19, 2024 at 12:21 AM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Rob Herring (2024-10-17 13:38:10)
> > On Wed, Oct 16, 2024 at 02:20:15PM -0700, Stephen Boyd wrote:
> > > Some configurations want to enable CONFIG_KUNIT without enabling
> > > CONFIG_OF_OVERLAY. The kunit overlay code already skips if
> > > CONFIG_OF_OVERLAY isn't enabled, so this select here isn't really doing
> > > anything besides making it easier to run the tests without them
> > > skipping. Remove the select and move the config setting to the
> > > drivers/of/.kunitconfig file so that the overlay tests can be run with
> > > or without CONFIG_OF_OVERLAY set to test either behavior.

Thanks for your patch, which is now commit 027af8c26fdc0642 ("of: Allow
overlay kunit tests to run CONFIG_OF_OVERLAY=n") in dt-rh/for-next.

> > > Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
> >
> > Doesn't really seem like a fix.
>
> Ok. Feel free to drop the tag.
>
> > Does this need to go into 6.12?
>
> It's only important for 6.12 if kernel configurators want to build the
> kernel with OF_OVERLAY_KUNIT_TEST enabled and not be forced to enable
> CONFIG_OF_OVERLAY. I don't mind if it waits a while.

I'd say it's a fix, so please keep at last the Fixes tag. Merely
enabling kunit tests (which can be modular) should not increase the
possible attack vector on a product by enabling extra unneeded code.

So
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n
Posted by Rob Herring 2 weeks, 3 days ago
On Fri, Nov 08, 2024 at 09:56:15AM +0100, Geert Uytterhoeven wrote:
> Hi Stephen,
> 
> On Sat, Oct 19, 2024 at 12:21 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > Quoting Rob Herring (2024-10-17 13:38:10)
> > > On Wed, Oct 16, 2024 at 02:20:15PM -0700, Stephen Boyd wrote:
> > > > Some configurations want to enable CONFIG_KUNIT without enabling
> > > > CONFIG_OF_OVERLAY. The kunit overlay code already skips if
> > > > CONFIG_OF_OVERLAY isn't enabled, so this select here isn't really doing
> > > > anything besides making it easier to run the tests without them
> > > > skipping. Remove the select and move the config setting to the
> > > > drivers/of/.kunitconfig file so that the overlay tests can be run with
> > > > or without CONFIG_OF_OVERLAY set to test either behavior.
> 
> Thanks for your patch, which is now commit 027af8c26fdc0642 ("of: Allow
> overlay kunit tests to run CONFIG_OF_OVERLAY=n") in dt-rh/for-next.
> 
> > > > Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
> > >
> > > Doesn't really seem like a fix.
> >
> > Ok. Feel free to drop the tag.
> >
> > > Does this need to go into 6.12?
> >
> > It's only important for 6.12 if kernel configurators want to build the
> > kernel with OF_OVERLAY_KUNIT_TEST enabled and not be forced to enable
> > CONFIG_OF_OVERLAY. I don't mind if it waits a while.
> 
> I'd say it's a fix, so please keep at last the Fixes tag. Merely
> enabling kunit tests (which can be modular) should not increase the
> possible attack vector on a product by enabling extra unneeded code.

Not sure I buy that that is an actual problem. However, not worth 
arguing over. I only really care because if there's a Fixes, then this 
really should go to Linus for 6.12 rather than eventually get 
auto-selected from 6.13 to go to stable. So I moved it to send to Linus, 
but I found that CONFIG_OF_OVERLAY is still selected with this patch. 
That's because the clock kunit tests also select CONFIG_OF_OVERLAY. 
That's fixed in next, but it's not queued up for 6.12.

Rob
Re: [PATCH] of: Allow overlay kunit tests to run CONFIG_OF_OVERLAY=n
Posted by Stephen Boyd 1 week, 6 days ago
Quoting Rob Herring (2024-11-08 08:55:32)
> On Fri, Nov 08, 2024 at 09:56:15AM +0100, Geert Uytterhoeven wrote:
> > > > > Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
> > > >
> > > > Doesn't really seem like a fix.
> > >
> > > Ok. Feel free to drop the tag.
> > >
> > > > Does this need to go into 6.12?
> > >
> > > It's only important for 6.12 if kernel configurators want to build the
> > > kernel with OF_OVERLAY_KUNIT_TEST enabled and not be forced to enable
> > > CONFIG_OF_OVERLAY. I don't mind if it waits a while.
> > 
> > I'd say it's a fix, so please keep at last the Fixes tag. Merely
> > enabling kunit tests (which can be modular) should not increase the
> > possible attack vector on a product by enabling extra unneeded code.
> 
> Not sure I buy that that is an actual problem. However, not worth 
> arguing over. I only really care because if there's a Fixes, then this 
> really should go to Linus for 6.12 rather than eventually get 
> auto-selected from 6.13 to go to stable. So I moved it to send to Linus, 
> but I found that CONFIG_OF_OVERLAY is still selected with this patch. 
> That's because the clock kunit tests also select CONFIG_OF_OVERLAY. 
> That's fixed in next, but it's not queued up for 6.12.
> 

I was planning to send the clk patch in the next merge window. Are you
sending the fix now? If you want you can also send the clk side patch
and I can drop it from the clk tree.