Re: [edk2-devel] [PATCH v2 07/15] OvmfPkg/MemEncryptSevLib: Obtain encryption mask using the new interface

["Laszlo Ersek" <lersek@redhat.com>]

On 01/06/21 22:21, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108
> 
> The early assembler code performs validation for some of the SEV-related
> information, specifically the encryption bit position. The new
> MemEncryptSevGetEncryptionMask() interface provides access to this
> validated value.
> 
> To ensure that we always use a validated encryption mask for an SEV-ES
> guest, update all locations that use CPUID to calculate the encryption
> mask to use the new interface.
> 
> Also, clean up some call areas where extra masking was being performed
> and where a function call was being used instead of the local variable
> that was just set using the function.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/Bhyve/PlatformPei/AmdSev.c                       | 12 ++----------
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c | 15 +++++----------
>  OvmfPkg/PlatformPei/AmdSev.c                             | 12 ++----------
>  OvmfPkg/XenPlatformPei/AmdSev.c                          | 12 ++----------
>  4 files changed, 11 insertions(+), 40 deletions(-)

(1) Because we modify multiple modules in this patch, the subject line
should rather be:

OvmfPkg: Obtain SEV encryption mask with the new MemEncryptSevLib API

(69 chars).

I can do this if / when I merge v2.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo


