[PATCH v7 00/22] x86: strict separation of startup code

Ard Biesheuvel posted 22 patches 1 week ago
arch/x86/Kbuild                            |   2 +
arch/x86/Makefile                          |   1 -
arch/x86/boot/compressed/Makefile          |   2 +-
arch/x86/boot/compressed/misc.c            |   2 +
arch/x86/boot/compressed/sev-handle-vc.c   |   3 +
arch/x86/boot/compressed/sev.c             | 120 ++------
arch/x86/boot/startup/Makefile             |  22 ++
arch/x86/boot/startup/exports.h            |  14 +
arch/x86/boot/startup/gdt_idt.c            |   4 +-
arch/x86/boot/startup/map_kernel.c         |   4 +-
arch/x86/boot/startup/sev-shared.c         | 314 ++++++--------------
arch/x86/boot/startup/sev-startup.c        | 205 ++-----------
arch/x86/boot/startup/sme.c                |  27 +-
arch/x86/coco/sev/Makefile                 |   8 +-
arch/x86/coco/sev/core.c                   | 173 ++++++++---
arch/x86/coco/sev/{sev-nmi.c => noinstr.c} |  74 +++++
arch/x86/coco/sev/vc-handle.c              |   2 +
arch/x86/coco/sev/vc-shared.c              | 143 ++++++++-
arch/x86/include/asm/boot.h                |   2 +
arch/x86/include/asm/init.h                |   6 -
arch/x86/include/asm/setup.h               |   1 +
arch/x86/include/asm/sev-internal.h        |  26 +-
arch/x86/include/asm/sev.h                 |  23 +-
arch/x86/kernel/head64.c                   |   5 +-
arch/x86/kernel/head_32.S                  |   2 +-
arch/x86/kernel/head_64.S                  |  10 +-
arch/x86/kernel/vmlinux.lds.S              |   9 +-
arch/x86/mm/mem_encrypt_amd.c              |   6 -
arch/x86/mm/mem_encrypt_boot.S             |   6 +-
arch/x86/platform/pvh/head.S               |   2 +-
arch/x86/tools/relocs.c                    |   8 +-
drivers/firmware/efi/libstub/x86-stub.c    |   4 +-
tools/objtool/arch/x86/decode.c            |  12 +
tools/objtool/builtin-check.c              |   2 +
tools/objtool/check.c                      |  47 ++-
tools/objtool/include/objtool/arch.h       |   1 +
tools/objtool/include/objtool/builtin.h    |   1 +
37 files changed, 671 insertions(+), 622 deletions(-)
create mode 100644 arch/x86/boot/startup/exports.h
rename arch/x86/coco/sev/{sev-nmi.c => noinstr.c} (61%)
[PATCH v7 00/22] x86: strict separation of startup code
Posted by Ard Biesheuvel 1 week ago
From: Ard Biesheuvel <ardb@kernel.org>

This series implements a strict separation between startup code and
ordinary code, where startup code is built in a way that tolerates being
invoked from the initial 1:1 mapping of memory.

The existing approach of emitting this code into .head.text and checking
for absolute relocations in that section is not 100% safe, and produces
diagnostics that are sometimes difficult to interpret. [0]

Instead, rely on symbol prefixes, similar to how this is implemented for
the EFI stub and for the startup code in the arm64 port. This ensures
that startup code can only call other startup code, unless a special
symbol alias is emitted that exposes a non-startup routine to the
startup code.

This is somewhat intrusive, as there are many data objects that are
referenced both by startup code and by ordinary code, and an alias needs
to be emitted for each of those. If startup code references anything
that has not been made available to it explicitly, a build time link
error will occur.

This ultimately allows the .head.text section to be dropped entirely, as
it no longer has a special significance. Instead, code that only
executes at boot is emitted into .init.text as it should.

The majority of changes is around early SEV code. The main issue is that
its use of GHCB pages and SVSM calling areas in code that may run from
both the 1:1 mapping and the kernel virtual mapping is problematic as it
relies on __pa() to perform VA to PA translations, which are ambiguous
in this context. Also, __pa() pulls in non-trivial instrumented code
when CONFIG_DEBUG_VIRTUAL=y and so it is better to avoid VA to PA
translations altogether in the startup code.

Changes since v6:
- Rebase onto latest tip/master which incorporates v6.17-rc1
- Use a descriptor struct instead of two additional function arguments
  in patch #8
- Clarify in commit log why the latent entropy GCC plugin needs to be
  disabled for startup code
- Apply Tom's Rb to patch #3

