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 <leg...@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
