From:   Alexander Lobakin <alexandr.lobakin@intel.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Alexander Lobakin <alexandr.lobakin@intel.com>,
        linux-hardening@vger.kernel.org, x86@kernel.org,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Kristen Carlson Accardi <kristen@linux.intel.com>,
        Kees Cook <keescook@chromium.org>,
        Miklos Szeredi <miklos@szeredi.hu>,
        Ard Biesheuvel <ardb@kernel.org>,
        Tony Luck <tony.luck@intel.com>,
        Bruce Schlobohm <bruce.schlobohm@intel.com>,
        Jessica Yu <jeyu@kernel.org>,
        kernel test robot <lkp@intel.com>,
        Miroslav Benes <mbenes@suse.cz>,
        Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <will@kernel.org>, Ingo Molnar <mingo@redhat.com>,
        Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Andy Lutomirski <luto@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Marios Pomonis <pomonis@google.com>,
        Sami Tolvanen <samitolvanen@google.com>,
        linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
        linux-arch@vger.kernel.org, live-patching@vger.kernel.org,
        llvm@lists.linux.dev
Subject: Re: [PATCH v8 04/14] linkage: add macros for putting ASM functions into own sections
Date:   Fri,  3 Dec 2021 15:08:20 +0100
Message-Id: <20211203140820.82341-1-alexandr.lobakin@intel.com>
In-Reply-To: <Yanj8qvo3Wj4ePyV@hirez.programming.kicks-ass.net>
References: <20211202223214.72888-1-alexandr.lobakin@intel.com> <20211202223214.72888-5-alexandr.lobakin@intel.com> <Yanj8qvo3Wj4ePyV@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Peter Zijlstra <peterz@infradead.org>
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