Re: [PATCH v9 30/43] KVM: SEV: Add documentation for SEV-SNP CPUID Enforcement

[Michael Roth]

On Mon, Feb 07, 2022 at 11:48:11PM +0000, Sean Christopherson wrote:
> On Fri, Jan 28, 2022, Brijesh Singh wrote:
> > From: Michael Roth <michael.roth@xxxxxxx>
> > 
> > Update the documentation with SEV-SNP CPUID enforcement.
> > 
> > Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
> > Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> > ---
> >  .../virt/kvm/amd-memory-encryption.rst        | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/virt/kvm/amd-memory-encryption.rst b/Documentation/virt/kvm/amd-memory-encryption.rst
> > index 1c6847fff304..0c72f44cc11a 100644
> > --- a/Documentation/virt/kvm/amd-memory-encryption.rst
> > +++ b/Documentation/virt/kvm/amd-memory-encryption.rst
> 
> This doc is specifically for KVM's host-side implemenation, whereas the below is
> (a) mostly targeted at the guest and (b) has nothing to do with KVM.
> 
> Documentation/x86/amd-memory-encryption.rst isn't a great fit either.
> 
> Since TDX will need a fair bit of documentation, and SEV-ES could retroactively
> use docs as well, what about adding a sub-directory:
> 
> 	Documentation/virt/confidential_compute

There's actually a Documentation/virt/coco/sevguest.rst that was added
in this series as part of:

  "virt: Add SEV-SNP guest driver"

Maybe that's good choice?

I've been wondering about potentially adding the:

  "Guest/Hypervisor Implementation Notes for SEV-SNP CPUID Enforcement"

document that was sent to SNP mailing list under Documentation/
somewhere. If we were to do that, it would be a good place to move the
documentation from this patch into as well. Any thoughts on that?

> 
> to match the "cc_platform_has" stuffr, and then we can add sev.rst and tdx.rst
> there?  Or sev-es.rst, sev-snp.rst, etc... if we want to split things up more.
> 
> It might be worth extracting the SEV details from x86/amd-memory-encryption.rst
> into virt/ as well.  A big chunk of that file appears to be SEV specific, and it
> appears to have gotten a little out-of-whack.  E.g. this section no longer makes
> sense as the last paragraph below appears to be talking about SME (bit 23 in MSR
> 0xc0010010), but walking back "this bit" would reference SEV.  I suspect a
> mostly-standalone sev.rst would be easier to follow than an intertwined SME+SEV.
> 
>   If support for SME is present, MSR 0xc00100010 (MSR_AMD64_SYSCFG) can be used to
>   determine if SME is enabled and/or to enable memory encryption::
> 
>           0xc0010010:
>                   Bit[23]   0 = memory encryption features are disabled
>                             1 = memory encryption features are enabled
> 
>   If SEV is supported, MSR 0xc0010131 (MSR_AMD64_SEV) can be used to determine if
>   SEV is active::
> 
>           0xc0010131:
>                   Bit[0]    0 = memory encryption is not active
>                             1 = memory encryption is active
> 
>   Linux relies on BIOS to set this bit if BIOS has determined that the reduction
>   in the physical address space as a result of enabling memory encryption (see
>   CPUID information above) will not conflict with the address space resource
>   requirements for the system.  If this bit is not set upon Linux startup then
>   Linux itself will not set it and memory encryption will not be possible.

I'll check with Brijesh on these.

Thanks!

-Mike