[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALALjgxsPMzK3TgkAjvc=fHseEzB7SYkZsCidFWkXZJMBra-6A@mail.gmail.com>
Date: Wed, 8 Jun 2022 12:10:04 +0300
From: Joe Damato <jdamato@...tly.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Andrew Cooper <Andrew.Cooper3@...rix.com>,
"x86@...nel.org" <x86@...nel.org>,
"jpoimboe@...hat.com" <jpoimboe@...hat.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jiangshanlai@...il.com" <jiangshanlai@...il.com>,
"bp@...e.de" <bp@...e.de>, "brgerst@...il.com" <brgerst@...il.com>
Subject: Re: 5.19-rc1 x86 build failure
On Tue, Jun 07, 2022 at 05:59:56PM -0700, Josh Poimboeuf wrote:
> On Tue, Jun 07, 2022 at 12:42:33PM +0000, Andrew Cooper wrote:
> > On 07/06/2022 13:19, Joe Damato wrote:
> > > 96 .pushsection .brk_reservation,"aw",@nobits
> > > 97 .brk.early_pgt_alloc:
> > > 98 ???? 00000000 .skip ((2 * 3) * ((1UL) << 12))
> > > **** Error: missing ')'
> > > **** Error: missing ')'
> > > **** Error: missing ')'
> > > **** Error: junk at end of line, first unrecognized character is `U'
> > > 98 0000
> > > 100 .popsection
> > >
> > > This comes from arch/x86/mm/init.c, which has the following code:
> > >
> > > RESERVE_BRK(early_pgt_alloc, INIT_PGT_BUF_SIZE);
> > >
> > > wherein INIT_PGT_BUF_SIZE (via PAGE_SIZE) has a "1UL" which makes the
> > > assembler unhappy.
> > >
> > > I don't really know what the correct way to fix this is; it seems that the
> > > macro _AC should handle this if ASSEMBLY is defined, IIUC, but that does
> > > not seem to be the case at this point in init.c.
> > >
> > > Perhaps I am doing something incorrect during the build process causing
> > > this to happen?
> >
> > The problem is that _AC() is evaluated in C context (so gains the UL/ULL
> > suffix), and the C'd string is fed directly into the assembler (where
> > older binutils doesn't tolerate the suffix).
> >
> > Short of having a _PAGE_SIZE which is an explicitly non-AC()'d constant,
> > I'm not sure what to suggest. Ideally, you'd want to temporarily define
> > __ASSEMBLY__ around the expansion of __stringify(), but I don't think
> > that's possible as RESERVE_BRK() is a macro itself.
>
> Joe, what version of binutils do you have?
I am using binutils: 2.26.1-1ubuntu1~16.04.8+esm4.
> We can fix this by taking a completely different approach: define the
> variable in C and just do the "nobits" in the linker script, like below.
> I can work up a proper patch tomorrow.
The change below makes sense to me. I applied it locally and was able to
build the kernel again.
Let me know if you'd like me to try building the next patch you write up.
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index 7590ac2570b9..4704184a2d78 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,
> + * Reserve space in the .brk section. The name must be unique within vmlinux,
> * 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.
> */
> #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")
> + char __brk_##name[size] __section(".brk_reservation");
>
> extern void probe_roms(void);
> #ifdef __i386__
> @@ -133,12 +125,16 @@ 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 */
> +
> +#define RESERVE_BRK(name, size) \
> + .pushsection .brk_reservation, "aw"; \
> +__brk_##name: \
> +1: .skip size; \
> + .size __brk_##name, . - 1b; \
> .popsection
> +
> #endif /* __ASSEMBLY__ */
> +
> #endif /* _ASM_X86_SETUP_H */
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 91831b9d8aa8..f105b8aa055e 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -392,7 +392,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 */
Powered by blists - more mailing lists