Message-ID: <CAMj1kXEzKEuePEiHB+HxvfQbFz0sTiHdn4B++zVBJ2mhkPkQ4Q@mail.gmail.com>
Date: Tue, 13 May 2025 16:01:08 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Borislav Petkov <bp@...en8.de>
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, 13 May 2025 at 15:17, Borislav Petkov <bp@...en8.de> wrote:
>
> On Tue, May 13, 2025 at 01:22:16PM +0200, Ingo Molnar wrote:
...
> > Note that two of those fixes were from Ard who is working on further
> > robustifying the startup code - a much needed change.
>
> Really? Much needed huh?
>
> Please do explain why is it much needed?
>
> Because the reason Ard is doing it is a different one but maybe
> I misunderstood him...
>

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.

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.

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.

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,
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
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).

Powered by blists - more mailing lists