From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.skyhub.de (mail.skyhub.de [5.9.137.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D354672 for ; Mon, 18 Oct 2021 14:29:15 +0000 (UTC) Received: from zn.tnic (p200300ec2f085700a5f06031787ecc0a.dip0.t-ipconnect.de [IPv6:2003:ec:2f08:5700:a5f0:6031:787e:cc0a]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id D2FC91EC04C2; Mon, 18 Oct 2021 16:29:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1634567347; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:in-reply-to:in-reply-to: references:references; bh=c9xEu9dyx4wrnGD++qak7Rf0JfD9ujFQ9E5yOZB19Q0=; b=aKWwYTlr9cCQu7ia5dZP/kuxrxzN93CyPBgOxttie/QBj3Td/GyHk64AiEAbXaIbAsIGqx BgGTrkPaemD3IjwBuWI2GydbibFzG2kuXi6yqjHnJLtsSpqF8n17AX6r45/y+wPNjuoNcV Bkj7MRJet38z0l3P6cMLUbOfmJ1xF50= Date: Mon, 18 Oct 2021 16:29:07 +0200 From: Borislav Petkov To: Brijesh Singh Cc: x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, Thomas Gleixner , Ingo Molnar , Joerg Roedel , Tom Lendacky , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Gonda , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Michael Roth , Vlastimil Babka , "Kirill A . Shutemov" , Andi Kleen , "Dr . David Alan Gilbert" , tony.luck@intel.com, marcorr@google.com, sathyanarayanan.kuppuswamy@linux.intel.com Subject: Re: [PATCH v6 08/42] x86/sev-es: initialize sev_status/features within #VC handler Message-ID: References: <20211008180453.462291-1-brijesh.singh@amd.com> <20211008180453.462291-9-brijesh.singh@amd.com> Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20211008180453.462291-9-brijesh.singh@amd.com> On Fri, Oct 08, 2021 at 01:04:19PM -0500, Brijesh Singh wrote: > From: Michael Roth > > Generally access to MSR_AMD64_SEV is only safe if the 0x8000001F CPUID > leaf indicates SEV support. With SEV-SNP, CPUID responses from the > hypervisor are not considered trustworthy, particularly for 0x8000001F. > SEV-SNP provides a firmware-validated CPUID table to use as an > alternative, but prior to checking MSR_AMD64_SEV there are no > guarantees that this is even an SEV-SNP guest. > > Rather than relying on these CPUID values early on, allow SEV-ES and > SEV-SNP guests to instead use a cpuid instruction to trigger a #VC and > have it cache MSR_AMD64_SEV in sev_status, since it is known to be safe > to access MSR_AMD64_SEV if a #VC has triggered. > > Signed-off-by: Michael Roth > Signed-off-by: Brijesh Singh > --- > arch/x86/kernel/sev-shared.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c > index 8ee27d07c1cd..2796c524d174 100644 > --- a/arch/x86/kernel/sev-shared.c > +++ b/arch/x86/kernel/sev-shared.c > @@ -191,6 +191,20 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code) > if (exit_code != SVM_EXIT_CPUID) > goto fail; > > + /* > + * A #VC implies that either SEV-ES or SEV-SNP are enabled, so the SEV > + * MSR is also available. Go ahead and initialize sev_status here to > + * allow SEV features to be checked without relying solely on the SEV > + * cpuid bit to indicate whether it is safe to do so. > + */ > + if (!sev_status) { > + unsigned long lo, hi; > + > + asm volatile("rdmsr" : "=a" (lo), "=d" (hi) > + : "c" (MSR_AMD64_SEV)); > + sev_status = (hi << 32) | lo; > + } > + > sev_es_wr_ghcb_msr(GHCB_CPUID_REQ(fn, GHCB_CPUID_REQ_EAX)); > VMGEXIT(); > val = sev_es_rd_ghcb_msr(); > -- Ok, you guys are killing me. ;-\ How is bolting some pretty much unrelated code into the early #VC handler not a hack? Do you not see it? So sme_enable() is reading MSR_AMD64_SEV and setting up everything there, including sev_status. If a SNP guest does not trust CPUID, why can't you attempt to read that MSR there, even if CPUID has lied to the guest? And not just slap it somewhere just because it works? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette