[RFC] x86/sev: x86/sev: enforce PC-relative addressing in clang
Commit Message
SEV/SME code can execute prior to page table fixups for kernel
relocation. However, as with global variables accessed in
__startup_64(), clang does not currently generate PC-relative accesses
for SEV/SME global variables, causing certain flavors of SEV hosts and
guests to crash.
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:
`-fPIE`) that result in PC-relative accesses. While architecture-
specific code for Linux can be pervasively compiled as position-
independent on select architectures (ex: RISC-V), this is not currently
the case for x86-64 and would require extensive changes (see "[PATCH
RFC 00/43] x86/pie: Make kernel image's virtual address flexible" for
example).
Fortunately, the relevant files for SEV/SME code do indeed support
position-independent clang compilation, so we can use this technique to
ensure all global variables in these files are accessed via PC-relative
addressing.
Unlike clang, gcc does not currently allow `-fPIE` in conjunction with
`mcmodel=kernel`. Thus, to preserve existing gcc behavior, this patch
does not remove the (otherwise unnecessary) inline assembly that
already enforces PC-relative addressing for select SEV/SME globals
(mentioned above). If gcc supports these joint options in the future,
we can remove such inline assembly and also apply this patch to gcc
builds.
Tested by successful boot of SEV-SNP guest built with clang, alongside
Adam Dunlap's necessary "[PATCH v2] x86/asm: Force native_apic_mem_read
to use mov".
Fixes: 95d33bfaa3e1 ("x86/sev: Register GHCB memory when SEV-SNP is active")
Fixes: ee0bfa08a345 ("x86/compressed/64: Add support for SEV-SNP CPUID table in #VC handlers")
Fixes: 1cd9c22fee3a ("x86/mm/encrypt: Move page table helpers into separate translation unit")
Fixes: c9f09539e16e ("x86/head/64: Check SEV encryption before switching to kernel page-table")
Fixes: b577f542f93c ("x86/coco: Add API to handle encryption mask")
Tested-by: Kevin Loughlin <kevinloughlin@google.com>
Signed-off-by: Kevin Loughlin <kevinloughlin@google.com>
---
arch/x86/coco/Makefile | 10 ++++++++++
arch/x86/kernel/Makefile | 10 ++++++++++
arch/x86/mm/Makefile | 13 +++++++++++++
3 files changed, 33 insertions(+)
Comments
On Wed, Jan 10, 2024 at 01:26:39AM +0000, Kevin Loughlin wrote:
> SEV/SME code can execute prior to page table fixups for kernel
That seems to be fundamental problem.
> relocation. However, as with global variables accessed in
> __startup_64(), clang does not currently generate PC-relative accesses
> for SEV/SME global variables, causing certain flavors of SEV hosts and
> guests to crash.
And that's a hack to work around it.
>
> 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.
-Andi
On Wed, Jan 10, 2024 at 01:26:39AM +0000, Kevin Loughlin wrote:
> SEV/SME code can execute prior to page table fixups for kernel
> relocation. However, as with global variables accessed in
> __startup_64(), clang does not currently generate PC-relative accesses
> for SEV/SME global variables, causing certain flavors of SEV hosts and
> guests to crash.
>
> 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:
> `-fPIE`) that result in PC-relative accesses. While architecture-
> specific code for Linux can be pervasively compiled as position-
> independent on select architectures (ex: RISC-V), this is not currently
> the case for x86-64 and would require extensive changes (see "[PATCH
> RFC 00/43] x86/pie: Make kernel image's virtual address flexible" for
> example).
>
> Fortunately, the relevant files for SEV/SME code do indeed support
> position-independent clang compilation, so we can use this technique to
> ensure all global variables in these files are accessed via PC-relative
> addressing.
>
> Unlike clang, gcc does not currently allow `-fPIE` in conjunction with
> `mcmodel=kernel`. Thus, to preserve existing gcc behavior, this patch
> does not remove the (otherwise unnecessary) inline assembly that
> already enforces PC-relative addressing for select SEV/SME globals
> (mentioned above). If gcc supports these joint options in the future,
> we can remove such inline assembly and also apply this patch to gcc
> builds.
>
> Tested by successful boot of SEV-SNP guest built with clang, alongside
> Adam Dunlap's necessary "[PATCH v2] x86/asm: Force native_apic_mem_read
> to use mov".
>
Similar issues was fixed before with fixup_pointer() tricks. Have you
tried looking this direction.
Relevant thread starting with:
https://lore.kernel.org/all/20210920192341.maue7db4lcbdn46x@box.shutemov.name
> 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.
That looks like a far better solution indeed.
Ideally objtool would check for this, perhaps with a new ELF
section. But actually doing that might be far more work, so perhaps not
worth it.
Thanks,
-Andi
@@ -5,4 +5,14 @@ CFLAGS_core.o += -fno-stack-protector
obj-y += core.o
+# clang allows -fPIE with mcmodel=kernel; gcc currently does not.
+ifdef CONFIG_CC_IS_CLANG
+# Enforce PC-relative addressing for SEV/SME global variables.
+CFLAGS_core.o += -fPIE
+# Disable relocation relaxation in case the link is not PIE.
+CFLAGS_core.o += $(call cc-option,-Wa$(comma)-mrelax-relocations=no)
+# Avoid unnecessary GOT overhead in PC-relative addressing.
+CFLAGS_core.o += -include $(srctree)/include/linux/hidden.h
+endif
+
obj-$(CONFIG_INTEL_TDX_GUEST) += tdx/
@@ -7,6 +7,16 @@ extra-y += vmlinux.lds
CPPFLAGS_vmlinux.lds += -U$(UTS_MACHINE)
+# clang allows -fPIE with mcmodel=kernel; gcc currently does not.
+ifdef CONFIG_CC_IS_CLANG
+# Enforce PC-relative addressing for SEV/SME global variables.
+CFLAGS_sev.o += -fPIE
+# Disable relocation relaxation in case the link is not PIE.
+CFLAGS_sev.o += $(call cc-option,-Wa$(comma)-mrelax-relocations=no)
+# Avoid unnecessary GOT overhead in PC-relative addressing.
+CFLAGS_sev.o += -include $(srctree)/include/linux/hidden.h
+endif
+
ifdef CONFIG_FUNCTION_TRACER
# Do not profile debug and lowlevel utilities
CFLAGS_REMOVE_tsc.o = -pg
@@ -17,6 +17,19 @@ KCSAN_SANITIZE := n
# Avoid recursion by not calling KMSAN hooks for CEA code.
KMSAN_SANITIZE_cpu_entry_area.o := n
+# clang allows -fPIE with mcmodel=kernel; gcc currently does not.
+ifdef CONFIG_CC_IS_CLANG
+# Enforce PC-relative addressing for SEV/SME global variables.
+CFLAGS_mem_encrypt_amd.o += -fPIE
+CFLAGS_mem_encrypt_identity.o += -fPIE
+# Disable relocation relaxation in case the link is not PIE.
+CFLAGS_mem_encrypt_amd.o += $(call cc-option,-Wa$(comma)-mrelax-relocations=no)
+CFLAGS_mem_encrypt_identity.o += $(call cc-option,-Wa$(comma)-mrelax-relocations=no)
+# Avoid unnecessary GOT overhead in PC-relative addressing.
+CFLAGS_mem_encrypt_amd.o += -include $(srctree)/include/linux/hidden.h
+CFLAGS_mem_encrypt_identity.o += -include $(srctree)/include/linux/hidden.h
+endif
+
ifdef CONFIG_FUNCTION_TRACER
CFLAGS_REMOVE_mem_encrypt.o = -pg
CFLAGS_REMOVE_mem_encrypt_amd.o = -pg