]> git.proxmox.com Git - mirror_edk2.git/commitdiff
OvmfPkg/PlatformPei: rewrite MemTypeInfo HOB production logic
authorLaszlo Ersek <lersek@redhat.com>
Fri, 8 May 2020 12:16:49 +0000 (14:16 +0200)
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Mon, 18 May 2020 15:48:48 +0000 (15:48 +0000)
The previous patch has no effect -- i.e., it cannot stop the tracking of
BS Code/Data in MemTypeInfo -- if the virtual machine already has a
MemoryTypeInformation UEFI variable.

In that case, our current logic allows the DXE IPL PEIM to translate the
UEFI variable to the HOB, and that translation is verbatim. If the
variable already contains records for BS Code/Data, the issues listed in
the previous patch persist for the virtual machine.

For this reason, *always* install PlatformPei's own MemTypeInfo HOB. This
prevents the DXE IPL PEIM's variable-to-HOB translation.

In PlatformPei, consume the records in the MemoryTypeInformation UEFI
variable as hints:

- Ignore all memory types for which we wouldn't by default install records
  in the HOB. This hides BS Code/Data from any existent
  MemoryTypeInformation variable.

- For the memory types that our defaults cover, enable the records in the
  UEFI variable to increase (and *only* to increase) the page counts.

  This lets the MemoryTypeInformation UEFI variable function as designed,
  but it eliminates a reboot when such a new OVMF binary is deployed (a)
  that has higher memory consumption than tracked by the virtual machine's
  UEFI variable previously, *but* (b) whose defaults also reflect those
  higher page counts.

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2706
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200508121651.16045-3-lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
OvmfPkg/PlatformPei/MemTypeInfo.c

index 8100a2db7d44d6b62f0ed319bdb66c554c4f069a..d287fb9d7df015e87b9cb37712261b0cb373311b 100644 (file)
@@ -1,7 +1,5 @@
 /** @file\r
-  Produce a default memory type information HOB unless we can determine, from\r
-  the existence of the "MemoryTypeInformation" variable, that the DXE IPL PEIM\r
-  will produce the HOB.\r
+  Produce the memory type information HOB.\r
 \r
   Copyright (C) 2017-2020, Red Hat, Inc.\r
 \r
@@ -25,7 +23,7 @@
 // of BIN hints that made sense at a particular time, for some (now likely\r
 // unknown) workloads / boot paths.\r
 //\r
-STATIC EFI_MEMORY_TYPE_INFORMATION mDefaultMemoryTypeInformation[] = {\r
+STATIC EFI_MEMORY_TYPE_INFORMATION mMemoryTypeInformation[] = {\r
   { EfiACPIMemoryNVS,       0x004 },\r
   { EfiACPIReclaimMemory,   0x008 },\r
   { EfiReservedMemoryType,  0x004 },\r
@@ -42,75 +40,144 @@ BuildMemTypeInfoHob (
 {\r
   BuildGuidDataHob (\r
     &gEfiMemoryTypeInformationGuid,\r
-    mDefaultMemoryTypeInformation,\r
-    sizeof mDefaultMemoryTypeInformation\r
+    mMemoryTypeInformation,\r
+    sizeof mMemoryTypeInformation\r
     );\r
-  DEBUG ((DEBUG_INFO, "%a: default memory type information HOB built\n",\r
-    __FUNCTION__));\r
 }\r
 \r
 /**\r
-  Notification function called when EFI_PEI_READ_ONLY_VARIABLE2_PPI becomes\r
-  available.\r
+  Refresh the mMemoryTypeInformation array (which we'll turn into the\r
+  MemoryTypeInformation HOB) from the MemoryTypeInformation UEFI variable.\r
 \r
-  @param[in] PeiServices      Indirect reference to the PEI Services Table.\r
-  @param[in] NotifyDescriptor Address of the notification descriptor data\r
-                              structure.\r
-  @param[in] Ppi              Address of the PPI that was installed.\r
+  Normally, the DXE IPL PEIM builds the HOB from the UEFI variable. But it does\r
+  so *transparently*. Instead, we consider the UEFI variable as a list of\r
+  hints, for updating our HOB defaults:\r
 \r
-  @return  Status of the notification. The status code returned from this\r
-           function is ignored.\r
+  - Record types not covered in mMemoryTypeInformation are ignored. In\r
+    particular, this hides record types from the UEFI variable that may lead to\r
+    reboots without benefiting SMM security, such as EfiBootServicesData.\r
+\r
+  - Records that would lower the defaults in mMemoryTypeInformation are also\r
+    ignored.\r
+\r
+  @param[in] ReadOnlyVariable2  The EFI_PEI_READ_ONLY_VARIABLE2_PPI used for\r
+                                retrieving the MemoryTypeInformation UEFI\r
+                                variable.\r
 **/\r
 STATIC\r
