[PATCH] x86/mm: Fix copy error in comments
Direct page mapping in bottom-up way will allocate memory from low
address for page structures in a range, which is the *bottom*,
not the *end*.
Signed-off-by: Cao jin
---
arch/x86/mm/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index e26f5c5c6565..bc2f871c75f1 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -663,7 +663,7 @@ static void __init memory_map_bottom_up(unsigned long
map_start,
/*
* We start from the bottom (@map_start) and go to the top (@map_end).
* The memblock_find_in_range() gets us a block of RAM from the
-* end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
+* bottom of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
* for page table.
*/
while (start < map_end) {
--
2.30.1
[tip: x86/cleanups] x86/setup: Remove unused RESERVE_BRK_ARRAY()
The following commit has been merged into the x86/cleanups branch of tip:
Commit-ID: 81519f778830d1ab02274eeaaeab6797fdc4ec52
Gitweb:
https://git.kernel.org/tip/81519f778830d1ab02274eeaaeab6797fdc4ec52
Author:Cao jin
AuthorDate:Thu, 11 Mar 2021 16:39:19 +08:00
Committer: Borislav Petkov
CommitterDate: Thu, 11 Mar 2021 11:47:37 +01:00
x86/setup: Remove unused RESERVE_BRK_ARRAY()
Since a13f2ef168cb ("x86/xen: remove 32-bit Xen PV guest support"),
RESERVE_BRK_ARRAY() has no user anymore so drop it.
Update related comments too.
Signed-off-by: Cao jin
Signed-off-by: Borislav Petkov
Link: https://lkml.kernel.org/r/20210311083919.27530-1-jojin...@gmail.com
---
arch/x86/include/asm/setup.h | 5 -
arch/x86/kernel/setup.c | 6 +++---
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 389d851..a12458a 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -130,11 +130,6 @@ void *extend_brk(size_t size, size_t align);
: : "i" (sz)); \
}
-/* Helper for reserving space for arrays of things */
-#define RESERVE_BRK_ARRAY(type, name, entries) \
- type *name; \
- RESERVE_BRK(name, sizeof(type) * entries)
-
extern void probe_roms(void);
#ifdef __i386__
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d883176..cdf7bbd 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -65,7 +65,7 @@ RESERVE_BRK(dmi_alloc, 65536);
/*
* Range of the BSS area. The size of the BSS area is determined
- * at link time, with RESERVE_BRK*() facility reserving additional
+ * at link time, with RESERVE_BRK() facility reserving additional
* chunks.
*/
unsigned long _brk_start = (unsigned long)__brk_base;
@@ -1038,8 +1038,8 @@ void __init setup_arch(char **cmdline_p)
/*
* Need to conclude brk, before e820__memblock_setup()
-* it could use memblock_find_in_range, could overlap with
-* brk area.
+* it could use memblock_find_in_range, could overlap with
+* brk area.
*/
reserve_brk();
[PATCH] bootconfig: Update prototype of setup_boot_config()
Parameter "cmdline" has no use, drop it.
Signed-off-by: Cao jin
---
init/main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/init/main.c b/init/main.c
index c68d784376ca..621a11ed18fb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -404,7 +404,7 @@ static int __init bootconfig_params(char *param, char *val,
return 0;
}
-static void __init setup_boot_config(const char *cmdline)
+static void __init setup_boot_config(void)
{
static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
const char *msg;
@@ -471,7 +471,7 @@ static void __init setup_boot_config(const char *cmdline)
#else
-static void __init setup_boot_config(const char *cmdline)
+static void __init setup_boot_config(void)
{
/* Remove bootconfig data from initrd */
get_boot_config_from_initrd(NULL, NULL);
@@ -869,7 +869,7 @@ asmlinkage __visible void __init __no_sanitize_address
start_kernel(void)
pr_notice("%s", linux_banner);
early_security_init();
setup_arch(_line);
- setup_boot_config(command_line);
+ setup_boot_config();
setup_command_line(command_line);
setup_nr_cpu_ids();
setup_per_cpu_areas();
--
2.29.2
[PATCH] x86/brk: Drop RESERVE_BRK_ARRAY()
Since a13f2ef168cb ("x86/xen: remove 32-bit Xen PV guest support"),
RESERVE_BRK_ARRAY() has no user anymore, let's drop it.
Update related comments too.
Signed-off-by: Cao jin
---
arch/x86/include/asm/setup.h | 5 -
arch/x86/kernel/setup.c | 6 +++---
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 389d851a02c4..a12458a7a8d4 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -130,11 +130,6 @@ void *extend_brk(size_t size, size_t align);
: : "i" (sz)); \
}
-/* Helper for reserving space for arrays of things */
-#define RESERVE_BRK_ARRAY(type, name, entries) \
- type *name; \
- RESERVE_BRK(name, sizeof(type) * entries)
-
extern void probe_roms(void);
#ifdef __i386__
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 740f3bdb3f61..3af27bf1f837 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -66,7 +66,7 @@ RESERVE_BRK(dmi_alloc, 65536);
/*
* Range of the BSS area. The size of the BSS area is determined
- * at link time, with RESERVE_BRK*() facility reserving additional
+ * at link time, with RESERVE_BRK() facility reserving additional
* chunks.
*/
unsigned long _brk_start = (unsigned long)__brk_base;
@@ -1039,8 +1039,8 @@ void __init setup_arch(char **cmdline_p)
/*
* Need to conclude brk, before e820__memblock_setup()
-* it could use memblock_find_in_range, could overlap with
-* brk area.
+* it could use memblock_find_in_range, could overlap with
+* brk area.
*/
reserve_brk();
--
2.29.2
[PATCH] Documentation/x86/boot.rst: Correct the example of SETUP_INDIRECT
struct setup_data.len is the length of data field. In case of
SETUP_INDIRECT, it should be sizeof(setup_indirect).
Signed-off-by: Cao jin
---
Documentation/x86/boot.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index abb9fc164657..fc844913dece 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -851,7 +851,7 @@ Protocol: 2.09+
struct setup_data {
__u64 next = 0 or ;
__u32 type = SETUP_INDIRECT;
- __u32 len = sizeof(setup_data);
+ __u32 len = sizeof(setup_indirect);
__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
__u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
__u32 reserved = 0;
--
2.29.2
[RFC PATCH] Documentation/x86/boot.rst: minor languge improvement
Suggested-by: H. Peter Anvin Signed-off-by: Cao jin --- for 64-bit protocol, setup data still needs to be mapped, as there is operation on it in extract_kernel(), like: sanitize_boot_params(boot_params); initrd doesn't need to be mapped, which is also what KASLR does in its mem_avoid_init(). Documentation/x86/boot.rst | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst index 7fafc7ac00d7..392c6e147e70 100644 --- a/Documentation/x86/boot.rst +++ b/Documentation/x86/boot.rst @@ -1353,12 +1353,12 @@ In 32-bit boot protocol, the kernel is started by jumping to the 32/64-bit kernel. At entry, the CPU must be in 32-bit protected mode with paging -disabled; a GDT must be loaded with the descriptors for selectors -__BOOT_CS(0x10) and __BOOT_DS(0x18); both descriptors must be 4G flat -segment; __BOOT_CS must have execute/read permission, and __BOOT_DS -must have read/write permission; CS must be __BOOT_CS and DS, ES, SS -must be __BOOT_DS; interrupt must be disabled; %esi must hold the base -address of the struct boot_params; %ebp, %edi and %ebx must be zero. +disabled and a GDT must be loaded with the descriptors for selectors +__BOOT_CS(0x10) and __BOOT_DS(0x18): both descriptors must be 4G flat +segment with __BOOT_CS having execute/read permission and __BOOT_DS +having read/write permission. CS must be __BOOT_CS and DS, ES, SS +must be __BOOT_DS. Interrupt must be disabled and %esi must hold the +base address of the struct boot_params. %ebp, %edi and %ebx must be zero. 64-bit Boot Protocol @@ -1379,7 +1379,7 @@ can be calculated as follows:: In addition to read/modify/write the setup header of the struct boot_params as that of 16-bit boot protocol, the boot loader should also fill the additional fields of the struct boot_params as described -in zero-page.txt. +in zero-page.rst. After setting up the struct boot_params, the boot loader can load 64-bit kernel in the same way as that of 16-bit boot protocol, but @@ -1389,15 +1389,14 @@ In 64-bit boot protocol, the kernel is started by jumping to the 64-bit kernel entry point, which is the start address of loaded 64-bit kernel plus 0x200. -At entry, the CPU must be in 64-bit mode with paging enabled. -The range with setup_header.init_size from start address of loaded -kernel and zero page and command line buffer get ident mapping; -a GDT must be loaded with the descriptors for selectors -__BOOT_CS(0x10) and __BOOT_DS(0x18); both descriptors must be 4G flat -segment; __BOOT_CS must have execute/read permission, and __BOOT_DS -must have read/write permission; CS must be __BOOT_CS and DS, ES, SS -must be __BOOT_DS; interrupt must be disabled; %rsi must hold the base -address of the struct boot_params. +At entry, the CPU must be in 64-bit mode. The range with +setup_header.init_size from start address of loaded kernel, the zero page, +and the command line buffer get identity-mapped, and a GDT must be loaded +with the descriptors for selectors __BOOT_CS(0x10) and __BOOT_DS(0x18): +both descriptors must be 4G flat segment with __BOOT_CS having execute/read +permission and __BOOT_DS having read/write permission. CS must be __BOOT_CS +and DS, ES, SS must be __BOOT_DS. Interrupt must be disabled and %rsi must +hold the base address of the struct boot_params. EFI Handover Protocol (deprecated) == -- 2.21.0
Re: [PATCH] Documentation/x86/boot.rst: minor improvement
On 9/1/20 1:35 PM, H. Peter Anvin wrote: > If you are going to fix the language... > > On 2020-08-31 22:25, Cao jin wrote: >> Sorry, I mis-copied 2 addresses. make sure they are CCed. >> >> On 9/1/20 11:41 AM, Cao jin wrote: >>> Typo fix & file name update >>> I did quick search in dict & google, didn't see ident = identity, my omission. >>> Signed-off-by: Cao jin ... >>> @@ -1391,7 +1391,7 @@ In 64-bit boot protocol, the kernel is started by >>> jumping to the >>> >>> At entry, the CPU must be in 64-bit mode with paging enabled. > > (Paging enabled is redundant here.) Yes, 64-bit defaults with paging enabled. Maybe it is a little bit helpful for newbies. > >>> The range with setup_header.init_size from start address of loaded >>> -kernel and zero page and command line buffer get ident mapping; >>> +kernel and zero page and command line buffer get identity mapping; > > The range with setup_header.init_size from start address of the loaded kernel, > the zero page, and the command line buffer get identity-mapped, anda GDT must > be loaded with the descriptors for selectors __BOOT_CS(0x10) and > __BOOT_DS(0x18): both descriptors must be 4G flat segment with __BOOT_CS > having execute/read > permission and __BOOT_DS... > > Also, it might be useful to take a look to see if other data structures, like > setup_data and the initrd also need to be in the identity map. > Thanks for your info. I have indistinct memory that KASLR need them identity mapped. I will take a look. -- Sincerely, Cao jin
Re: [PATCH] Documentation/x86/boot.rst: minor improvement
Sorry, I mis-copied 2 addresses. make sure they are CCed. On 9/1/20 11:41 AM, Cao jin wrote: > Typo fix & file name update > > Signed-off-by: Cao jin > --- > Documentation/x86/boot.rst | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst > index 7fafc7ac00d7..c04afec90486 100644 > --- a/Documentation/x86/boot.rst > +++ b/Documentation/x86/boot.rst > @@ -1379,7 +1379,7 @@ can be calculated as follows:: > In addition to read/modify/write the setup header of the struct > boot_params as that of 16-bit boot protocol, the boot loader should > also fill the additional fields of the struct boot_params as described > -in zero-page.txt. > +in zero-page.rst. > > After setting up the struct boot_params, the boot loader can load > 64-bit kernel in the same way as that of 16-bit boot protocol, but > @@ -1391,7 +1391,7 @@ In 64-bit boot protocol, the kernel is started by > jumping to the > > At entry, the CPU must be in 64-bit mode with paging enabled. > The range with setup_header.init_size from start address of loaded > -kernel and zero page and command line buffer get ident mapping; > +kernel and zero page and command line buffer get identity mapping; > a GDT must be loaded with the descriptors for selectors > __BOOT_CS(0x10) and __BOOT_DS(0x18); both descriptors must be 4G flat > segment; __BOOT_CS must have execute/read permission, and __BOOT_DS > -- Sincerely, Cao jin
[PATCH] Documentation/x86/boot.rst: minor improvement
Typo fix & file name update Signed-off-by: Cao jin --- Documentation/x86/boot.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst index 7fafc7ac00d7..c04afec90486 100644 --- a/Documentation/x86/boot.rst +++ b/Documentation/x86/boot.rst @@ -1379,7 +1379,7 @@ can be calculated as follows:: In addition to read/modify/write the setup header of the struct boot_params as that of 16-bit boot protocol, the boot loader should also fill the additional fields of the struct boot_params as described -in zero-page.txt. +in zero-page.rst. After setting up the struct boot_params, the boot loader can load 64-bit kernel in the same way as that of 16-bit boot protocol, but @@ -1391,7 +1391,7 @@ In 64-bit boot protocol, the kernel is started by jumping to the At entry, the CPU must be in 64-bit mode with paging enabled. The range with setup_header.init_size from start address of loaded -kernel and zero page and command line buffer get ident mapping; +kernel and zero page and command line buffer get identity mapping; a GDT must be loaded with the descriptors for selectors __BOOT_CS(0x10) and __BOOT_DS(0x18); both descriptors must be 4G flat segment; __BOOT_CS must have execute/read permission, and __BOOT_DS -- 2.21.0
Re: [PATCH v2] perf: fix compilation failure on i386
BTW, this is reported by kernel test robot , so please help to
add:
Reported-by: kernel test robot
when it got merged.
--
Sincerely,
Cao jin
On 5/1/20 4:25 PM, Cao jin wrote:
> Compilation on i386 complains as following:
>
> util/session.c: In function 'perf_session__process_compressed_event':
> util/session.c:91:11: error: format '%ld' expects argument of type 'long
> int', but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
> pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
>^
>
> util/zstd.c: In function 'zstd_decompress_stream':
> util/zstd.c:102:11: error: format '%ld' expects argument of type 'long int',
> but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
> pr_err("failed to decompress (B): %ld -> %ld, dst_size %ld : %s\n",
>^
>
> Fix them by pairing "%zd" to size_t.
>
> Also revert an unnecessary conversion: "(long)src_size" to plain "src_size"
> with conversion specifier "%zd".
>
> Signed-off-by: Cao jin
> ---
> tools/perf/util/session.c | 2 +-
> tools/perf/util/zstd.c| 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 0b0bfe5bef17..50c2ffa388ad 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -88,7 +88,7 @@ static int perf_session__process_compressed_event(struct
> perf_session *session,
> session->decomp_last = decomp;
> }
>
> - pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
> + pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
>
> return 0;
> }
> diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c
> index d2202392ffdb..877bfb79e4af 100644
> --- a/tools/perf/util/zstd.c
> +++ b/tools/perf/util/zstd.c
> @@ -74,8 +74,8 @@ size_t zstd_compress_stream_to_records(struct zstd_data
> *data, void *dst, size_t
> ret = ZSTD_compressStream(data->cstream, , );
> ZSTD_flushStream(data->cstream, );
> if (ZSTD_isError(ret)) {
> - pr_err("failed to compress %ld bytes: %s\n",
> - (long)src_size, ZSTD_getErrorName(ret));
> + pr_err("failed to compress %zd bytes: %s\n",
> + src_size, ZSTD_getErrorName(ret));
> memcpy(dst, src, src_size);
> return src_size;
> }
> @@ -99,7 +99,7 @@ size_t zstd_decompress_stream(struct zstd_data *data, void
> *src, size_t src_size
> while (input.pos < input.size) {
> ret = ZSTD_decompressStream(data->dstream, , );
> if (ZSTD_isError(ret)) {
> - pr_err("failed to decompress (B): %ld -> %ld, dst_size
> %ld : %s\n",
> + pr_err("failed to decompress (B): %zd -> %zd, dst_size
> %zd : %s\n",
> src_size, output.size, dst_size,
> ZSTD_getErrorName(ret));
> break;
> }
>
[PATCH v2] perf: fix compilation failure on i386
Compilation on i386 complains as following:
util/session.c: In function 'perf_session__process_compressed_event':
util/session.c:91:11: error: format '%ld' expects argument of type 'long int',
but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
^
util/zstd.c: In function 'zstd_decompress_stream':
util/zstd.c:102:11: error: format '%ld' expects argument of type 'long int',
but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
pr_err("failed to decompress (B): %ld -> %ld, dst_size %ld : %s\n",
^
Fix them by pairing "%zd" to size_t.
Also revert an unnecessary conversion: "(long)src_size" to plain "src_size"
with conversion specifier "%zd".
Signed-off-by: Cao jin
---
tools/perf/util/session.c | 2 +-
tools/perf/util/zstd.c| 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 0b0bfe5bef17..50c2ffa388ad 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -88,7 +88,7 @@ static int perf_session__process_compressed_event(struct
perf_session *session,
session->decomp_last = decomp;
}
- pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
+ pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
return 0;
}
diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c
index d2202392ffdb..877bfb79e4af 100644
--- a/tools/perf/util/zstd.c
+++ b/tools/perf/util/zstd.c
@@ -74,8 +74,8 @@ size_t zstd_compress_stream_to_records(struct zstd_data
*data, void *dst, size_t
ret = ZSTD_compressStream(data->cstream, , );
ZSTD_flushStream(data->cstream, );
if (ZSTD_isError(ret)) {
- pr_err("failed to compress %ld bytes: %s\n",
- (long)src_size, ZSTD_getErrorName(ret));
+ pr_err("failed to compress %zd bytes: %s\n",
+ src_size, ZSTD_getErrorName(ret));
memcpy(dst, src, src_size);
return src_size;
}
@@ -99,7 +99,7 @@ size_t zstd_decompress_stream(struct zstd_data *data, void
*src, size_t src_size
while (input.pos < input.size) {
ret = ZSTD_decompressStream(data->dstream, , );
if (ZSTD_isError(ret)) {
- pr_err("failed to decompress (B): %ld -> %ld, dst_size
%ld : %s\n",
+ pr_err("failed to decompress (B): %zd -> %zd, dst_size
%zd : %s\n",
src_size, output.size, dst_size,
ZSTD_getErrorName(ret));
break;
}
--
2.21.0
Re: [PATCH] perf: fix compilation failure on i386
Sorry, the commit message isn't accurate. v2 coming.
--
Sincerely,
Cao jin
On 5/1/20 4:15 PM, Cao jin wrote:
> Compilation on i386 complains as following:
>
> util/session.c: In function 'perf_session__process_compressed_event':
> util/session.c:91:11: error: format '%ld' expects argument of type 'long
> int', but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
> pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
>^
>
> util/zstd.c: In function 'zstd_decompress_stream':
> util/zstd.c:102:11: error: format '%ld' expects argument of type 'long int',
> but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
> pr_err("failed to decompress (B): %ld -> %ld, dst_size %ld : %s\n",
>^
>
> util/zstd.c: In function 'zstd_compress_stream_to_records':
> util/zstd.c:77:11: error: format '%zd' expects argument of type 'signed
> size_t', but argument 4 has type 'long int' [-Werror=format=]
> pr_err("failed to compress %zd bytes: %s\n",
>^
>
> Fix them by pairing "%zd" to size_t.
>
> Signed-off-by: Cao jin
> ---
> tools/perf/util/session.c | 2 +-
> tools/perf/util/zstd.c| 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 0b0bfe5bef17..50c2ffa388ad 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -88,7 +88,7 @@ static int perf_session__process_compressed_event(struct
> perf_session *session,
> session->decomp_last = decomp;
> }
>
> - pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
> + pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
>
> return 0;
> }
> diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c
> index d2202392ffdb..877bfb79e4af 100644
> --- a/tools/perf/util/zstd.c
> +++ b/tools/perf/util/zstd.c
> @@ -74,8 +74,8 @@ size_t zstd_compress_stream_to_records(struct zstd_data
> *data, void *dst, size_t
> ret = ZSTD_compressStream(data->cstream, , );
> ZSTD_flushStream(data->cstream, );
> if (ZSTD_isError(ret)) {
> - pr_err("failed to compress %ld bytes: %s\n",
> - (long)src_size, ZSTD_getErrorName(ret));
> + pr_err("failed to compress %zd bytes: %s\n",
> + src_size, ZSTD_getErrorName(ret));
> memcpy(dst, src, src_size);
> return src_size;
> }
> @@ -99,7 +99,7 @@ size_t zstd_decompress_stream(struct zstd_data *data, void
> *src, size_t src_size
> while (input.pos < input.size) {
> ret = ZSTD_decompressStream(data->dstream, , );
> if (ZSTD_isError(ret)) {
> - pr_err("failed to decompress (B): %ld -> %ld, dst_size
> %ld : %s\n",
> + pr_err("failed to decompress (B): %zd -> %zd, dst_size
> %zd : %s\n",
> src_size, output.size, dst_size,
> ZSTD_getErrorName(ret));
> break;
> }
>
[PATCH] perf: fix compilation failure on i386
Compilation on i386 complains as following:
util/session.c: In function 'perf_session__process_compressed_event':
util/session.c:91:11: error: format '%ld' expects argument of type 'long int',
but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
^
util/zstd.c: In function 'zstd_decompress_stream':
util/zstd.c:102:11: error: format '%ld' expects argument of type 'long int',
but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
pr_err("failed to decompress (B): %ld -> %ld, dst_size %ld : %s\n",
^
util/zstd.c: In function 'zstd_compress_stream_to_records':
util/zstd.c:77:11: error: format '%zd' expects argument of type 'signed
size_t', but argument 4 has type 'long int' [-Werror=format=]
pr_err("failed to compress %zd bytes: %s\n",
^
Fix them by pairing "%zd" to size_t.
Signed-off-by: Cao jin
---
tools/perf/util/session.c | 2 +-
tools/perf/util/zstd.c| 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 0b0bfe5bef17..50c2ffa388ad 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -88,7 +88,7 @@ static int perf_session__process_compressed_event(struct
perf_session *session,
session->decomp_last = decomp;
}
- pr_debug("decomp (B): %ld to %ld\n", src_size, decomp_size);
+ pr_debug("decomp (B): %zd to %zd\n", src_size, decomp_size);
return 0;
}
diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c
index d2202392ffdb..877bfb79e4af 100644
--- a/tools/perf/util/zstd.c
+++ b/tools/perf/util/zstd.c
@@ -74,8 +74,8 @@ size_t zstd_compress_stream_to_records(struct zstd_data
*data, void *dst, size_t
ret = ZSTD_compressStream(data->cstream, , );
ZSTD_flushStream(data->cstream, );
if (ZSTD_isError(ret)) {
- pr_err("failed to compress %ld bytes: %s\n",
- (long)src_size, ZSTD_getErrorName(ret));
+ pr_err("failed to compress %zd bytes: %s\n",
+ src_size, ZSTD_getErrorName(ret));
memcpy(dst, src, src_size);
return src_size;
}
@@ -99,7 +99,7 @@ size_t zstd_decompress_stream(struct zstd_data *data, void
*src, size_t src_size
while (input.pos < input.size) {
ret = ZSTD_decompressStream(data->dstream, , );
if (ZSTD_isError(ret)) {
- pr_err("failed to decompress (B): %ld -> %ld, dst_size
%ld : %s\n",
+ pr_err("failed to decompress (B): %zd -> %zd, dst_size
%zd : %s\n",
src_size, output.size, dst_size,
ZSTD_getErrorName(ret));
break;
}
--
2.21.0
Re: [RFC PATCH] x86/doc/boot_protocol: Correct the description of "reloc"
On 9/26/19 2:01 PM, Ingo Molnar wrote:
> * Cao jin wrote:
>
>> The fields marked with (reloc) actually are not dedicated for writing,
>> but communicating info for relocatable kernel with boot loaders. For
>> example:
>>
>>
>> Field name: pref_address
>> Type: read (reloc)
>> Offset/size:0x258/8
>> Protocol: 2.10+
>>
>>
>>
>> Field name: code32_start
>> Type: modify (optional, reloc)
>> Offset/size:0x214/4
>> Protocol: 2.00+
>>
>>
>> Signed-off-by: Cao jin
>> ---
>> Unless I have incorrect non-native understanding for "fill in", I think
>> this is inaccurate.
>>
>> Documentation/x86/boot.rst | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>> index 08a2f100c0e6..a611bf04492d 100644
>> --- a/Documentation/x86/boot.rst
>> +++ b/Documentation/x86/boot.rst
>> @@ -243,7 +243,7 @@ bootloader ("modify").
>>
>> All general purpose boot loaders should write the fields marked
>> (obligatory). Boot loaders who want to load the kernel at a
>> -nonstandard address should fill in the fields marked (reloc); other
>> +nonstandard address should consult with the fields marked (reloc); other
>> boot loaders can ignore those fields.
>>
>> The byte order of all fields is littleendian (this is x86, after all.)
>
> Well, this documentation is written from the point of view of a
> *bootloader*, not the kernel. So the 'fill in' says that the bootloader
> should write those fields - which is correct, right?
>
Take pref_address or relocatable_kernel for example, they have type:
read (reloc), does boot loader need to write them? I don't see grub does
this at least.
--
Sincerely,
Cao jin
[PATCH] mm/memblock: cleanup doc
fix typos for: elaboarte -> elaborate architecure -> architecture compltes -> completes And, convert the markup :c:func:`foo` to foo() as kernel documentation toolchain can recognize foo() as a function. Suggested-by: Mike Rapoport Signed-off-by: Cao jin --- mm/memblock.c | 44 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/mm/memblock.c b/mm/memblock.c index 7d4f61ae666a..c23b370cc49e 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -57,42 +57,38 @@ * at build time. The region arrays for the "memory" and "reserved" * types are initially sized to %INIT_MEMBLOCK_REGIONS and for the * "physmap" type to %INIT_PHYSMEM_REGIONS. - * The :c:func:`memblock_allow_resize` enables automatic resizing of - * the region arrays during addition of new regions. This feature - * should be used with care so that memory allocated for the region - * array will not overlap with areas that should be reserved, for - * example initrd. + * The memblock_allow_resize() enables automatic resizing of the region + * arrays during addition of new regions. This feature should be used + * with care so that memory allocated for the region array will not + * overlap with areas that should be reserved, for example initrd. * * The early architecture setup should tell memblock what the physical - * memory layout is by using :c:func:`memblock_add` or - * :c:func:`memblock_add_node` functions. The first function does not - * assign the region to a NUMA node and it is appropriate for UMA - * systems. Yet, it is possible to use it on NUMA systems as well and - * assign the region to a NUMA node later in the setup process using - * :c:func:`memblock_set_node`. The :c:func:`memblock_add_node` - * performs such an assignment directly. + * memory layout is by using memblock_add() or memblock_add_node() + * functions. The first function does not assign the region to a NUMA + * node and it is appropriate for UMA systems. Yet, it is possible to + * use it on NUMA systems as well and assign the region to a NUMA node + * later in the setup process using memblock_set_node(). The + * memblock_add_node() performs such an assignment directly. * * Once memblock is setup the memory can be allocated using one of the * API variants: * - * * :c:func:`memblock_phys_alloc*` - these functions return the - * **physical** address of the allocated memory - * * :c:func:`memblock_alloc*` - these functions return the **virtual** - * address of the allocated memory. + * * memblock_phys_alloc*() - these functions return the **physical** + * address of the allocated memory + * * memblock_alloc*() - these functions return the **virtual** address + * of the allocated memory. * * Note, that both API variants use implict assumptions about allowed * memory ranges and the fallback methods. Consult the documentation - * of :c:func:`memblock_alloc_internal` and - * :c:func:`memblock_alloc_range_nid` functions for more elaboarte - * description. + * of memblock_alloc_internal() and memblock_alloc_range_nid() + * functions for more elaborate description. * - * As the system boot progresses, the architecture specific - * :c:func:`mem_init` function frees all the memory to the buddy page - * allocator. + * As the system boot progresses, the architecture specific mem_init() + * function frees all the memory to the buddy page allocator. * - * Unless an architecure enables %CONFIG_ARCH_KEEP_MEMBLOCK, the + * Unless an architecture enables %CONFIG_ARCH_KEEP_MEMBLOCK, the * memblock data structures will be discarded after the system - * initialization compltes. + * initialization completes. */ #ifndef CONFIG_NEED_MULTIPLE_NODES -- 2.21.0
Re: [PATCH] mm/memblock: fix typo in memblock doc
On 9/12/19 6:35 PM, Mike Rapoport wrote: > On Thu, Sep 12, 2019 at 10:54:09AM +0800, Cao jin wrote: >> On 9/11/19 10:42 PM, Mike Rapoport wrote: >> >> Sure. BTW, do you want convert all the markups too? >> >> :c:type:`foo` -> struct foo >> %FOO -> FOO >> ``foo`` -> foo >> **foo** -> foo > > The documentation toolchain can recognize now foo() as a function and does > not require the :c:func: prefix for that. AFAIK it only works for > functions, so please don't change the rest of the markup. > I see. Thanks for the info. Patch on the way. -- Sincerely, Cao jin
Re: [PATCH] mm/memblock: fix typo in memblock doc
On 9/11/19 10:42 PM, Mike Rapoport wrote: > On Wed, Sep 11, 2019 at 11:08:56AM +0800, Cao jin wrote: >> elaboarte -> elaborate >> architecure -> architecture >> compltes -> completes >> >> Signed-off-by: Cao jin >> --- >> mm/memblock.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/mm/memblock.c b/mm/memblock.c >> index 7d4f61ae666a..0d0f92003d18 100644 >> --- a/mm/memblock.c >> +++ b/mm/memblock.c >> @@ -83,16 +83,16 @@ >> * Note, that both API variants use implict assumptions about allowed >> * memory ranges and the fallback methods. Consult the documentation >> * of :c:func:`memblock_alloc_internal` and >> - * :c:func:`memblock_alloc_range_nid` functions for more elaboarte >> + * :c:func:`memblock_alloc_range_nid` functions for more elaborate > > While on it, could you please replace the > :c:func:`memblock_alloc_range_nid` construct with > memblock_alloc_range_nid()? > > And that would be really great to see all the :c:func:`foo` changed to > foo(). > Sure. BTW, do you want convert all the markups too? :c:type:`foo` -> struct foo %FOO -> FOO ``foo`` -> foo **foo** -> foo -- Sincerely, Cao jin
[PATCH] mm/memblock: fix typo in memblock doc
elaboarte -> elaborate architecure -> architecture compltes -> completes Signed-off-by: Cao jin --- mm/memblock.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mm/memblock.c b/mm/memblock.c index 7d4f61ae666a..0d0f92003d18 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -83,16 +83,16 @@ * Note, that both API variants use implict assumptions about allowed * memory ranges and the fallback methods. Consult the documentation * of :c:func:`memblock_alloc_internal` and - * :c:func:`memblock_alloc_range_nid` functions for more elaboarte + * :c:func:`memblock_alloc_range_nid` functions for more elaborate * description. * * As the system boot progresses, the architecture specific * :c:func:`mem_init` function frees all the memory to the buddy page * allocator. * - * Unless an architecure enables %CONFIG_ARCH_KEEP_MEMBLOCK, the + * Unless an architecture enables %CONFIG_ARCH_KEEP_MEMBLOCK, the * memblock data structures will be discarded after the system - * initialization compltes. + * initialization completes. */ #ifndef CONFIG_NEED_MULTIPLE_NODES -- 2.21.0
Re: [PATCH] x86/cpufeature: explicit comments for duplicate macro
On 8/28/19 2:42 PM, Borislav Petkov wrote: > On Wed, Aug 28, 2019 at 02:11:00PM +0800, Cao jin wrote: > > For the future: > >> Subject: Re: [PATCH] x86/cpufeature: explicit comments for duplicate macro > > your subject needs to have a verb and start with a capital letter after > the subsystem/path prefix. In this case, something like this, for > example: > > Subject: [PATCH] x86/cpufeature: Explain the macro duplication > Kept that in mind. Thanks very much! -- Sincerely, Cao jin
[tip: x86/cleanups] x86/cpufeature: Explain the macro duplication
The following commit has been merged into the x86/cleanups branch of tip:
Commit-ID: cbb1133b563a63901cf778220eb17e3ff1425aed
Gitweb:
https://git.kernel.org/tip/cbb1133b563a63901cf778220eb17e3ff1425aed
Author:Cao Jin
AuthorDate:Wed, 28 Aug 2019 14:11:00 +08:00
Committer: Borislav Petkov
CommitterDate: Wed, 28 Aug 2019 08:38:39 +02:00
x86/cpufeature: Explain the macro duplication
Explain the intent behind the duplication of the
BUILD_BUG_ON_ZERO(NCAPINTS != n)
check in *_MASK_CHECK and its immediate use in the *MASK_BIT_SET macros
too.
[ bp: Massage. ]
Suggested-by: Borislav Petkov
Signed-off-by: Cao Jin
Signed-off-by: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: "H. Peter Anvin"
Cc: Ingo Molnar
Cc: Jann Horn
Cc: Masahiro Yamada
Cc: Michael Ellerman
Cc: Nadav Amit
Cc: Thomas Gleixner
Cc: x86-ml
Link: https://lkml.kernel.org/r/20190828061100.27032-1-caoj.f...@cn.fujitsu.com
---
arch/x86/include/asm/cpufeature.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/include/asm/cpufeature.h
b/arch/x86/include/asm/cpufeature.h
index 58acda5..59bf91c 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -61,6 +61,13 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
#define CHECK_BIT_IN_MASK_WORD(maskname, word, bit)\
(((bit)>>5)==(word) && (1UL<<((bit)&31) & maskname##word ))
+/*
+ * {REQUIRED,DISABLED}_MASK_CHECK below may seem duplicated with the
+ * following BUILD_BUG_ON_ZERO() check but when NCAPINTS gets changed, all
+ * header macros which use NCAPINTS need to be changed. The duplicated macro
+ * use causes the compiler to issue errors for all headers so that all usage
+ * sites can be corrected.
+ */
#define REQUIRED_MASK_BIT_SET(feature_bit) \
( CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 0, feature_bit) ||\
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 1, feature_bit) ||\
[PATCH] x86/cpufeature: explicit comments for duplicate macro
Help people to understand the author's intent of apparent duplication of BUILD_BUG_ON_ZERO(NCAPINTS != n), which is hard to detect by eyes. CC: Dave Hansen Suggested-by: Borislav Petkov Signed-off-by: Cao jin --- Tried my best to describe it accurately, in case of any inaccuracy, feel free to rephrase. arch/x86/include/asm/cpufeature.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h index 58acda503817..e943174abf1e 100644 --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -61,6 +61,17 @@ extern const char * const x86_bug_flags[NBUGINTS*32]; #define CHECK_BIT_IN_MASK_WORD(maskname, word, bit)\ (((bit)>>5)==(word) && (1UL<<((bit)&31) & maskname##word )) +/* + * REQUIRED_MASK_CHECK may seems duplicate, but actually has its reason to + * live here. + * New CPUID leaf added or feature bit adjustment would/may result in increase + * in NCAPINTS. When it does, two required-features.h and here need to be + * modified correspondingly. BUILD_BUG_ON_ZERO assures the modification to be + * carried out, checking NCAPINTS also reminds the additional lines for new + * word. But, required-features.h as a single header file, can't be compiled + * directly, that is why a wrapper is defined there and called here. + * Totally the same case for DISABLED_MASK_BIT_SET. + */ #define REQUIRED_MASK_BIT_SET(feature_bit) \ ( CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 0, feature_bit) ||\ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 1, feature_bit) ||\ -- 2.17.0
Re: [PATCH] x86/cpufeature: drop *_MASK_CEHCK
On 8/28/19 1:20 AM, Borislav Petkov wrote: > On Tue, Aug 27, 2019 at 09:33:11AM -0700, Dave Hansen wrote: >> The point was that there are 5 files in the code that need to be changed >> if you change NCAPINTS: >> >> 1. arch/x86/include/asm/required-features.h >> 2. arch/x86/include/asm/disabled-features.h >> 3. tools/arch/x86/include/asm/disabled-features.h >> 4. tools/arch/x86/include/asm/required-features.h >> 5. arch/x86/include/asm/cpufeature.h (2 sites) >> >> Each of those sites has a compile-time check for NCAPINTS. The problem >> is that the *-features.h code doesn't get compiled directly so a >> BUILD_BUG_ON() doesn't work by itself. So, for the sites there, we put >> it somewhere that *will* get compiled: the macros that actually check >> the bits. >> >> It looks weird, but the end effect is good: If you change NCAPINTS, you >> get compile errors in 5 files and have to go edit those 5 files to fix >> it. Your patch makes it easier to introduce errors and miss one of >> those sites. > > ... and we wouldn't want to debug any strange bugs from missing a case. > So, Cao, I wouldn't mind having the gist of that above somewhere around > there in a comment explicitly. > A subtle issue hard to detect by eyes. Sure, on the way. -- Sincerely, Cao jin
[PATCH] x86/cpufeature: drop *_MASK_CEHCK
They are wrappers of BUILD_BUG_ON_ZERO(NCAPINTS != n), which is already present in corresponding *_MASK_BIT_SET. And fill the missing period in head comments by the way. Signed-off-by: Cao jin --- arch/x86/include/asm/cpufeature.h| 2 -- arch/x86/include/asm/disabled-features.h | 1 - arch/x86/include/asm/required-features.h | 3 +-- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h index 58acda503817..232ffb88039c 100644 --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -81,7 +81,6 @@ extern const char * const x86_bug_flags[NBUGINTS*32]; CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 16, feature_bit) ||\ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) ||\ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) ||\ - REQUIRED_MASK_CHECK||\ BUILD_BUG_ON_ZERO(NCAPINTS != 19)) #define DISABLED_MASK_BIT_SET(feature_bit) \ @@ -104,7 +103,6 @@ extern const char * const x86_bug_flags[NBUGINTS*32]; CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 16, feature_bit) ||\ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) ||\ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) ||\ - DISABLED_MASK_CHECK||\ BUILD_BUG_ON_ZERO(NCAPINTS != 19)) #define cpu_has(c, bit) \ diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h index a5ea841cc6d2..8a2eafa86739 100644 --- a/arch/x86/include/asm/disabled-features.h +++ b/arch/x86/include/asm/disabled-features.h @@ -84,6 +84,5 @@ #define DISABLED_MASK16 (DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP) #define DISABLED_MASK170 #define DISABLED_MASK180 -#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19) #endif /* _ASM_X86_DISABLED_FEATURES_H */ diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h index 6847d85400a8..cb98b66d3e81 100644 --- a/arch/x86/include/asm/required-features.h +++ b/arch/x86/include/asm/required-features.h @@ -1,7 +1,7 @@ #ifndef _ASM_X86_REQUIRED_FEATURES_H #define _ASM_X86_REQUIRED_FEATURES_H -/* Define minimum CPUID feature set for kernel These bits are checked +/* Define minimum CPUID feature set for kernel. These bits are checked really early to actually display a visible error message before the kernel dies. Make sure to assign features to the proper mask! @@ -101,6 +101,5 @@ #define REQUIRED_MASK160 #define REQUIRED_MASK170 #define REQUIRED_MASK180 -#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19) #endif /* _ASM_X86_REQUIRED_FEATURES_H */ -- 2.17.0
[tip: x86/mm] x86/fixmap: Cleanup outdated comments
The following commit has been merged into the x86/mm branch of tip:
Commit-ID: c84b82dd3e593db217f23c60f7edae02c76a3c4c
Gitweb:
https://git.kernel.org/tip/c84b82dd3e593db217f23c60f7edae02c76a3c4c
Author:Cao jin
AuthorDate:Fri, 09 Aug 2019 19:46:12 +08:00
Committer: Thomas Gleixner
CommitterDate: Mon, 19 Aug 2019 21:50:19 +02:00
x86/fixmap: Cleanup outdated comments
Remove stale comments and fix the not longer valid pagetable entry
reference.
Signed-off-by: Cao jin
Signed-off-by: Thomas Gleixner
Link: https://lkml.kernel.org/r/20190809114612.2569-1-caoj.f...@cn.fujitsu.com
---
arch/x86/include/asm/fixmap.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 9da8ccc..0c47aa8 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -42,8 +42,7 @@
* Because of this, FIXADDR_TOP x86 integration was left as later work.
*/
#ifdef CONFIG_X86_32
-/* used by vmalloc.c, vsyscall.lds.S.
- *
+/*
* Leave one empty page between vmalloc'ed areas and
* the start of the fixmap.
*/
@@ -120,7 +119,7 @@ enum fixed_addresses {
* before ioremap() is functional.
*
* If necessary we round it up to the next 512 pages boundary so
-* that we can have a single pgd entry and a single pte table:
+* that we can have a single pmd entry and a single pte table:
*/
#define NR_FIX_BTMAPS 64
#define FIX_BTMAPS_SLOTS 8
Re: [PATCH] x86/fixmap: update stale comments
Hi,
Wish to know whether the patch make sense.
On 8/9/19 7:46 PM, Cao jin wrote:
> Signed-off-by: Cao jin
> ---
> arch/x86/include/asm/fixmap.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
> index 9da8cccdf3fb..0c47aa82e2e2 100644
> --- a/arch/x86/include/asm/fixmap.h
> +++ b/arch/x86/include/asm/fixmap.h
> @@ -42,8 +42,7 @@
> * Because of this, FIXADDR_TOP x86 integration was left as later work.
> */
> #ifdef CONFIG_X86_32
> -/* used by vmalloc.c, vsyscall.lds.S.
> - *
> +/*
Not seeing variable __FIXADDR_TOP & macro FIXADDR_TOP under
CONFIG_X86_32 referred in vmalloc.c, and there is no vsyscall.lds.S now.
> * Leave one empty page between vmalloc'ed areas and
> * the start of the fixmap.
> */
> @@ -120,7 +119,7 @@ enum fixed_addresses {
>* before ioremap() is functional.
>*
>* If necessary we round it up to the next 512 pages boundary so
> - * that we can have a single pgd entry and a single pte table:
> + * that we can have a single pmd entry and a single pte table:
The comments seems missed to be updated in an ancient commit 551889a6e2a24
>*/
> #define NR_FIX_BTMAPS64
> #define FIX_BTMAPS_SLOTS 8
>
--
Sincerely,
Cao jin
[PATCH] x86/fixmap: update stale comments
Signed-off-by: Cao jin
---
arch/x86/include/asm/fixmap.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 9da8cccdf3fb..0c47aa82e2e2 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -42,8 +42,7 @@
* Because of this, FIXADDR_TOP x86 integration was left as later work.
*/
#ifdef CONFIG_X86_32
-/* used by vmalloc.c, vsyscall.lds.S.
- *
+/*
* Leave one empty page between vmalloc'ed areas and
* the start of the fixmap.
*/
@@ -120,7 +119,7 @@ enum fixed_addresses {
* before ioremap() is functional.
*
* If necessary we round it up to the next 512 pages boundary so
-* that we can have a single pgd entry and a single pte table:
+* that we can have a single pmd entry and a single pte table:
*/
#define NR_FIX_BTMAPS 64
#define FIX_BTMAPS_SLOTS 8
--
2.17.0
Re: [PATCH] x86/irq/64: fix the missing update on comment
On 7/22/19 4:53 PM, Thomas Gleixner wrote:
> Cao,
>
> On Fri, 19 Jul 2019, Cao jin wrote:
>
>> Commit e6401c130931 ("x86/irq/64: Split the IRQ stack into its own pages")
>> missed to update one piece of comment as it did to its peer in Xen, which
>> will confuse people who still need to read comment.
>>
>> A bonus fix to identation in ZO's linker script: spaces -> tab.
>
> Please don't add 'bonus' changes. A patch which fixes a stale comment has
> nothing to do with a indentation change in an unrelated file.
>
Kept that in mind. Sorry for the inconvenience.
--
Sincerely,
Cao jin
[tip:x86/urgent] x86/irq/64: Update stale comment
Commit-ID: 385065734cd417b9d7739b2ebb62c960aeb3ccb5
Gitweb: https://git.kernel.org/tip/385065734cd417b9d7739b2ebb62c960aeb3ccb5
Author: Cao jin
AuthorDate: Fri, 19 Jul 2019 16:16:35 +0800
Committer: Thomas Gleixner
CommitDate: Mon, 22 Jul 2019 10:54:27 +0200
x86/irq/64: Update stale comment
Commit e6401c130931 ("x86/irq/64: Split the IRQ stack into its own pages")
missed to update one piece of comment as it did to its peer in Xen, which
will confuse people who still need to read comment.
Signed-off-by: Cao jin
Signed-off-by: Thomas Gleixner
Link: https://lkml.kernel.org/r/20190719081635.26528-1-caoj.f...@cn.fujitsu.com
---
arch/x86/kernel/head_64.S | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index a6342c899be5..f3d3e9646a99 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -193,10 +193,10 @@ ENTRY(secondary_startup_64)
/* Set up %gs.
*
-* The base of %gs always points to the bottom of the irqstack
-* union. If the stack protector canary is enabled, it is
-* located at %gs:40. Note that, on SMP, the boot cpu uses
-* init data section till per cpu areas are set up.
+* The base of %gs always points to fixed_percpu_data. If the
+* stack protector canary is enabled, it is located at %gs:40.
+* Note that, on SMP, the boot cpu uses init data section until
+* the per cpu areas are set up.
*/
movl$MSR_GS_BASE,%ecx
movlinitial_gs(%rip),%eax
[PATCH] x86/irq/64: fix the missing update on comment
Commit e6401c130931 ("x86/irq/64: Split the IRQ stack into its own pages")
missed to update one piece of comment as it did to its peer in Xen, which
will confuse people who still need to read comment.
A bonus fix to identation in ZO's linker script: spaces -> tab.
Signed-off-by: Cao jin
---
arch/x86/boot/compressed/vmlinux.lds.S | 4 ++--
arch/x86/kernel/head_64.S | 8
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
b/arch/x86/boot/compressed/vmlinux.lds.S
index 508cfa6828c5..23100c52a7d0 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -64,8 +64,8 @@ SECTIONS
_ebss = .;
}
#ifdef CONFIG_X86_64
- . = ALIGN(PAGE_SIZE);
- .pgtable : {
+ . = ALIGN(PAGE_SIZE);
+ .pgtable : {
_pgtable = . ;
*(.pgtable)
_epgtable = . ;
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index bcd206c8ac90..cba94468795e 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -195,10 +195,10 @@ ENTRY(secondary_startup_64)
/* Set up %gs.
*
-* The base of %gs always points to the bottom of the irqstack
-* union. If the stack protector canary is enabled, it is
-* located at %gs:40. Note that, on SMP, the boot cpu uses
-* init data section till per cpu areas are set up.
+* The base of %gs always points to fixed_percpu_data. If the
+* stack protector canary is enabled, it is located at %gs:40.
+* Note that, on SMP, the boot cpu uses init data section till
+* per cpu areas are set up.
*/
movl$MSR_GS_BASE,%ecx
movlinitial_gs(%rip),%eax
--
2.17.0
[RFC PATCH] fcntl header: drop duplicate O_NDELAY
drop it from VALID_OPEN_FLAGS. Signed-off-by: Cao jin --- include/linux/fcntl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h index d019df946cb2..8a2ca434be6d 100644 --- a/include/linux/fcntl.h +++ b/include/linux/fcntl.h @@ -6,8 +6,8 @@ /* list of all valid flags for the open/openat flags argument: */ #define VALID_OPEN_FLAGS \ - (O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \ -O_APPEND | O_NDELAY | O_NONBLOCK | O_NDELAY | __O_SYNC | O_DSYNC | \ + (O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | \ +O_TRUNC | O_APPEND | O_NDELAY | O_NONBLOCK | __O_SYNC | O_DSYNC | \ FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \ O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE) -- 2.17.2
Re: [PATCH] x86/boot: minor improvement in kaslr
On 2/1/19 4:20 PM, Kees Cook wrote: > On Fri, Feb 1, 2019 at 6:51 PM Cao jin wrote: >> >> comments fix: input_size is ZO image size which just don't count .bss >> in, but has .text, .data, etc; >> drop unecessary alignment: minimum is either 512M or output, both are >> CONFIG_PHYSICAL_ALIGN aligned(output is aligned in head_32/64.S). But >> mention it in earlier comments. >> >> Signed-off-by: Cao jin >> --- >> arch/x86/boot/compressed/kaslr.c | 9 +++-- >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/boot/compressed/kaslr.c >> b/arch/x86/boot/compressed/kaslr.c >> index 9ed9709d9947..a947c5aba34e 100644 >> --- a/arch/x86/boot/compressed/kaslr.c >> +++ b/arch/x86/boot/compressed/kaslr.c >> @@ -360,7 +360,7 @@ static void handle_mem_options(void) >> * (i.e. it does not include its run size). This range must be avoided >> * because it contains the data used for decompression. >> * >> - * [input+input_size, output+init_size) is [_text, _end) for ZO. This >> + * [input+input_size, output+init_size) is [_bss, _end) for ZO. This > > This isn't right. The comment was correct before. See > arch/x86/boot/compressed/vmlinux.lds.S for the layout of the ZO image: > after the compressed image is _text, _rodata, _got, _data, _bss, > _pgtable, and _end. "[_text, _end)" correctly identifies the span > used. > Finally see why I am wrong here, I mixed up with the input_size & ZO image file size. Sorry for the noise, and thanks very much for your hint, Kees! -- Sincerely, Cao jin
Re: [PATCH] x86/boot: comments cleanup & indentation fix
On 1/4/19 8:04 PM, Cao jin wrote: > No real code modification, just cleanup: > 1. remove redundant comments which have already appeared above > 2. comments improvement: > "aligned to a 2M boundary" > --> > "aligned up to CONFIG_PHYSICAL_ALIGN boundary" Finally I see why I have inaccurate understanding in 2. Sorry for the noise. > 3. typo fix: uncompression --> decompression > 4. indentation fix in linker script: spaces -> tab > I will leave the other 3 alone, unless they are wanted. -- Sincerely, Cao jin
Re: [PATCH] x86/boot: minor improvement in kaslr
On 2/1/19 4:20 PM, Kees Cook wrote: > On Fri, Feb 1, 2019 at 6:51 PM Cao jin wrote: >> >> comments fix: input_size is ZO image size which just don't count .bss >> in, but has .text, .data, etc; >> drop unecessary alignment: minimum is either 512M or output, both are >> CONFIG_PHYSICAL_ALIGN aligned(output is aligned in head_32/64.S). But >> mention it in earlier comments. >> >> Signed-off-by: Cao jin >> --- >> arch/x86/boot/compressed/kaslr.c | 9 +++-- >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/boot/compressed/kaslr.c >> b/arch/x86/boot/compressed/kaslr.c >> index 9ed9709d9947..a947c5aba34e 100644 >> --- a/arch/x86/boot/compressed/kaslr.c >> +++ b/arch/x86/boot/compressed/kaslr.c >> @@ -360,7 +360,7 @@ static void handle_mem_options(void) >> * (i.e. it does not include its run size). This range must be avoided >> * because it contains the data used for decompression. >> * >> - * [input+input_size, output+init_size) is [_text, _end) for ZO. This >> + * [input+input_size, output+init_size) is [_bss, _end) for ZO. This > > This isn't right. The comment was correct before. See > arch/x86/boot/compressed/vmlinux.lds.S for the layout of the ZO image: > after the compressed image is _text, _rodata, _got, _data, _bss, > _pgtable, and _end. "[_text, _end)" correctly identifies the span > used. > I am confused, doesn't input_size = ZO image size = .head.text + .rodata..compressed + .rodata + .got + .data + .pgtable ? As I know, .bss don't occupy any space in file, and ZO's full run size is against the end of buffer, so I think the tiny gap here is just .bss, which is also the stack and heap. Do I get it wrong? >> * range includes ZO's heap and stack, and must be avoided since it >> * performs the decompression. >> * >> @@ -763,9 +763,6 @@ static unsigned long find_random_phys_addr(unsigned long >> minimum, >> return 0; >> } >> >> - /* Make sure minimum is aligned. */ >> - minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN); >> - > > I would prefer to keep this runtime calculation since it enforces the > requirement instead of making leaving it in a comment. When this goes > wrong, you get an unbootable kernel, which is very frustrating to > debug. > I find that I maybe wrong here. It is said that CONFIG_PHYSICAL_ALIGN must be a multiple of 0x20 on 64-bit, so it could be 2M, 4M, 6M, 8M, 12M, 14M, 16M, while 512M can't be divided by 6, 10, 12, 14 without remainder. -- Sincerely, Cao jin
[PATCH] x86/boot: minor improvement in kaslr
comments fix: input_size is ZO image size which just don't count .bss in, but has .text, .data, etc; drop unecessary alignment: minimum is either 512M or output, both are CONFIG_PHYSICAL_ALIGN aligned(output is aligned in head_32/64.S). But mention it in earlier comments. Signed-off-by: Cao jin --- arch/x86/boot/compressed/kaslr.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 9ed9709d9947..a947c5aba34e 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -360,7 +360,7 @@ static void handle_mem_options(void) * (i.e. it does not include its run size). This range must be avoided * because it contains the data used for decompression. * - * [input+input_size, output+init_size) is [_text, _end) for ZO. This + * [input+input_size, output+init_size) is [_bss, _end) for ZO. This * range includes ZO's heap and stack, and must be avoided since it * performs the decompression. * @@ -763,9 +763,6 @@ static unsigned long find_random_phys_addr(unsigned long minimum, return 0; } - /* Make sure minimum is aligned. */ - minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN); - if (process_efi_entries(minimum, image_size)) return slots_fetch_random(); @@ -831,8 +828,8 @@ void choose_random_location(unsigned long input, /* * Low end of the randomization range should be the -* smaller of 512M or the initial kernel image -* location: +* smaller of 512M or the initial kernel image location. +* Should be aligned to CONFIG_PHYSICAL_ALIGN. */ min_addr = min(*output, 512UL << 20); -- 2.17.0
Re: [PATCH] x86/boot: comments cleanup & indentation fix
Hi,
Is the patch not right?
--
Sincerely,
Cao jin
On 1/4/19 8:04 PM, Cao jin wrote:
> No real code modification, just cleanup:
> 1. remove redundant comments which have already appeared above
> 2. comments improvement:
> "aligned to a 2M boundary"
> -->
> "aligned up to CONFIG_PHYSICAL_ALIGN boundary"
> 3. typo fix: uncompression --> decompression
> 4. indentation fix in linker script: spaces -> tab
>
> Signed-off-by: Cao jin
> ---
> arch/x86/boot/compressed/head_64.S | 13 +
> arch/x86/boot/compressed/vmlinux.lds.S | 4 ++--
> 2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/head_64.S
> b/arch/x86/boot/compressed/head_64.S
> index 64037895b085..58f6a467f1fa 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -90,9 +90,6 @@ ENTRY(startup_32)
> jnz no_longmode
>
> /*
> - * Compute the delta between where we were compiled to run at
> - * and where the code will actually run at.
> - *
> * %ebp contains the address we are loaded at by the boot loader and %ebx
> * contains the address where we should move the kernel image temporarily
> * for safe in-place decompression.
> @@ -272,12 +269,12 @@ ENTRY(startup_64)
>
> /*
>* Compute the decompressed kernel start address. It is where
> - * we were loaded at aligned to a 2M boundary. %rbp contains the
> - * decompressed kernel start address.
> + * we were loaded at aligned up to CONFIG_PHYSICAL_ALIGN boundary.
> + * %rbp contains the decompressed kernel start address.
>*
>* If it is a relocatable kernel then decompress and run the kernel
> - * from load address aligned to 2MB addr, otherwise decompress and
> - * run the kernel from LOAD_PHYSICAL_ADDR
> + * from load address aligned up to CONFIG_PHYSICAL_ALIGN, otherwise
> + * decompress and run the kernel from LOAD_PHYSICAL_ADDR
>*
>* We cannot rely on the calculation done in 32-bit mode, since we
>* may have been invoked via the 64-bit entry point.
> @@ -525,7 +522,7 @@ relocated:
> */
> pushq %rsi/* Save the real mode argument */
> movq%rsi, %rdi /* real mode address */
> - leaqboot_heap(%rip), %rsi /* malloc area for uncompression */
> + leaqboot_heap(%rip), %rsi /* malloc area for decompression */
> leaqinput_data(%rip), %rdx /* input_data */
> movl$z_input_len, %ecx /* input_len */
> movq%rbp, %r8 /* output target address */
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
> b/arch/x86/boot/compressed/vmlinux.lds.S
> index f491bbde8493..c07c8aba0755 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -64,8 +64,8 @@ SECTIONS
> _ebss = .;
> }
> #ifdef CONFIG_X86_64
> - . = ALIGN(PAGE_SIZE);
> - .pgtable : {
> + . = ALIGN(PAGE_SIZE);
> + .pgtable : {
> _pgtable = . ;
> *(.pgtable)
> _epgtable = . ;
>
[tip:x86/cleanups] x86/boot: Save several bytes in decompressor
Commit-ID: 0a278662f53159354a0391a17741a20db736256a Gitweb: https://git.kernel.org/tip/0a278662f53159354a0391a17741a20db736256a Author: Cao jin AuthorDate: Wed, 23 Jan 2019 18:00:14 +0800 Committer: Thomas Gleixner CommitDate: Tue, 29 Jan 2019 22:09:12 +0100 x86/boot: Save several bytes in decompressor gdt64 represents the content of GDTR under x86-64, which actually needs 10 bytes only, ".long" & ".word" is superfluous. Signed-off-by: Cao jin Signed-off-by: Thomas Gleixner Acked-by: Kirill A. Shutemov Cc: Cc: Link: https://lkml.kernel.org/r/20190123100014.23721-1-caoj.f...@cn.fujitsu.com --- arch/x86/boot/compressed/head_64.S | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 64037895b085..b27b338d2f6d 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -645,8 +645,6 @@ no_longmode: .data gdt64: .word gdt_end - gdt - .long 0 - .word 0 .quad 0 gdt: .word gdt_end - gdt
[PATCH] x86/boot: save several bytes in decompressor
gdt64 represents the content of GDTR under x86-64, which actually needs 10 bytes only, ".long" & ".word" is superfluous. Signed-off-by: Cao jin --- arch/x86/boot/compressed/head_64.S | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 58f6a467f1fa..541358454f10 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -642,8 +642,6 @@ no_longmode: .data gdt64: .word gdt_end - gdt - .long 0 - .word 0 .quad 0 gdt: .word gdt_end - gdt -- 2.17.0
minimal version requirement for ld
Hi, (Not sure I am sending the right place) Documentation/process/changes.rst says the minimal version for binutils is 2.20. I am trying the binutils-2.20.51.0.2-5.48.el6.x86_64.rpm of Centos6 on Fedora28, and it give me: HOSTCC scripts/basic/fixdep /usr/bin/ld: unrecognized option '-plugin' /usr/bin/ld: use the --help option for usage information collect2: error: ld returned 1 exit status make[2]: *** [scripts/Makefile.host:90: scripts/basic/fixdep] Error 1 make[1]: *** [Makefile:464: scripts_basic] Error 2 make: *** No rule to make target 'include/config/auto.conf', needed by 'include/config/kernel.release'. Stop. make: *** Waiting for unfinished jobs HOSTCC scripts/basic/fixdep /usr/bin/ld: unrecognized option '-plugin' /usr/bin/ld: use the --help option for usage information collect2: error: ld returned 1 exit status make[1]: *** [scripts/Makefile.host:90: scripts/basic/fixdep] Error 1 make: *** [Makefile:464: scripts_basic] Error 2 Then I tried adding: HOSTCFLAGS_fixdep.o := -fno-use-linker-plugin in scripts/basic/Makefile, and it give me: HOSTCC scripts/basic/fixdep /usr/bin/ld: BFD version 2.20.51.0.2-5.48.el6 20100205 internal error, aborting at reloc.c line 443 in bfd_get_reloc_size /usr/bin/ld: Please report this bug. collect2: error: ld returned 1 exit status make[2]: *** [scripts/Makefile.host:90: scripts/basic/fixdep] Error 1 make[1]: *** [Makefile:464: scripts_basic] Error 2 make: *** No rule to make target 'include/config/auto.conf', needed by 'include/config/kernel.release'. Stop. Any idea on this problem? Or anyone has ever tried ld 2.20 successfully? -- Sincerely, Cao jin
Re: question about head_64.S
On 1/22/19 9:08 PM, Kirill A. Shutemov wrote: > On Tue, Jan 22, 2019 at 03:31:25PM +0800, Cao jin wrote: >> Hi, Kirll, >> >>> 2. >>> Why gdt64 has following definition?: >>> >>> gdt64: >>> .word gdt_end - gdt >>> .long 0 >>> .word 0 >>> .quad 0 >>> >>> obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes >>> long, so why not just: >>> >>> gdt64: >>> .word gdt_end - gdt >>> .quad 0 >>> >>> With above modification, it can boot. >>> >> >> Seems you introduced gdt64 code in commit beebaccd50, could you help >> with this question? > > Looks like you are right. I've got confused at some point. > > Could you prepare a patch? Sure. > >> And it also remind me of another question about adjust_got which is also >> introduced by you. Because I failed to construct a test environment with >> ld version less than 2.24 until now, so I wanna do a quick ask here: >> does it make sense to adjust GOT from the 4th entry of it? Because as I >> know, the first 3 entries are special one, which (I guess) will be not used. > > No. > > These 3 entries are reserved for a special symbols (like entry 0 for > _DYNAMIC). It means linker should not use these entries for normal > symbols, but it doesn't mean that they don't need to be adjusted during > the load. > Thanks for your info! BTW, could I know how you construct the test environment? I tried centos6, the GCC version is too old to compile; then tried fedora28 with binutils-2.20.51.0.2-5.48.el6.x86_64.rpm from centos6, ld reported errors; and then tried compiling binutils source with tag 2.23, stopped at configure phase:( -- Sincerely, Cao jin
Re: question about head_64.S
Hi, Kirll, On 1/15/19 7:45 PM, Cao jin wrote: > Hi, > I have been digging into this file for a while, and I still have 2 > questions unclear, hope to get your help. > > > 2. > Why gdt64 has following definition?: > > gdt64: > .word gdt_end - gdt > .long 0 > .word 0 > .quad 0 > > obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes > long, so why not just: > > gdt64: > .word gdt_end - gdt > .quad 0 > > With above modification, it can boot. > Seems you introduced gdt64 code in commit beebaccd50, could you help with this question? And it also remind me of another question about adjust_got which is also introduced by you. Because I failed to construct a test environment with ld version less than 2.24 until now, so I wanna do a quick ask here: does it make sense to adjust GOT from the 4th entry of it? Because as I know, the first 3 entries are special one, which (I guess) will be not used. -- Sincerely, Cao jin
Re: question about head_64.S
On 1/15/19 11:55 PM, Thomas Gleixner wrote:
> On Tue, 15 Jan 2019, Cao jin wrote:
>
>> Hi,
>> I have been digging into this file for a while, and I still have 2
>> questions unclear, hope to get your help.
>>
>> 1.
>> At the entry of startup_64, we set all the data segment registers to 0,
>> according to commit 08da5a2ca("x86_64: Early segment setup for VT"), it
>> is said to accelerate the decompression under VT. I don't know Intel VT,
>> but I did test under physical machine and virtual machine(with KVM, and
>> intel VT enabled in BIOS) with following patch:
>>
>> diff --git a/arch/x86/boot/compressed/head_64.S
>> b/arch/x86/boot/compressed/head_64.S
>> index 58f6a467f1fa..595f3c300173 100644
>> --- a/arch/x86/boot/compressed/head_64.S
>> +++ b/arch/x86/boot/compressed/head_64.S
>> @@ -260,12 +260,12 @@ ENTRY(startup_64)
>> */
>>
>> /* Setup data segments. */
>> - xorl%eax, %eax
>> - movl%eax, %ds
>> - movl%eax, %es
>> - movl%eax, %ss
>> - movl%eax, %fs
>> - movl%eax, %gs
>> +// xorl%eax, %eax
>> +// movl%eax, %ds
>> +// movl%eax, %es
>> +// movl%eax, %ss
>> +// movl%eax, %fs
>> +// movl%eax, %gs
>>
>> I don't see any obvious booting time difference, is there anything I missed?
>> Also, I don't find explicit document saying we should zero these
>> registers under VT.
>
> The decompressor is position independent code, so all segments have to be
> set to 0.
>
Thank you Thomas! But I've never heard that PIC is correlated with
segment register value, could you elaborate a little bit? Because as I
know, startup_64 is in long mode, and CPU will treat all segment(except
fs, gs) base as 0, no matter whatever in them. And until now, I only see
fs is touched when parsing command line, not see any explicit gs usage.
On the other hand, I test the patch above, it can boot up, so seems
segment register value here is not necessary to be 0?
> The patch you mentioned was just adding fs/gs to the list of segments
> which are cleared and the commit message is not very clear. Though if you
> dig further down then you find the original version of that patch:
>
> commit ffb6017563aa("[PATCH] x86-64: x86_64 - Fix FS/GS registers for VT
> execution")
>
> That one has a proper explantaion.
>
Oh, I still not reach to the real kernel itself yet. At first glance,
"but it is important to reload them in protected mode" make sense to me.
But more confusion rise up: under 64-bit boot protocol, we can have more
than one entry? startup_64 in both:
arch/x86/kernel/head_64.S
and
arch/x86/boot/compressed/head_64.S
can be jumped to via bootloader? Seems Documentation/x86/boot.txt
doesn't say that.
--
Sincerely,
Cao jin
question about head_64.S
Hi,
I have been digging into this file for a while, and I still have 2
questions unclear, hope to get your help.
1.
At the entry of startup_64, we set all the data segment registers to 0,
according to commit 08da5a2ca("x86_64: Early segment setup for VT"), it
is said to accelerate the decompression under VT. I don't know Intel VT,
but I did test under physical machine and virtual machine(with KVM, and
intel VT enabled in BIOS) with following patch:
diff --git a/arch/x86/boot/compressed/head_64.S
b/arch/x86/boot/compressed/head_64.S
index 58f6a467f1fa..595f3c300173 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -260,12 +260,12 @@ ENTRY(startup_64)
*/
/* Setup data segments. */
- xorl%eax, %eax
- movl%eax, %ds
- movl%eax, %es
- movl%eax, %ss
- movl%eax, %fs
- movl%eax, %gs
+// xorl%eax, %eax
+// movl%eax, %ds
+// movl%eax, %es
+// movl%eax, %ss
+// movl%eax, %fs
+// movl%eax, %gs
I don't see any obvious booting time difference, is there anything I missed?
Also, I don't find explicit document saying we should zero these
registers under VT.
2.
Why gdt64 has following definition?:
gdt64:
.word gdt_end - gdt
.long 0
.word 0
.quad 0
obviously, gdt64 stores the GDTR content under x86_64, which is 10 bytes
long, so why not just:
gdt64:
.word gdt_end - gdt
.quad 0
With above modification, it can boot.
--
Sincerely,
Cao jin
Re: [PATCH] x86/boot: drop memset from copy.S
On 1/8/19 4:46 PM, Cao jin wrote: > One more question: in compressed/, for mem*(), it seems we both use the > macros of boot/string.h, and the functions of compressed/string.c. Is > that what we want? > > compressed/ is compiled with -O2, so it cannot be told by objdump -d, > but still can be confirmed by nm <*.o>, for example: > > $nm arch/x86/boot/compressed/eboot.o > U memcpy > U memset > > $nm arch/x86/boot/compressed/pgtable_64.o > # No entry of mem*() > > both of eboot.c and pgtable_64.c #include "../string.h", and use some of > mem*(), it is counter-intuitive to me. Very appreciate it someone can > leave a hint. > Well, I think HPA's previous answer is also suitable for this question, with -O2, sometimes __builtin_mem*() is optimized as inline code, while sometimes just emit a call to corresponding self-defined mem*() functions. -- Sincerely, Cao jin
Re: [PATCH] x86/boot: drop memset from copy.S
Hello HPA,
On 1/8/19 4:38 PM, h...@zytor.com wrote:
> On January 7, 2019 12:52:57 AM PST, Cao jin wrote:
>> Hi,
>>
>> On 1/7/19 3:59 PM, h...@zytor.com wrote:
>>> On January 6, 2019 11:40:56 PM PST, Cao jin
>> wrote:
>>>> According to objdump output of setup, function memset is not used in
>>>> setup code. Currently, all usage of memset in setup come from macro
>>>> definition of string.h.
>>>>
>>>> Signed-off-by: Cao jin
>>>> ---
>>>> Compiled and booted under x86_64; compiled under i386.
>>>>
>>>> Questions: now there is 2 definition of memcpy, one lies in copy.S,
>>>> another lies in string.h which is mapped to gcc builtin function. Do
>> we
>>>> still need 2 definition? Could we move the content of copy.S to
>>>> boot/string.c?
>>>>
>>>> At first glance, the usage of string.{c.h} of setup is kind of
>>>> confusing,
>>>> they are also used in compressed/ and purgatory/
>>>>
>>>> arch/x86/boot/copy.S | 15 ---
>>>> 1 file changed, 15 deletions(-)
>>>>
>>>> diff --git a/arch/x86/boot/copy.S b/arch/x86/boot/copy.S
>>>> index 15d9f74b0008..5157d08b0ff2 100644
>>>> --- a/arch/x86/boot/copy.S
>>>> +++ b/arch/x86/boot/copy.S
>>>> @@ -33,21 +33,6 @@ GLOBAL(memcpy)
>>>>retl
>>>> ENDPROC(memcpy)
>>>>
>>>> -GLOBAL(memset)
>>>> - pushw %di
>>>> - movw%ax, %di
>>>> - movzbl %dl, %eax
>>>> - imull $0x01010101,%eax
>>>> - pushw %cx
>>>> - shrw$2, %cx
>>>> - rep; stosl
>>>> - popw%cx
>>>> - andw$3, %cx
>>>> - rep; stosb
>>>> - popw%di
>>>> - retl
>>>> -ENDPROC(memset)
>>>> -
>>>> GLOBAL(copy_from_fs)
>>>>pushw %ds
>>>>pushw %fs
>>>
>>> This is dependent on both gcc version and flags.
>>>
>>
>> Thanks for your info, but I still don't quite get your point. All files
>> who has memset reference in setup will also #include "string.h", so how
>> gcc version and flags will associate memset reference to the assembly
>> function by force? Hope to get a little more details when you are
>> convenient:)
>
> GCC will sometimes emit calls to certain functions including memcpy().
>
Thanks very much. After spending some time on GCC document, I think you
are talking about a condition that, for example, __builtin_memcpy() is
not optimized as inline code, but a function call to memcpy() in copy.S.
But I failed to find out the details how would gcc version & flags make
it this way, even I checked out the .cmd file of these .c. Or is this
born to be obscure for programmers?
--
Sincerely,
Cao jin
Re: [PATCH] x86/boot: drop memset from copy.S
One more question: in compressed/, for mem*(), it seems we both use the
macros of boot/string.h, and the functions of compressed/string.c. Is
that what we want?
compressed/ is compiled with -O2, so it cannot be told by objdump -d,
but still can be confirmed by nm <*.o>, for example:
$nm arch/x86/boot/compressed/eboot.o
U memcpy
U memset
$nm arch/x86/boot/compressed/pgtable_64.o
# No entry of mem*()
both of eboot.c and pgtable_64.c #include "../string.h", and use some of
mem*(), it is counter-intuitive to me. Very appreciate it someone can
leave a hint.
--
Sincerely,
Cao jin
On 1/7/19 3:40 PM, Cao jin wrote:
> According to objdump output of setup, function memset is not used in
> setup code. Currently, all usage of memset in setup come from macro
> definition of string.h.
>
> Signed-off-by: Cao jin
> ---
> Compiled and booted under x86_64; compiled under i386.
>
> Questions: now there is 2 definition of memcpy, one lies in copy.S,
> another lies in string.h which is mapped to gcc builtin function. Do we
> still need 2 definition? Could we move the content of copy.S to
> boot/string.c?
>
> At first glance, the usage of string.{c.h} of setup is kind of confusing,
> they are also used in compressed/ and purgatory/
>
> arch/x86/boot/copy.S | 15 ---
> 1 file changed, 15 deletions(-)
>
> diff --git a/arch/x86/boot/copy.S b/arch/x86/boot/copy.S
> index 15d9f74b0008..5157d08b0ff2 100644
> --- a/arch/x86/boot/copy.S
> +++ b/arch/x86/boot/copy.S
> @@ -33,21 +33,6 @@ GLOBAL(memcpy)
> retl
> ENDPROC(memcpy)
>
> -GLOBAL(memset)
> - pushw %di
> - movw%ax, %di
> - movzbl %dl, %eax
> - imull $0x01010101,%eax
> - pushw %cx
> - shrw$2, %cx
> - rep; stosl
> - popw%cx
> - andw$3, %cx
> - rep; stosb
> - popw%di
> - retl
> -ENDPROC(memset)
> -
> GLOBAL(copy_from_fs)
> pushw %ds
> pushw %fs
>
Re: [PATCH] x86/boot: drop memset from copy.S
Hi,
On 1/7/19 3:59 PM, h...@zytor.com wrote:
> On January 6, 2019 11:40:56 PM PST, Cao jin wrote:
>> According to objdump output of setup, function memset is not used in
>> setup code. Currently, all usage of memset in setup come from macro
>> definition of string.h.
>>
>> Signed-off-by: Cao jin
>> ---
>> Compiled and booted under x86_64; compiled under i386.
>>
>> Questions: now there is 2 definition of memcpy, one lies in copy.S,
>> another lies in string.h which is mapped to gcc builtin function. Do we
>> still need 2 definition? Could we move the content of copy.S to
>> boot/string.c?
>>
>> At first glance, the usage of string.{c.h} of setup is kind of
>> confusing,
>> they are also used in compressed/ and purgatory/
>>
>> arch/x86/boot/copy.S | 15 ---
>> 1 file changed, 15 deletions(-)
>>
>> diff --git a/arch/x86/boot/copy.S b/arch/x86/boot/copy.S
>> index 15d9f74b0008..5157d08b0ff2 100644
>> --- a/arch/x86/boot/copy.S
>> +++ b/arch/x86/boot/copy.S
>> @@ -33,21 +33,6 @@ GLOBAL(memcpy)
>> retl
>> ENDPROC(memcpy)
>>
>> -GLOBAL(memset)
>> -pushw %di
>> -movw%ax, %di
>> -movzbl %dl, %eax
>> -imull $0x01010101,%eax
>> -pushw %cx
>> -shrw$2, %cx
>> -rep; stosl
>> -popw%cx
>> -andw$3, %cx
>> -rep; stosb
>> -popw%di
>> -retl
>> -ENDPROC(memset)
>> -
>> GLOBAL(copy_from_fs)
>> pushw %ds
>> pushw %fs
>
> This is dependent on both gcc version and flags.
>
Thanks for your info, but I still don't quite get your point. All files
who has memset reference in setup will also #include "string.h", so how
gcc version and flags will associate memset reference to the assembly
function by force? Hope to get a little more details when you are
convenient:)
[PATCH] x86/boot: drop memset from copy.S
According to objdump output of setup, function memset is not used in
setup code. Currently, all usage of memset in setup come from macro
definition of string.h.
Signed-off-by: Cao jin
---
Compiled and booted under x86_64; compiled under i386.
Questions: now there is 2 definition of memcpy, one lies in copy.S,
another lies in string.h which is mapped to gcc builtin function. Do we
still need 2 definition? Could we move the content of copy.S to
boot/string.c?
At first glance, the usage of string.{c.h} of setup is kind of confusing,
they are also used in compressed/ and purgatory/
arch/x86/boot/copy.S | 15 ---
1 file changed, 15 deletions(-)
diff --git a/arch/x86/boot/copy.S b/arch/x86/boot/copy.S
index 15d9f74b0008..5157d08b0ff2 100644
--- a/arch/x86/boot/copy.S
+++ b/arch/x86/boot/copy.S
@@ -33,21 +33,6 @@ GLOBAL(memcpy)
retl
ENDPROC(memcpy)
-GLOBAL(memset)
- pushw %di
- movw%ax, %di
- movzbl %dl, %eax
- imull $0x01010101,%eax
- pushw %cx
- shrw$2, %cx
- rep; stosl
- popw%cx
- andw$3, %cx
- rep; stosb
- popw%di
- retl
-ENDPROC(memset)
-
GLOBAL(copy_from_fs)
pushw %ds
pushw %fs
--
2.17.0
[PATCH] x86/boot: comments cleanup & indentation fix
No real code modification, just cleanup:
1. remove redundant comments which have already appeared above
2. comments improvement:
"aligned to a 2M boundary"
-->
"aligned up to CONFIG_PHYSICAL_ALIGN boundary"
3. typo fix: uncompression --> decompression
4. indentation fix in linker script: spaces -> tab
Signed-off-by: Cao jin
---
arch/x86/boot/compressed/head_64.S | 13 +
arch/x86/boot/compressed/vmlinux.lds.S | 4 ++--
2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/x86/boot/compressed/head_64.S
b/arch/x86/boot/compressed/head_64.S
index 64037895b085..58f6a467f1fa 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -90,9 +90,6 @@ ENTRY(startup_32)
jnz no_longmode
/*
- * Compute the delta between where we were compiled to run at
- * and where the code will actually run at.
- *
* %ebp contains the address we are loaded at by the boot loader and %ebx
* contains the address where we should move the kernel image temporarily
* for safe in-place decompression.
@@ -272,12 +269,12 @@ ENTRY(startup_64)
/*
* Compute the decompressed kernel start address. It is where
-* we were loaded at aligned to a 2M boundary. %rbp contains the
-* decompressed kernel start address.
+* we were loaded at aligned up to CONFIG_PHYSICAL_ALIGN boundary.
+* %rbp contains the decompressed kernel start address.
*
* If it is a relocatable kernel then decompress and run the kernel
-* from load address aligned to 2MB addr, otherwise decompress and
-* run the kernel from LOAD_PHYSICAL_ADDR
+* from load address aligned up to CONFIG_PHYSICAL_ALIGN, otherwise
+* decompress and run the kernel from LOAD_PHYSICAL_ADDR
*
* We cannot rely on the calculation done in 32-bit mode, since we
* may have been invoked via the 64-bit entry point.
@@ -525,7 +522,7 @@ relocated:
*/
pushq %rsi/* Save the real mode argument */
movq%rsi, %rdi /* real mode address */
- leaqboot_heap(%rip), %rsi /* malloc area for uncompression */
+ leaqboot_heap(%rip), %rsi /* malloc area for decompression */
leaqinput_data(%rip), %rdx /* input_data */
movl$z_input_len, %ecx /* input_len */
movq%rbp, %r8 /* output target address */
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S
b/arch/x86/boot/compressed/vmlinux.lds.S
index f491bbde8493..c07c8aba0755 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -64,8 +64,8 @@ SECTIONS
_ebss = .;
}
#ifdef CONFIG_X86_64
- . = ALIGN(PAGE_SIZE);
- .pgtable : {
+ . = ALIGN(PAGE_SIZE);
+ .pgtable : {
_pgtable = . ;
*(.pgtable)
_epgtable = . ;
--
2.17.0
question about reserve_bios_regions
Hi,
I am looking into these code via find_trampoline_placement, and I have
some questions still not clear after lots of investigation, hope to get
some hints.
1. How is the value of BIOS_START_MIN(128k) determined? I don't find any
clue about it until now.
2. BIOS_START_MAX is define as 0x9f000U(636k), but both the comments say
640k, is that a quirk or just typo? I thought the 4k difference is the
quirk, but later I guess the real quirk lies in latter judgment of :
if (ebda_start >= BIOS_START_MIN && ebda_start < bios_start)
I have checked out commit edce21216a8: ("x86/boot: Reorganize and clean
up the BIOS area reservation code"), still no clue.
--
Sincerely,
Cao jin
Re: [PATCH] x86/mkpiggy: Drop endianness transforming
On 11/9/18 9:00 PM, Thomas Gleixner wrote: > Cao, > > On Fri, 9 Nov 2018, Cao jin wrote: > >> gzip file has 4-byte little-endian file size encoded at the end of file, >> while all the other compressed kernel file has size_append operation in >> the Makefile which also append the 4-byte little-endian file size. There >> is no need to do endianness transforming by mkpiggy. > > Did you test that on a BE host cross-compiling the kernel for x86? > Oh, I don't know and never think of there is this kind of condition, thanks for your info and sorry for the noise. Sincerely, Cao jin
Re: [PATCH] x86/mkpiggy: Drop endianness transforming
On 11/9/18 9:00 PM, Thomas Gleixner wrote: > Cao, > > On Fri, 9 Nov 2018, Cao jin wrote: > >> gzip file has 4-byte little-endian file size encoded at the end of file, >> while all the other compressed kernel file has size_append operation in >> the Makefile which also append the 4-byte little-endian file size. There >> is no need to do endianness transforming by mkpiggy. > > Did you test that on a BE host cross-compiling the kernel for x86? > Oh, I don't know and never think of there is this kind of condition, thanks for your info and sorry for the noise. Sincerely, Cao jin
[PATCH] x86/mkpiggy: Drop endianness transforming
gzip file has 4-byte little-endian file size encoded at the end of file,
while all the other compressed kernel file has size_append operation in
the Makefile which also append the 4-byte little-endian file size. There
is no need to do endianness transforming by mkpiggy.
Signed-off-by: Cao jin
---
A simple printf debug line before transformation also shows they are the
same among 6 compression.
arch/x86/boot/compressed/Makefile | 1 -
arch/x86/boot/compressed/mkpiggy.c | 2 --
2 files changed, 3 deletions(-)
diff --git a/arch/x86/boot/compressed/Makefile
b/arch/x86/boot/compressed/Makefile
index 466f66c8a7f8..8b2bcfd65920 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -58,7 +58,6 @@ endif
LDFLAGS_vmlinux := -T
hostprogs-y:= mkpiggy
-HOST_EXTRACFLAGS += -I$(srctree)/tools/include
sed-voffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW]
\(_text\|__bss_start\|_end\)$$/\#define VO_\2 _AC(0x\1,UL)/p'
diff --git a/arch/x86/boot/compressed/mkpiggy.c
b/arch/x86/boot/compressed/mkpiggy.c
index 72bad2c8debe..d5402fcd844a 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -28,7 +28,6 @@
#include
#include
#include
-#include
int main(int argc, char *argv[])
{
@@ -61,7 +60,6 @@ int main(int argc, char *argv[])
}
ilen = ftell(f);
- olen = get_unaligned_le32();
printf(".section \".rodata..compressed\",\"a\",@progbits\n");
printf(".globl z_input_len\n");
--
2.17.0
[PATCH] x86/mkpiggy: Drop endianness transforming
gzip file has 4-byte little-endian file size encoded at the end of file,
while all the other compressed kernel file has size_append operation in
the Makefile which also append the 4-byte little-endian file size. There
is no need to do endianness transforming by mkpiggy.
Signed-off-by: Cao jin
---
A simple printf debug line before transformation also shows they are the
same among 6 compression.
arch/x86/boot/compressed/Makefile | 1 -
arch/x86/boot/compressed/mkpiggy.c | 2 --
2 files changed, 3 deletions(-)
diff --git a/arch/x86/boot/compressed/Makefile
b/arch/x86/boot/compressed/Makefile
index 466f66c8a7f8..8b2bcfd65920 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -58,7 +58,6 @@ endif
LDFLAGS_vmlinux := -T
hostprogs-y:= mkpiggy
-HOST_EXTRACFLAGS += -I$(srctree)/tools/include
sed-voffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW]
\(_text\|__bss_start\|_end\)$$/\#define VO_\2 _AC(0x\1,UL)/p'
diff --git a/arch/x86/boot/compressed/mkpiggy.c
b/arch/x86/boot/compressed/mkpiggy.c
index 72bad2c8debe..d5402fcd844a 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -28,7 +28,6 @@
#include
#include
#include
-#include
int main(int argc, char *argv[])
{
@@ -61,7 +60,6 @@ int main(int argc, char *argv[])
}
ilen = ftell(f);
- olen = get_unaligned_le32();
printf(".section \".rodata..compressed\",\"a\",@progbits\n");
printf(".globl z_input_len\n");
--
2.17.0
Re: kconfig usage in automatic kernel test
On 07/08/2018 10:15 AM, Masahiro Yamada wrote: > 2018-07-06 17:49 GMT+09:00 Cao jin : >> Masahiro-san, >> >> I am writing some utility for internal kdump test with latest kernel, >> my purpose is to test the new introduced kernel feature. For automatical >> test, I see several config target could help, like olddefconfig, all*config. >> >> But for my purpose, I don't find a good way. For example, olddefconfig >> will let the now config item has default value, while some feature may >> default to "N"; allyesconfig will slow the compilation notably. >> But "all*config" has KCONFIG_ALLCONFIG help to customizing some config >> item, that is a good utility, but seems it can't be used in olddefconfig. >> >> All these things let me have 2 questions: >> >> 1. What would you suggest for my purpose? > > > scripts/kconfig/merge_config.sh > can be used with any *config target. > I took a quick glance at the script, it seems a good method to me. Thanks very much, Masahiro-san. > If you want to tweak some symbols based on olddefconfig, > this could be the one you want. > > >> 2. allyesconfig, allmodconfig, randconfig seems useful for test kbuild, >> but what's the purpose of allnoconfig, alldefconfig? In others words, >> when people would need allnoconfig, alldefconfig? > > I sometimes use allnoconfig for build testing. > > When I want to test the whole build process quickly, > I disable as many drivers as possible to save time. > > > I do not use alldefconfig. > Anyway, it would not hurt to have it for completeness. > Thanks for clarifying. -- Sincerely, Cao jin
Re: kconfig usage in automatic kernel test
On 07/08/2018 10:15 AM, Masahiro Yamada wrote: > 2018-07-06 17:49 GMT+09:00 Cao jin : >> Masahiro-san, >> >> I am writing some utility for internal kdump test with latest kernel, >> my purpose is to test the new introduced kernel feature. For automatical >> test, I see several config target could help, like olddefconfig, all*config. >> >> But for my purpose, I don't find a good way. For example, olddefconfig >> will let the now config item has default value, while some feature may >> default to "N"; allyesconfig will slow the compilation notably. >> But "all*config" has KCONFIG_ALLCONFIG help to customizing some config >> item, that is a good utility, but seems it can't be used in olddefconfig. >> >> All these things let me have 2 questions: >> >> 1. What would you suggest for my purpose? > > > scripts/kconfig/merge_config.sh > can be used with any *config target. > I took a quick glance at the script, it seems a good method to me. Thanks very much, Masahiro-san. > If you want to tweak some symbols based on olddefconfig, > this could be the one you want. > > >> 2. allyesconfig, allmodconfig, randconfig seems useful for test kbuild, >> but what's the purpose of allnoconfig, alldefconfig? In others words, >> when people would need allnoconfig, alldefconfig? > > I sometimes use allnoconfig for build testing. > > When I want to test the whole build process quickly, > I disable as many drivers as possible to save time. > > > I do not use alldefconfig. > Anyway, it would not hurt to have it for completeness. > Thanks for clarifying. -- Sincerely, Cao jin
Re: [PATCH] kbuild: document the KBUILD_KCONFIG env. variable
On 07/05/2018 10:47 AM, Randy Dunlap wrote: > From: Randy Dunlap > > Add usage info for the Kbuild environment variable KBUILD_KCONFIG. > > Signed-off-by: Randy Dunlap I can recall this variable does take me a little while to find out its meaning during my earliest time in kbuild, so, Acked-by: Cao jin -- Sincerely, Cao jin > --- > Documentation/kbuild/kbuild.txt |5 + > 1 file changed, 5 insertions(+) > > --- lnx-418-rc3.orig/Documentation/kbuild/kbuild.txt > +++ lnx-418-rc3/Documentation/kbuild/kbuild.txt > @@ -50,6 +50,11 @@ LDFLAGS_MODULE > -- > Additional options used for $(LD) when linking modules. > > +KBUILD_KCONFIG > +-- > +Set the top-level Kconfig file to the value of this environment > +variable. The default name is "Kconfig". > + > KBUILD_VERBOSE > -- > Set the kbuild verbosity. Can be assigned same values as "V=...". > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > >
Re: [PATCH] kbuild: document the KBUILD_KCONFIG env. variable
On 07/05/2018 10:47 AM, Randy Dunlap wrote: > From: Randy Dunlap > > Add usage info for the Kbuild environment variable KBUILD_KCONFIG. > > Signed-off-by: Randy Dunlap I can recall this variable does take me a little while to find out its meaning during my earliest time in kbuild, so, Acked-by: Cao jin -- Sincerely, Cao jin > --- > Documentation/kbuild/kbuild.txt |5 + > 1 file changed, 5 insertions(+) > > --- lnx-418-rc3.orig/Documentation/kbuild/kbuild.txt > +++ lnx-418-rc3/Documentation/kbuild/kbuild.txt > @@ -50,6 +50,11 @@ LDFLAGS_MODULE > -- > Additional options used for $(LD) when linking modules. > > +KBUILD_KCONFIG > +-- > +Set the top-level Kconfig file to the value of this environment > +variable. The default name is "Kconfig". > + > KBUILD_VERBOSE > -- > Set the kbuild verbosity. Can be assigned same values as "V=...". > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > >
kconfig usage in automatic kernel test
Masahiro-san, I am writing some utility for internal kdump test with latest kernel, my purpose is to test the new introduced kernel feature. For automatical test, I see several config target could help, like olddefconfig, all*config. But for my purpose, I don't find a good way. For example, olddefconfig will let the now config item has default value, while some feature may default to "N"; allyesconfig will slow the compilation notably. But "all*config" has KCONFIG_ALLCONFIG help to customizing some config item, that is a good utility, but seems it can't be used in olddefconfig. All these things let me have 2 questions: 1. What would you suggest for my purpose? 2. allyesconfig, allmodconfig, randconfig seems useful for test kbuild, but what's the purpose of allnoconfig, alldefconfig? In others words, when people would need allnoconfig, alldefconfig? -- Sincerely, Cao jin
kconfig usage in automatic kernel test
Masahiro-san, I am writing some utility for internal kdump test with latest kernel, my purpose is to test the new introduced kernel feature. For automatical test, I see several config target could help, like olddefconfig, all*config. But for my purpose, I don't find a good way. For example, olddefconfig will let the now config item has default value, while some feature may default to "N"; allyesconfig will slow the compilation notably. But "all*config" has KCONFIG_ALLCONFIG help to customizing some config item, that is a good utility, but seems it can't be used in olddefconfig. All these things let me have 2 questions: 1. What would you suggest for my purpose? 2. allyesconfig, allmodconfig, randconfig seems useful for test kbuild, but what's the purpose of allnoconfig, alldefconfig? In others words, when people would need allnoconfig, alldefconfig? -- Sincerely, Cao jin
Re: [PATCH] x86/tools/relocs: comments cleanup
ping?
On 03/05/2018 03:15 PM, Cao jin wrote:
> 1. typo fix: there -> their
> 2. remove superfluous "by"
>
> Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
> ---
> arch/x86/tools/relocs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index 5d73c443e778..fcf3fad01b08 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -37,7 +37,7 @@ static struct section *secs;
>
> static const char * const sym_regex_kernel[S_NSYMTYPES] = {
> /*
> - * Following symbols have been audited. There values are constant and do
> + * Following symbols have been audited. Their values are constant and do
> * not change if bzImage is loaded at a different physical address than
> * the address for which it has been compiled. Don't warn user about
> * absolute relocations present w.r.t these symbols.
> @@ -690,7 +690,7 @@ static void walk_relocs(int (*process)(struct section
> *sec, Elf_Rel *rel,
> * to the start of the per_cpu area that does not change).
> *
> * Relocations that apply to the per_cpu area need to have their
> - * offset adjusted by by the value of __per_cpu_load to make them
> + * offset adjusted by the value of __per_cpu_load to make them
> * point to the correct place in the loaded image (because the
> * virtual address of .data..percpu is 0).
> *
>
--
Sincerely,
Cao jin
Re: [PATCH] x86/tools/relocs: comments cleanup
ping?
On 03/05/2018 03:15 PM, Cao jin wrote:
> 1. typo fix: there -> their
> 2. remove superfluous "by"
>
> Signed-off-by: Cao jin
> ---
> arch/x86/tools/relocs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index 5d73c443e778..fcf3fad01b08 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -37,7 +37,7 @@ static struct section *secs;
>
> static const char * const sym_regex_kernel[S_NSYMTYPES] = {
> /*
> - * Following symbols have been audited. There values are constant and do
> + * Following symbols have been audited. Their values are constant and do
> * not change if bzImage is loaded at a different physical address than
> * the address for which it has been compiled. Don't warn user about
> * absolute relocations present w.r.t these symbols.
> @@ -690,7 +690,7 @@ static void walk_relocs(int (*process)(struct section
> *sec, Elf_Rel *rel,
> * to the start of the per_cpu area that does not change).
> *
> * Relocations that apply to the per_cpu area need to have their
> - * offset adjusted by by the value of __per_cpu_load to make them
> + * offset adjusted by the value of __per_cpu_load to make them
> * point to the correct place in the loaded image (because the
> * virtual address of .data..percpu is 0).
> *
>
--
Sincerely,
Cao jin
[tip:x86/build] x86/build: Don't pass in -D__KERNEL__ multiple times
Commit-ID: d6289f36aa7d5893d091a7a0c67eee7798719f03 Gitweb: https://git.kernel.org/tip/d6289f36aa7d5893d091a7a0c67eee7798719f03 Author: Cao jin <caoj.f...@cn.fujitsu.com> AuthorDate: Fri, 16 Mar 2018 16:49:44 +0800 Committer: Ingo Molnar <mi...@kernel.org> CommitDate: Sat, 31 Mar 2018 08:02:07 +0200 x86/build: Don't pass in -D__KERNEL__ multiple times Some ..cmd files under arch/x86 are showing two instances of -D__KERNEL__, like arch/x86/boot/ and arch/x86/realmode/rm/. __KERNEL__ is already defined in KBUILD_CPPFLAGS in the top Makefile, so it can be dropped safely. Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> Cc: H. Peter Anvin <h...@zytor.com> Cc: Linus Torvalds <torva...@linux-foundation.org> Cc: Masahiro Yamada <yamada.masah...@socionext.com> Cc: Michal Marek <michal.l...@markovi.net> Cc: Peter Zijlstra <pet...@infradead.org> Cc: Thomas Gleixner <t...@linutronix.de> Cc: linux-kbu...@vger.kernel.org Link: http://lkml.kernel.org/r/20180316084944.3997-1-caoj.f...@cn.fujitsu.com Signed-off-by: Ingo Molnar <mi...@kernel.org> --- arch/x86/Makefile | 3 +-- arch/x86/boot/compressed/Makefile | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/Makefile b/arch/x86/Makefile index d798e36d103c..a517852dad55 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -31,8 +31,7 @@ endif CODE16GCC_CFLAGS := -m32 -Wa,$(srctree)/arch/x86/boot/code16gcc.h M16_CFLAGS := $(call cc-option, -m16, $(CODE16GCC_CFLAGS)) -REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \ - -DDISABLE_BRANCH_PROFILING \ +REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -DDISABLE_BRANCH_PROFILING \ -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \ -fno-strict-aliasing -fomit-frame-pointer -fno-pic \ -mno-mmx -mno-sse diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index f25e1530e064..f484ae0ece93 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -26,7 +26,7 @@ KCOV_INSTRUMENT := n targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \ vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 +KBUILD_CFLAGS := -m$(BITS) -O2 KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING cflags-$(CONFIG_X86_32) := -march=i386
[tip:x86/build] x86/build: Don't pass in -D__KERNEL__ multiple times
Commit-ID: d6289f36aa7d5893d091a7a0c67eee7798719f03 Gitweb: https://git.kernel.org/tip/d6289f36aa7d5893d091a7a0c67eee7798719f03 Author: Cao jin AuthorDate: Fri, 16 Mar 2018 16:49:44 +0800 Committer: Ingo Molnar CommitDate: Sat, 31 Mar 2018 08:02:07 +0200 x86/build: Don't pass in -D__KERNEL__ multiple times Some ..cmd files under arch/x86 are showing two instances of -D__KERNEL__, like arch/x86/boot/ and arch/x86/realmode/rm/. __KERNEL__ is already defined in KBUILD_CPPFLAGS in the top Makefile, so it can be dropped safely. Signed-off-by: Cao jin Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Masahiro Yamada Cc: Michal Marek Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: linux-kbu...@vger.kernel.org Link: http://lkml.kernel.org/r/20180316084944.3997-1-caoj.f...@cn.fujitsu.com Signed-off-by: Ingo Molnar --- arch/x86/Makefile | 3 +-- arch/x86/boot/compressed/Makefile | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/Makefile b/arch/x86/Makefile index d798e36d103c..a517852dad55 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -31,8 +31,7 @@ endif CODE16GCC_CFLAGS := -m32 -Wa,$(srctree)/arch/x86/boot/code16gcc.h M16_CFLAGS := $(call cc-option, -m16, $(CODE16GCC_CFLAGS)) -REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \ - -DDISABLE_BRANCH_PROFILING \ +REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -DDISABLE_BRANCH_PROFILING \ -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \ -fno-strict-aliasing -fomit-frame-pointer -fno-pic \ -mno-mmx -mno-sse diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index f25e1530e064..f484ae0ece93 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -26,7 +26,7 @@ KCOV_INSTRUMENT := n targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \ vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 +KBUILD_CFLAGS := -m$(BITS) -O2 KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING cflags-$(CONFIG_X86_32) := -march=i386
Re: [PATCH] x86/build: drop a superfluous macro
ping? On 03/16/2018 04:49 PM, Cao jin wrote: > Some ..cmd files under arch/x86 report -D__KERNEL__ appears two > times in the command line, like arch/x86/boot, arch/x86/realmode/rm. > It is already defined in KBUILD_CPPFLAGS from top Makefile. so it can be > dropped saftely from these Makefiles. > > Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> > --- > arch/x86/Makefile | 3 +-- > arch/x86/boot/compressed/Makefile | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > index a20eacd9c7e9..592489a15d94 100644 > --- a/arch/x86/Makefile > +++ b/arch/x86/Makefile > @@ -31,8 +31,7 @@ endif > CODE16GCC_CFLAGS := -m32 -Wa,$(srctree)/arch/x86/boot/code16gcc.h > M16_CFLAGS:= $(call cc-option, -m16, $(CODE16GCC_CFLAGS)) > > -REALMODE_CFLAGS := $(M16_CFLAGS) -g -Os -D__KERNEL__ \ > --DDISABLE_BRANCH_PROFILING \ > +REALMODE_CFLAGS := $(M16_CFLAGS) -g -Os -DDISABLE_BRANCH_PROFILING \ > -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \ > -fno-strict-aliasing -fomit-frame-pointer -fno-pic \ > -mno-mmx -mno-sse > diff --git a/arch/x86/boot/compressed/Makefile > b/arch/x86/boot/compressed/Makefile > index 4b7575b00563..78e3400c22d7 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -26,7 +26,7 @@ KCOV_INSTRUMENT := n > targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 > vmlinux.bin.lzma \ > vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 > > -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 > +KBUILD_CFLAGS := -m$(BITS) -O2 > KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) > KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING > cflags-$(CONFIG_X86_32) := -march=i386 > -- Sincerely, Cao jin
Re: [PATCH] x86/build: drop a superfluous macro
ping? On 03/16/2018 04:49 PM, Cao jin wrote: > Some ..cmd files under arch/x86 report -D__KERNEL__ appears two > times in the command line, like arch/x86/boot, arch/x86/realmode/rm. > It is already defined in KBUILD_CPPFLAGS from top Makefile. so it can be > dropped saftely from these Makefiles. > > Signed-off-by: Cao jin > --- > arch/x86/Makefile | 3 +-- > arch/x86/boot/compressed/Makefile | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > index a20eacd9c7e9..592489a15d94 100644 > --- a/arch/x86/Makefile > +++ b/arch/x86/Makefile > @@ -31,8 +31,7 @@ endif > CODE16GCC_CFLAGS := -m32 -Wa,$(srctree)/arch/x86/boot/code16gcc.h > M16_CFLAGS:= $(call cc-option, -m16, $(CODE16GCC_CFLAGS)) > > -REALMODE_CFLAGS := $(M16_CFLAGS) -g -Os -D__KERNEL__ \ > --DDISABLE_BRANCH_PROFILING \ > +REALMODE_CFLAGS := $(M16_CFLAGS) -g -Os -DDISABLE_BRANCH_PROFILING \ > -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \ > -fno-strict-aliasing -fomit-frame-pointer -fno-pic \ > -mno-mmx -mno-sse > diff --git a/arch/x86/boot/compressed/Makefile > b/arch/x86/boot/compressed/Makefile > index 4b7575b00563..78e3400c22d7 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -26,7 +26,7 @@ KCOV_INSTRUMENT := n > targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 > vmlinux.bin.lzma \ > vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 > > -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 > +KBUILD_CFLAGS := -m$(BITS) -O2 > KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) > KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING > cflags-$(CONFIG_X86_32) := -march=i386 > -- Sincerely, Cao jin
Re: questions about header.S
Thanks very much for you hint! On 03/21/2018 05:57 PM, Thomas Gleixner wrote: > On Wed, 21 Mar 2018, Cao jin wrote: >> On 03/17/2018 06:01 PM, Cao jin wrote: >>> I find two small questions which confuse me a little. >>> >>> 1. >>> # Check signature at end of setup >>>cmpl$0x5a5aaa55, setup_sig >>>jne setup_bad >>> >>> setup_sig is defined in setup.ld, which points to the constant also >>> defined in setup.ld, so I don't figure out in which case they don't >>> equal and jump to setup_bad? > > That's a lame sanity check to make sure that nothing overwrote the loader. > I see. >>> In my test, drop these 2 lines seems fine, system can boot without any >>> obvious error. > > Sure it does as long as you have no corruption. > >>> 2. >>> # Zero the bss >>> movw$__bss_start, %di >>> movw$_end+3, %cx >>> xorl%eax, %eax >>> subw%di, %cx >>> shrw$2, %cx >>> rep; stosl >>> >>> It is not a big deal, but I think replace "_end" with "__bss_end" make >>> more sense, and "_end" is already aligned to word length. And, there is >>> no other code use symbol "__bss_end". So I don't know is there any >>> reason to use "_end" here? > > It doesn't matter at all. But its also pointless to change it. > It does not matter and is pointless to change for kernel itself. It just may confuse a little for newbies who has interests. -- Sincerely, Cao jin
Re: questions about header.S
Thanks very much for you hint! On 03/21/2018 05:57 PM, Thomas Gleixner wrote: > On Wed, 21 Mar 2018, Cao jin wrote: >> On 03/17/2018 06:01 PM, Cao jin wrote: >>> I find two small questions which confuse me a little. >>> >>> 1. >>> # Check signature at end of setup >>>cmpl$0x5a5aaa55, setup_sig >>>jne setup_bad >>> >>> setup_sig is defined in setup.ld, which points to the constant also >>> defined in setup.ld, so I don't figure out in which case they don't >>> equal and jump to setup_bad? > > That's a lame sanity check to make sure that nothing overwrote the loader. > I see. >>> In my test, drop these 2 lines seems fine, system can boot without any >>> obvious error. > > Sure it does as long as you have no corruption. > >>> 2. >>> # Zero the bss >>> movw$__bss_start, %di >>> movw$_end+3, %cx >>> xorl%eax, %eax >>> subw%di, %cx >>> shrw$2, %cx >>> rep; stosl >>> >>> It is not a big deal, but I think replace "_end" with "__bss_end" make >>> more sense, and "_end" is already aligned to word length. And, there is >>> no other code use symbol "__bss_end". So I don't know is there any >>> reason to use "_end" here? > > It doesn't matter at all. But its also pointless to change it. > It does not matter and is pointless to change for kernel itself. It just may confuse a little for newbies who has interests. -- Sincerely, Cao jin
Re: questions about header.S
Dear Maintainers, Could you help to give a hint? Thanks in advance. -- Sincerely, Cao jin On 03/17/2018 06:01 PM, Cao jin wrote: > Hi, > > I find two small questions which confuse me a little. > > 1. > # Check signature at end of setup >cmpl$0x5a5aaa55, setup_sig >jne setup_bad > > setup_sig is defined in setup.ld, which points to the constant also > defined in setup.ld, so I don't figure out in which case they don't > equal and jump to setup_bad? > > In my test, drop these 2 lines seems fine, system can boot without any > obvious error. > > 2. > # Zero the bss > movw$__bss_start, %di > movw$_end+3, %cx > xorl%eax, %eax > subw%di, %cx > shrw$2, %cx > rep; stosl > > It is not a big deal, but I think replace "_end" with "__bss_end" make > more sense, and "_end" is already aligned to word length. And, there is > no other code use symbol "__bss_end". So I don't know is there any > reason to use "_end" here? > > In my test, replace the 2nd line with: > > movw$_end, %cx > > or: > > movw$__bss_end+3, %cx > > are both fine. >
Re: questions about header.S
Dear Maintainers, Could you help to give a hint? Thanks in advance. -- Sincerely, Cao jin On 03/17/2018 06:01 PM, Cao jin wrote: > Hi, > > I find two small questions which confuse me a little. > > 1. > # Check signature at end of setup >cmpl$0x5a5aaa55, setup_sig >jne setup_bad > > setup_sig is defined in setup.ld, which points to the constant also > defined in setup.ld, so I don't figure out in which case they don't > equal and jump to setup_bad? > > In my test, drop these 2 lines seems fine, system can boot without any > obvious error. > > 2. > # Zero the bss > movw$__bss_start, %di > movw$_end+3, %cx > xorl%eax, %eax > subw%di, %cx > shrw$2, %cx > rep; stosl > > It is not a big deal, but I think replace "_end" with "__bss_end" make > more sense, and "_end" is already aligned to word length. And, there is > no other code use symbol "__bss_end". So I don't know is there any > reason to use "_end" here? > > In my test, replace the 2nd line with: > > movw$_end, %cx > > or: > > movw$__bss_end+3, %cx > > are both fine. >
questions about header.S
Hi, I find two small questions which confuse me a little. 1. # Check signature at end of setup cmpl$0x5a5aaa55, setup_sig jne setup_bad setup_sig is defined in setup.ld, which points to the constant also defined in setup.ld, so I don't figure out in which case they don't equal and jump to setup_bad? In my test, drop these 2 lines seems fine, system can boot without any obvious error. 2. # Zero the bss movw$__bss_start, %di movw$_end+3, %cx xorl%eax, %eax subw%di, %cx shrw$2, %cx rep; stosl It is not a big deal, but I think replace "_end" with "__bss_end" make more sense, and "_end" is already aligned to word length. And, there is no other code use symbol "__bss_end". So I don't know is there any reason to use "_end" here? In my test, replace the 2nd line with: movw$_end, %cx or: movw$__bss_end+3, %cx are both fine. -- Sincerely, Cao jin
questions about header.S
Hi, I find two small questions which confuse me a little. 1. # Check signature at end of setup cmpl$0x5a5aaa55, setup_sig jne setup_bad setup_sig is defined in setup.ld, which points to the constant also defined in setup.ld, so I don't figure out in which case they don't equal and jump to setup_bad? In my test, drop these 2 lines seems fine, system can boot without any obvious error. 2. # Zero the bss movw$__bss_start, %di movw$_end+3, %cx xorl%eax, %eax subw%di, %cx shrw$2, %cx rep; stosl It is not a big deal, but I think replace "_end" with "__bss_end" make more sense, and "_end" is already aligned to word length. And, there is no other code use symbol "__bss_end". So I don't know is there any reason to use "_end" here? In my test, replace the 2nd line with: movw$_end, %cx or: movw$__bss_end+3, %cx are both fine. -- Sincerely, Cao jin
[PATCH] x86/build: drop a superfluous macro
Some ..cmd files under arch/x86 report -D__KERNEL__ appears two times in the command line, like arch/x86/boot, arch/x86/realmode/rm. It is already defined in KBUILD_CPPFLAGS from top Makefile. so it can be dropped saftely from these Makefiles. Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> --- arch/x86/Makefile | 3 +-- arch/x86/boot/compressed/Makefile | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/Makefile b/arch/x86/Makefile index a20eacd9c7e9..592489a15d94 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -31,8 +31,7 @@ endif CODE16GCC_CFLAGS := -m32 -Wa,$(srctree)/arch/x86/boot/code16gcc.h M16_CFLAGS := $(call cc-option, -m16, $(CODE16GCC_CFLAGS)) -REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \ - -DDISABLE_BRANCH_PROFILING \ +REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -DDISABLE_BRANCH_PROFILING \ -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \ -fno-strict-aliasing -fomit-frame-pointer -fno-pic \ -mno-mmx -mno-sse diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 4b7575b00563..78e3400c22d7 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -26,7 +26,7 @@ KCOV_INSTRUMENT := n targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \ vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 +KBUILD_CFLAGS := -m$(BITS) -O2 KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING cflags-$(CONFIG_X86_32) := -march=i386 -- 2.14.3
[PATCH] x86/build: drop a superfluous macro
Some ..cmd files under arch/x86 report -D__KERNEL__ appears two times in the command line, like arch/x86/boot, arch/x86/realmode/rm. It is already defined in KBUILD_CPPFLAGS from top Makefile. so it can be dropped saftely from these Makefiles. Signed-off-by: Cao jin --- arch/x86/Makefile | 3 +-- arch/x86/boot/compressed/Makefile | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/Makefile b/arch/x86/Makefile index a20eacd9c7e9..592489a15d94 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -31,8 +31,7 @@ endif CODE16GCC_CFLAGS := -m32 -Wa,$(srctree)/arch/x86/boot/code16gcc.h M16_CFLAGS := $(call cc-option, -m16, $(CODE16GCC_CFLAGS)) -REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -D__KERNEL__ \ - -DDISABLE_BRANCH_PROFILING \ +REALMODE_CFLAGS:= $(M16_CFLAGS) -g -Os -DDISABLE_BRANCH_PROFILING \ -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \ -fno-strict-aliasing -fomit-frame-pointer -fno-pic \ -mno-mmx -mno-sse diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 4b7575b00563..78e3400c22d7 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -26,7 +26,7 @@ KCOV_INSTRUMENT := n targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \ vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4 -KBUILD_CFLAGS := -m$(BITS) -D__KERNEL__ -O2 +KBUILD_CFLAGS := -m$(BITS) -O2 KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC) KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING cflags-$(CONFIG_X86_32) := -march=i386 -- 2.14.3
Re: [PATCH 0/5] kbuild: always define KBUILD_MODNAME, and clean-up/fix modname
The series build successfully on upstream in my: make allyesconfig and allmodconfig, so, Tested-by: Cao jin <caoj.f...@cn.fujitsu.com> -- Sincerely, Cao jin On 03/08/2018 09:04 AM, Masahiro Yamada wrote: > > 3/5 takes into account '-m' case for multi-used-m. > > 2/5 is necessary beforehand because 3/5 would cause a build error for > drivers/net/ethernet/cavium/liquidio/lio_ethtool.c > > 1, 4, 5 are just clean-ups. > > > > Cao jin (1): > kbuild: fix modname for composite modules > > Masahiro Yamada (4): > kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi > kbuild: define KBUILD_MODNAME even if multiple modules share objects > kbuild: simplify modname calculation > kbuild: move modname and modname-multi close to modname_flags > > scripts/Makefile.build | 12 > scripts/Makefile.lib | 22 +- > 2 files changed, 9 insertions(+), 25 deletions(-) >
Re: [PATCH 0/5] kbuild: always define KBUILD_MODNAME, and clean-up/fix modname
The series build successfully on upstream in my: make allyesconfig and allmodconfig, so, Tested-by: Cao jin -- Sincerely, Cao jin On 03/08/2018 09:04 AM, Masahiro Yamada wrote: > > 3/5 takes into account '-m' case for multi-used-m. > > 2/5 is necessary beforehand because 3/5 would cause a build error for > drivers/net/ethernet/cavium/liquidio/lio_ethtool.c > > 1, 4, 5 are just clean-ups. > > > > Cao jin (1): > kbuild: fix modname for composite modules > > Masahiro Yamada (4): > kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi > kbuild: define KBUILD_MODNAME even if multiple modules share objects > kbuild: simplify modname calculation > kbuild: move modname and modname-multi close to modname_flags > > scripts/Makefile.build | 12 > scripts/Makefile.lib | 22 +- > 2 files changed, 9 insertions(+), 25 deletions(-) >
Re: [PATCH 5/5] kbuild: move modname and modname-multi close to modname_flags
On 03/08/2018 09:05 AM, Masahiro Yamada wrote: > Just a cosmetic change to put related code close together. > > Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com> Reviewed-by: Cao jin <caoj.f...@cn.fujitsu.com> -- Sincerely, Cao jin
Re: [PATCH 5/5] kbuild: move modname and modname-multi close to modname_flags
On 03/08/2018 09:05 AM, Masahiro Yamada wrote: > Just a cosmetic change to put related code close together. > > Signed-off-by: Masahiro Yamada Reviewed-by: Cao jin -- Sincerely, Cao jin
Re: [PATCH 4/5] kbuild: simplify modname calculation
On 03/08/2018 09:05 AM, Masahiro Yamada wrote: > modname can be calculated much more simply. If modname-multi is > empty, it is a single-used object. So, modname = $(basetarget).> Otherwise, > modname = $(modname-multi). > > Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com> > --- > > scripts/Makefile.build | 12 +--- > scripts/Makefile.lib | 7 --- > 2 files changed, 1 insertion(+), 18 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 4f2b25d..b8aecb7 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -131,17 +131,7 @@ $(real-objs-m:.o=.lst): quiet_modtag := [M] > > $(obj-m) : quiet_modtag := [M] > > -# Default for not multi-part modules > -modname = $(basetarget) > - > -$(multi-objs-m) : modname = $(modname-multi) > -$(multi-objs-m:.o=.i) : modname = $(modname-multi) > -$(multi-objs-m:.o=.s) : modname = $(modname-multi) > -$(multi-objs-m:.o=.lst) : modname = $(modname-multi) > -$(multi-objs-y) : modname = $(modname-multi) > -$(multi-objs-y:.o=.i) : modname = $(modname-multi) > -$(multi-objs-y:.o=.s) : modname = $(modname-multi) > -$(multi-objs-y:.o=.lst) : modname = $(modname-multi) > +modname = $(if $(modname-multi),$(modname-multi),$(basetarget)) > > quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ > cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 9217d40..e4e8e1b 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -47,11 +47,6 @@ multi-used-m := $(sort $(foreach m,$(obj-m), $(if $(strip > $($(m:.o=-objs)) $($(m > multi-used := $(multi-used-y) $(multi-used-m) > single-used-m := $(sort $(filter-out $(multi-used-m),$(obj-m))) > > -# Build list of the parts of our composite objects, our composite > -# objects depend on those (obviously) > -multi-objs-y := $(foreach m, $(multi-used-y), $($(m:.o=-objs)) $($(m:.o=-y))) > -multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)) $($(m:.o=-y)) > $($(m:.o=-m))) > - > # $(subdir-obj-y) is the list of objects in $(obj-y) which uses dir/ to > # tell kbuild to descend > subdir-obj-y := $(filter %/built-in.o, $(obj-y)) > @@ -80,8 +75,6 @@ real-objs-m := $(addprefix $(obj)/,$(real-objs-m)) > single-used-m:= $(addprefix $(obj)/,$(single-used-m)) > multi-used-y := $(addprefix $(obj)/,$(multi-used-y)) > multi-used-m := $(addprefix $(obj)/,$(multi-used-m)) > -multi-objs-y := $(addprefix $(obj)/,$(multi-objs-y)) > -multi-objs-m := $(addprefix $(obj)/,$(multi-objs-m)) > subdir-ym:= $(addprefix $(obj)/,$(subdir-ym)) > > # These flags are needed for modversions and compiling, so we define them > here > multi-objs-y/m seems only exist for rules above with target-specific variable assignment. A great simplification, So Reviewed-by: Cao jin <caoj.f...@cn.fujitsu.com> -- Sincerely, Cao jin
Re: [PATCH 4/5] kbuild: simplify modname calculation
On 03/08/2018 09:05 AM, Masahiro Yamada wrote: > modname can be calculated much more simply. If modname-multi is > empty, it is a single-used object. So, modname = $(basetarget).> Otherwise, > modname = $(modname-multi). > > Signed-off-by: Masahiro Yamada > --- > > scripts/Makefile.build | 12 +--- > scripts/Makefile.lib | 7 --- > 2 files changed, 1 insertion(+), 18 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 4f2b25d..b8aecb7 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -131,17 +131,7 @@ $(real-objs-m:.o=.lst): quiet_modtag := [M] > > $(obj-m) : quiet_modtag := [M] > > -# Default for not multi-part modules > -modname = $(basetarget) > - > -$(multi-objs-m) : modname = $(modname-multi) > -$(multi-objs-m:.o=.i) : modname = $(modname-multi) > -$(multi-objs-m:.o=.s) : modname = $(modname-multi) > -$(multi-objs-m:.o=.lst) : modname = $(modname-multi) > -$(multi-objs-y) : modname = $(modname-multi) > -$(multi-objs-y:.o=.i) : modname = $(modname-multi) > -$(multi-objs-y:.o=.s) : modname = $(modname-multi) > -$(multi-objs-y:.o=.lst) : modname = $(modname-multi) > +modname = $(if $(modname-multi),$(modname-multi),$(basetarget)) > > quiet_cmd_cc_s_c = CC $(quiet_modtag) $@ > cmd_cc_s_c = $(CC) $(c_flags) $(DISABLE_LTO) -fverbose-asm -S -o $@ $< > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 9217d40..e4e8e1b 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -47,11 +47,6 @@ multi-used-m := $(sort $(foreach m,$(obj-m), $(if $(strip > $($(m:.o=-objs)) $($(m > multi-used := $(multi-used-y) $(multi-used-m) > single-used-m := $(sort $(filter-out $(multi-used-m),$(obj-m))) > > -# Build list of the parts of our composite objects, our composite > -# objects depend on those (obviously) > -multi-objs-y := $(foreach m, $(multi-used-y), $($(m:.o=-objs)) $($(m:.o=-y))) > -multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)) $($(m:.o=-y)) > $($(m:.o=-m))) > - > # $(subdir-obj-y) is the list of objects in $(obj-y) which uses dir/ to > # tell kbuild to descend > subdir-obj-y := $(filter %/built-in.o, $(obj-y)) > @@ -80,8 +75,6 @@ real-objs-m := $(addprefix $(obj)/,$(real-objs-m)) > single-used-m:= $(addprefix $(obj)/,$(single-used-m)) > multi-used-y := $(addprefix $(obj)/,$(multi-used-y)) > multi-used-m := $(addprefix $(obj)/,$(multi-used-m)) > -multi-objs-y := $(addprefix $(obj)/,$(multi-objs-y)) > -multi-objs-m := $(addprefix $(obj)/,$(multi-objs-m)) > subdir-ym:= $(addprefix $(obj)/,$(subdir-ym)) > > # These flags are needed for modversions and compiling, so we define them > here > multi-objs-y/m seems only exist for rules above with target-specific variable assignment. A great simplification, So Reviewed-by: Cao jin -- Sincerely, Cao jin
Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects
Masahiro-san On 03/08/2018 06:21 PM, Masahiro Yamada wrote: > 2018-03-08 19:11 GMT+09:00 Cao jin <caoj.f...@cn.fujitsu.com>: >> >> >> On 03/08/2018 09:05 AM, Masahiro Yamada wrote: >>> Currently, KBUILD_MODNAME is defined only when $(modname) contains >>> just one word. If an object is shared among multiple modules, >>> undefined KBUILD_MODNAME could cause a build error. For example, >>> if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates >>> .modname, then fails to build due to undefined KBUILD_MODNAME. >>> >>> Take the following code as an example: >>> >>> obj-m += foo.o >>> obj-m += bar.o >>> foo-objs := foo-bar-common.o foo-main.o >>> bar-objs := foo-bar-common.o bar-main.o >>> >>> In this case, there is room for argument what to define for >>> KBUILD_MODNAME when foo-bar-common.o is being compiled. >>> "foo", "bar", or what else? >>> >>> One idea is to define colon-separated modules that share the object, >>> in this case, "bar:foo" (modules are sorted alphabetically by >>> $(sort ...). >>> >>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com> >>> --- >>> >>> scripts/Makefile.lib | 9 + >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>> index a7e315f..a1fbd6a 100644 >>> --- a/scripts/Makefile.lib >>> +++ b/scripts/Makefile.lib >>> @@ -92,8 +92,7 @@ subdir-ym := $(addprefix $(obj)/,$(subdir-ym)) >>> # differ in different configs. >>> name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst >>> -,_,$1))$(quote)$(squote) >>> basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget)) >>> -modname_flags = $(if $(filter 1,$(words $(modname))),\ >>> - -DKBUILD_MODNAME=$(call name-fix,$(modname))) >>> +modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) >>> >> >> I guess there is comment also need to be modified above this code hunk: >> >> Note: Files that end up in two or more modules are compiled without the >> KBUILD_MODNAME definition. The reason is that any made-up name >> would differ in different configs. > > > Why do I have to add lying comments here? > > The commit subject/log claims KBUILD_MODNAME should be _always_ defined. > I feel you misunderstand my intention, the comment I mentioned is already there, so they should be modified according to your patch. -- Sincerely, Cao jin
Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects
Masahiro-san On 03/08/2018 06:21 PM, Masahiro Yamada wrote: > 2018-03-08 19:11 GMT+09:00 Cao jin : >> >> >> On 03/08/2018 09:05 AM, Masahiro Yamada wrote: >>> Currently, KBUILD_MODNAME is defined only when $(modname) contains >>> just one word. If an object is shared among multiple modules, >>> undefined KBUILD_MODNAME could cause a build error. For example, >>> if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates >>> .modname, then fails to build due to undefined KBUILD_MODNAME. >>> >>> Take the following code as an example: >>> >>> obj-m += foo.o >>> obj-m += bar.o >>> foo-objs := foo-bar-common.o foo-main.o >>> bar-objs := foo-bar-common.o bar-main.o >>> >>> In this case, there is room for argument what to define for >>> KBUILD_MODNAME when foo-bar-common.o is being compiled. >>> "foo", "bar", or what else? >>> >>> One idea is to define colon-separated modules that share the object, >>> in this case, "bar:foo" (modules are sorted alphabetically by >>> $(sort ...). >>> >>> Signed-off-by: Masahiro Yamada >>> --- >>> >>> scripts/Makefile.lib | 9 + >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >>> index a7e315f..a1fbd6a 100644 >>> --- a/scripts/Makefile.lib >>> +++ b/scripts/Makefile.lib >>> @@ -92,8 +92,7 @@ subdir-ym := $(addprefix $(obj)/,$(subdir-ym)) >>> # differ in different configs. >>> name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst >>> -,_,$1))$(quote)$(squote) >>> basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget)) >>> -modname_flags = $(if $(filter 1,$(words $(modname))),\ >>> - -DKBUILD_MODNAME=$(call name-fix,$(modname))) >>> +modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) >>> >> >> I guess there is comment also need to be modified above this code hunk: >> >> Note: Files that end up in two or more modules are compiled without the >> KBUILD_MODNAME definition. The reason is that any made-up name >> would differ in different configs. > > > Why do I have to add lying comments here? > > The commit subject/log claims KBUILD_MODNAME should be _always_ defined. > I feel you misunderstand my intention, the comment I mentioned is already there, so they should be modified according to your patch. -- Sincerely, Cao jin
Re: [PATCH 1/5] kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi
On 03/08/2018 09:04 AM, Masahiro Yamada wrote: > In the context ... > > $(obj)/%.s: $(src)/%.c FORCE > $(call if_changed_dep,cc_s_c) > > $(obj)/%.i: $(src)/%.c FORCE > $(call if_changed_dep,cpp_i_c) > > $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE > $(call cmd,force_checksrc) > $(call if_changed_rule,cc_o_c) > > $(obj)/%.lst: $(src)/%.c FORCE > $(call if_changed_dep,cc_lst_c) > > '$*' returns the stem of the target (the part of '%'), so $(obj)/ has > already been ripped off. > > $(subst $(obj)/,,$*.o) is the same as $(*.o) > > Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com> > --- > > scripts/Makefile.lib | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 5589bae..a7e315f 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -175,7 +175,7 @@ dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc >\ > > # Finds the multi-part object the current object will be linked into > modname-multi = $(sort $(foreach m,$(multi-used),\ > - $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) > $($(m:.o=-y))),$(m:.o= > + $(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o= > > # Useful for describing the dependency of composite objects > # Usage: > A subtle catch! And in my test, a debug line $(info $@ = $*) in rule: $(obj)/%.o: $(src)/%.c xxx does tell me it is correct. So, Reviewed-by: Cao jin <caoj.f...@cn.fujitsu.com> -- Sincerely, Cao jin
Re: [PATCH 1/5] kbuild: remove unnecessary $(subst $(obj)/,,...) in modname-multi
On 03/08/2018 09:04 AM, Masahiro Yamada wrote: > In the context ... > > $(obj)/%.s: $(src)/%.c FORCE > $(call if_changed_dep,cc_s_c) > > $(obj)/%.i: $(src)/%.c FORCE > $(call if_changed_dep,cpp_i_c) > > $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE > $(call cmd,force_checksrc) > $(call if_changed_rule,cc_o_c) > > $(obj)/%.lst: $(src)/%.c FORCE > $(call if_changed_dep,cc_lst_c) > > '$*' returns the stem of the target (the part of '%'), so $(obj)/ has > already been ripped off. > > $(subst $(obj)/,,$*.o) is the same as $(*.o) > > Signed-off-by: Masahiro Yamada > --- > > scripts/Makefile.lib | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 5589bae..a7e315f 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -175,7 +175,7 @@ dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc >\ > > # Finds the multi-part object the current object will be linked into > modname-multi = $(sort $(foreach m,$(multi-used),\ > - $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) > $($(m:.o=-y))),$(m:.o= > + $(if $(filter $*.o, $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o= > > # Useful for describing the dependency of composite objects > # Usage: > A subtle catch! And in my test, a debug line $(info $@ = $*) in rule: $(obj)/%.o: $(src)/%.c xxx does tell me it is correct. So, Reviewed-by: Cao jin -- Sincerely, Cao jin
Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects
On 03/08/2018 09:05 AM, Masahiro Yamada wrote: > Currently, KBUILD_MODNAME is defined only when $(modname) contains > just one word. If an object is shared among multiple modules, > undefined KBUILD_MODNAME could cause a build error. For example, > if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates > .modname, then fails to build due to undefined KBUILD_MODNAME. > > Take the following code as an example: > > obj-m += foo.o > obj-m += bar.o > foo-objs := foo-bar-common.o foo-main.o > bar-objs := foo-bar-common.o bar-main.o > > In this case, there is room for argument what to define for > KBUILD_MODNAME when foo-bar-common.o is being compiled. > "foo", "bar", or what else? > > One idea is to define colon-separated modules that share the object, > in this case, "bar:foo" (modules are sorted alphabetically by > $(sort ...). > > Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com> > --- > > scripts/Makefile.lib | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index a7e315f..a1fbd6a 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -92,8 +92,7 @@ subdir-ym := $(addprefix $(obj)/,$(subdir-ym)) > # differ in different configs. > name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst > -,_,$1))$(quote)$(squote) > basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget)) > -modname_flags = $(if $(filter 1,$(words $(modname))),\ > - -DKBUILD_MODNAME=$(call name-fix,$(modname))) > +modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) > I guess there is comment also need to be modified above this code hunk: Note: Files that end up in two or more modules are compiled without the KBUILD_MODNAME definition. The reason is that any made-up name would differ in different configs. -- Sincerely, Cao jin
Re: [PATCH 2/5] kbuild: define KBUILD_MODNAME even if multiple modules share objects
On 03/08/2018 09:05 AM, Masahiro Yamada wrote: > Currently, KBUILD_MODNAME is defined only when $(modname) contains > just one word. If an object is shared among multiple modules, > undefined KBUILD_MODNAME could cause a build error. For example, > if CONFIG_DYNAMIC_DEBUG is enabled, any call of printk() populates > .modname, then fails to build due to undefined KBUILD_MODNAME. > > Take the following code as an example: > > obj-m += foo.o > obj-m += bar.o > foo-objs := foo-bar-common.o foo-main.o > bar-objs := foo-bar-common.o bar-main.o > > In this case, there is room for argument what to define for > KBUILD_MODNAME when foo-bar-common.o is being compiled. > "foo", "bar", or what else? > > One idea is to define colon-separated modules that share the object, > in this case, "bar:foo" (modules are sorted alphabetically by > $(sort ...). > > Signed-off-by: Masahiro Yamada > --- > > scripts/Makefile.lib | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index a7e315f..a1fbd6a 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -92,8 +92,7 @@ subdir-ym := $(addprefix $(obj)/,$(subdir-ym)) > # differ in different configs. > name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst > -,_,$1))$(quote)$(squote) > basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget)) > -modname_flags = $(if $(filter 1,$(words $(modname))),\ > - -DKBUILD_MODNAME=$(call name-fix,$(modname))) > +modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) > I guess there is comment also need to be modified above this code hunk: Note: Files that end up in two or more modules are compiled without the KBUILD_MODNAME definition. The reason is that any made-up name would differ in different configs. -- Sincerely, Cao jin
[PATCH] x86/tools/relocs: comments cleanup
1. typo fix: there -> their
2. remove superfluous "by"
Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
---
arch/x86/tools/relocs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 5d73c443e778..fcf3fad01b08 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -37,7 +37,7 @@ static struct section *secs;
static const char * const sym_regex_kernel[S_NSYMTYPES] = {
/*
- * Following symbols have been audited. There values are constant and do
+ * Following symbols have been audited. Their values are constant and do
* not change if bzImage is loaded at a different physical address than
* the address for which it has been compiled. Don't warn user about
* absolute relocations present w.r.t these symbols.
@@ -690,7 +690,7 @@ static void walk_relocs(int (*process)(struct section *sec,
Elf_Rel *rel,
* to the start of the per_cpu area that does not change).
*
* Relocations that apply to the per_cpu area need to have their
- * offset adjusted by by the value of __per_cpu_load to make them
+ * offset adjusted by the value of __per_cpu_load to make them
* point to the correct place in the loaded image (because the
* virtual address of .data..percpu is 0).
*
--
2.14.3
[PATCH] x86/tools/relocs: comments cleanup
1. typo fix: there -> their
2. remove superfluous "by"
Signed-off-by: Cao jin
---
arch/x86/tools/relocs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 5d73c443e778..fcf3fad01b08 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -37,7 +37,7 @@ static struct section *secs;
static const char * const sym_regex_kernel[S_NSYMTYPES] = {
/*
- * Following symbols have been audited. There values are constant and do
+ * Following symbols have been audited. Their values are constant and do
* not change if bzImage is loaded at a different physical address than
* the address for which it has been compiled. Don't warn user about
* absolute relocations present w.r.t these symbols.
@@ -690,7 +690,7 @@ static void walk_relocs(int (*process)(struct section *sec,
Elf_Rel *rel,
* to the start of the per_cpu area that does not change).
*
* Relocations that apply to the per_cpu area need to have their
- * offset adjusted by by the value of __per_cpu_load to make them
+ * offset adjusted by the value of __per_cpu_load to make them
* point to the correct place in the loaded image (because the
* virtual address of .data..percpu is 0).
*
--
2.14.3
[PATCH] kbuild/kallsyms: trivial typo fix
Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
---
I guess this belongs to kbuild though get_maintainer.pl doesn't tell me that.
scripts/kallsyms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 9ee9bf7fd1a2..65792650c630 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -595,7 +595,7 @@ static void optimize_result(void)
* original char code */
if (!best_table_len[i]) {
- /* find the token with the breates profit value */
+ /* find the token with the best profit value */
best = find_best_token();
if (token_profit[best] == 0)
break;
--
2.14.3
[PATCH] kbuild/kallsyms: trivial typo fix
Signed-off-by: Cao jin
---
I guess this belongs to kbuild though get_maintainer.pl doesn't tell me that.
scripts/kallsyms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 9ee9bf7fd1a2..65792650c630 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -595,7 +595,7 @@ static void optimize_result(void)
* original char code */
if (!best_table_len[i]) {
- /* find the token with the breates profit value */
+ /* find the token with the best profit value */
best = find_best_token();
if (token_profit[best] == 0)
break;
--
2.14.3
[PATCH RFC] kbuild: drop superfluous GCC_PLUGINS_CFLAGS assignment
GCC_PLUGINS_CFLAGS is already in the environment, so it is superfluous
to add it in commanline of final build of init/
Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
---
This is only tested with Randomizing Structure Layout plugin. The test
method is not so grace but I think it can prove the correctness of this
patch. On the other hand, if we concerns that some flags cannot be
passed during final build, should also consider all the other flags.
Currently, with Randomizing plugin enabled, the crash utility can't work
with it, the symptom is a Segmentation fault due to infinite function call.
With the patch, the symptom is exactly the same, so I am sure the plugin works.
scripts/link-vmlinux.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index e6818b8e7141..e07b2d251ad6 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -248,7 +248,7 @@ else
fi;
# final build of init/
-${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
+${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
archive_builtin
--
2.14.3
[PATCH RFC] kbuild: drop superfluous GCC_PLUGINS_CFLAGS assignment
GCC_PLUGINS_CFLAGS is already in the environment, so it is superfluous
to add it in commanline of final build of init/
Signed-off-by: Cao jin
---
This is only tested with Randomizing Structure Layout plugin. The test
method is not so grace but I think it can prove the correctness of this
patch. On the other hand, if we concerns that some flags cannot be
passed during final build, should also consider all the other flags.
Currently, with Randomizing plugin enabled, the crash utility can't work
with it, the symptom is a Segmentation fault due to infinite function call.
With the patch, the symptom is exactly the same, so I am sure the plugin works.
scripts/link-vmlinux.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index e6818b8e7141..e07b2d251ad6 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -248,7 +248,7 @@ else
fi;
# final build of init/
-${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
+${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
archive_builtin
--
2.14.3
Re: why scripts/link-vmlinux.sh has a final build of init/
Sorry for late.
On 02/14/2018 07:31 PM, Masahiro Yamada wrote:
> 2018-02-13 16:08 GMT+09:00 Cao jin <caoj.f...@cn.fujitsu.com>:
>
>> BTW, I still have 2 questions.
>>
>> 1. In final build, why need
>>
>>GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
>>
>> Doesn't GCC_PLUGINS_CFLAGS already exist in the environment?
>>
>> I also tested the Randomizing Structure Layout plugin with this patch,
>> the plugin seems works in my test.
>
>
> I have not tested, but GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
> is probably unnecessary.
>
>
>
>> 2. scripts/link-vmlinux.sh seems just handle only one argument: clean.
>> So why shouldn't it be:
>
>
> To detect the change of $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> because link-vmlinux.sh depends on them.
>
I understood, I missed the existence of .vmlinux.cmd file.
Thanks very much, Masahiro-san.
--
Sincerely,
Cao jin
>
>> diff --git a/Makefile b/Makefile
>> index ccd981892ef2..21d93b545381 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -998,7 +998,7 @@ ARCH_POSTLINK := $(wildcard
>> $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>>
>> # Final link of vmlinux with optional arch pass after final link
>> cmd_link-vmlinux = \
>> - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
>> + $(CONFIG_SHELL) $<; \
>> $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>>
>> vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
>
>
>
Re: why scripts/link-vmlinux.sh has a final build of init/
Sorry for late.
On 02/14/2018 07:31 PM, Masahiro Yamada wrote:
> 2018-02-13 16:08 GMT+09:00 Cao jin :
>
>> BTW, I still have 2 questions.
>>
>> 1. In final build, why need
>>
>>GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
>>
>> Doesn't GCC_PLUGINS_CFLAGS already exist in the environment?
>>
>> I also tested the Randomizing Structure Layout plugin with this patch,
>> the plugin seems works in my test.
>
>
> I have not tested, but GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
> is probably unnecessary.
>
>
>
>> 2. scripts/link-vmlinux.sh seems just handle only one argument: clean.
>> So why shouldn't it be:
>
>
> To detect the change of $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> because link-vmlinux.sh depends on them.
>
I understood, I missed the existence of .vmlinux.cmd file.
Thanks very much, Masahiro-san.
--
Sincerely,
Cao jin
>
>> diff --git a/Makefile b/Makefile
>> index ccd981892ef2..21d93b545381 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -998,7 +998,7 @@ ARCH_POSTLINK := $(wildcard
>> $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>>
>> # Final link of vmlinux with optional arch pass after final link
>> cmd_link-vmlinux = \
>> - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
>> + $(CONFIG_SHELL) $<; \
>> $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>>
>> vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
>
>
>
[tip:x86/build] x86/build: Drop superfluous ALIGN from the linker script
Commit-ID: a06cc94f3f8dfab74fe7fac3a6e9f15d77566d00 Gitweb: https://git.kernel.org/tip/a06cc94f3f8dfab74fe7fac3a6e9f15d77566d00 Author: Cao jin <caoj.f...@cn.fujitsu.com> AuthorDate: Thu, 8 Feb 2018 14:38:57 +0800 Committer: Ingo Molnar <mi...@kernel.org> CommitDate: Tue, 13 Feb 2018 13:08:46 +0100 x86/build: Drop superfluous ALIGN from the linker script ALIGN(8) is superfluous since macro TEXT_TEXT already has one. bonus cleanups: - indentation fix - spaces -> tab. Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> Cc: Andy Lutomirski <l...@kernel.org> Cc: Borislav Petkov <b...@alien8.de> Cc: Linus Torvalds <torva...@linux-foundation.org> Cc: Peter Zijlstra <pet...@infradead.org> Cc: Thomas Gleixner <t...@linutronix.de> Link: http://lkml.kernel.org/r/20180208063857.15197-1-caoj.f...@cn.fujitsu.com Signed-off-by: Ingo Molnar <mi...@kernel.org> --- arch/x86/kernel/vmlinux.lds.S | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 9b138a0..1c43a2e 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -102,7 +102,6 @@ SECTIONS _stext = .; /* bootstrapping code */ HEAD_TEXT - . = ALIGN(8); TEXT_TEXT SCHED_TEXT CPUIDLE_TEXT @@ -198,7 +197,7 @@ SECTIONS . = __vvar_beginning_hack + PAGE_SIZE; } :data - . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE); + . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE); /* Init code and data - will be freed after init */ . = ALIGN(PAGE_SIZE); @@ -366,8 +365,8 @@ SECTIONS . = ALIGN(PAGE_SIZE); /* keep VO_INIT_SIZE page aligned */ _end = .; -STABS_DEBUG -DWARF_DEBUG + STABS_DEBUG + DWARF_DEBUG /* Sections to be discarded */ DISCARDS
[tip:x86/build] x86/build: Drop superfluous ALIGN from the linker script
Commit-ID: a06cc94f3f8dfab74fe7fac3a6e9f15d77566d00 Gitweb: https://git.kernel.org/tip/a06cc94f3f8dfab74fe7fac3a6e9f15d77566d00 Author: Cao jin AuthorDate: Thu, 8 Feb 2018 14:38:57 +0800 Committer: Ingo Molnar CommitDate: Tue, 13 Feb 2018 13:08:46 +0100 x86/build: Drop superfluous ALIGN from the linker script ALIGN(8) is superfluous since macro TEXT_TEXT already has one. bonus cleanups: - indentation fix - spaces -> tab. Signed-off-by: Cao jin Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/20180208063857.15197-1-caoj.f...@cn.fujitsu.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/vmlinux.lds.S | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 9b138a0..1c43a2e 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -102,7 +102,6 @@ SECTIONS _stext = .; /* bootstrapping code */ HEAD_TEXT - . = ALIGN(8); TEXT_TEXT SCHED_TEXT CPUIDLE_TEXT @@ -198,7 +197,7 @@ SECTIONS . = __vvar_beginning_hack + PAGE_SIZE; } :data - . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE); + . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE); /* Init code and data - will be freed after init */ . = ALIGN(PAGE_SIZE); @@ -366,8 +365,8 @@ SECTIONS . = ALIGN(PAGE_SIZE); /* keep VO_INIT_SIZE page aligned */ _end = .; -STABS_DEBUG -DWARF_DEBUG + STABS_DEBUG + DWARF_DEBUG /* Sections to be discarded */ DISCARDS
Re: why scripts/link-vmlinux.sh has a final build of init/
On 02/12/2018 10:48 PM, Masahiro Yamada wrote:
> 2018-02-12 13:22 GMT+09:00 Cao jin <caoj.f...@cn.fujitsu.com>:
>> Hi Masahiro-san,
>>
>> As I remember, init/ is already built during recursive make, and I did
>> a clean build(make mrproper, make localmodconfig) with all plugins
>> included on x86_64 with following patch, the kernel can boot without any
>> obvious problem. So, I don't figure out why we need this final build?
>>
>>
>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>> index e6818b8e7141..086b2efd4d53 100755
>> --- a/scripts/link-vmlinux.sh
>> +++ b/scripts/link-vmlinux.sh
>> @@ -247,9 +247,6 @@ else
>> expr 0$(cat .old_version) + 1 >.version;
>> fi;
>>
>> -# final build of init/
>> -${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
>> GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
>> -
>> archive_builtin
>>
>> #link vmlinux.o
>>
>
>
> link-vmlinux.sh increments '.version'
> so it must descend into init/
> to update include/generated/compile.h
> and re-compile init/version.o
>
> We do not increment '.version'
> at the first descend into init/
> because we never know whether the kernel
> is updated or not before the final link stage.
>
Oh, I wasn't aware of this process, now understood. Thanks Masahiro-san.
BTW, I still have 2 questions.
1. In final build, why need
GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
Doesn't GCC_PLUGINS_CFLAGS already exist in the environment?
I also tested the Randomizing Structure Layout plugin with this patch,
the plugin seems works in my test.
2. scripts/link-vmlinux.sh seems just handle only one argument: clean.
So why shouldn't it be:
diff --git a/Makefile b/Makefile
index ccd981892ef2..21d93b545381 100644
--- a/Makefile
+++ b/Makefile
@@ -998,7 +998,7 @@ ARCH_POSTLINK := $(wildcard
$(srctree)/arch/$(SRCARCH)/Makefile.postlink)
# Final link of vmlinux with optional arch pass after final link
cmd_link-vmlinux = \
- $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
+ $(CONFIG_SHELL) $<; \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
--
Sincerely,
Cao jin
Re: why scripts/link-vmlinux.sh has a final build of init/
On 02/12/2018 10:48 PM, Masahiro Yamada wrote:
> 2018-02-12 13:22 GMT+09:00 Cao jin :
>> Hi Masahiro-san,
>>
>> As I remember, init/ is already built during recursive make, and I did
>> a clean build(make mrproper, make localmodconfig) with all plugins
>> included on x86_64 with following patch, the kernel can boot without any
>> obvious problem. So, I don't figure out why we need this final build?
>>
>>
>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>> index e6818b8e7141..086b2efd4d53 100755
>> --- a/scripts/link-vmlinux.sh
>> +++ b/scripts/link-vmlinux.sh
>> @@ -247,9 +247,6 @@ else
>> expr 0$(cat .old_version) + 1 >.version;
>> fi;
>>
>> -# final build of init/
>> -${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
>> GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
>> -
>> archive_builtin
>>
>> #link vmlinux.o
>>
>
>
> link-vmlinux.sh increments '.version'
> so it must descend into init/
> to update include/generated/compile.h
> and re-compile init/version.o
>
> We do not increment '.version'
> at the first descend into init/
> because we never know whether the kernel
> is updated or not before the final link stage.
>
Oh, I wasn't aware of this process, now understood. Thanks Masahiro-san.
BTW, I still have 2 questions.
1. In final build, why need
GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
Doesn't GCC_PLUGINS_CFLAGS already exist in the environment?
I also tested the Randomizing Structure Layout plugin with this patch,
the plugin seems works in my test.
2. scripts/link-vmlinux.sh seems just handle only one argument: clean.
So why shouldn't it be:
diff --git a/Makefile b/Makefile
index ccd981892ef2..21d93b545381 100644
--- a/Makefile
+++ b/Makefile
@@ -998,7 +998,7 @@ ARCH_POSTLINK := $(wildcard
$(srctree)/arch/$(SRCARCH)/Makefile.postlink)
# Final link of vmlinux with optional arch pass after final link
cmd_link-vmlinux = \
- $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
+ $(CONFIG_SHELL) $<; \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
--
Sincerely,
Cao jin
why scripts/link-vmlinux.sh has a final build of init/
Hi Masahiro-san,
As I remember, init/ is already built during recursive make, and I did
a clean build(make mrproper, make localmodconfig) with all plugins
included on x86_64 with following patch, the kernel can boot without any
obvious problem. So, I don't figure out why we need this final build?
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index e6818b8e7141..086b2efd4d53 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -247,9 +247,6 @@ else
expr 0$(cat .old_version) + 1 >.version;
fi;
-# final build of init/
-${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
-
archive_builtin
#link vmlinux.o
--
Sincerely,
Cao jin
why scripts/link-vmlinux.sh has a final build of init/
Hi Masahiro-san,
As I remember, init/ is already built during recursive make, and I did
a clean build(make mrproper, make localmodconfig) with all plugins
included on x86_64 with following patch, the kernel can boot without any
obvious problem. So, I don't figure out why we need this final build?
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index e6818b8e7141..086b2efd4d53 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -247,9 +247,6 @@ else
expr 0$(cat .old_version) + 1 >.version;
fi;
-# final build of init/
-${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init
GCC_PLUGINS_CFLAGS="${GCC_PLUGINS_CFLAGS}"
-
archive_builtin
#link vmlinux.o
--
Sincerely,
Cao jin
[PATCH] x86/linker script: drop superfluous ALIGN
ALIGN(8) is superfluous since macro TEXT_TEXT already has one. bonus: indentation fix, spaces -> tab. Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> --- arch/x86/kernel/vmlinux.lds.S | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index a4009fb9be87..f4dc9e39ea40 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -96,7 +96,6 @@ SECTIONS _stext = .; /* bootstrapping code */ HEAD_TEXT - . = ALIGN(8); TEXT_TEXT SCHED_TEXT CPUIDLE_TEXT @@ -175,7 +174,7 @@ SECTIONS . = __vvar_beginning_hack + PAGE_SIZE; } :data - . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE); + . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE); /* Init code and data - will be freed after init */ . = ALIGN(PAGE_SIZE); @@ -343,8 +342,8 @@ SECTIONS . = ALIGN(PAGE_SIZE); /* keep VO_INIT_SIZE page aligned */ _end = .; -STABS_DEBUG -DWARF_DEBUG + STABS_DEBUG + DWARF_DEBUG /* Sections to be discarded */ DISCARDS -- 2.14.3
