On Fri, Feb 24, 2023 at 05:35:34PM +0000, Marc Zyngier wrote:
> On Fri, 24 Feb 2023 17:21:40 +0000,
> Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
> >
> > On Fri, Feb 24, 2023 at 03:32:51PM +0000, Marc Zyngier wrote:
> > > On Fri, 24 Feb 2023 15:07:53 +0000,
> > > Nick Alcock <nick.alcock@xxxxxxxxxx> wrote:
> > > >
> > > > Since commit 8b41fc4454e ("kbuild: create modules.builtin without
> > > > Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
> > > > are used to identify modules. As a consequence, uses of the macro
> > > > in non-modules will cause modprobe to misidentify their containing
> > > > object file as a module when it is not (false positives), and modprobe
> > > > might succeed rather than failing with a suitable error message.
> > > >
> > > > So remove it in the files in this commit, none of which can be built as
> > > > modules.
> > > >
> > > > Signed-off-by: Nick Alcock <nick.alcock@xxxxxxxxxx>
> > > > Suggested-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> > > > Cc: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> > > > Cc: linux-modules@xxxxxxxxxxxxxxx
> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > > Cc: Hitomi Hasegawa <hasegawa-hitomi@xxxxxxxxxxx>
> > > > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > > > Cc: Marc Zyngier <maz@xxxxxxxxxx>
> > > > Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/irqchip/irq-renesas-rzg2l.c | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > index 25fd8ee66565..4bbfa2b0a4df 100644
> > > > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > @@ -390,4 +390,3 @@ IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
> > > > IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
> > > > MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>");
> > > > MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
> > > > -MODULE_LICENSE("GPL");
> > >
> > > I'm probably missing some context here, but I find it odd to drop
> > > something that is a important piece of information because of what
> > > looks like a tooling regression.
> > >
> > > It also means that once a random driver gets enabled as a module, it
> > > won't load because it is now missing a MODULE_LICENSE() annotation.
> > >
> > > It feels like MODULE_LICENSE should instead degrade to an empty
> > > statement when MODULE isn't defined. Why isn't this approach the
> > > correct one?
> > >
> > > I expect the cover letter would have some pretty good information on
> > > this, but lore.kernel.org doesn't seem to have it at the time I write
> > > this ("Message-ID <20230224150811.80316-1-nick.alcock@xxxxxxxxxx> not
> > > found").
> >
> > The right thing is to not even have this and have the module license
> > inferred from the SPDX tag. But for now we want to remove the tag from
> > things we know for sure are not modules.
>
> I understand that you want to remove it. I don't get why this is the
> right solution. Can you please assume that, in this particular
> instance, I am a complete idiot and spell it out for me?
>
> Why isn't that a problem for modules that are compiled-in?
Modules that are compiled in should succeed with a modprobe call as its
already loaded. The construct we're looking for is a way to detect
things which are built-in but *could* be modules. The annotation today
is done at build time for something built-in using a file path using
modinfo.
All of the module macros which peg .modinfo section information for
built-in code can be extracted from vmlinux using objcopy -j .modinfo, and
that's exactly how modules.builtin.modinfo is built:
objcopy -j .modinfo -O binary vmlinux.o modules.builtin.modinfo
>From this we grep out the "file:" and sed it with a ^kernel prefix.
You can look at the commit 8b41fc4454e ("kbuild: create modules.builtin
without Makefile.modbuiltin or tristate.conf") which did that.
If a module is built-in then MODULE_FILE() is used we and we add a
MODULE_INFO(file, KBUILD_MODFILE), and so the modinfo exists for the
"file:" tag for it. At build time we sed for all those with a kernel prefix
to build the modules.builtin file. That file is used by modprobe to tell
us "yes your module is loaded as its built-in".
So the thing we wish to not have present is when built-in code is being
compiled but *cannot possibly* be module, and we have no way to verify that.
So one way to go about this is to simply *not* use the MODULE_LICENSE()
which cannot possibly be modules so to simplfy the build process. I
think the alternative would be to have a kconfig tristate add a new define for
the case where the tristate was y.
So something like this:
diff --git a/Makefile b/Makefile
index 197dcb83b2e1..f825d0546a7f 100644
--- a/Makefile
+++ b/Makefile
@@ -574,6 +574,7 @@ KBUILD_CFLAGS_KERNEL :=
KBUILD_RUSTFLAGS_KERNEL :=
KBUILD_AFLAGS_MODULE := -DMODULE
KBUILD_CFLAGS_MODULE := -DMODULE
+KBUILD_CFLAGS_MODULE_POSSIBLE := -DMODULE_POSSIBLE
KBUILD_RUSTFLAGS_MODULE := --cfg MODULE
KBUILD_LDFLAGS_MODULE :=
KBUILD_LDFLAGS :=
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 100a386fcd71..13f31766eee3 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -211,12 +211,15 @@ endif
endif
part-of-module = $(if $(filter $(basename $@).o, $(real-obj-m)),y)
+could-be-module = $(if $(filter $(basename $@).o, $(possible-obj-m)),y)
quiet_modtag = $(if $(part-of-module),[M], )
modkern_cflags = \
$(if $(part-of-module), \
$(KBUILD_CFLAGS_MODULE) $(CFLAGS_MODULE), \
- $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL) $(modfile_flags))
+ $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL) $(modfile_flags)) \
+ $(if $(could-be-module),, \
+ $(KBUILD_CFLAGS_MODULE_POSSIBLE)
modkern_rustflags = \
$(if $(part-of-module), \
The difficulty would be the possible-obj-m, to do it without incurring
a huge slow down on the build process.
Luis
