From: Ard Biesheuvel <ardb@kernel.org>

This RFT 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 existsing 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.

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.

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

Build tested defconfig and allmodconfig

!!! Boot tested on non-SEV guest ONLY !!!!

Again, I will need to lean on Tom to determine whether this breaks
SEV-SNP guest boot. As I mentioned before, I am still waiting for
SEV-SNP capable hardware to be delivered.

Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
Cc: Kevin Loughlin <kevinloughlin@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>

Ard Biesheuvel (23):
  x86/boot: Move early_setup_gdt() back into head64.c
  x86/boot: Disregard __supported_pte_mask in __startup_64()
  x86/boot: Drop global variables keeping track of LA57 state
  x86/sev: Make sev_snp_enabled() a static function
  x86/sev: Move instruction decoder into separate source file
  x86/sev: Disentangle #VC handling code from startup code
  x86/sev: Separate MSR and GHCB based snp_cpuid() via a callback
  x86/sev: Fall back to early page state change code only during boot
  x86/sev: Move GHCB page based HV communication out of startup code
  x86/sev: Use boot SVSM CA for all startup and init code
  x86/boot: Drop redundant RMPADJUST in SEV SVSM presence check
  x86/sev: Unify SEV-SNP hypervisor feature check
  x86/linkage: Add SYM_PIC_ALIAS() macro helper to emit symbol aliases
  x86/boot: Add a bunch of PIC aliases
  x86/boot: Provide __pti_set_user_pgtbl() to startup code
  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 ordinary use
  x86/boot: Created a confined code area for startup code
  x86/boot: Move startup code out of __head section
  x86/boot: Disallow absolute symbol references in startup code
  x86/boot: Revert "Reject absolute references in .head.text"
  x86/boot: Get rid of the .head.text section

 arch/x86/boot/compressed/Makefile              |    6 +-
 arch/x86/boot/compressed/misc.h                |   12 +-
 arch/x86/boot/compressed/pgtable_64.c          |   12 -
 arch/x86/boot/compressed/sev-handle-vc.c       |  134 +++
 arch/x86/boot/compressed/sev.c                 |  210 +---
 arch/x86/boot/compressed/sev.h                 |   21 +-
 arch/x86/boot/compressed/vmlinux.lds.S         |    1 +
 arch/x86/boot/startup/Makefile                 |   21 +
 arch/x86/boot/startup/exports.h                |   14 +
 arch/x86/boot/startup/gdt_idt.c                |   17 +-
 arch/x86/boot/startup/map_kernel.c             |   18 +-
 arch/x86/boot/startup/sev-shared.c             |  804 +-------------
 arch/x86/boot/startup/sev-startup.c            | 1169 +-------------------
 arch/x86/boot/startup/sme.c                    |   45 +-
 arch/x86/coco/core.c                           |    2 +
 arch/x86/coco/sev/Makefile                     |    6 +-
 arch/x86/coco/sev/core.c                       |  189 +++-
 arch/x86/coco/sev/{sev-nmi.c => sev-noinstr.c} |   74 ++
 arch/x86/coco/sev/vc-handle.c                  | 1060 ++++++++++++++++++
 arch/x86/coco/sev/vc-shared.c                  |  614 ++++++++++
 arch/x86/include/asm/init.h                    |    6 -
 arch/x86/include/asm/linkage.h                 |   10 +
 arch/x86/include/asm/pgtable_64_types.h        |   43 +-
 arch/x86/include/asm/setup.h                   |    2 +
 arch/x86/include/asm/sev-internal.h            |   30 +-
 arch/x86/include/asm/sev.h                     |   78 ++
 arch/x86/kernel/cpu/common.c                   |    3 +-
 arch/x86/kernel/head64.c                       |   27 +-
 arch/x86/kernel/head_32.S                      |    2 +-
 arch/x86/kernel/head_64.S                      |   18 +-
 arch/x86/kernel/setup.c                        |    1 +
 arch/x86/kernel/vmlinux.lds.S                  |   11 +-
 arch/x86/lib/memcpy_64.S                       |    1 +
 arch/x86/lib/memset_64.S                       |    1 +
 arch/x86/lib/retpoline.S                       |    2 +
 arch/x86/mm/kasan_init_64.c                    |    3 -
 arch/x86/mm/mem_encrypt_amd.c                  |    2 +
 arch/x86/mm/mem_encrypt_boot.S                 |    6 +-
 arch/x86/mm/pgtable.c                          |    1 +
 arch/x86/platform/pvh/head.S                   |    2 +-
 arch/x86/tools/relocs.c                        |    8 +-
 tools/objtool/arch/x86/decode.c                |    6 +-
 42 files changed, 2367 insertions(+), 2325 deletions(-)
 create mode 100644 arch/x86/boot/compressed/sev-handle-vc.c
 create mode 100644 arch/x86/boot/startup/exports.h
 rename arch/x86/coco/sev/{sev-nmi.c => sev-noinstr.c} (61%)
 create mode 100644 arch/x86/coco/sev/vc-handle.c
 create mode 100644 arch/x86/coco/sev/vc-shared.c


