| Message ID | 164847778869.3060675.8115416881394543419.stgit@devnote2 (mailing list archive) |
|---|---|
| Headers | show |
| Series | bootconfig: Support embedding a bootconfig in kernel for non initrd boot | expand |
On Mon, Mar 28, 2022 at 7:29 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > KNOWN ISSUE: > > According to the report from Padmanabha[3] and my analysis [4], the embedded > bootconfig data may not be updated if you do incremental build the kernel > with CONFIG_LTO_CLANG_THIN. > > [3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u > [4] https://lore.kernel.org/all/20220327115526.cc4b0ff55fc53c97683c3e4d@kernel.org/ > > This seems like clang's LTO Thin mode issue. It may not detect the inline > asm depends on external files. > > I think the possible workaround is to split the inline asm which includes > '.incbin' directive into an asm file. But this should be done in another > seires because there are other features which uses '.incbin'. (e.g. > /proc/config.gz) Hi Masami, I saw Padmanabha's report (thanks for the report); sorry for not responding sooner, I've been traveling recently for a funeral. Any chance we can use CFLAGS_REMOVE_<file>.o := $(CC_FLAGS_LTO) a la commit d2dcd3e37475 ("x86, cpu: disable LTO for cpu.c") with a comment linking to https://github.com/ClangBuiltLinux/linux/issues/1618 for the Translation Units using .incbin, until we have had more time to triage+fix?
Hi Nick, On Wed, 30 Mar 2022 11:04:50 -0700 Nick Desaulniers <ndesaulniers@google.com> wrote: > On Mon, Mar 28, 2022 at 7:29 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > KNOWN ISSUE: > > > > According to the report from Padmanabha[3] and my analysis [4], the embedded > > bootconfig data may not be updated if you do incremental build the kernel > > with CONFIG_LTO_CLANG_THIN. > > > > [3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u > > [4] https://lore.kernel.org/all/20220327115526.cc4b0ff55fc53c97683c3e4d@kernel.org/ > > > > This seems like clang's LTO Thin mode issue. It may not detect the inline > > asm depends on external files. > > > > I think the possible workaround is to split the inline asm which includes > > '.incbin' directive into an asm file. But this should be done in another > > seires because there are other features which uses '.incbin'. (e.g. > > /proc/config.gz) > > Hi Masami, > I saw Padmanabha's report (thanks for the report); sorry for not > responding sooner, I've been traveling recently for a funeral. Oh, sorry about that. I think this is not so hurry. > > Any chance we can use > > CFLAGS_REMOVE_<file>.o := $(CC_FLAGS_LTO) > > a la > commit d2dcd3e37475 ("x86, cpu: disable LTO for cpu.c") Hm, this looks good to me. Let me confirm that works. (Does this mean the bootconfig.o will be compiled to elf binary?) > > with a comment linking to > https://github.com/ClangBuiltLinux/linux/issues/1618 Thanks for reporting! > > for the Translation Units using .incbin, until we have had more time > to triage+fix? Yes. For this series, I can update with above one, but it doesn't cover the other parts. And since this issue is independent from the bootconfig, I think we need a wider patch series to mitigate this issue on config.gz (and other .incbin users) too. Thank you, > > -- > Thanks, > ~Nick Desaulniers
On Thu, Mar 31, 2022 at 10:45 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Hi Nick, > > On Wed, 30 Mar 2022 11:04:50 -0700 > Nick Desaulniers <ndesaulniers@google.com> wrote: > > > On Mon, Mar 28, 2022 at 7:29 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > > > KNOWN ISSUE: > > > > > > According to the report from Padmanabha[3] and my analysis [4], the embedded > > > bootconfig data may not be updated if you do incremental build the kernel > > > with CONFIG_LTO_CLANG_THIN. > > > > > > [3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u > > > [4] https://lore.kernel.org/all/20220327115526.cc4b0ff55fc53c97683c3e4d@kernel.org/ > > > > > > This seems like clang's LTO Thin mode issue. It may not detect the inline > > > asm depends on external files. > > > > > > I think the possible workaround is to split the inline asm which includes > > > '.incbin' directive into an asm file. But this should be done in another > > > seires because there are other features which uses '.incbin'. (e.g. > > > /proc/config.gz) > > > > Hi Masami, > > I saw Padmanabha's report (thanks for the report); sorry for not > > responding sooner, I've been traveling recently for a funeral. > > Oh, sorry about that. I think this is not so hurry. > > > > > Any chance we can use > > > > CFLAGS_REMOVE_<file>.o := $(CC_FLAGS_LTO) > > > > a la > > commit d2dcd3e37475 ("x86, cpu: disable LTO for cpu.c") > > Hm, this looks good to me. Let me confirm that works. > (Does this mean the bootconfig.o will be compiled to elf binary?) Why are you using ".incbin" in the C file in the first place? You might have mimicked kernel/configs.c but there was a reason; kernel/configs.c can be a module, but we cannot put MODULE_LICENSE() in *.S file. (commit 13610aa908dcfce77135bb799c0a10d0172da6ba) In this case, CONFIG_EMBED_BOOT_CONFIG is a bool option. Why don't you simply move the asm() part to a separate bootconfig-data.S ? Clang lto flags are only passed to *.c files, so we do not need to be worried about .incbin in *.S files. Then, Makefile will be even cleaner (no ifeq-block) lib-$(CONFIG_BOOT_CONFIG) += bootconfig.o obj-$(CONFIG_EMBED_BOOT_CONFIG) += bootconfig-data.o $(obj)/bootconfig-data.o: $(obj)/default.bconf targets += default.bconf filechk_defbconf = cat $(or $(real-prereqs), /dev/null) $(obj)/default.bconf: $(CONFIG_EMBED_BOOT_CONFIG_FILE) FORCE $(call filechk,defbconf) BTW, why lib-$(CONFIG_BOOT_CONFIG), instead of obj-$(CONFIG_BOOT_CONFIG) ? > > > > > with a comment linking to > > https://github.com/ClangBuiltLinux/linux/issues/1618 > > Thanks for reporting! > > > > > for the Translation Units using .incbin, until we have had more time > > to triage+fix? > > Yes. For this series, I can update with above one, but it doesn't cover the > other parts. And since this issue is independent from the bootconfig, > I think we need a wider patch series to mitigate this issue on config.gz > (and other .incbin users) too. > > Thank you, > > > > > -- > > Thanks, > > ~Nick Desaulniers > > > -- > Masami Hiramatsu <mhiramat@kernel.org> -- Best Regards Masahiro Yamada
On Wed, Mar 30, 2022 at 6:45 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Hi Nick, > > On Wed, 30 Mar 2022 11:04:50 -0700 > Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > > Any chance we can use > > > > CFLAGS_REMOVE_<file>.o := $(CC_FLAGS_LTO) > > > > a la > > commit d2dcd3e37475 ("x86, cpu: disable LTO for cpu.c") > > Hm, this looks good to me. Let me confirm that works. > (Does this mean the bootconfig.o will be compiled to elf binary?) I know we went with Masahiro's suggestion, which is clever and better, but to answer this question; yes, under LTO, the linker can link together inputs that are a mix of ELF object files (basically, no LTO optimizations) with LLVM IR (w/ LTO optimizations between such files).
Hi, Here are the 5th version of the patchset to enable kernel embedded bootconfig for non-initrd kernel boot environment. This version fixes to sort .gitignore and fixes lib/Makefile to cleanup default.bconf correctly and to allow user to specify a relative path to CONFIG_EMBED_BOOT_CONFIG_FILE. (Thanks Masahiro!) Also this update the document patch about the relative path. Here is the previous thread [1]. [1] https://lore.kernel.org/all/164833878595.2575750.1483106296151574233.stgit@devnote2/T/#u You can embed a bootconfig file into the kernel as a default bootconfig, which will be used if there is no initrd or no bootconfig is attached to initrd. This needs 2 options: CONFIG_EMBED_BOOT_CONFIG=y and set the file path to CONFIG_EMBED_BOOT_CONFIG_FILE. Even if you embed the bootconfig file to the kernel, it will not be enabled unless you pass "bootconfig" kernel command line option at boot. Moreover, since this is just a "default" bootconfig, you can override it with a new bootconfig if you attach another bootconfig to the initrd (if possible). CONFIG_EMBED_BOOT_CONFIG_FILE can take both absolute and relative path, but to simplify and make it independent from the build environment, I recommend you to use an absolute path for that. This is requested by Padmanabha at the below thread[2]; [2] https://lore.kernel.org/all/20220307184011.GA2570@pswork/T/#u KNOWN ISSUE: According to the report from Padmanabha[3] and my analysis [4], the embedded bootconfig data may not be updated if you do incremental build the kernel with CONFIG_LTO_CLANG_THIN. [3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u [4] https://lore.kernel.org/all/20220327115526.cc4b0ff55fc53c97683c3e4d@kernel.org/ This seems like clang's LTO Thin mode issue. It may not detect the inline asm depends on external files. I think the possible workaround is to split the inline asm which includes '.incbin' directive into an asm file. But this should be done in another seires because there are other features which uses '.incbin'. (e.g. /proc/config.gz) Thank you, --- Masami Hiramatsu (3): bootconfig: Check the checksum before removing the bootconfig from initrd bootconfig: Support embedding a bootconfig file in kernel docs: bootconfig: Add how to embed the bootconfig into kernel Documentation/admin-guide/bootconfig.rst | 31 +++++++++++++++++++++++++++--- include/linux/bootconfig.h | 10 ++++++++++ init/Kconfig | 21 ++++++++++++++++++++ init/main.c | 31 +++++++++++++++--------------- lib/.gitignore | 1 + lib/Makefile | 9 +++++++++ lib/bootconfig.c | 23 ++++++++++++++++++++++ 7 files changed, 108 insertions(+), 18 deletions(-) -- Masami Hiramatsu (Linaro) <mhiramat@kernel.org>