Re: [RFC PATCH] x86/sev: x86/sev: enforce PC-relative addressing in clang

From: Kevin Loughlin <kevinloughlin@google.com>
To: Andi Kleen <ak@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	 Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	 Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	 Rick Edgecombe <rick.p.edgecombe@intel.com>,
	Kees Cook <keescook@chromium.org>,
	 "Masami Hiramatsu (Google)" <mhiramat@kernel.org>,
	Ze Gao <zegao2021@gmail.com>,
	 Josh Poimboeuf <jpoimboe@kernel.org>,
	Pengfei Xu <pengfei.xu@intel.com>,
	 Brijesh Singh <brijesh.singh@amd.com>,
	Michael Roth <michael.roth@amd.com>,
	 Ashish Kalra <ashish.kalra@amd.com>,
	 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	 Joerg Roedel <jroedel@suse.de>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
	 linux-coco@lists.linux.dev, Adam Dunlap <acdunlap@google.com>,
	 Peter Gonda <pgonda@google.com>, Jacob Xu <jacobhxu@google.com>,
	 Sidharth Telang <sidtelang@google.com>
Subject: Re: [RFC PATCH] x86/sev: x86/sev: enforce PC-relative addressing in clang
Date: Wed, 10 Jan 2024 09:14:21 -0800	[thread overview]
Message-ID: <CAGdbjmJcQeLWroBkbRC-cFQ6OuMHONsGLzpUrrA5k3uc8Rzkmw@mail.gmail.com> (raw)
In-Reply-To: <ZZ6DSdaaoYKzgp4K@tassilo>

On Wed, Jan 10, 2024 at 3:45 AM Andi Kleen <ak@linux.intel.com> wrote:
>
> On Wed, Jan 10, 2024 at 01:26:39AM +0000, Kevin Loughlin wrote:
> >
> > While an attempt was made to force PC-relative addressing for certain
> > global SEV/SME variables via inline assembly (see snp_cpuid_get_table()
> > for example), PC-relative addressing must be pervasively-enforced for
> > SEV/SME global variables that can be accessed prior to page table
> > fixups.
> >
> > To avoid the error-prone approach of manually referencing each SEV/SME
> > global variable via a general form of snp_cpuid_get_table(), it is
> > preferable to use compiler flags for position-independent code (ex:
>
> But if gcc doesn't support it then it doesn't work.
>
> It seems your approach with incompatible execution models between
> the compilers is just a recipe for future patches only working
> on one of the compilers because most code submitters probably
> won't test both.
>
> It would be better to at least use a unified execution model, if you want
> to extend the hack and not fix the underlying issue.

Fair point; while gcc (unlike clang) appears to currently generate
PC-relative accesses to these vars, the patch in its current state
would not account for gcc's behavior potentially changing in the
future.

On that note, I do have another version of this patch that abstracts
snp_cpuid_get_table() into a macro along the lines of...

#define GET_RIP_RELATIVE_PTR(var) \
({  \
 void *ptr; \
 asm ("lea "#var"(%%rip), %0" \
      : "=r" (ptr) \
      : "p" (&var)); \
 ptr; \
})

...and uses this new macro to access all SEV/SME global variables (not
just the cpuid_table). It's similar in nature to `fixup_pointer()`
(currently defined in arch/x86/kernel/head64.c) but doesn't require us
to pass around `physaddr` from `__startup64()`. This wouldn't
introduce any new execution model changes between clang vs gcc and
would be consistent with the kernel's current approach of relying on
developers to manually apply fixups for global variable accesses prior
to kernel relocation. I can send an RFC v2 for the
GET_RIP_RELATIVE_PTR() version of this patch.