[PATCH ovmf 0/5] Enable AMD SEV-ES DebugSwap

[PATCH ovmf 0/5] Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

This is to prevent #DB interception on SEV-ES VM with
enabled DebugSwap feature, more details in 3/5.

This is based on sha1 dd3ba82d31a6 and pushed out at
https://github.com/aik/edk2/commits/debugswap

The corresponding Linux change (HV and VM) is here:
https://github.com/aik/linux/commits/debugswap
and has just been posted to lkml/etc as
"[PATCH kernel 0/3] KVM: SEV: Enable AMD SEV-ES DebugSwap"

Please comment. Thanks.



Alexey Kardashevskiy (5):
  MdePkg/Register/Amd: Define all bits from MSR_SEV_STATUS_REGISTER
  MdePkg: Add AMD SEV features to PcdConfidentialComputingGuestAttr
  OvmfPkg: Add AMD SEV-ES DebugSwap feature support
  UefiCpuPkg: Add AMD SEV-ES features support
  OvmfPkf: Enable AMD SEV-ES DebugSwap for DXE

 MdePkg/Include/ConfidentialComputingGuestAttr.h                    |  5 +-
 MdePkg/Include/Register/Amd/Fam17Msr.h                             | 57 +++++++++++++++++++-
 OvmfPkg/Include/Library/MemEncryptSevLib.h                         | 12 +++++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 25 +++++++--
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 19 +++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 20 +++++++
 OvmfPkg/Library/CcExitLib/CcExitVcHandler.c                        |  8 +++
 OvmfPkg/PlatformPei/AmdSev.c                                       | 13 +++--
 UefiCpuPkg/Library/MpInitLib/MpLib.c                               | 10 ++--
 9 files changed, 158 insertions(+), 11 deletions(-)

-- 
2.38.1

[PATCH ovmf 1/5] MdePkg/Register/Amd: Define all bits from MSR_SEV_STATUS_REGISTER

[Alexey Kardashevskiy]

We will need soon DebugSwap but others likely too.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 MdePkg/Include/Register/Amd/Fam17Msr.h | 57 +++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index bb4e143e2456..f9474e6776f2 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -121,7 +121,62 @@ typedef union {
     ///
     UINT32    SevSnpBit : 1;
 
-    UINT32    Reserved2 : 29;
+    ///
+    /// [Bit 3] The guest was run with the Virtual TOM feature enabled in SEV_FEATURES[1]
+    ///
+    UINT32    vTOM_Enabled : 1;
+
+    ///
+    /// [Bit 4] The guest was run with the ReflectVC feature enabled in SEV_FEATURES[2]
+    ///
+    UINT32    ReflectVC : 1;
+
+    ///
+    /// [Bit 5] The guest was run with the Restricted Injection feature enabled in SEV_FEATURES[3]
+    ///
+    UINT32    RestrictedInjection : 1;
+
+    ///
+    /// [Bit 6] The guest was run with the Alternate Injection feature enabled in SEV_FEATURES[4]
+    ///
+    UINT32    AlternateInjection : 1;
+
+    ///
+    /// [Bit 7] This guest was run with debug register swapping enabled in SEV_FEATURES[5]
+    ///
+    UINT32    DebugSwap : 1;
+
+    ///
+    /// [Bit 8]  This guest was run with the PreventHostIBS feature enabled in SEV_FEATURES[6]
+    ///
+    UINT32    PreventHostIBS : 1;
+
+    ///
+    /// [Bit 9] The guest was run with the BTB isolation feature enabled in SEV_FEATURES[7]
+    ///
+    UINT32    SNPBTBIsolation : 1;
+
+    ///
+    /// [Bit 10]
+    ///
+    UINT32    Reserved0 : 1;
+
+    ///
+    /// [Bit 11] The guest was run with the Secure TSC feature enabled in SEV_FEATURES[9]
+    ///
+    UINT32    SecureTsc : 1;
+
+    ///
+    /// [Bits 12 13 14 15]
+    ///
+    UINT32    Reserved1 : 4;
+
+    ///
+    /// [Bit 16] The guest was run with the VMSA Register Protection feature enabled in SEV_FEATURES[14]
+    ///
+    UINT32    VmsaRegProt_Enabled : 1;
+
+    UINT32    Reserved2 : 15;
   } Bits;
   ///
   /// All bit fields as a 32-bit value
