Re: [RFT PATCH v2 00/23] x86: strict separation of startup code

Ard Biesheuvel
2025-05-07T12:05:50Z

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.