[edk2-devel] [PATCH v8 17/32] OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI phase

Brijesh Singh via groups.io posted 32 patches 4 years, 4 months ago
There is a newer version of this series
[edk2-devel] [PATCH v8 17/32] OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI phase
Posted by Brijesh Singh via groups.io 4 years, 4 months ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The initial page built during the SEC phase is used by the
MemEncryptSevSnpValidateSystemRam() for the system RAM validation. The
page validation process requires using the PVALIDATE instruction;  the
instruction accepts a virtual address of the memory region that needs
to be validated. If hardware encounters a page table walk failure (due
to page-not-present) then it raises #GP.

The initial page table built in SEC phase address up to 4GB. Add an
internal function to extend the page table to cover > 4GB. The function
builds 1GB entries in the page table for access > 4GB. This will provide
the support to call PVALIDATE instruction for the virtual address >
4GB in PEI phase.

Cc: Michael Roth <michael.roth@amd.com>
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: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 .../BaseMemEncryptSevLib/X64/VirtualMemory.h  |  19 +++
 .../X64/PeiDxeVirtualMemory.c                 | 115 ++++++++++++++++++
 .../X64/PeiSnpSystemRamValidate.c             |  22 ++++
 3 files changed, 156 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
index 21bbbd1c4f9c..aefef68c30c0 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
@@ -143,4 +143,23 @@ InternalMemEncryptSevClearMmioPageEncMask (
   IN  PHYSICAL_ADDRESS        PhysicalAddress,
   IN  UINTN                   Length
   );