Changes since v5:
- Some cosmetic touchups to patch #1
- Fix error spotted by Tom in patch #2
- Rejig objtool 'noabs' feature using an arch hook to check for
  relocations that should be considered absolute, which includes all of
  R_X86_64_32, R_X86_64_32S and R_X86_64_64 on x86_64

Changes since v4:
- Incorporate feedback from Tom, and add a couple of RBs
- Drop patch that moved the MSR save/restore out of the early page state
  change helper - this is less efficient but likely negligible in
  practice
- Drop patch that unified the SEV-SNP hypervisor feature check, which
  was identified by Nikunj as the one breaking SEV-SNP boot.

Changes since RFT/v3:
- Rebase onto tip/master
- Incorporate Borislav's feedback on v3
- Switch to objtool to check for absolute references in startup code
- Remap inittext R-X when running on EFI implementations that require
  strict R-X/RW- separation
- Include a kbuild fix to incorporate arch/x86/boot/startup/ in the
  right manner
- For now, omit the LA57 changes that remove the problematic early
  5-level paging checks. We can revisit this once there is agreement on
  the approach. 

Changes since RFT/v2:
- Rebase onto tip/x86/boot and drop the patches from the previous
  revision that have been applied in the meantime.
- Omit the pgtable_l5_enabled() changes for now, and just expose PIC
  aliases for the variables in question - this can be sorted later.
- Don't use the boot SVSM calling area in snp_kexec_finish(), but pass
  down the correct per-CPU one to the early page state API.
- Rename arch/x86/coco/sev/sev-noinstr.o to arch/x86/coco/sev/noinstr.o
- Further reduce the amount of SEV code that needs to be constructed in
  a special way.

Change since RFC/v1:
- Include a major disentanglement/refactor of the SEV-SNP startup code,
  so that only code that really needs to run from the 1:1 mapping is
  included in the startup/ code

- Incorporate some early notes from Ingo

Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Kevin Loughlin <kevinloughlin@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Nikunj A Dadhania <nikunj@amd.com>

[0] https://lore.kernel.org/all/CAHk-=wj7k9nvJn6cpa3-5Ciwn2RGyE605BMkjWE4MqnvC9E92A@mail.gmail.com/


Ard Biesheuvel (22):
  x86/sev: Separate MSR and GHCB based snp_cpuid() via a callback
  x86/sev: Use MSR protocol for remapping SVSM calling area
  x86/sev: Use MSR protocol only for early SVSM PVALIDATE call
  x86/sev: Run RMPADJUST on SVSM calling area page to test VMPL
  x86/sev: Move GHCB page based HV communication out of startup code
  x86/sev: Avoid global variable to store virtual address of SVSM area
  x86/sev: Share implementation of MSR-based page state change
  x86/sev: Pass SVSM calling area down to early page state change API
  x86/sev: Use boot SVSM CA for all startup and init code
  x86/boot: Drop redundant RMPADJUST in SEV SVSM presence check
  x86/boot: Provide PIC aliases for 5-level paging related constants
  x86/sev: Provide PIC aliases for SEV related data objects
  x86/sev: Move __sev_[get|put]_ghcb() into separate noinstr object
  x86/sev: Export startup routines for later use
  objtool: Add action to check for absence of absolute relocations
  x86/boot: Check startup code for absence of absolute relocations
  x86/boot: Revert "Reject absolute references in .head.text"
  x86/kbuild: Incorporate boot/startup/ via Kbuild makefile
  x86/boot: Create a confined code area for startup code
  efistub/x86: Remap inittext read-execute when needed
  x86/boot: Move startup code out of __head section
  x86/boot: Get rid of the .head.text section

 arch/x86/Kbuild                            |   2 +
 arch/x86/Makefile                          |   1 -
 arch/x86/boot/compressed/Makefile          |   2 +-
 arch/x86/boot/compressed/misc.c            |   2 +
 arch/x86/boot/compressed/sev-handle-vc.c   |   3 +
 arch/x86/boot/compressed/sev.c             | 120 ++------
 arch/x86/boot/startup/Makefile             |  22 ++
 arch/x86/boot/startup/exports.h            |  14 +
 arch/x86/boot/startup/gdt_idt.c            |   4 +-
 arch/x86/boot/startup/map_kernel.c         |   4 +-
 arch/x86/boot/startup/sev-shared.c         | 314 ++++++--------------
 arch/x86/boot/startup/sev-startup.c        | 205 ++-----------
 arch/x86/boot/startup/sme.c                |  27 +-
 arch/x86/coco/sev/Makefile                 |   8 +-
 arch/x86/coco/sev/core.c                   | 173 ++++++++---
 arch/x86/coco/sev/{sev-nmi.c => noinstr.c} |  74 +++++
 arch/x86/coco/sev/vc-handle.c              |   2 +
 arch/x86/coco/sev/vc-shared.c              | 143 ++++++++-
 arch/x86/include/asm/boot.h                |   2 +
 arch/x86/include/asm/init.h                |   6 -
 arch/x86/include/asm/setup.h               |   1 +
 arch/x86/include/asm/sev-internal.h        |  26 +-
 arch/x86/include/asm/sev.h                 |  23 +-
 arch/x86/kernel/head64.c                   |   5 +-
 arch/x86/kernel/head_32.S                  |   2 +-
 arch/x86/kernel/head_64.S                  |  10 +-
 arch/x86/kernel/vmlinux.lds.S              |   9 +-
 arch/x86/mm/mem_encrypt_amd.c              |   6 -
 arch/x86/mm/mem_encrypt_boot.S             |   6 +-
 arch/x86/platform/pvh/head.S               |   2 +-
 arch/x86/tools/relocs.c                    |   8 +-
 drivers/firmware/efi/libstub/x86-stub.c    |   4 +-
 tools/objtool/arch/x86/decode.c            |  12 +
 tools/objtool/builtin-check.c              |   2 +
 tools/objtool/check.c                      |  47 ++-
 tools/objtool/include/objtool/arch.h       |   1 +
 tools/objtool/include/objtool/builtin.h    |   1 +
 37 files changed, 671 insertions(+), 622 deletions(-)
 create mode 100644 arch/x86/boot/startup/exports.h
 rename arch/x86/coco/sev/{sev-nmi.c => noinstr.c} (61%)