-- 
2.38.1

[PATCH ovmf 3/5] OvmfPkg: Add AMD SEV-ES DebugSwap feature support

[Alexey Kardashevskiy]

The SEV-ES DebugSwap feature enables type B swaping of debug registers
on #VMEXIT and makes #DB and DR7 intercepts unnecessary and unwanted.

When DebugSwap is enabled, this stops booting if #VC for #DB or
DB7 read/write occurs as this signals unwanted interaction from the HV.

This adds new API which uses SEV-ES working area in PEI and SEC.

This does not change the existing behavour for DXE just yet but soon.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h                         | 12 ++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 25 +++++++++++++++++---
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 19 +++++++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 20 ++++++++++++++++
 OvmfPkg/Library/CcExitLib/CcExitVcHandler.c                        |  8 +++++++
 5 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 4fa9c0d70083..0fa86aecc38c 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -166,6 +166,18 @@ MemEncryptSevGetEncryptionMask (
   VOID
   );
 
+/**
+  Returns a boolean to indicate whether DebugSwap is enabled.
+
+  @retval TRUE           DebugSwap is enabled
+  @retval FALSE          DebugSwap is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevEsDebugSwapIsEnabled (
+  VOID
+  );
+
 /**
   Returns the encryption state of the specified virtual address range.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
index 4aba0075b9e2..ffe5399c73ca 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
@@ -40,19 +40,23 @@ AmdMemEncryptionAttrCheck (
   IN  CONFIDENTIAL_COMPUTING_GUEST_ATTR  Attr
   )
 {
+  UINT64 CurrentLevel = CurrentAttr & ~CCAttrAmdSevFeatureMask;
+
   switch (Attr) {
     case CCAttrAmdSev:
       //
       // SEV is automatically enabled if SEV-ES or SEV-SNP is active.
       //
-      return CurrentAttr >= CCAttrAmdSev;
+      return CurrentLevel >= CCAttrAmdSev;
     case CCAttrAmdSevEs:
       //
       // SEV-ES is automatically enabled if SEV-SNP is active.
       //
-      return CurrentAttr >= CCAttrAmdSevEs;
+      return CurrentLevel >= CCAttrAmdSevEs;
     case CCAttrAmdSevSnp:
-      return CurrentAttr == CCAttrAmdSevSnp;
+      return CurrentLevel == CCAttrAmdSevSnp;
+    case CCAttrAmdSevFeatureDebugSwap:
+      return !!(CurrentAttr & CCAttrAmdSevFeatureDebugSwap);
     default:
       return FALSE;
   }
@@ -159,3 +163,18 @@ MemEncryptSevGetEncryptionMask (
 
   return mSevEncryptionMask;
 }
+
+/**
+  Returns a boolean to indicate whether DebugSwap is enabled.
+
+  @retval TRUE           DebugSwap is enabled
+  @retval FALSE          DebugSwap is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevEsDebugSwapIsEnabled (
+  VOID
+  )
+{
+  return ConfidentialComputingGuestHas (CCAttrAmdSevFeatureDebugSwap);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index 41d1246a5b31..e2ebc8afcaee 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -141,3 +141,22 @@ MemEncryptSevGetEncryptionMask (
 
   return SevEsWorkArea->EncryptionMask;
 }
+
+/**
+  Returns a boolean to indicate whether DebugSwap is enabled.
+
+  @retval TRUE           DebugSwap is enabled
+  @retval FALSE          DebugSwap is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevEsDebugSwapIsEnabled (
+  VOID
+  )
+{
+  MSR_SEV_STATUS_REGISTER  Msr;
+
+  Msr.Uint32 = InternalMemEncryptSevStatus ();
+
+  return Msr.Bits.DebugSwap ? TRUE : FALSE;
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
index 27148c7e337a..974be9555296 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -142,6 +142,26 @@ MemEncryptSevGetEncryptionMask (
   return SevEsWorkArea->EncryptionMask;
 }
 
+
+/**
+  Returns a boolean to indicate whether DebugSwap is enabled.
+
+  @retval TRUE           DebugSwap is enabled
+  @retval FALSE          DebugSwap is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevEsDebugSwapIsEnabled (
+  VOID
+  )
+{
+  MSR_SEV_STATUS_REGISTER  Msr;
+
+  Msr.Uint32 = InternalMemEncryptSevStatus ();
+
+  return Msr.Bits.DebugSwap ? TRUE : FALSE;
+}
+
 /**
   Locate the page range that covers the initial (pre-SMBASE-relocation) SMRAM
   Save State Map.
diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
index 985e5479775c..811cad164ea2 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
+++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
@@ -2136,6 +2136,10 @@ Dr7WriteExit (
   UINT64                         *Register;
   UINT64                         Status;
 
+  if (MemEncryptSevEsDebugSwapIsEnabled ()) {
+    return UnsupportedExit (Ghcb, Regs, InstructionData);
+  }
+
   Ext       = &InstructionData->Ext;
   SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
 
@@ -2188,6 +2192,10 @@ Dr7ReadExit (
   SEV_ES_PER_CPU_DATA            *SevEsData;
   UINT64                         *Register;
 
+  if (MemEncryptSevEsDebugSwapIsEnabled ()) {
+    return UnsupportedExit (Ghcb, Regs, InstructionData);
+  }
+
   Ext       = &InstructionData->Ext;
   SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
 
-- 
2.38.1

[PATCH ovmf 4/5] UefiCpuPkg: Add AMD SEV-ES features support

[Alexey Kardashevskiy]

CONFIDENTIAL_COMPUTING_GUEST_ATTR is not a simple SEV level anymore
and includes a feature mask since a previous commit.

This fixes AmdMemEncryptionAttrCheck to check the level and feature
correctly and adds DebugSwap support.

Since the actual feature flag is not set yet, this should cause
no behavioral change.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index e5dc852ed95f..26e763295a7e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2849,19 +2849,23 @@ AmdMemEncryptionAttrCheck (
   IN  CONFIDENTIAL_COMPUTING_GUEST_ATTR  Attr
   )
 {
+  UINT64 CurrentLevel = CurrentAttr & ~CCAttrAmdSevFeatureMask;
+
   switch (Attr) {
     case CCAttrAmdSev:
       //
       // SEV is automatically enabled if SEV-ES or SEV-SNP is active.
       //
-      return CurrentAttr >= CCAttrAmdSev;
+      return CurrentLevel >= CCAttrAmdSev;
     case CCAttrAmdSevEs:
       //
       // SEV-ES is automatically enabled if SEV-SNP is active.
       //
-      return CurrentAttr >= CCAttrAmdSevEs;
+      return CurrentLevel >= CCAttrAmdSevEs;
     case CCAttrAmdSevSnp:
-      return CurrentAttr == CCAttrAmdSevSnp;
+      return CurrentLevel == CCAttrAmdSevSnp;
+    case CCAttrAmdSevFeatureDebugSwap:
+      return !!(CurrentAttr & CCAttrAmdSevFeatureDebugSwap);
     default:
       return FALSE;
   }
-- 
2.38.1

[PATCH ovmf 5/5] OvmfPkf: Enable AMD SEV-ES DebugSwap for DXE

[Alexey Kardashevskiy]

This writes the feature bit into PcdConfidentialComputingGuestAttr
and enables DebugSwap for the DXE stage too.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 OvmfPkg/PlatformPei/AmdSev.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index e1b9fd9b7f68..479f898adff4 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -324,6 +324,7 @@ AmdSevInitialize (
   )
 {
   UINT64         EncryptionMask;
+  UINT64         CCGuestAttr;
   RETURN_STATUS  PcdStatus;
 
   //
@@ -407,13 +408,19 @@ AmdSevInitialize (
   // technology is active.
   //
   if (MemEncryptSevSnpIsEnabled ()) {
-    PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSevSnp);
+    CCGuestAttr = CCAttrAmdSevSnp;
   } else if (MemEncryptSevEsIsEnabled ()) {
-    PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSevEs);
+    CCGuestAttr = CCAttrAmdSevEs;
   } else {
-    PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSev);
+    CCGuestAttr = CCAttrAmdSev;
   }
 
+  if (MemEncryptSevEsDebugSwapIsEnabled ()) {
+    CCGuestAttr |= CCAttrAmdSevFeatureDebugSwap;
+  }
+
+  PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCGuestAttr);
+
   ASSERT_RETURN_ERROR (PcdStatus);
 }
 
-- 
2.38.1

[PATCH ovmf 2/5] MdePkg: Add AMD SEV features to PcdConfidentialComputingGuestAttr

[Alexey Kardashevskiy]

PcdConfidentialComputingGuestAttr so far only contained an SEV mode bit
but there are more other features which do not translate to levels
such as DebugSwap or SecureTsc.

This adds the features mask and the DebugSwap feature bit to a PCD.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
 MdePkg/Include/ConfidentialComputingGuestAttr.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h b/MdePkg/Include/ConfidentialComputingGuestAttr.h
index 44e6df800207..1fd09a51ea52 100644
--- a/MdePkg/Include/ConfidentialComputingGuestAttr.h
+++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
@@ -26,12 +26,15 @@ typedef enum {
   CCAttrAmdSev    = 0x100,
   CCAttrAmdSevEs  = 0x101,
   CCAttrAmdSevSnp = 0x102,
+  CCAttrAmdSevFeatureMask      = 0xffff0000,
+  CCAttrAmdSevFeatureDebugSwap = 0x00010000,
 
   /* The guest is running with Intel TDX memory encryption enabled. */
   CCAttrIntelTdx = 0x200,
 } CONFIDENTIAL_COMPUTING_GUEST_ATTR;
 
 #define CC_GUEST_IS_TDX(x)  ((x) == CCAttrIntelTdx)
