mbox series

[v4,00/11] x86: Refactor and consolidate startup code

Message ID 20250410134117.3713574-13-ardb+git@google.com
Headers show
Series x86: Refactor and consolidate startup code | expand

Message

Ard Biesheuvel April 10, 2025, 1:41 p.m. UTC
From: Ard Biesheuvel <ardb@kernel.org>

!! NOTE: patches #7 - #10 depend on [0] !!

Reorganize C code that is used during early boot, either in the
decompressor/EFI stub or the kernel proper, but before the kernel
virtual mapping is up.

v4:
- drop patches that were queued up
- fix address space error in patch #1
- add patches for SEV-SNP boot code - these cannot be applied yet, but
  are included for completeness

v3:
- keep rip_rel_ptr() around in PIC code - sadly, it is still needed in
  some cases
- remove RIP_REL_REF() uses in separate patches
- keep __head annotations for now, they will all be removed later
- disable objtool validation for library objects (i.e., pieces that are
  not linked into vmlinux)

I will follow up with a series that gets rid of .head.text altogether,
as it will no longer be needed at all once the startup code is checked
for absolute relocations.

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
Cc: Kevin Loughlin <kevinloughlin@google.com>

[0] https://lore.kernel.org/all/20250410132850.3708703-2-ardb+git@google.com/T/#u

Ard Biesheuvel (11):
  x86/asm: Make rip_rel_ptr() usable from fPIC code
  x86/boot: Move the early GDT/IDT setup code into startup/
  x86/boot: Move early kernel mapping code into startup/
  x86/boot: Drop RIP_REL_REF() uses from early mapping code
  x86/boot: Move early SME init code into startup/
  x86/boot: Drop RIP_REL_REF() uses from SME startup code
  x86/sev: Prepare for splitting off early SEV code
  x86/sev: Split off startup code from core code
  x86/boot: Move SEV startup code into startup/
  x86/boot: Drop RIP_REL_REF() uses from early SEV code
  x86/asm: Retire RIP_REL_REF()

 arch/x86/boot/compressed/Makefile                          |    2 +-
 arch/x86/boot/compressed/sev.c                             |   17 +-
 arch/x86/boot/startup/Makefile                             |   16 +
 arch/x86/boot/startup/gdt_idt.c                            |   84 +
 arch/x86/boot/startup/map_kernel.c                         |  225 +++
 arch/x86/{coco/sev/shared.c => boot/startup/sev-shared.c}  |  375 +----
 arch/x86/boot/startup/sev-startup.c                        | 1395 ++++++++++++++++
 arch/x86/{mm/mem_encrypt_identity.c => boot/startup/sme.c} |   19 +-
 arch/x86/coco/sev/Makefile                                 |   19 -
 arch/x86/coco/sev/core.c                                   | 1726 ++++----------------
 arch/x86/include/asm/asm.h                                 |    5 -
 arch/x86/include/asm/coco.h                                |    2 +-
 arch/x86/include/asm/mem_encrypt.h                         |    2 +-
 arch/x86/include/asm/sev-internal.h                        |  112 ++
 arch/x86/include/asm/sev.h                                 |   37 +
 arch/x86/kernel/head64.c                                   |  285 +---
 arch/x86/mm/Makefile                                       |    6 -
 17 files changed, 2208 insertions(+), 2119 deletions(-)
 create mode 100644 arch/x86/boot/startup/gdt_idt.c
 create mode 100644 arch/x86/boot/startup/map_kernel.c
 rename arch/x86/{coco/sev/shared.c => boot/startup/sev-shared.c} (78%)
 create mode 100644 arch/x86/boot/startup/sev-startup.c
 rename arch/x86/{mm/mem_encrypt_identity.c => boot/startup/sme.c} (97%)
 create mode 100644 arch/x86/include/asm/sev-internal.h

Comments

