linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot
@ 2022-03-26 23:53 Masami Hiramatsu
  2022-03-26 23:53 ` [PATCH v4 1/3] bootconfig: Check the checksum before removing the bootconfig from initrd Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-26 23:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap, Nick Desaulniers, Sami Tolvanen,
	Nathan Chancellor, llvm, Masahiro Yamada,
	Linux Kbuild mailing list

Hi,

Here are the 4th version of the patchset to enable kernel embedded bootconfig
for non-initrd kernel boot environment. I've fixed Makefile to determine
update by checking the contents of embedding bootconfig file with filechk macro.
Thanks Masahiro! Here is the previous version [1] (sorry, I missed to update the
version tag in that series)

[1] https://lore.kernel.org/all/164724890153.731226.1478494969800777757.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], the embedded bootconfig data may not
be updated if you do incremental build the kernel with CONFIG_LTO. This is
under investigation.

[3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u

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 |   30 ++++++++++++++++++++++++++---
 include/linux/bootconfig.h               |   10 ++++++++++
 init/Kconfig                             |   21 ++++++++++++++++++++
 init/main.c                              |   31 +++++++++++++++---------------
 lib/.gitignore                           |    1 +
 lib/Makefile                             |   10 ++++++++++
 lib/bootconfig.c                         |   23 ++++++++++++++++++++++
 7 files changed, 108 insertions(+), 18 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/3] bootconfig: Check the checksum before removing the bootconfig from initrd
  2022-03-26 23:53 [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
@ 2022-03-26 23:53 ` Masami Hiramatsu
  2022-03-26 23:53 ` [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-26 23:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap, Nick Desaulniers, Sami Tolvanen,
	Nathan Chancellor, llvm, Masahiro Yamada,
	Linux Kbuild mailing list

Check the bootconfig's checksum before removing the bootcinfig data
from initrd to avoid modifying initrd by mistake.
This will also simplifies the get_boot_config_from_initrd() interface.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 init/main.c |   22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/init/main.c b/init/main.c
index 560f45c27ffe..4f3ba3b84e34 100644
--- a/init/main.c
+++ b/init/main.c
@@ -265,7 +265,7 @@ static int __init loglevel(char *str)
 early_param("loglevel", loglevel);
 
 #ifdef CONFIG_BLK_DEV_INITRD
-static void * __init get_boot_config_from_initrd(u32 *_size, u32 *_csum)
+static void * __init get_boot_config_from_initrd(u32 *_size)
 {
 	u32 size, csum;
 	char *data;
@@ -299,17 +299,20 @@ static void * __init get_boot_config_from_initrd(u32 *_size, u32 *_csum)
 		return NULL;
 	}
 
+	if (xbc_calc_checksum(data, size) != csum) {
+		pr_err("bootconfig checksum failed\n");
+		return NULL;
+	}
+
 	/* Remove bootconfig from initramfs/initrd */
 	initrd_end = (unsigned long)data;
 	if (_size)
 		*_size = size;
-	if (_csum)
-		*_csum = csum;
 
 	return data;
 }
 #else
-static void * __init get_boot_config_from_initrd(u32 *_size, u32 *_csum)
+static void * __init get_boot_config_from_initrd(u32 *_size)
 {
 	return NULL;
 }
@@ -408,12 +411,12 @@ static void __init setup_boot_config(void)
 	static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
 	const char *msg;
 	int pos;
-	u32 size, csum;
+	u32 size;
 	char *data, *err;
 	int ret;
 
 	/* Cut out the bootconfig data even if we have no bootconfig option */
-	data = get_boot_config_from_initrd(&size, &csum);
+	data = get_boot_config_from_initrd(&size);
 
 	strlcpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
 	err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
@@ -437,11 +440,6 @@ static void __init setup_boot_config(void)
 		return;
 	}
 
-	if (xbc_calc_checksum(data, size) != csum) {
-		pr_err("bootconfig checksum failed\n");
-		return;
-	}
-
 	ret = xbc_init(data, size, &msg, &pos);
 	if (ret < 0) {
 		if (pos < 0)
@@ -470,7 +468,7 @@ static void __init exit_boot_config(void)
 static void __init setup_boot_config(void)
 {
 	/* Remove bootconfig data from initrd */
-	get_boot_config_from_initrd(NULL, NULL);
+	get_boot_config_from_initrd(NULL);
 }
 
 static int __init warn_bootconfig(char *str)


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel
  2022-03-26 23:53 [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
  2022-03-26 23:53 ` [PATCH v4 1/3] bootconfig: Check the checksum before removing the bootconfig from initrd Masami Hiramatsu
