Messages in this thread | | | From | Joe Damato <> | Date | Thu, 9 Jun 2022 11:03:33 +0300 | Subject | Re: [PATCH v2] x86/mm: Fix RESERVE_BRK() for older binutils |
| |
On Thu, Jun 09, 2022 at 12:17:32AM -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. > > Inline asm was only needed here to convince the toolchain to add the > STT_NOBITS flag. However, if a C variable is placed in a section whose > name is prefixed with ".bss", GCC and Clang automatically set > STT_NOBITS. In fact, ".bss..page_aligned" already relies on this trick. > > So fix the build failure (and simplify the macro) by allocating the > variable in C. > > Also, add NOLOAD to the ".brk" output section clause in the linker > script. This is a failsafe in case the ".bss" prefix magic trick ever > stops working somehow. If there's a section type mismatch, the GNU > linker will force the ".brk" output section to be STT_NOBITS. The LLVM > linker will fail with a "section type mismatch" error. > > Note this also 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 <jdamato@fastly.com> > Reported-by: Byungchul Park <byungchul.park@lge.com> > Fixes: a1e2c031ec39 ("x86/mm: Simplify RESERVE_BRK()") > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > --- > arch/x86/include/asm/setup.h | 38 +++++++++++++++++++---------------- > arch/x86/kernel/setup.c | 5 ----- > arch/x86/kernel/vmlinux.lds.S | 4 ++-- > 3 files changed, 23 insertions(+), 24 deletions(-) > > diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h > index 7590ac2570b9..f8b9ee97a891 100644 > --- a/arch/x86/include/asm/setup.h > +++ b/arch/x86/include/asm/setup.h > @@ -108,19 +108,16 @@ 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. > + * Reserve space in the .brk section, which is a block of memory from which the > + * caller is allowed to allocate very early (before even memblock is available) > + * by calling extend_brk(). All allocated memory will be eventually converted > + * to memblock. Any leftover unallocated memory will be freed. > * > - * 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. > + * 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(".bss..brk") __aligned(1) __used \ > + static char __brk_##name[size] > > extern void probe_roms(void); > #ifdef __i386__ > @@ -133,12 +130,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 .bss..brk, "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/setup.c b/arch/x86/kernel/setup.c > index 3ebb85327edb..bd6c6fd373ae 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -67,11 +67,6 @@ RESERVE_BRK(dmi_alloc, 65536); > #endif > > > -/* > - * Range of the BSS area. The size of the BSS area is determined > - * at link time, with RESERVE_BRK() facility reserving additional > - * chunks. > - */ > unsigned long _brk_start = (unsigned long)__brk_base; > unsigned long _brk_end = (unsigned long)__brk_base; > > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index f5f6dc2e8007..81aba718ecd5 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -385,10 +385,10 @@ 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 */ > + *(.bss..brk) /* areas brk users have reserved */ > __brk_limit = .; > } > > -- > 2.34.3 >
I applied the v2 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 builds successfully for me.
The resulting kernel boots fine on the machine, as well.
I built the kernel with: - gcc (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609 - binutils 2.26.1-1ubuntu1~16.04.8+esm1
I have not tested with any other build toolchains.
Tested-by: Joe Damato <jdamato@fastly.com>
| |