base-commit: 18ea89eae404d119ced26d80ac3e62255ce15409
-- 
2.49.0.906.g1f30a19c02-goog

* Ard Biesheuvel <ardb+git@google.com> wrote:

> Ard Biesheuvel (23):
>   x86/boot: Move early_setup_gdt() back into head64.c
>   x86/boot: Disregard __supported_pte_mask in __startup_64()
>   x86/boot: Drop global variables keeping track of LA57 state
>   x86/sev: Make sev_snp_enabled() a static function
>   x86/sev: Move instruction decoder into separate source file
>   x86/sev: Disentangle #VC handling code from startup code
>   x86/sev: Separate MSR and GHCB based snp_cpuid() via a callback
>   x86/sev: Fall back to early page state change code only during boot
>   x86/sev: Move GHCB page based HV communication out of startup code
>   x86/sev: Use boot SVSM CA for all startup and init code
>   x86/boot: Drop redundant RMPADJUST in SEV SVSM presence check
>   x86/sev: Unify SEV-SNP hypervisor feature check
>   x86/linkage: Add SYM_PIC_ALIAS() macro helper to emit symbol aliases
>   x86/boot: Add a bunch of PIC aliases
>   x86/boot: Provide __pti_set_user_pgtbl() to startup code
>   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 ordinary use
>   x86/boot: Created a confined code area for startup code
>   x86/boot: Move startup code out of __head section
>   x86/boot: Disallow absolute symbol references in startup code
>   x86/boot: Revert "Reject absolute references in .head.text"
>   x86/boot: Get rid of the .head.text section

>  42 files changed, 2367 insertions(+), 2325 deletions(-)

So to move this forward I applied the following 7 patches to 
tip:x86/boot:

	x86/boot: Move early_setup_gdt() back into head64.c
	x86/boot: Disregard __supported_pte_mask in __startup_64()
	x86/sev: Make sev_snp_enabled() a static function
	x86/sev: Move instruction decoder into separate source file
	x86/linkage: Add SYM_PIC_ALIAS() macro helper to emit symbol aliases
	x86/boot: Add a bunch of PIC aliases
	x86/boot: Provide __pti_set_user_pgtbl() to startup code

Which are I believe independent of SEV testing.

I also merged in pending upstream fixes, including:

   8ed12ab1319b ("x86/boot/sev: Support memory acceptance in the EFI stub under SVSM")

Which should make tip:x86/boot a good base for your series going 
forward?

Thanks,

	Ingo

On Sun, 4 May 2025 at 16:04, Ingo Molnar <mingo@kernel.org> wrote:
>
>
...
>
> So to move this forward I applied the following 7 patches to
> tip:x86/boot:
>
>         x86/boot: Move early_setup_gdt() back into head64.c
>         x86/boot: Disregard __supported_pte_mask in __startup_64()
>         x86/sev: Make sev_snp_enabled() a static function
>         x86/sev: Move instruction decoder into separate source file
>         x86/linkage: Add SYM_PIC_ALIAS() macro helper to emit symbol aliases
>         x86/boot: Add a bunch of PIC aliases
>         x86/boot: Provide __pti_set_user_pgtbl() to startup code
>
> Which are I believe independent of SEV testing.
>

Excellent.

> I also merged in pending upstream fixes, including:
>
>    8ed12ab1319b ("x86/boot/sev: Support memory acceptance in the EFI stub under SVSM")
>
> Which should make tip:x86/boot a good base for your series going
> forward?
>

