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

["Lendacky, Thomas via groups.io" <thomas.lendacky=amd.com@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 <ardb+tianocore@kernel.org>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> ---
>   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 (pre-SMBASE-relocation) SMRAM
>     Save State Map.
> diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> index da8f1e5db9fa..29e244df3007 100644
> --- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> +++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> @@ -1609,6 +1609,10 @@ Dr7WriteExit (
>     UINT64                     *Register;
>     UINT64                     Status;
>   
> +  if (MemEncryptSevEsDebugSwapIsEnabled ()) {
> +    return UnsupportedExit (Ghcb, Regs, InstructionData);
> +  }
> +
>     Ext       = &InstructionData->Ext;
>     SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
>   
> @@ -1659,6 +1663,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);
>   


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119087): https://edk2.groups.io/g/devel/message/119087
Mute This Topic: https://groups.io/mt/105863824/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-