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

[Laszlo Ersek <lersek@redhat.com>]

On 03/27/17 13:58, Brijesh Singh wrote:
> 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.

OK, if we can't reach an agreement for UefiCpuPkg, I'm fine with OvmfPkg.

Thanks
Laszlo

> 
> 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
>>>
>>
>>
>