Re: [PATCH v1 5/7] modpost: Create modalias for builtin modules

[Alexey Gladkov <legion@kernel.org>]

On Tue, Apr 29, 2025 at 11:25:47AM +0200, Petr Pavlu wrote:
> On 4/26/25 18:16, Alexey Gladkov wrote:
> > For some modules, modalias is generated using the modpost utility and
> > the section is added to the module file.
> > 
> > When a module is added inside vmlinux, modpost does not generate
> > modalias for such modules and the information is lost.
> > 
> > As a result kmod (which uses modules.builtin.modinfo in userspace)
> > cannot determine that modalias is handled by a builtin kernel module.
> > 
> > $ cat /sys/devices/pci0000:00/0000:00:14.0/modalias
> > pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> > 
> > $ modinfo xhci_pci
> > name:           xhci_pci
> > filename:       (builtin)
> > license:        GPL
> > file:           drivers/usb/host/xhci-pci
> > description:    xHCI PCI Host Controller Driver
> > 
> > Missing modalias "pci:v*d*sv*sd*bc0Csc03i30*" which will be generated by
> > modpost if the module is built separately.
> > 
> > To fix this it is necessary to generate the same modalias for vmlinux as
> > for the individual modules. Fortunately '.vmlinux.export.o' is already
> > generated from which '.modinfo' can be extracted in the same way as for
> > vmlinux.o.
> > 
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> >  include/linux/module.h   |  4 ----
> >  scripts/mod/file2alias.c | 13 ++++++++++++-
> >  scripts/mod/modpost.c    | 21 ++++++++++++++++++---
> >  scripts/mod/modpost.h    |  7 ++++++-
> >  4 files changed, 36 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 7250b4a527ec..6225793ddcd4 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -257,14 +257,10 @@ extern void cleanup_module(void);
> >  	__PASTE(type,			\
> >  	__PASTE(__, name)))))))
> >  
> > -#ifdef MODULE
> >  /* Creates an alias so file2alias.c can find device table. */
> >  #define MODULE_DEVICE_TABLE(type, name)			\
> >  extern typeof(name) __mod_device_table(type, name)	\
> >    __attribute__ ((unused, alias(__stringify(name))))
> > -#else  /* !MODULE */
> > -#define MODULE_DEVICE_TABLE(type, name)
> > -#endif
> >  
> >  /* Version of form [<epoch>:]<version>[-<extra-version>].
> >   * Or for CVS/RCS ID version, everything but the number is stripped.
> > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> > index dff1799a4c79..0fa3f031b904 100644
> > --- a/scripts/mod/file2alias.c
> > +++ b/scripts/mod/file2alias.c
> > @@ -1471,7 +1471,8 @@ static const struct devtable devtable[] = {
> >  /* Create MODULE_ALIAS() statements.
> >   * At this time, we cannot write the actual output C source yet,
> >   * so we write into the mod->dev_table_buf buffer. */
> > -void handle_moddevtable(struct module *mod, struct elf_info *info,
> > +void handle_moddevtable(struct list_head *modules,
> > +			struct module *mod, struct elf_info *info,
> >  			Elf_Sym *sym, const char *symname)
> >  {
> >  	void *symval;
> 
> The new modules parameter is unused.

Indeed. It is no longer needed after the optimization. Thank you.

> > @@ -1509,6 +1510,16 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> >  	typelen = name - type;
> >  	name += strlen("__");
> >  
> > +	if (mod->is_vmlinux) {
> > +		struct module *builtin_mod;
> > +
> > +		builtin_mod = new_module(modname, modnamelen);
> > +		builtin_mod->is_vmlinux = mod->is_vmlinux;
> > +		builtin_mod->dump_file = MODULE_BUILTIN_FNAME;
> 
> The module.dump_file member is described in scripts/mod/modpost.h as
> "path to the .symvers file if loaded from a file". However, that is not
> the case here.
> 
> Similarly, the module struct in scripts/mod/modpost.h is commented as
> "represent a module (vmlinux or *.ko)", but this patch expands its scope
> to also include builtin modules.

Well, an alternative would be to add a separate flag. I used dump_file
because it allows you to exclude such "builtin" modules from processing in
write_dump(), write_namespace_deps_files(), etc.

But yes, I agree that it's an abuse.

> I'm not sure it's best to overload this data in this way. I think mixing
> actual files and "logical" modules in the modules list is somewhat
> confusing.
> 
> An alternative would be to keep a single module struct for vmlinux and
> record the discovered aliases under it?

It is possible to extend struct module_alias and add the module name. The
problem is that alias is added by module_alias_printf() and we will have
to add the module name to the arguments to each do_entry handler in
addition to struct module where there is already a name (but in our case
it is vmlinux).

I can do that if you think it's a better way.

-- 
Rgrds, legion