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

["Fan, Jeff" <jeff.fan@intel.com>]

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
>