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

[Alexey Gladkov <legion@kernel.org>]

On Mon, May 05, 2025 at 11:38:29AM +0200, 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>
> ---
> 
> v2: As Petr Pavlu suggested, I separated the builtin modules from the external
>     modules. I've also added a search for duplicate modules.
> 
> ---
>  include/linux/module.h   |  4 ----
>  scripts/mod/file2alias.c |  5 +++++
>  scripts/mod/modpost.c    | 35 +++++++++++++++++++++++++++--------
>  scripts/mod/modpost.h    | 15 ++++++++++++++-
>  4 files changed, 46 insertions(+), 13 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..be221923f582 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1509,6 +1509,11 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
>  	typelen = name - type;
>  	name += strlen("__");
>  
> +	if (mod->is_vmlinux) {
> +		mod = find_module(NULL, modname, modnamelen);
> +		mod = mod ?: new_builtin_module(modname, modnamelen);
> +	}
> +
>  	/* Handle all-NULL symbols allocated into .bss */
>  	if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
>  		zeros = calloc(1, sym->st_size);
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index be89921d60b6..db3c172d4528 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -168,22 +168,26 @@ char *get_line(char **stringp)
>  	return orig;
>  }
>  
> -/* A list of all modules we processed */
> +/* A list of all modules (vmlinux or *.ko) we processed */
>  LIST_HEAD(modules);
>  
> -static struct module *find_module(const char *filename, const char *modname)
> +/* A list of all builtin modules we processed */
> +LIST_HEAD(builtin_modules);
> +
> +struct module *find_module(const char *filename, const char *name, size_t namelen)
>  {
>  	struct module *mod;
>  
>  	list_for_each_entry(mod, &modules, list) {
> -		if (!strcmp(mod->dump_file, filename) &&
> -		    !strcmp(mod->name, modname))
> +		if ((mod->dump_file && !strcmp(mod->dump_file, filename)) &&
> +		    namelen != strlen(mod->name) &&

Of course there has to be an '==' here. I'll fix it if this patch fits.

> +		    !strncmp(mod->name, name, namelen))
>  			return mod;
>  	}
>  	return NULL;
>  }
>  
> -static struct module *new_module(const char *name, size_t namelen)
> +struct module *create_module(const char *name, size_t namelen, bool is_builtin)
>  {
>  	struct module *mod;
>  
> @@ -207,7 +211,10 @@ static struct module *new_module(const char *name, size_t namelen)
>  	 */
>  	mod->is_gpl_compatible = true;
>  
> -	list_add_tail(&mod->list, &modules);
> +	if (is_builtin)
> +		list_add_tail(&mod->list, &builtin_modules);
> +	else
> +		list_add_tail(&mod->list, &modules);
>  
>  	return mod;
>  }
> @@ -2021,11 +2028,23 @@ static void write_if_changed(struct buffer *b, const char *fname)
>  static void write_vmlinux_export_c_file(struct module *mod)
>  {
>  	struct buffer buf = { };
> +	struct module_alias *alias, *next;
>  
>  	buf_printf(&buf,
> -		   "#include <linux/export-internal.h>\n");
> +		   "#include <linux/export-internal.h>\n"
> +		   "#include <linux/module.h>\n");
>  
>  	add_exported_symbols(&buf, mod);
> +
> +	list_for_each_entry(mod, &builtin_modules, list) {
> +		list_for_each_entry_safe(alias, next, &mod->aliases, node) {
> +			buf_printf(&buf, "MODULE_ALIAS_MODNAME(\"%s\", \"%s\");\n",
> +					mod->name, alias->str);
> +			list_del(&alias->node);
> +			free(alias);
> +		}
> +	}
> +
>  	write_if_changed(&buf, ".vmlinux.export.c");
>  	free(buf.p);
>  }
> @@ -2112,7 +2131,7 @@ static void read_dump(const char *fname)
>  			continue;
>  		}
>  
> -		mod = find_module(fname, modname);
> +		mod = find_module(fname, modname, strlen(modname));
>  		if (!mod) {
>  			mod = new_module(modname, strlen(modname));
>  			mod->dump_file = fname;
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 9133e4c3803f..1d0dd4ee944a 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -107,7 +107,7 @@ struct module_alias {
>  };
>  
>  /**
> - * struct module - represent a module (vmlinux or *.ko)
> + * struct module - represent a module (vmlinux, a builtin module, or *.ko)
>   *
>   * @dump_file: path to the .symvers file if loaded from a file
>   * @aliases: list head for module_aliases
> @@ -199,6 +199,19 @@ static inline bool is_valid_name(struct elf_info *elf, Elf_Sym *sym)
>  	return !is_mapping_symbol(name);
>  }
>  
> +struct module *find_module(const char *filename, const char *name, size_t namelen);
> +struct module *create_module(const char *name, size_t namelen, bool is_builtin);
> +
> +static inline struct module *new_module(const char *name, size_t namelen)
> +{
> +	return create_module(name, namelen, false);
> +}
> +
> +static inline struct module *new_builtin_module(const char *name, size_t namelen)
> +{
> +	return create_module(name, namelen, true);
> +}
> +
>  /* symsearch.c */
>  void symsearch_init(struct elf_info *elf);
>  void symsearch_finish(struct elf_info *elf);
> -- 
> 2.49.0
> 

-- 
Rgrds, legion