Re: [PATCH v9 37/43] x86/sev: Add SEV-SNP feature detection/setup

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>,
	<brijesh.ksingh@gmail.com>, <tony.luck@intel.com>,
	<marcorr@google.com>,
	<sathyanarayanan.kuppuswamy@linux.intel.com>
Subject: Re: [PATCH v9 37/43] x86/sev: Add SEV-SNP feature detection/setup
Date: Mon, 7 Feb 2022 23:25:42 -0600	[thread overview]
Message-ID: <20220208052542.3g6nskck7uhjnfji@amd.com> (raw)
In-Reply-To: <YgAjq3mSudt+G+6Y@zn.tnic>

On Sun, Feb 06, 2022 at 08:38:19PM +0100, Borislav Petkov wrote:
> On Fri, Jan 28, 2022 at 11:17:58AM -0600, Brijesh Singh wrote:
> > +static __init struct cc_blob_sev_info *snp_find_cc_blob(struct boot_params *bp)
> > +{
> > +	struct cc_blob_sev_info *cc_info;
> > +
> > +	/* Boot kernel would have passed the CC blob via boot_params. */
> > +	if (bp->cc_blob_address) {
> > +		cc_info = (struct cc_blob_sev_info *)(unsigned long)bp->cc_blob_address;
> > +		goto found_cc_info;
> > +	}
> 
> What is the difference here, why aren't you looking for the blob in an
> EFI table?
> 
> Even if you're booted directly by firmware, there should still be EFI
> there or?
> 
> And if so, then I think you should share some of the code through
> sev-shared.c so that there's not so much duplication...

In order to scan EFI this early in the boot of kernel proper, we'd need
to pull in the helpers from arch/x86/boot/compressed/efi.c, since the
normal EFI facilities don't get set up until later. It's doable, but
since any entry via boot/compressed kernel will have already done that
work and stashed it in boot_params->cc_blob_address, it seems like it
would only introduce more complexity and potential for breakage.

For direct entry to kernel proper, our thinking is that it would be for
things like the PVH entry path:

  https://stefano-garzarella.github.io/posts/2019-08-23-qemu-linux-kernel-pvh/

which doesn't use EFI, or other container-focused applications which use
lightweight non-EFI firmwares like qboot. So to support direct entry into
kernel proper, relying on the CC blob setup_data structure instead seems
more suited for these cases, so that's why kernel proper only uses the
setup_data structure and relies on boot/compressed to handle EFI entry.

> 
> -- 
> 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%7Ccb8e665b2ed14dbd400c08d9e9a841ff%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637797731199858639%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=5Pd7PA4BdfnfWzcXpah8HkrtVfu4h6nUIR8b3mB%2BIxM%3D&amp;reserved=0