2022-04-29 21:50:35

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH] x86/mm: Simplify RESERVE_BRK()

RESERVE_BRK() reserves data in the .brk_reservation section. The data
is initialized to zero, like BSS, so the macro specifies 'nobits' to
prevent the data from taking up space in the vmlinux binary. The only
way to get the compiler to do that (without putting the variable in .bss
proper) is to use inline asm.

The macro also has a hack which encloses the inline asm in a discarded
function, which allows the size to be passed (global inline asm doesn't
allow inputs).

Remove the need for the discarded function hack by just stringifying the
size rather than supplying it as an input to the inline asm.

Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/setup.h | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index a1b107f2a12a..7590ac2570b9 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -108,27 +108,19 @@ 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. Must be
- * used at file scope.
+ * Reserve space in the brk section. The name must be unique within the file,
+ * and somewhat descriptive. The size is in bytes.
*
- * (This uses a temp function to wrap the asm so we can pass it the
- * size parameter; otherwise we wouldn't be able to. We can't use a
- * "section" attribute on a normal variable because it always ends up
- * being @progbits, which ends up allocating space in the vmlinux
- * executable.)
+ * 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,sz) \
- static void __section(".discard.text") __noendbr __used notrace \
- __brk_reservation_fn_##name##__(void) { \
- asm volatile ( \
- ".pushsection .brk_reservation,\"aw\",@nobits;" \
- ".brk." #name ":" \
- " 1:.skip %c0;" \
- " .size .brk." #name ", . - 1b;" \
- " .popsection" \
- : : "i" (sz)); \
- }
+#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")

extern void probe_roms(void);
#ifdef __i386__
--
2.34.1


2022-05-01 21:50:59

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Simplify RESERVE_BRK()

On Thu, Apr 28, 2022 at 01:04:25PM -0700, Josh Poimboeuf wrote:
> RESERVE_BRK() reserves data in the .brk_reservation section. The data
> is initialized to zero, like BSS, so the macro specifies 'nobits' to
> prevent the data from taking up space in the vmlinux binary. The only
> way to get the compiler to do that (without putting the variable in .bss
> proper) is to use inline asm.
>
> The macro also has a hack which encloses the inline asm in a discarded
> function, which allows the size to be passed (global inline asm doesn't
> allow inputs).
>
> Remove the need for the discarded function hack by just stringifying the
> size rather than supplying it as an input to the inline asm.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>

Tested-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>

2022-05-04 23:22:20

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/mm: Simplify RESERVE_BRK()

On Thu, Apr 28, 2022 at 01:04:25PM -0700, Josh Poimboeuf wrote:
> RESERVE_BRK() reserves data in the .brk_reservation section. The data
> is initialized to zero, like BSS, so the macro specifies 'nobits' to
> prevent the data from taking up space in the vmlinux binary. The only
> way to get the compiler to do that (without putting the variable in .bss
> proper) is to use inline asm.
>
> The macro also has a hack which encloses the inline asm in a discarded
> function, which allows the size to be passed (global inline asm doesn't
> allow inputs).
>
> Remove the need for the discarded function hack by just stringifying the
> size rather than supplying it as an input to the inline asm.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> arch/x86/include/asm/setup.h | 30 +++++++++++-------------------
> 1 file changed, 11 insertions(+), 19 deletions(-)

Nice.

Reviewed-by: Borislav Petkov <[email protected]>

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette