public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Brijesh Singh" <brijesh.singh@amd.com>
To: devel@edk2.groups.io
Cc: Brijesh Singh <brijesh.singh@amd.com>,
	James Bottomley <jejb@linux.ibm.com>, Min Xu <min.m.xu@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Laszlo Ersek <lersek@redhat.com>
Subject: [RFC PATCH 19/19] OvmfPkg/MemEncryptSevLib: Skip page state change for non RAM region
Date: Wed, 24 Mar 2021 10:32:15 -0500	[thread overview]
Message-ID: <20210324153215.17971-20-brijesh.singh@amd.com> (raw)
In-Reply-To: <20210324153215.17971-1-brijesh.singh@amd.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The PEI phases uses MemEncryptSevSnpValidateSystemRam() to validate
the system RAM. MemEncryptSev{Set,Clear}PageEncMask() is
later used by libraries/drivers to change the page state from private
to shared and vice versa.

The AmdSevDxe driver calls the MemEncryptSevClearPageEncMask() to remove
the encryption attribute from the reserved and non-existent memory regions.
Those regions where not part of system RAM, and was not pre-validated
during PEI phase, so we fail to change the page state for it.

There are multiple approaches to fix it:

1) Add a new parameter to MemEncryptSevClearPageEncMask(), that should be
   set by the caller to indicate whether the region is RAM.
OR
2) Lookup the address in the interval tree maintained by the
   MemEncryptSevSnpValidateSystemRam() to determine whether we validated
   the page in the past.
OR
3) Iterate through the Memory space GCD to calculate if the address range
   is RAM.

For now, we have chosen #2, it does not require a changes to the
caller of MemEncryptSevClearPageEncMask() and lookup routine is
already available.

Extend the SEV-ES workarea to pass the interval tree root pointer
so that we can perform the lookup later. If the specified address
was not present in the tree, then do not invalidate the page as it
could result in page state change failure.

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h                         |  3 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf       |  4 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c   | 31 +++++++++++++++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c | 42 ++++++++++++--------
 4 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 47d6802b61..712590b64d 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -54,6 +54,9 @@ typedef struct _SEC_SEV_ES_WORK_AREA {
   UINT64   RandomData;
 
   UINT64   EncryptionMask;
+
+  UINT64   SnpSystemRamValidatedRootAddress;
+
 } SEC_SEV_ES_WORK_AREA;
 
 //
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index fa8f7719a7..43b842254f 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -38,6 +38,7 @@
   X64/PeiDxeVirtualMemory.c
   X64/SnpPageStateChangeInternal.c
   X64/PeiDxeSnpSetPageState.c
+  X64/SnpPageStateTrack.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
@@ -58,3 +59,6 @@
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
+
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
index 0a3d58ac22..9fe6831368 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
@@ -10,8 +10,12 @@
 
 #include <Uefi/UefiBaseType.h>
 #include <Library/BaseLib.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DebugLib.h>
 
 #include "PeiSnpPageStateChange.h"
+#include "SnpPageStateTrack.h"
 
 VOID
 SnpSetMemoryPrivate (
@@ -28,5 +32,32 @@ SnpSetMemoryShared (
   IN  UINTN                 Length
   )
 {
+  SEC_SEV_ES_WORK_AREA       *SevEsWorkArea;
+  SNP_VALIDATED_RANGE        *RootNode, *Range;
+
+  //
+  // Get the Page State tracker root node. The information will be used to lookup
+  // the address in the page state tracker.
+  //
+  SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
+  RootNode = (SNP_VALIDATED_RANGE *) SevEsWorkArea->SnpSystemRamValidatedRootAddress;
+
+  //
+  // Check if the region is validated during the System RAM validation process.
+  // If region is not validated then do nothing. This typically will happen if
+  // we are getting called to make the page state change for the MMIO region.
+  // The MMIO regions fall within reserved memory type and does not require
+  // page state changes.
+  //
+  Range = FindOverlapRange (RootNode, PhysicalAddress, PhysicalAddress + Length);
+  if (Range == NULL) {
+    DEBUG ((EFI_D_INFO, "%a:%a %Lx - %Lx is not RAM, skipping it.\n",
+          gEfiCallerBaseName,
+          __FUNCTION__,
+          PhysicalAddress,
+          PhysicalAddress + Length));
+    return;
+  }
+
   SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), SevSnpPageShared, FALSE);
 }
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
index 41bf301efe..2e049d3df7 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -18,8 +18,6 @@
 #include "SnpPageStateTrack.h"
 #include "VirtualMemory.h"
 
