Re: [PATCH v8 04/14] linkage: add macros for putting ASM functions into own sections

[Alexander Lobakin]

From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Date: Fri, 3 Dec 2021 10:31:30 +0100

> On Thu, Dec 02, 2021 at 11:32:04PM +0100, Alexander Lobakin wrote:
> 
> > diff --git a/include/linux/linkage.h b/include/linux/linkage.h
> > index dbf8506decca..baaab7dece08 100644
> > --- a/include/linux/linkage.h
> > +++ b/include/linux/linkage.h
> > @@ -355,4 +355,86 @@
> >  
> >  #endif /* __ASSEMBLY__ */
> >  
> > +/*
> > + * Allow ASM symbols to have their own unique sections if they are being
> > + * generated by the compiler for C functions (DCE, FG-KASLR, LTO).
> > + */
> > +#if (defined(CONFIG_LD_DEAD_CODE_DATA_ELIMINATION) && !defined(MODULE)) || \
> > +    (defined(CONFIG_FG_KASLR) && !defined(MODULE)) || \
> > +    (defined(CONFIG_MODULE_FG_KASLR) && defined(MODULE)) || \
> > +    (defined(CONFIG_LTO_CLANG))
> > +
> > +#define SYM_TEXT_SECTION(name)				\
> > +	.pushsection .text.##name, "ax"
> > +
> > +#define ASM_TEXT_SECTION(name)				\
> > +	".text." #name
> > +
> > +#define ASM_PUSH_SECTION(name)				\
> > +	".pushsection .text." #name ", \"ax\""
> > +
> > +#else /* just .text */
> > +
> > +#define SYM_TEXT_SECTION(name)				\
> > +	.pushsection .text, "ax"
> > +
> > +#define ASM_TEXT_SECTION(name)				\
> > +	".text"
> > +
> > +#define ASM_PUSH_SECTION(name)				\
> > +	".pushsection .text, \"ax\""
> > +
> > +#endif /* just .text */
> 
> That's terribly inconsistent, SYM_TEXT_SECTION is in fact
> PUSH_TEXT_SECTION, ASM_PUSH_SECTION is in fact ASM_PUSH_TEXT_SECTION and
> should be stringify(PUSH_TEXT_SECTION()) or something, and they're all
> repeating that ASM_TEXT_SECTION thing :/

Right. In fact I was waiting for someone to review them to pick more
fitting names, so I'll change them for sure.

> > +
> > +#ifdef __ASSEMBLY__
> > +
> > +#define SYM_TEXT_END_SECTION				\
> > +	.popsection
> > +
> > +#define SYM_FUNC_START_LOCAL_ALIAS_SECTION(name)	\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_FUNC_START_LOCAL_ALIAS(name)
> > +
> > +#define SYM_FUNC_START_LOCAL_SECTION(name)		\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_FUNC_START_LOCAL(name)
> > +
> > +#define SYM_FUNC_START_NOALIGN_SECTION(name)		\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_FUNC_START_NOALIGN(name)
> > +
> > +#define SYM_FUNC_START_WEAK_SECTION(name)		\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_FUNC_START_WEAK(name)
> > +
> > +#define SYM_FUNC_START_SECTION(name)			\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_FUNC_START(name)
> > +
> > +#define SYM_CODE_START_LOCAL_NOALIGN_SECTION(name)	\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_CODE_START_LOCAL_NOALIGN(name)
> > +
> > +#define SYM_CODE_START_NOALIGN_SECTION(name)		\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_CODE_START_NOALIGN(name)
> > +
> > +#define SYM_CODE_START_SECTION(name)			\
> > +	SYM_TEXT_SECTION(name) ASM_NL			\
> > +	SYM_CODE_START(name)
> > +
> > +#define SYM_FUNC_END_ALIAS_SECTION(name)		\
> > +	SYM_FUNC_END_ALIAS(name) ASM_NL			\
> > +	SYM_TEXT_END_SECTION
> > +
> > +#define SYM_FUNC_END_SECTION(name)			\
> > +	SYM_FUNC_END(name) ASM_NL			\
> > +	SYM_TEXT_END_SECTION
> > +
> > +#define SYM_CODE_END_SECTION(name)			\
> > +	SYM_CODE_END(name) ASM_NL			\
> > +	SYM_TEXT_END_SECTION
> > +
> > +#endif /* __ASSEMBLY__ */
> 
> *URGH* why do we have to have new macros for this? SYM_FUNC_START*()
> already takes the name as argument.

I'm not sure whether we should have new macros for this.
I introduced them since there's plenty of code which use
SYM_FUNC_*(), but does .{push,pop}section outside of it, e.g. to
place something to .noinstr or .init. If I'd replace them, those
would be broken.
So I'm fine with the replacement, but those cases need to be
adressed somehow then. Are there any tricks to check if we're
already outside of .text to not push it into a new section?

Al