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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXGJk1JsehK3jtbR7yOKFV4UvySYbq10x8td1gM9ryHeBw@mail.gmail.com>
Date: Wed, 7 May 2025 14:05:37 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Borislav Petkov <bp@...en8.de>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org, 
	linux-efi@...r.kernel.org, x86@...nel.org, Ingo Molnar <mingo@...nel.org>, 
	Dionna Amalie Glaze <dionnaglaze@...gle.com>, Kevin Loughlin <kevinloughlin@...gle.com>, 
	Tom Lendacky <thomas.lendacky@....com>
Subject: Re: [RFT PATCH v2 00/23] x86: strict separation of startup code

On Wed, 7 May 2025 at 11:53, Borislav Petkov <bp@...en8.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.