@ 2022-03-26 23:53 ` Masami Hiramatsu
  2022-03-28  6:27   ` Masahiro Yamada
  2022-03-26 23:53 ` [PATCH v4 3/3] docs: bootconfig: Add how to embed the bootconfig into kernel Masami Hiramatsu
  2022-03-27  2:55 ` [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
  3 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-26 23:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap, Nick Desaulniers, Sami Tolvanen,
	Nathan Chancellor, llvm, Masahiro Yamada,
	Linux Kbuild mailing list

This allows kernel developer to embed a default bootconfig file in
the kernel instead of embedding it in the initrd. This will be good
for who are using the kernel without initrd, or who needs a default
bootconfigs.
This needs to set two kconfigs: CONFIG_EMBED_BOOT_CONFIG=y and set
the file path to CONFIG_EMBED_BOOT_CONFIG_FILE.

Note that you still need 'bootconfig' command line option to load the
embedded bootconfig. Also if you boot using an initrd with a different
bootconfig, the kernel will use the bootconfig in the initrd, instead
of the default bootconfig.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v3:
  - Avoid updating the default.bconf if the file is not changed.
---
 include/linux/bootconfig.h |   10 ++++++++++
 init/Kconfig               |   21 +++++++++++++++++++++
 init/main.c                |   13 ++++++++-----
 lib/.gitignore             |    1 +
 lib/Makefile               |   10 ++++++++++
 lib/bootconfig.c           |   23 +++++++++++++++++++++++
 6 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index a4665c7ab07c..5dbda5e3e9bb 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -289,4 +289,14 @@ int __init xbc_get_info(int *node_size, size_t *data_size);
 /* XBC cleanup data structures */
 void __init xbc_exit(void);
 
+/* XBC embedded bootconfig data in kernel */
+#ifdef CONFIG_EMBED_BOOT_CONFIG
+char * __init xbc_get_embedded_bootconfig(size_t *size);
+#else
+static inline char *xbc_get_embedded_bootconfig(size_t *size)
+{
+	return NULL;
+}
+#endif
+
 #endif
diff --git a/init/Kconfig b/init/Kconfig
index beb5b866c318..bff308a782f8 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1357,6 +1357,27 @@ config BOOT_CONFIG
 
 	  If unsure, say Y.
 
+config EMBED_BOOT_CONFIG
+	bool "Embed bootconfig file in the kernel"
+	depends on BOOT_CONFIG
+	default n
+	help
+	  Embed a bootconfig file given by EMBED_BOOT_CONFIG_FILE in the
+	  kernel. Usually, the bootconfig file is loaded with the initrd
+	  image. But if the system doesn't support initrd, this option will
+	  help you by embedding a bootconfig file while building the kernel.
+
+	  If unsure, say N.
+
+config EMBED_BOOT_CONFIG_FILE
+	string "Embedded bootconfig file path"
+	default ""
+	depends on EMBED_BOOT_CONFIG
+	help
+	  Specify a bootconfig file which will be embedded to the kernel.
+	  This bootconfig will be used if there is no initrd or no other
+	  bootconfig in the initrd.
+
 choice
 	prompt "Compiler optimization level"
 	default CC_OPTIMIZE_FOR_PERFORMANCE
diff --git a/init/main.c b/init/main.c
index 4f3ba3b84e34..180511324c95 100644
--- a/init/main.c
+++ b/init/main.c
@@ -265,7 +265,7 @@ static int __init loglevel(char *str)
 early_param("loglevel", loglevel);
 
 #ifdef CONFIG_BLK_DEV_INITRD
-static void * __init get_boot_config_from_initrd(u32 *_size)
+static void * __init get_boot_config_from_initrd(size_t *_size)
 {
 	u32 size, csum;
 	char *data;
@@ -411,12 +411,15 @@ static void __init setup_boot_config(void)
 	static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
 	const char *msg;
 	int pos;
-	u32 size;
+	size_t size;
 	char *data, *err;
 	int ret;
 
 	/* Cut out the bootconfig data even if we have no bootconfig option */
 	data = get_boot_config_from_initrd(&size);
+	/* If there is no bootconfig in initrd, try embedded one. */
+	if (!data)
+		data = xbc_get_embedded_bootconfig(&size);
 
 	strlcpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
 	err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
@@ -435,8 +438,8 @@ static void __init setup_boot_config(void)
 	}
 
 	if (size >= XBC_DATA_MAX) {
-		pr_err("bootconfig size %d greater than max size %d\n",
-			size, XBC_DATA_MAX);
+		pr_err("bootconfig size %ld greater than max size %d\n",
+			(long)size, XBC_DATA_MAX);
 		return;
 	}
 
@@ -449,7 +452,7 @@ static void __init setup_boot_config(void)
 				msg, pos);
 	} else {
 		xbc_get_info(&ret, NULL);
-		pr_info("Load bootconfig: %d bytes %d nodes\n", size, ret);
+		pr_info("Load bootconfig: %ld bytes %d nodes\n", (long)size, ret);
 		/* keys starting with "kernel." are passed via cmdline */
 		extra_command_line = xbc_make_cmdline("kernel");
 		/* Also, "init." keys are init arguments */
diff --git a/lib/.gitignore b/lib/.gitignore
index e5e217b8307b..30a2a5db7033 100644
--- a/lib/.gitignore
+++ b/lib/.gitignore
@@ -6,3 +6,4 @@
 /oid_registry_data.c
 /test_fortify.log
 /test_fortify/*.log
+/default.bconf
diff --git a/lib/Makefile b/lib/Makefile
index 353bc09ce38d..dd9f3ebb62ca 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -276,6 +276,16 @@ $(foreach file, $(libfdt_files), \
 	$(eval CFLAGS_$(file) = -I $(srctree)/scripts/dtc/libfdt))
 lib-$(CONFIG_LIBFDT) += $(libfdt_files)
 
+ifeq ($(CONFIG_EMBED_BOOT_CONFIG),y)
+$(obj)/bootconfig.o: $(obj)/default.bconf
+
+targets += default.bconf
+filechk_defbconf = cat /dev/null $(CONFIG_EMBED_BOOT_CONFIG_FILE)
+$(obj)/default.bconf: FORCE
+	$(call filechk,defbconf)
+
+endif
+
 lib-$(CONFIG_BOOT_CONFIG) += bootconfig.o
 
 obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 74f3201ab8e5..3a3bf3a208e3 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -12,6 +12,29 @@
 #include <linux/kernel.h>
 #include <linux/memblock.h>
 #include <linux/string.h>
+
+#ifdef CONFIG_EMBED_BOOT_CONFIG
+asm (
+"	.pushsection .init.data, \"aw\"			\n"
+"	.global embedded_bootconfig_data		\n"
+"embedded_bootconfig_data:				\n"
+"	.incbin \"lib/default.bconf\"			\n"
+"	.global embedded_bootconfig_data_end		\n"
+"embedded_bootconfig_data_end:				\n"
+"	.popsection					\n"
+);
+
+extern __visible char embedded_bootconfig_data[];
+extern __visible char embedded_bootconfig_data_end[];
+
+char * __init xbc_get_embedded_bootconfig(size_t *size)
+{
+	*size = embedded_bootconfig_data_end - embedded_bootconfig_data;
+	return (*size) ? embedded_bootconfig_data : NULL;
+}
+
+#endif
+
 #else /* !__KERNEL__ */
 /*
  * NOTE: This is only for tools/bootconfig, because tools/bootconfig will


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/3] docs: bootconfig: Add how to embed the bootconfig into kernel
  2022-03-26 23:53 [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
  2022-03-26 23:53 ` [PATCH v4 1/3] bootconfig: Check the checksum before removing the bootconfig from initrd Masami Hiramatsu
  2022-03-26 23:53 ` [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel Masami Hiramatsu
@ 2022-03-26 23:53 ` Masami Hiramatsu
  2022-03-27  2:55 ` [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
  3 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-26 23:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap, Nick Desaulniers, Sami Tolvanen,
	Nathan Chancellor, llvm, Masahiro Yamada,
	Linux Kbuild mailing list

Add a description how to embed the bootconfig file into kernel.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v3:
  - Fix typos.
 Changes in v2:
  - Corrected the text accoding to Randy's suggestion.
  - Do not reccomend to use relative path for CONFIG_EMBED_BOOT_CONFIG_FILE.
---
 Documentation/admin-guide/bootconfig.rst |   30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index a1860fc0ca88..9028e0f1307b 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -158,9 +158,15 @@ Each key-value pair is shown in each line with following style::
 Boot Kernel With a Boot Config
 ==============================
 
-Since the boot configuration file is loaded with initrd, it will be added
-to the end of the initrd (initramfs) image file with padding, size,
-checksum and 12-byte magic word as below.
+There are two options to boot the kernel with bootconfig: attaching the
+bootconfig to the initrd image or embedding it in the kernel itself.
+
+Attaching a Boot Config to Initrd
+---------------------------------
+
+Since the boot configuration file is loaded with initrd by default,
+it will be added to the end of the initrd (initramfs) image file with
+padding, size, checksum and 12-byte magic word as below.
 
 [initrd][bootconfig][padding][size(le32)][checksum(le32)][#BOOTCONFIG\n]
 
@@ -196,6 +202,24 @@ To remove the config from the image, you can use -d option as below::
 Then add "bootconfig" on the normal kernel command line to tell the
 kernel to look for the bootconfig at the end of the initrd file.
 
+Embedding a Boot Config into Kernel
+-----------------------------------
+
+If you can not use initrd, you can also embed the bootconfig file in the
+kernel by Kconfig options. In this case, you need to recompile the kernel
+with the following configs::
+
+ CONFIG_EMBED_BOOT_CONFIG=y
+ CONFIG_EMBED_BOOT_CONFIG_FILE="/PATH/TO/BOOTCONFIG/FILE"
+
+``CONFIG_EMBED_BOOT_CONFIG_FILE`` requires a correct absolute path to
+the bootconfig file. The kernel will embed it as the default bootconfig.
+
+Just as when attaching the bootconfig to the initrd, you need ``bootconfig``
+option on the kernel command line to enable the embedded bootconfig.
+
+Note that even if you set this option, you can override the embedded
+bootconfig by another bootconfig which attached to the initrd.
 
 Kernel parameters via Boot Config
 =================================


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot
  2022-03-26 23:53 [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2022-03-26 23:53 ` [PATCH v4 3/3] docs: bootconfig: Add how to embed the bootconfig into kernel Masami Hiramatsu
@ 2022-03-27  2:55 ` Masami Hiramatsu
  2022-04-13 13:23   ` Padmanabha Srinivasaiah
  3 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-27  2:55 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Steven Rostedt, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap, Nick Desaulniers, Sami Tolvanen,
	Nathan Chancellor, llvm, Masahiro Yamada,
	Linux Kbuild mailing list

Hi Padmanabha,

On Sun, 27 Mar 2022 08:53:06 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> 
> KNOWN ISSUE:
> 
> According to the report from Padmanabha[3], the embedded bootconfig data may not
> be updated if you do incremental build the kernel with CONFIG_LTO. This is
> under investigation.

I tried to test this version with LTO_CLANG_FULL and LTO_CLANG_THIN with
switching the embedded bootconfig file by CONFIG_EMBED_BOOT_CONFIG_FILE (on x86).

I found that when I use LTO_CLANG_FULL, the embedded bootconfig was updated
correctly.
But with the LTO_CLANG_THIN, the embedded bootconfig was *NOT* updated.

I used the latest prebuild llvm 15.0.0 on x86 [4]. Padmanabha, can you confirm
with this latest LLVM? I guess something wrong with your old LLVM.

[4] https://download.01.org/0day-ci/cross-package/clang-latest/clang-latest/clang.tar.xz

Here is the test procedure.

1. Prepare 2 different bootconfig files (bconf1, bconf2).
2. Configure kernel with LTO_CLANG and setting the full path of bconf1 to
   CONFIG_EMBED_BOOT_CONFIG_FILE.
3. Build the kernel
4. Boot the kernel with "bootconfig" in the kernel cmdline.
5. Check the /proc/bootconfig is same as bconf1.
6. Reconfigure kernel with the full path of *bconf2* to CONFIG_EMBED_BOOT_CONFIG_FILE.
7. Rebuild the kernel (no cleanup)
8. Boot the kernel with "bootconfig" in the kernel cmdline.
9. Check the /proc/bootconfig is same as bconf2.

So with LTO_CLANG_FULL, at the step 9 /proc/bootconfig shows bconf2, but with
LTO_CLANG_THIN, it shows bconf1.

In both cases, build log showed that the default.bconf was updated (I confirmed the
lib/default.bconf is updated)

  UPD     lib/default.bconf
  CC      lib/bootconfig.o
  AR      lib/lib.a


Here is my guess. I found that when we enable LTO_CLANG, the compiler compiles
C source file into LLVM IR bitcode.

$ file work/linux/build-x86_64/lib/bootconfig.o 
work/linux/build-x86_64/lib/bootconfig.o: LLVM IR bitcode

This means at this point the object file doesn't include the lib/default.bconf
because it will be embedded by assembler. The bitcode seems only have the
inline asm code (which only has an .incbin directive) as a constatns block[5].

[5]
  Block ID #11 (CONSTANTS_BLOCK):
      Num Instances: 32
         Total Size: 54305b/6788.12B/1697W
    Percent of file: 19.9792%
       Average Size: 1697.03/212.13B/53W
  Tot/Avg SubBlocks: 0/0.000000e+00
    Tot/Avg Abbrevs: 4/1.250000e-01
    Tot/Avg Records: 486/1.518750e+01
    Percent Abbrevs: 80.8642%

        Record Histogram:
                  Count    # Bits     b/Rec   % Abv  Record Kind
                    219      4860      22.2  100.00  INTEGER
                    144      1728      12.0  100.00  SETTYPE
                     41       656      16.0          NULL
                     39      2970      76.2          CE_INBOUNDS_GEP
                     26      3504     134.8  100.00  CSTRING
                     10     37720    3772.0          INLINEASM
                      4        96      24.0  100.00  CE_CAST
                      1        58                    CE_CMP
                      1        52                    CE_SELECT
                      1        46                    CE_BINOP

And when the LLVM runs LTO with THIN mode, it might not update (not rebuild to
machine code) that inline asm code block because that block is not updated.
I confirmed that the block (bootconfig.o) is not updated after rebuilding
the kernel as below.

After step 3.
$ llvm-bcanalyzer work/linux/build-x86_64/lib/bootconfig.o > bconf.dump1
After step 7.
$ llvm-bcanalyzer work/linux/build-x86_64/lib/bootconfig.o > bconf.dump2
$ diff bconf.dump*
(No difference)

Thank you,

> 
> [3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u
> 
> 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 |   30 ++++++++++++++++++++++++++---
>  include/linux/bootconfig.h               |   10 ++++++++++
>  init/Kconfig                             |   21 ++++++++++++++++++++
>  init/main.c                              |   31 +++++++++++++++---------------
>  lib/.gitignore                           |    1 +
>  lib/Makefile                             |   10 ++++++++++
>  lib/bootconfig.c                         |   23 ++++++++++++++++++++++
>  7 files changed, 108 insertions(+), 18 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel
  2022-03-26 23:53 ` [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel Masami Hiramatsu
@ 2022-03-28  6:27   ` Masahiro Yamada
  2022-03-28 11:16     ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2022-03-28  6:27 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Steven Rostedt, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	open list:DOCUMENTATION, Randy Dunlap, Nick Desaulniers,
	Sami Tolvanen, Nathan Chancellor, llvm,
	Linux Kbuild mailing list

On Sun, Mar 27, 2022 at 8:53 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> This allows kernel developer to embed a default bootconfig file in
> the kernel instead of embedding it in the initrd. This will be good
> for who are using the kernel without initrd, or who needs a default
> bootconfigs.
> This needs to set two kconfigs: CONFIG_EMBED_BOOT_CONFIG=y and set
> the file path to CONFIG_EMBED_BOOT_CONFIG_FILE.
>
> Note that you still need 'bootconfig' command line option to load the
> embedded bootconfig. Also if you boot using an initrd with a different
> bootconfig, the kernel will use the bootconfig in the initrd, instead
> of the default bootconfig.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  Changes in v3:
>   - Avoid updating the default.bconf if the file is not changed.
> ---
>  include/linux/bootconfig.h |   10 ++++++++++
>  init/Kconfig               |   21 +++++++++++++++++++++
>  init/main.c                |   13 ++++++++-----
>  lib/.gitignore             |    1 +
>  lib/Makefile               |   10 ++++++++++
>  lib/bootconfig.c           |   23 +++++++++++++++++++++++
>  6 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
> index a4665c7ab07c..5dbda5e3e9bb 100644
> --- a/include/linux/bootconfig.h
> +++ b/include/linux/bootconfig.h
> @@ -289,4 +289,14 @@ int __init xbc_get_info(int *node_size, size_t *data_size);
>  /* XBC cleanup data structures */
>  void __init xbc_exit(void);
>
> +/* XBC embedded bootconfig data in kernel */
> +#ifdef CONFIG_EMBED_BOOT_CONFIG
> +char * __init xbc_get_embedded_bootconfig(size_t *size);
> +#else
> +static inline char *xbc_get_embedded_bootconfig(size_t *size)
> +{
> +       return NULL;
> +}
> +#endif
> +
>  #endif
> diff --git a/init/Kconfig b/init/Kconfig
> index beb5b866c318..bff308a782f8 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1357,6 +1357,27 @@ config BOOT_CONFIG
>
>           If unsure, say Y.
>
> +config EMBED_BOOT_CONFIG
> +       bool "Embed bootconfig file in the kernel"
> +       depends on BOOT_CONFIG
> +       default n
> +       help
> +         Embed a bootconfig file given by EMBED_BOOT_CONFIG_FILE in the
> +         kernel. Usually, the bootconfig file is loaded with the initrd
> +         image. But if the system doesn't support initrd, this option will
> +         help you by embedding a bootconfig file while building the kernel.
> +
> +         If unsure, say N.
> +
> +config EMBED_BOOT_CONFIG_FILE
> +       string "Embedded bootconfig file path"
> +       default ""
> +       depends on EMBED_BOOT_CONFIG
> +       help
> +         Specify a bootconfig file which will be embedded to the kernel.
> +         This bootconfig will be used if there is no initrd or no other
> +         bootconfig in the initrd.
> +
>  choice
>         prompt "Compiler optimization level"
>         default CC_OPTIMIZE_FOR_PERFORMANCE
> diff --git a/init/main.c b/init/main.c
> index 4f3ba3b84e34..180511324c95 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -265,7 +265,7 @@ static int __init loglevel(char *str)
>  early_param("loglevel", loglevel);
>
>  #ifdef CONFIG_BLK_DEV_INITRD
> -static void * __init get_boot_config_from_initrd(u32 *_size)
> +static void * __init get_boot_config_from_initrd(size_t *_size)
>  {
>         u32 size, csum;
>         char *data;
> @@ -411,12 +411,15 @@ static void __init setup_boot_config(void)
>         static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
>         const char *msg;
>         int pos;
> -       u32 size;
> +       size_t size;
>         char *data, *err;
>         int ret;
>
>         /* Cut out the bootconfig data even if we have no bootconfig option */
>         data = get_boot_config_from_initrd(&size);
> +       /* If there is no bootconfig in initrd, try embedded one. */
> +       if (!data)
> +               data = xbc_get_embedded_bootconfig(&size);
>
>         strlcpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>         err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
> @@ -435,8 +438,8 @@ static void __init setup_boot_config(void)
>         }
>
>         if (size >= XBC_DATA_MAX) {
> -               pr_err("bootconfig size %d greater than max size %d\n",
> -                       size, XBC_DATA_MAX);
> +               pr_err("bootconfig size %ld greater than max size %d\n",
> +                       (long)size, XBC_DATA_MAX);
>                 return;
>         }
>
> @@ -449,7 +452,7 @@ static void __init setup_boot_config(void)
>                                 msg, pos);
>         } else {
>                 xbc_get_info(&ret, NULL);
> -               pr_info("Load bootconfig: %d bytes %d nodes\n", size, ret);
> +               pr_info("Load bootconfig: %ld bytes %d nodes\n", (long)size, ret);
>                 /* keys starting with "kernel." are passed via cmdline */
>                 extra_command_line = xbc_make_cmdline("kernel");
>                 /* Also, "init." keys are init arguments */
> diff --git a/lib/.gitignore b/lib/.gitignore
> index e5e217b8307b..30a2a5db7033 100644
> --- a/lib/.gitignore
> +++ b/lib/.gitignore
> @@ -6,3 +6,4 @@
>  /oid_registry_data.c
>  /test_fortify.log
>  /test_fortify/*.log
> +/default.bconf


I think lib/.gitignore is alphabetically sorted.

Please insert the new one
to the proper line.





> diff --git a/lib/Makefile b/lib/Makefile
> index 353bc09ce38d..dd9f3ebb62ca 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -276,6 +276,16 @@ $(foreach file, $(libfdt_files), \
>         $(eval CFLAGS_$(file) = -I $(srctree)/scripts/dtc/libfdt))
>  lib-$(CONFIG_LIBFDT) += $(libfdt_files)
>
> +ifeq ($(CONFIG_EMBED_BOOT_CONFIG),y)
> +$(obj)/bootconfig.o: $(obj)/default.bconf
> +
> +targets += default.bconf


I did not test this patch, but presumably
"make clean" will miss to clean up default.bconf

The 'targets' must exist outside the ifeq-block.
Move the 'endif' up.


ifeq ($(CONFIG_EMBED_BOOT_CONFIG),y)
$(obj)/bootconfig.o: $(obj)/default.bconf
endif

targets += default.bconf

...





> +filechk_defbconf = cat /dev/null $(CONFIG_EMBED_BOOT_CONFIG_FILE)
> +$(obj)/default.bconf: FORCE
> +       $(call filechk,defbconf)


This will work, but users will be confused when they
try to build out-of-tree with the O= option.

If CONFIG_EMBED_BOOT_CONFIG_FILE is a relative path,
it is actually relative to the object tree, not the source tree.
I am not sure if that is the expected behavior, but it is not documented
anywhere.


If you want to search both in the objtree and srctree,
you can write like follows:   [UNTESTED]


filechk_defbconf = cat $(or $(real-prereqs), /dev/null)
$(obj)/default.bconf: $(CONFIG_EMBED_BOOT_CONFIG_FILE) FORCE
        $(call filechk,defbconf)




> +
> +endif
> +
>  lib-$(CONFIG_BOOT_CONFIG) += bootconfig.o
>
>  obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o
> diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> index 74f3201ab8e5..3a3bf3a208e3 100644
> --- a/lib/bootconfig.c
> +++ b/lib/bootconfig.c
> @@ -12,6 +12,29 @@
>  #include <linux/kernel.h>
>  #include <linux/memblock.h>
>  #include <linux/string.h>
> +
> +#ifdef CONFIG_EMBED_BOOT_CONFIG
> +asm (
> +"      .pushsection .init.data, \"aw\"                 \n"
> +"      .global embedded_bootconfig_data                \n"
> +"embedded_bootconfig_data:                             \n"
> +"      .incbin \"lib/default.bconf\"                   \n"
> +"      .global embedded_bootconfig_data_end            \n"
> +"embedded_bootconfig_data_end:                         \n"
> +"      .popsection                                     \n"
> +);
> +
> +extern __visible char embedded_bootconfig_data[];
> +extern __visible char embedded_bootconfig_data_end[];
> +
> +char * __init xbc_get_embedded_bootconfig(size_t *size)
> +{
> +       *size = embedded_bootconfig_data_end - embedded_bootconfig_data;
> +       return (*size) ? embedded_bootconfig_data : NULL;
> +}
> +
> +#endif
> +
>  #else /* !__KERNEL__ */
>  /*
>   * NOTE: This is only for tools/bootconfig, because tools/bootconfig will
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel
  2022-03-28  6:27   ` Masahiro Yamada
@ 2022-03-28 11:16     ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-28 11:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Steven Rostedt, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	open list:DOCUMENTATION, Randy Dunlap, Nick Desaulniers,
	Sami Tolvanen, Nathan Chancellor, llvm,
	Linux Kbuild mailing list

On Mon, 28 Mar 2022 15:27:43 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:

> On Sun, Mar 27, 2022 at 8:53 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > This allows kernel developer to embed a default bootconfig file in
> > the kernel instead of embedding it in the initrd. This will be good
> > for who are using the kernel without initrd, or who needs a default
> > bootconfigs.
> > This needs to set two kconfigs: CONFIG_EMBED_BOOT_CONFIG=y and set
> > the file path to CONFIG_EMBED_BOOT_CONFIG_FILE.
> >
> > Note that you still need 'bootconfig' command line option to load the
> > embedded bootconfig. Also if you boot using an initrd with a different
> > bootconfig, the kernel will use the bootconfig in the initrd, instead
> > of the default bootconfig.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  Changes in v3:
> >   - Avoid updating the default.bconf if the file is not changed.
> > ---
> >  include/linux/bootconfig.h |   10 ++++++++++
> >  init/Kconfig               |   21 +++++++++++++++++++++
> >  init/main.c                |   13 ++++++++-----
> >  lib/.gitignore             |    1 +
> >  lib/Makefile               |   10 ++++++++++
> >  lib/bootconfig.c           |   23 +++++++++++++++++++++++
> >  6 files changed, 73 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
> > index a4665c7ab07c..5dbda5e3e9bb 100644
> > --- a/include/linux/bootconfig.h
> > +++ b/include/linux/bootconfig.h
> > @@ -289,4 +289,14 @@ int __init xbc_get_info(int *node_size, size_t *data_size);
> >  /* XBC cleanup data structures */
> >  void __init xbc_exit(void);
> >
> > +/* XBC embedded bootconfig data in kernel */
> > +#ifdef CONFIG_EMBED_BOOT_CONFIG
> > +char * __init xbc_get_embedded_bootconfig(size_t *size);
> > +#else
> > +static inline char *xbc_get_embedded_bootconfig(size_t *size)
> > +{
> > +       return NULL;
> > +}
> > +#endif
> > +
> >  #endif
> > diff --git a/init/Kconfig b/init/Kconfig
> > index beb5b866c318..bff308a782f8 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1357,6 +1357,27 @@ config BOOT_CONFIG
> >
> >           If unsure, say Y.
> >
> > +config EMBED_BOOT_CONFIG
> > +       bool "Embed bootconfig file in the kernel"
> > +       depends on BOOT_CONFIG
> > +       default n
> > +       help
> > +         Embed a bootconfig file given by EMBED_BOOT_CONFIG_FILE in the
> > +         kernel. Usually, the bootconfig file is loaded with the initrd
> > +         image. But if the system doesn't support initrd, this option will
> > +         help you by embedding a bootconfig file while building the kernel.
> > +
> > +         If unsure, say N.
> > +
> > +config EMBED_BOOT_CONFIG_FILE
> > +       string "Embedded bootconfig file path"
> > +       default ""
> > +       depends on EMBED_BOOT_CONFIG
> > +       help
> > +         Specify a bootconfig file which will be embedded to the kernel.
> > +         This bootconfig will be used if there is no initrd or no other
> > +         bootconfig in the initrd.
> > +
> >  choice
> >         prompt "Compiler optimization level"
> >         default CC_OPTIMIZE_FOR_PERFORMANCE
> > diff --git a/init/main.c b/init/main.c
> > index 4f3ba3b84e34..180511324c95 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -265,7 +265,7 @@ static int __init loglevel(char *str)
> >  early_param("loglevel", loglevel);
> >
> >  #ifdef CONFIG_BLK_DEV_INITRD
> > -static void * __init get_boot_config_from_initrd(u32 *_size)
> > +static void * __init get_boot_config_from_initrd(size_t *_size)
> >  {
> >         u32 size, csum;
> >         char *data;
> > @@ -411,12 +411,15 @@ static void __init setup_boot_config(void)
> >         static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
> >         const char *msg;
> >         int pos;
> > -       u32 size;
> > +       size_t size;
> >         char *data, *err;
> >         int ret;
> >
> >         /* Cut out the bootconfig data even if we have no bootconfig option */
> >         data = get_boot_config_from_initrd(&size);
> > +       /* If there is no bootconfig in initrd, try embedded one. */
> > +       if (!data)
> > +               data = xbc_get_embedded_bootconfig(&size);
> >
> >         strlcpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> >         err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
> > @@ -435,8 +438,8 @@ static void __init setup_boot_config(void)
> >         }
> >
> >         if (size >= XBC_DATA_MAX) {
> > -               pr_err("bootconfig size %d greater than max size %d\n",
> > -                       size, XBC_DATA_MAX);
> > +               pr_err("bootconfig size %ld greater than max size %d\n",
> > +                       (long)size, XBC_DATA_MAX);
> >                 return;
> >         }
> >
> > @@ -449,7 +452,7 @@ static void __init setup_boot_config(void)
> >                                 msg, pos);
> >         } else {
> >                 xbc_get_info(&ret, NULL);
> > -               pr_info("Load bootconfig: %d bytes %d nodes\n", size, ret);
> > +               pr_info("Load bootconfig: %ld bytes %d nodes\n", (long)size, ret);
> >                 /* keys starting with "kernel." are passed via cmdline */
> >                 extra_command_line = xbc_make_cmdline("kernel");
> >                 /* Also, "init." keys are init arguments */
> > diff --git a/lib/.gitignore b/lib/.gitignore
> > index e5e217b8307b..30a2a5db7033 100644
> > --- a/lib/.gitignore
> > +++ b/lib/.gitignore
> > @@ -6,3 +6,4 @@
> >  /oid_registry_data.c
> >  /test_fortify.log
> >  /test_fortify/*.log
> > +/default.bconf
> 
> 
> I think lib/.gitignore is alphabetically sorted.
> 
> Please insert the new one
> to the proper line.

Ah, OK. Let me fix that.

> > diff --git a/lib/Makefile b/lib/Makefile
> > index 353bc09ce38d..dd9f3ebb62ca 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -276,6 +276,16 @@ $(foreach file, $(libfdt_files), \
> >         $(eval CFLAGS_$(file) = -I $(srctree)/scripts/dtc/libfdt))
> >  lib-$(CONFIG_LIBFDT) += $(libfdt_files)
> >
> > +ifeq ($(CONFIG_EMBED_BOOT_CONFIG),y)
> > +$(obj)/bootconfig.o: $(obj)/default.bconf
> > +
> > +targets += default.bconf
> 
> 
> I did not test this patch, but presumably
> "make clean" will miss to clean up default.bconf
> 
> The 'targets' must exist outside the ifeq-block.
> Move the 'endif' up.
> 
> 
> ifeq ($(CONFIG_EMBED_BOOT_CONFIG),y)
> $(obj)/bootconfig.o: $(obj)/default.bconf
> endif
> 
> targets += default.bconf
> 
> ...

You're right! I didn't know this behavior. Thanks for the notice.
I misunderstood that the files in targets always be built...

After moving the 'targets' out of ifeq-block, I confirmed that the
default.bconf was not generated when CONFIG_EMBED_BOOT_CONFIG is not
set, and it is cleaned up when CONFIG_EMBED_BOOT_CONFIG=y.
Thanks!

> 
> > +filechk_defbconf = cat /dev/null $(CONFIG_EMBED_BOOT_CONFIG_FILE)
> > +$(obj)/default.bconf: FORCE
> > +       $(call filechk,defbconf)
> 
> 
> This will work, but users will be confused when they
> try to build out-of-tree with the O= option.
> 
> If CONFIG_EMBED_BOOT_CONFIG_FILE is a relative path,
> it is actually relative to the object tree, not the source tree.

Yes, that's right. I would like to fix this issue.

> I am not sure if that is the expected behavior, but it is not documented
> anywhere.
> 
> 
> If you want to search both in the objtree and srctree,
> you can write like follows:   [UNTESTED]
> 
> 
> filechk_defbconf = cat $(or $(real-prereqs), /dev/null)
> $(obj)/default.bconf: $(CONFIG_EMBED_BOOT_CONFIG_FILE) FORCE
>         $(call filechk,defbconf)

Great! let me check it works.

Thank you,

> 
> 
> 
> 
> > +
> > +endif
> > +
> >  lib-$(CONFIG_BOOT_CONFIG) += bootconfig.o
> >
> >  obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o
> > diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> > index 74f3201ab8e5..3a3bf3a208e3 100644
> > --- a/lib/bootconfig.c
> > +++ b/lib/bootconfig.c
> > @@ -12,6 +12,29 @@
> >  #include <linux/kernel.h>
> >  #include <linux/memblock.h>
> >  #include <linux/string.h>
> > +
> > +#ifdef CONFIG_EMBED_BOOT_CONFIG
> > +asm (
> > +"      .pushsection .init.data, \"aw\"                 \n"
> > +"      .global embedded_bootconfig_data                \n"
> > +"embedded_bootconfig_data:                             \n"
> > +"      .incbin \"lib/default.bconf\"                   \n"
> > +"      .global embedded_bootconfig_data_end            \n"
> > +"embedded_bootconfig_data_end:                         \n"
> > +"      .popsection                                     \n"
> > +);
> > +
> > +extern __visible char embedded_bootconfig_data[];
> > +extern __visible char embedded_bootconfig_data_end[];
> > +
> > +char * __init xbc_get_embedded_bootconfig(size_t *size)
> > +{
> > +       *size = embedded_bootconfig_data_end - embedded_bootconfig_data;
> > +       return (*size) ? embedded_bootconfig_data : NULL;
> > +}
> > +
> > +#endif
> > +
> >  #else /* !__KERNEL__ */
> >  /*
> >   * NOTE: This is only for tools/bootconfig, because tools/bootconfig will
> >
> 
> 
> -- 
> Best Regards
> Masahiro Yamada


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot
  2022-03-27  2:55 ` [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
@ 2022-04-13 13:23   ` Padmanabha Srinivasaiah
  0 siblings, 0 replies; 10+ messages in thread
From: Padmanabha Srinivasaiah @ 2022-04-13 13:23 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Steven Rostedt, LKML, Jonathan Corbet, linux-doc, Randy Dunlap,
	Nick Desaulniers, Sami Tolvanen, Nathan Chancellor, llvm,
	Masahiro Yamada, Linux Kbuild mailing list

Hello Masami Hiramatsu,

Sorry for long delay, was out of town.

>On Sun, Mar 27, 2022 at 11:55:26AM +0900, Masami Hiramatsu wrote:
> Hi Padmanabha,
> 
> On Sun, 27 Mar 2022 08:53:06 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > 
> > KNOWN ISSUE:
> > 
> > According to the report from Padmanabha[3], the embedded bootconfig data may not
> > be updated if you do incremental build the kernel with CONFIG_LTO. This is
> > under investigation.
> 
> I tried to test this version with LTO_CLANG_FULL and LTO_CLANG_THIN with
> switching the embedded bootconfig file by CONFIG_EMBED_BOOT_CONFIG_FILE (on x86).
> 
> I found that when I use LTO_CLANG_FULL, the embedded bootconfig was updated
> correctly.
> But with the LTO_CLANG_THIN, the embedded bootconfig was *NOT* updated.
> 
> I used the latest prebuild llvm 15.0.0 on x86 [4]. Padmanabha, can you confirm
> with this latest LLVM? I guess something wrong with your old LLVM.
> 
> [4] https://download.01.org/0day-ci/cross-package/clang-latest/clang-latest/clang.tar.xz
>

Thanks for the link for prebuilt llvm 15.0.0.

[p1] https://www.spinics.net/lists/kernel/msg4307362.html

With llvm 15.0.0 and latest patchset v8 for bootconfig [p1] on arm64 the
test observations are:

For bootconfig:

I dont observe anymore an issue with incremental builds, irresepctive of LTO
type selected: FULL/THIN.

For config.gz:

The test observations is exactly similar to yours.
With LTO_CLANG_THIN configs were *NOT* updated.
But with LTO_CLANG_FULL confs were updated.

Test results validates and confirms your earlier analysis on issue with
inline asm and .incbin directive and THIN_LTO. 

> Here is the test procedure.
> 
> 1. Prepare 2 different bootconfig files (bconf1, bconf2).
> 2. Configure kernel with LTO_CLANG and setting the full path of bconf1 to
>    CONFIG_EMBED_BOOT_CONFIG_FILE.
> 3. Build the kernel
> 4. Boot the kernel with "bootconfig" in the kernel cmdline.
> 5. Check the /proc/bootconfig is same as bconf1.
> 6. Reconfigure kernel with the full path of *bconf2* to CONFIG_EMBED_BOOT_CONFIG_FILE.
> 7. Rebuild the kernel (no cleanup)
> 8. Boot the kernel with "bootconfig" in the kernel cmdline.
> 9. Check the /proc/bootconfig is same as bconf2.
> 
> So with LTO_CLANG_FULL, at the step 9 /proc/bootconfig shows bconf2, but with
> LTO_CLANG_THIN, it shows bconf1.
> 
> In both cases, build log showed that the default.bconf was updated (I confirmed the
> lib/default.bconf is updated)
> 
>   UPD     lib/default.bconf
>   CC      lib/bootconfig.o
>   AR      lib/lib.a
> 
> 
> Here is my guess. I found that when we enable LTO_CLANG, the compiler compiles
> C source file into LLVM IR bitcode.
> 
> $ file work/linux/build-x86_64/lib/bootconfig.o 
> work/linux/build-x86_64/lib/bootconfig.o: LLVM IR bitcode
> 
> This means at this point the object file doesn't include the lib/default.bconf
> because it will be embedded by assembler. The bitcode seems only have the
> inline asm code (which only has an .incbin directive) as a constatns block[5].
>
> [5]
>   Block ID #11 (CONSTANTS_BLOCK):
>       Num Instances: 32
>          Total Size: 54305b/6788.12B/1697W
>     Percent of file: 19.9792%
>        Average Size: 1697.03/212.13B/53W
>   Tot/Avg SubBlocks: 0/0.000000e+00
>     Tot/Avg Abbrevs: 4/1.250000e-01
>     Tot/Avg Records: 486/1.518750e+01
>     Percent Abbrevs: 80.8642%
> 
>         Record Histogram:
>                   Count    # Bits     b/Rec   % Abv  Record Kind
>                     219      4860      22.2  100.00  INTEGER
>                     144      1728      12.0  100.00  SETTYPE
>                      41       656      16.0          NULL
>                      39      2970      76.2          CE_INBOUNDS_GEP
>                      26      3504     134.8  100.00  CSTRING
>                      10     37720    3772.0          INLINEASM
>                       4        96      24.0  100.00  CE_CAST
>                       1        58                    CE_CMP
>                       1        52                    CE_SELECT
>                       1        46                    CE_BINOP
> 
> And when the LLVM runs LTO with THIN mode, it might not update (not rebuild to
> machine code) that inline asm code block because that block is not updated.
> I confirmed that the block (bootconfig.o) is not updated after rebuilding
> the kernel as below.
>
Yes, thanks for details. Analysis is in sync with observations from test
results.

Thanks and Regards,
Padmanabha.S
> After step 3.
> $ llvm-bcanalyzer work/linux/build-x86_64/lib/bootconfig.o > bconf.dump1
> After step 7.
> $ llvm-bcanalyzer work/linux/build-x86_64/lib/bootconfig.o > bconf.dump2
> $ diff bconf.dump*
> (No difference)
> 
> Thank you,
> 
> > 
> > [3] https://lore.kernel.org/all/20220321183500.GA4065@pswork/T/#u
> > 
> > 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 |   30 ++++++++++++++++++++++++++---
> >  include/linux/bootconfig.h               |   10 ++++++++++
> >  init/Kconfig                             |   21 ++++++++++++++++++++
> >  init/main.c                              |   31 +++++++++++++++---------------
> >  lib/.gitignore                           |    1 +
> >  lib/Makefile                             |   10 ++++++++++
> >  lib/bootconfig.c                         |   23 ++++++++++++++++++++++
> >  7 files changed, 108 insertions(+), 18 deletions(-)
> > 
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot
  2022-03-26 23:36 Masami Hiramatsu
@ 2022-03-26 23:38 ` Masami Hiramatsu
  0 siblings, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-26 23:38 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Steven Rostedt, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap

Hi,

I forgot to add LTO people and Masahiro to this mail. Let me update the
cover mail and resubmit.

On Sun, 27 Mar 2022 08:36:53 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here are the 4th version of the patchset to enable kernel embedded bootconfig
> for non-initrd kernel boot environment. I've fixed Makefile to determine
> update by checking the contents of embedding bootconfig file. Thanks
> Masahiro!
> 
> 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;
> 
>  https://lore.kernel.org/all/20220307184011.GA2570@pswork/T/#u
> 
> 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 |   30 ++++++++++++++++++++++++++---
>  include/linux/bootconfig.h               |   10 ++++++++++
>  init/Kconfig                             |   21 ++++++++++++++++++++
>  init/main.c                              |   31 +++++++++++++++---------------
>  lib/.gitignore                           |    1 +
>  lib/Makefile                             |   10 ++++++++++
>  lib/bootconfig.c                         |   23 ++++++++++++++++++++++
>  7 files changed, 108 insertions(+), 18 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot
@ 2022-03-26 23:36 Masami Hiramatsu
  2022-03-26 23:38 ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2022-03-26 23:36 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Padmanabha Srinivasaiah, LKML, Jonathan Corbet,
	linux-doc, Randy Dunlap

Hi,

Here are the 4th version of the patchset to enable kernel embedded bootconfig
for non-initrd kernel boot environment. I've fixed Makefile to determine
update by checking the contents of embedding bootconfig file. Thanks
Masahiro!

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;

 https://lore.kernel.org/all/20220307184011.GA2570@pswork/T/#u

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 |   30 ++++++++++++++++++++++++++---
 include/linux/bootconfig.h               |   10 ++++++++++
 init/Kconfig                             |   21 ++++++++++++++++++++
 init/main.c                              |   31 +++++++++++++++---------------
 lib/.gitignore                           |    1 +
 lib/Makefile                             |   10 ++++++++++
 lib/bootconfig.c                         |   23 ++++++++++++++++++++++
 7 files changed, 108 insertions(+), 18 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-04-13 13:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-26 23:53 [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
2022-03-26 23:53 ` [PATCH v4 1/3] bootconfig: Check the checksum before removing the bootconfig from initrd Masami Hiramatsu
2022-03-26 23:53 ` [PATCH v4 2/3] bootconfig: Support embedding a bootconfig file in kernel Masami Hiramatsu
2022-03-28  6:27   ` Masahiro Yamada
2022-03-28 11:16     ` Masami Hiramatsu
2022-03-26 23:53 ` [PATCH v4 3/3] docs: bootconfig: Add how to embed the bootconfig into kernel Masami Hiramatsu
2022-03-27  2:55 ` [PATCH v4 0/3] bootconfig: Support embedding a bootconfig in kernel for non initrd boot Masami Hiramatsu
2022-04-13 13:23   ` Padmanabha Srinivasaiah
  -- strict thread matches above, loose matches on Subject: below --
2022-03-26 23:36 Masami Hiramatsu
2022-03-26 23:38 ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).