-#define CC_GUEST_IS_SEV(x)  ((x) == CCAttrAmdSev || (x) == CCAttrAmdSevEs || (x) == CCAttrAmdSevSnp)
+#define _CC_GUEST_IS_SEV(x) ((x) == CCAttrAmdSev || (x) == CCAttrAmdSevEs || (x) == CCAttrAmdSevSnp)
+#define CC_GUEST_IS_SEV(x)  _CC_GUEST_IS_SEV((x) & ~CCAttrAmdSevFeatureMask)
 
 #endif
-- 
2.38.1

Re: Subject: [PATCH ovmf 2/5] MdePkg: Add AMD SEV features to PcdConfidentialComputingGuestAttr

[Lendacky, Thomas]

Added the subject as somehow it didn't get set.

On 11/30/22 20:35, Alexey Kardashevskiy wrote:
> Date: Tue, 22 Nov 2022 16:12:55 +1100
> Subject: [PATCH ovmf 2/5] MdePkg: Add AMD SEV features to
>   PcdConfidentialComputingGuestAttr
> 
> PcdConfidentialComputingGuestAttr so far only contained an SEV mode bit
> but there are more other features which do not translate to levels
> such as DebugSwap or SecureTsc.
> 
> This adds the features mask and the DebugSwap feature bit to a PCD.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
>   MdePkg/Include/ConfidentialComputingGuestAttr.h | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h b/MdePkg/Include/ConfidentialComputingGuestAttr.h
> index 44e6df800207..1fd09a51ea52 100644
> --- a/MdePkg/Include/ConfidentialComputingGuestAttr.h
> +++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
> @@ -26,12 +26,15 @@ typedef enum {
>     CCAttrAmdSev    = 0x100,
>     CCAttrAmdSevEs  = 0x101,
>     CCAttrAmdSevSnp = 0x102,
> +  CCAttrAmdSevFeatureMask      = 0xffff0000,

The PCD for this is 64-bits, should this be 0xffffffffffff0000?

Thanks,
Tom

> +  CCAttrAmdSevFeatureDebugSwap = 0x00010000,
>   
>     /* The guest is running with Intel TDX memory encryption enabled. */
>     CCAttrIntelTdx = 0x200,
>   } CONFIDENTIAL_COMPUTING_GUEST_ATTR;
>   
>   #define CC_GUEST_IS_TDX(x)  ((x) == CCAttrIntelTdx)
> -#define CC_GUEST_IS_SEV(x)  ((x) == CCAttrAmdSev || (x) == CCAttrAmdSevEs || (x) == CCAttrAmdSevSnp)
> +#define _CC_GUEST_IS_SEV(x) ((x) == CCAttrAmdSev || (x) == CCAttrAmdSevEs || (x) == CCAttrAmdSevSnp)
> +#define CC_GUEST_IS_SEV(x)  _CC_GUEST_IS_SEV((x) & ~CCAttrAmdSevFeatureMask)
>   
>   #endif