Borislav Petkov April 11, 2025, 7:15 p.m. UTC | #1
On Thu, Apr 10, 2025 at 03:41:18PM +0200, Ard Biesheuvel wrote:
> Ard Biesheuvel (11):
>   x86/asm: Make rip_rel_ptr() usable from fPIC code
>   x86/boot: Move the early GDT/IDT setup code into startup/
>   x86/boot: Move early kernel mapping code into startup/
>   x86/boot: Drop RIP_REL_REF() uses from early mapping code
>   x86/boot: Move early SME init code into startup/
>   x86/boot: Drop RIP_REL_REF() uses from SME startup code
>   x86/sev: Prepare for splitting off early SEV code
>   x86/sev: Split off startup code from core code
>   x86/boot: Move SEV startup code into startup/
>   x86/boot: Drop RIP_REL_REF() uses from early SEV code
>   x86/asm: Retire RIP_REL_REF()
> 
>  arch/x86/boot/compressed/Makefile                          |    2 +-
>  arch/x86/boot/compressed/sev.c                             |   17 +-
>  arch/x86/boot/startup/Makefile                             |   16 +
>  arch/x86/boot/startup/gdt_idt.c                            |   84 +
>  arch/x86/boot/startup/map_kernel.c                         |  225 +++
>  arch/x86/{coco/sev/shared.c => boot/startup/sev-shared.c}  |  375 +----
>  arch/x86/boot/startup/sev-startup.c                        | 1395 ++++++++++++++++
>  arch/x86/{mm/mem_encrypt_identity.c => boot/startup/sme.c} |   19 +-
>  arch/x86/coco/sev/Makefile                                 |   19 -
>  arch/x86/coco/sev/core.c                                   | 1726 ++++----------------
>  arch/x86/include/asm/asm.h                                 |    5 -
>  arch/x86/include/asm/coco.h                                |    2 +-
>  arch/x86/include/asm/mem_encrypt.h                         |    2 +-
>  arch/x86/include/asm/sev-internal.h                        |  112 ++
>  arch/x86/include/asm/sev.h                                 |   37 +
>  arch/x86/kernel/head64.c                                   |  285 +---
>  arch/x86/mm/Makefile                                       |    6 -
>  17 files changed, 2208 insertions(+), 2119 deletions(-)
>  create mode 100644 arch/x86/boot/startup/gdt_idt.c
>  create mode 100644 arch/x86/boot/startup/map_kernel.c
>  rename arch/x86/{coco/sev/shared.c => boot/startup/sev-shared.c} (78%)
>  create mode 100644 arch/x86/boot/startup/sev-startup.c
>  rename arch/x86/{mm/mem_encrypt_identity.c => boot/startup/sme.c} (97%)
>  create mode 100644 arch/x86/include/asm/sev-internal.h

Looks sensible at a glance. The devil's in the detail with that stuff,
ofc, so we will have to test it with as many toolchains and usage
scenarios as possible.

Thx.
Ingo Molnar April 12, 2025, 12:23 p.m. UTC | #2
* Ard Biesheuvel <ardb+git@google.com> wrote:

> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Disentangle the SEV core code and the SEV code that is called during
> early boot. The latter piece will be moved into startup/ in a subsequent
> patch.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/boot/compressed/sev.c |    2 +
>  arch/x86/coco/sev/Makefile     |   12 +-
>  arch/x86/coco/sev/core.c       | 1574 ++++----------------
>  arch/x86/coco/sev/shared.c     |  281 ----
>  arch/x86/coco/sev/startup.c    | 1395 +++++++++++++++++
>  5 files changed, 1658 insertions(+), 1606 deletions(-)

x86-64 allmodconfig build failure:

arch/x86/boot/compressed/sev.c:263:13: error: implicit declaration of function ‘vmgexit_psc’ [-Wimplicit-function-declaration]
|             ^~~~~~~~~~~
arch/x86/boot/compressed/sev.c:266:9: error: implicit declaration of function ‘pvalidate_pages’; did you mean ‘pvalidate_4k_page’? [-Wimplicit-function-declaration]
|         ^~~~~~~~~~~~~~~
|         pvalidate_4k_page

Thanks,

	Ingo