> 
> diff --git a/OvmfPkg/Bhyve/PlatformPei/AmdSev.c b/OvmfPkg/Bhyve/PlatformPei/AmdSev.c
> index e484f4b311fe..e3ed78581c1b 100644
> --- a/OvmfPkg/Bhyve/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/Bhyve/PlatformPei/AmdSev.c
> @@ -1,7 +1,7 @@
>  /**@file
>    Initialize Secure Encrypted Virtualization (SEV) support
>  
> -  Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2020, Advanced Micro Devices. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -15,8 +15,6 @@
>  #include <Library/MemEncryptSevLib.h>
>  #include <Library/PcdLib.h>
>  #include <PiPei.h>
> -#include <Register/Amd/Cpuid.h>
> -#include <Register/Cpuid.h>
>  #include <Register/Intel/SmramSaveStateMap.h>
>  
>  #include "Platform.h"
> @@ -32,7 +30,6 @@ AmdSevInitialize (
>    VOID
>    )
>  {
> -  CPUID_MEMORY_ENCRYPTION_INFO_EBX  Ebx;
>    UINT64                            EncryptionMask;
>    RETURN_STATUS                     PcdStatus;
>  
> @@ -43,15 +40,10 @@ AmdSevInitialize (
>      return;
>    }
>  
> -  //
> -  // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
> -  //
> -  AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
> -  EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> -
>    //
>    // Set Memory Encryption Mask PCD
>    //
> +  EncryptionMask = MemEncryptSevGetEncryptionMask ();
>    PcdStatus = PcdSet64S (PcdPteMemoryEncryptionAddressOrMask, EncryptionMask);
>    ASSERT_RETURN_ERROR (PcdStatus);
>  
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> index 5e110c84ff81..6422bc53bd5d 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
> @@ -3,7 +3,7 @@
>    Virtual Memory Management Services to set or clear the memory encryption bit
>  
>    Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> -  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -12,6 +12,7 @@
>  **/
>  
>  #include <Library/CpuLib.h>
> +#include <Library/MemEncryptSevLib.h>
>  #include <Register/Amd/Cpuid.h>
>  #include <Register/Cpuid.h>
>  
> @@ -39,17 +40,12 @@ GetMemEncryptionAddressMask (
>    )
>  {
>    UINT64                            EncryptionMask;
> -  CPUID_MEMORY_ENCRYPTION_INFO_EBX  Ebx;
>  
>    if (mAddressEncMaskChecked) {
>      return mAddressEncMask;
>    }
>  
> -  //
> -  // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
> -  //
> -  AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
> -  EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> +  EncryptionMask = MemEncryptSevGetEncryptionMask ();
>  
>    mAddressEncMask = EncryptionMask & PAGING_1G_ADDRESS_MASK_64;
>    mAddressEncMaskChecked = TRUE;
> @@ -289,8 +285,7 @@ SetPageTablePoolReadOnly (
>    LevelSize[3] = SIZE_1GB;
>    LevelSize[4] = SIZE_512GB;
>  
> -  AddressEncMask  = GetMemEncryptionAddressMask() &
> -                    PAGING_1G_ADDRESS_MASK_64;
> +  AddressEncMask  = GetMemEncryptionAddressMask();
>    PageTable       = (UINT64 *)(UINTN)PageTableBase;
>    PoolUnitSize    = PAGE_TABLE_POOL_UNIT_SIZE;
>  
> @@ -437,7 +432,7 @@ Split1GPageTo2M (
>  
>    AddressEncMask = GetMemEncryptionAddressMask ();
>    ASSERT (PageDirectoryEntry != NULL);
> -  ASSERT (*PageEntry1G & GetMemEncryptionAddressMask ());
> +  ASSERT (*PageEntry1G & AddressEncMask);
>    //
>    // Fill in 1G page entry.
>    //
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index 4a515a484720..954d53eba4e8 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -1,7 +1,7 @@
>  /**@file
>    Initialize Secure Encrypted Virtualization (SEV) support
>  
> -  Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2020, Advanced Micro Devices. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -17,9 +17,7 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
>  #include <PiPei.h>
> -#include <Register/Amd/Cpuid.h>
>  #include <Register/Amd/Msr.h>
> -#include <Register/Cpuid.h>
>  #include <Register/Intel/SmramSaveStateMap.h>
>  
>  #include "Platform.h"
> @@ -116,7 +114,6 @@ AmdSevInitialize (
>    VOID
>    )
>  {
> -  CPUID_MEMORY_ENCRYPTION_INFO_EBX  Ebx;
>    UINT64                            EncryptionMask;
>    RETURN_STATUS                     PcdStatus;
>  
> @@ -127,15 +124,10 @@ AmdSevInitialize (
>      return;
>    }
>  
> -  //
> -  // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
> -  //
> -  AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
> -  EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> -
>    //
>    // Set Memory Encryption Mask PCD
>    //
> +  EncryptionMask = MemEncryptSevGetEncryptionMask ();
>    PcdStatus = PcdSet64S (PcdPteMemoryEncryptionAddressOrMask, EncryptionMask);
>    ASSERT_RETURN_ERROR (PcdStatus);
>  
> diff --git a/OvmfPkg/XenPlatformPei/AmdSev.c b/OvmfPkg/XenPlatformPei/AmdSev.c
> index 7ebbb5cc1fd2..4ed448632ae2 100644
> --- a/OvmfPkg/XenPlatformPei/AmdSev.c
> +++ b/OvmfPkg/XenPlatformPei/AmdSev.c
> @@ -1,7 +1,7 @@
>  /**@file
>    Initialize Secure Encrypted Virtualization (SEV) support
>  
> -  Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2020, Advanced Micro Devices. All rights reserved.<BR>
>    Copyright (c) 2019, Citrix Systems, Inc.
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -14,8 +14,6 @@
>  #include <Library/MemEncryptSevLib.h>
>  #include <Library/PcdLib.h>
>  #include <PiPei.h>
> -#include <Register/Amd/Cpuid.h>
> -#include <Register/Cpuid.h>
>  
>  #include "Platform.h"
>  
> @@ -30,7 +28,6 @@ AmdSevInitialize (
>    VOID
>    )
>  {
> -  CPUID_MEMORY_ENCRYPTION_INFO_EBX  Ebx;
>    UINT64                            EncryptionMask;
>    RETURN_STATUS                     PcdStatus;
>  
> @@ -41,15 +38,10 @@ AmdSevInitialize (
>      return;
>    }
>  
> -  //
> -  // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position)
> -  //
> -  AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL);
> -  EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> -
>    //
>    // Set Memory Encryption Mask PCD
>    //
> +  EncryptionMask = MemEncryptSevGetEncryptionMask ();
>    PcdStatus = PcdSet64S (PcdPteMemoryEncryptionAddressOrMask, EncryptionMask);
>    ASSERT_RETURN_ERROR (PcdStatus);
>  
>