+
+/**
+  Create 1GB identity mapping for the specified virtual address range.
+
+  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
+                                      current CR3)
+  @param[in]  VirtualAddress          Virtual address
+  @param[in]  Length                  Length of virtual address range
+
+  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
+
+**/
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevCreateIdentityMap1G (
+  IN    PHYSICAL_ADDRESS      Cr3BaseAddress,
+  IN    PHYSICAL_ADDRESS      PhysicalAddress,
+  IN    UINTN                 Length
+  );
 #endif
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index c696745f9d26..f146f6d61cc5 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -536,6 +536,121 @@ EnableReadOnlyPageWriteProtect (
   AsmWriteCr0 (AsmReadCr0() | BIT16);
 }
 
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevCreateIdentityMap1G (
+  IN    PHYSICAL_ADDRESS      Cr3BaseAddress,
+  IN    PHYSICAL_ADDRESS      PhysicalAddress,
+  IN    UINTN                 Length
+  )
+{
+  PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
+  PAGE_TABLE_1G_ENTRY            *PageDirectory1GEntry;
+  UINT64                         PgTableMask;
+  UINT64                         AddressEncMask;
+  BOOLEAN                        IsWpEnabled;
+  RETURN_STATUS                  Status;
+
+  //
+  // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
+  //
+  PageMapLevel4Entry = NULL;
+
+  DEBUG ((
+    DEBUG_VERBOSE,
+    "%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx\n",
+    gEfiCallerBaseName,
+    __FUNCTION__,
+    Cr3BaseAddress,
+    PhysicalAddress,
+    (UINT64)Length
+    ));
+
+  if (Length == 0) {
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  //
+  // Check if we have a valid memory encryption mask
+  //
+  AddressEncMask = InternalGetMemEncryptionAddressMask ();
+  if (!AddressEncMask) {
+    return RETURN_ACCESS_DENIED;
+  }
+
+  PgTableMask = AddressEncMask | EFI_PAGE_MASK;
+
+
+  //
+  // Make sure that the page table is changeable.
+  //
+  IsWpEnabled = IsReadOnlyPageWriteProtected ();
+  if (IsWpEnabled) {
+    DisableReadOnlyPageWriteProtect ();
+  }
+
+  Status = EFI_SUCCESS;
+
+  while (Length)
+  {
+    //
+    // If Cr3BaseAddress is not specified then read the current CR3
+    //
+    if (Cr3BaseAddress == 0) {
+      Cr3BaseAddress = AsmReadCr3();
+    }
+
+    PageMapLevel4Entry = (VOID*) (Cr3BaseAddress & ~PgTableMask);
+    PageMapLevel4Entry += PML4_OFFSET(PhysicalAddress);
+    if (!PageMapLevel4Entry->Bits.Present) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a:%a: bad PML4 for Physical=0x%Lx\n",
+        gEfiCallerBaseName,
+        __FUNCTION__,
+        PhysicalAddress
+        ));
+      Status = RETURN_NO_MAPPING;
+      goto Done;
+    }
+
+    PageDirectory1GEntry = (VOID *)(
+                             (PageMapLevel4Entry->Bits.PageTableBaseAddress <<
+                              12) & ~PgTableMask
+                             );
+    PageDirectory1GEntry += PDP_OFFSET(PhysicalAddress);
+    if (!PageDirectory1GEntry->Bits.Present) {
+      PageDirectory1GEntry->Bits.Present = 1;
+      PageDirectory1GEntry->Bits.MustBe1 = 1;
+      PageDirectory1GEntry->Bits.MustBeZero = 0;
+      PageDirectory1GEntry->Bits.ReadWrite = 1;
+      PageDirectory1GEntry->Uint64 |= (UINT64)PhysicalAddress | AddressEncMask;
+    }
+
+    if (Length <= BIT30) {
+      Length = 0;
+    } else {
+      Length -= BIT30;
+    }
+
+    PhysicalAddress += BIT30;
+  }
+
+  //
+  // Flush TLB
+  //
+  CpuFlushTlb();
+
+Done:
+  //
+  // Restore page table write protection, if any.
+  //
+  if (IsWpEnabled) {
+    EnableReadOnlyPageWriteProtect ();
+  }
+
+  return Status;
+}
 
 /**
   This function either sets or clears memory encryption bit for the memory
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
index eae7a31773a4..a0803e1255dc 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -10,9 +10,12 @@
 
 #include <Uefi/UefiBaseType.h>
 #include <Library/BaseLib.h>
+#include <Library/PcdLib.h>
+#include <Library/DebugLib.h>
 #include <Library/MemEncryptSevLib.h>
 
 #include "SnpPageStateChange.h"
+#include "VirtualMemory.h"
 
 typedef struct {
   UINT64    StartAddress;
@@ -105,6 +108,7 @@ MemEncryptSevSnpPreValidateSystemRam (
 {
   PHYSICAL_ADDRESS          EndAddress;
   SNP_PRE_VALIDATED_RANGE   OverlapRange;
+  EFI_STATUS                Status;
 
   if (!MemEncryptSevSnpIsEnabled ()) {
     return;
@@ -112,6 +116,24 @@ MemEncryptSevSnpPreValidateSystemRam (
 
   EndAddress = BaseAddress + EFI_PAGES_TO_SIZE (NumPages);
 
+  //
+  // 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 #GP.
+  //
+  if (BaseAddress >= SIZE_4GB) {
+    Status = InternalMemEncryptSevCreateIdentityMap1G (
+                0,
+                BaseAddress,
+                EFI_PAGES_TO_SIZE (NumPages)
+                );
+    if (EFI_ERROR (Status)) {
+      ASSERT (FALSE);
+      CpuDeadLoop ();
+    }
+  }
+
   while (BaseAddress < EndAddress) {
     //
     // Check if the range overlaps with the pre-validated ranges.
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80900): https://edk2.groups.io/g/devel/message/80900
Mute This Topic: https://groups.io/mt/85749032/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v8 17/32] OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI phase
Posted by Gerd Hoffmann 4 years, 4 months ago
On Mon, Sep 20, 2021 at 01:45:49PM -0500, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> The initial page built during the SEC phase is used by the
> MemEncryptSevSnpValidateSystemRam() for the system RAM validation. The
> page validation process requires using the PVALIDATE instruction;  the
> instruction accepts a virtual address of the memory region that needs
> to be validated. If hardware encounters a page table walk failure (due
> to page-not-present) then it raises #GP.
> 
> The initial page table built in SEC phase address up to 4GB. Add an
> internal function to extend the page table to cover > 4GB. The function
> builds 1GB entries in the page table for access > 4GB. This will provide
> the support to call PVALIDATE instruction for the virtual address >
> 4GB in PEI phase.

Hmm, well, playing with page tables like that in sev-specific code
instead of having memory core handle this properly is quite hackish.

What is the long-term plan with this?  I assume once support for lazy
acceptance/validation is merged we can simply delete this?

Assuming this is only a temporary solution I think we can tolerate the
hacks.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80952): https://edk2.groups.io/g/devel/message/80952
Mute This Topic: https://groups.io/mt/85749032/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v8 17/32] OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI phase
Posted by Brijesh Singh via groups.io 4 years, 4 months ago
On 9/22/21 3:21 AM, Gerd Hoffmann wrote:
> On Mon, Sep 20, 2021 at 01:45:49PM -0500, Brijesh Singh wrote:
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cdb7ae27f87684e0252d008d97da1f85f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637678956888503398%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Cm1E%2BJSrQ%2B%2BCjv5ZqC%2BXNqVGbzwZ32PFGDTZtoL8e84%3D&amp;reserved=0
>>
>> The initial page built during the SEC phase is used by the
>> MemEncryptSevSnpValidateSystemRam() for the system RAM validation. The
>> page validation process requires using the PVALIDATE instruction;  the
>> instruction accepts a virtual address of the memory region that needs
>> to be validated. If hardware encounters a page table walk failure (due
>> to page-not-present) then it raises #GP.
>>
>> The initial page table built in SEC phase address up to 4GB. Add an
>> internal function to extend the page table to cover > 4GB. The function
>> builds 1GB entries in the page table for access > 4GB. This will provide
>> the support to call PVALIDATE instruction for the virtual address >
>> 4GB in PEI phase.
> Hmm, well, playing with page tables like that in sev-specific code
> instead of having memory core handle this properly is quite hackish.
>
> What is the long-term plan with this?  I assume once support for lazy
> acceptance/validation is merged we can simply delete this?

Yes, this is just an interim problem. Once we move to lazy validation
then this will be removed.


>
> Assuming this is only a temporary solution I think we can tolerate the
> hacks.
>
> take care,
>   Gerd
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81085): https://edk2.groups.io/g/devel/message/81085
Mute This Topic: https://groups.io/mt/85749032/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-