Yes, that helps a lot, thanks.

Please also consider the patch

  x86/sev: Disentangle #VC handling code from startup code
  11 files changed, 1694 insertions(+), 1643 deletions(-)

It just moves code around, but it is rather large and is likely to
cause merge conflicts if it lives out of tree for too long. The +/-
delta is mostly down to the fact that a new file vc-handle.c is added
which duplicates most of the #includes of the file that it was split
off from.

* Ard Biesheuvel <ardb@kernel.org> wrote:

> On Sun, 4 May 2025 at 16:04, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> ...
> >
> > So to move this forward I applied the following 7 patches to
> > tip:x86/boot:
> >
> >         x86/boot: Move early_setup_gdt() back into head64.c
> >         x86/boot: Disregard __supported_pte_mask in __startup_64()
> >         x86/sev: Make sev_snp_enabled() a static function
> >         x86/sev: Move instruction decoder into separate source file
> >         x86/linkage: Add SYM_PIC_ALIAS() macro helper to emit symbol aliases
> >         x86/boot: Add a bunch of PIC aliases
> >         x86/boot: Provide __pti_set_user_pgtbl() to startup code
> >
> > Which are I believe independent of SEV testing.
> >
> 
> Excellent.
> 
> > I also merged in pending upstream fixes, including:
> >
> >    8ed12ab1319b ("x86/boot/sev: Support memory acceptance in the EFI stub under SVSM")
> >
> > Which should make tip:x86/boot a good base for your series going
> > forward?
> >
> 
> Yes, that helps a lot, thanks.
> 
> Please also consider the patch
> 
>   x86/sev: Disentangle #VC handling code from startup code
>   11 files changed, 1694 insertions(+), 1643 deletions(-)
> 
> It just moves code around, but it is rather large and is likely to
> cause merge conflicts if it lives out of tree for too long. The +/-
> delta is mostly down to the fact that a new file vc-handle.c is added
> which duplicates most of the #includes of the file that it was split
> off from.

Sure, applied, will push it out after testing. I almost applied it 
yesterday, for these exact reasons.

Thanks,

	Ingo

On Sun, May 04, 2025 at 11:52:30AM +0200, Ard Biesheuvel wrote:
>  arch/x86/boot/compressed/Makefile              |    6 +-
>  arch/x86/boot/compressed/misc.h                |   12 +-
>  arch/x86/boot/compressed/pgtable_64.c          |   12 -
>  arch/x86/boot/compressed/sev-handle-vc.c       |  134 +++
>  arch/x86/boot/compressed/sev.c                 |  210 +---
>  arch/x86/boot/compressed/sev.h                 |   21 +-
>  arch/x86/boot/compressed/vmlinux.lds.S         |    1 +
>  arch/x86/boot/startup/Makefile                 |   21 +
>  arch/x86/boot/startup/exports.h                |   14 +
>  arch/x86/boot/startup/gdt_idt.c                |   17 +-
>  arch/x86/boot/startup/map_kernel.c             |   18 +-
>  arch/x86/boot/startup/sev-shared.c             |  804 +-------------
>  arch/x86/boot/startup/sev-startup.c            | 1169 +-------------------
>  arch/x86/boot/startup/sme.c                    |   45 +-
>  arch/x86/coco/core.c                           |    2 +
>  arch/x86/coco/sev/Makefile                     |    6 +-
>  arch/x86/coco/sev/core.c                       |  189 +++-
>  arch/x86/coco/sev/{sev-nmi.c => sev-noinstr.c} |   74 ++

Can we drop the "sev-" prefix to filenames which are already in sev/
filepaths?

>  arch/x86/coco/sev/vc-handle.c                  | 1060 ++++++++++++++++++
>  arch/x86/coco/sev/vc-shared.c                  |  614 ++++++++++
>  arch/x86/include/asm/init.h                    |    6 -
>  arch/x86/include/asm/linkage.h                 |   10 +
>  arch/x86/include/asm/pgtable_64_types.h        |   43 +-
>  arch/x86/include/asm/setup.h                   |    2 +
>  arch/x86/include/asm/sev-internal.h            |   30 +-
>  arch/x86/include/asm/sev.h                     |   78 ++

Pfff, sev-internal and sev.

I guess I'll know how the new structure would look like once I go through this
but there are so many sev* files now.

Can we tone that down pls, through aggregation, moving up into headers and so
on?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On Wed, 7 May 2025 at 11:53, Borislav Petkov <bp@alien8.de> wrote:
>
> On Sun, May 04, 2025 at 11:52:30AM +0200, Ard Biesheuvel wrote:
> >  arch/x86/boot/compressed/Makefile              |    6 +-
> >  arch/x86/boot/compressed/misc.h                |   12 +-
> >  arch/x86/boot/compressed/pgtable_64.c          |   12 -
> >  arch/x86/boot/compressed/sev-handle-vc.c       |  134 +++
> >  arch/x86/boot/compressed/sev.c                 |  210 +---
> >  arch/x86/boot/compressed/sev.h                 |   21 +-
> >  arch/x86/boot/compressed/vmlinux.lds.S         |    1 +
> >  arch/x86/boot/startup/Makefile                 |   21 +
> >  arch/x86/boot/startup/exports.h                |   14 +
> >  arch/x86/boot/startup/gdt_idt.c                |   17 +-
> >  arch/x86/boot/startup/map_kernel.c             |   18 +-
> >  arch/x86/boot/startup/sev-shared.c             |  804 +-------------
> >  arch/x86/boot/startup/sev-startup.c            | 1169 +-------------------
> >  arch/x86/boot/startup/sme.c                    |   45 +-
> >  arch/x86/coco/core.c                           |    2 +
> >  arch/x86/coco/sev/Makefile                     |    6 +-
> >  arch/x86/coco/sev/core.c                       |  189 +++-
> >  arch/x86/coco/sev/{sev-nmi.c => sev-noinstr.c} |   74 ++
>
> Can we drop the "sev-" prefix to filenames which are already in sev/
> filepaths?
>

Sure.

> >  arch/x86/coco/sev/vc-handle.c                  | 1060 ++++++++++++++++++
> >  arch/x86/coco/sev/vc-shared.c                  |  614 ++++++++++
> >  arch/x86/include/asm/init.h                    |    6 -
> >  arch/x86/include/asm/linkage.h                 |   10 +
> >  arch/x86/include/asm/pgtable_64_types.h        |   43 +-
> >  arch/x86/include/asm/setup.h                   |    2 +
> >  arch/x86/include/asm/sev-internal.h            |   30 +-
> >  arch/x86/include/asm/sev.h                     |   78 ++
>
> Pfff, sev-internal and sev.
>
> I guess I'll know how the new structure would look like once I go through this
> but there are so many sev* files now.
>
> Can we tone that down pls, through aggregation, moving up into headers and so
> on?
>

I think the main issue with this code is that everything was in a
single source file, with no structure or layering whatsoever.

So I'd actually argue for splitting this up even more rather than
bundling it all together, although the sev vs sev-internal distinction
is a bit dubious - it would be better to split this across functional
lines.

I added sev-internal.h so that that single mother-of-all-source-files
could be hacked up without exposing implementation details to external
users that were hidden before. I.e., the high-level APIs that other
callers need to use should be in sev.h, and the implementation of that
API should be carved up meaningfully. For example, perhaps the #VC
handling stuff (which now lives in a separate source file) could be
exposed via sev-vc.h, and only included in places where that
particular functionality is being used.

On Wed, May 07, 2025 at 02:05:37PM +0200, Ard Biesheuvel wrote:
> So I'd actually argue for splitting this up even more rather than
> bundling it all together,

Splitting it into logically separated bits? Yes. Just because: no.

The SEV stuff is still largely in motion so once it starts settling down,
splitting it would be the first thing that irks me and I'll go do it.

> although the sev vs sev-internal distinction is a bit dubious - it would be
> better to split this across functional lines.
> 
> I added sev-internal.h so that that single mother-of-all-source-files
> could be hacked up without exposing implementation details to external
> users that were hidden before. I.e., the high-level APIs that other
> callers need to use should be in sev.h, and the implementation of that
> API should be carved up meaningfully. For example, perhaps the #VC
> handling stuff (which now lives in a separate source file) could be
> exposed via sev-vc.h, and only included in places where that
> particular functionality is being used.

Right, please put those considerations in the commit messages - it helps a lot
with the review.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette