Re: [PATCH v8 29/40] x86/compressed/64: add support for SEV-SNP CPUID table in #VC handlers

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 v8 29/40] x86/compressed/64: add support for SEV-SNP CPUID table in #VC handlers
Date: Tue, 18 Jan 2022 11:20:43 -0600	[thread overview]
Message-ID: <20220118172043.djhy3dwg4fhhfqfs@amd.com> (raw)
In-Reply-To: <YebsKcpnYzvjaEjs@zn.tnic>

On Tue, Jan 18, 2022 at 05:34:49PM +0100, Borislav Petkov wrote:
> On Tue, Jan 18, 2022 at 08:37:30AM -0600, Michael Roth wrote:
> > Actually, no, because doing that would provide hypervisor a means to
> > effectively disable CPUID page for an SNP guest by provided a table with
> > count == 0, which needs to be guarded against.
> 
> Err, I'm confused.
> 
> Isn't that "SEV-SNP guests will be provided the location of special
> 'secrets' 'CPUID' pages via the Confidential Computing blob..." and the
> HV has no say in there?
> 
> Why does the HV provide the CPUID page?

The HV fills out the initial contents of the CPUID page, which includes
the count. SNP/PSP firmware will validate the contents the HV tries to put
in the initial page, but does not currently enforce that the 'count' field
is non-zero. So we can't rely on the 'count' field as an indicator of
whether or not the CPUID page is active, we need to rely on the presence
of the ccblob as the true indicator, then treat a non-zero 'count' field
as an invalid state.

I think we discussed this to some extent in the past. The following
document was added to clarify the security model for CPUID page:

https://lore.kernel.org/lkml/20211210154332.11526-29-brijesh.singh@amd.com/

> 
> And when I read "secrets page" I think, encrypted/signed and given
> directly to the guest, past the HV which cannot even touch it.

The CPUID page is also encrypted, but its initial contents come from the
HV, which are then passed through the PSP for initial validation before
being placed in the CPUID page. But the count==0 case is not disallowed
by the PSP firmware, so can't be relied upon as a means to indicate that
the CPUID page is not active.

> 
> Hmmm.
> 
> -- 
> 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%7C9e8f64f998744605658608d9daa073ee%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637781205020570217%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=26Fkw4FJ9jOLVRW7SMs6IWYyaY5gO8iUfdm3x4HDaJk%3D&amp;reserved=0