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 10:59 AM, Duran, Leo <leo.duran@amd.com> wrote:

> Jeff, et al,
>
> Given that:
> 1) UefiCpuPkg/Include/Register/Cpuid.h... Does reference an Intel SDM
> 2) UefiCpuPkg/Include/Register/Msr... Includes intel SoC's
>
> I vote for:
> 1) UefiCpuPkg/Include/Register/Amd/Cpuid.h
> 2) UefiCpuPkg/Include/Register/Amd/Msr.h + UefiCpuPkg/Include/Register/
> Amd/Msr/XxxMsr.h
>
>

OK, I will take care of this in next rev.



> Leo.
>
> > -----Original Message-----
> > From: Fan, Jeff [mailto:jeff.fan@intel.com]
> > Sent: Monday, March 27, 2017 2:58 AM
> > To: Laszlo Ersek <lersek@redhat.com>; Brijesh Singh
> > <brijesh.ksingh@gmail.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Justen, Jordan L
> > <jordan.l.justen@intel.com>; edk2-devel@ml01.01.org; Gao, Liming
> > <liming.gao@intel.com>
> > Cc: Duran, Leo <leo.duran@amd.com>; Singh, Brijesh
> > <brijesh.singh@amd.com>; Lendacky, Thomas
> > <Thomas.Lendacky@amd.com>
> > Subject: RE: [edk2] [RFC PATCH v2 01/10] OvmfPkg/Include: Define SEV
> > specific CPUID and MSR
> >
> > Laszlo,
> >
> > One Amd directory under UefiCpuPkg/Include/Register is better.
> >
> > Does Brijesh/Leo have any comments, or have other suggestions?
> >
> > Thanks!
> > Jeff
> >
> > -----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
> > >
>
>


-- 
Confusion is always the most honest response.