2022-06-08 16:08:41

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH] x86/mm: Fix RESERVE_BRK() for older binutils

With binutils 2.26, RESERVE_BRK() causes a build failure:

/tmp/ccnGOKZ5.s: Assembler messages:
/tmp/ccnGOKZ5.s:98: Error: missing ')'
/tmp/ccnGOKZ5.s:98: Error: missing ')'
/tmp/ccnGOKZ5.s:98: Error: missing ')'
/tmp/ccnGOKZ5.s:98: Error: junk at end of line, first unrecognized
character is `U'

The problem is this line:

RESERVE_BRK(early_pgt_alloc, INIT_PGT_BUF_SIZE)

Specifically, the INIT_PGT_BUF_SIZE macro which (via PAGE_SIZE's use
_AC()) has a "1UL", which makes older versions of the assembler unhappy.
Unfortunately the _AC() macro doesn't work for inline asm.

Fix it (and further simplify RESERVE_BRK) by taking a completely
different approach. Instead of specifying the "nobits" at compile-time,
do so at link-time. Then the variable can just be allocated in C.

Note this changes the name of the variable from .brk.##name to
__brk_##name. The variable names aren't actually used anywhere, so it's
harmless.

Reported-by: Joe Damato <[email protected]>
Fixes: a1e2c031ec39 ("x86/mm: Simplify RESERVE_BRK()")
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/setup.h | 35 +++++++++++++++++------------------
arch/x86/kernel/vmlinux.lds.S | 2 +-
2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 7590ac2570b9..ef9bb6caad8c 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -108,19 +108,11 @@ extern unsigned long _brk_end;
void *extend_brk(size_t size, size_t align);

/*
- * Reserve space in the brk section. The name must be unique within the file,
- * and somewhat descriptive. The size is in bytes.
- *
- * The allocation is done using inline asm (rather than using a section
- * attribute on a normal variable) in order to allow the use of @nobits, so
- * that it doesn't take up any space in the vmlinux file.
+ * Reserve space in the .brk section. The size is in bytes.
*/
-#define RESERVE_BRK(name, size) \
- asm(".pushsection .brk_reservation,\"aw\",@nobits\n\t" \
- ".brk." #name ":\n\t" \
- ".skip " __stringify(size) "\n\t" \
- ".size .brk." #name ", " __stringify(size) "\n\t" \
- ".popsection\n\t")
+#define RESERVE_BRK(name, size) \
+ __section(".brk_reservation") __aligned(1) __used \
+ static char __brk_##name[size]

extern void probe_roms(void);
#ifdef __i386__
@@ -133,12 +125,19 @@ asmlinkage void __init x86_64_start_reservations(char *real_mode_data);

#endif /* __i386__ */
#endif /* _SETUP */
-#else
-#define RESERVE_BRK(name,sz) \
- .pushsection .brk_reservation,"aw",@nobits; \
-.brk.name: \
-1: .skip sz; \
- .size .brk.name,.-1b; \
+
+#else /* __ASSEMBLY */
+
+.macro __RESERVE_BRK name, size
+ .pushsection .brk_reservation, "aw"
+SYM_DATA_START(__brk_\name)
+ .skip \size
+SYM_DATA_END(__brk_\name)
.popsection
+.endm
+
+#define RESERVE_BRK(name, size) __RESERVE_BRK name, size
+
#endif /* __ASSEMBLY__ */
+
#endif /* _ASM_X86_SETUP_H */
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index f5f6dc2e8007..9b63f8a00b4f 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -385,7 +385,7 @@ SECTIONS
__end_of_kernel_reserve = .;

. = ALIGN(PAGE_SIZE);
- .brk : AT(ADDR(.brk) - LOAD_OFFSET) {
+ .brk (NOLOAD) : AT(ADDR(.brk) - LOAD_OFFSET) {
__brk_base = .;
. += 64 * 1024; /* 64k alignment slop space */
*(.brk_reservation) /* areas brk users have reserved */
--
2.34.3


2022-06-08 22:46:30

by Joe Damato

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Fix RESERVE_BRK() for older binutils

On Wed, Jun 08, 2022 at 08:40:42AM -0700, Josh Poimboeuf wrote:
> With binutils 2.26, RESERVE_BRK() causes a build failure:
>
> /tmp/ccnGOKZ5.s: Assembler messages:
> /tmp/ccnGOKZ5.s:98: Error: missing ')'
> /tmp/ccnGOKZ5.s:98: Error: missing ')'
> /tmp/ccnGOKZ5.s:98: Error: missing ')'
> /tmp/ccnGOKZ5.s:98: Error: junk at end of line, first unrecognized
> character is `U'
>
> The problem is this line:
>
> RESERVE_BRK(early_pgt_alloc, INIT_PGT_BUF_SIZE)
>
> Specifically, the INIT_PGT_BUF_SIZE macro which (via PAGE_SIZE's use
> _AC()) has a "1UL", which makes older versions of the assembler unhappy.
> Unfortunately the _AC() macro doesn't work for inline asm.
>
> Fix it (and further simplify RESERVE_BRK) by taking a completely
> different approach. Instead of specifying the "nobits" at compile-time,
> do so at link-time. Then the variable can just be allocated in C.
>
> Note this changes the name of the variable from .brk.##name to
> __brk_##name. The variable names aren't actually used anywhere, so it's
> harmless.
>
> Reported-by: Joe Damato <[email protected]>
> Fixes: a1e2c031ec39 ("x86/mm: Simplify RESERVE_BRK()")
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> arch/x86/include/asm/setup.h | 35 +++++++++++++++++------------------
> arch/x86/kernel/vmlinux.lds.S | 2 +-
> 2 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index 7590ac2570b9..ef9bb6caad8c 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -108,19 +108,11 @@ extern unsigned long _brk_end;
> void *extend_brk(size_t size, size_t align);
>
> /*
> - * Reserve space in the brk section. The name must be unique within the file,
> - * and somewhat descriptive. The size is in bytes.
> - *
> - * The allocation is done using inline asm (rather than using a section
> - * attribute on a normal variable) in order to allow the use of @nobits, so
> - * that it doesn't take up any space in the vmlinux file.
> + * Reserve space in the .brk section. The size is in bytes.
> */
> -#define RESERVE_BRK(name, size) \
> - asm(".pushsection .brk_reservation,\"aw\",@nobits\n\t" \
> - ".brk." #name ":\n\t" \
> - ".skip " __stringify(size) "\n\t" \
> - ".size .brk." #name ", " __stringify(size) "\n\t" \
> - ".popsection\n\t")
> +#define RESERVE_BRK(name, size) \
> + __section(".brk_reservation") __aligned(1) __used \
> + static char __brk_##name[size]
>
> extern void probe_roms(void);
> #ifdef __i386__
> @@ -133,12 +125,19 @@ asmlinkage void __init x86_64_start_reservations(char *real_mode_data);
>
> #endif /* __i386__ */
> #endif /* _SETUP */
> -#else
> -#define RESERVE_BRK(name,sz) \
> - .pushsection .brk_reservation,"aw",@nobits; \
> -.brk.name: \
> -1: .skip sz; \
> - .size .brk.name,.-1b; \
> +
> +#else /* __ASSEMBLY */
> +
> +.macro __RESERVE_BRK name, size
> + .pushsection .brk_reservation, "aw"
> +SYM_DATA_START(__brk_\name)
> + .skip \size
> +SYM_DATA_END(__brk_\name)
> .popsection
> +.endm
> +
> +#define RESERVE_BRK(name, size) __RESERVE_BRK name, size
> +
> #endif /* __ASSEMBLY__ */
> +
> #endif /* _ASM_X86_SETUP_H */
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index f5f6dc2e8007..9b63f8a00b4f 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -385,7 +385,7 @@ SECTIONS
> __end_of_kernel_reserve = .;
>
> . = ALIGN(PAGE_SIZE);
> - .brk : AT(ADDR(.brk) - LOAD_OFFSET) {
> + .brk (NOLOAD) : AT(ADDR(.brk) - LOAD_OFFSET) {
> __brk_base = .;
> . += 64 * 1024; /* 64k alignment slop space */
> *(.brk_reservation) /* areas brk users have reserved */
> --
> 2.34.3
>

I applied the patch on top of commit 58f9d52ff689 ("Merge tag
'net-5.19-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net") and the kernel
now builds successfully for me.

The resulting kernel boots fine on the machine, as well.

Tested-by: Joe Damato <[email protected]>

2022-06-08 23:19:48

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Fix RESERVE_BRK() for older binutils

On Thu, Jun 09, 2022 at 01:12:09AM +0300, Joe Damato wrote:
> I applied the patch on top of commit 58f9d52ff689 ("Merge tag
> 'net-5.19-rc1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net") and the kernel
> now builds successfully for me.
>
> The resulting kernel boots fine on the machine, as well.
>
> Tested-by: Joe Damato <[email protected]>

Thanks. Unfortunately, the LLVM linker doesn't like the patch...

>> ld.lld: warning: section type mismatch for .brk_reservation
>>> arch/x86/built-in.a(kernel/setup.o):(.brk_reservation): SHT_PROGBITS
>>> output section .brk: SHT_NOBITS
--
>> ld.lld: warning: section type mismatch for .brk_reservation
>>> arch/x86/built-in.a(mm/init.o):(.brk_reservation): SHT_PROGBITS
>>> output section .brk: SHT_NOBITS
>> ld.lld: warning: section type mismatch for .brk_reservation
>>> arch/x86/built-in.a(kernel/setup.o):(.brk_reservation): SHT_PROGBITS
>>> output section .brk: SHT_NOBITS
--
>> ld.lld: warning: section type mismatch for .brk_reservation
>>> arch/x86/built-in.a(mm/init.o):(.brk_reservation): SHT_PROGBITS
>>> output section .brk: SHT_NOBITS

--
Josh

2022-06-08 23:44:51

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Fix RESERVE_BRK() for older binutils

On Wed, Jun 08, 2022 at 03:57:27PM -0700, Josh Poimboeuf wrote:
> On Thu, Jun 09, 2022 at 01:12:09AM +0300, Joe Damato wrote:
> > I applied the patch on top of commit 58f9d52ff689 ("Merge tag
> > 'net-5.19-rc1' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net") and the kernel
> > now builds successfully for me.
> >
> > The resulting kernel boots fine on the machine, as well.
> >
> > Tested-by: Joe Damato <[email protected]>
>
> Thanks. Unfortunately, the LLVM linker doesn't like the patch...

I've discovered that if you put a variable in ".bss..whatever", the
section will magically be made NOBITS. Will try that for v2.

--
Josh