Re: Subject: [PATCH ovmf 2/5] MdePkg: Add AMD SEV features to PcdConfidentialComputingGuestAttr

[Alexey Kardashevskiy]

On 2/12/22 01:47, Tom Lendacky wrote:
> Added the subject as somehow it didn't get set.
> 
> On 11/30/22 20:35, Alexey Kardashevskiy wrote:
>> Date: Tue, 22 Nov 2022 16:12:55 +1100
>> Subject: [PATCH ovmf 2/5] MdePkg: Add AMD SEV features to
>>   PcdConfidentialComputingGuestAttr
>>
>> PcdConfidentialComputingGuestAttr so far only contained an SEV mode bit
>> but there are more other features which do not translate to levels
>> such as DebugSwap or SecureTsc.
>>
>> This adds the features mask and the DebugSwap feature bit to a PCD.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
>> ---
>>   MdePkg/Include/ConfidentialComputingGuestAttr.h | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h 
>> b/MdePkg/Include/ConfidentialComputingGuestAttr.h
>> index 44e6df800207..1fd09a51ea52 100644
>> --- a/MdePkg/Include/ConfidentialComputingGuestAttr.h
>> +++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
>> @@ -26,12 +26,15 @@ typedef enum {
>>     CCAttrAmdSev    = 0x100,
>>     CCAttrAmdSevEs  = 0x101,
>>     CCAttrAmdSevSnp = 0x102,
>> +  CCAttrAmdSevFeatureMask      = 0xffff0000,
> 
> The PCD for this is 64-bits, should this be 0xffffffffffff0000?

True but does not that really depend on how greedy I am? :) For now I 
like 16bits and leave the rest for everyone else.




