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-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXHZhFQ0DQWGDXtX4+-RWUFgyfL1qJh4hCONdi29NeBzhQ@mail.gmail.com>
Date: Thu, 10 Jul 2025 17:36:33 +1000
From: Ard Biesheuvel <ardb@...nel.org>
To: Nikunj A Dadhania <nikunj@....com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, linux-efi <linux-efi@...r.kernel.org>, 
	X86 ML <x86@...nel.org>, Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...nel.org>, 
	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

(please keep the CCs)

On Thu, 10 Jul 2025 at 16:54, Nikunj A Dadhania <nikunj@....com> wrote:
>
> Ard Biesheuvel <ardb@...nel.org> writes:
>
> > 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,
>
> Not sure if I understand, this check is there since:
>
> cbd3d4f7c4e5 x86/sev: Check SEV-SNP features support
>
> > even though it is redundant,
> > given that SNP support implies ghcb version >= 2
> >
> > Would the below change be sufficient too?
>
> KVM_SEV_INIT2 allows the user space to provide ghcb_version, setting
> ghcb_version=1 in QEMU;
>
> with my changes: SNP guest does not boot, as the ghcb_version < 2
> with the below change: SNP guest boots, which is not correct
>
> This needs to be fixed in KVM as well to not allow creating SNP guest if
> ghcb_version < 2.
>

OK, thanks for clariyfing this - it means that checking the GHCB
version remains necessary, even though it is only done for SNP guests,
where GHCB version >=2 is mandated.

I'll take this into account in the next revision.