linux-kernel - Re: [PATCH v4 12/24] x86/sev: Unify SEV-SNP hypervisor feature check

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: <CAMj1kXF73myE9sKos5j9ErZi14ExDQBDZvp8PEhG3K4PELQrSw@mail.gmail.com>
Date: Thu, 10 Jul 2025 14:24:53 +1000
From: Ard Biesheuvel <ardb@...nel.org>
To: Nikunj A Dadhania <nikunj@....com>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org, 
	linux-efi@...r.kernel.org, x86@...nel.org, Borislav Petkov <bp@...en8.de>, 
	Ingo Molnar <mingo@...nel.org>, Dionna Amalie Glaze <dionnaglaze@...gle.com>, 
	Kevin Loughlin <kevinloughlin@...gle.com>, Tom Lendacky <thomas.lendacky@....com>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v4 12/24] x86/sev: Unify SEV-SNP hypervisor feature check

On Thu, 10 Jul 2025 at 14:21, Nikunj A Dadhania <nikunj@....com> wrote:
>
> Ard Biesheuvel <ardb+git@...gle.com> writes:
>
> > From: Ard Biesheuvel <ardb@...nel.org>
>
> ...
>
> > So move the HV feature check into a helper function and call that
> > instead. For the core kernel, move the check to an earlier boot stage,
> > right after the point where it is established that the guest is
> > executing in SEV-SNP mode.
>
> This change is causing the SNP guest to fail ...
>
> >
> > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> > ---
>
> >  arch/x86/boot/startup/sme.c         |  2 ++
> >  arch/x86/coco/sev/core.c            | 11 -------
>
> > diff --git a/arch/x86/boot/startup/sme.c b/arch/x86/boot/startup/sme.c
> > index 70ea1748c0a7..529090e20d2a 100644
> > --- a/arch/x86/boot/startup/sme.c
> > +++ b/arch/x86/boot/startup/sme.c
> > @@ -533,6 +533,8 @@ void __head sme_enable(struct boot_params *bp)
> >       if (snp_en ^ !!(msr & MSR_AMD64_SEV_SNP_ENABLED))
> >               snp_abort();
> >
> > +     sev_hv_features = snp_check _hv_features();
> > +
>
> ...
> snp_check_hv_features()
>  -> get_hv_features() fails as ghcb_version is not yet initalized
>
> ghcb_version is initialized as part of sev_es_negotiate_protocol(),
> atleast this function needs to be called before get_hv_features() is
> called.
>

Thanks for the diagnosis.

I added back the ghcb_version check, even though it is redundant,
given that SNP support implies ghcb version >= 2

Would the below change be sufficient too?

--- a/arch/x86/boot/startup/sev-shared.c
+++ b/arch/x86/boot/startup/sev-shared.c
@@ -49,9 +49,6 @@ static u64 __init get_hv_features(void)
 {
        u64 val;

-       if (ghcb_version < 2)
-               return 0;
-
        sev_es_wr_ghcb_msr(GHCB_MSR_HV_FT_REQ);
        VMGEXIT();