-- 
Alexey

Re: Subject: [PATCH ovmf 2/5] MdePkg: Add AMD SEV features to PcdConfidentialComputingGuestAttr

[Gerd Hoffmann]

On Fri, Dec 02, 2022 at 11:26:56PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 2/12/22 01:47, Tom Lendacky wrote:
> > Added the subject as somehow it didn't get set.
> > 
> > On 11/30/22 20:35, Alexey Kardashevskiy wrote:
> > > Date: Tue, 22 Nov 2022 16:12:55 +1100
> > > Subject: [PATCH ovmf 2/5] MdePkg: Add AMD SEV features to
> > >   PcdConfidentialComputingGuestAttr
> > > 
> > > PcdConfidentialComputingGuestAttr so far only contained an SEV mode bit
> > > but there are more other features which do not translate to levels
> > > such as DebugSwap or SecureTsc.
> > > 
> > > This adds the features mask and the DebugSwap feature bit to a PCD.
> > > 
> > > Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> > > ---
> > >   MdePkg/Include/ConfidentialComputingGuestAttr.h | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/MdePkg/Include/ConfidentialComputingGuestAttr.h
> > > b/MdePkg/Include/ConfidentialComputingGuestAttr.h
> > > index 44e6df800207..1fd09a51ea52 100644
> > > --- a/MdePkg/Include/ConfidentialComputingGuestAttr.h
> > > +++ b/MdePkg/Include/ConfidentialComputingGuestAttr.h
> > > @@ -26,12 +26,15 @@ typedef enum {
> > >     CCAttrAmdSev    = 0x100,
> > >     CCAttrAmdSevEs  = 0x101,
> > >     CCAttrAmdSevSnp = 0x102,
> > > +  CCAttrAmdSevFeatureMask      = 0xffff0000,
> > 
> > The PCD for this is 64-bits, should this be 0xffffffffffff0000?
> 
> True but does not that really depend on how greedy I am? :) For now I like
> 16bits and leave the rest for everyone else.

If we want add feature bits to that PCD it should IMHO not be
AmdSev-specific, i.e. we should have something along the lines of:

CCAttrTypeMask     0x000000000000ffff
CCAttrFeatureMask  0xffffffffffff0000

Alternatively use another pcd for the feature bits.

take care,
  Gerd

回复: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd: Define all bits from MSR_SEV_STATUS_REGISTER

[gaoliming]

Alexey:


> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Alexey
> Kardashevskiy via groups.io
> 发送时间: 2022年12月1日 10:35
> 收件人: devel@edk2.groups.io
> 抄送: Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao
> <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Brijesh Singh <brijesh.singh@amd.com>;
> Erdem Aktas <erdemaktas@google.com>; James Bottomley
> <jejb@linux.ibm.com>; Min Xu <min.m.xu@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Alexey Kardashevskiy <aik@amd.com>
> 主题: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd: Define all bits
> from MSR_SEV_STATUS_REGISTER
> 
> We will need soon DebugSwap but others likely too.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
>  MdePkg/Include/Register/Amd/Fam17Msr.h | 57 +++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h
> b/MdePkg/Include/Register/Amd/Fam17Msr.h
> index bb4e143e2456..f9474e6776f2 100644
> --- a/MdePkg/Include/Register/Amd/Fam17Msr.h
> +++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
> @@ -121,7 +121,62 @@ typedef union {
>      ///
>      UINT32    SevSnpBit : 1;
> 
> -    UINT32    Reserved2 : 29;
> +    ///
> +    /// [Bit 3] The guest was run with the Virtual TOM feature enabled in
> SEV_FEATURES[1]
> +    ///
> +    UINT32    vTOM_Enabled : 1;
> +
This name doesn't follow name convention. You can create pull request to see
CI test result. 

Thanks
Liming
> +    ///
> +    /// [Bit 4] The guest was run with the ReflectVC feature enabled in
> SEV_FEATURES[2]
> +    ///
> +    UINT32    ReflectVC : 1;
> +
> +    ///
> +    /// [Bit 5] The guest was run with the Restricted Injection feature
> enabled in SEV_FEATURES[3]
> +    ///
> +    UINT32    RestrictedInjection : 1;
> +
> +    ///
> +    /// [Bit 6] The guest was run with the Alternate Injection feature
> enabled in SEV_FEATURES[4]
> +    ///
> +    UINT32    AlternateInjection : 1;
> +
> +    ///
> +    /// [Bit 7] This guest was run with debug register swapping enabled
in
> SEV_FEATURES[5]
> +    ///
> +    UINT32    DebugSwap : 1;
> +
> +    ///
> +    /// [Bit 8]  This guest was run with the PreventHostIBS feature
> enabled in SEV_FEATURES[6]
> +    ///
> +    UINT32    PreventHostIBS : 1;
> +
> +    ///
> +    /// [Bit 9] The guest was run with the BTB isolation feature enabled
in
> SEV_FEATURES[7]
> +    ///
> +    UINT32    SNPBTBIsolation : 1;
> +
> +    ///
> +    /// [Bit 10]
> +    ///
> +    UINT32    Reserved0 : 1;
> +
> +    ///
> +    /// [Bit 11] The guest was run with the Secure TSC feature enabled in
> SEV_FEATURES[9]
> +    ///
> +    UINT32    SecureTsc : 1;
> +
> +    ///
> +    /// [Bits 12 13 14 15]
> +    ///
> +    UINT32    Reserved1 : 4;
> +
> +    ///
> +    /// [Bit 16] The guest was run with the VMSA Register Protection
> feature enabled in SEV_FEATURES[14]
> +    ///
> +    UINT32    VmsaRegProt_Enabled : 1;
> +
> +    UINT32    Reserved2 : 15;
>    } Bits;
>    ///
>    /// All bit fields as a 32-bit value
> --
> 2.38.1
> 
> 
> 
> 
> 

Re: 回复: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd: Define all bits from MSR_SEV_STATUS_REGISTER

[Alexey Kardashevskiy]

