Re: [PATCH v3 4/6] module: script to generate offset ranges for builtin modules
amp; /^ \./ && $1 == type && NF == 4 {
> + sub(addr_prefix, "0x", $2);
> + addr = strtonum($2);
> + sect_addend[type] = addr - base;
> +
> + if (anchor) {
> + base = 0;
> + type = 0;
> + }
> +
> + next;
> +}
> +
> +# (4) Collect offset ranges (relative to the section base address) for
> built-in
> +# modules.
> +#
> +FC == 3 && /^ \./ && NF == 4 && $3 != "0x0" {
> + type = $1;
> + if (!(type in sect_addend))
> + next;
This assumes sections are 1:1 mapping
between vmlinux.o and vmlinux.
How far does this assumption work?
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION will not work
at least.
As I said in the previous review,
gawk is not documented in Documentation/process/changes.rst
Please add it if you go with gawk.
> +
> + sub(addr_prefix, "0x", $2);
> + addr = strtonum($2) + sect_addend[type];
> +
> + mod = get_module_info($4);
> +# printf "[%s, %08x] %s [%s] %08x\n", mod_name, mod_start, $4, mod, addr;
> + if (mod == mod_name)
> + next;
> +
> + if (mod_name) {
> + idx = mod_start + sect_base[type] + sect_addend[type];
> + entries[idx] = sprintf("%s %08x-%08x %s", type, mod_start,
> addr, mod_name);
> + count[type]++;
> + }
> +# if (mod == "")
> +# printf "ENTRY WITHOUT MOD - MODULE MAY END AT %08x\n", addr
> +
> + mod_name = mod;
> + mod_start = addr;
> +}
> +
> +END {
> + for (type in count) {
> + if (type in sect_anchor)
> + entries[sect_base[type]] = sect_anchor[type];
> + }
> +
> + n = asorti(entries, indices);
> + for (i = 1; i <= n; i++)
> + print entries[indices[i]];
> +}
> --
> 2.43.0
>
--
Best Regards
Masahiro Yamada
Re: [PATCH v3 2/6] trace: add CONFIG_BUILTIN_MODULE_RANGES option
On Fri, May 17, 2024 at 1:30 PM Kris Van Hees wrote: > > The CONFIG_BUILTIN_MODULE_RANGES option controls whether offset range data > is generated for kernel modules that are built into the kernel image. > > Signed-off-by: Kris Van Hees > Reviewed-by: Nick Alcock > Reviewed-by: Alan Maguire > --- > Changes since v2: > - Add explicit dependency on FTRACE for CONFIG_BUILTIN_MODULE_RANGES > --- > kernel/trace/Kconfig | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 47345bf1d4a9f..d0c82b4b3a61e 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -188,6 +188,24 @@ menuconfig FTRACE > > if FTRACE > > +config BUILTIN_MODULE_RANGES > + bool "Generate address range information for builtin modules" > + depends on FTRACE > + select VMLINUX_MAP I still got this warning. WARNING: unmet direct dependencies detected for VMLINUX_MAP Depends on [n]: EXPERT [=n] Selected by [y]: - BUILTIN_MODULE_RANGES [=y] && FTRACE [=y] I recommend changing 'select VMLINUX_MAP' to 'depends on VMLINUX_MAP'. BTW, do you need to put this inside 'if FTRACE'? FTRACE is not required to generate the ranges file. -- Best Regards Masahiro Yamada
Re: [PATCH v3 2/6] trace: add CONFIG_BUILTIN_MODULE_RANGES option
On Fri, May 17, 2024 at 1:30 PM Kris Van Hees wrote: > > The CONFIG_BUILTIN_MODULE_RANGES option controls whether offset range data > is generated for kernel modules that are built into the kernel image. > > Signed-off-by: Kris Van Hees > Reviewed-by: Nick Alcock > Reviewed-by: Alan Maguire > --- > Changes since v2: > - Add explicit dependency on FTRACE for CONFIG_BUILTIN_MODULE_RANGES > --- > kernel/trace/Kconfig | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 47345bf1d4a9f..d0c82b4b3a61e 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -188,6 +188,24 @@ menuconfig FTRACE > > if FTRACE > > +config BUILTIN_MODULE_RANGES > + bool "Generate address range information for builtin modules" > + depends on FTRACE This 'depends on' is redundant because this config is already located between 'if FTRACE' and 'endif'. I believe 2/6 thru 5/6 should be squashed into one commit. Adding only the config option does not make much sense. > + select VMLINUX_MAP > + help > + When modules are built into the kernel, there will be no module name > + associated with its symbols in /proc/kallsyms. Tracers may want to > + identify symbols by module name and symbol name regardless of > whether > + the module is configured as loadable or not. > + > + This option generates modules.builtin.ranges in the build tree with > + offset ranges (per ELF section) for the module(s) they belong to. > + It also records an anchor symbol to determine the load address of > the > + section. > + > + It is fully compatible with CONFIG_RANDOMIZE_BASE and similar late- > + address-modification options. > + > config BOOTTIME_TRACING > bool "Boot-time Tracing support" > depends on TRACING > -- > 2.43.0 > -- Best Regards Masahiro Yamada
Re: [PATCH v3 6/6] module: add install target for modules.builtin.ranges
On Fri, May 17, 2024 at 1:32 PM Kris Van Hees wrote: > > When CONFIG_BUILTIN_MODULE_RANGES is enabled, the modules.builtin.ranges > file should be installed in the module install location. > > Signed-off-by: Kris Van Hees > Reviewed-by: Nick Alcock > --- > Changes since v2: > - Include modules.builtin.ranges in modules install target > --- > scripts/Makefile.modinst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > index 0afd75472679f..f5160ddd74239 100644 > --- a/scripts/Makefile.modinst > +++ b/scripts/Makefile.modinst > @@ -30,10 +30,10 @@ $(MODLIB)/modules.order: modules.order FORCE > quiet_cmd_install_modorder = INSTALL $@ >cmd_install_modorder = sed 's:^\(.*\)\.o$$:kernel/\1.ko:' $< > $@ > > -# Install modules.builtin(.modinfo) even when CONFIG_MODULES is disabled. > -install-y += $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo) > +# Install modules.builtin(.modinfo,.ranges) even when CONFIG_MODULES is > disabled. > +install-y += $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo > modules.builtin.ranges) This will break modules_install when CONFIG_BUILTIN_MODULE_RANGES is disabled. modules.builtin.ranges should be added to install-y conditionally, like this: # Install modules.builtin(.modinfo) even when CONFIG_MODULES is disabled. install-y += $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo) install-$(CONFIG_BUILTIN_MODULE_RANGES) += $(MODLIB)/modules.builtin.ranges > -$(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo): > $(MODLIB)/%: % FORCE > +$(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo > modules.builtin.ranges): $(MODLIB)/%: % FORCE > $(call cmd,install) > > endif > -- > 2.43.0 > -- Best Regards Masahiro Yamada
Re: [PATCH v2 0/6] Generate address range data for built-in modules
On Thu, May 16, 2024 at 1:50 AM Kris Van Hees wrote: > > On Mon, May 13, 2024 at 01:43:15PM +0900, Masahiro Yamada wrote: > > On Sun, May 12, 2024 at 7:42???AM Kris Van Hees > > wrote: > > > > > > Especially for tracing applications, it is convenient to be able to > > > refer to a symbol using a pair and to be able > > > to translate an address into a pair. But > > > that does not work if the module is built into the kernel because the > > > object files that comprise the built-in module implementation are simply > > > linked into the kernel image along with all other kernel object files. > > > > > > This is especially visible when providing tracing scripts for support > > > purposes, where the developer of the script targets a particular kernel > > > version, but does not have control over whether the target system has > > > a particular module as loadable module or built-in module. When tracing > > > symbols within a module, referring them by > > > pairs is both convenient and aids symbol lookup. But that naming will > > > not work if the module name information is lost if the module is built > > > into the kernel on the target system. > > > > > > Earlier work addressing this loss of information for built-in modules > > > involved adding module name information to the kallsyms data, but that > > > required more invasive code in the kernel proper. This work never did > > > get merged into the kernel tree. > > > > > > All that is really needed is knowing whether a given address belongs to > > > a particular module (or multiple modules if they share an object file). > > > Or in other words, whether that address falls within an address range > > > that is associated with one or more modules. > > > > > > This patch series is baaed on Luis Chamberlain's patch to generate > > > modules.builtin.objs, associating built-in modules with their object > > > files. Using this data, vmlinux.o.map and vmlinux.map can be parsed in > > > a single pass to generate a modules.buitin.ranges file with offset range > > > information (relative to the base address of the associated section) for > > > built-in modules. The file gets installed along with the other > > > modules.builtin.* files. > > > > > > > > I still do not want to see modules.builtin.objs. > > > > > > During the vmlinux.o.map parse, every time an object path > > is encountered, you can open the corresponding .cmd file. > > > > > > > > Let's say, you have the following in vmlinux.o.map: > > > > .text 0x007d4fe0 0x46c8 drivers/i2c/i2c-core-base.o > > > > > > > > You can check drivers/i2c/.i2c-core-base.o.cmd > > > > > > $ cat drivers/i2c/.i2c-core-base.o.cmd | tr ' ' '\n' | grep KBUILD_MODFILE > > -DKBUILD_MODFILE='"drivers/i2c/i2c-core"' > > > > > > Now you know this object is part of drivers/i2c/i2c-core > > (that is, its modname is "i2c-core") > > > > > > > > > > Next, you will get the following: > > > > .text 0x007dc550 0x13c4 drivers/i2c/i2c-core-acpi.o > > > > > > $ cat drivers/i2c/.i2c-core-acpi.o.cmd | tr ' ' '\n' | grep KBUILD_MODFILE > > -DKBUILD_MODFILE='"drivers/i2c/i2c-core"' > > > > > > This one is also a part of drivers/i2c/i2c-core > > > > > > You will get the address range of "i2c-core" without changing Makefiles. > > Thank you for this suggestion. I have this approach now implemented, making > use of both KBUILD_MODFILE and KBUILD_MODNAME (both are needed to conclusively > determine that an object belongs to a module). > > However, this is not catching objects that are compiled from assembler source, > because modfile_flags and modname_flags are not added to the assembler flags, > and thus KBUILD_MODFILE and KBUILD_MODNAME are not present in the .cmd file > for those objects. > > It would seem that it is harmless to add those flags to assembler flags, so > would that be an acceptable solution? It definitely would provide consistency > with non-asm objects. And we already pass modfile and modname flags to the > non-asm builds for objects that most certainly do not belong in modules > amnyway, > e.g. > > $ cat arch/x86/boot/.cmdline.o.cmd| tr ' ' '\n' | grep -- -DKBUILD_MOD > -DKBUILD_MODFILE='"arch/x86/boot/cmdline"' > -DKBUILD_MODNAME='"cmdline"' I am fine with passing these to *.S files, as the -D is a preprocessor option. -- Best Regards Masahiro Yamada
Re: [PATCH] modules: Drop the .export_symbol section from the final modules
On Sun, May 12, 2024 at 7:42 AM Luis Chamberlain wrote:
>
> On Wed, Apr 17, 2024 at 01:35:30PM +0800, wang...@lemote.com wrote:
> > From: Wang Yao
> >
> > Commit ddb5cdbafaaa ("kbuild: generate KSYMTAB entries by modpost")
> > forget drop the .export_symbol section from the final modules.
> >
> > Signed-off-by: Wang Yao
>
> Masahiro, commit ddb5cdbafaaa ("kbuild: generate KSYMTAB entries by
> modpost") was your change, wanna address / take it through your
> tree? It makes sense to me though.
Yes, applied now.
Thanks for the reminder.
>
> Luis
>
> > ---
> > scripts/module.lds.S | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> > index bf5bcf2836d8..89ff01a22634 100644
> > --- a/scripts/module.lds.S
> > +++ b/scripts/module.lds.S
> > @@ -13,6 +13,7 @@ SECTIONS {
> > /DISCARD/ : {
> > *(.discard)
> > *(.discard.*)
> > + *(.export_symbol)
> > }
> >
> > __ksymtab 0 : { *(SORT(___ksymtab+*)) }
> > --
> > 2.27.0
> >
--
Best Regards
Masahiro Yamada
Re: [PATCH v2 6/6] module: add install target for modules.builtin.ranges
On Mon, May 13, 2024 at 2:22 PM Masahiro Yamada wrote: > > On Sun, May 12, 2024 at 7:59 AM Kris Van Hees > wrote: > > > > When CONFIG_BUILTIN_MODULE_RANGES is enabled, the modules.builtin.ranges > > file should be installed in the module install location. > > > > Signed-off-by: Kris Van Hees > > Reviewed-by: Nick Alcock > > --- > > Changes since v1: > > - Renamed CONFIG_BUILTIN_RANGES to CONFIG_BUILTIN_MODULE_RANGES > > --- > > scripts/Makefile.modinst | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > > index b45586aa1de49..e185dacae892a 100644 > > --- a/scripts/Makefile.modinst > > +++ b/scripts/Makefile.modinst > > @@ -36,6 +36,11 @@ install-y += $(addprefix $(MODLIB)/, modules.builtin > > modules.builtin.modinfo mod > > $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo > > modules.builtin.objs): $(MODLIB)/%: % FORCE > > $(call cmd,install) > > > > +install-$(CONFIG_BUILTIN_MODULE_RANGES) += $(MODLIB)/modules.builtin.ranges > > + > > +$(MODLIB)/modules.builtin.ranges: modules.builtin.ranges FORCE > > + $(call cmd,install) > > + > > Why add this to $(addprefix ...) part? I meant, why don't you add this to $(addprefix ...) Like this: $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo modules.builtin.ranges): $(MODLIB)/%: % FORCE $(call cmd,install) -- Best Regards Masahiro Yamada
Re: [PATCH v2 6/6] module: add install target for modules.builtin.ranges
On Sun, May 12, 2024 at 7:59 AM Kris Van Hees wrote: > > When CONFIG_BUILTIN_MODULE_RANGES is enabled, the modules.builtin.ranges > file should be installed in the module install location. > > Signed-off-by: Kris Van Hees > Reviewed-by: Nick Alcock > --- > Changes since v1: > - Renamed CONFIG_BUILTIN_RANGES to CONFIG_BUILTIN_MODULE_RANGES > --- > scripts/Makefile.modinst | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > index b45586aa1de49..e185dacae892a 100644 > --- a/scripts/Makefile.modinst > +++ b/scripts/Makefile.modinst > @@ -36,6 +36,11 @@ install-y += $(addprefix $(MODLIB)/, modules.builtin > modules.builtin.modinfo mod > $(addprefix $(MODLIB)/, modules.builtin modules.builtin.modinfo > modules.builtin.objs): $(MODLIB)/%: % FORCE > $(call cmd,install) > > +install-$(CONFIG_BUILTIN_MODULE_RANGES) += $(MODLIB)/modules.builtin.ranges > + > +$(MODLIB)/modules.builtin.ranges: modules.builtin.ranges FORCE > + $(call cmd,install) > + Why add this to $(addprefix ...) part? > endif > > modules := $(call read-file, $(MODORDER)) > -- > 2.43.0 > > -- Best Regards Masahiro Yamada
Re: [PATCH v2 5/6] kbuild: generate modules.builtin.ranges when linking the kernel
On Sun, May 12, 2024 at 7:44 AM Kris Van Hees wrote: > > Signed-off-by: Kris Van Hees > Reviewed-by: Nick Alcock > --- > Changes since v1: > - Renamed CONFIG_BUILTIN_RANGES to CONFIG_BUILTIN_MODULE_RANGES > --- > scripts/Makefile.vmlinux | 17 + > 1 file changed, 17 insertions(+) > > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > index c9f3e03124d7f..54095d72f7fd7 100644 > --- a/scripts/Makefile.vmlinux > +++ b/scripts/Makefile.vmlinux > @@ -36,6 +36,23 @@ targets += vmlinux > vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE > +$(call if_changed_dep,link_vmlinux) > > +# module.builtin.ranges > +# --- > +ifdef CONFIG_BUILTIN_MODULE_RANGES > +__default: modules.builtin.ranges > + > +quiet_cmd_modules_builtin_ranges = GEN $@ > + cmd_modules_builtin_ranges = \ > + $(srctree)/scripts/generate_builtin_ranges.awk \ > + $(filter-out FORCE,$+) > $@ $(filter-out FORCE,$+) -> $(real-prereqs) > + > +vmlinux.map: vmlinux > + > +targets += modules.builtin.ranges > +modules.builtin.ranges: modules.builtin.objs vmlinux.map vmlinux.o.map FORCE > + $(call if_changed,modules_builtin_ranges) > +endif > + > # Add FORCE to the prequisites of a target to force it to be always rebuilt. > # ------- > > -- > 2.43.0 > > -- Best Regards Masahiro Yamada
Re: [PATCH v2 0/6] Generate address range data for built-in modules
h | 4 +- > kernel/trace/Kconfig| 17 > scripts/Makefile.lib| 5 +- > scripts/Makefile.modinst| 11 ++- > scripts/Makefile.vmlinux| 17 > scripts/Makefile.vmlinux_o | 18 - > scripts/generate_builtin_ranges.awk | 149 > > 11 files changed, 228 insertions(+), 10 deletions(-) > create mode 100755 scripts/generate_builtin_ranges.awk > > > base-commit: dd5a440a31fae6e459c0d627162825505361 > -- > 2.42.0 > > -- Best Regards Masahiro Yamada
Re: [PATCH RFC 0/4] Introduce uts_release
On Wed, Feb 21, 2024 at 6:01 PM John Garry wrote:
>
> On 08/02/2024 10:08, John Garry wrote:
> > On 05/02/2024 23:10, Masahiro Yamada wrote:
> >>>> I think what you can contribute are:
> >>>>
> >>>>- Explore the UTS_RELEASE users, and check if you can get rid of it.
> >>> Unfortunately I expect resistance for this. I also expect places like FW
> >>> loader it is necessary. And when this is used in sysfs, people will say
> >>> that it is part of the ABI now.
> >>>
> >>> How about I send the patch to update to use init_uts_ns and mention also
> >>> that it would be better to not use at all, if possible? I can cc you.
> >>
> >> OK.
> >>
> >>
> >> As I mentioned in the previous reply, the replacement is safe
> >> for builtin code.
> >>
> >> When you touch modular code, please pay a little more care,
> >> because UTS_RELEASE and init_utsname()->release
> >> may differ when CONFIG_MODVERSIONS=y.
> >>
> >
> > Are you saying that we may have a different release version kernel and
> > module built with CONFIG_MODVERSIONS=y, and the module was using
> > UTS_RELEASE for something? That something may be like setting some info
> > in a sysfs file, like in this example:
> >
> > static ssize_t target_core_item_version_show(struct config_item *item,
> > char *page)
> > {
> > return sprintf(page, "Target Engine Core ConfigFS Infrastructure %s"
> > " on %s/%s on "UTS_RELEASE"\n", TARGET_CORE_VERSION,
> > utsname()->sysname, utsname()->machine);
> > }
> >
> > And the intention is to use the module codebase release version and not
> > the kernel codebase release version. Hence utsname() is used for
> > .sysname and .machine, but not .release .
>
> Hi Masahiro,
>
> Can you comment on whether I am right about CONFIG_MODVERSIONS, above?
>
> Thanks,
> John
Your understanding about CONFIG_MODVERSIONS is correct.
--
Best Regards
Masahiro Yamada
[PATCH] kbuild: remove EXPERT and !COMPILE_TEST guarding from TRIM_UNUSED_KSYMS
This reverts the following two commits:
- a555bdd0c58c ("Kbuild: enable TRIM_UNUSED_KSYMS again, with some guarding")
- 5cf0fd591f2e ("Kbuild: disable TRIM_UNUSED_KSYMS option")
Commit 5e9e95cc9148 ("kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without
recursion") solved the build time issue.
Signed-off-by: Masahiro Yamada
---
kernel/module/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 0ea1b2970a23..c3ced519e14b 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -362,8 +362,7 @@ config MODPROBE_PATH
userspace can still load modules explicitly).
config TRIM_UNUSED_KSYMS
- bool "Trim unused exported kernel symbols" if EXPERT
- depends on !COMPILE_TEST
+ bool "Trim unused exported kernel symbols"
help
The kernel and some modules make many symbols available for
other modules to use via EXPORT_SYMBOL() and variants. Depending
--
2.40.1
Re: [PATCH RFC 0/4] Introduce uts_release
On Mon, Feb 5, 2024 at 5:25 PM John Garry wrote:
>
> On 02/02/2024 15:01, Masahiro Yamada wrote:
> >> --
> >> 2.35.3
> >
> > As you see, several drivers store UTS_RELEASE in their driver data,
> > and even print it in debug print.
> >
> >
> > I do not see why it is useful.
>
> I would tend to agree, and mentioned that earlier.
>
> > As you discussed in 3/4, if UTS_RELEASE is unneeded,
> > it is better to get rid of it.
>
> Jakub replied about this.
>
> >
> >
> > If such version information is useful for drivers, the intention is
> > whether the version of the module, or the version of vmlinux.
> > That is a question.
> > They differ when CONFIG_MODVERSION.
> >
>
> I think often this information in UTS_RELEASE is shared as informative
> only, so the user can conveniently know the specific kernel git version.
>
> >
> > When module developers intend to printk the git version
> > from which the module was compiled from,
> > presumably they want to use UTS_RELEASE, which
> > was expanded at the compile time of the module.
> >
> > If you replace it with uts_release, it is the git version
> > of vmlinux.
> >
> >
> > Of course, the replacement is safe for always-builtin code.
> >
> >
> >
> > Lastly, we can avoid using UTS_RELEASE without relying
> > on your patch.
> >
> >
> >
> > For example, commit 3a3a11e6e5a2bc0595c7e36ae33c861c9e8c75b1
> > replaced UTS_RELEASE with init_uts_ns.name.release
> >
> >
> > So, is your uts_release a shorthand of init_uts_ns.name.release?
>
> Yes - well that both are strings containing UTS_RELEASE. Using a struct
> sub-member is bit ungainly, but I suppose that we should not be making
> life easy for people using this.
>
> However we already have init_utsname in:
>
> static inline struct new_utsname *init_utsname(void)
> {
> return _uts_ns.name;
> }
>
> So could use init_utsname()->release, which is a bit nicer.
>
> >
> >
> >
> > I think what you can contribute are:
> >
> > - Explore the UTS_RELEASE users, and check if you can get rid of it.
>
> Unfortunately I expect resistance for this. I also expect places like FW
> loader it is necessary. And when this is used in sysfs, people will say
> that it is part of the ABI now.
>
> How about I send the patch to update to use init_uts_ns and mention also
> that it would be better to not use at all, if possible? I can cc you.
OK.
As I mentioned in the previous reply, the replacement is safe
for builtin code.
When you touch modular code, please pay a little more care,
because UTS_RELEASE and init_utsname()->release
may differ when CONFIG_MODVERSIONS=y.
>
> >
> > - Where UTS_RELEASE is useful, consider if it is possible
> > to replace it with init_uts_ns.name.release
>
> ok, but, as above, could use init_utsname()->release also
I am fine with it.
init_utsname()->release is more commonly used
(but less common than UTS_RELEASE)
$ git grep 'init_utsname()->release' | wc
28 922065
$ git grep 'init_uts_ns.name.release' | wc
7 34 587
$ git grep 'UTS_RELEASE' | wc
57 3044741
--
Best Regards
Masahiro Yamada
Re: [PATCH v2 0/3] modules: few of alignment fixes
On Fri, Feb 2, 2024 at 3:05 AM Luis Chamberlain wrote: > > On Wed, Jan 31, 2024 at 02:11:44PM -0800, Luis Chamberlain wrote: > > On Mon, Jan 29, 2024 at 11:26:39AM -0800, Luis Chamberlain wrote: > > > Masahiro, if there no issues feel free to take this or I can take them in > > > too via the modules-next tree. Lemme know! > > > > I've queued this onto modules-testing to get winder testing [0] > > I've moved it to modules-next as I've found no issues. > > Luis I believe this patch series is wrong. I thought we agreed that the alignment must be added to individual asm code, not to the linker script. I am surprised that you came back to this. -- Best Regards Masahiro Yamada
Re: [PATCH RFC 0/4] Introduce uts_release
On Wed, Jan 31, 2024 at 7:49 PM John Garry wrote: > > When hacking it is a waste of time and compute energy that we need to > rebuild much kernel code just for changing the head git commit, like this: > > > touch include/generated/utsrelease.h > > time make -j3 > mkdir -p /home/john/mnt_sda4/john/kernel-dev2/tools/objtool && make > O=/home/john/mnt_sda4/john/kernel-dev2 subdir=tools/objtool > --no-print-directory -C objtool > INSTALL libsubcmd_headers > CALLscripts/checksyscalls.sh > CC init/version.o > AR init/built-in.a > CC kernel/sys.o > CC kernel/module/main.o > AR kernel/module/built-in.a > CC drivers/base/firmware_loader/main.o > CC kernel/trace/trace.o > AR drivers/base/firmware_loader/built-in.a > AR drivers/base/built-in.a > CC net/ethtool/ioctl.o > AR kernel/trace/built-in.a > AR kernel/built-in.a > AR net/ethtool/built-in.a > AR net/built-in.a > AR drivers/built-in.a > AR built-in.a > ... > > Files like drivers/base/firmware_loader/main.c needs to be recompiled as > it includes generated/utsrelease.h for UTS_RELEASE macro, and utsrelease.h > is regenerated when the head commit changes. > > Introduce global char uts_release[] in init/version.c, which this > mentioned code can use instead of UTS_RELEASE, meaning that we don't need > to rebuild for changing the head commit - only init/version.c needs to be > rebuilt. Whether all the references to UTS_RELEASE in the codebase are > proper is a different matter. > > For an x86_64 defconfig build for this series on my old laptop, here is > before and after rebuild time: > > before: > real0m53.591s > user1m1.842s > sys 0m9.161s > > after: > real0m37.481s > user0m46.461s > sys 0m7.199s > > Sending as an RFC as I need to test more of the conversions and I would > like to also convert more UTS_RELEASE users to prove this is proper > approach. > > John Garry (4): > init: Add uts_release > tracing: Use uts_release > net: ethtool: Use uts_release > firmware_loader: Use uts_release > > drivers/base/firmware_loader/main.c | 39 +++-- > include/linux/utsname.h | 1 + > init/version.c | 3 +++ > kernel/trace/trace.c| 4 +-- > net/ethtool/ioctl.c | 4 +-- > 5 files changed, 39 insertions(+), 12 deletions(-) > > -- > 2.35.3 > As you see, several drivers store UTS_RELEASE in their driver data, and even print it in debug print. I do not see why it is useful. As you discussed in 3/4, if UTS_RELEASE is unneeded, it is better to get rid of it. If such version information is useful for drivers, the intention is whether the version of the module, or the version of vmlinux. That is a question. They differ when CONFIG_MODVERSION. When module developers intend to printk the git version from which the module was compiled from, presumably they want to use UTS_RELEASE, which was expanded at the compile time of the module. If you replace it with uts_release, it is the git version of vmlinux. Of course, the replacement is safe for always-builtin code. Lastly, we can avoid using UTS_RELEASE without relying on your patch. For example, commit 3a3a11e6e5a2bc0595c7e36ae33c861c9e8c75b1 replaced UTS_RELEASE with init_uts_ns.name.release So, is your uts_release a shorthand of init_uts_ns.name.release? I think what you can contribute are: - Explore the UTS_RELEASE users, and check if you can get rid of it. - Where UTS_RELEASE is useful, consider if it is possible to replace it with init_uts_ns.name.release -- Best Regards Masahiro Yamada
Re: [PATCH v7] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
On Mon, Jan 1, 2024 at 8:11 PM Alessandro Carminati wrote: > > Hello, > > > > Il giorno dom 31 dic 2023 alle ore 06:35 Masahiro Yamada > ha scritto: > > > > On Tue, Dec 5, 2023 at 6:48 AM Alessandro Carminati (Red Hat) > > wrote: > > > > > > In the kernel environment, situations frequently arise where identical > > > names are shared among symbols within both the core image and modules. > > > While this doesn't pose issues for the kernel's binary itself, it > > > complicates trace or probe operations using tools like kprobe. > > > > > > This patch introduces "kas_alias" to address this challenge. > > > > > > During the kernel's build process, just before linking the vmlinux > > > image in the "scripts/link-vmlinux.sh", symbol name frequencies are > > > collected. > > > This collection includes both the core kernel components and modules. > > > Subsequently, within the same action, the nm data relative to vmlinux > > > is modified by adding aliases based on the comprehensive symbol > > > information gathered. > > > > > > The collection process occurs in two phases: > > > > > > 1. First phase: Executed during the linking of vmlinux, "kas_alias" scans > > >all symbols provided by the 'nm' data against the vmlinux core image > > >and all objects used for module linkage. This phase requires all > > >modules objects to be produced at this stage, thereby adding a vmlinux > > >dependency for linking modules in 'scripts/Makefile.modfinal'. > > > > > > 2. Second phase: In a subsequent run in the same build, "kas_alias" > > >processes module objects and injects aliases into the objects' symbol > > >tables where necessary. This operation is done by modifying > > >'scripts/Makefile.modfinal' to include an action for each processed > > >module. > > > > > > Example: > > > > > > Consider the symbol "device_show", you can expect an output like the > > > following: > > > > > > ~ # cat /proc/kallsyms | grep " name_show" > > > caa2bb4f01c8 t name_show > > > caa2bb4f01c8 t name_show@kernel_irq_irqdesc_c_264 > > > caa2bb9c1a30 t name_show > > > caa2bb9c1a30 t name_show@drivers_pnp_card_c_186 > > > caa2bbac4754 t name_show > > > caa2bbac4754 t name_show@drivers_regulator_core_c_678 > > > caa2bbba4900 t name_show > > > caa2bbba4900 t name_show@drivers_base_power_wakeup_stats_c_93 > > > caa2bbec4038 t name_show > > > caa2bbec4038 t name_show@drivers_rtc_sysfs_c_26 > > > caa2bbecc920 t name_show > > > caa2bbecc920 t name_show@drivers_i2c_i2c_core_base_c_660 > > > caa2bbed3840 t name_show > > > caa2bbed3840 t name_show@drivers_i2c_i2c_dev_c_100 > > > caa2bbef7210 t name_show > > > caa2bbef7210 t name_show@drivers_pps_sysfs_c_66 > > > caa2bbf03328 t name_show > > > caa2bbf03328 t name_show@drivers_hwmon_hwmon_c_72 > > > caa2bbff6f3c t name_show > > > caa2bbff6f3c t name_show@drivers_remoteproc_remoteproc_sysfs_c_215 > > > caa2bbff8d78 t name_show > > > caa2bbff8d78 t name_show@drivers_rpmsg_rpmsg_core_c_455 > > > caa2bbfff7a4 t name_show > > > caa2bbfff7a4 t name_show@drivers_devfreq_devfreq_c_1395 > > > caa2bc001f60 t name_show > > > caa2bc001f60 t name_show@drivers_extcon_extcon_c_389 > > > caa2bc009890 t name_show > > > caa2bc009890 t name_show@drivers_iio_industrialio_core_c_1396 > > > caa2bc01212c t name_show > > > caa2bc01212c t name_show@drivers_iio_industrialio_trigger_c_51 > > > caa2bc025e2c t name_show > > > caa2bc025e2c t name_show@drivers_fpga_fpga_mgr_c_618 > > > caa2a052102c t name_show[hello] > > > caa2a052102c t name_show@hello_hello_c_8[hello] > > > caa2a051955c t name_show[rpmsg_char] > > > caa2a051955c t name_show@drivers_rpmsg_rpmsg_char_c_365 > > > [rpmsg_char] > > > > > > where hello, is a plain helloworld module built OOT. > > > > > > Tested-by: Masami Hiramatsu (Google) > > > Signed-off-by: Alessandro Carminati (Red Hat) > > > > > > > > > --- > > > > > > NOTE1: > > > About the symbols name duplication that happens as consequence of the > > > inclusion compat_binfm
Re: [PATCH v7] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
;,
> dest="target_module", required=False, help="Sets a tharget module to
> operate.")
> +
> +parser.add_argument('-j', "--symbol_frequency",
> dest="symbol_frequency_file", required=True, help="Specify the symbol
> frequency needed to use for producing aliases")
> +parser.add_argument('-z', "--debug", dest="debug", required=False,
> help="Set the debug level.", choices=[f"{level.value}" for level in
> DebugLevel], default="1" )
> +parser.add_argument('-a', "--addr2line", dest="addr2line_file",
> required=True, help="Set the addr2line executable to be used.")
> +parser.add_argument('-b', "--basedir", dest="linux_base_dir",
> required=True, help="Set base directory of the source kernel code.")
> +parser.add_argument('-s', "--separator", dest="separator",
> required=False, help="Set separator, character that separates original name
> from the addr2line data in alias symbols.", default="@", type=SeparatorType())
> +parser.add_argument('-d', "--process_data", dest="process_data_sym",
> required=False, help="Requires the tool to process data symbols along with
> text symbols.", action='store_true')
> +parser.add_argument('-e', "--nm", dest="nm_file", required=True,
> help="Set the nm executable to be used.")
> +
> +config = parser.parse_args()
> +
> +try:
> +# The core_image target is utilized for gathering symbol statistics
> from the core image and modules,
> +# generating aliases for the core image. This target is designed to
> be invoked from scripts/link-vmlinux.sh
> +if config.action == 'core_image':
> +debug_print(config, DebugLevel.INFO.value,"Start core_image
> processing")
> +
> +# Determine kernel source code base directory
> +if not config.linux_base_dir.startswith('/'):
> +config.linux_base_dir = os.path.normpath(os.getcwd() + "/" +
> config.linux_base_dir) + "/"
> +debug_print(config, DebugLevel.DEBUG_BASIC.value,
> f"Configuration: {config}")
> +
> +debug_print(config, DebugLevel.INFO.value, "Process nm data from
> vmlinux")
> +# Process nm data from vmlinux
> +debug_print(config, DebugLevel.DEBUG_BASIC.value,
> f"fetch_file_lines({config.nm_data_file})")
> +vmlinux_nm_lines = fetch_file_lines(config, config.nm_data_file)
> +vmlinux_symbol_list, name_occurrences = parse_nm_lines(config,
> vmlinux_nm_lines)
> +
> +debug_print(config, DebugLevel.INFO.value,"Process nm data for
> modules")
> +# Process nm data for modules
> +debug_print(config, DebugLevel.DEBUG_BASIC.value,
> f"fetch_file_lines({config.nm_data_file})")
> +module_list = fetch_file_lines(config, config.module_list)
> +module_symbol_list = {}
> +for module in module_list:
> +module_nm_lines = do_nm(module, config)
> +module_symbol_list[module], name_occurrences =
> parse_nm_lines(config, module_nm_lines, name_occurrences)
> +
> +debug_print(config, DebugLevel.INFO.value, f"Save
> name_occurrences data: {config.symbol_frequency_file}")
> +with open(config.symbol_frequency_file, 'w') as file:
> +
Re: [PATCH 1/4] linux/export: Fix alignment for 64-bit ksymtab entries
On Sat, Dec 23, 2023 at 5:11 AM Luis Chamberlain wrote: > > On Fri, Dec 22, 2023 at 04:01:30PM +0900, Masahiro Yamada wrote: > > On Fri, Dec 22, 2023 at 3:08 PM Luis Chamberlain wrote: > > > > > > On Thu, Dec 21, 2023 at 10:07:13PM -0800, Luis Chamberlain wrote: > > > > > > > > If we want to go bananas we could even get a graph of size of modules > > > > > > Sorry I meant size of number of symbols Vs cost. > > > > > > Luis > > > > > > > > But, 1/4 is really a bug-fix, isn't it? > > Ah you mean a regression fix, yeah sure, thanks I see ! > > Luis Now, I applied 1/4 to linux-kbuild/fixes. Thanks. -- Best Regards Masahiro Yamada
Re: [PATCH 3/4] vmlinux.lds.h: Fix alignment for __ksymtab*, __kcrctab_* and .pci_fixup sections
On Fri, Dec 22, 2023 at 6:02 PM Helge Deller wrote:
>
> On 12/21/23 14:07, Masahiro Yamada wrote:
> > On Thu, Nov 23, 2023 at 7:18 AM wrote:
> >>
> >> From: Helge Deller
> >>
> >> On 64-bit architectures without CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> >> (e.g. ppc64, ppc64le, parisc, s390x,...) the __KSYM_REF() macro stores
> >> 64-bit pointers into the __ksymtab* sections.
> >> Make sure that the start of those sections is 64-bit aligned in the vmlinux
> >> executable, otherwise unaligned memory accesses may happen at runtime.
> >
> >
> > Are you solving a real problem?
>
> Not any longer.
> I faced a problem on parisc when neither #1 and #3 were applied
> because of a buggy unalignment exception handler. But this is
> not something which I would count a "real generic problem".
>
> > 1/4 already ensures the proper alignment of __ksymtab*, doesn't it?
>
> Yes, it does.
>
> >...
> > So, my understanding is this patch is unneeded.
>
> Yes, it's not required and I'm fine if we drop it.
>
> But regarding __kcrctab:
>
> >> @@ -498,6 +501,7 @@
> >> } \
> >> \
> >> /* Kernel symbol table: Normal symbols */ \
> >> + . = ALIGN(4); \
> >> __kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
> >> __start___kcrctab = .; \
> >> KEEP(*(SORT(___kcrctab+*))) \
>
> I think this patch would be beneficial to get proper alignment:
>
> diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
> index cd253eb51d6c..d445705ac13c 100644
> --- a/include/linux/export-internal.h
> +++ b/include/linux/export-internal.h
> @@ -64,6 +64,7 @@
>
> #define SYMBOL_CRC(sym, crc, sec) \
> asm(".section \"___kcrctab" sec "+" #sym "\",\"a\"" "\n" \
> + ".balign 4" "\n" \
> "__crc_" #sym ":" "\n" \
> ".long " #crc "\n" \
> ".previous" "\n")
Yes!
Please send a patch with this:
Fixes: f3304ecd7f06 ("linux/export: use inline assembler to populate
symbol CRCs")
--
Best Regards
Masahiro Yamada
Re: [PATCH 0/4] Section alignment issues?
On Fri, Dec 22, 2023 at 5:23 PM Helge Deller wrote: > > On 12/21/23 16:42, Masahiro Yamada wrote: > > On Thu, Dec 21, 2023 at 10:40 PM Masahiro Yamada > > wrote: > >> > >> On Thu, Nov 23, 2023 at 7:18 AM wrote: > >>> > >>> From: Helge Deller > >>> > >>> While working on the 64-bit parisc kernel, I noticed that the __ksymtab[] > >>> table was not correctly 64-bit aligned in many modules. > >>> The following patches do fix some of those issues in the generic code. > >>> > >>> But further investigation shows that multiple sections in the kernel and > >>> in > >>> modules are possibly not correctly aligned, and thus may lead to > >>> performance > >>> degregations at runtime (small on x86, huge on parisc, sparc and others > >>> which > >>> need exception handlers). Sometimes wrong alignments may also be simply > >>> hidden > >>> by the linker or kernel module loader which pulls in the sections by luck > >>> with > >>> a correct alignment (e.g. because the previous section was aligned > >>> already). > >>> > >>> An objdump on a x86 module shows e.g.: > >>> > >>> ./kernel/net/netfilter/nf_log_syslog.ko: file format elf64-x86-64 > >>> Sections: > >>> Idx Name Size VMA LMA File off > >>> Algn > >>>0 .text 1fdf 0040 > >>> 2**4 > >>>CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE > >>>1 .init.text00f6 2020 > >>> 2**4 > >>>CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE > >>>2 .exit.text005c 2120 > >>> 2**4 > >>>CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE > >>>3 .rodata.str1.8 00dc > >>> 2180 2**3 > >>>CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>4 .rodata.str1.1 030a > >>> 225c 2**0 > >>>CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>5 .rodata 00b0 2580 > >>> 2**5 > >>>CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>6 .modinfo 019e 2630 > >>> 2**0 > >>>CONTENTS, ALLOC, LOAD, READONLY, DATA > >>>7 .return_sites 0034 27ce > >>> 2**0 > >>>CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA > >>>8 .call_sites 029c 2802 > >>> 2**0 > >>>CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA > >>> > >>> In this example I believe the ".return_sites" and ".call_sites" should > >>> have > >>> an alignment of at least 32-bit (4 bytes). > >>> > >>> On other architectures or modules other sections like ".altinstructions" > >>> or > >>> "__ex_table" may show up wrongly instead. > >>> > >>> In general I think it would be beneficial to search for wrong alignments > >>> at > >>> link time, and maybe at runtime. > >>> > >>> The patch at the end of this cover letter > >>> - adds compile time checks to the "modpost" tool, and > >>> - adds a runtime check to the kernel module loader at runtime. > >>> And it will possibly show false positives too (!!!) > >>> I do understand that some of those sections are not performce critical > >>> and thus any alignment is OK. > >>> > >>> The modpost patch will emit at compile time such warnings (on x86-64 > >>> kernel build): > >>> > >>> WARNING: modpost: vmlinux: section .initcall7.init (type 1, flags 2) has > >>> alignment of 1, expected at least 4. > >>> Maybe you need to add ALIGN() to the modules.lds file (or fix modpost) ? > >>> WARNING: modpost: vmlinux: section .altinstructions (type 1, flags 2) has > >>> alignment of 1, expected at least 2. > >>>
Re: [PATCH 1/4] linux/export: Fix alignment for 64-bit ksymtab entries
On Fri, Dec 22, 2023 at 3:08 PM Luis Chamberlain wrote:
>
> On Thu, Dec 21, 2023 at 10:07:13PM -0800, Luis Chamberlain wrote:
> >
> > If we want to go bananas we could even get a graph of size of modules
>
> Sorry I meant size of number of symbols Vs cost.
>
> Luis
But, 1/4 is really a bug-fix, isn't it?
ksymtab was previously 8-byte aligned for CONFIG_64BIT,
but now is only 4-byte aligned.
$ git show ddb5cdbafaaa^:include/linux/export.h |
head -n66 | tail -n5
struct kernel_symbol {
unsigned long value;
const char *name;
const char *namespace;
};
$ git show ddb5cdbafaaa^:include/asm-generic/export.h |
head -23 | tail -8
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
#define KSYM_ALIGN 4
#elif defined(CONFIG_64BIT)
#define KSYM_ALIGN 8
#else
#define KSYM_ALIGN 4
#endif
In the old behavior, used C code
for producing ksymtab, hence it was naturally
aligned by the compiler. (unsigned long and pointer
require 8-byte alignment for CONFIG_64BIT)
explicitly required
8-byte alignment for CONFIG_64BIT.
In the current behavior,
produces all ksymtab by using inline assembler,
but it hard-codes ".balign 4".
--
Best Regards
Masahiro Yamada
Re: [PATCH 1/4] linux/export: Fix alignment for 64-bit ksymtab entries
On Thu, Dec 21, 2023 at 7:22 PM Masahiro Yamada wrote:
>
> On Thu, Nov 23, 2023 at 7:18 AM wrote:
> >
> > From: Helge Deller
> >
> > An alignment of 4 bytes is wrong for 64-bit platforms which don't define
> > CONFIG_HAVE_ARCH_PREL32_RELOCATIONS (which then store 64-bit pointers).
> > Fix their alignment to 8 bytes.
> >
> > Signed-off-by: Helge Deller
>
>
> This is correct.
>
> Acked-by: Masahiro Yamada
>
> Please add
>
>
> Fixes: ddb5cdbafaaa ("kbuild: generate KSYMTAB entries by modpost")
>
>
If there is no objection, I will pick this up
to linux-kbuild/fixes.
Thanks.
--
Best Regards
Masahiro Yamada
Re: [PATCH 0/4] Section alignment issues?
On Thu, Dec 21, 2023 at 10:40 PM Masahiro Yamada wrote: > > On Thu, Nov 23, 2023 at 7:18 AM wrote: > > > > From: Helge Deller > > > > While working on the 64-bit parisc kernel, I noticed that the __ksymtab[] > > table was not correctly 64-bit aligned in many modules. > > The following patches do fix some of those issues in the generic code. > > > > But further investigation shows that multiple sections in the kernel and in > > modules are possibly not correctly aligned, and thus may lead to performance > > degregations at runtime (small on x86, huge on parisc, sparc and others > > which > > need exception handlers). Sometimes wrong alignments may also be simply > > hidden > > by the linker or kernel module loader which pulls in the sections by luck > > with > > a correct alignment (e.g. because the previous section was aligned already). > > > > An objdump on a x86 module shows e.g.: > > > > ./kernel/net/netfilter/nf_log_syslog.ko: file format elf64-x86-64 > > Sections: > > Idx Name Size VMA LMA File off > > Algn > > 0 .text 1fdf 0040 > > 2**4 > > CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE > > 1 .init.text00f6 2020 > > 2**4 > > CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE > > 2 .exit.text005c 2120 > > 2**4 > > CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE > > 3 .rodata.str1.8 00dc 2180 > > 2**3 > > CONTENTS, ALLOC, LOAD, READONLY, DATA > > 4 .rodata.str1.1 030a 225c > > 2**0 > > CONTENTS, ALLOC, LOAD, READONLY, DATA > > 5 .rodata 00b0 2580 > > 2**5 > > CONTENTS, ALLOC, LOAD, READONLY, DATA > > 6 .modinfo 019e 2630 > > 2**0 > > CONTENTS, ALLOC, LOAD, READONLY, DATA > > 7 .return_sites 0034 27ce > > 2**0 > > CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA > > 8 .call_sites 029c 2802 > > 2**0 > > CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA > > > > In this example I believe the ".return_sites" and ".call_sites" should have > > an alignment of at least 32-bit (4 bytes). > > > > On other architectures or modules other sections like ".altinstructions" or > > "__ex_table" may show up wrongly instead. > > > > In general I think it would be beneficial to search for wrong alignments at > > link time, and maybe at runtime. > > > > The patch at the end of this cover letter > > - adds compile time checks to the "modpost" tool, and > > - adds a runtime check to the kernel module loader at runtime. > > And it will possibly show false positives too (!!!) > > I do understand that some of those sections are not performce critical > > and thus any alignment is OK. > > > > The modpost patch will emit at compile time such warnings (on x86-64 kernel > > build): > > > > WARNING: modpost: vmlinux: section .initcall7.init (type 1, flags 2) has > > alignment of 1, expected at least 4. > > Maybe you need to add ALIGN() to the modules.lds file (or fix modpost) ? > > WARNING: modpost: vmlinux: section .altinstructions (type 1, flags 2) has > > alignment of 1, expected at least 2. > > WARNING: modpost: vmlinux: section .initcall6.init (type 1, flags 2) has > > alignment of 1, expected at least 4. > > WARNING: modpost: vmlinux: section .initcallearly.init (type 1, flags 2) > > has alignment of 1, expected at least 4. > > WARNING: modpost: vmlinux: section .rodata.cst2 (type 1, flags 18) has > > alignment of 2, expected at least 64. > > WARNING: modpost: vmlinux: section .static_call_tramp_key (type 1, flags 2) > > has alignment of 1, expected at least 8. > > WARNING: modpost: vmlinux: section .con_initcall.init (type 1, flags 2) has > > alignment of 1, expected at least 8. > > WARNING: modpost: vmlinux: section __bug_table (type 1, flags 3) has > > alignment of 1, expected at least 4. > > ... > > > > > modpost acts on vmlinux.o instead of vmlinux. > > > vmli
Re: [PATCH 0/4] Section alignment issues?
*/
> + { 1,".altinstr_replacement" },
> + { 1,".altinstr_aux" },
> + { 4,".call_sites" },
> + { 4,".return_sites" },
> + { 1,".note.Linux" },/* correct? */
> + { 4,"__ex_table" },
> + { 4,".initcall" }, /* at least 4 ? */
> { 0,NULL }
> };
>
> @@ -1545,16 +1554,17 @@ static void check_sec_alignment(struct module *mod,
> struct elf_info *elf)
> should_shalign = target_64bit ? 8 : 4;
> if (match(sec, prel32_sec_list))
> should_shalign = target_prel32_relocations ? 4 :
> should_shalign;
> - else if (strstr(sec, "text")) /* assume text segments to be
> 4-byte aligned */
> - should_shalign = 4;
> else if (match(sec, byte_sec_list)) /* any alignment is OK. */
> continue;
> + else if (strstr(sec, "text")) /* assume text segments to be
> 4-byte aligned */
> + should_shalign = 4;
> else {
> /* search in list with special alignment */
> - k = 0;
> - while (special_list[k].align && strstarts(sec,
> special_list[k].name)) {
> - should_shalign = special_list[k].align;
> - break;
> + for (k = 0; special_list[k].align; k++) {
> + if (strstarts(sec, special_list[k].name)) {
> + should_shalign =
> special_list[k].align;
> + break;
> + }
> }
> }
>
> Helge Deller (4):
> linux/export: Fix alignment for 64-bit ksymtab entries
> modules: Ensure 64-bit alignment on __ksymtab_* sections
> vmlinux.lds.h: Fix alignment for __ksymtab*, __kcrctab_* and
> .pci_fixup sections
> modules: Add missing entry for __ex_table
>
> include/asm-generic/vmlinux.lds.h | 5 +
> include/linux/export-internal.h | 5 -
> scripts/module.lds.S | 9 +
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
Re: [PATCH 3/4] vmlinux.lds.h: Fix alignment for __ksymtab*, __kcrctab_* and .pci_fixup sections
table: GPL-only symbols */ \
> + . = ALIGN(4); \
> __kcrctab_gpl : AT(ADDR(__kcrctab_gpl) - LOAD_OFFSET) { \
> __start___kcrctab_gpl = .; \
> KEEP(*(SORT(___kcrctab_gpl+*))) \
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
Re: [PATCH 1/4] linux/export: Fix alignment for 64-bit ksymtab entries
On Thu, Nov 23, 2023 at 7:18 AM wrote:
>
> From: Helge Deller
>
> An alignment of 4 bytes is wrong for 64-bit platforms which don't define
> CONFIG_HAVE_ARCH_PREL32_RELOCATIONS (which then store 64-bit pointers).
> Fix their alignment to 8 bytes.
>
> Signed-off-by: Helge Deller
This is correct.
Acked-by: Masahiro Yamada
Please add
Fixes: ddb5cdbafaaa ("kbuild: generate KSYMTAB entries by modpost")
> ---
> include/linux/export-internal.h | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
> index 69501e0ec239..cd253eb51d6c 100644
> --- a/include/linux/export-internal.h
> +++ b/include/linux/export-internal.h
> @@ -16,10 +16,13 @@
> * and eliminates the need for absolute relocations that require runtime
> * processing on relocatable kernels.
> */
> +#define __KSYM_ALIGN ".balign 4"
> #define __KSYM_REF(sym)".long " #sym "- ."
> #elif defined(CONFIG_64BIT)
> +#define __KSYM_ALIGN ".balign 8"
> #define __KSYM_REF(sym)".quad " #sym
> #else
> +#define __KSYM_ALIGN ".balign 4"
> #define __KSYM_REF(sym)".long " #sym
> #endif
>
> @@ -42,7 +45,7 @@
> " .asciz \"" ns "\"" "\n"
> \
> " .previous" "\n"
> \
> " .section \"___ksymtab" sec "+" #name "\", \"a\"""\n"
> \
> - " .balign 4" "\n"
> \
> + __KSYM_ALIGN "\n"
> \
> "__ksymtab_" #name ":" "\n"
> \
> __KSYM_REF(sym) "\n"
> \
> __KSYM_REF(__kstrtab_ ##name) "\n"
> \
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
Re: [PATCH 0/2] kmod /usr support
On Thu, Dec 7, 2023 at 3:37 AM Lucas De Marchi wrote:
>
> On Fri, Nov 10, 2023 at 01:13:53PM +0100, Michal Suchanek wrote:
> >Hello,
> >
> >This is resend of the last patch in the series that adds prefix support
> >to kernel module location together with additional patch for validating
> >the user supplied input to options that are interpreted as directories.
> >
> >Thanks
>
> applied, thanks
>
> Lucas De Marchi
If I understood this correctly, MODULE_DIRECTORY is determined
by "configure --with-module-directory=...", and there is no
way to change it after that.
If so, how to work with cross-building?
Cross-building is typical when building embedded Linux systems.
Consider this scenario:
- Your build machine adopts
MODULE_DIRECTORY=/usr/lib/modules
- The target embedded system adopts
MODULE_DIRECTORY=/lib/modules
(or vice a versa)
depmod is used also for cross-building because
it is executed as a part of "make module_install".
The counterpart patch set for Kbuild provides
KERNEL_MODULE_DIRECTORY, which only changes
the destination directory to which *.ko are copied.
You cannot change the directory where the
depmod searches for modules, as it is fixed
at the compile-time of kmod.
In this case, what we can do is to build another
instance of kmod configured for the target system,
and use it for modules_install:
1. In the kmod source directory
./configure --with=module-directory=/lib/modules
make
2. make modules_install INSTALL_MOD_PATH=
KERNEL_MODULE_DIRECTORY=/lib/modules
DEPMOD=
If you use OpenEmbedded etc., this is what you do
because host tools are built from sources.
But, should it be required all the time?
Even when the target embedded system uses
busybox-based modprobe instead of kmod?
depmod provides --basedir option, which changes
the prefix part, but there is no way to override
the stem part, MODULE_DIRECTRY.
In the review of the counter patch set,
I am suggesting an option to override MODULE_DIRECTRY
(let's say --moduledir) at least for depmod.
(Perhaps modinfo too, as it also supports --basedir)
Then, we can change scripts/depmod.sh so that
Kbuild can propagate KERNEL_MODULE_DIRECTORY
to depmod.
if ; then
set -- "$@" --moduledir "${KERNEL_MODULE_DIRECTORY}"
fi
Does it make sense?
--
Best Regards
Masahiro Yamada
Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Tue, Dec 12, 2023 at 10:12 PM Michal Suchánek wrote: > > On Mon, Dec 11, 2023 at 01:33:23PM +0900, Masahiro Yamada wrote: > > On Mon, Dec 11, 2023 at 6:09 AM Michal Suchánek wrote: > > > > > > On Mon, Dec 11, 2023 at 03:44:35AM +0900, Masahiro Yamada wrote: > > > > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek > > > > wrote: > > > > > > > > > > The default MODLIB value is composed of three variables > > > > > > > > > > MODLIB = > > > > > $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE) > > > > > > > > > > However, the kernel.spec hadcodes the default value of > > > > > $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when > > > > > building the package. > > > > > > > > > > Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem. > > > > > > > > > > Signed-off-by: Michal Suchanek > > > > > --- > > > > > Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY > > > > > > > > > > > > The SRPM package created by 'make srcrpm-pkg' may not work > > > > if rpmbuild is executed in a different machine. > > > > > > That's why there is an option to override KERNEL_MODULE_DIRECTORY? > > > > > > Yes. > > But, as I pointed out in 1/2, depmod must follow the packager's decision. > > > > 'make srcrpm-pkg' creates a SRPM on machine A. > > 'rpmbuild' builds it into binary RPMs on machine B. > > > > If A and B disagree about kmod.pc, depmod will fail > > because there is no code to force the decision made > > on machine A. > > There is. It's the ?= in the top Makefile. Nope. Only Kbuild follows the specified KERNEL_MODULE_DIRECTORY. depmod still uses the MODULE_DRECTORY determined when it was compiled. > > Currently the test that determines the module directory uses make logic > so it's not possible to pass on the shell magic before executing it so > it could be executed inside the rpm spec file as well. > > OUtsourcing it into an external script would mean that the sources need > to be unpacked before the script can be executed. That would require > using dynamically generated file list in the spec file because the > module location would not be known at spec parse time. Possible but > convoluted. I do not require that. This is simple; builders must follow the packager's decision. To make it work, depmod must follow MODULE_DIRECTORY given from an external environment. > In the end I do not think this is a problem that needs solving. Most > distributions that build kernel packages would use their own packaging > files, not rpm-pkg. That limits rpm-pkg to ad-hoc use when people want > to build one-off test kernel. It's reasonable to do on the same > distribution as the target system. The option to do so on a distribution > with different module directory is available if somebody really needs > that. > > Thanks > > Michal -- Best Regards Masahiro Yamada
Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory
On Tue, Dec 12, 2023 at 10:03 PM Michal Suchánek wrote:
>
> On Mon, Dec 11, 2023 at 01:29:15PM +0900, Masahiro Yamada wrote:
> > On Mon, Dec 11, 2023 at 6:07 AM Michal Suchánek wrote:
> > >
> > > Hello!
> > >
> > > On Mon, Dec 11, 2023 at 03:43:44AM +0900, Masahiro Yamada wrote:
> > > > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek
> > > > wrote:
> > > > >
> > > > > Some distributions aim at shipping all files in /usr.
> > > > >
> > > > > The path under which kernel modules are installed is hardcoded to /lib
> > > > > which conflicts with this goal.
> > > > >
> > > > > When kmod provides kmod.pc, use it to determine the correct module
> > > > > installation path.
> > > > >
> > > > > With kmod that does not provide the config /lib/modules is used as
> > > > > before.
> > > > >
> > > > > While pkg-config does not return an error when a variable does not
> > > > > exist
> > > > > the kmod configure script puts some effort into ensuring that
> > > > > module_directory is non-empty. With that empty module_directory from
> > > > > pkg-config can be used to detect absence of the variable.
> > > > >
> > > > > Signed-off-by: Michal Suchanek
> > > > > ---
> > > > > v6:
> > > > > - use ?= instead of := to make it easier to override the value
> > > >
> > > >
> > > > "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> > > > will override the install destination, but
> > > > depmod will not be not aware of it.
> > >
> > > At the same time if you know what you are doing you can build a src rpm
> > > for another system that uses a different location.
> > >
> > > > How to avoid the depmod error?
> > >
> > > Not override the variable?
> >
> > You are not answering my question.
> > You intentionally changed := to ?=.
> >
> > This implies that KERNEL_MODULE_DIRECTORY is an interface to users,
> > and should be documented in Documentation/kbuild/kbuild.rst
>
> That's reasonable
>
> > However, it never works if it is overridden from the env variable
> > or make command line because there is no way to let depmod know
> > the fact that KERNEL_MODULE_DIRECTORY has been overridden.
>
> And there should not. kmod is not aware, kbuild is. That's the
> direction of information flow. kmod defines where it looks for the
> modules, and kbuild shoukld install the modules there.
Then, you cannot explain why KERNEL_MODULE_DIRECTORY should be exposed
as a user interface.
The MODULE_DIRECTORY in depmod is determined when kmod is compiled.
Kbuild takes KERNEL_MODULE_DIRECTORY from pkg-config.
If these two do not agree, it never works.
> If the user knows better (eg. possibility of building src-rpm for a
> different you brought up) they can override the autodetection.
No, it does not work.
The user has no way to override the MODULE_DIRECTORY in depmod.
> > In my understanding, depmod does not provide an option to
> > specify the module directory from a command line option, does it?
>
> No it does not.
>
> > If not, is it reasonable to add a new option to depmod?
>
> I don't think so. The module directory is intentionally in a fixed
> location. It can be set at compile time, and that's it.
>
> Then when running depmod on the target distribution either kbuild and
> kmod agree on the location or the build fails. That's the intended
> outcome.
>
> kmod recently grew the ability to use modules outside of module
> directory. For that to work internally the path to these out-of-kernel
> modules is stored as absolute path, and the path of modules that are in
> the module directory is stored relative to the module directory.
>
> Setting the module directory location dynamically still should not break
> this but I am not sure it's a great idea. In the end modprobe needs to
> find those modules, and if depmod puts the modules.dep in arbitrary
> location it will not.
That is true when modules are compiled and installed on the local machine.
If you create an SRPM with KERNEL_MODULE_DIRECTORY,
builders must follow it.
>
> > depmod provides the "-b basedir" option, but it only allows
> > adding a prefix to the default "/lib/modules/".
>
> Yes, that's for installation into a staging directory, and there again
> the modules that are inside the module directory are considedred
> 'in-kernel'. Not sure how well this even works with 'out-of-kernel'
> modules.
>
> > (My original idea to provide the prefix_part, it would have worked
> > like -b "${INSTALL_MOD_PATH}${MOD_PREFIX}", which you refused)
>
> It's not clear that adding a prefix covers all use cases. It is an
> arbitrary limitation that the module path must end with '/lib/modules'.
>
> It may allow taking some shortcuts in some places but is unnecessarily
> limiting.
>
> Thanks
>
> Michal
--
Best Regards
Masahiro Yamada
Re: Building signed debs
On Mon, Dec 11, 2023 at 7:19 PM Tom Cook wrote:
>
> On Sat, Dec 9, 2023 at 6:18 PM Masahiro Yamada wrote:
> > On Fri, Dec 8, 2023 at 8:14 PM Tom Cook wrote:
> > >
> > > I'm trying to build a signed .deb kernel package of
> > > https://github.com/torvalds/linux/tree/v6.6. I've copied
> > > certs/default_x509.genkey to certs/x509.genkey. The .config is the
> > > one from Ubuntu 23.10's default kernel with all new options accepted
> > > at their default and CONFIG_SYSTEM_TRUSTED_KEYS="" and
> > > CONFIG_SYSTEM_REVOCATION_KEYS="".
> > >
> > > This builds the kernel and modules, signs the modules, compresses the
> > > modules and then attempts to sign the modules again. That fails,
> > > because the .ko module files are now .ko.zst files and the file it's
> > > trying to sign isn't there. Full failure is pasted below.
> >
> >
> >
> > Modules are signed before the compression.
>
> Reading back through my earlier email, I wasn't clear that when I said "This
> builds the kernel..." I meant "`make deb-pkg` builds the kernel". I'm not
> doing anything outlandish here, I think, just expecting the build system to
> work.
>
> Regards,
> Tom
OK, I understood.
The error is caused by the following lines
in scripts/package/builddeb:
# re-sign stripped modules
if is_enabled CONFIG_MODULE_SIG_ALL; then
${MAKE} -f ${srctree}/Makefile
INSTALL_MOD_PATH="${image_pdir}" modules_sign
fi
The error happens when all of the following three
are enabled.
- CONFIG_DEBUG_INFO
- CONFIG_MODULE_SIG_ALL
- CONFIG_MODULE_COMPRESS_*
If you disable one of them, you can do deb-pkg.
--
Best Regards
Masahiro Yamada
Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Mon, Dec 11, 2023 at 6:09 AM Michal Suchánek wrote:
>
> On Mon, Dec 11, 2023 at 03:44:35AM +0900, Masahiro Yamada wrote:
> > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek wrote:
> > >
> > > The default MODLIB value is composed of three variables
> > >
> > > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> > >
> > > However, the kernel.spec hadcodes the default value of
> > > $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> > > building the package.
> > >
> > > Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
> > >
> > > Signed-off-by: Michal Suchanek
> > > ---
> > > Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
> >
> >
> > The SRPM package created by 'make srcrpm-pkg' may not work
> > if rpmbuild is executed in a different machine.
>
> That's why there is an option to override KERNEL_MODULE_DIRECTORY?
Yes.
But, as I pointed out in 1/2, depmod must follow the packager's decision.
'make srcrpm-pkg' creates a SRPM on machine A.
'rpmbuild' builds it into binary RPMs on machine B.
If A and B disagree about kmod.pc, depmod will fail
because there is no code to force the decision made
on machine A.
> Thanks
>
> Michal
>
> >
> >
> >
> > %{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot}
> > KERNEL_MODULE_DIRECTORY=%{KERNEL_MODULE_DIRECTORY} modules_install
> >
> >
> > will align with the specified install destination,
> > but depmod will still fail.
> > (same issue as 1/2)
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > ---
> > > scripts/package/kernel.spec | 8
> > > scripts/package/mkspec | 1 +
> > > 2 files changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> > > index 3eee0143e0c5..12996ed365f8 100644
> > > --- a/scripts/package/kernel.spec
> > > +++ b/scripts/package/kernel.spec
> > > @@ -67,7 +67,7 @@ cp $(%{make} %{makeflags} -s image_name)
> > > %{buildroot}/boot/vmlinuz-%{KERNELRELEA
> > > %{make} %{makeflags} INSTALL_HDR_PATH=%{buildroot}/usr headers_install
> > > cp System.map %{buildroot}/boot/System.map-%{KERNELRELEASE}
> > > cp .config %{buildroot}/boot/config-%{KERNELRELEASE}
> > > -ln -fns /usr/src/kernels/%{KERNELRELEASE}
> > > %{buildroot}/lib/modules/%{KERNELRELEASE}/build
> > > +ln -fns /usr/src/kernels/%{KERNELRELEASE}
> > > %{buildroot}%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> > > %if %{with_devel}
> > > %{make} %{makeflags} run-command
> > > KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build
> > > %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> > > %endif
> > > @@ -98,8 +98,8 @@ fi
> > >
> > > %files
> > > %defattr (-, root, root)
> > > -/lib/modules/%{KERNELRELEASE}
> > > -%exclude /lib/modules/%{KERNELRELEASE}/build
> > > +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}
> > > +%exclude %{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> > > /boot/*
> > >
> > > %files headers
> > > @@ -110,5 +110,5 @@ fi
> > > %files devel
> > > %defattr (-, root, root)
> > > /usr/src/kernels/%{KERNELRELEASE}
> > > -/lib/modules/%{KERNELRELEASE}/build
> > > +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> > > %endif
> > > diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> > > index ce201bfa8377..e952fa4f2937 100755
> > > --- a/scripts/package/mkspec
> > > +++ b/scripts/package/mkspec
> > > @@ -24,6 +24,7 @@ fi
> > > cat< > > %define ARCH ${ARCH}
> > > %define KERNELRELEASE ${KERNELRELEASE}
> > > +%define KERNEL_MODULE_DIRECTORY ${KERNEL_MODULE_DIRECTORY}
> > > %define pkg_release $("${srctree}/init/build-version")
> > > EOF
> > >
> > > --
> > > 2.42.0
> > >
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
--
Best Regards
Masahiro Yamada
Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory
On Mon, Dec 11, 2023 at 6:07 AM Michal Suchánek wrote:
>
> Hello!
>
> On Mon, Dec 11, 2023 at 03:43:44AM +0900, Masahiro Yamada wrote:
> > On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek wrote:
> > >
> > > Some distributions aim at shipping all files in /usr.
> > >
> > > The path under which kernel modules are installed is hardcoded to /lib
> > > which conflicts with this goal.
> > >
> > > When kmod provides kmod.pc, use it to determine the correct module
> > > installation path.
> > >
> > > With kmod that does not provide the config /lib/modules is used as
> > > before.
> > >
> > > While pkg-config does not return an error when a variable does not exist
> > > the kmod configure script puts some effort into ensuring that
> > > module_directory is non-empty. With that empty module_directory from
> > > pkg-config can be used to detect absence of the variable.
> > >
> > > Signed-off-by: Michal Suchanek
> > > ---
> > > v6:
> > > - use ?= instead of := to make it easier to override the value
> >
> >
> > "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install"
> > will override the install destination, but
> > depmod will not be not aware of it.
>
> At the same time if you know what you are doing you can build a src rpm
> for another system that uses a different location.
>
> > How to avoid the depmod error?
>
> Not override the variable?
You are not answering my question.
You intentionally changed := to ?=.
This implies that KERNEL_MODULE_DIRECTORY is an interface to users,
and should be documented in Documentation/kbuild/kbuild.rst
However, it never works if it is overridden from the env variable
or make command line because there is no way to let depmod know
the fact that KERNEL_MODULE_DIRECTORY has been overridden.
In my understanding, depmod does not provide an option to
specify the module directory from a command line option, does it?
If not, is it reasonable to add a new option to depmod?
depmod provides the "-b basedir" option, but it only allows
adding a prefix to the default "/lib/modules/".
(My original idea to provide the prefix_part, it would have worked
like -b "${INSTALL_MOD_PATH}${MOD_PREFIX}", which you refused)
> Thanks
>
> Michal
>
> > > - use shorter expression for determining the module directory assuming
> > >it's non-empty
> > > ---
> > > Makefile | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 511b5616aa41..84f32bd563d4 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1081,7 +1081,9 @@ export INSTALL_DTBS_PATH ?=
> > > $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
> > > # makefile but the argument can be passed to make if needed.
> > > #
> > >
> > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
> > > +export KERNEL_MODULE_DIRECTORY ?= $(or $(shell pkg-config
> > > --variable=module_directory kmod 2>/dev/null),/lib/modules)
> > > +
> > > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
> > > export MODLIB
> > >
> > > PHONY += prepare0
> > > --
> > > 2.42.0
> > >
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
>
--
Best Regards
Masahiro Yamada
Re: [PATCH v6 2/2] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek wrote:
>
> The default MODLIB value is composed of three variables
>
> MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE)
>
> However, the kernel.spec hadcodes the default value of
> $(KERNEL_MODULE_DIRECTORY), and changed value is not reflected when
> building the package.
>
> Pass KERNEL_MODULE_DIRECTORY to kernel.spec to fix this problem.
>
> Signed-off-by: Michal Suchanek
> ---
> Build on top of the previous patch adding KERNEL_MODULE_DIRECTORY
The SRPM package created by 'make srcrpm-pkg' may not work
if rpmbuild is executed in a different machine.
%{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot}
KERNEL_MODULE_DIRECTORY=%{KERNEL_MODULE_DIRECTORY} modules_install
will align with the specified install destination,
but depmod will still fail.
(same issue as 1/2)
> ---
> scripts/package/kernel.spec | 8
> scripts/package/mkspec | 1 +
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 3eee0143e0c5..12996ed365f8 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -67,7 +67,7 @@ cp $(%{make} %{makeflags} -s image_name)
> %{buildroot}/boot/vmlinuz-%{KERNELRELEA
> %{make} %{makeflags} INSTALL_HDR_PATH=%{buildroot}/usr headers_install
> cp System.map %{buildroot}/boot/System.map-%{KERNELRELEASE}
> cp .config %{buildroot}/boot/config-%{KERNELRELEASE}
> -ln -fns /usr/src/kernels/%{KERNELRELEASE}
> %{buildroot}/lib/modules/%{KERNELRELEASE}/build
> +ln -fns /usr/src/kernels/%{KERNELRELEASE}
> %{buildroot}%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> %if %{with_devel}
> %{make} %{makeflags} run-command
> KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build
> %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> %endif
> @@ -98,8 +98,8 @@ fi
>
> %files
> %defattr (-, root, root)
> -/lib/modules/%{KERNELRELEASE}
> -%exclude /lib/modules/%{KERNELRELEASE}/build
> +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}
> +%exclude %{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> /boot/*
>
> %files headers
> @@ -110,5 +110,5 @@ fi
> %files devel
> %defattr (-, root, root)
> /usr/src/kernels/%{KERNELRELEASE}
> -/lib/modules/%{KERNELRELEASE}/build
> +%{KERNEL_MODULE_DIRECTORY}/%{KERNELRELEASE}/build
> %endif
> diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> index ce201bfa8377..e952fa4f2937 100755
> --- a/scripts/package/mkspec
> +++ b/scripts/package/mkspec
> @@ -24,6 +24,7 @@ fi
> cat< %define ARCH ${ARCH}
> %define KERNELRELEASE ${KERNELRELEASE}
> +%define KERNEL_MODULE_DIRECTORY ${KERNEL_MODULE_DIRECTORY}
> %define pkg_release $("${srctree}/init/build-version")
> EOF
>
> --
> 2.42.0
>
>
--
Best Regards
Masahiro Yamada
Re: [PATCH v6 1/2] depmod: Handle installing modules under a different directory
On Thu, Dec 7, 2023 at 4:48 AM Michal Suchanek wrote: > > Some distributions aim at shipping all files in /usr. > > The path under which kernel modules are installed is hardcoded to /lib > which conflicts with this goal. > > When kmod provides kmod.pc, use it to determine the correct module > installation path. > > With kmod that does not provide the config /lib/modules is used as > before. > > While pkg-config does not return an error when a variable does not exist > the kmod configure script puts some effort into ensuring that > module_directory is non-empty. With that empty module_directory from > pkg-config can be used to detect absence of the variable. > > Signed-off-by: Michal Suchanek > --- > v6: > - use ?= instead of := to make it easier to override the value "KERNEL_MODULE_DIRECTORY=/local/usr/lib/modules make modules_install" will override the install destination, but depmod will not be not aware of it. How to avoid the depmod error? > - use shorter expression for determining the module directory assuming >it's non-empty > --- > Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 511b5616aa41..84f32bd563d4 100644 > --- a/Makefile > +++ b/Makefile > @@ -1081,7 +1081,9 @@ export INSTALL_DTBS_PATH ?= > $(INSTALL_PATH)/dtbs/$(KERNELRELEASE) > # makefile but the argument can be passed to make if needed. > # > > -MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE) > +export KERNEL_MODULE_DIRECTORY ?= $(or $(shell pkg-config > --variable=module_directory kmod 2>/dev/null),/lib/modules) > + > +MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE) > export MODLIB > > PHONY += prepare0 > -- > 2.42.0 > > -- Best Regards Masahiro Yamada
Re: [PATCH] init: move THIS_MODULE from to
On Tue, Dec 5, 2023 at 2:54 PM Luis Chamberlain wrote:
>
> On Sun, Nov 26, 2023 at 04:19:14PM +0900, Masahiro Yamada wrote:
> > Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into
> > export.h") appropriately separated EXPORT_SYMBOL into
> > because modules and EXPORT_SYMBOL are orthogonal; modules are symbol
> > consumers, while EXPORT_SYMBOL are used by symbol providers, which
> > may not be necessarily a module.
> >
> > However, that commit also relocated THIS_MODULE. As explained in the
> > commit description, the intention was to define THIS_MODULE in a
> > lightweight header, but I do not believe was the
> > suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated.
> >
> > Move it to another lightweight header, . The reason for
> > choosing is to make self-contained
> > without relying on incorrectly including
> > .
> >
> > With this adjustment, the role of becomes clearer as
> > it only defines EXPORT_SYMBOL.
> >
> > Signed-off-by: Masahiro Yamada
>
> Reviewed-by: Luis Chamberlain
I will fold your reviewed-by tag.
Thanks.
>
> Do you want this this to go through modules-next or your tree? I'm fine
> it goes either way.
>
> Luis
--
Best Regards
Masahiro Yamada
Re: Building signed debs
On Fri, Dec 8, 2023 at 8:14 PM Tom Cook wrote: > > I'm trying to build a signed .deb kernel package of > https://github.com/torvalds/linux/tree/v6.6. I've copied > certs/default_x509.genkey to certs/x509.genkey. The .config is the > one from Ubuntu 23.10's default kernel with all new options accepted > at their default and CONFIG_SYSTEM_TRUSTED_KEYS="" and > CONFIG_SYSTEM_REVOCATION_KEYS="". > > This builds the kernel and modules, signs the modules, compresses the > modules and then attempts to sign the modules again. That fails, > because the .ko module files are now .ko.zst files and the file it's > trying to sign isn't there. Full failure is pasted below. Modules are signed before the compression. Once you compress modules, you cannot re-sign them again unless you decompress modules, sign them, and compress them again. Now, the kernel can directly load compressed modules, so perhaps allowing users to sign compressed modules might be beneficial. (that is, reverse the order of singing and compression) I am not sure whether it is possible or not, though. I added linux-modules ML. > > Unsetting CONFIG_MODULE_COMPRESS_ZSTD is a workaround (ie disable > module compression). > > Is there a way to build a .deb of a signed kernel with compressed modules? > > Thanks for any help, > Tom > > INSTALL debian/linux-libc-dev/usr/include > SIGN > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/amd/amd-uncore.ko > SIGN > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/intel/intel-cstate.ko > At main.c:298: > - SSL error:8002:system library::No such file or > directory: ../crypto/bio/bss_file.c:67 > - SSL error:1080:BIO routines::no such file: ../crypto/bio/bss_file.c:75 > sign-file: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/amd/amd-uncore.ko > SIGN > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/rapl.ko > At main.c:298: > - SSL error:8002:system library::No such file or > directory: ../crypto/bio/bss_file.c:67 > - SSL error:1080:BIO routines::no such file: ../crypto/bio/bss_file.c:75 > sign-file: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/intel/intel-cstate.ko > SIGN > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/kernel/cpu/mce/mce-inject.ko > make[6]: *** [scripts/Makefile.modinst:137: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/amd/amd-uncore.ko] > Error 1 > make[6]: *** Waiting for unfinished jobs > make[6]: *** [scripts/Makefile.modinst:137: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/intel/intel-cstate.ko] > Error 1 > SIGN > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/kernel/msr.ko > At main.c:298: > - SSL error:8002:system library::No such file or > directory: ../crypto/bio/bss_file.c:67 > - SSL error:1080:BIO routines::no such file: ../crypto/bio/bss_file.c:75 > sign-file: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/rapl.ko > make[6]: *** [scripts/Makefile.modinst:137: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/events/rapl.ko] > Error 1 > At main.c:298: > - SSL error:8002:system library::No such file or > directory: ../crypto/bio/bss_file.c:67 > - SSL error:1080:BIO routines::no such file: ../crypto/bio/bss_file.c:75 > sign-file: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/kernel/cpu/mce/mce-inject.ko > make[6]: *** [scripts/Makefile.modinst:137: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/kernel/cpu/mce/mce-inject.ko] > Error 1 > At main.c:298: > - SSL error:8002:system library::No such file or > directory: ../crypto/bio/bss_file.c:67 > - SSL error:1080:BIO routines::no such file: ../crypto/bio/bss_file.c:75 > sign-file: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/kernel/msr.ko > make[6]: *** [scripts/Makefile.modinst:137: > debian/linux-image/lib/modules/6.6.0-local/kernel/arch/x86/kernel/msr.ko] > Error 1 > make[5]: *** [Makefile:1821: modules_install] Error 2 > make[4]: *** [Makefile:2036: run-command] Error 2 > make[3]: *** [debian/rules:17: binary-arch] Error 2 > dpkg-buildpackage: error: make -f debian/rules binary subprocess > returned exit status 2 > make[2]: *** [scripts/Makefile.package:146: bindeb-pkg] Error 2 > make[1]: *** [/home/tkcook/git/linux/v6.6/Makefile:1538: bindeb-pkg] Error 2 > make: *** [Makefile:234: __sub-make] Error 2 -- Best Regards Masahiro Yamada
Re: [PATCH] init: move THIS_MODULE from to
On Sun, Nov 26, 2023 at 4:19 PM Masahiro Yamada wrote:
>
> Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into
> export.h") appropriately separated EXPORT_SYMBOL into
> because modules and EXPORT_SYMBOL are orthogonal; modules are symbol
> consumers, while EXPORT_SYMBOL are used by symbol providers, which
> may not be necessarily a module.
>
> However, that commit also relocated THIS_MODULE. As explained in the
> commit description, the intention was to define THIS_MODULE in a
> lightweight header, but I do not believe was the
> suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated.
>
> Move it to another lightweight header, . The reason for
> choosing is to make self-contained
> without relying on incorrectly including
> .
>
> With this adjustment, the role of becomes clearer as
> it only defines EXPORT_SYMBOL.
>
> Signed-off-by: Masahiro Yamada
> ---
Applied to kbuild.
I did not get any report from the 0day bot so far,
but I hope it will get a little more compile tests
before getting into linux-next.
>
> include/linux/export.h | 18 --
> include/linux/init.h | 7 +++
> 2 files changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 9911508a9604..0bbd02fd351d 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -6,15 +6,6 @@
> #include
> #include
>
> -/*
> - * Export symbols from the kernel to modules. Forked from module.h
> - * to reduce the amount of pointless cruft we feed to gcc when only
> - * exporting a simple symbol or two.
> - *
> - * Try not to add #includes here. It slows compilation and makes kernel
> - * hackers place grumpy comments in header files.
> - */
> -
> /*
> * This comment block is used by fixdep. Please do not remove.
> *
> @@ -23,15 +14,6 @@
> * side effect of the *.o build rule.
> */
>
> -#ifndef __ASSEMBLY__
> -#ifdef MODULE
> -extern struct module __this_module;
> -#define THIS_MODULE (&__this_module)
> -#else
> -#define THIS_MODULE ((struct module *)0)
> -#endif
> -#endif /* __ASSEMBLY__ */
> -
> #ifdef CONFIG_64BIT
> #define __EXPORT_SYMBOL_REF(sym) \
> .balign 8 ASM_NL \
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 01b52c9c7526..3fa3f6241350 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -179,6 +179,13 @@ extern void (*late_time_init)(void);
>
> extern bool initcall_debug;
>
> +#ifdef MODULE
> +extern struct module __this_module;
> +#define THIS_MODULE (&__this_module)
> +#else
> +#define THIS_MODULE ((struct module *)0)
> +#endif
> +
> #endif
>
> #ifndef MODULE
> --
> 2.40.1
>
--
Best Regards
Masahiro Yamada
[PATCH] init: move THIS_MODULE from to
Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into
export.h") appropriately separated EXPORT_SYMBOL into
because modules and EXPORT_SYMBOL are orthogonal; modules are symbol
consumers, while EXPORT_SYMBOL are used by symbol providers, which
may not be necessarily a module.
However, that commit also relocated THIS_MODULE. As explained in the
commit description, the intention was to define THIS_MODULE in a
lightweight header, but I do not believe was the
suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated.
Move it to another lightweight header, . The reason for
choosing is to make self-contained
without relying on incorrectly including
.
With this adjustment, the role of becomes clearer as
it only defines EXPORT_SYMBOL.
Signed-off-by: Masahiro Yamada
---
include/linux/export.h | 18 --
include/linux/init.h | 7 +++
2 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/include/linux/export.h b/include/linux/export.h
index 9911508a9604..0bbd02fd351d 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -6,15 +6,6 @@
#include
#include
-/*
- * Export symbols from the kernel to modules. Forked from module.h
- * to reduce the amount of pointless cruft we feed to gcc when only
- * exporting a simple symbol or two.
- *
- * Try not to add #includes here. It slows compilation and makes kernel
- * hackers place grumpy comments in header files.
- */
-
/*
* This comment block is used by fixdep. Please do not remove.
*
@@ -23,15 +14,6 @@
* side effect of the *.o build rule.
*/
-#ifndef __ASSEMBLY__
-#ifdef MODULE
-extern struct module __this_module;
-#define THIS_MODULE (&__this_module)
-#else
-#define THIS_MODULE ((struct module *)0)
-#endif
-#endif /* __ASSEMBLY__ */
-
#ifdef CONFIG_64BIT
#define __EXPORT_SYMBOL_REF(sym) \
.balign 8 ASM_NL \
diff --git a/include/linux/init.h b/include/linux/init.h
index 01b52c9c7526..3fa3f6241350 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -179,6 +179,13 @@ extern void (*late_time_init)(void);
extern bool initcall_debug;
+#ifdef MODULE
+extern struct module __this_module;
+#define THIS_MODULE (&__this_module)
+#else
+#define THIS_MODULE ((struct module *)0)
+#endif
+
#endif
#ifndef MODULE
--
2.40.1
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
On Thu, Nov 23, 2023 at 6:05 PM Greg KH wrote: > > On Wed, Nov 22, 2023 at 01:04:09PM -0800, Matthew Maurer wrote: > > > So, even if you enable CONFIG_MODVERSIONS, > > > nothing is checked for Rust. > > > Genksyms computes a CRC from "int foo", and > > > the module subsystem confirms it is a "int" > > > variable. > > > > > > We know this check always succeeds. > > > > > > Why is this useful? > > The reason this is immediately useful is that it allows us to have Rust > > in use with a kernel where C modules are able to benefit from MODVERSIONS > > checking. The check would effectively be a no-op for now, as you have > > correctly > > determined, but we could refine it to make it more restrictive later. > > Since the > > existing C approach errs on the side of "it could work" rather than "it will > > work", I thought being more permissive was the correct initial solution. > > But it's just providing "fake" information to the CRC checker, which > means that the guarantee of a ABI check is not true at all. > > So the ask for the user of "ensure that the ABI checking is correct" is > being circumvented here, and any change in the rust side can not be > detected at all. > > The kernel is a "whole", either an option works for it, or it doesn't, > and you are splitting that guarantee here by saying "modversions will > only work for a portion of the kernel, not the whole thing" which is > going to cause problems for when people expect it to actually work > properly. > > So, I'd strongly recommend fixing this for the rust code if you wish to > allow modversions to be enabled at all. > > > With regards to future directions that likely won't work for loosening it: > > Unfortunately, the .rmeta format itself is not stable, so I wouldn't want to > > teach genksyms to open it up and split out the pieces for specific > > functions. > > Extending genksyms to parse Rust would also not solve the situation - > > layouts are allowed to differ across compiler versions or even (in rare > > cases) seemingly unrelated code changes. > > What do you mean by "layout" here? Yes, the crcs can be different > across compiler versions and seemingly unrelated code changes (genksyms > is VERY fragile) but that's ok, that's not what you are checking here. > You want to know if the rust function signature changes or not from the > last time you built the code, with the same compiler and options, that's > all you are verifying. > > > Future directions that might work for loosening it: > > * Generating crcs from debuginfo + compiler + flags > > * Adding a feature to the rust compiler to dump this information. This > > is likely to > > get pushback because Rust's current stance is that there is no ability to > > load > > object code built against a different library. > > Why not parse the function signature like we do for C? > > > Would setting up Rust symbols so that they have a crc built out of .rmeta be > > sufficient for you to consider this useful? If not, can you help me > > understand > > what level of precision would be required? > > What exactly does .rmeta have to do with the function signature? That's > all you care about here. rmeta is generated per crate. CRC is computed per symbol. They have different granularity. It is weird to refuse a module for incompatibility of a symbol that it is not using at all. > thanks, > > greg k-h -- Best Regards Masahiro Yamada
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
On Sat, Nov 18, 2023 at 11:58 AM Matthew Maurer wrote:
>
> The goal of this patch series is to allow MODVERSIONS and RUST to be
> enabled simultaneously. The primary issue with doing this at the moment
> is that Rust uses some extremely long symbol names - for those
> unfamiliar with Rust, it may be helpful to think of some of the mangled
> C++ names you may have seen in binaries in the past.
>
> Previously, Gary Guo attempted to accomplish this by modifying the
> existing modversion format [1] to support variable-length symbol names.
> This was unfortunately considered to be a potential userspace break
> because kmod tools inspect this kernel module metadata. Masahiro Yamada
> suggested [2] that this could instead be done with a section per-field.
> This gives us the ability to be more flexible with this format in the
> future, as a new field or additional information will be in a new
> section which userspace tools will not yet attempt to read.
>
> In the previous version of this patchset, Luis Chamberlain suggested [3]
> I move validation out of the version checking and into the elf validity
> checker, and also add kernel-docs over there. I found
> elf_validity_cached_copy to be fairly dense and difficult to directly
> describe, so I refactored it into easier to explain pieces. In the
> process, I found a few missing checks and added those as well. See
> [PATCH 2/5] for more details. If this is too much, I'm more than happy
> to drop this patch from the series in favor of just adding the
> kernel-doc to the original code, but figured I'd offer it up in case the
> added clarity and checks were valuable.
>
> [1] https://lore.kernel.org/lkml/2023061155.1349375-1-g...@garyguo.net/
> [2]
> https://lore.kernel.org/lkml/CAK7LNATsuszFR7JB5ZkqVS1W=hWr9=e7btf+mvgj+nxt3az...@mail.gmail.com/
> [3] https://lore.kernel.org/lkml/zvznh%2fpa5hivr...@bombadil.infradead.org/
I want to know why this is useful.
To clarify my question, let me first explain
what the modversion is.
In C, a function callee and callers must agree
with the interface of the function.
This is usually done by having a function prototype
in a header file.
Say, we have a function declaration
int foo(int x, int y);
in include/linux/foo.h
Then, the C file that defines foo() and all C files
that call it must include so that
argument mismatch can be detected.
Same for EXPORT_SYMBOL; the symbol provider and consumers
must agree with the interface of exported symbols.
In the kernel, however, there is no promise for in-kernel ABI
compatibility across different kernel versions.
The kernel only promises the compatibility of the userspace interface.
To load modules, by principle, vmlinux and modules must have
the same version.
To slightly loosen the limitation, CONFIG_MODVERSIONS was
introduced; when it is enabled, you can load a module
as long as all the prototypes of exported symbols match.
To do this, we need to encode information about prototypes.
This is done by a tool called genksyms (scripts/genksyms/genksyms).
Say, we have this code:
int foo(int x, int y)
{
// genksyms does not care about
// the function body.
}
EXPORT_SYMBOL(foo);
Genksyms parses the code and computes a CRC value for 'foo'.
Genksyms is only interested in the function name and its prototype.
It sees
int foo(int, int)
and it transforms it into a CRC.
Any change to the prototype results in a
different CRC, so the module subsystem
can check the interface compatibility
before loading a module.
It is obvious that this is impossible for Rust source
because scripts/genksyms/genksyms is only able to
parse C code.
Then, what is happening here?
See rust/exports.c
#define EXPORT_SYMBOL_RUST_GPL(sym) extern int sym; EXPORT_SYMBOL_GPL(sym)
The global scope symbols in Rust (i.e. 'pub) are automatically
exported, and all of them are visible as 'int' variables
from C world.
Genksyms will see this code:
extern int foo;
EXPORT_SYMBOL_GPL(foo);
Of course, this is not a true prototype.
The real signature on the Rust side might be:
fn foo(x: i32, y: i32) -> i32
So, even if you enable CONFIG_MODVERSIONS,
nothing is checked for Rust.
Genksyms computes a CRC from "int foo", and
the module subsystem confirms it is a "int"
variable.
We know this check always succeeds.
Why is this useful?
> Matthew Maurer (5):
> export_report: Rehabilitate script
> modules: Refactor + kdoc elf_validity_cached_copy
> modpost: Extended modversion support
> rust: Allow MODVERSIONS
> export_report: Use new version info format
>
> arch/powerpc/kernel/module_64.c | 25 +-
> init/Kconfig| 1 -
> kernel/module/internal.h| 18 +-
> kernel/module/main.c| 663 +---
> kernel/module/version.c
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Tue, Oct 17, 2023 at 9:27 PM Michal Suchánek wrote:
>
> On Tue, Oct 17, 2023 at 09:05:29PM +0900, Masahiro Yamada wrote:
> > On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek wrote:
> > >
> > > On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
> > > > > >
> > > > > > Let me add more context to my question.
> > > > > >
> > > > > >
> > > > > > I am interested in the timing when
> > > > > > 'pkg-config --print-variables kmod | grep module_directory'
> > > > > > is executed.
> > > > > >
> > > > > >
> > > > > >
> > > > > > 1. Build a SRPM on machine A
> > > > > >
> > > > > > 2. Copy the SRPM from machine A to machine B
> > > > > >
> > > > > > 3. Run rpmbuild on machine B to build the SRPM into a RPM
> > > > > >
> > > > > > 4. Copy the RPM from machine B to machine C
> > > > > >
> > > > > > 5. Install the RPM to machine C
> > > > >
> > > > > As far as I am aware the typical use case is two step:
> > > > >
> > > > > 1. run make rpm-pkg on machine A
> > > > > 2. install the binary rpm on machine C that might not have build tools
> > > > >or powerful enough CPU
> > > > >
> > > > > While it's theoretically possible to use the srpm to rebuild the
> > > > > binary
> > > > > rpm independently of the kernel git tree I am not aware of people
> > > > > commonly doing this.
> > > >
> > > >
> > > >
> > > > If I correctly understand commit
> > > > 8818039f959b2efc0d6f2cb101f8061332f0c77e,
> > > > those Redhat guys pack a SRPM on a local machine,
> > > > then send it to their build server called 'koji'.
> > > >
> > > > Otherwise, there is no reason
> > > > to have 'make srcrpm-pkg'.
> > > >
> > > >
> > > >
> > > > I believe "A == B" is not always true,
> > > > but we can assume "distro(A) == distro(B)" is always met
> > > > for simplicity.
> > > >
> > > > So, I am OK with configuration at the SRPM time.
> > >
> > > Even if the distro does not match it will likely work to configure SRPM
> > > for non-matching distro and then build it on the target distro but I have
> > > not tested it.
> >
> >
> >
> > Your approach specifies %{MODLIB} as a fixed string
> > when generating kernel.spec, i.e. at the SRPM time.
> >
> >
> > %files
> > %defattr (-, root, root)
> > -/lib/modules/%{KERNELRELEASE}
> > -%exclude /lib/modules/%{KERNELRELEASE}/build
> > +%{MODLIB}
> > +%exclude %{MODLIB}/build
> > /boot/*
> >
> >
> > Then, how to change the path later?
>
> Why would you need to change the path later?
>
> The SRPM has sources, it does not need to build on the system on which
> it is authored if it is intended for another distribution.
>
> Of course, you would need to know for what distribution and where it
> wants its modules so that you can specify the location when creating the
> SRPM.
Simply I wrongly understood your description.
If you manage to correctly configure for the target distro
when building SRPM, that's fine.
>
> > > > > If rebuilding the source rpm on a different machine from where the git
> > > > > tree is located, and possibly on a different distribution is desirable
> > > > > then the detection of the KERNEL_MODULE_DIRECTORY should be added in
> > > > > the
> > > > > rpm spec file as well.
> > > > >
> > > > > > Of course, we are most interested in the module path
> > > > > > of machine C, but it is difficult/impossible to
> > > > > > guess it at the time of building.
> > > > > >
> > > > > > We can assume machine B == machine C.
> > > > > >
> > > > > > We are the second most interested in the module
> > > > > > path on machine B.
> > > > > >
> > > > > > The module path of machine A is not important.
> > > > > >
> > > > > > So, I am asking where you would inject
> > > > > > 'pkg-config --print-variables kmod | grep module_directory'.
&g
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Tue, Oct 17, 2023 at 7:44 PM Michal Suchánek wrote:
>
> On Tue, Oct 17, 2023 at 07:15:50PM +0900, Masahiro Yamada wrote:
> > > >
> > > > Let me add more context to my question.
> > > >
> > > >
> > > > I am interested in the timing when
> > > > 'pkg-config --print-variables kmod | grep module_directory'
> > > > is executed.
> > > >
> > > >
> > > >
> > > > 1. Build a SRPM on machine A
> > > >
> > > > 2. Copy the SRPM from machine A to machine B
> > > >
> > > > 3. Run rpmbuild on machine B to build the SRPM into a RPM
> > > >
> > > > 4. Copy the RPM from machine B to machine C
> > > >
> > > > 5. Install the RPM to machine C
> > >
> > > As far as I am aware the typical use case is two step:
> > >
> > > 1. run make rpm-pkg on machine A
> > > 2. install the binary rpm on machine C that might not have build tools
> > >or powerful enough CPU
> > >
> > > While it's theoretically possible to use the srpm to rebuild the binary
> > > rpm independently of the kernel git tree I am not aware of people
> > > commonly doing this.
> >
> >
> >
> > If I correctly understand commit
> > 8818039f959b2efc0d6f2cb101f8061332f0c77e,
> > those Redhat guys pack a SRPM on a local machine,
> > then send it to their build server called 'koji'.
> >
> > Otherwise, there is no reason
> > to have 'make srcrpm-pkg'.
> >
> >
> >
> > I believe "A == B" is not always true,
> > but we can assume "distro(A) == distro(B)" is always met
> > for simplicity.
> >
> > So, I am OK with configuration at the SRPM time.
>
> Even if the distro does not match it will likely work to configure SRPM
> for non-matching distro and then build it on the target distro but I have
> not tested it.
Your approach specifies %{MODLIB} as a fixed string
when generating kernel.spec, i.e. at the SRPM time.
%files
%defattr (-, root, root)
-/lib/modules/%{KERNELRELEASE}
-%exclude /lib/modules/%{KERNELRELEASE}/build
+%{MODLIB}
+%exclude %{MODLIB}/build
/boot/*
Then, how to change the path later?
I do not know if the relocatable package
is a sensible solution because the kernel package has /boot/
http://ftp.rpm.org/api/4.4.2.2/relocatable.html
We might be able to tweak installation paths in %post section.
Or perhaps, %{shell } can defer the module path detection
until building RPM.
%define MOD_PREFIX%{shell pkg-config --variable=module_prefix
libkmod 2>/dev/null}
Overall, I did not find a cool solution.
>
> > > If rebuilding the source rpm on a different machine from where the git
> > > tree is located, and possibly on a different distribution is desirable
> > > then the detection of the KERNEL_MODULE_DIRECTORY should be added in the
> > > rpm spec file as well.
> > >
> > > > Of course, we are most interested in the module path
> > > > of machine C, but it is difficult/impossible to
> > > > guess it at the time of building.
> > > >
> > > > We can assume machine B == machine C.
> > > >
> > > > We are the second most interested in the module
> > > > path on machine B.
> > > >
> > > > The module path of machine A is not important.
> > > >
> > > > So, I am asking where you would inject
> > > > 'pkg-config --print-variables kmod | grep module_directory'.
> > >
> > > I don't. I don't think there will be a separate machine B.
> > >
> > > And I can't really either - so far any attempt at adding support for
> > > this has been rejected.
> > >
> > > Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one
> > > giving the script to run, and one running it, and then it could be run
> > > independently in the SRPM as well.
> >
> >
> > At first, I thought your patch [1] was very ugly,
> > but I do not think it is so ugly if cleanly implemented.
> >
> > It won't hurt to allow users to specify the middle part of MODLIB.
> >
> >
> > There are two options.
> >
> >
> > [A] Add 'MOD_PREFIX' to specify the middle part of MODLIB
> >
> >
> > The top Makefile will look as follows:
> >
> >
> > MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE)
> > export MODLIB
> >
> >
> > It is easier than specifying the entire MODLIB, but you still need
> > t
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
> > > > Let me add more context to my question. > > > > > > I am interested in the timing when > > 'pkg-config --print-variables kmod | grep module_directory' > > is executed. > > > > > > > > 1. Build a SRPM on machine A > > > > 2. Copy the SRPM from machine A to machine B > > > > 3. Run rpmbuild on machine B to build the SRPM into a RPM > > > > 4. Copy the RPM from machine B to machine C > > > > 5. Install the RPM to machine C > > As far as I am aware the typical use case is two step: > > 1. run make rpm-pkg on machine A > 2. install the binary rpm on machine C that might not have build tools >or powerful enough CPU > > While it's theoretically possible to use the srpm to rebuild the binary > rpm independently of the kernel git tree I am not aware of people > commonly doing this. If I correctly understand commit 8818039f959b2efc0d6f2cb101f8061332f0c77e, those Redhat guys pack a SRPM on a local machine, then send it to their build server called 'koji'. Otherwise, there is no reason to have 'make srcrpm-pkg'. I believe "A == B" is not always true, but we can assume "distro(A) == distro(B)" is always met for simplicity. So, I am OK with configuration at the SRPM time. > If rebuilding the source rpm on a different machine from where the git > tree is located, and possibly on a different distribution is desirable > then the detection of the KERNEL_MODULE_DIRECTORY should be added in the > rpm spec file as well. > > > Of course, we are most interested in the module path > > of machine C, but it is difficult/impossible to > > guess it at the time of building. > > > > We can assume machine B == machine C. > > > > We are the second most interested in the module > > path on machine B. > > > > The module path of machine A is not important. > > > > So, I am asking where you would inject > > 'pkg-config --print-variables kmod | grep module_directory'. > > I don't. I don't think there will be a separate machine B. > > And I can't really either - so far any attempt at adding support for > this has been rejected. > > Technically the KERNEL_MODULE_DIRECTORY could be set in two steps - one > giving the script to run, and one running it, and then it could be run > independently in the SRPM as well. At first, I thought your patch [1] was very ugly, but I do not think it is so ugly if cleanly implemented. It won't hurt to allow users to specify the middle part of MODLIB. There are two options. [A] Add 'MOD_PREFIX' to specify the middle part of MODLIB The top Makefile will look as follows: MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE) export MODLIB It is easier than specifying the entire MODLIB, but you still need to manually pass "MOD_PREFIX=/usr" from an env variable or the command line. If MOD_PREFIX is not given, MODLIB is the same as the current one. [B] Support a dynamic configuration as well MOD_PREFIX ?= $(shell pkg-config --variable=module_prefix libkmod 2>/dev/null) export MOD_PREFIX MODLIB = $(INSTALL_MOD_PATH)$(MOD_PREFIX)/lib/modules/$(KERNELRELEASE) export MODLIB If MOD_PREFIX is given from an env variable or from the command line, it is respected. If "pkg-config --variable=module_prefix libkmod" works, that configuration is applied. Otherwise, MOD_PREFIX is empty, i.e. fall back to the current behavior. I prefer 'MOD_PREFIX' to 'KERNEL_MODULE_DIRECTORY' in your patch [1] because "|| echo /lib/modules" can be omitted. I do not think we will have such a crazy distro that installs modules under /opt/ directory. I could not understand why you inserted "--print-variables kmod 2>/dev/null | grep '^module_directory$$' >/dev/null" but I guess the reason is the same. "pkg-config --variable=module_directory kmod" always succeeds, so "|| echo /lib/modules" is never processed. I do not know why you parsed kmod.pc instead of libkmod.pc [2] [1] https://lore.kernel.org/linux-kbuild/20230718120348.383-1-msucha...@suse.de/ [2] https://github.com/kmod-project/kmod/blob/v31/configure.ac#L295 -- Best Regards Masahiro Yamada
[PATCH v2 1/2] kbuild: unify vdso_install rules
Currently, there is no standard implementation for vdso_install,
leading to various issues:
1. Code duplication
Many architectures duplicate similar code just for copying files
to the install destination.
Some architectures (arm, sparc, x86) create build-id symlinks,
introducing more code duplication.
2. Unintended updates of in-tree build artifacts
The vdso_install rule depends on the vdso files to install.
It may update in-tree build artifacts. This can be problematic,
as explained in commit 19514fc665ff ("arm, kbuild: make
"make install" not depend on vmlinux").
3. Broken code in some architectures
Makefile code is often copied from one architecture to another
without proper adaptation.
'make vdso_install' for parisc does not work.
'make vdso_install' for s390 installs vdso64, but not vdso32.
To address these problems, this commit introduces a generic vdso_install
rule.
Architectures that support vdso_install need to define vdso-install-y
in arch/*/Makefile.
vdso-install-y lists the files to install. For example, arch/x86/Makefile
looks like this:
vdso-install-$(CONFIG_X86_64) += arch/x86/entry/vdso/vdso64.so.dbg
vdso-install-$(CONFIG_X86_X32_ABI) += arch/x86/entry/vdso/vdsox32.so.dbg
vdso-install-$(CONFIG_X86_32) += arch/x86/entry/vdso/vdso32.so.dbg
vdso-install-$(CONFIG_IA32_EMULATION) += arch/x86/entry/vdso/vdso32.so.dbg
These files will be installed to $(MODLIB)/vdso/ with the .dbg suffix,
if exists, stripped away.
vdso-install-y can optionally take the second field after the colon
separator. This is needed because some architectures install a vdso
file as a different base name.
The following is a snippet from arch/arm64/Makefile.
vdso-install-$(CONFIG_COMPAT_VDSO) +=
arch/arm64/kernel/vdso32/vdso.so.dbg:vdso32.so
This will rename vdso.so.dbg to vdso32.so during installation. If such
architectures change their implementation so that the base names match,
this workaround will go away.
Signed-off-by: Masahiro Yamada
Acked-by: Sven Schnelle # s390
Reviewed-by: Nicolas Schier
Reviewed-by: Guo Ren
Acked-by: Helge Deller # parisc
---
Changes in v2:
- Add vdso-install-y for parisc
Makefile | 9 ++
arch/arm/Makefile | 7 +---
arch/arm/vdso/Makefile | 25 --
arch/arm64/Makefile| 9 ++
arch/arm64/kernel/vdso/Makefile| 10 --
arch/arm64/kernel/vdso32/Makefile | 10 --
arch/loongarch/Makefile| 4 +--
arch/loongarch/vdso/Makefile | 10 --
arch/parisc/Makefile | 8 ++---
arch/riscv/Makefile| 9 ++
arch/riscv/kernel/compat_vdso/Makefile | 10 --
arch/riscv/kernel/vdso/Makefile| 10 --
arch/s390/Makefile | 6 ++--
arch/s390/kernel/vdso32/Makefile | 10 --
arch/s390/kernel/vdso64/Makefile | 10 --
arch/sparc/Makefile| 5 ++-
arch/sparc/vdso/Makefile | 27
arch/x86/Makefile | 7 ++--
arch/x86/entry/vdso/Makefile | 27
scripts/Makefile.vdsoinst | 45 ++
20 files changed, 73 insertions(+), 185 deletions(-)
create mode 100644 scripts/Makefile.vdsoinst
diff --git a/Makefile b/Makefile
index 3de6dd959bd1..1ae5d5180f20 100644
--- a/Makefile
+++ b/Makefile
@@ -1317,6 +1317,14 @@ scripts_unifdef: scripts_basic
quiet_cmd_install = INSTALL $(INSTALL_PATH)
cmd_install = unset sub_make_done; $(srctree)/scripts/install.sh
+# ---
+# vDSO install
+
+PHONY += vdso_install
+vdso_install: export INSTALL_FILES = $(vdso-install-y)
+vdso_install:
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vdsoinst
+
# ---
# Tools
@@ -1560,6 +1568,7 @@ help:
@echo '* vmlinux - Build the bare kernel'
@echo '* modules - Build all modules'
@echo ' modules_install - Install all modules to INSTALL_MOD_PATH
(default: /)'
+ @echo ' vdso_install- Install unstripped vdso to INSTALL_MOD_PATH
(default: /)'
@echo ' dir/- Build all files in dir and below'
@echo ' dir/file.[ois] - Build specified target only'
@echo ' dir/file.ll - Build the LLVM assembly file'
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 547e5856eaa0..5ba42f69f8ce 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -304,11 +304,7 @@ $(INSTALL_TARGETS): KBUILD_IMAGE = $(boot)/$(patsubst
%install,%Image,$@)
$(INSTALL_TARGETS):
$(call cmd,install)
-PHONY += vdso_install
-vdso_install:
-ifeq ($(CONFIG_VDSO),y)
- $(Q)$(MAKE) $(build)=arch/arm/vdso $@
-endif
+vdso-i
[PATCH v2 2/2] kbuild: unify no-compiler-targets and no-sync-config-targets
Now that vdso_install does not depend on any in-tree build artifact, it no longer needs a compiler, making no-compiler-targets the same as no-sync-config-targets. Signed-off-by: Masahiro Yamada --- Changes in v2: - Revive need-compiler flag Makefile | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 1ae5d5180f20..fed9a6cc3665 100644 --- a/Makefile +++ b/Makefile @@ -277,10 +277,6 @@ no-dot-config-targets := $(clean-targets) \ $(version_h) headers headers_% archheaders archscripts \ %asm-generic kernelversion %src-pkg dt_binding_check \ outputmakefile rustavailable rustfmt rustfmtcheck -# Installation targets should not require compiler. Unfortunately, vdso_install -# is an exception where build artifacts may be updated. This must be fixed. -no-compiler-targets := $(no-dot-config-targets) install dtbs_install \ - headers_install modules_install modules_sign kernelrelease image_name no-sync-config-targets := $(no-dot-config-targets) %install modules_sign kernelrelease \ image_name single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %.symtypes %/ @@ -288,7 +284,6 @@ single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %.symtypes % config-build := mixed-build:= need-config:= 1 -need-compiler := 1 may-sync-config:= 1 single-build := @@ -298,18 +293,14 @@ ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),) endif endif -ifneq ($(filter $(no-compiler-targets), $(MAKECMDGOALS)),) - ifeq ($(filter-out $(no-compiler-targets), $(MAKECMDGOALS)),) - need-compiler := - endif -endif - ifneq ($(filter $(no-sync-config-targets), $(MAKECMDGOALS)),) ifeq ($(filter-out $(no-sync-config-targets), $(MAKECMDGOALS)),) may-sync-config := endif endif +need-compiler := $(may-sync-config) + ifneq ($(KBUILD_EXTMOD),) may-sync-config := endif -- 2.39.2
Re: [PATCH 1/5] csky: remove unused cmd_vdso_install
On Tue, Oct 10, 2023 at 12:16 AM Guo Ren wrote: > > On Mon, Oct 9, 2023 at 8:42 PM Masahiro Yamada wrote: > > > > You cannot run this code because arch/csky/Makefile does not define the > > vdso_install target. > > > > It appears that this code was blindly copied from another architecture. > Yes, I do that. Thx for pointing it out. > > Acked-by: Guo Ren Applied to linux-kbuild. -- Best Regards Masahiro Yamada
Re: [PATCH 5/5] kbuild: unify no-compiler-targets and no-sync-config-targets
On Tue, Oct 10, 2023 at 1:44 AM Nathan Chancellor wrote:
>
> On Mon, Oct 09, 2023 at 09:42:10PM +0900, Masahiro Yamada wrote:
> > Now that vdso_install does not depend on any in-tree build artifact,
> > it no longer invokes a compiler, making no-compiler-targets the same
> > as no-sync-config-targets.
> >
> > Signed-off-by: Masahiro Yamada
> > ---
> >
> > Makefile | 13 +
> > 1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 2170d56630e8..982b1ad33287 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -277,10 +277,6 @@ no-dot-config-targets := $(clean-targets) \
> >$(version_h) headers headers_% archheaders
> > archscripts \
> >%asm-generic kernelversion %src-pkg dt_binding_check
> > \
> >outputmakefile rustavailable rustfmt rustfmtcheck
> > -# Installation targets should not require compiler. Unfortunately,
> > vdso_install
> > -# is an exception where build artifacts may be updated. This must be fixed.
> > -no-compiler-targets := $(no-dot-config-targets) install dtbs_install \
> > - headers_install modules_install modules_sign
> > kernelrelease image_name
> > no-sync-config-targets := $(no-dot-config-targets) %install modules_sign
> > kernelrelease \
> > image_name
> > single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s
> > %.symtypes %/
> > @@ -288,7 +284,6 @@ single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod
> > %.o %.rsi %.s %.symtypes %
> > config-build :=
> > mixed-build :=
> > need-config := 1
> > -need-compiler:= 1
> > may-sync-config := 1
> > single-build :=
> >
> > @@ -298,12 +293,6 @@ ifneq ($(filter $(no-dot-config-targets),
> > $(MAKECMDGOALS)),)
> > endif
> > endif
> >
> > -ifneq ($(filter $(no-compiler-targets), $(MAKECMDGOALS)),)
> > - ifeq ($(filter-out $(no-compiler-targets), $(MAKECMDGOALS)),)
> > - need-compiler :=
> > - endif
> > -endif
> > -
>
> MIPS and LoongArch seem to have grown a usage of need-compiler in
> 4fe4a6374c4d ("MIPS: Only fiddle with CHECKFLAGS if `need-compiler'")
> and 54c2c9df083f ("LoongArch: Only fiddle with CHECKFLAGS if
> `need-compiler'"). With this removal, should those be updated as well?
Right, but may-sync-config and need-compiler are not
interchangeable due to the following code.
ifneq ($(KBUILD_EXTMOD),)
may-sync-config :=
endif
I will keep both.
--
Best Regards
Masahiro Yamada
Re: [PATCH 4/5] kbuild: unify vdso_install rules
On Wed, Oct 11, 2023 at 11:24 AM Guo Ren wrote: > > On Mon, Oct 9, 2023 at 8:42 PM Masahiro Yamada wrote: > > --- a/arch/riscv/Makefile > > +++ b/arch/riscv/Makefile > > @@ -131,12 +131,6 @@ endif > > libs-y += arch/riscv/lib/ > > libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > -PHONY += vdso_install > > -vdso_install: > > - $(Q)$(MAKE) $(build)=arch/riscv/kernel/vdso $@ > > - $(if $(CONFIG_COMPAT),$(Q)$(MAKE) \ > > - $(build)=arch/riscv/kernel/compat_vdso compat_$@) > > - > > ifeq ($(KBUILD_EXTMOD),) > > ifeq ($(CONFIG_MMU),y) > > prepare: vdso_prepare > > @@ -148,6 +142,9 @@ vdso_prepare: prepare0 > > endif > > endif > > > > +vdso-install-y += arch/riscv/kernel/vdso/vdso.so.dbg > > +vdso-install-$(CONFIG_COMPAT) += > > arch/riscv/kernel/compat_vdso/compat_vdso.so.dbg:../compat_vdso/compat_vdso.so > Why do we need ":../compat_vdso/compat_vdso.so" here? All architectures except riscv install vdso files to /lib/modules/$(uname -r)/vdso/. See the following code in arch/riscv/kernel/compat_vdso/Makefile: quiet_cmd_compat_vdso_install = INSTALL $@ cmd_compat_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/compat_vdso/$@ Riscv copies the compat vdso to /lib/modules/$(uname -r)/compat_vdso/. This commit preserves the current installation path as-is. If the riscv maintainers agree, we can change the installation destination to /lib/modules/$(uname -r)/vdso/ for consistency. -- Best Regards Masahiro Yamada
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Mon, Oct 9, 2023 at 11:07 PM Michal Suchánek wrote: > > On Mon, Oct 09, 2023 at 09:34:10PM +0900, Masahiro Yamada wrote: > > On Mon, Oct 9, 2023 at 5:52 PM Michal Suchánek wrote: > > > > > > Hello, > > > > > > On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote: > > > > On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek > > > > wrote: > > > > > > > > > > The default MODLIB value is composed of two variables and the > > > > > hardcoded > > > > > string '/lib/modules/'. > > > > > > > > > > MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE) > > > > > > > > > > Defining this middle part as a variable was rejected on the basis that > > > > > users can pass the whole MODLIB to make, such as > > > > > > > > > > > > In other words, do you want to say > > > > > > > > "If defining this middle part as a variable had been accepted, > > > > this patch would have been unneeded." ? > > > > > > If it were accepted I would not have to guess what the middle part is, > > > and could use the variable that unambiguosly defines it instead. > > > > > > How? > > > > scripts/package/kernel.spec hardcodes 'lib/modules' > > in a couple of places. > > > > I am asking how to derive the module path. > > Not sure what you are asking here. The path is hardcoded, everywhere. > > The current Makefile has > > MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE) > > and there is no reliable way to learn what the middle part was after the > fact - $(INSTALL_MOD_PATH) can be non-empty. > > The rejected patch was changing this to a variable, and also default to > adjusting the content to what kmod exports in pkgconfig after applying a > proposed patch to make this hardcoded part configurable: > > export KERNEL_MODULE_DIRECTORY := $(shell pkg-config --print-variables kmod > 2>/dev/null | grep '^module_directory$$' >/dev/null && pkg-config > --variable=module_directory kmod || echo /lib/modules) > > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE) > > It would be completely posible to only define the middle part as a > variable that could then be used in rpm-pkg: > > export KERNEL_MODULE_DIRECTORY := /lib/modules > > MODLIB = $(INSTALL_MOD_PATH)$(KERNEL_MODULE_DIRECTORY)/$(KERNELRELEASE) > > Thanks > > Michal > > Let me add more context to my question. I am interested in the timing when 'pkg-config --print-variables kmod | grep module_directory' is executed. 1. Build a SRPM on machine A 2. Copy the SRPM from machine A to machine B 3. Run rpmbuild on machine B to build the SRPM into a RPM 4. Copy the RPM from machine B to machine C 5. Install the RPM to machine C Of course, we are most interested in the module path of machine C, but it is difficult/impossible to guess it at the time of building. We can assume machine B == machine C. We are the second most interested in the module path on machine B. The module path of machine A is not important. So, I am asking where you would inject 'pkg-config --print-variables kmod | grep module_directory'. -- Best Regards Masahiro Yamada
[PATCH 5/5] kbuild: unify no-compiler-targets and no-sync-config-targets
Now that vdso_install does not depend on any in-tree build artifact, it no longer invokes a compiler, making no-compiler-targets the same as no-sync-config-targets. Signed-off-by: Masahiro Yamada --- Makefile | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 2170d56630e8..982b1ad33287 100644 --- a/Makefile +++ b/Makefile @@ -277,10 +277,6 @@ no-dot-config-targets := $(clean-targets) \ $(version_h) headers headers_% archheaders archscripts \ %asm-generic kernelversion %src-pkg dt_binding_check \ outputmakefile rustavailable rustfmt rustfmtcheck -# Installation targets should not require compiler. Unfortunately, vdso_install -# is an exception where build artifacts may be updated. This must be fixed. -no-compiler-targets := $(no-dot-config-targets) install dtbs_install \ - headers_install modules_install modules_sign kernelrelease image_name no-sync-config-targets := $(no-dot-config-targets) %install modules_sign kernelrelease \ image_name single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %.symtypes %/ @@ -288,7 +284,6 @@ single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %.symtypes % config-build := mixed-build:= need-config:= 1 -need-compiler := 1 may-sync-config:= 1 single-build := @@ -298,12 +293,6 @@ ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),) endif endif -ifneq ($(filter $(no-compiler-targets), $(MAKECMDGOALS)),) - ifeq ($(filter-out $(no-compiler-targets), $(MAKECMDGOALS)),) - need-compiler := - endif -endif - ifneq ($(filter $(no-sync-config-targets), $(MAKECMDGOALS)),) ifeq ($(filter-out $(no-sync-config-targets), $(MAKECMDGOALS)),) may-sync-config := @@ -675,7 +664,7 @@ endif # Include this also for config targets because some architectures need # cc-cross-prefix to determine CROSS_COMPILE. -ifdef need-compiler +ifdef may-sync-config include $(srctree)/scripts/Makefile.compiler endif -- 2.39.2
[PATCH 4/5] kbuild: unify vdso_install rules
Currently, there is no standard implementation for vdso_install,
leading to various issues:
1. Code duplication
Many architectures duplicate similar code just for copying files
to the install destination.
Some architectures (arm, sparc, x86) create build-id symlinks,
introducing more code duplication.
2. Accidental updates of in-tree build artifacts
The vdso_install rule depends on the vdso files to install.
It may update in-tree build artifacts. This can be problematic,
as explained in commit 19514fc665ff ("arm, kbuild: make
"make install" not depend on vmlinux").
3. Broken code in some architectures
Makefile code is often copied from one architecture to another
without proper adaptation or testing.
The previous commits removed broken code from csky, UML, and parisc.
Another issue is that 'make vdso_install' for ARCH=s390 installs
vdso64, but not vdso32.
To address these problems, this commit introduces the generic vdso_install.
Architectures that support vdso_install need to define vdso-install-y
in arch/*/Makefile.
vdso-install-y lists the files to install. For example, arch/x86/Makefile
looks like this:
vdso-install-$(CONFIG_X86_64) += arch/x86/entry/vdso/vdso64.so.dbg
vdso-install-$(CONFIG_X86_X32_ABI) += arch/x86/entry/vdso/vdsox32.so.dbg
vdso-install-$(CONFIG_X86_32) += arch/x86/entry/vdso/vdso32.so.dbg
vdso-install-$(CONFIG_IA32_EMULATION) += arch/x86/entry/vdso/vdso32.so.dbg
These files will be installed to $(MODLIB)/vdso/ with the .dbg suffix,
if exists, stripped away.
vdso-install-y can optionally take the second field after the colon
separator. This is needed because some architectures install vdso
files as a different base name.
The following is a snippet from arch/arm64/Makefile.
vdso-install-$(CONFIG_COMPAT_VDSO) +=
arch/arm64/kernel/vdso32/vdso.so.dbg:vdso32.so
This will rename vdso.so.dbg to vdso32.so during installation. If such
architectures change their implementation so that the file names match,
this workaround will go away.
Signed-off-by: Masahiro Yamada
---
Makefile | 9 ++
arch/arm/Makefile | 7 +---
arch/arm/vdso/Makefile | 25 --
arch/arm64/Makefile| 9 ++
arch/arm64/kernel/vdso/Makefile| 10 --
arch/arm64/kernel/vdso32/Makefile | 10 --
arch/loongarch/Makefile| 4 +--
arch/loongarch/vdso/Makefile | 10 --
arch/riscv/Makefile| 9 ++
arch/riscv/kernel/compat_vdso/Makefile | 10 --
arch/riscv/kernel/vdso/Makefile| 10 --
arch/s390/Makefile | 6 ++--
arch/s390/kernel/vdso32/Makefile | 10 --
arch/s390/kernel/vdso64/Makefile | 10 --
arch/sparc/Makefile| 5 ++-
arch/sparc/vdso/Makefile | 27
arch/x86/Makefile | 7 ++--
arch/x86/entry/vdso/Makefile | 27
scripts/Makefile.vdsoinst | 45 ++
19 files changed, 71 insertions(+), 179 deletions(-)
create mode 100644 scripts/Makefile.vdsoinst
diff --git a/Makefile b/Makefile
index 373649c7374e..2170d56630e8 100644
--- a/Makefile
+++ b/Makefile
@@ -1317,6 +1317,14 @@ scripts_unifdef: scripts_basic
quiet_cmd_install = INSTALL $(INSTALL_PATH)
cmd_install = unset sub_make_done; $(srctree)/scripts/install.sh
+# ---
+# vDSO install
+
+PHONY += vdso_install
+vdso_install: export INSTALL_FILES = $(vdso-install-y)
+vdso_install:
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vdsoinst
+
# ---
# Tools
@@ -1560,6 +1568,7 @@ help:
@echo '* vmlinux - Build the bare kernel'
@echo '* modules - Build all modules'
@echo ' modules_install - Install all modules to INSTALL_MOD_PATH
(default: /)'
+ @echo ' vdso_install- Install unstripped vdso to INSTALL_MOD_PATH
(default: /)'
@echo ' dir/- Build all files in dir and below'
@echo ' dir/file.[ois] - Build specified target only'
@echo ' dir/file.ll - Build the LLVM assembly file'
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 547e5856eaa0..5ba42f69f8ce 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -304,11 +304,7 @@ $(INSTALL_TARGETS): KBUILD_IMAGE = $(boot)/$(patsubst
%install,%Image,$@)
$(INSTALL_TARGETS):
$(call cmd,install)
-PHONY += vdso_install
-vdso_install:
-ifeq ($(CONFIG_VDSO),y)
- $(Q)$(MAKE) $(build)=arch/arm/vdso $@
-endif
+vdso-install-$(CONFIG_VDSO) += arch/arm/vdso/vdso.so.dbg
# My testing targets (bypasses dependencies)
bp:; $(Q)$(MAKE) $(build)=$(boot) $(boot)/bootpImage
@@ -33
[PATCH 3/5] parisc: remove broken vdso_install
'make ARCH=parisc vdso_install' has never worked. It attempts to descend into arch/parisc/kernel/vdso/, which does not exist. The command just fails: scripts/Makefile.build:41: arch/parisc/kernel/vdso/Makefile: No such file or directory The second line is also meaningless because parisc does not define CONFIG_COMPAT_VDSO. It appears that this code was copied from another architecture without proper adaptation. Remove the broken code. Signed-off-by: Masahiro Yamada --- arch/parisc/Makefile | 7 --- 1 file changed, 7 deletions(-) diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile index 968ebe17494c..4222fa73c34a 100644 --- a/arch/parisc/Makefile +++ b/arch/parisc/Makefile @@ -177,13 +177,6 @@ vdso_prepare: prepare0 $(Q)$(MAKE) $(build)=arch/parisc/kernel/vdso32 include/generated/vdso32-offsets.h endif -PHONY += vdso_install - -vdso_install: - $(Q)$(MAKE) $(build)=arch/parisc/kernel/vdso $@ - $(if $(CONFIG_COMPAT_VDSO), \ - $(Q)$(MAKE) $(build)=arch/parisc/kernel/vdso32 $@) - install: KBUILD_IMAGE := vmlinux zinstall: KBUILD_IMAGE := vmlinuz install zinstall: -- 2.39.2
[PATCH 2/5] UML: remove unused cmd_vdso_install
You cannot run this code because arch/um/Makefile does not define the vdso_install target. It appears that this code was blindly copied from another architecture. Remove the dead code. Signed-off-by: Masahiro Yamada --- arch/x86/um/vdso/Makefile | 12 1 file changed, 12 deletions(-) diff --git a/arch/x86/um/vdso/Makefile b/arch/x86/um/vdso/Makefile index 6825e146a62f..b86d634730b2 100644 --- a/arch/x86/um/vdso/Makefile +++ b/arch/x86/um/vdso/Makefile @@ -67,15 +67,3 @@ quiet_cmd_vdso = VDSO$@ VDSO_LDFLAGS = -fPIC -shared -Wl,--hash-style=sysv -z noexecstack GCOV_PROFILE := n - -# -# Install the unstripped copy of vdso*.so listed in $(vdso-install-y). -# -quiet_cmd_vdso_install = INSTALL $@ - cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@ -$(vdso-install-y): %.so: $(obj)/%.so.dbg FORCE - @mkdir -p $(MODLIB)/vdso - $(call cmd,vdso_install) - -PHONY += vdso_install $(vdso-install-y) -vdso_install: $(vdso-install-y) -- 2.39.2
[PATCH 1/5] csky: remove unused cmd_vdso_install
You cannot run this code because arch/csky/Makefile does not define the vdso_install target. It appears that this code was blindly copied from another architecture. Remove the dead code. Signed-off-by: Masahiro Yamada --- arch/csky/kernel/vdso/Makefile | 10 -- 1 file changed, 10 deletions(-) diff --git a/arch/csky/kernel/vdso/Makefile b/arch/csky/kernel/vdso/Makefile index 299e4e41ebc5..ddf784a62c11 100644 --- a/arch/csky/kernel/vdso/Makefile +++ b/arch/csky/kernel/vdso/Makefile @@ -58,13 +58,3 @@ quiet_cmd_vdsold = VDSOLD $@ # that contains the same symbols at the same offsets. quiet_cmd_so2s = SO2S$@ cmd_so2s = $(NM) -D $< | $(srctree)/$(src)/so2s.sh > $@ - -# install commands for the unstripped file -quiet_cmd_vdso_install = INSTALL $@ - cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@ - -vdso.so: $(obj)/vdso.so.dbg - @mkdir -p $(MODLIB)/vdso - $(call cmd,vdso_install) - -vdso_install: vdso.so -- 2.39.2
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Mon, Oct 9, 2023 at 5:52 PM Michal Suchánek wrote: > > Hello, > > On Mon, Oct 09, 2023 at 05:31:02PM +0900, Masahiro Yamada wrote: > > On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek wrote: > > > > > > The default MODLIB value is composed of two variables and the hardcoded > > > string '/lib/modules/'. > > > > > > MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE) > > > > > > Defining this middle part as a variable was rejected on the basis that > > > users can pass the whole MODLIB to make, such as > > > > > > In other words, do you want to say > > > > "If defining this middle part as a variable had been accepted, > > this patch would have been unneeded." ? > > If it were accepted I would not have to guess what the middle part is, > and could use the variable that unambiguosly defines it instead. How? scripts/package/kernel.spec hardcodes 'lib/modules' in a couple of places. I am asking how to derive the module path. -- Best Regards Masahiro Yamada
Re: [PATCH rebased] kbuild: rpm-pkg: Fix build with non-default MODLIB
On Fri, Oct 6, 2023 at 12:49 AM Michal Suchanek wrote:
>
> The default MODLIB value is composed of two variables and the hardcoded
> string '/lib/modules/'.
>
> MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
>
> Defining this middle part as a variable was rejected on the basis that
> users can pass the whole MODLIB to make, such as
In other words, do you want to say
"If defining this middle part as a variable had been accepted,
this patch would have been unneeded." ?
I do not think so.
If your original patch were accepted, how would this patch look like?
kernel.spec needs to know the module directory somehow.
Would you add the following in scripts/package/mkspec ?
%define MODLIB $(pkg-config --print-variables kmod 2>/dev/null | grep
'^module_directory$' >/dev/null && pkg-config
--variable=module_directory kmod || echo /lib/modules)
>
> make 'MODLIB=$(INSTALL_MOD_PATH)/usr/lib/modules/$(KERNELRELEASE)'
>
> However, this middle part of MODLIB is independently hardcoded by
> rpm-pkg, and when the user alters MODLIB this is not reflected when
> building the package.
>
> Given that $(INSTALL_MOD_PATH) is overridden during the rpm package build
> it is likely going to be empty. Then MODLIB can be passed to the rpm
> package, and used in place of the whole
> /usr/lib/modules/$(KERNELRELEASE) part.
>
> Signed-off-by: Michal Suchanek
> ---
> scripts/package/kernel.spec | 8
> scripts/package/mkspec | 1 +
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 3eee0143e0c5..15f49c5077db 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -67,7 +67,7 @@ cp $(%{make} %{makeflags} -s image_name)
> %{buildroot}/boot/vmlinuz-%{KERNELRELEA
> %{make} %{makeflags} INSTALL_HDR_PATH=%{buildroot}/usr headers_install
> cp System.map %{buildroot}/boot/System.map-%{KERNELRELEASE}
> cp .config %{buildroot}/boot/config-%{KERNELRELEASE}
> -ln -fns /usr/src/kernels/%{KERNELRELEASE}
> %{buildroot}/lib/modules/%{KERNELRELEASE}/build
> +ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}%{MODLIB}/build
> %if %{with_devel}
> %{make} %{makeflags} run-command
> KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build
> %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> %endif
> @@ -98,8 +98,8 @@ fi
>
> %files
> %defattr (-, root, root)
> -/lib/modules/%{KERNELRELEASE}
> -%exclude /lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}
> +%exclude %{MODLIB}/build
> /boot/*
>
> %files headers
> @@ -110,5 +110,5 @@ fi
> %files devel
> %defattr (-, root, root)
> /usr/src/kernels/%{KERNELRELEASE}
> -/lib/modules/%{KERNELRELEASE}/build
> +%{MODLIB}/build
> %endif
> diff --git a/scripts/package/mkspec b/scripts/package/mkspec
> index d41608efb747..d41b2e5304ac 100755
> --- a/scripts/package/mkspec
> +++ b/scripts/package/mkspec
> @@ -18,6 +18,7 @@ fi
> cat< %define ARCH ${ARCH}
> %define KERNELRELEASE ${KERNELRELEASE}
> +%define MODLIB ${MODLIB}
> %define pkg_release $("${srctree}/init/build-version")
> EOF
>
> --
> 2.42.0
>
--
Best Regards
Masahiro Yamada
[PATCH v2] kbuild: deb-pkg: change the source package name to linux-upstream
Change the source package name from 'linux-$(KERNELRELEASE)' to
'linux-upstream'.
Initially, I tried to use 'linux' to be aligned with the Debian
kernel package, but Ben suggested 'linux-upstream' so that it is
clearly distinguished from distribution packages. [1]
The filenames will be changed as follows:
[Before]
linux-5.12.0-rc3+_5.12.0-rc3+-1.dsc
linux-5.12.0-rc3+_5.12.0-rc3+.orig.tar.gz
linux-5.12.0-rc3+_5.12.0-rc3+-1.diff.gz
[After]
linux-upstream_5.12.0-rc3+-1.dsc
linux-upstream_5.12.0-rc3+.orig.tar.gz
linux-upstream_5.12.0-rc3+-1.diff.gz
Commit 3716001bcb7f ("deb-pkg: add source package") introduced
KDEB_SOURCENAME. If you are unhappy with the default name, you can
override it via KDEB_SOURCENAME.
[1]:
https://lore.kernel.org/linux-kbuild/06ffa2a690d57f867b4bc1b42f0026917b1dd3cd.ca...@decadent.org.uk/T/#m2c4afa0eca5ced5e57795b002f2dbcb05d7a4a44
Suggested-by: Ben Hutchings
Signed-off-by: Masahiro Yamada
---
Changes in v2:
- Use 'linux-upstream' for the package name
scripts/Makefile.package | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.package b/scripts/Makefile.package
index f952fb64789d..b74c65284fb2 100644
--- a/scripts/Makefile.package
+++ b/scripts/Makefile.package
@@ -25,7 +25,7 @@ include $(srctree)/scripts/Kbuild.include
# Remove hyphens since they have special meaning in RPM filenames
KERNELPATH := kernel-$(subst -,_,$(KERNELRELEASE))
-KDEB_SOURCENAME ?= linux-$(KERNELRELEASE)
+KDEB_SOURCENAME ?= linux-upstream
KBUILD_PKG_ROOTCMD ?="fakeroot -u"
export KDEB_SOURCENAME
# Include only those top-level files that are needed by make, plus the GPL copy
--
2.27.0
[PATCH] kbuild: deb-pkg: change the source package name to 'linux'
Change the source package name from 'linux-$(KERNELRELEASE)' to 'linux',
which is aligned with the source package from the Debian community.
The filenames will be changed as follows:
[before]
linux-5.12.0-rc3+_5.12.0-rc3+-1.dsc
linux-5.12.0-rc3+_5.12.0-rc3+.orig.tar.gz
linux-5.12.0-rc3+_5.12.0-rc3+-1.diff.gz
[After]
linux_5.12.0-rc3+-1.dsc
linux_5.12.0-rc3+.orig.tar.gz
linux_5.12.0-rc3+-1.diff.gz
Commit 3716001bcb7f ("deb-pkg: add source package") introduced
KDEB_SOURCENAME. If you are unhappy with the default name, you can
override it via KDEB_SOURCENAME.
Signed-off-by: Masahiro Yamada
---
scripts/Makefile.package | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.package b/scripts/Makefile.package
index f952fb64789d..c5834a480545 100644
--- a/scripts/Makefile.package
+++ b/scripts/Makefile.package
@@ -25,7 +25,7 @@ include $(srctree)/scripts/Kbuild.include
# Remove hyphens since they have special meaning in RPM filenames
KERNELPATH := kernel-$(subst -,_,$(KERNELRELEASE))
-KDEB_SOURCENAME ?= linux-$(KERNELRELEASE)
+KDEB_SOURCENAME ?= linux
KBUILD_PKG_ROOTCMD ?="fakeroot -u"
export KDEB_SOURCENAME
# Include only those top-level files that are needed by make, plus the GPL copy
--
2.27.0
Re: [PATCH 1/2] kconfig: highlight gconfig 'comment' lines with '***'
On Sun, Apr 18, 2021 at 2:51 PM Randy Dunlap wrote: > > Mark Kconfig "comment" lines with "*** ***" > so that it is clear that these lines are comments and not some > kconfig item that cannot be modified. > > This is helpful in some menus to be able to provide a menu > "sub-heading" for groups of similar config items. > > This also makes the comments be presented in a way that is > similar to menuconfig and nconfig. > > Signed-off-by: Randy Dunlap > Cc: Masahiro Yamada > Cc: linux-kbu...@vger.kernel.org > --- Both applied to linux-kbuild. Thanks. -- Best Regards Masahiro Yamada
Re: [PATCH 1/2] kconfig: remove unused PACKAGE definition
On Sat, Apr 17, 2021 at 11:51 PM Masahiro Yamada wrote:
>
> Commit 3b9fa0931dd8 ("[PATCH] Kconfig i18n support") added this code,
> and then commit ("kconfig: drop localization support") removed the
> i18n support entirely.
>
> Remove the left-over.
>
> Signed-off-by: Masahiro Yamada
> ---
Both applied to linux-kbuild.
--
Best Regards
Masahiro Yamada
Re: [PATCH 04/13] Kbuild: Rust support
On Thu, Apr 15, 2021 at 9:43 AM Miguel Ojeda
wrote:
>
> On Thu, Apr 15, 2021 at 1:19 AM Nick Desaulniers
> wrote:
> >
> > Rather than check the origin (yikes, are we intentionally avoiding env
> > vars?), can this simply be
> > ifneq ($(CLIPPY),)
> > KBUILD_CLIPPY := $(CLIPPY)
> > endif
> >
> > Then you can specify whatever value you want, support command line or
> > env vars, etc.?
>
> I was following the other existing cases like `V`. Masahiro can
> probably answer why they are done like this.
You are asking about this code:
ifeq ("$(origin V)", "command line")
KBUILD_VERBOSE = $(V)
endif
You can pass V=1 from the Make command line,
but not from the environment.
KBUILD_VERBOSE is intended as an environment variable,
but you can use it from the Make command line.
Work:
- make V=1
- make KBUILD_VERBOSE=1
- KBUILD_VERBOSE=1 make
Not work:
- V=1 make
The behavior is like that before I became the maintainer.
In my best guess, the reason is,
V=1 is a useful shorthand of KBUILD_VERBOSE=1,
but it is too short. It should not accidentally
pick up an unintended environment variable.
--
Best Regards
Masahiro Yamada
Re: [PATCH v2] tools: do not include scripts/Kbuild.include
On Fri, Apr 16, 2021 at 10:01 PM Masahiro Yamada wrote:
>
> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> scripts/Makefile.compiler"), some kselftests fail to build.
>
> The tools/ directory opted out Kbuild, and went in a different
> direction. They copy any kind of files to the tools/ directory
> in order to do whatever they want in their world.
>
> tools/build/Build.include mimics scripts/Kbuild.include, but some
> tool Makefiles included the Kbuild one to import a feature that is
> missing in tools/build/Build.include:
>
> - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
>only if supported") included scripts/Kbuild.include from
>tools/thermal/tmon/Makefile to import the cc-option macro.
>
> - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
>not support -no-pie") included scripts/Kbuild.include from
>tools/testing/selftests/kvm/Makefile to import the try-run macro.
>
> - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
>failures") included scripts/Kbuild.include from
>tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
>target.
>
> - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
>unrecognized option") included scripts/Kbuild.include from
>tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
>try-run macro.
>
> Copy what they need into tools/build/Build.include, and make them
> include it instead of scripts/Kbuild.include.
>
> Link:
> https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f4...@linux.ibm.com/
> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> scripts/Makefile.compiler")
> Reported-by: Janosch Frank
> Reported-by: Christian Borntraeger
> Signed-off-by: Masahiro Yamada
Applied to linux-kbuild.
> ---
>
> Changes in v2:
> - copy macros to tools/build/BUild.include
>
> tools/build/Build.include | 24 +++
> tools/testing/selftests/bpf/Makefile | 2 +-
> tools/testing/selftests/kvm/Makefile | 2 +-
> .../selftests/powerpc/pmu/ebb/Makefile| 2 +-
> tools/thermal/tmon/Makefile | 2 +-
> 5 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/tools/build/Build.include b/tools/build/Build.include
> index 585486e40995..2cf3b1bde86e 100644
> --- a/tools/build/Build.include
> +++ b/tools/build/Build.include
> @@ -100,3 +100,27 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS)
> -D"BUILD_STR(s)=\#s" $(CXX
> ## HOSTCC C flags
>
> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS)
> -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> +
> +# output directory for tests below
> +TMPOUT = .tmp_
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" serves as a temporary file and is
> +# automatically cleaned up.
> +try-run = $(shell set -e; \
> + TMP=$(TMPOUT)/tmp; \
> + mkdir -p $(TMPOUT); \
> + trap "rm -rf $(TMPOUT)" EXIT; \
> + if ($(1)) >/dev/null 2>&1; \
> + then echo "$(2)"; \
> + else echo "$(3)"; \
> + fi)
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +cc-option = $(call try-run, \
> + $(CC) -Werror $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> +
> +# delete partially updated (i.e. corrupted) files on error
> +.DELETE_ON_ERROR:
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 044bfdcf5b74..17a5cdf48d37 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> -include ../../../../scripts/Kbuild.include
> +include ../../../build/Build.include
> include ../../../scripts/Makefile.arch
> include ../../../scripts/Makefile.include
>
> diff --git a/tools/testing/selftests/kvm/Makefile
> b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..5ef141f265bd 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -include ../../../../scripts/Kbuild.include
> +include ../../../build/Build.include
>
> all:
>
> diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
> b/tools/testing/sel
[PATCH 2/2] kconfig: gconf: remove unused code
Remove the unused inclusion, and commented out lines. Signed-off-by: Masahiro Yamada --- scripts/kconfig/gconf.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/scripts/kconfig/gconf.c b/scripts/kconfig/gconf.c index 5527482c3077..7698eea6fb6e 100644 --- a/scripts/kconfig/gconf.c +++ b/scripts/kconfig/gconf.c @@ -3,10 +3,6 @@ * Copyright (C) 2002-2003 Romain Lievin */ -#ifdef HAVE_CONFIG_H -# include -#endif - #include #include "lkc.h" #include "images.h" @@ -1452,9 +1448,6 @@ int main(int ac, char *av[]) gtk_init(, ); glade_init(); - //add_pixmap_directory (PACKAGE_DATA_DIR "/" PACKAGE "/pixmaps"); - //add_pixmap_directory (PACKAGE_SOURCE_DIR "/pixmaps"); - /* Determine GUI path */ env = getenv(SRCTREE); if (env) -- 2.27.0
[PATCH 1/2] kconfig: remove unused PACKAGE definition
Commit 3b9fa0931dd8 ("[PATCH] Kconfig i18n support") added this code,
and then commit ("kconfig: drop localization support") removed the
i18n support entirely.
Remove the left-over.
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/lkc.h | 4
1 file changed, 4 deletions(-)
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index 45599c52478d..fa8c010aa683 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -20,10 +20,6 @@ extern "C" {
#define SRCTREE "srctree"
-#ifndef PACKAGE
-#define PACKAGE "linux"
-#endif
-
#ifndef CONFIG_
#define CONFIG_ "CONFIG_"
#endif
--
2.27.0
[PATCH v2] tools: do not include scripts/Kbuild.include
Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler"), some kselftests fail to build.
The tools/ directory opted out Kbuild, and went in a different
direction. They copy any kind of files to the tools/ directory
in order to do whatever they want in their world.
tools/build/Build.include mimics scripts/Kbuild.include, but some
tool Makefiles included the Kbuild one to import a feature that is
missing in tools/build/Build.include:
- Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
only if supported") included scripts/Kbuild.include from
tools/thermal/tmon/Makefile to import the cc-option macro.
- Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
not support -no-pie") included scripts/Kbuild.include from
tools/testing/selftests/kvm/Makefile to import the try-run macro.
- Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
failures") included scripts/Kbuild.include from
tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
target.
- Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
unrecognized option") included scripts/Kbuild.include from
tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
try-run macro.
Copy what they need into tools/build/Build.include, and make them
include it instead of scripts/Kbuild.include.
Link:
https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f4...@linux.ibm.com/
Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler")
Reported-by: Janosch Frank
Reported-by: Christian Borntraeger
Signed-off-by: Masahiro Yamada
---
Changes in v2:
- copy macros to tools/build/BUild.include
tools/build/Build.include | 24 +++
tools/testing/selftests/bpf/Makefile | 2 +-
tools/testing/selftests/kvm/Makefile | 2 +-
.../selftests/powerpc/pmu/ebb/Makefile| 2 +-
tools/thermal/tmon/Makefile | 2 +-
5 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/tools/build/Build.include b/tools/build/Build.include
index 585486e40995..2cf3b1bde86e 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -100,3 +100,27 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS)
-D"BUILD_STR(s)=\#s" $(CXX
## HOSTCC C flags
host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS)
-D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
+
+# output directory for tests below
+TMPOUT = .tmp_
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" serves as a temporary file and is
+# automatically cleaned up.
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+cc-option = $(call try-run, \
+ $(CC) -Werror $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
+
+# delete partially updated (i.e. corrupted) files on error
+.DELETE_ON_ERROR:
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..17a5cdf48d37 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../scripts/Kbuild.include
+include ../../../build/Build.include
include ../../../scripts/Makefile.arch
include ../../../scripts/Makefile.include
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..5ef141f265bd 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
-include ../../../../scripts/Kbuild.include
+include ../../../build/Build.include
all:
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
index af3df79d8163..c5ecb4634094 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../../../scripts/Kbuild.include
+include ../../../../../build/Build.include
noarg:
$(MAKE) -C ../../
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index 59e417ec3e13..9db867df7679 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
# We need this for the "cc-option" macro.
-include ../../../scripts/Kbuild.include
+include ../../build/Build.include
VERSION = 1.0
--
2.27.0
Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
On Fri, Apr 16, 2021 at 2:56 PM Christian Borntraeger
wrote:
>
>
> On 15.04.21 10:06, Christian Borntraeger wrote:
> >
> > On 15.04.21 09:27, Masahiro Yamada wrote:
> >> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> >> scripts/Makefile.compiler"), some kselftests fail to build.
> >>
> >> The tools/ directory opted out Kbuild, and went in a different
> >> direction. They copy any kind of files to the tools/ directory
> >> in order to do whatever they want to do in their world.
> >>
> >> tools/build/Build.include mimics scripts/Kbuild.include, but some
> >> tool Makefiles included the Kbuild one to import a feature that is
> >> missing in tools/build/Build.include:
> >>
> >> - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
> >> only if supported") included scripts/Kbuild.include from
> >> tools/thermal/tmon/Makefile to import the cc-option macro.
> >>
> >> - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
> >> not support -no-pie") included scripts/Kbuild.include from
> >> tools/testing/selftests/kvm/Makefile to import the try-run macro.
> >>
> >> - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
> >> failures") included scripts/Kbuild.include from
> >> tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
> >> target.
> >>
> >> - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
> >> unrecognized option") included scripts/Kbuild.include from
> >> tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
> >> try-run macro.
> >>
> >> Copy what they want there, and stop including scripts/Kbuild.include
> >> from the tool Makefiles.
> >>
> >> Link:
> >> https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f4...@linux.ibm.com/
> >> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> >> scripts/Makefile.compiler")
> >> Reported-by: Janosch Frank
> >> Reported-by: Christian Borntraeger
> >> Signed-off-by: Masahiro Yamada
> >
> > When applying this on top of d9f4ff50d2aa ("kbuild: spilt cc-option and
> > friends to scripts/Makefile.compiler")
> >
> > I still do get
> >
> > # Test Assertion Failure
> > # lib/kvm_util.c:142: vm->fd >= 0
> > # pid=315635 tid=315635 - Invalid argument
> > # 10x01002f4b: vm_open at kvm_util.c:142
> > # 2 (inlined by) vm_create at kvm_util.c:258
> > # 30x010015ef: test_add_max_memory_regions at
> > set_memory_region_test.c:351
> > # 4 (inlined by) main at set_memory_region_test.c:397
> > # 50x03ff971abb89: ?? ??:0
> > # 60x010017ad: .annobin_abi_note.c.hot at crt1.o:?
> > # KVM_CREATE_VM ioctl failed, rc: -1 errno: 22
> > not ok 7 selftests: kvm: set_memory_region_test # exit=254
> >
> > and the testcase compilation does not pickup the pgste option.
>
> What does work is the following:
> diff --git a/tools/testing/selftests/kvm/Makefile
> b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..d9c6d9c2069e 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> include ../../../../scripts/Kbuild.include
> +include ../../../../scripts/Makefile.compiler
>
> all:
>
>
> as it does pickup the linker option handling.
Kbuild and the tools are divorced.
They cannot be married unless the tools/
build system is largely refactored.
That will be a tons of works (and
I am not sure if it is welcome).
The Kbuild refactoring should not be bothered by
the tools.
For now, I want them separated from each other.
--
Best Regards
Masahiro Yamada
Re: [PATCH v2] kconfig: redo fake deps at include/config/*.h
("fixdep");
> - exit(1);
> - }
> -}
> -
Applied to linux-kbuild.
I just found one more minor thing to fix up.
fixdep no longer calls putchar(), so I squashed the following:
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 5bee6a80992f..44e887cff49b 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -107,8 +107,8 @@ static void usage(void)
/*
* In the intended usage of this program, the stdout is redirected to .*.cmd
- * files. The return value of printf() and putchar() must be checked to catch
- * any error, e.g. "No space left on device".
+ * files. The return value of printf() must be checked to catch any error,
+ * e.g. "No space left on device".
*/
static void xprintf(const char *format, ...)
{
--
Best Regards
Masahiro Yamada
Re: [PATCH v2] uml: fix W=1 missing-include-dirs warnings
On Fri, Apr 16, 2021 at 4:14 AM Masahiro Yamada wrote: > > On Fri, Apr 16, 2021 at 2:14 AM Randy Dunlap wrote: > > > > Currently when using "W=1" with UML builds, there are over 700 warnings > > like so: > > > > CC arch/um/drivers/stderr_console.o > > cc1: warning: ./arch/um/include/uapi: No such file or directory > > [-Wmissing-include-dirs] > > > > but arch/um/ does not have include/uapi/ at all, so add that > > subdir and put one Kbuild file into it (since git does not track > > empty subdirs). > > > > Signed-off-by: Randy Dunlap > > Cc: Masahiro Yamada > > Cc: Michal Marek > > Cc: linux-kbu...@vger.kernel.org > > Cc: Jeff Dike > > Cc: Richard Weinberger > > Cc: Anton Ivanov > > Cc: linux...@lists.infradead.org > > --- > > v2: use Option 4 from v1: add arch/um/include/uapi so that 'make' is > > placated -- and just like all other arch's have. > > > > Assuming the UML maintainer will pick up this: > > Reviewed-by: Masahiro Yamada Now I see this patch queued up to UML repository. Thanks. -- Best Regards Masahiro Yamada
Re: [PATCH v3] kconfig: nconf: stop endless search loops
On Thu, Apr 15, 2021 at 4:28 PM Mihai Moldovan wrote:
>
> If the user selects the very first entry in a page and performs a
> search-up operation, or selects the very last entry in a page and
> performs a search-down operation that will not succeed (e.g., via
> [/]asdfzzz[Up Arrow]), nconf will never terminate searching the page.
>
> The reason is that in this case, the starting point will be set to -1
> or n, which is then translated into (n - 1) (i.e., the last entry of
> the page) or 0 (i.e., the first entry of the page) and finally the
> search begins. This continues to work fine until the index reaches 0 or
> (n - 1), at which point it will be decremented to -1 or incremented to
> n, but not checked against the starting point right away. Instead, it's
> wrapped around to the bottom or top again, after which the starting
> point check occurs... and naturally fails.
>
> My original implementation added another check for -1 before wrapping
> the running index variable around, but Masahiro Yamada pointed out that
> the actual issue is that the comparison point (starting point) exceeds
> bounds (i.e., the [0,n-1] interval) in the first place and that,
> instead, the starting point should be fixed.
>
> This has the welcome side-effect of also fixing the case where the
> starting point was n while searching down, which also lead to an
> infinite loop.
>
> OTOH, this code is now essentially all his work.
>
> Amazingly, nobody seems to have been hit by this for 11 years - or at
> the very least nobody bothered to debug and fix this.
>
> Signed-off-by: Mihai Moldovan
> ---
Applied to linux-kbuild. Thanks.
> v2: swap constant in comparison to right side, as requested by
> Randy Dunlap
> v3: reimplement as suggested by Masahiro Yamada ,
> which has the side-effect of also fixing endless looping in the
> symmetric down-direction
>
> scripts/kconfig/nconf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index e0f965529166..af814b39b876 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -504,8 +504,8 @@ static int get_mext_match(const char *match_str, match_f
> flag)
> else if (flag == FIND_NEXT_MATCH_UP)
> --match_start;
>
> + match_start = (match_start + items_num) % items_num;
> index = match_start;
> - index = (index + items_num) % items_num;
> while (true) {
> char *str = k_menu_items[index].str;
> if (strcasestr(str, match_str) != NULL)
> --
> 2.30.1
>
--
Best Regards
Masahiro Yamada
Re: [PATCH] clk: uniphier: Fix potential infinite loop
On Fri, Apr 16, 2021 at 3:19 AM Dan Carpenter wrote:
>
> On Fri, Apr 09, 2021 at 03:46:47PM +0900, Masahiro Yamada wrote:
> > On Thu, Apr 8, 2021 at 12:25 AM Colin King wrote:
> > >
> > > From: Colin Ian King
> > >
> > > The for-loop iterates with a u8 loop counter i and compares this
> > > with the loop upper limit of num_parents that is an int type.
> > > There is a potential infinite loop if num_parents is larger than
> > > the u8 loop counter. Fix this by making the loop counter the same
> > > type as num_parents.
> > >
> > > Addresses-Coverity: ("Infinite loop")
> > > Fixes: 734d82f4a678 ("clk: uniphier: add core support code for UniPhier
> > > clock driver")
> > > Signed-off-by: Colin Ian King
> > > ---
> > > drivers/clk/uniphier/clk-uniphier-mux.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/uniphier/clk-uniphier-mux.c
> > > b/drivers/clk/uniphier/clk-uniphier-mux.c
> > > index 462c84321b2d..ce219e0d2a85 100644
> > > --- a/drivers/clk/uniphier/clk-uniphier-mux.c
> > > +++ b/drivers/clk/uniphier/clk-uniphier-mux.c
> > > @@ -34,7 +34,7 @@ static u8 uniphier_clk_mux_get_parent(struct clk_hw *hw)
> > > int num_parents = clk_hw_get_num_parents(hw);
> > > int ret;
> > > unsigned int val;
> > > - u8 i;
> > > + int i;
> > >
> > > ret = regmap_read(mux->regmap, mux->reg, );
> > > if (ret)
> > > --
> > > 2.30.2
> > >
> >
> > clk_hw_get_num_parents() returns 'unsigned int', so
> > I think 'num_parents' should also have been 'unsigned int'.
> >
> > Maybe, the loop counter 'i' also should be 'unsigned int' then?
>
> The clk_hw_get_num_parents() function returns 0-255 so the original code
> works fine.
True. clk->core->num_parents is u8,
but it is not clear just by looking at the
prototype of clk_hw_get_num_parents().
At least, it is not clear enough for tools,
and actually Coverity raised a flag.
Personally, I prefer 'unsigned int' (or 'int')
when I count the number of something.
Historically, the clk subsystem uses u8,
(maybe to save memory??), and there exists
distortion.
For example, the return type of
uniphier_clk_mux_get_parent() is u8,
but it actually returns -EINVAL for error cases.
So, u8 is not wide enough, in my opinion.
>
> It should basically always be "int i;" That's the safest assumption.
> There are other case where it has to be size_t but in those cases I
> think people should call the list iterator something else instead of "i"
> like "size_t pg_idx;".
>
> Making everthing u32 causes more bugs than it prevents. Signedness bugs
> with comparing to zero, type promotion bugs, or subtraction bugs where
> subtracting wraps to a high value. It's rare to loop more than INT_MAX
> times in the kernel. When we do need to count about 2 million then
> we're probably not going to stop counting at 4 million, we're going to
> go to 10 million or higher so size_t is more appropriate than u32.
>
> Btw, if you have a loop that does:
>
> for (i = 0; i < UINT_MAX; i++) {
>
> that loop works exactly the same if "i" is an int or if it's a u32
> because of type promotion.
You are right.
Perhaps, in hindsight, the following were natural:
unsigned int num_parents = clk_hw_get_num_parents(hw);
...
int i;
I am fine with this if it is not too late.
But, Stephen has already picked up this patch.
> So you have to look really hard to find a
> place where changing a loop iterator from int to u32 fixes bug in real
> life.
>
> regards,
> dan carpenter
--
Best Regards
Masahiro Yamada
Re: [PATCH v2] uml: fix W=1 missing-include-dirs warnings
On Fri, Apr 16, 2021 at 2:14 AM Randy Dunlap wrote: > > Currently when using "W=1" with UML builds, there are over 700 warnings > like so: > > CC arch/um/drivers/stderr_console.o > cc1: warning: ./arch/um/include/uapi: No such file or directory > [-Wmissing-include-dirs] > > but arch/um/ does not have include/uapi/ at all, so add that > subdir and put one Kbuild file into it (since git does not track > empty subdirs). > > Signed-off-by: Randy Dunlap > Cc: Masahiro Yamada > Cc: Michal Marek > Cc: linux-kbu...@vger.kernel.org > Cc: Jeff Dike > Cc: Richard Weinberger > Cc: Anton Ivanov > Cc: linux...@lists.infradead.org > --- > v2: use Option 4 from v1: add arch/um/include/uapi so that 'make' is > placated -- and just like all other arch's have. Assuming the UML maintainer will pick up this: Reviewed-by: Masahiro Yamada > arch/um/include/uapi/asm/Kbuild |1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/um/include/uapi/asm/Kbuild b/arch/um/include/uapi/asm/Kbuild > new file mode 100644 > index ..f66554cd5c45 > --- /dev/null > +++ b/arch/um/include/uapi/asm/Kbuild > @@ -0,0 +1 @@ > +# SPDX-License-Identifier: GPL-2.0 -- Best Regards Masahiro Yamada
Re: [PATCH] kconfig: split menu.c out of parser.y
On Thu, Apr 15, 2021 at 7:54 PM Boris Kolpackov wrote: > > Masahiro Yamada writes: > > > --- /dev/null > > +++ b/scripts/kconfig/internal.h > > @@ -0,0 +1,9 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +#ifndef INTERNAL_H > > +#define INTERNAL_H > > + > > +struct menu; > > + > > +extern struct menu *current_menu, *current_entry; > > + > > +#endif /* INTERNAL_H */ > > Maybe call it menu.h instead of internal.h? Unless you have plans > to put other "internal" stuff in there. Yes, I will use this header for further cleanups of headers. > > > --- a/scripts/kconfig/parser.y > > +++ b/scripts/kconfig/parser.y > > @@ -28,7 +29,7 @@ static bool zconf_endtoken(const char *tokenname, > > > > struct symbol *symbol_hash[SYMBOL_HASHSIZE]; > > > > -static struct menu *current_menu, *current_entry; > > +struct menu *current_menu, *current_entry; > > Why not put these in menu.c? These variables are defined here before I started to maintain Kconfig. For now, I am just removing the 'static' directive. -- Best Regards Masahiro Yamada
Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
On Thu, Apr 15, 2021 at 4:40 PM Paolo Bonzini wrote:
>
> On 15/04/21 09:27, Masahiro Yamada wrote:
> > Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> > scripts/Makefile.compiler"), some kselftests fail to build.
> >
> > The tools/ directory opted out Kbuild, and went in a different
> > direction. They copy any kind of files to the tools/ directory
> > in order to do whatever they want to do in their world.
> >
> > tools/build/Build.include mimics scripts/Kbuild.include, but some
> > tool Makefiles included the Kbuild one to import a feature that is
> > missing in tools/build/Build.include:
> >
> > - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
> > only if supported") included scripts/Kbuild.include from
> > tools/thermal/tmon/Makefile to import the cc-option macro.
> >
> > - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
> > not support -no-pie") included scripts/Kbuild.include from
> > tools/testing/selftests/kvm/Makefile to import the try-run macro.
> >
> > - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
> > failures") included scripts/Kbuild.include from
> > tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
> > target.
> >
> > - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
> > unrecognized option") included scripts/Kbuild.include from
> > tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
> > try-run macro.
> >
> > Copy what they want there, and stop including scripts/Kbuild.include
> > from the tool Makefiles.
>
> I think it would make sense to add try-run, cc-option and
> .DELETE_ON_ERROR to tools/build/Build.include?
To be safe, I just copy-pasted what the makefiles need.
If someone wants to refactor the tool build system, that is fine,
but, to me, I do not see consistent rules or policy under tools/.
--
Best Regards
Masahiro Yamada
Re: [PATCH 3/4] kbuild: spilt cc-option and friends to scripts/Makefile.compiler
On Tue, Apr 13, 2021 at 10:11 PM Christian Borntraeger
wrote:
>
>
>
> On 13.04.21 14:51, Janosch Frank wrote:
> > On 2/28/21 7:10 AM, Masahiro Yamada wrote:
> >> scripts/Kbuild.include is included everywhere, but macros such as
> >> cc-option are needed by build targets only.
> >>
> >> For example, when 'make clean' traverses the tree, it does not need
> >> to evaluate $(call cc-option,).
> >>
> >> Split cc-option, ld-option, etc. to scripts/Makefile.compiler, which
> >> is only included from the top Makefile and scripts/Makefile.build.
> >>
> >> Signed-off-by: Masahiro Yamada
> >
> > This commit broke the KVM selftest compilation under s390 in linux-next
> > for me. Funny enough the compilation is only broken on Ubuntu, under
> > Fedora the test fails with an assertion.
> >
> > FEDORA:
> > [root@fedora kvm]# ./set_memory_region_test
> > Allowed number of memory slots: 32767
> > Test Assertion Failure
> >lib/kvm_util.c:142: vm->fd >= 0
> >pid=1541645 tid=1541645 - Invalid argument
> > 1 0x01002f4b: vm_open at kvm_util.c:142
> > 2(inlined by) vm_create at kvm_util.c:258
> > 3 0x010015ef: test_add_max_memory_regions at
> > set_memory_region_test.c:351
> > 4(inlined by) main at set_memory_region_test.c:397
> > 5 0x03ffa3d2bb89: ?? ??:0
> > 6 0x010017ad: .annobin_abi_note.c.hot at crt1.o:?
> >KVM_CREATE_VM ioctl failed, rc: -1 errno: 22
> >
> >
> > Ubuntu:
> > make[1]: Leaving directory '/mnt/dev/linux'
> > gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99
> > -fno-stack-protector -fno-PIE -I../../../../tools/include
> > -I../../../../tools/arch/s390/include -I../../../../usr/include/
> > -Iinclude -Ilib -Iinclude/s390x -I.. -c lib/sparsebit.c -o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/sparsebit.o
> > gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99
> > -fno-stack-protector -fno-PIE -I../../../../tools/include
> > -I../../../../tools/arch/s390/include -I../../../../usr/include/
> > -Iinclude -Ilib -Iinclude/s390x -I.. -c lib/kvm_util.c -o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/kvm_util.o
> > gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99
> > -fno-stack-protector -fno-PIE -I../../../../tools/include
> > -I../../../../tools/arch/s390/include -I../../../../usr/include/
> > -Iinclude -Ilib/s390x -Iinclude/s390x -I.. -c
> > lib/s390x/diag318_test_handler.c -o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.o
> > ar crs /mnt/dev/linux/tools/testing/selftests/kvm/libkvm.a
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/assert.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/elf.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/io.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/kvm_util.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/sparsebit.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/test_util.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/guest_modes.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/perf_test_util.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/processor.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/ucall.o
> > /mnt/dev/linux/tools/testing/selftests/kvm/lib/s390x/diag318_test_handler.o
> > gcc -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99
> > -fno-stack-protector -fno-PIE -I../../../../tools/include
> > -I../../../../tools/arch/s390/include -I../../../../usr/include/
> > -Iinclude -Is390x -Iinclude/s390x -I.. -pthreads390x/memop.c
> > /mnt/dev/linux/tools/testing/selftests/kvm/libkvm.a -o
> > /mnt/dev/linux/tools/testing/selftests/kvm/s390x/memop
> > /usr/bin/ld: /tmp/ccFU8WYF.o: `stdout@@GLIBC_2.2' non-PLT reloc for
> > symbol defined in shared library and accessed from executable (rebuild
> > file with -fPIC ?)
> > /usr/bin/ld: final link failed: bad value
> > collect2: error: ld returned 1 exit status
> > make: *** [../lib.mk:139:
> > /mnt/dev/linux/tools/testing/selftests/kvm/s390x/memop] Error 1
> >
>
>
>
> It looks like that from tools/testing/selftests/kvm/Makefile
> additional linker flags are being ignored with this patch.
>
> no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
> $(CC) -Werror -no-pie -x c - -o "$$TMP", -no-pie)
>
> # On s390, build the testcases KVM-enabled
> pgste-option = $(call try-run, echo 'int main() { return 0; }' | \
> $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o
> "$$TMP",-Wl$(comma)--s390-pgste)
>
>
> LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
>
Thanks.
I will separate Kbuild and the tool build system.
https://patchwork.kernel.org/project/linux-kbuild/patch/20210415072700.147125-2-masahi...@kernel.org/
I do not want to be bothered by the can of worms.
--
Best Regards
Masahiro Yamada
[PATCH 2/2] tools: do not include scripts/Kbuild.include
Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler"), some kselftests fail to build.
The tools/ directory opted out Kbuild, and went in a different
direction. They copy any kind of files to the tools/ directory
in order to do whatever they want to do in their world.
tools/build/Build.include mimics scripts/Kbuild.include, but some
tool Makefiles included the Kbuild one to import a feature that is
missing in tools/build/Build.include:
- Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
only if supported") included scripts/Kbuild.include from
tools/thermal/tmon/Makefile to import the cc-option macro.
- Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
not support -no-pie") included scripts/Kbuild.include from
tools/testing/selftests/kvm/Makefile to import the try-run macro.
- Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
failures") included scripts/Kbuild.include from
tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
target.
- Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
unrecognized option") included scripts/Kbuild.include from
tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
try-run macro.
Copy what they want there, and stop including scripts/Kbuild.include
from the tool Makefiles.
Link:
https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f4...@linux.ibm.com/
Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler")
Reported-by: Janosch Frank
Reported-by: Christian Borntraeger
Signed-off-by: Masahiro Yamada
---
tools/testing/selftests/bpf/Makefile | 3 ++-
tools/testing/selftests/kvm/Makefile | 12 +++-
.../selftests/powerpc/pmu/ebb/Makefile| 11 ++-
tools/thermal/tmon/Makefile | 19 +--
4 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d872b9f41543 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,5 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../scripts/Kbuild.include
include ../../../scripts/Makefile.arch
include ../../../scripts/Makefile.include
@@ -476,3 +475,5 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR)
$(HOST_SCRATCH_DIR) \
prog_tests/tests.h map_tests/tests.h verifier/tests.h \
feature \
$(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc bpf_testmod.ko)
+
+.DELETE_ON_ERROR:
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..8b45bc417d83 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,15 @@
# SPDX-License-Identifier: GPL-2.0-only
-include ../../../../scripts/Kbuild.include
+
+TMPOUT = .tmp_
+
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
all:
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
index af3df79d8163..d5d3e869df93 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
@@ -1,5 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../../../scripts/Kbuild.include
noarg:
$(MAKE) -C ../../
@@ -8,6 +7,16 @@ noarg:
CFLAGS += -m64
TMPOUT = $(OUTPUT)/TMPDIR/
+
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
+
# Toolchains may build PIE by default which breaks the assembly
no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o
"$$TMP", -no-pie)
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index 59e417ec3e13..92a683e4866c 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -1,6 +1,21 @@
# SPDX-License-Identifier: GPL-2.0
-# We need this for the "cc-option" macro.
-include ../../../scripts/Kbuild.include
+
+TMPOUT = .tmp_
+
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(T
[PATCH 1/2] kbuild: remove TMPO from try-run
TMPO is only used by arch/x86/Makefile. Change arch/x86/Makefile to use $$TMPO.o and remove TMPO from scripts/Makefile.compiler. Signed-off-by: Masahiro Yamada --- arch/x86/Makefile | 4 ++-- scripts/Makefile.compiler | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 2d6d5a28c3bf..c55da2833fe8 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -129,8 +129,8 @@ ifdef CONFIG_X86_X32 x32_ld_ok := $(call try-run,\ /bin/echo -e '1: .quad 1b' | \ $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" - && \ - $(OBJCOPY) -O elf32-x86-64 "$$TMP" "$$TMPO" && \ - $(LD) -m elf32_x86_64 "$$TMPO" -o "$$TMP",y,n) + $(OBJCOPY) -O elf32-x86-64 "$$TMP" "$$TMP.o" && \ + $(LD) -m elf32_x86_64 "$$TMP.o" -o "$$TMP",y,n) ifeq ($(x32_ld_ok),y) CONFIG_X86_X32_ABI := y KBUILD_AFLAGS += -DCONFIG_X86_X32_ABI diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 3f2f3665216f..86ecd2ac874c 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -21,7 +21,6 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_ # automatically cleaned up. try-run = $(shell set -e; \ TMP=$(TMPOUT)/tmp; \ - TMPO=$(TMPOUT)/tmp.o; \ mkdir -p $(TMPOUT); \ trap "rm -rf $(TMPOUT)" EXIT; \ if ($(1)) >/dev/null 2>&1; \ -- 2.27.0
Re: [PATCH] uml: fix W=1 missing-include-dirs warnings
On Thu, Apr 15, 2021 at 4:02 PM Randy Dunlap wrote: > > On 4/14/21 11:52 PM, Masahiro Yamada wrote: > > On Thu, Apr 15, 2021 at 4:27 AM Randy Dunlap wrote: > >> > >> Currently when using "W=1" with UML builds, there are over 700 warnings > >> like so: > >> > >> CC arch/um/drivers/stderr_console.o > >> cc1: warning: ./arch/um/include/uapi: No such file or directory > >> [-Wmissing-include-dirs] > >> > >> but arch/um/ does not have include/uapi/ at all, so don't > >> include arch/um/include/uapi/ in USERINCLUDE for UML. > > > >> Option 4: simply mkdir arch/um/include/uapi > >> That's what I did first, just as a test, and it works. > > > > > > I like Option 4. > > > > But, you cannot do "mkdir -p arch/um/include/uapi" at build-time > > because the build system should not touch the source tree(, which > > might be read-only) > > for O= building. > > > > How about adding > > > > arch/um/include/uapi/asm/Kbuild, > > > > which is just having a SPDX one-liner? > > Wow! :) > That's what Al Viro suggested also. > I'll submit that patch later today (Thursday my time). > > thanks. > -- > ~Randy > BTW, after fixing this UML problem, can we move -Wmissing-include-dirs to the top Makefile? Is there any other source of -Wmissing-include-dirs warnings? -- Best Regards Masahiro Yamada
Re: [PATCH] uml: fix W=1 missing-include-dirs warnings
On Thu, Apr 15, 2021 at 4:27 AM Randy Dunlap wrote: > > Currently when using "W=1" with UML builds, there are over 700 warnings > like so: > > CC arch/um/drivers/stderr_console.o > cc1: warning: ./arch/um/include/uapi: No such file or directory > [-Wmissing-include-dirs] > > but arch/um/ does not have include/uapi/ at all, so don't > include arch/um/include/uapi/ in USERINCLUDE for UML. > > Signed-off-by: Randy Dunlap > Cc: Masahiro Yamada > Cc: Michal Marek > Cc: linux-kbu...@vger.kernel.org > Cc: Jeff Dike > Cc: Richard Weinberger > Cc: Anton Ivanov > Cc: linux...@lists.infradead.org > --- > Makefile | 10 ++ > 1 file changed, 10 insertions(+) > > Option 2: change the setting of USERINCLUDE. This could alter > (a) build times and > (b) which header files get used: if there are multiple > header files named foobar.h in the $(USERINCLUDE) > subdirectories, this Option changes the order in which > they would be found. > > - linux-next-20210413.orig/Makefile > + linux-next-20210413/Makefile > @@ -501,13 +501,16 @@ LDFLAGS_vmlinux = > > # Use USERINCLUDE when you must reference the UAPI directories only. > USERINCLUDE:= \ > - -I$(srctree)/arch/$(SRCARCH)/include/uapi \ > -I$(objtree)/arch/$(SRCARCH)/include/generated/uapi \ > -I$(srctree)/include/uapi \ > -I$(objtree)/include/generated/uapi \ > -include $(srctree)/include/linux/compiler-version.h \ > -include $(srctree)/include/linux/kconfig.h > > +ifneq ($(ARCH),um) > +USERINCLUDE+= -I$(srctree)/arch/$(SRCARCH)/include/uapi > +endif > + > # Use LINUXINCLUDE when you must reference the include/ directory. > # Needed to be compatible with the O= option > LINUXINCLUDE:= \ > > Option 3: modify scripts/Makefile.extrawarn not to set > -Wmissing-include-dirs for arch=um. I think that this is not > a good idea: it could cause valid problem reports not to be > reported. > > Option 4: simply mkdir arch/um/include/uapi > That's what I did first, just as a test, and it works. I like Option 4. But, you cannot do "mkdir -p arch/um/include/uapi" at build-time because the build system should not touch the source tree(, which might be read-only) for O= building. How about adding arch/um/include/uapi/asm/Kbuild, which is just having a SPDX one-liner? > > --- linux-next-20210413.orig/Makefile > +++ linux-next-20210413/Makefile > @@ -500,6 +500,15 @@ AFLAGS_KERNEL = > LDFLAGS_vmlinux = > > # Use USERINCLUDE when you must reference the UAPI directories only. > +# Note: arch/um/ does not have an include/uapi/ subdir. > +ifeq ($(ARCH),um) > +USERINCLUDE:= \ > + -I$(objtree)/arch/$(SRCARCH)/include/generated/uapi \ > + -I$(srctree)/include/uapi \ > + -I$(objtree)/include/generated/uapi \ > +-include $(srctree)/include/linux/compiler-version.h \ > +-include $(srctree)/include/linux/kconfig.h > +else > USERINCLUDE:= \ > -I$(srctree)/arch/$(SRCARCH)/include/uapi \ > -I$(objtree)/arch/$(SRCARCH)/include/generated/uapi \ > @@ -507,6 +516,7 @@ USERINCLUDE:= \ > -I$(objtree)/include/generated/uapi \ > -include $(srctree)/include/linux/compiler-version.h \ > -include $(srctree)/include/linux/kconfig.h > +endif > > # Use LINUXINCLUDE when you must reference the include/ directory. > # Needed to be compatible with the O= option -- Best Regards Masahiro Yamada
Re: [PATCH] kconfig: redo fake deps at include/config/*.h
On Thu, Apr 15, 2021 at 7:01 AM Alexey Dobriyan wrote:
>
> Make include/config/foo/bar.h fake deps files generation simpler.
>
> * delete .h suffix
> those aren't header files, shorten filenames,
>
> * delete tolower()
> Linux filesystems can deal with both upper and lowercase
> filenames very well,
>
> * put everything in 1 directory
> Presumably 'mkdir -p' split is from dark times when filesystems
> handled huge directories badly, disks were round adding to
> seek times.
I am not sure about the impact of this change
given various file systems in the wild,
but this simplification is attractive.
With a quick search, I found a comment
'performance issues past 10,000' on ext2 [1]
but that may not be what we care about much...
[1]:
https://webmasters.stackexchange.com/questions/99539/what-is-a-recommended-maximum-number-of-files-in-a-directory-on-your-webserver
> @@ -124,36 +124,12 @@ static void xprintf(const char *format, ...)
> va_end(ap);
> }
>
> -static void xputchar(int c)
> -{
> - int ret;
> -
> - ret = putchar(c);
> - if (ret == EOF) {
> - perror("fixdep");
> - exit(1);
> - }
> -}
> -
> /*
> * Print out a dependency path from a symbol name
> */
> static void print_dep(const char *m, int slen, const char *dir)
> {
> - int c, prev_c = '/', i;
> -
> - xprintf("$(wildcard %s/", dir);
> - for (i = 0; i < slen; i++) {
> - c = m[i];
> - if (c == '_')
> - c = '/';
> - else
> - c = tolower(c);
> - if (c != '/' || prev_c != '/')
> - xputchar(c);
> - prev_c = c;
> - }
> - xprintf(".h) \\\n");
> + xprintf("$(wildcard %s/%.*s) \\\n", dir, slen, m);
Since this function now contains just one line,
can you hard-code
xprintf("$(wildcard include/config/%.*s) \\\n", slen, m);
in use_config() ?
> }
>
> struct item {
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -130,19 +130,14 @@ static size_t depfile_prefix_len;
> static int conf_touch_dep(const char *name)
> {
> int fd, ret;
> - const char *s;
> - char *d, c;
> + char *d;
>
> /* check overflow: prefix + name + ".h" + '\0' must fit in buffer. */
> if (depfile_prefix_len + strlen(name) + 3 > sizeof(depfile_path))
Since you dropped the ".h" suffix,
please fix up this line.
Also, you can fix
# changed, Kconfig touches the corresponding timestamp file
include/config/*.h.
in kernel/gen_kheaders.sh
> return -1;
>
> d = depfile_path + depfile_prefix_len;
> - s = name;
> -
> - while ((c = *s++))
> - *d++ = (c == '_') ? '/' : tolower(c);
> - strcpy(d, ".h");
> + strcpy(d, name);
>
> /* Assume directory path already exists. */
> fd = open(depfile_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
> @@ -465,7 +460,7 @@ int conf_read_simple(const char *name, int def)
> * Reading from
> include/config/auto.conf
> * If CONFIG_FOO previously existed in
> * auto.conf but it is missing now,
> - * include/config/foo.h must be
> touched.
> +* include/config/FOO must be touched.
> */
> conf_touch_dep(line +
> strlen(CONFIG_));
> else
--
Best Regards
Masahiro Yamada
[PATCH] kconfig: split menu.c out of parser.y
Compile menu.c as an independent compilation unit. Signed-off-by: Masahiro Yamada --- scripts/kconfig/Makefile | 4 ++-- scripts/kconfig/internal.h | 9 + scripts/kconfig/menu.c | 1 + scripts/kconfig/parser.y | 5 ++--- 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 scripts/kconfig/internal.h diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index 1d1a7f83ee8d..5a215880b268 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -143,8 +143,8 @@ help: # === # object files used by all kconfig flavours -common-objs:= confdata.o expr.o lexer.lex.o parser.tab.o preprocess.o \ - symbol.o util.o +common-objs:= confdata.o expr.o lexer.lex.o menu.o parser.tab.o \ + preprocess.o symbol.o util.o $(obj)/lexer.lex.o: $(obj)/parser.tab.h HOSTCFLAGS_lexer.lex.o := -I $(srctree)/$(src) diff --git a/scripts/kconfig/internal.h b/scripts/kconfig/internal.h new file mode 100644 index ..2f7298c21b64 --- /dev/null +++ b/scripts/kconfig/internal.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef INTERNAL_H +#define INTERNAL_H + +struct menu; + +extern struct menu *current_menu, *current_entry; + +#endif /* INTERNAL_H */ diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index 8b2108b74821..606ba8a63c24 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -9,6 +9,7 @@ #include #include "lkc.h" +#include "internal.h" static const char nohelp_text[] = "There is no help available for this option."; diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y index e90889edf5b3..2af7ce4e1531 100644 --- a/scripts/kconfig/parser.y +++ b/scripts/kconfig/parser.y @@ -12,6 +12,7 @@ #include #include "lkc.h" +#include "internal.h" #define printd(mask, fmt...) if (cdebug & (mask)) printf(fmt) @@ -28,7 +29,7 @@ static bool zconf_endtoken(const char *tokenname, struct symbol *symbol_hash[SYMBOL_HASHSIZE]; -static struct menu *current_menu, *current_entry; +struct menu *current_menu, *current_entry; %} @@ -713,5 +714,3 @@ void zconfdump(FILE *out) } } } - -#include "menu.c" -- 2.27.0
Re: [PATCH] kbuild: use ?= to assign CROSS_COMPILE by arch-Makefile
On Mon, Apr 12, 2021 at 5:15 PM Masahiro Yamada wrote:
>
> On Mon, Apr 12, 2021 at 4:44 PM Geert Uytterhoeven
> wrote:
> >
> > Hi Yamada-san,
> >
> > On Sun, Apr 11, 2021 at 3:56 PM Masahiro Yamada
> > wrote:
> > > Use ?= operator to let arch/*/Makefile to assign CROSS_COMPILE only
> > > when CROSS_COMPILE is undefined.
> > >
> > > This allows arch-Makefiles to drop the ifeq ($(CROSS_COMPILE),)
> > > conditional.
> > >
> > > This slightly changes the behavior; the arch-Makefile previously
> > > overrode CROSS_COMPILE when CROSS_COMPILE has already been made empty
> > > via an environment variable as in 'export CROSS_COMPILE='.
> > >
> > > With this commit, arch-Makefle will respect the user's environment
> > > set-up, which seems to be a more correct behavior.
> > >
> > > Signed-off-by: Masahiro Yamada
> >
> > Thanks for your patch!
> >
> > > ---
> > >
> > > arch/arc/Makefile| 4 +---
> > > arch/h8300/Makefile | 4 +---
> > > arch/m68k/Makefile | 4 +---
> > > arch/mips/Makefile | 4 +---
> > > arch/parisc/Makefile | 6 ++
> > > arch/sh/Makefile | 4 +---
> >
> > What about arch/xtensa/Makefile?
> >
> > > --- a/arch/m68k/Makefile
> > > +++ b/arch/m68k/Makefile
> > > @@ -17,10 +17,8 @@
> > > KBUILD_DEFCONFIG := multi_defconfig
> > >
> > > ifneq ($(SUBARCH),$(ARCH))
> > > - ifeq ($(CROSS_COMPILE),)
> > > - CROSS_COMPILE := $(call cc-cross-prefix, \
> > > + CROSS_COMPILE ?= $(call cc-cross-prefix, \
> > > m68k-linux-gnu- m68k-linux-
> > > m68k-unknown-linux-gnu-)
> > > - endif
> > > endif
> >
> > This does not seem to work as expected: my standard build scripts
> > (using "make ARCH=m68k") no longer pick up the cross-compiler,
> > but fall back to the native compiler, thus breaking the build.
>
>
> Agh, sorry, this patch does not work
> because the top Makefile exports CROSS_COMPILE.
>
> export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS
> CROSS_COMPILE LD CC
>
>
>
> Removing CROSS_COMPILE from that makes ?= work,
> but it would break other parts.
>
>
> Please ignore this patch.
>
>
>
>
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > ge...@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
>
The following will make this patch work, but probably it is better to
not do this...
diff --git a/Makefile b/Makefile
index cc77fd45ca64..26bf482f0d88 100644
--- a/Makefile
+++ b/Makefile
@@ -506,7 +506,7 @@ KBUILD_LDFLAGS_MODULE :=
KBUILD_LDFLAGS :=
CLANG_FLAGS :=
-export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS
CROSS_COMPILE LD CC
+export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS LD CC
export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS
LEX YACC AWK INSTALLKERNEL
export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD
@@ -681,6 +681,8 @@ export RETPOLINE_VDSO_CFLAGS
include arch/$(SRCARCH)/Makefile
+export CROSS_COMPILE
+
ifdef need-config
ifdef may-sync-config
# Read in dependencies to all Kconfig* files, make sure to run syncconfig if
--
Best Regards
Masahiro Yamada
Re: [PATCH] kbuild: use ?= to assign CROSS_COMPILE by arch-Makefile
On Mon, Apr 12, 2021 at 4:44 PM Geert Uytterhoeven wrote:
>
> Hi Yamada-san,
>
> On Sun, Apr 11, 2021 at 3:56 PM Masahiro Yamada wrote:
> > Use ?= operator to let arch/*/Makefile to assign CROSS_COMPILE only
> > when CROSS_COMPILE is undefined.
> >
> > This allows arch-Makefiles to drop the ifeq ($(CROSS_COMPILE),)
> > conditional.
> >
> > This slightly changes the behavior; the arch-Makefile previously
> > overrode CROSS_COMPILE when CROSS_COMPILE has already been made empty
> > via an environment variable as in 'export CROSS_COMPILE='.
> >
> > With this commit, arch-Makefle will respect the user's environment
> > set-up, which seems to be a more correct behavior.
> >
> > Signed-off-by: Masahiro Yamada
>
> Thanks for your patch!
>
> > ---
> >
> > arch/arc/Makefile| 4 +---
> > arch/h8300/Makefile | 4 +---
> > arch/m68k/Makefile | 4 +---
> > arch/mips/Makefile | 4 +---
> > arch/parisc/Makefile | 6 ++
> > arch/sh/Makefile | 4 +---
>
> What about arch/xtensa/Makefile?
>
> > --- a/arch/m68k/Makefile
> > +++ b/arch/m68k/Makefile
> > @@ -17,10 +17,8 @@
> > KBUILD_DEFCONFIG := multi_defconfig
> >
> > ifneq ($(SUBARCH),$(ARCH))
> > - ifeq ($(CROSS_COMPILE),)
> > - CROSS_COMPILE := $(call cc-cross-prefix, \
> > + CROSS_COMPILE ?= $(call cc-cross-prefix, \
> > m68k-linux-gnu- m68k-linux- m68k-unknown-linux-gnu-)
> > - endif
> > endif
>
> This does not seem to work as expected: my standard build scripts
> (using "make ARCH=m68k") no longer pick up the cross-compiler,
> but fall back to the native compiler, thus breaking the build.
Agh, sorry, this patch does not work
because the top Makefile exports CROSS_COMPILE.
export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS
CROSS_COMPILE LD CC
Removing CROSS_COMPILE from that makes ?= work,
but it would break other parts.
Please ignore this patch.
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@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
--
Best Regards
Masahiro Yamada
[PATCH 3/3] kconfig: nconf: refactor in print_in_middle()
This helper is the same as the sample code in the NCURSES HOWTO [1],
but it is over-engineering to be used for nconf.
I do not see any good reason to use the 'float' type just for the
division by 2.
All the call-sites pass a non-NULL pointer to the first argument,
so 'if (win == NULL) win = stdscr;' is dead code.
'if (startx != 0) x = startx;' is dead code because 'x' will be
overridden some lines below, by 'x = startx + (int)temp;'.
All the call-sites pass a non-zero value to the second argument,
so 'if (starty != 0)' is always true.
getyx(win, y, x) is also dead-code because both 'y' and 'x' are
overridden.
All the call-sites pass 0 to the third parameter, so 'startx' can
be removed.
All the call-sites pass a non-zero value to the fourth parameter,
so 'if (width == 0) width = 80;' is dead code.
Change the type of the last parameter from 'chtype' to 'int' to be
aligned with the prototype, 'int wattrset(WINDOW *win, int attrs);'
I also slightly cleaned up the indentation style.
[1]: https://tldp.org/HOWTO/NCURSES-Programming-HOWTO/color.html
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/nconf.c | 2 +-
scripts/kconfig/nconf.gui.c | 31 ---
scripts/kconfig/nconf.h | 7 +--
3 files changed, 6 insertions(+), 34 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 5209a18eeacb..b11b75f83f7e 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -953,7 +953,7 @@ static void show_menu(const char *prompt, const char
*instructions,
current_instructions = instructions;
clear();
- print_in_middle(stdscr, 1, 0, getmaxx(stdscr),
+ print_in_middle(stdscr, 1, getmaxx(stdscr),
menu_backtitle,
attr_main_heading);
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index e747590cee17..9aedf40f1dc0 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -117,32 +117,10 @@ void set_colors(void)
}
/* this changes the windows attributes !!! */
-void print_in_middle(WINDOW *win,
- int starty,
- int startx,
- int width,
- const char *string,
- chtype color)
-{ int length, x, y;
- float temp;
-
-
- if (win == NULL)
- win = stdscr;
- getyx(win, y, x);
- if (startx != 0)
- x = startx;
- if (starty != 0)
- y = starty;
- if (width == 0)
- width = 80;
-
- length = strlen(string);
- temp = (width - length) / 2;
- x = startx + (int)temp;
- wattrset(win, color);
- mvwprintw(win, y, x, "%s", string);
- refresh();
+void print_in_middle(WINDOW *win, int y, int width, const char *str, int attrs)
+{
+ wattrset(win, attrs);
+ mvwprintw(win, y, (width - strlen(str)) / 2, "%s", str);
}
int get_line_no(const char *text)
@@ -577,7 +555,6 @@ void show_scroll_win(WINDOW *main_window,
text_cols, 0);
print_in_middle(win,
text_lines+2,
- 0,
text_cols,
"",
attr_dialog_menu_fore);
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 90a1ae331878..6f925bc74eb3 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -68,12 +68,7 @@ typedef enum {
void set_colors(void);
/* this changes the windows attributes !!! */
-void print_in_middle(WINDOW *win,
- int starty,
- int startx,
- int width,
- const char *string,
- chtype color);
+void print_in_middle(WINDOW *win, int y, int width, const char *str, int
attrs);
int get_line_length(const char *line);
int get_line_no(const char *text);
const char *get_line(const char *text, int line_no);
--
2.27.0
[PATCH 1/3] kconfig: nconf: change set_config_filename() to void function
No one uses the return value of this function.
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/nconf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index e86d3511b939..d8a6ab5fb521 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -631,13 +631,12 @@ static int item_is_tag(char tag)
static char filename[PATH_MAX+1];
static char menu_backtitle[PATH_MAX+128];
-static const char *set_config_filename(const char *config_filename)
+static void set_config_filename(const char *config_filename)
{
snprintf(menu_backtitle, sizeof(menu_backtitle), "%s - %s",
config_filename, rootmenu.prompt->text);
snprintf(filename, sizeof(filename), "%s", config_filename);
- return menu_backtitle;
}
/* return = 0 means we are successful.
--
2.27.0
[PATCH 2/3] kconfig: nconf: remove meaningless wattrset() call from show_menu()
This attribute is not used because it will be overridden some lines below: wattrset(main_window, attr_main_menu_box); Signed-off-by: Masahiro Yamada --- scripts/kconfig/nconf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c index d8a6ab5fb521..5209a18eeacb 100644 --- a/scripts/kconfig/nconf.c +++ b/scripts/kconfig/nconf.c @@ -953,7 +953,6 @@ static void show_menu(const char *prompt, const char *instructions, current_instructions = instructions; clear(); - wattrset(main_window, attr_normal); print_in_middle(stdscr, 1, 0, getmaxx(stdscr), menu_backtitle, attr_main_heading); -- 2.27.0
[PATCH] kbuild: use ?= to assign CROSS_COMPILE by arch-Makefile
Use ?= operator to let arch/*/Makefile to assign CROSS_COMPILE only when CROSS_COMPILE is undefined. This allows arch-Makefiles to drop the ifeq ($(CROSS_COMPILE),) conditional. This slightly changes the behavior; the arch-Makefile previously overrode CROSS_COMPILE when CROSS_COMPILE has already been made empty via an environment variable as in 'export CROSS_COMPILE='. With this commit, arch-Makefle will respect the user's environment set-up, which seems to be a more correct behavior. Signed-off-by: Masahiro Yamada --- arch/arc/Makefile| 4 +--- arch/h8300/Makefile | 4 +--- arch/m68k/Makefile | 4 +--- arch/mips/Makefile | 4 +--- arch/parisc/Makefile | 6 ++ arch/sh/Makefile | 4 +--- 6 files changed, 7 insertions(+), 19 deletions(-) diff --git a/arch/arc/Makefile b/arch/arc/Makefile index 4392c9c189c4..bd5a9daa3461 100644 --- a/arch/arc/Makefile +++ b/arch/arc/Makefile @@ -5,9 +5,7 @@ KBUILD_DEFCONFIG := haps_hs_smp_defconfig -ifeq ($(CROSS_COMPILE),) -CROSS_COMPILE := $(call cc-cross-prefix, arc-linux- arceb-linux-) -endif +CROSS_COMPILE ?= $(call cc-cross-prefix, arc-linux- arceb-linux-) cflags-y += -fno-common -pipe -fno-builtin -mmedium-calls -D__linux__ diff --git a/arch/h8300/Makefile b/arch/h8300/Makefile index ba0f26cfad61..d6e466dbfc00 100644 --- a/arch/h8300/Makefile +++ b/arch/h8300/Makefile @@ -26,9 +26,7 @@ KBUILD_LDFLAGS += $(ldflags-y) CHECKFLAGS += -msize-long -ifeq ($(CROSS_COMPILE),) -CROSS_COMPILE := $(call cc-cross-prefix, h8300-unknown-linux- h8300-linux-) -endif +CROSS_COMPILE ?= $(call cc-cross-prefix, h8300-unknown-linux- h8300-linux-) core-y += arch/$(ARCH)/kernel/ arch/$(ARCH)/mm/ core-y += arch/$(ARCH)/boot/dts/ diff --git a/arch/m68k/Makefile b/arch/m68k/Makefile index ea14f2046fb4..79208ad7a355 100644 --- a/arch/m68k/Makefile +++ b/arch/m68k/Makefile @@ -17,10 +17,8 @@ KBUILD_DEFCONFIG := multi_defconfig ifneq ($(SUBARCH),$(ARCH)) - ifeq ($(CROSS_COMPILE),) - CROSS_COMPILE := $(call cc-cross-prefix, \ + CROSS_COMPILE ?= $(call cc-cross-prefix, \ m68k-linux-gnu- m68k-linux- m68k-unknown-linux-gnu-) - endif endif # diff --git a/arch/mips/Makefile b/arch/mips/Makefile index e71d587af49c..75e4e46532a4 100644 --- a/arch/mips/Makefile +++ b/arch/mips/Makefile @@ -51,9 +51,7 @@ UTS_MACHINE := mips64 endif ifneq ($(SUBARCH),$(ARCH)) - ifeq ($(CROSS_COMPILE),) -CROSS_COMPILE := $(call cc-cross-prefix, $(tool-archpref)-linux- $(tool-archpref)-linux-gnu- $(tool-archpref)-unknown-linux-gnu-) - endif + CROSS_COMPILE ?= $(call cc-cross-prefix, $(tool-archpref)-linux- $(tool-archpref)-linux-gnu- $(tool-archpref)-unknown-linux-gnu-) endif ifdef CONFIG_FUNCTION_GRAPH_TRACER diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile index 7d9f71aa829a..62272cb3513c 100644 --- a/arch/parisc/Makefile +++ b/arch/parisc/Makefile @@ -42,12 +42,10 @@ endif export LD_BFD ifneq ($(SUBARCH),$(UTS_MACHINE)) - ifeq ($(CROSS_COMPILE),) - CC_SUFFIXES = linux linux-gnu unknown-linux-gnu - CROSS_COMPILE := $(call cc-cross-prefix, \ + CC_SUFFIXES = linux linux-gnu unknown-linux-gnu + CROSS_COMPILE ?= $(call cc-cross-prefix, \ $(foreach a,$(CC_ARCHES), \ $(foreach s,$(CC_SUFFIXES),$(a)-$(s)-))) - endif endif ifdef CONFIG_DYNAMIC_FTRACE diff --git a/arch/sh/Makefile b/arch/sh/Makefile index 3bcbf52fb30e..0e8277be362e 100644 --- a/arch/sh/Makefile +++ b/arch/sh/Makefile @@ -10,9 +10,7 @@ # for more details. # ifneq ($(SUBARCH),$(ARCH)) - ifeq ($(CROSS_COMPILE),) -CROSS_COMPILE := $(call cc-cross-prefix, sh-linux- sh-linux-gnu- sh-unknown-linux-gnu-) - endif + CROSS_COMPILE ?= $(call cc-cross-prefix, sh-linux- sh-linux-gnu- sh-unknown-linux-gnu-) endif KBUILD_DEFCONFIG := shx3_defconfig -- 2.27.0
Re: [PATCH 1/2] ia64: syscalls: switch to generic syscalltbl.sh
On Mon, Mar 1, 2021 at 11:20 PM Masahiro Yamada wrote:
>
> Many architectures duplicate similar shell scripts.
>
> This commit converts ia64 to use scripts/syscalltbl.sh.
>
> Signed-off-by: Masahiro Yamada
> ---
Applied to linux-kbuild.
>
> arch/ia64/kernel/entry.S| 3 +--
> arch/ia64/kernel/syscalls/Makefile | 8 ++-
> arch/ia64/kernel/syscalls/syscalltbl.sh | 32 -
> 3 files changed, 3 insertions(+), 40 deletions(-)
> delete mode 100644 arch/ia64/kernel/syscalls/syscalltbl.sh
>
> diff --git a/arch/ia64/kernel/entry.S b/arch/ia64/kernel/entry.S
> index e98e3dafffd8..5eba3fb2e311 100644
> --- a/arch/ia64/kernel/entry.S
> +++ b/arch/ia64/kernel/entry.S
> @@ -1420,10 +1420,9 @@ END(ftrace_stub)
>
> #endif /* CONFIG_FUNCTION_TRACER */
>
> -#define __SYSCALL(nr, entry, nargs) data8 entry
> +#define __SYSCALL(nr, entry) data8 entry
> .rodata
> .align 8
> .globl sys_call_table
> sys_call_table:
> #include
> -#undef __SYSCALL
> diff --git a/arch/ia64/kernel/syscalls/Makefile
> b/arch/ia64/kernel/syscalls/Makefile
> index bf4bda0f63eb..2d2e420749b0 100644
> --- a/arch/ia64/kernel/syscalls/Makefile
> +++ b/arch/ia64/kernel/syscalls/Makefile
> @@ -7,7 +7,7 @@ _dummy := $(shell [ -d '$(uapi)' ] || mkdir -p '$(uapi)')
> \
>
> syscall := $(src)/syscall.tbl
> syshdr := $(srctree)/$(src)/syscallhdr.sh
> -systbl := $(srctree)/$(src)/syscalltbl.sh
> +systbl := $(srctree)/scripts/syscalltbl.sh
>
> quiet_cmd_syshdr = SYSHDR $@
>cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \
> @@ -16,16 +16,12 @@ quiet_cmd_syshdr = SYSHDR $@
>'$(syshdr_offset_$(basetarget))'
>
> quiet_cmd_systbl = SYSTBL $@
> - cmd_systbl = $(CONFIG_SHELL) '$(systbl)' '$<' '$@' \
> - '$(systbl_abis_$(basetarget))' \
> - '$(systbl_abi_$(basetarget))'\
> - '$(systbl_offset_$(basetarget))'
> + cmd_systbl = $(CONFIG_SHELL) $(systbl) $< $@
>
> syshdr_offset_unistd_64 := __NR_Linux
> $(uapi)/unistd_64.h: $(syscall) $(syshdr) FORCE
> $(call if_changed,syshdr)
>
> -systbl_offset_syscall_table := 1024
> $(kapi)/syscall_table.h: $(syscall) $(systbl) FORCE
> $(call if_changed,systbl)
>
> diff --git a/arch/ia64/kernel/syscalls/syscalltbl.sh
> b/arch/ia64/kernel/syscalls/syscalltbl.sh
> deleted file mode 100644
> index 85d78d9309ad..
> --- a/arch/ia64/kernel/syscalls/syscalltbl.sh
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -in="$1"
> -out="$2"
> -my_abis=`echo "($3)" | tr ',' '|'`
> -my_abi="$4"
> -offset="$5"
> -
> -emit() {
> - t_nxt="$1"
> - t_nr="$2"
> - t_entry="$3"
> -
> - while [ $t_nxt -lt $t_nr ]; do
> - printf "__SYSCALL(%s, sys_ni_syscall, )\n" "${t_nxt}"
> - t_nxt=$((t_nxt+1))
> - done
> - printf "__SYSCALL(%s, %s, )\n" "${t_nxt}" "${t_entry}"
> -}
> -
> -grep -E "^[0-9A-Fa-fXx]+[[:space:]]+${my_abis}" "$in" | sort -n | (
> - nxt=0
> - if [ -z "$offset" ]; then
> - offset=0
> - fi
> -
> - while read nr abi name entry ; do
> - emit $((nxt+offset)) $((nr+offset)) $entry
> - nxt=$((nr+1))
> - done
> -) > "$out"
> --
> 2.27.0
>
--
Best Regards
Masahiro Yamada
Re: [PATCH 1/2] alpha: syscalls: switch to generic syscalltbl.sh
On Mon, Mar 1, 2021 at 11:18 PM Masahiro Yamada wrote:
>
> Many architectures duplicate similar shell scripts.
>
> This commit converts alpha to use scripts/syscalltbl.sh.
>
> Signed-off-by: Masahiro Yamada
Applied to linux-kbuild.
> ---
>
> arch/alpha/kernel/syscalls/Makefile | 7 ++
> arch/alpha/kernel/syscalls/syscalltbl.sh | 32
> arch/alpha/kernel/systbls.S | 3 +--
> 3 files changed, 3 insertions(+), 39 deletions(-)
> delete mode 100644 arch/alpha/kernel/syscalls/syscalltbl.sh
>
> diff --git a/arch/alpha/kernel/syscalls/Makefile
> b/arch/alpha/kernel/syscalls/Makefile
> index 285aaba832d9..ad2492cb5568 100644
> --- a/arch/alpha/kernel/syscalls/Makefile
> +++ b/arch/alpha/kernel/syscalls/Makefile
> @@ -7,7 +7,7 @@ _dummy := $(shell [ -d '$(uapi)' ] || mkdir -p '$(uapi)')
> \
>
> syscall := $(src)/syscall.tbl
> syshdr := $(srctree)/$(src)/syscallhdr.sh
> -systbl := $(srctree)/$(src)/syscalltbl.sh
> +systbl := $(srctree)/scripts/syscalltbl.sh
>
> quiet_cmd_syshdr = SYSHDR $@
>cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \
> @@ -16,10 +16,7 @@ quiet_cmd_syshdr = SYSHDR $@
>'$(syshdr_offset_$(basetarget))'
>
> quiet_cmd_systbl = SYSTBL $@
> - cmd_systbl = $(CONFIG_SHELL) '$(systbl)' '$<' '$@' \
> - '$(systbl_abis_$(basetarget))' \
> - '$(systbl_abi_$(basetarget))'\
> - '$(systbl_offset_$(basetarget))'
> + cmd_systbl = $(CONFIG_SHELL) $(systbl) $< $@
>
> $(uapi)/unistd_32.h: $(syscall) $(syshdr) FORCE
> $(call if_changed,syshdr)
> diff --git a/arch/alpha/kernel/syscalls/syscalltbl.sh
> b/arch/alpha/kernel/syscalls/syscalltbl.sh
> deleted file mode 100644
> index 85d78d9309ad..
> --- a/arch/alpha/kernel/syscalls/syscalltbl.sh
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -in="$1"
> -out="$2"
> -my_abis=`echo "($3)" | tr ',' '|'`
> -my_abi="$4"
> -offset="$5"
> -
> -emit() {
> - t_nxt="$1"
> - t_nr="$2"
> - t_entry="$3"
> -
> - while [ $t_nxt -lt $t_nr ]; do
> - printf "__SYSCALL(%s, sys_ni_syscall, )\n" "${t_nxt}"
> - t_nxt=$((t_nxt+1))
> - done
> - printf "__SYSCALL(%s, %s, )\n" "${t_nxt}" "${t_entry}"
> -}
> -
> -grep -E "^[0-9A-Fa-fXx]+[[:space:]]+${my_abis}" "$in" | sort -n | (
> - nxt=0
> - if [ -z "$offset" ]; then
> - offset=0
> - fi
> -
> - while read nr abi name entry ; do
> - emit $((nxt+offset)) $((nr+offset)) $entry
> - nxt=$((nr+1))
> - done
> -) > "$out"
> diff --git a/arch/alpha/kernel/systbls.S b/arch/alpha/kernel/systbls.S
> index 9704f22ed5e3..68f3e4f329eb 100644
> --- a/arch/alpha/kernel/systbls.S
> +++ b/arch/alpha/kernel/systbls.S
> @@ -7,10 +7,9 @@
>
> #include
>
> -#define __SYSCALL(nr, entry, nargs) .quad entry
> +#define __SYSCALL(nr, entry) .quad entry
> .data
> .align 3
> .globl sys_call_table
> sys_call_table:
> #include
> -#undef __SYSCALL
> --
> 2.27.0
>
--
Best Regards
Masahiro Yamada
Re: [PATCH] sysctl: use min() helper for namecmp()
On Tue, Mar 2, 2021 at 8:47 AM Kees Cook wrote:
>
> On Sun, Feb 28, 2021 at 04:44:22PM +0900, Masahiro Yamada wrote:
> > (CC: Andrew Morton)
> >
> > A friendly reminder.
> >
> >
> > This is just a minor clean-up.
> >
> > If nobody picks it up,
> > I hope perhaps Andrew Morton will do.
> >
> > This patch:
> > https://lore.kernel.org/patchwork/patch/1360092/
> >
> >
> >
> >
> >
> > On Mon, Jan 4, 2021 at 5:33 PM Masahiro Yamada wrote:
> > >
> > > Make it slightly readable by using min().
> > >
> > > Signed-off-by: Masahiro Yamada
>
> Acked-by: Kees Cook
>
> Feel free to take this via your tree Masahiro. Thanks!
>
> -Kees
>
> > > ---
> > >
> > > fs/proc/proc_sysctl.c | 7 +--
> > > 1 file changed, 1 insertion(+), 6 deletions(-)
> > >
> > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > > index 317899222d7f..86341c0f0c40 100644
> > > --- a/fs/proc/proc_sysctl.c
> > > +++ b/fs/proc/proc_sysctl.c
> > > @@ -94,14 +94,9 @@ static void sysctl_print_dir(struct ctl_dir *dir)
> > >
> > > static int namecmp(const char *name1, int len1, const char *name2, int
> > > len2)
> > > {
> > > - int minlen;
> > > int cmp;
> > >
> > > - minlen = len1;
> > > - if (minlen > len2)
> > > - minlen = len2;
> > > -
> > > - cmp = memcmp(name1, name2, minlen);
> > > + cmp = memcmp(name1, name2, min(len1, len2));
> > > if (cmp == 0)
> > > cmp = len1 - len2;
> > > return cmp;
> > > --
> > > 2.27.0
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
>
> --
> Kees Cook
>
> Reviewed-by: Kees Cook
>
> --
> Kees Cook
Applied to linux-kbuild.
--
Best Regards
Masahiro Yamada
Re: [PATCH 1/2] sparc: syscalls: switch to generic syscalltbl.sh
On Mon, Mar 1, 2021 at 11:51 PM Masahiro Yamada wrote:
>
> Many architectures duplicate similar shell scripts.
>
> This commit converts sparc to use scripts/syscalltbl.sh. This also
> unifies syscall_table_64.h and syscall_table_c32.h.
>
> Signed-off-by: Masahiro Yamada
Could you check this series, please?
Thanks.
Masahiro
> ---
>
> arch/sparc/include/asm/Kbuild| 1 -
> arch/sparc/kernel/syscalls/Makefile | 19 -
> arch/sparc/kernel/syscalls/syscalltbl.sh | 36
> arch/sparc/kernel/systbls_32.S | 4 +--
> arch/sparc/kernel/systbls_64.S | 8 --
> 5 files changed, 12 insertions(+), 56 deletions(-)
> delete mode 100644 arch/sparc/kernel/syscalls/syscalltbl.sh
>
> diff --git a/arch/sparc/include/asm/Kbuild b/arch/sparc/include/asm/Kbuild
> index aec20406145e..0b9d98ced34a 100644
> --- a/arch/sparc/include/asm/Kbuild
> +++ b/arch/sparc/include/asm/Kbuild
> @@ -1,7 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> generated-y += syscall_table_32.h
> generated-y += syscall_table_64.h
> -generated-y += syscall_table_c32.h
> generic-y += export.h
> generic-y += kvm_para.h
> generic-y += mcs_spinlock.h
> diff --git a/arch/sparc/kernel/syscalls/Makefile
> b/arch/sparc/kernel/syscalls/Makefile
> index 283f64407b07..11424f1c8d9e 100644
> --- a/arch/sparc/kernel/syscalls/Makefile
> +++ b/arch/sparc/kernel/syscalls/Makefile
> @@ -7,7 +7,7 @@ _dummy := $(shell [ -d '$(uapi)' ] || mkdir -p '$(uapi)')
> \
>
> syscall := $(src)/syscall.tbl
> syshdr := $(srctree)/$(src)/syscallhdr.sh
> -systbl := $(srctree)/$(src)/syscalltbl.sh
> +systbl := $(srctree)/scripts/syscalltbl.sh
>
> quiet_cmd_syshdr = SYSHDR $@
>cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \
> @@ -16,10 +16,7 @@ quiet_cmd_syshdr = SYSHDR $@
>'$(syshdr_offset_$(basetarget))'
>
> quiet_cmd_systbl = SYSTBL $@
> - cmd_systbl = $(CONFIG_SHELL) '$(systbl)' '$<' '$@' \
> - '$(systbl_abis_$(basetarget))' \
> - '$(systbl_abi_$(basetarget))'\
> - '$(systbl_offset_$(basetarget))'
> + cmd_systbl = $(CONFIG_SHELL) $(systbl) --abis $(abis) $< $@
>
> syshdr_abis_unistd_32 := common,32
> $(uapi)/unistd_32.h: $(syscall) $(syshdr) FORCE
> @@ -29,23 +26,17 @@ syshdr_abis_unistd_64 := common,64
> $(uapi)/unistd_64.h: $(syscall) $(syshdr) FORCE
> $(call if_changed,syshdr)
>
> -systbl_abis_syscall_table_32 := common,32
> +$(kapi)/syscall_table_32.h: abis := common,32
> $(kapi)/syscall_table_32.h: $(syscall) $(systbl) FORCE
> $(call if_changed,systbl)
>
> -systbl_abis_syscall_table_64 := common,64
> +$(kapi)/syscall_table_64.h: abis := common,64
> $(kapi)/syscall_table_64.h: $(syscall) $(systbl) FORCE
> $(call if_changed,systbl)
>
> -systbl_abis_syscall_table_c32 := common,32
> -systbl_abi_syscall_table_c32 := c32
> -$(kapi)/syscall_table_c32.h: $(syscall) $(systbl) FORCE
> - $(call if_changed,systbl)
> -
> uapisyshdr-y += unistd_32.h unistd_64.h
> kapisyshdr-y += syscall_table_32.h \
> - syscall_table_64.h \
> - syscall_table_c32.h
> + syscall_table_64.h
>
> uapisyshdr-y := $(addprefix $(uapi)/, $(uapisyshdr-y))
> kapisyshdr-y := $(addprefix $(kapi)/, $(kapisyshdr-y))
> diff --git a/arch/sparc/kernel/syscalls/syscalltbl.sh
> b/arch/sparc/kernel/syscalls/syscalltbl.sh
> deleted file mode 100644
> index 77cf0143ba19..
> --- a/arch/sparc/kernel/syscalls/syscalltbl.sh
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -in="$1"
> -out="$2"
> -my_abis=`echo "($3)" | tr ',' '|'`
> -my_abi="$4"
> -offset="$5"
> -
> -emit() {
> - t_nxt="$1"
> - t_nr="$2"
> - t_entry="$3"
> -
> - while [ $t_nxt -lt $t_nr ]; do
> - printf "__SYSCALL(%s, sys_nis_syscall, )\n" "${t_nxt}"
> - t_nxt=$((t_nxt+1))
> - done
> - printf "__SYSCALL(%s, %s, )\n" "${t_nxt}" "${t_entry}"
> -}
> -
> -grep -E "^[0-9A-Fa-fXx]+[[:space:]]+${my_abis}" "$in" | sort -n | (
> - nxt=0
> - if [ -z "$offset" ]; then
> - offset=0
> - fi
> -
> - while read nr abi name entry compat ; do
> - if [ "$my_abi" = "c32" ] && [ !
Re: [PATCH 1/2] powerpc: syscalls: switch to generic syscalltbl.sh
Hi Michael,
On Tue, Mar 2, 2021 at 12:31 AM Masahiro Yamada wrote:
>
> Many architectures duplicate similar shell scripts.
>
> This commit converts powerpc to use scripts/syscalltbl.sh. This also
> unifies syscall_table_32.h and syscall_table_c32.h.
>
> Signed-off-by: Masahiro Yamada
Could you check this series?
Thanks.
Masahiro
> ---
>
> arch/powerpc/include/asm/Kbuild | 1 -
> arch/powerpc/kernel/syscalls/Makefile | 22 +++--
> arch/powerpc/kernel/syscalls/syscalltbl.sh | 36 -
> arch/powerpc/kernel/systbl.S| 5 ++-
> arch/powerpc/platforms/cell/spu_callbacks.c | 2 +-
> 5 files changed, 10 insertions(+), 56 deletions(-)
> delete mode 100644 arch/powerpc/kernel/syscalls/syscalltbl.sh
>
> diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
> index e1f9b4ea1c53..bcf95ce0964f 100644
> --- a/arch/powerpc/include/asm/Kbuild
> +++ b/arch/powerpc/include/asm/Kbuild
> @@ -1,7 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> generated-y += syscall_table_32.h
> generated-y += syscall_table_64.h
> -generated-y += syscall_table_c32.h
> generated-y += syscall_table_spu.h
> generic-y += export.h
> generic-y += kvm_types.h
> diff --git a/arch/powerpc/kernel/syscalls/Makefile
> b/arch/powerpc/kernel/syscalls/Makefile
> index 9e3be295dbba..df21c731c806 100644
> --- a/arch/powerpc/kernel/syscalls/Makefile
> +++ b/arch/powerpc/kernel/syscalls/Makefile
> @@ -7,7 +7,7 @@ _dummy := $(shell [ -d '$(uapi)' ] || mkdir -p '$(uapi)')
> \
>
> syscall := $(src)/syscall.tbl
> syshdr := $(srctree)/$(src)/syscallhdr.sh
> -systbl := $(srctree)/$(src)/syscalltbl.sh
> +systbl := $(srctree)/scripts/syscalltbl.sh
>
> quiet_cmd_syshdr = SYSHDR $@
>cmd_syshdr = $(CONFIG_SHELL) '$(syshdr)' '$<' '$@' \
> @@ -16,10 +16,7 @@ quiet_cmd_syshdr = SYSHDR $@
>'$(syshdr_offset_$(basetarget))'
>
> quiet_cmd_systbl = SYSTBL $@
> - cmd_systbl = $(CONFIG_SHELL) '$(systbl)' '$<' '$@' \
> - '$(systbl_abis_$(basetarget))' \
> - '$(systbl_abi_$(basetarget))'\
> - '$(systbl_offset_$(basetarget))'
> + cmd_systbl = $(CONFIG_SHELL) $(systbl) --abis $(abis) $< $@
>
> syshdr_abis_unistd_32 := common,nospu,32
> $(uapi)/unistd_32.h: $(syscall) $(syshdr) FORCE
> @@ -29,30 +26,21 @@ syshdr_abis_unistd_64 := common,nospu,64
> $(uapi)/unistd_64.h: $(syscall) $(syshdr) FORCE
> $(call if_changed,syshdr)
>
> -systbl_abis_syscall_table_32 := common,nospu,32
> -systbl_abi_syscall_table_32 := 32
> +$(kapi)/syscall_table_32.h: abis := common,nospu,32
> $(kapi)/syscall_table_32.h: $(syscall) $(systbl) FORCE
> $(call if_changed,systbl)
>
> -systbl_abis_syscall_table_64 := common,nospu,64
> -systbl_abi_syscall_table_64 := 64
> +$(kapi)/syscall_table_64.h: abis := common,nospu,64
> $(kapi)/syscall_table_64.h: $(syscall) $(systbl) FORCE
> $(call if_changed,systbl)
>
> -systbl_abis_syscall_table_c32 := common,nospu,32
> -systbl_abi_syscall_table_c32 := c32
> -$(kapi)/syscall_table_c32.h: $(syscall) $(systbl) FORCE
> - $(call if_changed,systbl)
> -
> -systbl_abis_syscall_table_spu := common,spu
> -systbl_abi_syscall_table_spu := spu
> +$(kapi)/syscall_table_spu.h: abis := common,spu
> $(kapi)/syscall_table_spu.h: $(syscall) $(systbl) FORCE
> $(call if_changed,systbl)
>
> uapisyshdr-y += unistd_32.h unistd_64.h
> kapisyshdr-y += syscall_table_32.h \
>syscall_table_64.h \
> - syscall_table_c32.h \
>syscall_table_spu.h
>
> uapisyshdr-y := $(addprefix $(uapi)/, $(uapisyshdr-y))
> diff --git a/arch/powerpc/kernel/syscalls/syscalltbl.sh
> b/arch/powerpc/kernel/syscalls/syscalltbl.sh
> deleted file mode 100644
> index f7393a7b18aa..
> --- a/arch/powerpc/kernel/syscalls/syscalltbl.sh
> +++ /dev/null
> @@ -1,36 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -in="$1"
> -out="$2"
> -my_abis=`echo "($3)" | tr ',' '|'`
> -my_abi="$4"
> -offset="$5"
> -
> -emit() {
> - t_nxt="$1"
> - t_nr="$2"
> - t_entry="$3"
> -
> - while [ $t_nxt -lt $t_nr ]; do
> - printf "__SYSCALL(%s,sys_ni_syscall)\n" "${t_nxt}"
> - t_nxt=$((t_nxt+1))
> - done
> - printf "__SYSCALL(%s,%s)\n" "${t_nxt}" "${t_entry}"
> -}
> -
Re: [PATCH 2/2] MAINTAINERS: add pattern for dummy-tools.
On Fri, Apr 9, 2021 at 6:31 AM Michal Suchanek wrote: > > scripts/get_maintainer.pl does not find a maintainer for new files > otherwise. > > Signed-off-by: Michal Suchanek > --- Applied to linux-kbuild. Thanks. > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index c80ad735b384..ce631ec44e1a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9690,6 +9690,7 @@ F:scripts/*vmlinux* > F: scripts/Kbuild* > F: scripts/Makefile* > F: scripts/basic/ > +F: scripts/dummy-tools/ > F: scripts/mk* > F: scripts/mod/ > F: scripts/package/ > -- > 2.26.2 > -- Best Regards Masahiro Yamada
Re: [PATCH 1/2] kbuild: dummy-tools: Add elfedit.
On Sun, Apr 11, 2021 at 7:18 PM Michal Suchánek wrote: > > On Sun, Apr 11, 2021 at 03:12:40AM +0900, Masahiro Yamada wrote: > > On Fri, Apr 9, 2021 at 6:31 AM Michal Suchanek wrote: > > > > > > elfedit is used in Makefile > > > > > > Makefile:GCC_TOOLCHAIN_DIR := $(dir $(shell which > > > $(CROSS_COMPILE)elfedit)) > > > > > > which causes this error getting printed > > > > > > which: no elfedit in (./scripts/dummy-tools) > > > > > > I am OK with this patch, but how did you reproduce it? > > make ARCH=arm CROSS_COMPILE=scripts/dummy-tools/ allmodconfig > > it possibly depends on the config you already have, too. > > Thanks > > Michal Maybey, are you working on linux-next? [1] $ git checkout add74f8473^ $ make ARCH=arm CROSS_COMPILE=scripts/dummy-tools/ allmodconfig [2] $ git checkout add74f8473 $ make ARCH=arm CROSS_COMPILE=scripts/dummy-tools/ allmodconfig If [1] is OK, but [2] is NG, commit add74f8473 is the root cause. At least, I do not think this would happen in the mainline kernel. -- Best Regards Masahiro Yamada
[PATCH 4/5] kconfig: nconf: remove unneeded default for menu prompt
The rootmenu always has a prompt even if the 'mainmenu' statement is
missing in the top Kconfig file.
conf_parse() calls menu_add_prompt(P_MENU, "Main menu", NULL) in this
case.
So, every 'menu' has a prompt.
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/nconf.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index daeeb5f81445..b6fe7b18a103 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -1068,7 +1068,6 @@ static int do_match(int key, struct match_state *state,
int *ans)
static void conf(struct menu *menu)
{
struct menu *submenu = NULL;
- const char *prompt = menu_get_prompt(menu);
struct symbol *sym;
int res;
int current_index = 0;
@@ -1086,9 +1085,8 @@ static void conf(struct menu *menu)
if (!child_count)
break;
- show_menu(prompt ? prompt : "Main Menu",
- menu_instructions,
- current_index, _top_row);
+ show_menu(menu_get_prompt(menu), menu_instructions,
+ current_index, _top_row);
keypad((menu_win(curses_menu)), TRUE);
while (!global_exit) {
if (match_state.in_search) {
--
2.27.0
[PATCH 3/5] kconfig: nconf: get rid of (void) casts from wattrset() calls
This reverts commit 10175ba65fde ("nconfig: Silence unused return values
from wattrset").
With recent GCC versions used, I no longer see "value computed is not
used" warnings even without the (void) casts.
The wattrset() used to be a macro like this:
#define wattrset(win,at) \
(NCURSES_OK_ADDR(win) \
? ((win)->_attrs = NCURSES_CAST(attr_t, at), \
OK) \
: ERR)
The GCC bugzilla [1] reported a false-positive -Wunused-value warning
in a similar test case. It was fixed by GCC 4.4.1.
Let's revert that commit, and see if somebody will claim the issue.
[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39889
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/nconf.c | 14 +++---
scripts/kconfig/nconf.gui.c | 20 ++--
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 232eb3011efe..daeeb5f81445 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -370,18 +370,18 @@ static void print_function_line(void)
int lines = getmaxy(stdscr);
for (i = 0; i < function_keys_num; i++) {
- (void) wattrset(main_window, attributes[FUNCTION_HIGHLIGHT]);
+ wattrset(main_window, attributes[FUNCTION_HIGHLIGHT]);
mvwprintw(main_window, lines-3, offset,
"%s",
function_keys[i].key_str);
- (void) wattrset(main_window, attributes[FUNCTION_TEXT]);
+ wattrset(main_window, attributes[FUNCTION_TEXT]);
offset += strlen(function_keys[i].key_str);
mvwprintw(main_window, lines-3,
offset, "%s",
function_keys[i].func);
offset += strlen(function_keys[i].func) + skip;
}
- (void) wattrset(main_window, attributes[NORMAL]);
+ wattrset(main_window, attributes[NORMAL]);
}
/* help */
@@ -956,16 +956,16 @@ static void show_menu(const char *prompt, const char
*instructions,
current_instructions = instructions;
clear();
- (void) wattrset(main_window, attributes[NORMAL]);
+ wattrset(main_window, attributes[NORMAL]);
print_in_middle(stdscr, 1, 0, getmaxx(stdscr),
menu_backtitle,
attributes[MAIN_HEADING]);
- (void) wattrset(main_window, attributes[MAIN_MENU_BOX]);
+ wattrset(main_window, attributes[MAIN_MENU_BOX]);
box(main_window, 0, 0);
- (void) wattrset(main_window, attributes[MAIN_MENU_HEADING]);
+ wattrset(main_window, attributes[MAIN_MENU_HEADING]);
mvwprintw(main_window, 0, 3, " %s ", prompt);
- (void) wattrset(main_window, attributes[NORMAL]);
+ wattrset(main_window, attributes[NORMAL]);
set_menu_items(curses_menu, curses_menu_items);
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index a914f81092d7..180d3158d380 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -167,7 +167,7 @@ void print_in_middle(WINDOW *win,
length = strlen(string);
temp = (width - length) / 2;
x = startx + (int)temp;
- (void) wattrset(win, color);
+ wattrset(win, color);
mvwprintw(win, y, x, "%s", string);
refresh();
}
@@ -297,11 +297,11 @@ int btn_dialog(WINDOW *main_window, const char *msg, int
btn_num, ...)
set_menu_fore(menu, attributes[DIALOG_MENU_FORE]);
set_menu_back(menu, attributes[DIALOG_MENU_BACK]);
- (void) wattrset(win, attributes[DIALOG_BOX]);
+ wattrset(win, attributes[DIALOG_BOX]);
box(win, 0, 0);
/* print message */
- (void) wattrset(msg_win, attributes[DIALOG_TEXT]);
+ wattrset(msg_win, attributes[DIALOG_TEXT]);
fill_window(msg_win, msg);
set_menu_win(menu, win);
@@ -405,16 +405,16 @@ int dialog_inputbox(WINDOW *main_window,
form_win = derwin(win, 1, prompt_width, prompt_lines+3, 2);
keypad(form_win, TRUE);
- (void) wattrset(form_win, attributes[INPUT_FIELD]);
+ wattrset(form_win, attributes[INPUT_FIELD]);
- (void) wattrset(win, attributes[INPUT_BOX]);
+ wattrset(win, attributes[INPUT_BOX]);
box(win, 0, 0);
- (void) wattrset(win, attributes[INPUT_HEADING]);
+ wattrset(win, attributes[INPUT_HEADING]);
if (title)
mvwprintw(win, 0, 3, "%s", title);
/* print message */
- (void) wattrset(prompt_win, attributes[INPUT_TEXT]);
+ wattrset(prompt_win, attributes[INPUT_TEXT]);
fill_window(prompt_win, prompt);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
@@ -576,7 +576,7 @@ void show_scroll_win(WINDOW *main_window,
/* create the pad */
pad
[PATCH 5/5] kconfig: nconf: refactor attributes setup code
The current attributes setup code is strange; the array attribute[]
is set to values outside the range of the attribute_t enum.
At least,
attributes_t attributes[ATTR_MAX+1] = {0};
... should be
int attribute[ATTR_MAX+1] = {0};
Also, there is no need to hard-code the color-pair numbers in
attributes_t.
The current code is horribly screwed up. Rewrite it.
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/nconf.c | 22 ++--
scripts/kconfig/nconf.gui.c | 253
scripts/kconfig/nconf.h | 44 +++
3 files changed, 144 insertions(+), 175 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index b6fe7b18a103..c47051598e9a 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -370,18 +370,18 @@ static void print_function_line(void)
int lines = getmaxy(stdscr);
for (i = 0; i < function_keys_num; i++) {
- wattrset(main_window, attributes[FUNCTION_HIGHLIGHT]);
+ wattrset(main_window, attr_function_highlight);
mvwprintw(main_window, lines-3, offset,
"%s",
function_keys[i].key_str);
- wattrset(main_window, attributes[FUNCTION_TEXT]);
+ wattrset(main_window, attr_function_text);
offset += strlen(function_keys[i].key_str);
mvwprintw(main_window, lines-3,
offset, "%s",
function_keys[i].func);
offset += strlen(function_keys[i].func) + skip;
}
- wattrset(main_window, attributes[NORMAL]);
+ wattrset(main_window, attr_normal);
}
/* help */
@@ -956,16 +956,16 @@ static void show_menu(const char *prompt, const char
*instructions,
current_instructions = instructions;
clear();
- wattrset(main_window, attributes[NORMAL]);
+ wattrset(main_window, attr_normal);
print_in_middle(stdscr, 1, 0, getmaxx(stdscr),
menu_backtitle,
- attributes[MAIN_HEADING]);
+ attr_main_heading);
- wattrset(main_window, attributes[MAIN_MENU_BOX]);
+ wattrset(main_window, attr_main_menu_box);
box(main_window, 0, 0);
- wattrset(main_window, attributes[MAIN_MENU_HEADING]);
+ wattrset(main_window, attr_main_menu_heading);
mvwprintw(main_window, 0, 3, " %s ", prompt);
- wattrset(main_window, attributes[NORMAL]);
+ wattrset(main_window, attr_normal);
set_menu_items(curses_menu, curses_menu_items);
@@ -1521,9 +1521,9 @@ int main(int ac, char **av)
menu_opts_on(curses_menu, O_NONCYCLIC);
menu_opts_on(curses_menu, O_IGNORECASE);
set_menu_mark(curses_menu, " ");
- set_menu_fore(curses_menu, attributes[MAIN_MENU_FORE]);
- set_menu_back(curses_menu, attributes[MAIN_MENU_BACK]);
- set_menu_grey(curses_menu, attributes[MAIN_MENU_GREY]);
+ set_menu_fore(curses_menu, attr_main_menu_fore);
+ set_menu_back(curses_menu, attr_main_menu_back);
+ set_menu_grey(curses_menu, attr_main_menu_grey);
set_config_filename(conf_get_configname());
setup_windows();
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 180d3158d380..e747590cee17 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -7,141 +7,114 @@
#include "nconf.h"
#include "lkc.h"
-/* a list of all the different widgets we use */
-attributes_t attributes[ATTR_MAX+1] = {0};
-
-/* available colors:
- COLOR_BLACK 0
- COLOR_RED 1
- COLOR_GREEN 2
- COLOR_YELLOW 3
- COLOR_BLUE4
- COLOR_MAGENTA 5
- COLOR_CYAN6
- COLOR_WHITE 7
- */
-static void set_normal_colors(void)
-{
- init_pair(NORMAL, -1, -1);
- init_pair(MAIN_HEADING, COLOR_MAGENTA, -1);
-
- /* FORE is for the selected item */
- init_pair(MAIN_MENU_FORE, -1, -1);
- /* BACK for all the rest */
- init_pair(MAIN_MENU_BACK, -1, -1);
- init_pair(MAIN_MENU_GREY, -1, -1);
- init_pair(MAIN_MENU_HEADING, COLOR_GREEN, -1);
- init_pair(MAIN_MENU_BOX, COLOR_YELLOW, -1);
-
- init_pair(SCROLLWIN_TEXT, -1, -1);
- init_pair(SCROLLWIN_HEADING, COLOR_GREEN, -1);
- init_pair(SCROLLWIN_BOX, COLOR_YELLOW, -1);
-
- init_pair(DIALOG_TEXT, -1, -1);
- init_pair(DIALOG_BOX, COLOR_YELLOW, -1);
- init_pair(DIALOG_MENU_BACK, COLOR_YELLOW, -1);
- init_pair(DIALOG_MENU_FORE, COLOR_RED, -1);
-
- init_pair(INPUT_BOX, COLOR_YELLOW, -1);
- init_pair(INPUT_HEADING, COLOR_GREEN, -1);
- init_pair(INPUT_TEXT, -1, -1);
- init_pair(INPUT_FIELD, -1, -1);
-
- init_pair(FUNCTION_HIGHLIGHT, -1, -1);
- init_pair(FUNCTION_TEXT, COLOR_YELLOW, -1);
-}
-
-/* available attributes:
- A_NORMALNormal di
[PATCH 2/5] kconfig: nconf: fix NORMAL attributes
The lower 8-bit of attributes should be 0, but this code wrongly
sets it to NORMAL (=1). The correct one is A_NORMAL.
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/nconf.gui.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 77f525a8617c..a914f81092d7 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -70,7 +70,7 @@ static void normal_color_theme(void)
/* automatically add color... */
#define mkattr(name, attr) do { \
attributes[name] = attr | COLOR_PAIR(name); } while (0)
- mkattr(NORMAL, NORMAL);
+ mkattr(NORMAL, A_NORMAL);
mkattr(MAIN_HEADING, A_BOLD | A_UNDERLINE);
mkattr(MAIN_MENU_FORE, A_REVERSE);
@@ -102,7 +102,7 @@ static void no_colors_theme(void)
/* automatically add highlight, no color */
#define mkattrn(name, attr) { attributes[name] = attr; }
- mkattrn(NORMAL, NORMAL);
+ mkattrn(NORMAL, A_NORMAL);
mkattrn(MAIN_HEADING, A_BOLD | A_UNDERLINE);
mkattrn(MAIN_MENU_FORE, A_STANDOUT);
--
2.27.0
[PATCH 1/5] kconfig: mconf,nconf: remove unneeded '\0' termination after snprintf()
snprintf() always terminates the destination buffer with '\0' even if
the buffer is not long enough. (In this case, the last element of the
buffer becomes '\0'.)
The explicit termination is unneeded.
Signed-off-by: Masahiro Yamada
---
scripts/kconfig/mconf.c | 11 +++
scripts/kconfig/nconf.c | 12 +++-
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 4cfbe62938cd..9d3cf510562f 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -299,17 +299,12 @@ static char filename[PATH_MAX+1];
static void set_config_filename(const char *config_filename)
{
static char menu_backtitle[PATH_MAX+128];
- int size;
- size = snprintf(menu_backtitle, sizeof(menu_backtitle),
- "%s - %s", config_filename, rootmenu.prompt->text);
- if (size >= sizeof(menu_backtitle))
- menu_backtitle[sizeof(menu_backtitle)-1] = '\0';
+ snprintf(menu_backtitle, sizeof(menu_backtitle), "%s - %s",
+config_filename, rootmenu.prompt->text);
set_dialog_backtitle(menu_backtitle);
- size = snprintf(filename, sizeof(filename), "%s", config_filename);
- if (size >= sizeof(filename))
- filename[sizeof(filename)-1] = '\0';
+ snprintf(filename, sizeof(filename), "%s", config_filename);
}
struct subtitle_part {
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 86ca42b5ab80..232eb3011efe 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -635,16 +635,10 @@ static char filename[PATH_MAX+1];
static char menu_backtitle[PATH_MAX+128];
static const char *set_config_filename(const char *config_filename)
{
- int size;
+ snprintf(menu_backtitle, sizeof(menu_backtitle), "%s - %s",
+config_filename, rootmenu.prompt->text);
- size = snprintf(menu_backtitle, sizeof(menu_backtitle),
- "%s - %s", config_filename, rootmenu.prompt->text);
- if (size >= sizeof(menu_backtitle))
- menu_backtitle[sizeof(menu_backtitle)-1] = '\0';
-
- size = snprintf(filename, sizeof(filename), "%s", config_filename);
- if (size >= sizeof(filename))
- filename[sizeof(filename)-1] = '\0';
+ snprintf(filename, sizeof(filename), "%s", config_filename);
return menu_backtitle;
}
--
2.27.0
Re: [PATCH 1/2] kbuild: dummy-tools: Add elfedit.
On Fri, Apr 9, 2021 at 6:31 AM Michal Suchanek wrote: > > elfedit is used in Makefile > > Makefile:GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit)) > > which causes this error getting printed > > which: no elfedit in (./scripts/dummy-tools) I am OK with this patch, but how did you reproduce it? > > Add elfedit to dummy-tools to avoid this error. > > Signed-off-by: Michal Suchanek > --- > scripts/dummy-tools/elfedit | 1 + > 1 file changed, 1 insertion(+) > create mode 12 scripts/dummy-tools/elfedit > > diff --git a/scripts/dummy-tools/elfedit b/scripts/dummy-tools/elfedit > new file mode 12 > index ..c0648b38dd42 > --- /dev/null > +++ b/scripts/dummy-tools/elfedit > @@ -0,0 +1 @@ > +ld > \ No newline at end of file > -- > 2.26.2 > -- Best Regards Masahiro Yamada
[PATCH] kconfig: use /boot/config-* etc. as DEFCONFIG_LIST only for native build
When the .config file is missing, 'make config', 'make menuconfig', etc. uses a file listed in DEFCONFIG_LIST as base configuration. Ususally, /boot/config-$(uname -r) exists, and is used as default. However, when you are cross-compiling the kernel, it does not make sense to use /boot/config-* from the build host. It should default to arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG). UML previously did not use DEFCONFIG_LIST at all, but it should be able to use arch/um/configs/$(KBUILD_DEFCONFIG) as a base config file. Signed-off-by: Masahiro Yamada --- Makefile | 5 + scripts/kconfig/Makefile | 8 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index f1093b972708..697eaf6c550e 100644 --- a/Makefile +++ b/Makefile @@ -393,6 +393,11 @@ ifeq ($(ARCH),sh64) SRCARCH := sh endif +export cross_compiling := +ifneq ($(SRCARCH),$(SUBARCH)) +cross_compiling := 1 +endif + KCONFIG_CONFIG ?= .config export KCONFIG_CONFIG diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index 46f2465177f0..1d1a7f83ee8d 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -18,14 +18,14 @@ silent := -s endif export KCONFIG_DEFCONFIG_LIST := -ifneq ($(SRCARCH),um) +ifndef cross_compiling kernel-release := $(shell uname -r) -KCONFIG_DEFCONFIG_LIST := \ +KCONFIG_DEFCONFIG_LIST += \ /lib/modules/$(kernel-release)/.config \ /etc/kernel-config \ - /boot/config-$(kernel-release) \ - arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) + /boot/config-$(kernel-release) endif +KCONFIG_DEFCONFIG_LIST += arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG) # We need this, in case the user has it in its environment unexport CONFIG_ -- 2.27.0