base-commit: cab48e960d35f35ff799f7aafce7a7bf7c56d8e3
-- 
2.51.0.268.g9569e192d0-goog
Re: [PATCH v7 00/22] x86: strict separation of startup code
Posted by Borislav Petkov 1 day, 13 hours ago
On Thu, Aug 28, 2025 at 12:22:03PM +0200, Ard Biesheuvel wrote:
> Changes since v6:
> - Rebase onto latest tip/master which incorporates v6.17-rc1a

...

So, due to the interactions with the Secure AVIC stuff, I've been doing some
patch tetris. Two patches: the first one goes ontop of x86/apic and the second
one goes ontop of this set.

Will run some build tests with them first tho...

patch 1:

---

commit aa532319e46228422f7deb8d54853c4b218276f1 (HEAD -> refs/heads/tip-x86-apic)
Author: Borislav Petkov (AMD) <bp@alien8.de>
Date:   Wed Sep 3 17:42:05 2025 +0200

    WIP
    
    Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index b64f43010a12..e858e2979db0 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1129,7 +1129,7 @@ u64 savic_ghcb_msr_read(u32 reg)
 	if (res != ES_OK) {
 		pr_err("Secure AVIC MSR (0x%llx) read returned error (%d)\n", msr, res);
 		/* MSR read failures are treated as fatal errors */
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL);
 	}
 
 	__sev_put_ghcb(&state);
@@ -1159,7 +1159,7 @@ void savic_ghcb_msr_write(u32 reg, u64 value)
 	if (res != ES_OK) {
 		pr_err("Secure AVIC MSR (0x%llx) write returned error (%d)\n", msr, res);
 		/* MSR writes should never fail. Any failure is fatal error for SNP guest */
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL);
 	}
 
 	__sev_put_ghcb(&state);
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 0020d77a0800..01a6e4dbe423 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -208,6 +208,7 @@ struct snp_psc_desc {
 #define GHCB_TERM_SVSM_CAA		9	/* SVSM is present but CAA is not page aligned */
 #define GHCB_TERM_SECURE_TSC		10	/* Secure TSC initialization failed */
 #define GHCB_TERM_SVSM_CA_REMAP_FAIL	11	/* SVSM is present but CA could not be remapped */
+#define GHCB_TERM_SAVIC_FAIL		12	/* Secure AVIC-specific failure */
 
 #define GHCB_RESP_CODE(v)		((v) & GHCB_MSR_INFO_MASK)
 
diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c
index b846de0fbcfa..2b82bb64055a 100644
--- a/arch/x86/kernel/apic/x2apic_savic.c
+++ b/arch/x86/kernel/apic/x2apic_savic.c
@@ -363,7 +363,7 @@ static void savic_setup(void)
 	 */
 	res = savic_register_gpa(gpa);
 	if (res != ES_OK)
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL);
 
 	native_wrmsrq(MSR_AMD64_SAVIC_CONTROL,
 		      gpa | MSR_AMD64_SAVIC_EN | MSR_AMD64_SAVIC_ALLOWEDNMI);