On 7/12/22 13:13, gaoliming wrote:
> Alexey:
> 
> 
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Alexey
>> Kardashevskiy via groups.io
>> 发送时间: 2022年12月1日 10:35
>> 收件人: devel@edk2.groups.io
>> 抄送: Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao
>> <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd
>> Hoffmann <kraxel@redhat.com>; Brijesh Singh <brijesh.singh@amd.com>;
>> Erdem Aktas <erdemaktas@google.com>; James Bottomley
>> <jejb@linux.ibm.com>; Min Xu <min.m.xu@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>; Alexey Kardashevskiy <aik@amd.com>
>> 主题: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd: Define all bits
>> from MSR_SEV_STATUS_REGISTER
>>
>> We will need soon DebugSwap but others likely too.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
>> ---
>>   MdePkg/Include/Register/Amd/Fam17Msr.h | 57 +++++++++++++++++++-
>>   1 file changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h
>> b/MdePkg/Include/Register/Amd/Fam17Msr.h
>> index bb4e143e2456..f9474e6776f2 100644
>> --- a/MdePkg/Include/Register/Amd/Fam17Msr.h
>> +++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
>> @@ -121,7 +121,62 @@ typedef union {
>>       ///
>>       UINT32    SevSnpBit : 1;
>>
>> -    UINT32    Reserved2 : 29;
>> +    ///
>> +    /// [Bit 3] The guest was run with the Virtual TOM feature enabled in
>> SEV_FEATURES[1]
>> +    ///
>> +    UINT32    vTOM_Enabled : 1;
>> +
> This name doesn't follow name convention. You can create pull request to see
> CI test result.


It is though what 40332.pdf ("AMD64 Architecture Programmer’s Manual: 
Volumes 1-5") uses and I find it _extremely_ useful when searching for a 
feature in 3000+ pages PDF.

What is the preferred way?

Thanks,


> 
> Thanks
> Liming
>> +    ///
>> +    /// [Bit 4] The guest was run with the ReflectVC feature enabled in
>> SEV_FEATURES[2]
>> +    ///
>> +    UINT32    ReflectVC : 1;
>> +
>> +    ///
>> +    /// [Bit 5] The guest was run with the Restricted Injection feature
>> enabled in SEV_FEATURES[3]
>> +    ///
>> +    UINT32    RestrictedInjection : 1;
>> +
>> +    ///
>> +    /// [Bit 6] The guest was run with the Alternate Injection feature
>> enabled in SEV_FEATURES[4]
>> +    ///
>> +    UINT32    AlternateInjection : 1;
>> +
>> +    ///
>> +    /// [Bit 7] This guest was run with debug register swapping enabled
> in
>> SEV_FEATURES[5]
>> +    ///
>> +    UINT32    DebugSwap : 1;
>> +
>> +    ///
>> +    /// [Bit 8]  This guest was run with the PreventHostIBS feature
>> enabled in SEV_FEATURES[6]
>> +    ///
>> +    UINT32    PreventHostIBS : 1;
>> +
>> +    ///
>> +    /// [Bit 9] The guest was run with the BTB isolation feature enabled
> in
>> SEV_FEATURES[7]
>> +    ///
>> +    UINT32    SNPBTBIsolation : 1;
>> +
>> +    ///
>> +    /// [Bit 10]
>> +    ///
>> +    UINT32    Reserved0 : 1;
>> +
>> +    ///
>> +    /// [Bit 11] The guest was run with the Secure TSC feature enabled in
>> SEV_FEATURES[9]
>> +    ///
>> +    UINT32    SecureTsc : 1;
>> +
>> +    ///
>> +    /// [Bits 12 13 14 15]
>> +    ///
>> +    UINT32    Reserved1 : 4;
>> +
>> +    ///
>> +    /// [Bit 16] The guest was run with the VMSA Register Protection
>> feature enabled in SEV_FEATURES[14]
>> +    ///
>> +    UINT32    VmsaRegProt_Enabled : 1;
>> +
>> +    UINT32    Reserved2 : 15;
>>     } Bits;
>>     ///
>>     /// All bit fields as a 32-bit value
>> --
>> 2.38.1
>>
>>
>>
>> 
>>
> 
> 
> 

-- 
Alexey

回复: 回复: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd: Define all bits from MSR_SEV_STATUS_REGISTER

[gaoliming]

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Alexey
> Kardashevskiy via groups.io
> 发送时间: 2022年12月7日 20:25
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: 'Ard Biesheuvel' <ardb+tianocore@kernel.org>; 'Jiewen Yao'
> <jiewen.yao@intel.com>; 'Jordan Justen' <jordan.l.justen@intel.com>; 'Gerd
> Hoffmann' <kraxel@redhat.com>; 'Brijesh Singh' <brijesh.singh@amd.com>;
> 'Erdem Aktas' <erdemaktas@google.com>; 'James Bottomley'
> <jejb@linux.ibm.com>; 'Min Xu' <min.m.xu@intel.com>; 'Tom Lendacky'
> <thomas.lendacky@amd.com>
> 主题: Re: 回复: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd:
> Define all bits from MSR_SEV_STATUS_REGISTER
> 
> 
> 
> On 7/12/22 13:13, gaoliming wrote:
> > Alexey:
> >
> >
> >> -----邮件原件-----
> >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Alexey
> >> Kardashevskiy via groups.io
> >> 发送时间: 2022年12月1日 10:35
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao
> >> <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd
> >> Hoffmann <kraxel@redhat.com>; Brijesh Singh <brijesh.singh@amd.com>;
> >> Erdem Aktas <erdemaktas@google.com>; James Bottomley
> >> <jejb@linux.ibm.com>; Min Xu <min.m.xu@intel.com>; Tom Lendacky
> >> <thomas.lendacky@amd.com>; Alexey Kardashevskiy <aik@amd.com>
> >> 主题: [edk2-devel] [PATCH ovmf 1/5] MdePkg/Register/Amd: Define all
> bits
> >> from MSR_SEV_STATUS_REGISTER
> >>
> >> We will need soon DebugSwap but others likely too.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> >> ---
> >>   MdePkg/Include/Register/Amd/Fam17Msr.h | 57
> +++++++++++++++++++-
> >>   1 file changed, 56 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h
> >> b/MdePkg/Include/Register/Amd/Fam17Msr.h
> >> index bb4e143e2456..f9474e6776f2 100644
> >> --- a/MdePkg/Include/Register/Amd/Fam17Msr.h
> >> +++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
> >> @@ -121,7 +121,62 @@ typedef union {
> >>       ///
> >>       UINT32    SevSnpBit : 1;
> >>
> >> -    UINT32    Reserved2 : 29;
> >> +    ///
> >> +    /// [Bit 3] The guest was run with the Virtual TOM feature enabled
> in
> >> SEV_FEATURES[1]
> >> +    ///
> >> +    UINT32    vTOM_Enabled : 1;
> >> +
> > This name doesn't follow name convention. You can create pull request to
> see
> > CI test result.
> 
> 
> It is though what 40332.pdf ("AMD64 Architecture Programmer’s Manual:
> Volumes 1-5") uses and I find it _extremely_ useful when searching for a
> feature in 3000+ pages PDF.
> 
> What is the preferred way?
> 

If CI reports this issue, you can add it into exception in MdePkg.ci.yaml.
Please create pull request to see CI result. 

Thanks
Liming

> Thanks,
> 
> 
> >
> > Thanks
> > Liming
> >> +    ///
> >> +    /// [Bit 4] The guest was run with the ReflectVC feature enabled in
> >> SEV_FEATURES[2]
> >> +    ///
> >> +    UINT32    ReflectVC : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 5] The guest was run with the Restricted Injection feature
> >> enabled in SEV_FEATURES[3]
> >> +    ///
> >> +    UINT32    RestrictedInjection : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 6] The guest was run with the Alternate Injection feature
> >> enabled in SEV_FEATURES[4]
> >> +    ///
> >> +    UINT32    AlternateInjection : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 7] This guest was run with debug register swapping enabled
> > in
> >> SEV_FEATURES[5]
> >> +    ///
> >> +    UINT32    DebugSwap : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 8]  This guest was run with the PreventHostIBS feature
> >> enabled in SEV_FEATURES[6]
> >> +    ///
> >> +    UINT32    PreventHostIBS : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 9] The guest was run with the BTB isolation feature enabled
> > in
> >> SEV_FEATURES[7]
> >> +    ///
> >> +    UINT32    SNPBTBIsolation : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 10]
> >> +    ///
> >> +    UINT32    Reserved0 : 1;
> >> +
> >> +    ///
> >> +    /// [Bit 11] The guest was run with the Secure TSC feature enabled
> in
> >> SEV_FEATURES[9]
> >> +    ///
> >> +    UINT32    SecureTsc : 1;
> >> +
> >> +    ///
> >> +    /// [Bits 12 13 14 15]
> >> +    ///
> >> +    UINT32    Reserved1 : 4;
> >> +
> >> +    ///
> >> +    /// [Bit 16] The guest was run with the VMSA Register Protection
> >> feature enabled in SEV_FEATURES[14]
> >> +    ///
> >> +    UINT32    VmsaRegProt_Enabled : 1;
> >> +
> >> +    UINT32    Reserved2 : 15;
> >>     } Bits;
> >>     ///
> >>     /// All bit fields as a 32-bit value
> >> --
> >> 2.38.1
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> 
> --
> Alexey
> 
> 
> 
>