Ingo Molnar April 12, 2025, 6:47 p.m. UTC | #3
* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Ard Biesheuvel <ardb+git@google.com> wrote:
> 
> > From: Ard Biesheuvel <ardb@kernel.org>
> > 
> > Disentangle the SEV core code and the SEV code that is called during
> > early boot. The latter piece will be moved into startup/ in a subsequent
> > patch.
> > 
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/x86/boot/compressed/sev.c |    2 +
> >  arch/x86/coco/sev/Makefile     |   12 +-
> >  arch/x86/coco/sev/core.c       | 1574 ++++----------------
> >  arch/x86/coco/sev/shared.c     |  281 ----
> >  arch/x86/coco/sev/startup.c    | 1395 +++++++++++++++++
> >  5 files changed, 1658 insertions(+), 1606 deletions(-)
> 
> x86-64 allmodconfig build failure:
> 
> arch/x86/boot/compressed/sev.c:263:13: error: implicit declaration of function ‘vmgexit_psc’ [-Wimplicit-function-declaration]
> |             ^~~~~~~~~~~
> arch/x86/boot/compressed/sev.c:266:9: error: implicit declaration of function ‘pvalidate_pages’; did you mean ‘pvalidate_4k_page’? [-Wimplicit-function-declaration]
> |         ^~~~~~~~~~~~~~~
> |         pvalidate_4k_page

Ignore that, I have now read the cover letter too, with the patch 
dependency mentioned there - as kindly pointed out by Ard in a private 
mail. :-)

Thanks,

	Ingo
Ingo Molnar April 12, 2025, 8:08 p.m. UTC | #4
* Ingo Molnar <mingo@kernel.org> wrote:

> Ignore that, I have now read the cover letter too, with the patch 
> dependency mentioned there - as kindly pointed out by Ard in a 
> private mail. :-)

But there are other problems during the allmodconfig final link:

  vmlinux.o: warning: objtool: __sev_es_nmi_complete+0x5a: call to __asan_memset() leaves .noinstr.text section
  ld: error: unplaced orphan section `.data.rel.local' from `vmlinux.o'
  make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux.unstripped] Error 1

The objtool warning is caused by:

  x86/sev: Split off startup code from core code

Tte link failure by:

  x86/boot: Move SEV startup code into startup/

Thanks,

	Ingo
Ard Biesheuvel April 12, 2025, 8:24 p.m. UTC | #5
On Sat, 12 Apr 2025 at 22:08, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Ingo Molnar <mingo@kernel.org> wrote:
>
> > Ignore that, I have now read the cover letter too, with the patch
> > dependency mentioned there - as kindly pointed out by Ard in a
> > private mail. :-)
>
> But there are other problems during the allmodconfig final link:
>
>   vmlinux.o: warning: objtool: __sev_es_nmi_complete+0x5a: call to __asan_memset() leaves .noinstr.text section

This is an odd one, because noinstr functions should not be
instrumented by kasan afaik.

>   ld: error: unplaced orphan section `.data.rel.local' from `vmlinux.o'

This should have been included in
68f3ea7ee199ef77551e090dfef5a49046ea8443, the commit log has the
details. TL;DR this is .rodata with relocatable quantities, which is
not emitted into .rodata when using -fPIC.

>   make[2]: *** [scripts/Makefile.vmlinux:91: vmlinux.unstripped] Error 1
>
> The objtool warning is caused by:
>
>   x86/sev: Split off startup code from core code
>
> Tte link failure by:
>
>   x86/boot: Move SEV startup code into startup/
>

Thanks for the report - I'll fix these up in the next revision.
Ingo Molnar April 12, 2025, 8:50 p.m. UTC | #6
* Ard Biesheuvel <ardb@kernel.org> wrote:

> On Sat, 12 Apr 2025 at 22:08, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Ingo Molnar <mingo@kernel.org> wrote:
> >
> > > Ignore that, I have now read the cover letter too, with the patch
> > > dependency mentioned there - as kindly pointed out by Ard in a
> > > private mail. :-)
> >
> > But there are other problems during the allmodconfig final link:
> >
> >   vmlinux.o: warning: objtool: __sev_es_nmi_complete+0x5a: call to __asan_memset() leaves .noinstr.text section
> 
> This is an odd one, because noinstr functions should not be
> instrumented by kasan afaik.

FWIW I'm not doing anything particularly weird on the build environment 
side: GCC 14.2.0.

Thanks,

	Ingo