@@ -376,13 +376,13 @@ static int savic_probe(void)
 
 	if (!x2apic_mode) {
 		pr_err("Secure AVIC enabled in non x2APIC mode\n");
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL);
 		/* unreachable */
 	}
 
 	savic_page = alloc_percpu(struct secure_avic_page);
 	if (!savic_page)
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL);;
 
 	return 1;
 }

---

patch 2

---

commit 07d41a19c5a01506e1080e352c26c50c8dce6e6b (refs/remotes/ps2/tip-x86-sev, refs/remotes/ps2/HEAD)
Author: Borislav Petkov (AMD) <bp@alien8.de>
Date:   Wed Sep 3 18:14:54 2025 +0200

    WIP
    
    Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>

diff --git a/arch/x86/boot/startup/sev-startup.c b/arch/x86/boot/startup/sev-startup.c
index 39465a0ff4e5..a9b0a9c32d8f 100644
--- a/arch/x86/boot/startup/sev-startup.c
+++ b/arch/x86/boot/startup/sev-startup.c
@@ -144,7 +144,7 @@ static struct cc_blob_sev_info *__init find_cc_blob(struct boot_params *bp)
 
 found_cc_info:
 	if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
 
 	return cc_info;
 }
@@ -218,8 +218,3 @@ bool __init snp_init(struct boot_params *bp)
 
 	return true;
 }
-
-void __init __noreturn snp_abort(void)
-{
-	sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
-}
diff --git a/arch/x86/boot/startup/sme.c b/arch/x86/boot/startup/sme.c
index 2ddde901c8c5..e7ea65f3f1d6 100644
--- a/arch/x86/boot/startup/sme.c
+++ b/arch/x86/boot/startup/sme.c
@@ -532,7 +532,7 @@ void __init sme_enable(struct boot_params *bp)
 	 * enablement abort the guest.
 	 */
 	if (snp_en ^ !!(msr & MSR_AMD64_SEV_SNP_ENABLED))
-		snp_abort();
+		sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
 
 	/* Check if memory encryption is enabled */
 	if (feature_mask == AMD_SME_BIT) {
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index f222bef9dca8..32c7dd916e4b 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -512,7 +512,6 @@ void snp_set_memory_shared(unsigned long vaddr, unsigned long npages);
 void snp_set_memory_private(unsigned long vaddr, unsigned long npages);
 void snp_set_wakeup_secondary_cpu(void);
 bool snp_init(struct boot_params *bp);
-void __noreturn snp_abort(void);
 void snp_dmi_setup(void);
 int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input);
 void snp_accept_memory(phys_addr_t start, phys_addr_t end);
@@ -597,7 +596,6 @@ static inline void snp_set_memory_shared(unsigned long vaddr, unsigned long npag
 static inline void snp_set_memory_private(unsigned long vaddr, unsigned long npages) { }
 static inline void snp_set_wakeup_secondary_cpu(void) { }
 static inline bool snp_init(struct boot_params *bp) { return false; }
-static inline void snp_abort(void) { }
 static inline void snp_dmi_setup(void) { }
 static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input)
 {
diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
index 6a922d046b8e..802895fae3ca 100644
--- a/tools/objtool/noreturns.h
+++ b/tools/objtool/noreturns.h
@@ -45,7 +45,6 @@ NORETURN(rewind_stack_and_make_dead)
 NORETURN(rust_begin_unwind)
 NORETURN(rust_helper_BUG)
 NORETURN(sev_es_terminate)
-NORETURN(snp_abort)
 NORETURN(start_kernel)
 NORETURN(stop_this_cpu)
 NORETURN(usercopy_abort)


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v7 00/22] x86: strict separation of startup code
Posted by Ard Biesheuvel 23 hours ago
On Wed, 3 Sept 2025 at 18:22, Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Aug 28, 2025 at 12:22:03PM +0200, Ard Biesheuvel wrote:
> > Changes since v6:
> > - Rebase onto latest tip/master which incorporates v6.17-rc1a
>
> ...
>
> So, due to the interactions with the Secure AVIC stuff, I've been doing some
> patch tetris. Two patches: the first one goes ontop of x86/apic and the second
> one goes ontop of this set.
>

Looks straight-forward enough ...