Message-ID: <20250513164432.GFaCN28I_GmSvzXeRJ@fat_crate.local>
Date: Tue, 13 May 2025 18:44:32 +0200
From: Borislav Petkov <bp@...en8.de>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>, Ard Biesheuvel <ardb+git@...gle.com>,
	linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org,
	x86@...nel.org, Dionna Amalie Glaze <dionnaglaze@...gle.com>,
	Kevin Loughlin <kevinloughlin@...gle.com>,
	Tom Lendacky <thomas.lendacky@....com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [RFT PATCH v3 00/21] x86: strict separation of startup code

On Tue, May 13, 2025 at 04:01:08PM +0100, Ard Biesheuvel wrote:
> I will refrain from inserting myself into the intra-tip review and
> testing policy debate, but let me at least provide a quick recap of
> what I am doing here and why.

Thanks for taking the time - much appreciated!

> Since commit
> 
>    c88d71508e36 x86/boot/64: Rewrite startup_64() in C
> 
> dated Jun 6 2017, we have been using C code on the boot path in a way
> that is not supported by the toolchain, i.e., to execute non-PIC C
> code from a mapping of memory that is different from the one provided
> to the linker. It should have been obvious at the time that this was a
> bad idea, given the need to sprinkle fixup_pointer() calls left and
> right to manipulate global variables (including non-pointer variables)
> without crashing.
> 
> This C startup code has been expanding, and in particular, the SEV-SNP
> startup code has been expanding over the past couple of years, and
> grown many of these warts, where the C code needs to use special
> annotations or helpers to access global objects.
> 
> Google uses Clang internally, and as expected, it does not behave
> quite like GCC in this regard either. The result is that the SEV-SNP
> boot tended to break in cryptic ways with Clang built kernels, due to
> absolute references in the startup code that runs before those
> absolute references are mapped.

Oh look, Google is using SEV-SNP too. Non! Si! Oh!

https://www.youtube.com/watch?v=pFjSDM6D500

> I've done a preliminary pass upstream with RIP_REL_REF() and
> rip_rel_ptr() and the use of the .head.text section for startup code
> to ensure that we detect such issues at build time, and it has already
> resulted in a number of true positives where the code in question
> would have failed at boot time. At this point, I'm not aware of any
> issues caused by absolute references going undetected.
> 
> However, Linus kindly indicated that the resulting diagnostics
> produced by the relocs tool do not meet his high standards, and so I
> proposed another approach, which I am implementing now (see cover
> letter for details). Note that this approach is also much more robust,
> as annotating things as __head by hand to get it emitted into the
> section to which the diagnostics are applied is obviously not
> foolproof.

AFAIR, this is what you've done for arm64 too, right?

> Fixing the existing 5-level paging and kernel mapping code was rather
> straight-forward. However, splitting up the SEV-SNP code has been
> rather challenging due to the way it was put together, i.e., as a
> single source file used everywhere, and to which additional
> functionality has been added piecemeal (i.e., the SVSM support).
> 
> It is obvious that these changes should be tested before being merged,

Preach!

> hence the RFT in the subject. And I have been struggling a bit to get
> access to usable hardware. (I do have access to internal development
> systems, but those do not fit the 'usable' description by any measure,
> given that I have to go through the cloud VM orchestration APIs to
> test boot a simple kernel image).
> 
> What Boris might allude to is the fact that some of these changes also
> form a prerequisite for being able to construct a generic EFI zboot
> image for x86, which is a long-term objective that I am working on in
> the background. But this is not the main reason.
> 
> In any case, there is no urgency wrt these changes as far as I am

THANK YOU!

So basically we can all slow down and do normal review and testing. Without
any of that hurried patching back'n'forth. And we didn't need any of that
grief!

Basically something which I've been trying to say from the very beginning but
no one listens to me!

Geez.

> concerned, and given that I already found an issue myself with v3,
> perhaps it is better if we disregard it for the time being, and we can
> come back to it for the next cycle. In the mean time, I can compare
> notes with Boris and Tom directly to ensure that this is in the right
> shape, and perhaps we could at least fix the pgtable_l5_enabled() mess
> as well (for which I sent out a RFC/v3 today).

Thanks man, let's also sync on IRC.

We have enough time to run the set through all our testing pile and shake out
any outstanding issues.

-- 
Regards/Gruss,
    Boris.

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

Powered by blists - more mailing lists