On 3/24/21 10:32 AM, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
>
> The MemEncryptSev{Set,Clear}PageEncMask() functions are used to set or
> clear the memory encryption attribute in the page table. When SEV-SNP is
> active, we also need to validate or invalidate the pages and update the
> RMP entry.
>
> Before clearing the encryption attribute we need to invalidate the memory,
> and then make the page shared in the RMP entry. Similarly, after setting
> the encryption attribute in the page table, we add the memory as private
> in the RMP entry and validate it.
>
> Cc: James Bottomley <j...@linux.ibm.com>
> Cc: Min Xu <min.m...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
> OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf | 3 ++
> OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf | 1 +
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c | 32
> +++++++++++++++++
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 36
> ++++++++++++++++++--
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h | 27
> +++++++++++++++
> 5 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> index f2e162d680..fa8f7719a7 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> @@ -36,6 +36,8 @@
> [Sources.X64]
> X64/MemEncryptSevLib.c
> X64/PeiDxeVirtualMemory.c
> + X64/SnpPageStateChangeInternal.c
> + X64/PeiDxeSnpSetPageState.c
> X64/VirtualMemory.c
> X64/VirtualMemory.h
>
> @@ -49,6 +51,7 @@
> DebugLib
> MemoryAllocationLib
> PcdLib
> + VmgExitLib
>
> [FeaturePcd]
> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> index cb9dd2bb21..d16ec44954 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> @@ -37,6 +37,7 @@
> [Sources.X64]
> X64/MemEncryptSevLib.c
> X64/PeiDxeVirtualMemory.c
> + X64/PeiDxeSnpSetPageState.c
> X64/PeiSnpSystemRamValidate.c
> X64/SnpPageStateTrack.c
> X64/SnpPageStateChangeInternal.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
> new file mode 100644
> index 0000000000..0a3d58ac22
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
> @@ -0,0 +1,32 @@
> +/** @file
> +
> + SEV-SNP Page Validation functions.
> +
> + Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Library/BaseLib.h>
> +
> +#include "PeiSnpPageStateChange.h"
There is one minor error in this hunk, Please ignore it during your
reviews. I will fix it in next version.
The include should be "../SnpPageStateChange.h" and not
"PeiSnppageStateChange.h".
> +
> +VOID
> +SnpSetMemoryPrivate (
> + IN PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINTN Length
> + )
> +{
> + SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length),
> SevSnpPagePrivate, FALSE);
> +}
> +
> +VOID
> +SnpSetMemoryShared (
> + IN PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINTN Length
> + )
> +{
> + SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length),
> SevSnpPageShared, FALSE);
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index 33d9bafe9f..26d363d427 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -17,6 +17,7 @@
> #include <Register/Cpuid.h>
>
> #include "VirtualMemory.h"
> +#include "SnpSetPageState.h"
>
> STATIC BOOLEAN mAddressEncMaskChecked = FALSE;
> STATIC UINT64 mAddressEncMask;
> @@ -700,22 +701,34 @@ SetMemoryEncDec (
> UINT64 AddressEncMask;
> BOOLEAN IsWpEnabled;
> RETURN_STATUS Status;
> + BOOLEAN NeedPageStateChange;
> + PHYSICAL_ADDRESS OrigPhysicalAddress;
> + UINTN OrigLength;
>
> //
> // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
> //
> PageMapLevel4Entry = NULL;
>
> + //
> + // When SEV-SNP is active, before clearing the encryption attribute from
> + // the page table we also need to update the RMP entry for the memory
> + // region to make the region shared. And after setting the encryption
> + // attribute, the region must be made private in the RMP table.
> + //
> + NeedPageStateChange = MemEncryptSevSnpIsEnabled ();
> +
> DEBUG ((
> DEBUG_VERBOSE,
> - "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a
> CacheFlush=%u\n",
> + "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a CacheFlush=%u
> Rmpupdate=%u\n",
> gEfiCallerBaseName,
> __FUNCTION__,
> Cr3BaseAddress,
> PhysicalAddress,
> (UINT64)Length,
> (Mode == SetCBit) ? "Encrypt" : "Decrypt",
> - (UINT32)CacheFlush
> + (UINT32)CacheFlush,
> + (UINT32)NeedPageStateChange
> ));
>
> //
> @@ -749,6 +762,18 @@ SetMemoryEncDec (
> DisableReadOnlyPageWriteProtect ();
> }
>
> + //
> + // Make the RMP updates before clearing the encryption attribute in the
> page table.
> + //
> + if (NeedPageStateChange && (Mode == ClearCBit)) {
> + SnpSetMemoryShared (PhysicalAddress, Length);
> + }
> +
> + //
> + // Save the values, we need it later during the Page state change.
> + //
> + OrigPhysicalAddress = PhysicalAddress;
> + OrigLength = Length;
> Status = EFI_SUCCESS;
>
> while (Length != 0)
> @@ -923,6 +948,13 @@ SetMemoryEncDec (
> //
> CpuFlushTlb();
>
> + //
> + // Make the RMP updates after setting the encryption attribute in the page
> table.
> + //
> + if (NeedPageStateChange && (Mode == SetCBit)) {
> + SnpSetMemoryPrivate (OrigPhysicalAddress, OrigLength);
> + }
> +
> Done:
> //
> // Restore page table write protection, if any.
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h
> new file mode 100644
> index 0000000000..0b29bad612
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h
> @@ -0,0 +1,27 @@
> +/** @file
> +
> + SEV-SNP Page Validation functions.
> +
> + Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef PEI_DXE_SNP_PAGE_STATE_INTERNAL_H_
> +#define PEI_DXE_SNP_PAGE_STATE_INTERNAL_H_
> +
> +#include "../SnpPageStateChange.h"
> +
> +VOID
> +SnpSetMemoryPrivate (
> + IN PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINTN Length
> + );
> +
> +VOID
> +SnpSetMemoryShared (
> + IN PHYSICAL_ADDRESS PhysicalAddress,
> + IN UINTN Length
> + );
> +#endif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73239): https://edk2.groups.io/g/devel/message/73239
Mute This Topic: https://groups.io/mt/81584595/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