-EFI_STATUS\r
-EFIAPI\r
-OnReadOnlyVariable2Available (\r
-  IN EFI_PEI_SERVICES           **PeiServices,\r
-  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,\r
-  IN VOID                       *Ppi\r
+VOID\r
+RefreshMemTypeInfo (\r
+  IN EFI_PEI_READ_ONLY_VARIABLE2_PPI *ReadOnlyVariable2\r
   )\r
 {\r
-  EFI_PEI_READ_ONLY_VARIABLE2_PPI *ReadOnlyVariable2;\r
-  UINTN                           DataSize;\r
-  EFI_STATUS                      Status;\r
-\r
-  DEBUG ((DEBUG_VERBOSE, "%a\n", __FUNCTION__));\r
+  UINTN                       DataSize;\r
+  EFI_MEMORY_TYPE_INFORMATION Entries[EfiMaxMemoryType + 1];\r
+  EFI_STATUS                  Status;\r
+  UINTN                       NumEntries;\r
+  UINTN                       HobRecordIdx;\r
 \r
   //\r
-  // Check if the "MemoryTypeInformation" variable exists, in the\r
+  // Read the MemoryTypeInformation UEFI variable from the\r
   // gEfiMemoryTypeInformationGuid namespace.\r
   //\r
-  ReadOnlyVariable2 = Ppi;\r
-  DataSize = 0;\r
+  DataSize = sizeof Entries;\r
   Status = ReadOnlyVariable2->GetVariable (\r
                                 ReadOnlyVariable2,\r
                                 EFI_MEMORY_TYPE_INFORMATION_VARIABLE_NAME,\r
                                 &gEfiMemoryTypeInformationGuid,\r
                                 NULL,\r
                                 &DataSize,\r
-                                NULL\r
+                                Entries\r
                                 );\r
-  switch (Status) {\r
-  case EFI_BUFFER_TOO_SMALL:\r
+  if (EFI_ERROR (Status)) {\r
     //\r
-    // The variable exists; the DXE IPL PEIM will build the HOB from it.\r
+    // If the UEFI variable does not exist (EFI_NOT_FOUND), we can't use it for\r
+    // udpating mMemoryTypeInformation.\r
     //\r
-    break;\r
-  case EFI_NOT_FOUND:\r
+    // If the UEFI variable exists but Entries is too small to hold it\r
+    // (EFI_BUFFER_TOO_SMALL), then the variable contents are arguably invalid.\r
+    // That's because Entries has room for every distinct EFI_MEMORY_TYPE,\r
+    // including the terminator record with EfiMaxMemoryType. Thus, we can't\r
+    // use the UEFI variable for updating mMemoryTypeInformation.\r
     //\r
-    // The variable does not exist; install the default memory type information\r
-    // HOB.\r
+    // If the UEFI variable couldn't be read for some other reason, we\r
+    // similarly can't use it for udpating mMemoryTypeInformation.\r
     //\r
-    BuildMemTypeInfoHob ();\r
-    break;\r
-  default:\r
-    DEBUG ((DEBUG_ERROR, "%a: unexpected: GetVariable(): %r\n", __FUNCTION__,\r
-      Status));\r
-    ASSERT (FALSE);\r
-    CpuDeadLoop ();\r
-    break;\r
+    DEBUG ((DEBUG_ERROR, "%a: GetVariable(): %r\n", __FUNCTION__, Status));\r
+    return;\r
   }\r
 \r
+  //\r
+  // Sanity-check the UEFI variable size against the record size.\r
+  //\r
+  if (DataSize % sizeof Entries[0] != 0) {\r
+    DEBUG ((DEBUG_ERROR, "%a: invalid UEFI variable size %Lu\n", __FUNCTION__,\r
+      (UINT64)DataSize));\r
+    return;\r
+  }\r
+  NumEntries = DataSize / sizeof Entries[0];\r
+\r
+  //\r
+  // For each record in mMemoryTypeInformation, except the terminator record,\r
+  // look up the first match (if any) in the UEFI variable, based on the memory\r
+  // type.\r
+  //\r
+  for (HobRecordIdx = 0;\r
+       HobRecordIdx < ARRAY_SIZE (mMemoryTypeInformation) - 1;\r
+       HobRecordIdx++) {\r
+    EFI_MEMORY_TYPE_INFORMATION *HobRecord;\r
+    UINTN                       Idx;\r
+    EFI_MEMORY_TYPE_INFORMATION *VariableRecord;\r
+\r
+    HobRecord = &mMemoryTypeInformation[HobRecordIdx];\r
+\r
+    for (Idx = 0; Idx < NumEntries; Idx++) {\r
+      VariableRecord = &Entries[Idx];\r
+\r
+      if (VariableRecord->Type == HobRecord->Type) {\r
+        break;\r
+      }\r
+    }\r
+\r
+    //\r
+    // If there is a match, allow the UEFI variable to increase NumberOfPages.\r
+    //\r
+    if (Idx < NumEntries &&\r
+        HobRecord->NumberOfPages < VariableRecord->NumberOfPages) {\r
+      DEBUG ((DEBUG_VERBOSE, "%a: Type 0x%x: NumberOfPages 0x%x -> 0x%x\n",\r
+        __FUNCTION__, HobRecord->Type, HobRecord->NumberOfPages,\r
+        VariableRecord->NumberOfPages));\r
+\r
+      HobRecord->NumberOfPages = VariableRecord->NumberOfPages;\r
+    }\r
+  }\r
+}\r
+\r
+/**\r
+  Notification function called when EFI_PEI_READ_ONLY_VARIABLE2_PPI becomes\r
+  available.\r
+\r
+  @param[in] PeiServices      Indirect reference to the PEI Services Table.\r
+  @param[in] NotifyDescriptor Address of the notification descriptor data\r
+                              structure.\r
+  @param[in] Ppi              Address of the PPI that was installed.\r
+\r
+  @return  Status of the notification. The status code returned from this\r
+           function is ignored.\r
+**/\r
+STATIC\r
+EFI_STATUS\r
+EFIAPI\r
+OnReadOnlyVariable2Available (\r
+  IN EFI_PEI_SERVICES           **PeiServices,\r
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,\r
+  IN VOID                       *Ppi\r
+  )\r
+{\r
+  DEBUG ((DEBUG_VERBOSE, "%a\n", __FUNCTION__));\r
+\r
+  RefreshMemTypeInfo (Ppi);\r
+  BuildMemTypeInfoHob ();\r
   return EFI_SUCCESS;\r
 }\r
 \r