Re: [RFC PATCH 0/6] Add AMD Secure Nested Paging (SEV-SNP) support

From: Dov Murik <dovmurik@linux.ibm.com>
To: Brijesh Singh <brijesh.singh@amd.com>, qemu-devel@nongnu.org
Cc: "Tom Lendacky" <thomas.lendacky@amd.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	kvm@vger.kernel.org, "Michael S . Tsirkin" <mst@redhat.com>,
	"Connor Kuehl" <ckuehl@redhat.com>,
	"Michael Roth" <michael.roth@amd.com>,
	"James Bottomley" <jejb@linux.ibm.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Dov Murik" <dovmurik@linux.ibm.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"David Gibson" <david@gibson.dropbear.id.au>
Subject: Re: [RFC PATCH 0/6] Add AMD Secure Nested Paging (SEV-SNP) support
Date: Tue, 13 Jul 2021 11:05:37 +0300	[thread overview]
Message-ID: <e68a9760-121f-72ee-f8ae-193b92bde403@linux.ibm.com> (raw)
In-Reply-To: <20210709215550.32496-1-brijesh.singh@amd.com>

Brijesh,

On 10/07/2021 0:55, Brijesh Singh wrote:
> SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
> new hardware-based memory protections. SEV-SNP adds strong memory integrity
> protection to help prevent malicious hypervisor-based attacks like data
> replay, memory re-mapping and more in order to create an isolated memory
> encryption environment.
> 
> The patches to support the SEV-SNP in Linux kernel and OVMF are available:
> https://lore.kernel.org/kvm/20210707181506.30489-1-brijesh.singh@amd.com/
> https://lore.kernel.org/kvm/20210707183616.5620-1-brijesh.singh@amd.com/
> https://edk2.groups.io/g/devel/message/77335?p=,,,20,0,0,0::Created,,posterid%3A5969970,20,2,20,83891508
> 
> The Qemu patches uses the command id added by the SEV-SNP hypervisor
> patches to bootstrap the SEV-SNP VMs.
> 
> TODO:
>  * Add support to filter CPUID values through the PSP.
> 
> Additional resources
> ---------------------
> SEV-SNP whitepaper
> https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf
> 
> APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)
> 
> GHCB spec:
> https://developer.amd.com/wp-content/resources/56421.pdf
> 
> SEV-SNP firmware specification:
> https://www.amd.com/system/files/TechDocs/56860.pdf
> 
> Brijesh Singh (6):
>   linux-header: add the SNP specific command
>   i386/sev: extend sev-guest property to include SEV-SNP
>   i386/sev: initialize SNP context
>   i386/sev: add the SNP launch start context
>   i386/sev: add support to encrypt BIOS when SEV-SNP is enabled
>   i386/sev: populate secrets and cpuid page and finalize the SNP launch
> 
>  docs/amd-memory-encryption.txt |  81 +++++-
>  linux-headers/linux/kvm.h      |  47 ++++
>  qapi/qom.json                  |   6 +
>  target/i386/sev.c              | 498 ++++++++++++++++++++++++++++++++-
>  target/i386/sev_i386.h         |   1 +
>  target/i386/trace-events       |   4 +
>  6 files changed, 628 insertions(+), 9 deletions(-)
> 

It might be useful to allow the user to view SNP-related status/settings
in HMP's `info sev` and QMP's qom-list/qom-get under
/machine/confidential-guest-support .

(Not sure whether HMP is deprecated and new stuff should not be added
there.)

Particularly confusing is the `policy` attribute which is only relevant
for SEV / SEV-ES, while there's a new `snp.policy` attribute for SNP...
Maybe the irrelevant attributes should not be added to the tree when not
in SNP.

-Dov