Re: [RFC Part1 PATCH v3 01/17] Documentation/x86: Add AMD Secure Encrypted Virtualization (SEV) descrption

[Brijesh Singh]

On 07/25/2017 12:45 AM, Borislav Petkov wrote:
> On Mon, Jul 24, 2017 at 02:07:41PM -0500, Brijesh Singh wrote:
>
> Subject: Re: [RFC Part1 PATCH v3 01/17] Documentation/x86: Add AMD Secure Encrypted Virtualization (SEV) descrption
> 													 ^^^^^^^^^^
>
> Please introduce a spellchecker into your workflow.
>
> > Update amd-memory-encryption document describing the AMD Secure Encrypted
>
> "Update the AMD memory encryption document...
>
> The patch has the proper URL already.
>
> > Virtualization (SEV) feature.
> >
> > Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> > ---
> >   Documentation/x86/amd-memory-encryption.txt | 29 ++++++++++++++++++++++++++---
> >   1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/x86/amd-memory-encryption.txt b/Documentation/x86/amd-memory-encryption.txt
> > index f512ab7..747df07 100644
> > --- a/Documentation/x86/amd-memory-encryption.txt
> > +++ b/Documentation/x86/amd-memory-encryption.txt
> > @@ -1,4 +1,5 @@
> > -Secure Memory Encryption (SME) is a feature found on AMD processors.
> > +Secure Memory Encryption (SME) and Secure Encrypted Virtualization (SEV) are
> > +features found on AMD processors.
> >   
> >   SME provides the ability to mark individual pages of memory as encrypted using
> >   the standard x86 page tables.  A page that is marked encrypted will be
> > @@ -6,6 +7,12 @@ automatically decrypted when read from DRAM and encrypted when written to
> >   DRAM.  SME can therefore be used to protect the contents of DRAM from physical
> >   attacks on the system.
> >   
> > +SEV enables running encrypted virtual machine (VMs) in which the code and data
>
> 					 machines
>
> > +of the virtual machine are secured so that decrypted version is available only
>
> ... of the guest VM ...		  ... so that a decrypted ...
>
> > +within the VM itself. SEV guest VMs have concept of private and shared memory.
>
> 				 	have *the* concept - you need to use
> 					definite and indefinite articles in your
> 					text.
>
> > +Private memory is encrypted with the guest-specific key, while shared memory
> > +may be encrypted with hypervisor key.
>
> And here you explain that the hypervisor key is the same key which we
> use in SME. So that people can make the connection.
>
> > +
> >   A page is encrypted when a page table entry has the encryption bit set (see
> >   below on how to determine its position).  The encryption bit can also be
> >   specified in the cr3 register, allowing the PGD table to be encrypted. Each
> > @@ -19,11 +26,20 @@ so that the PGD is encrypted, but not set the encryption bit in the PGD entry
> >   for a PUD which results in the PUD pointed to by that entry to not be
> >   encrypted.
> >   
> > -Support for SME can be determined through the CPUID instruction. The CPUID
> > -function 0x8000001f reports information related to SME:
> > +When SEV is enabled, certain type of memory (namely insruction pages and guest
>
> When SEV is enabled, instruction pages and guest page tables are ...
>
> > +page tables) are always treated as private. Due to security reasons all DMA
>
> security reasons??
>
> > +operations inside the guest must be performed on shared memory. Since the
> > +memory encryption bit is only controllable by the guest OS when it is operating
>
> 		     ... is controlled ...
>
> > +in 64-bit or 32-bit PAE mode, in all other modes the SEV hardware forces memory
>
> 								... forces the memory ...
>
> > +encryption bit to 1.
> > +
> > +Support for SME and SEV can be determined through the CPUID instruction. The
> > +CPUID function 0x8000001f reports information related to SME:
> >   
> >   	0x8000001f[eax]:
> >   		Bit[0] indicates support for SME
> > +	0x800001f[eax]:
>
> There's a 0 missing and you don't really need it as it is already above.
>
> > +		Bit[1] indicates support for SEV
> >   	0x8000001f[ebx]:
> >   		Bits[5:0]  pagetable bit number used to activate memory
> >   			   encryption
> > @@ -39,6 +55,13 @@ determine if SME is enabled and/or to enable memory encryption:
> >   		Bit[23]   0 = memory encryption features are disabled
> >   			  1 = memory encryption features are enabled
> >   
> > +If SEV is supported, MSR 0xc0010131 (MSR_F17H_SEV) can be used to determine if
>
> If this MSR is going to be part of the architecture - and I really think
> it is - then call it MSR_AMD64_SEV.

Thanks Boris, I'll update the doc per your feedbacks. And will rename the MSR to
MSR_AMD64_SEV.

-Brijesh