[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b5e2a11-b157-1288-f99b-cd8c7b8180d0@amd.com>
Date: Wed, 16 Jul 2025 09:27:23 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org
Cc: linux-efi@...r.kernel.org, x86@...nel.org,
Ard Biesheuvel <ardb@...nel.org>, Borislav Petkov <bp@...en8.de>,
Ingo Molnar <mingo@...nel.org>, Kevin Loughlin <kevinloughlin@...gle.com>,
Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
Nikunj A Dadhania <nikunj@....com>
Subject: Re: [PATCH v5 00/22] x86: strict separation of startup code
On 7/15/25 22:18, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@...nel.org>
Hi Ard,
I tried to apply this to tip/master but ran into conflicts. What commit
is the series based on?
Thanks,
Tom
>
> 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 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@...en8.de>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Kevin Loughlin <kevinloughlin@...gle.com>
> Cc: Tom Lendacky <thomas.lendacky@....com>
> Cc: Josh Poimboeuf <jpoimboe@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Nikunj A Dadhania <nikunj@....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 | 108 +------
> 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 | 317 ++++++--------------
> arch/x86/boot/startup/sev-startup.c | 196 ++----------
> arch/x86/boot/startup/sme.c | 27 +-
> arch/x86/coco/sev/Makefile | 6 +-
> arch/x86/coco/sev/core.c | 169 ++++++++---
> 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 | 27 +-
> arch/x86/include/asm/sev.h | 17 +-
> 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/builtin-check.c | 2 +
> tools/objtool/check.c | 39 ++-
> tools/objtool/include/objtool/builtin.h | 1 +
> 35 files changed, 620 insertions(+), 623 deletions(-)
> create mode 100644 arch/x86/boot/startup/exports.h
> rename arch/x86/coco/sev/{sev-nmi.c => noinstr.c} (61%)
>
Powered by blists - more mailing lists