Re: [RFC PATCH v2 01/10] OvmfPkg/Include: Define SEV specific CPUID and MSR

[Brijesh Singh <brijesh.ksingh@gmail.com>]

On Mon, Mar 27, 2017 at 2:57 AM, Fan, Jeff <jeff.fan@intel.com> wrote:

> Laszlo,
>
> One Amd directory under UefiCpuPkg/Include/Register is better.
>
> Does Brijesh/Leo have any comments, or have other suggestions?
>
>

I am not sure if creating a new UefiCpuPkg/Include/Register/Amd/Cpuid is a
good idea, CPUID Fn8000_001F
leaf can go in UefiCpuPkg/Include/Register/Cpuid.h. Looking at
UefiCpuPkg/Include/Register/Cpuid.h indicates
that this file defines the standard x86 CPUID's. The definition of CPUID's
listed in this file are same on both Intel
and AMD platforms. I was told by our architecture team that if Intel ever
decides to support the new leaf then it will
comply to the exact same bit definition.

I am a bit unsure about where the MSR_SEV_STATUS should go, one part of me
thinks that this MSR should be
defined in OvmfPkg and other part thinks that it can go into
UefiCpuPkg/Include/Amd/Msr.h. It is a read-only MSR
and has meaning in a virtualization context hence I was more inclined
towards defining into OvmfPkg instead of UefiCpuPkg.

Leo,  any comments/suggestions ?

-Brijesh


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, March 23, 2017 5:20 PM
> To: Fan, Jeff; Brijesh Singh; Kinney, Michael D; Justen, Jordan L;
> edk2-devel@ml01.01.org; Gao, Liming
> Cc: leo.duran@amd.com; brijesh.singh@amd.com; Thomas.Lendacky@amd.com
> Subject: Re: [edk2] [RFC PATCH v2 01/10] OvmfPkg/Include: Define SEV
> specific CPUID and MSR
>
> On 03/23/17 08:42, Fan, Jeff wrote:
> > Laszlo,
> >
> > UefiCpuPkg/Include/Register/Cpuid.h defined the CPUID only described in
> IA32 SDM.
> > UefiCpuPkg/Include/Register/ArchitecturalMsr.h defined the IA32
> > Architectural MSRs in IA32 SDM UefiCpuPkg/Include/Register/Msr/xxxxMsr.h
> defined the IA32 Model-specific MSRs in IA32 SDM.
> >
> > I am not sure if Brijesh/Leo has some idea to place SEV specific
> CPUID/MSRs definitions.
> > I think one new file or new folder is better.
>
> I agree, both would work for me. My main point is that this feature
> depends on physical processor attributes, not on emulated (virtual)
> hardware or on various hypervisors, plus it is defined in a public industry
> spec, so it seems to belong under UefiCpuPkg, not OvmfPkg.
>
> How about
>
>   UefiCpuPkg/Include/AmdRegister/Cpuid.h
>   UefiCpuPkg/Include/AmdRegister/ArchitecturalMsr.h
>   UefiCpuPkg/Include/AmdRegister/Msr/xxxxMsr.h
>
> or else:
>
>   UefiCpuPkg/Include/Register/Amd/Cpuid.h
>   UefiCpuPkg/Include/Register/Amd/ArchitecturalMsr.h
>   UefiCpuPkg/Include/Register/Amd/Msr/xxxxMsr.h
>
> (as appropriate -- I'm not saying that this patch should create all of
> these files / subdirectories at once).
>
> Thanks
> Laszlo
> >
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Laszlo Ersek
> > Sent: Thursday, March 23, 2017 12:04 AM
> > To: Brijesh Singh; Kinney, Michael D; Justen, Jordan L;
> > edk2-devel@ml01.01.org; Gao, Liming; Fan, Jeff
> > Cc: leo.duran@amd.com; brijesh.singh@amd.com; Thomas.Lendacky@amd.com
> > Subject: Re: [edk2] [RFC PATCH v2 01/10] OvmfPkg/Include: Define SEV
> > specific CPUID and MSR
> >
> > Adding Jeff
> >
> > On 03/21/17 22:12, Brijesh Singh wrote:
> >> The patch defines AMD's Memory Encryption Information CPUID leaf
> (0x8000_001F).
> >> The complete description for this CPUID leaf is available in APM
> >> volume 2 [1] Section 15.34 (Secure Encrypted Virtualization).
> >>
> >> [1] http://support.amd.com/TechDocs/24593.pdf
> >>
> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> >> ---
> >>  OvmfPkg/Include/Register/AmdSevMap.h |  133
> >> ++++++++++++++++++++++++++++++++++
> >>  1 file changed, 133 insertions(+)
> >>  create mode 100644 OvmfPkg/Include/Register/AmdSevMap.h
> >>
> >> diff --git a/OvmfPkg/Include/Register/AmdSevMap.h
> >> b/OvmfPkg/Include/Register/AmdSevMap.h
> >> new file mode 100644
> >> index 0000000..de80f39
> >> --- /dev/null
> >> +++ b/OvmfPkg/Include/Register/AmdSevMap.h
> >> @@ -0,0 +1,133 @@
> >> +/** @file
> >> +
> >> +AMD Secure Encrypted Virtualization (SEV) specific CPUID and MSR
> >> +definitions
> >> +
> >> +The complete description for this CPUID leaf is available in APM
> >> +volume 2 (Section 15.34) http://support.amd.com/TechDocs/24593.pdf
> >> +
> >> +Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
> >> +
> >> +This program and the accompanying materials are licensed and made
> >> +available under the terms and conditions of the BSD License which
> >> +accompanies this distribution.  The full text of the license may be
> >> +found at http://opensource.org/licenses/bsd-license.php
> >> +
> >> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> >> +BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#ifndef __AMD_SEV_MAP_H__
> >> +#define __AMD_SEV_MAP_H__
> >> +
> >> +#pragma pack (1)
> >> +
> >> +#define CPUID_MEMORY_ENCRYPTION_INFO             0x8000001F
> >> +
> >> +/**
> >> +  CPUID Memory Encryption support information EAX for CPUID leaf
> >> +  #CPUID_MEMORY_ENCRYPTION_INFO.
> >> +**/
> >> +typedef union {
> >> +  ///
> >> +  /// Individual bit fields
> >> +  ///
> >> +  struct {
> >> +    ///
> >> +    /// [Bit 0] Secure Memory Encryption (Sme) Support
> >> +    ///
> >> +    UINT32  SmeBit:1;
> >> +
> >> +    ///
> >> +    /// [Bit 1] Secure Encrypted Virtualization (Sev) Support
> >> +    ///
> >> +    UINT32  SevBit:1;
> >> +
> >> +    ///
> >> +    /// [Bit 2] Page flush MSR support
> >> +    ///
> >> +    UINT32  PageFlushMsrBit:1;
> >> +
> >> +    ///
> >> +    /// [Bit 3] Encrypted state support
> >> +    ///
> >> +    UINT32  SevEsBit:1;
> >> +
> >> +    ///
> >> +    /// [Bit 4:31] Reserved
> >> +    ///
> >> +    UINT32  ReservedBits:28;
> >> +  } Bits;
> >> +  ///
> >> +  /// All bit fields as a 32-bit value
> >> +  ///
> >> +  UINT32  Uint32;
> >> +} CPUID_MEMORY_ENCRYPTION_INFO_EAX;
> >> +
> >> +/**
> >> +  CPUID Memory Encryption support information EBX for CPUID leaf
> >> +  #CPUID_MEMORY_ENCRYPTION_INFO.
> >> +**/
> >> +typedef union {
> >> +  ///
> >> +  /// Individual bit fields
> >> +  ///
> >> +  struct {
> >> +    ///
> >> +    /// [Bit 0:5] Page table bit number used to enable memory
> encryption
> >> +    ///
> >> +    UINT32  PtePosBits:6;
> >> +
> >> +    ///
> >> +    /// [Bit 6:11] Reduction of system physical address space bits
> when memory encryption is enabled
> >> +    ///
> >> +    UINT32  ReducedPhysBits:5;
> >> +
> >> +    ///
> >> +    /// [Bit 12:31] Reserved
> >> +    ///
> >> +    UINT32  ReservedBits:21;
> >> +  } Bits;
> >> +  ///
> >> +  /// All bit fields as a 32-bit value
> >> +  ///
> >> +  UINT32  Uint32;
> >> +} CPUID_MEMORY_ENCRYPTION_INFO_EBX;
> >> +
> >> +/**
> >> +  Secure Encrypted Virtualization (SEV) status register
> >> +
> >> +**/
> >> +#define MSR_SEV_STATUS                     0xc0010131
> >> +
> >> +/**
> >> +  MSR information returned for #MSR_SEV_STATUS **/ typedef union {
> >> +  ///
> >> +  /// Individual bit fields
> >> +  ///
> >> +  struct {
> >> +    ///
> >> +    /// [Bit 0] Secure Encrypted Virtualization (Sev) is enabled
> >> +    ///
> >> +    UINT32  SevBit:1;
> >> +
> >> +    ///
> >> +    /// [Bit 1] Secure Encrypted Virtualization Encrypted State
> (SevEs) is enabled
> >> +    ///
> >> +    UINT32  SevEsBit:1;
> >> +
> >> +    UINT32  Reserved:30;
> >> +  } Bits;
> >> +  ///
> >> +  /// All bit fields as a 32-bit value
> >> +  ///
> >> +  UINT32  Uint32;
> >> +  ///
> >> +  /// All bit fields as a 64-bit value
> >> +  ///
> >> +  UINT64  Uint64;
> >> +} MSR_SEV_STATUS_REGISTER;
> >> +
> >> +#endif
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> >
> > I feel that these definitions should be added to
> "UefiCpuPkg/Include/Register/Cpuid.h", or else to another (new) header
> file in that directory.
> >
> > Jeff, what do you think?
> >
> > Thanks!
> > Laszlo
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
>
>