Re: [PATCH v7 02/45] x86/sev: detect/setup SEV/SME features earlier in boot

From: Michael Roth <michael.roth@amd.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>, <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 <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	"Vitaly Kuznetsov" <vkuznets@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	"Andy Lutomirski" <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Sergio Lopez <slp@redhat.com>, Peter Gonda <pgonda@google.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	David Rientjes <rientjes@google.com>,
	Dov Murik <dovmurik@linux.ibm.com>,
	Tobin Feldman-Fitzthum <tobin@ibm.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	"Kirill A . Shutemov" <kirill@shutemov.name>,
	Andi Kleen <ak@linux.intel.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	<tony.luck@intel.com>, <marcorr@google.com>,
	<sathyanarayanan.kuppuswamy@linux.intel.com>
Subject: Re: [PATCH v7 02/45] x86/sev: detect/setup SEV/SME features earlier in boot
Date: Mon, 15 Nov 2021 14:17:15 -0600	[thread overview]
Message-ID: <20211115201715.gv24iugujwhxmrdp@amd.com> (raw)
In-Reply-To: <YZKxCdhaFTTlSHAJ@zn.tnic>

On Mon, Nov 15, 2021 at 08:12:09PM +0100, Borislav Petkov wrote:
> On Wed, Nov 10, 2021 at 04:06:48PM -0600, Brijesh Singh wrote:
> > From: Michael Roth <michael.roth@amd.com>
> > 
> > sme_enable() handles feature detection for both SEV and SME. Future
> > patches will also use it for SEV-SNP feature detection/setup, which
> > will need to be done immediately after the first #VC handler is set up.
> > Move it now in preparation.
> 
> I don't mind the move - what I miss is the reason why you're moving it
> up.

The early #VC handlers are needed mainly so that early cpuid instructions
can be handled for SEV-ES. In the case of SNP, the cpuid table needs to be
set up at the same time so those #VC handlers can handle cpuid lookups for
SEV-SNP guests. Previously in v6, that CPUID table setup was done with a
separate routine, snp_cpuid_init(), which was awkward because between
the point snp_cpuid_init() was called, and sme_enable() was called, we
were in an in-between state where some SEV-ES/SEV-SNP features were in
use, but they weren't actually sanity-checked against SEV status MSR and
CPUID bits until later in sme_enable(). I tried adding some of those checks
from sme_enable() into snp_cpuid_init(), but you'd suggested instead moving
the CPUID table setup into sme_enable() to avoid the duplication:

https://lore.kernel.org/linux-mm/20211027151325.v3w3nghq5z2o5dto@amd.com/

but in order for that to happen soon enough to make use of the CPUID
table for all CPUID intructions, it needs to be moved to just after the first
#VC handler is setup (where snp_cpuid_init() used to be in v6).

As for why CPUID table needs to be used for all CPUID, it's mainly so
that the CPUID values used throughout boot can be attested to later
if the guest owner validates the CPUID values in the CPUID page. I
added some documentation for why this is the case in:

 [PATCH 33/45] KVM: SEV: Add documentation for SEV-SNP CPUID Enforcement

> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&amp;data=04%7C01%7Cmichael.roth%40amd.com%7C29ab37203c1a4c796bdc08d9a86bd7a4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637726003477466910%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=xxIMC4PM7kJ2O79gQKK7I%2BhnOsuEbckVPA9Gicz0S9w%3D&amp;reserved=0