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

[Lendacky, Thomas via groups.io]

On 5/2/24 09:34, Alexey Kardashevskiy wrote:

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.


This changes the SEC/PEI behavior while not changing DXE, which means 
two different behaviors. I wonder if the SEC and PEI changes that access 
the MSR value, should be part of the final patch that enables it for all 
stages. And in this patch, just have the SEC and PEI versions of 
MemEncryptSevEsDebugSwapIsEnabled() return FALSE for now.


Thanks,
Tom



Cc: Ard Biesheuvel 
Cc: Erdem Aktas 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Michael Roth 
Cc: Min Xu 
Cc: Tom Lendacky 
Signed-off-by: Alexey Kardashevskiy 
---
  OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 
+
  OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 27 
+---
  OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 19 
++
  OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 19 
++
  OvmfPkg/Library/CcExitLib/CcExitVcHandler.c|  8 ++
  5 files changed, 82 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..ebc4c9bb5d06 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
@@ -40,19 +40,25 @@ AmdMemEncryptionAttrCheck (
IN  CONFIDENTIAL_COMPUTING_GUEST_ATTR  Attr
)
  {
+  UINT64  CurrentLevel;
+
+  CurrentLevel = CurrentAttr & CCAttrTypeMask;
+
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 CCAttrFeatureAmdSevDebugSwap:
+  return !!(CurrentAttr & CCAttrFeatureAmdSevDebugSwap);
  default:
return FALSE;
}
@@ -159,3 +165,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 (CCAttrFeatureAmdSevDebugSwap);
+}
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..0e82dc85b299 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -142,6 +142,25 @@ MemEncryptSevGetEncryptionMask (
return SevEsWorkArea->EncryptionMask;
 

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

[Alexey Kardashevskiy via groups.io]

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.

Cc: Ard Biesheuvel 
Cc: Erdem Aktas 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Michael Roth 
Cc: Min Xu 
Cc: Tom Lendacky 
Signed-off-by: Alexey Kardashevskiy 
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 
+
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 27 
+---
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 19 
++
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 19 
++
 OvmfPkg/Library/CcExitLib/CcExitVcHandler.c|  8 ++
 5 files changed, 82 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..ebc4c9bb5d06 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
@@ -40,19 +40,25 @@ AmdMemEncryptionAttrCheck (
   IN  CONFIDENTIAL_COMPUTING_GUEST_ATTR  Attr
   )
 {
+  UINT64  CurrentLevel;
+
+  CurrentLevel = CurrentAttr & CCAttrTypeMask;
+
   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 CCAttrFeatureAmdSevDebugSwap:
+  return !!(CurrentAttr & CCAttrFeatureAmdSevDebugSwap);
 default:
   return FALSE;
   }
@@ -159,3 +165,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 (CCAttrFeatureAmdSevDebugSwap);
+}
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..0e82dc85b299 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -142,6 +142,25 @@ 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