From: Ard Biesheuvel <ardb@kernel.org>

snp_vmpl will be assigned a non-zero value when executing at a VMPL
other than 0, and this is inferred from a call to RMPADJUST, which only
works when running at VMPL0.

This means that testing snp_vmpl is sufficient, and there is no need to
perform the same check again.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/sev.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 4bdf5595ed96..d62722dd2de1 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -392,30 +392,16 @@ void sev_enable(struct boot_params *bp)
 	 */
 	if (sev_status & MSR_AMD64_SEV_SNP_ENABLED) {
 		u64 hv_features;
-		int ret;
 
 		hv_features = get_hv_features();
 		if (!(hv_features & GHCB_HV_FT_SNP))
 			sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
 
 		/*
-		 * Enforce running at VMPL0 or with an SVSM.
-		 *
-		 * Use RMPADJUST (see the rmpadjust() function for a description of
-		 * what the instruction does) to update the VMPL1 permissions of a
-		 * page. If the guest is running at VMPL0, this will succeed. If the
-		 * guest is running at any other VMPL, this will fail. Linux SNP guests
-		 * only ever run at a single VMPL level so permission mask changes of a
-		 * lesser-privileged VMPL are a don't-care.
+		 * Running at VMPL0 is required unless an SVSM is present and
+		 * the hypervisor supports the required SVSM GHCB events.
 		 */
-		ret = rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1);
-
-		/*
-		 * Running at VMPL0 is not required if an SVSM is present and the hypervisor
-		 * supports the required SVSM GHCB events.
-		 */
-		if (ret &&
-		    !(snp_vmpl && (hv_features & GHCB_HV_FT_SNP_MULTI_VMPL)))
+		if (snp_vmpl > 0 && !(hv_features & GHCB_HV_FT_SNP_MULTI_VMPL))
 			sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
 	}
 
-- 
2.50.0.727.gbf7dc18ff4-goog

On Tue, Jul 22, 2025 at 09:27:19AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> snp_vmpl will be assigned a non-zero value when executing at a VMPL
> other than 0, and this is inferred from a call to RMPADJUST, which only
> works when running at VMPL0.
> 
> This means that testing snp_vmpl is sufficient, and there is no need to
> perform the same check again.

Remind me again pls: the startup code is going to be executed unconditionally,
even by the decompressor, right?

Because if it is an alternative path, we can't do the below.

Thx.

-- 
Regards/Gruss,
    Boris.

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

On Mon, 11 Aug 2025 at 08:30, Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Jul 22, 2025 at 09:27:19AM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > snp_vmpl will be assigned a non-zero value when executing at a VMPL
> > other than 0, and this is inferred from a call to RMPADJUST, which only
> > works when running at VMPL0.
> >
> > This means that testing snp_vmpl is sufficient, and there is no need to
> > perform the same check again.
>
> Remind me again pls: the startup code is going to be executed unconditionally,
> even by the decompressor, right?
>
> Because if it is an alternative path, we can't do the below.
>

The rmpadjust() call has no meaningful side effects - it is only the
distinction between its success or failure that tells us which VMPL
the guest is running at. The value of snp_vmpl has already been set
based on the same criteria.

The decompressor is completely independent when it comes to
initializing the SEV-SNP context, and the core kernel will redo
everything from scratch. This is needed because the decompressor takes
over exception handling in order to be able to handle page faults and
lazily populate the identity mapping. IOW, the core kernel does not
care whether the decompressor has executed and does not reuse any of
the context that it has created.

This double initialization is no longer needed when booting via EFI,
and so once these changes are in, I will go back and resubmit some of
my previous work that removes the call to sev_enable() from the EFI
stub entirely. Non-EFI boot via the legacy decompressor (which
includes kexec boot, which makes it relevant for SEV-SNP guests) will
retain this redundant boot flow, unless we find a way to mandate a
complete 1:1 mapping from the boot chain (which EFI already
guarantees).

On Thu, Aug 28, 2025 at 09:36:15AM +0200, Ard Biesheuvel wrote:
> The decompressor is completely independent when it comes to
> initializing the SEV-SNP context, and the core kernel will redo
> everything from scratch.

Looking at the code, snp_vmpl gets set by svsm_setup_ca() which both the
decompressor and the startup code call. So I think this answers my question.

The core kernel won't rediscover the VMPL of the guest - sev-shared.c which
contains svsm_setup_ca() is only inserted here:

arch/x86/boot/compressed/sev.c:43:#include "../../boot/startup/sev-shared.c"
arch/x86/boot/startup/sev-startup.c:45:#include "sev-shared.c"

> This double initialization is no longer needed when booting via EFI,
> and so once these changes are in, I will go back and resubmit some of
> my previous work that removes the call to sev_enable() from the EFI
> stub entirely.

You'd need to keep the svsm_setup()->svsm_setup_ca() code path in
sev-startup.c so that the VMPL gets discovered properly.

> Non-EFI boot via the legacy decompressor (which includes kexec boot, which
> makes it relevant for SEV-SNP guests) will retain this redundant boot flow,
> unless we find a way to mandate a complete 1:1 mapping from the boot chain
> (which EFI already guarantees).

That's fine. I'd prefer if we slowly decommission the decompressor instead
once all machines start booting using EFI. We'll see how that ends up working
out in practice tho.

Thx.

-- 
Regards/Gruss,
    Boris.

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