-STATIC SNP_VALIDATED_RANGE     *mRootNode;
-
 STATIC
 SNP_VALIDATED_RANGE *
 SetPageStateChangeInitialize (
@@ -64,15 +62,34 @@ SevSnpValidateSystemRam (
   UINTN                   EndAddress;
   SNP_VALIDATED_RANGE     *Range;
   EFI_STATUS              Status;
+  SEC_SEV_ES_WORK_AREA    *SevEsWorkArea;
+  SNP_VALIDATED_RANGE     *RootNode;
 
   EndAddress = BaseAddress + EFI_PAGES_TO_SIZE (NumPages);
 
+  // The Root of SNP_VALIDATED_RANGE is saved in the EsWorkArea.
+  SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
+  RootNode = (SNP_VALIDATED_RANGE *) SevEsWorkArea->SnpSystemRamValidatedRootAddress;
+
+  //
+  // If the Root is NULL then its the first call. Lets initialize the List before
+  // we process the request.
+  //
+  if (RootNode == NULL) {
+    RootNode = SetPageStateChangeInitialize ();
+
+    //
+    // Save the RootNode in the workarea
+    //
+    SevEsWorkArea->SnpSystemRamValidatedRootAddress = (UINT64) (UINTN) RootNode;
+  }
+
   //
   // The page table used in PEI can address up to 4GB memory. If we are asked to validate
   // a range above the 4GB, then create an identity mapping so that the PVALIDATE instruction
-  // can execute correctly. If the page table entry is not present then PVALIDATE will
-  // cause the #GP.
   //
+  //
+  
   if (BaseAddress >= SIZE_4GB) {
     Status = InternalMemEncryptSevCreateIdentityMap1G (0, BaseAddress,
                   EFI_PAGES_TO_SIZE (NumPages));
@@ -81,25 +98,16 @@ SevSnpValidateSystemRam (
     }
   }
 
-  //
-  // If the Root is NULL then its the first call. Lets initialize the List before
-  // we process the request.
-  //
-  if (mRootNode == NULL) {
-    mRootNode = SetPageStateChangeInitialize ();
-  }
-
   //
   // Check if the range is already validated
   //
-  EndAddress = BaseAddress + EFI_PAGES_TO_SIZE(NumPages);
-  Range = FindOverlapRange (mRootNode, BaseAddress, EndAddress);
+  Range = FindOverlapRange (RootNode, BaseAddress, EndAddress);
 
   //
   // Range is not validated
   if (Range == NULL) {
     SetPageStateInternal (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
-    AddRangeToIntervalTree (mRootNode, BaseAddress, EndAddress);
+    AddRangeToIntervalTree (RootNode, BaseAddress, EndAddress);
     return;
   }
 
@@ -110,12 +118,12 @@ SevSnpValidateSystemRam (
   if (BaseAddress < Range->StartAddress) {
     NumPages = EFI_SIZE_TO_PAGES (Range->StartAddress - BaseAddress);
     SetPageStateInternal (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
-    AddRangeToIntervalTree (mRootNode, BaseAddress, Range->StartAddress);
+    AddRangeToIntervalTree (RootNode, BaseAddress, Range->StartAddress);
   }
 
   if (EndAddress > Range->EndAddress) {
     NumPages = EFI_SIZE_TO_PAGES (EndAddress - Range->EndAddress);
     SetPageStateInternal (Range->EndAddress, NumPages, SevSnpPagePrivate, TRUE);
-    AddRangeToIntervalTree (mRootNode, Range->StartAddress, EndAddress);
+    AddRangeToIntervalTree (RootNode, Range->StartAddress, EndAddress);
   }
 }
-- 
2.17.1


  parent reply	other threads:[~2021-03-24 15:32 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-24 15:31 [RFC PATCH 00/19] Add AMD Secure Nested Paging (SEV-SNP) support brijesh.singh
2021-03-24 15:31 ` [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest Brijesh Singh
2021-04-06  8:11   ` Min Xu
2021-04-06 12:16     ` Laszlo Ersek
2021-04-07  0:21       ` Min Xu
2021-04-07  0:44         ` James Bottomley
2021-04-07 15:02           ` Laszlo Ersek
2021-04-07 15:12             ` James Bottomley
2021-04-08  6:24             ` [edk2-devel] " Min Xu
2021-04-08 13:31               ` Lendacky, Thomas
2021-04-09 12:29                 ` Laszlo Ersek
2021-04-09 13:32                 ` Laszlo Ersek
2021-04-09 13:44                   ` Yao, Jiewen
2021-04-09 14:11                     ` separate OVMF binary for TDX? [was: OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest] Laszlo Ersek
2021-04-12  8:35                       ` Dr. David Alan Gilbert
2021-04-12 11:54                         ` [edk2-devel] " Yao, Jiewen
2021-04-12 14:33                           ` James Bottomley
2021-04-14 23:34                             ` erdemaktas
2021-04-15  7:59                               ` Paolo Bonzini
2021-04-15 19:42                                 ` Erdem Aktas
2021-04-21  0:38                                   ` Yao, Jiewen
2021-04-21 10:44                                     ` Laszlo Ersek
2021-04-21 17:07                                       ` Erdem Aktas
2021-04-22 14:20                                         ` Laszlo Ersek
2021-04-07 13:22         ` [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest Laszlo Ersek
2021-04-07 13:24           ` Laszlo Ersek
2021-04-08  0:45           ` Min Xu
2021-04-07  0:31       ` James Bottomley
2021-04-12 14:52   ` Brijesh Singh
2021-04-13  9:49     ` Laszlo Ersek
2021-04-13 11:29       ` Brijesh Singh
2021-04-13 13:13         ` Laszlo Ersek
2021-04-19 21:42       ` Brijesh Singh
2021-04-20  8:14         ` Laszlo Ersek
2021-03-24 15:31 ` [RFC PATCH 02/19] OvmfPkg: validate the data pages used in the SEC phase Brijesh Singh
2021-03-24 15:31 ` [RFC PATCH 03/19] MdePkg: Expand the SEV MSR to include the SNP definition Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 04/19] OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled() Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 05/19] MdePkg: Define the GHCB GPA structure Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 06/19] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 07/19] OvmfPkg: Add a library to support registering GHCB GPA Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 08/19] OvmfPkg: register GHCB gpa for the SEV-SNP guest Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 09/19] MdePkg: Add AsmPvalidate() support Brijesh Singh
2021-03-25  2:49   ` 回复: [edk2-devel] " gaoliming
2021-03-25 10:54     ` Brijesh Singh
2021-03-26 20:02       ` Andrew Fish
2021-03-24 15:32 ` [RFC PATCH 10/19] OvmfPkg: Define the Page State Change VMGEXIT structures Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 11/19] OvmfPkg/ResetVector: Invalidate the GHCB page Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 12/19] OvmfPkg/MemEncryptSevLib: Add support to validate system RAM Brijesh Singh
2021-04-01  6:37   ` Yao, Jiewen
2021-04-01 13:07     ` Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 13/19] OvmfPkg/SecMain: Validate the data/code pages used for the PEI phase Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 14/19] OvmfPkg/MemEncryptSevLib: Add support to validate RAM in " Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 15/19] OvmfPkg/PlatformPei: Validate the system RAM when SNP is active Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 16/19] OvmfPkg/MemEncryptSevLib: Add support to validate > 4GB memory in PEI phase Brijesh Singh
2021-04-01  6:43   ` Yao, Jiewen
2021-03-24 15:32 ` [RFC PATCH 17/19] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase Brijesh Singh
2021-03-24 15:32 ` [RFC PATCH 18/19] OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc attribute Brijesh Singh
2021-03-24 20:07   ` Brijesh Singh
2021-03-24 15:32 ` Brijesh Singh [this message]
2021-03-24 19:14 ` [edk2-devel] [RFC PATCH 00/19] Add AMD Secure Nested Paging (SEV-SNP) support Laszlo Ersek
2021-04-08  9:58 ` Laszlo Ersek
2021-04-08 11:59   ` Brijesh Singh
2021-04-09 12:24     ` Laszlo Ersek
2021-04-09 22:43       ` Brijesh Singh
2021-04-12 16:23         ` Laszlo Ersek
2021-04-12 20:14           ` Brijesh Singh
2021-04-13 13:00             ` Laszlo Ersek
2021-04-14 11:18               ` Brijesh Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210324153215.17971-20-brijesh.singh@amd.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox