public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@linaro.org>
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Laszlo Ersek <lersek@redhat.com>,
	Leif Lindholm <leif@nuviainc.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Ray Ni <ray.ni@intel.com>, Jiewen Yao <jiewen.yao@intel.com>,
	Bret Barkelew <Bret.Barkelew@microsoft.com>
Subject: [PATCH 2/4] MdeModulePkg: disable properties table generation but retain the code
Date: Thu, 26 Mar 2020 11:24:41 +0100	[thread overview]
Message-ID: <20200326102443.748-3-ard.biesheuvel@linaro.org> (raw)
In-Reply-To: <20200326102443.748-1-ard.biesheuvel@linaro.org>

This is the minimal change required to stop exposing the EFI properties
table, which is deprecated. Given how the implementation is entangled
with the code that exposes the related memory attributes table, most of
the code is retained, and further cleanups are relegated to subsequent
patches.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=2633
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Core/Dxe/DxeMain.inf                  |   2 -
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c |   7 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      |   1 -
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c       | 107 ++------------------
 MdeModulePkg/MdeModulePkg.dec                      |  24 -----
 MdeModulePkg/MdeModulePkg.uni                      |  21 ----
 6 files changed, 14 insertions(+), 148 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 61161bee28e1..75e0a968f0cf 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -120,7 +120,6 @@ [Guids]
   gEventExitBootServicesFailedGuid              ## SOMETIMES_PRODUCES   ## Event
   gEfiVectorHandoffTableGuid                    ## SOMETIMES_PRODUCES   ## SystemTable
   gEdkiiMemoryProfileGuid                       ## SOMETIMES_PRODUCES   ## GUID # Install protocol
-  gEfiPropertiesTableGuid                       ## SOMETIMES_PRODUCES   ## SystemTable
   gEfiMemoryAttributesTableGuid                 ## SOMETIMES_PRODUCES   ## SystemTable
   gEfiEndOfDxeEventGroupGuid                    ## SOMETIMES_CONSUMES   ## Event
   gEfiHobMemoryAllocStackGuid                   ## SOMETIMES_CONSUMES   ## SystemTable
@@ -180,7 +179,6 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileMemoryType                 ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask               ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath                 ## CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable                   ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy                   ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy             ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask        ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index ebdeb35079a2..0b0ed4413c28 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -18,7 +18,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Guid/EventGroup.h>
 
 #include <Guid/MemoryAttributesTable.h>
-#include <Guid/PropertiesTable.h>
 
 #include "DxeMain.h"
 
@@ -64,7 +63,7 @@ CoreGetMemoryMapWithSeparatedImageSection (
   OUT UINT32                    *DescriptorVersion
   );
 
-extern EFI_PROPERTIES_TABLE  mPropertiesTable;
+BOOLEAN                      mMemoryAttributesTableEnable = TRUE;
 EFI_MEMORY_ATTRIBUTES_TABLE  *mMemoryAttributesTable = NULL;
 BOOLEAN                      mMemoryAttributesTableReadyToBoot = FALSE;
 
@@ -96,8 +95,8 @@ InstallMemoryAttributesTable (
     return;
   }
 
-  if ((mPropertiesTable.MemoryProtectionAttribute & EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA) == 0) {
-    DEBUG ((EFI_D_VERBOSE, "MemoryProtectionAttribute NON_EXECUTABLE_PE_DATA is not set, "));
+  if (!mMemoryAttributesTableEnable) {
+    DEBUG ((EFI_D_VERBOSE, "Cannot install Memory Attributes Table "));
     DEBUG ((EFI_D_VERBOSE, "because Runtime Driver Section Alignment is not %dK.\n", RUNTIME_PAGE_ALLOCATION_GRANULARITY >> 10));
     return ;
   }
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 47edf86dfbf3..92a442f517b2 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -35,7 +35,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Guid/EventGroup.h>
 #include <Guid/MemoryAttributesTable.h>
-#include <Guid/PropertiesTable.h>
 
 #include <Protocol/FirmwareVolume2.h>
 #include <Protocol/SimpleFileSystem.h>
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 53bb6b7c912c..6ee8a8af9098 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -23,8 +23,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PeCoffGetEntryPointLib.h>
 #include <Protocol/Runtime.h>
 
-#include <Guid/PropertiesTable.h>
-
 #include "DxeMain.h"
 #include "HeapGuard.h"
 
@@ -47,18 +45,12 @@ IMAGE_PROPERTIES_PRIVATE_DATA  mImagePropertiesPrivateData = {
   INITIALIZE_LIST_HEAD_VARIABLE (mImagePropertiesPrivateData.ImageRecordList)
 };
 
-EFI_PROPERTIES_TABLE  mPropertiesTable = {
-  EFI_PROPERTIES_TABLE_VERSION,
-  sizeof(EFI_PROPERTIES_TABLE),
-  EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA
-};
-
 EFI_LOCK           mPropertiesTableLock = EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTIFY);
 
-BOOLEAN            mPropertiesTableEnable;
-
 BOOLEAN            mPropertiesTableEndOfDxe = FALSE;
 
+extern BOOLEAN     mMemoryAttributesTableEnable;
+
 //
 // Below functions are for MemoryMap
 //
@@ -359,11 +351,7 @@ SetNewRecord (
       //
       // DATA
       //
-      if (!mPropertiesTableEnable) {
-        NewRecord->Type = TempRecord.Type;
-      } else {
-        NewRecord->Type = EfiRuntimeServicesData;
-      }
+      NewRecord->Type          = TempRecord.Type;
       NewRecord->PhysicalStart = TempRecord.PhysicalStart;
       NewRecord->VirtualStart  = 0;
       NewRecord->NumberOfPages = EfiSizeToPages(ImageRecordCodeSection->CodeSegmentBase - NewRecord->PhysicalStart);
@@ -376,11 +364,7 @@ SetNewRecord (
       //
       // CODE
       //
-      if (!mPropertiesTableEnable) {
-        NewRecord->Type = TempRecord.Type;
-      } else {
-        NewRecord->Type = EfiRuntimeServicesCode;
-      }
+      NewRecord->Type          = TempRecord.Type;
       NewRecord->PhysicalStart = ImageRecordCodeSection->CodeSegmentBase;
       NewRecord->VirtualStart  = 0;
       NewRecord->NumberOfPages = EfiSizeToPages(ImageRecordCodeSection->CodeSegmentSize);
@@ -404,11 +388,7 @@ SetNewRecord (
   // Final DATA
   //
   if (TempRecord.PhysicalStart < ImageEnd) {
-    if (!mPropertiesTableEnable) {
-      NewRecord->Type = TempRecord.Type;
-    } else {
-      NewRecord->Type = EfiRuntimeServicesData;
-    }
+    NewRecord->Type          = TempRecord.Type;
     NewRecord->PhysicalStart = TempRecord.PhysicalStart;
     NewRecord->VirtualStart  = 0;
     NewRecord->NumberOfPages = EfiSizeToPages (ImageEnd - TempRecord.PhysicalStart);
@@ -519,14 +499,8 @@ SplitRecord (
         //
         NewRecord = PREVIOUS_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
         IsLastRecordData = FALSE;
-        if (!mPropertiesTableEnable) {
-          if ((NewRecord->Attribute & EFI_MEMORY_XP) != 0) {
-            IsLastRecordData = TRUE;
-          }
-        } else {
-          if (NewRecord->Type == EfiRuntimeServicesData) {
-            IsLastRecordData = TRUE;
-          }
+        if ((NewRecord->Attribute & EFI_MEMORY_XP) != 0) {
+          IsLastRecordData = TRUE;
         }
         if (IsLastRecordData) {
           //
@@ -538,11 +512,7 @@ SplitRecord (
           // Last record is CODE, create a new DATA entry.
           //
           NewRecord = NEXT_MEMORY_DESCRIPTOR (NewRecord, DescriptorSize);
-          if (!mPropertiesTableEnable) {
-            NewRecord->Type = TempRecord.Type;
-          } else {
-            NewRecord->Type = EfiRuntimeServicesData;
-          }
+          NewRecord->Type          = TempRecord.Type;
           NewRecord->PhysicalStart = TempRecord.PhysicalStart;
           NewRecord->VirtualStart  = 0;
           NewRecord->NumberOfPages = TempRecord.NumberOfPages;
@@ -751,7 +721,7 @@ CoreGetMemoryMapWithSeparatedImageSection (
   //
   // If PE code/data is not aligned, just return.
   //
-  if ((mPropertiesTable.MemoryProtectionAttribute & EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA) == 0) {
+  if (!mMemoryAttributesTableEnable) {
     return CoreGetMemoryMap (MemoryMapSize, MemoryMap, MapKey, DescriptorSize, DescriptorVersion);
   }
 
@@ -803,12 +773,9 @@ SetPropertiesTableSectionAlignment (
   )
 {
   if (((SectionAlignment & (RUNTIME_PAGE_ALLOCATION_GRANULARITY - 1)) != 0) &&
-      ((mPropertiesTable.MemoryProtectionAttribute & EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA) != 0)) {
+      mMemoryAttributesTableEnable) {
     DEBUG ((EFI_D_VERBOSE, "SetPropertiesTableSectionAlignment - Clear\n"));
-    mPropertiesTable.MemoryProtectionAttribute &= ~((UINT64)EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA);
-    gBS->GetMemoryMap = CoreGetMemoryMap;
-    gBS->Hdr.CRC32 = 0;
-    gBS->CalculateCrc32 ((UINT8 *)gBS, gBS->Hdr.HeaderSize, &gBS->Hdr.CRC32);
+    mMemoryAttributesTableEnable = FALSE;
   }
 }
 
@@ -1018,35 +985,6 @@ SortImageRecord (
   }
 }
 
-/**
-  Dump image record.
-**/
-STATIC
-VOID
-DumpImageRecord (
-  VOID
-  )
-{
-  IMAGE_PROPERTIES_RECORD      *ImageRecord;
-  LIST_ENTRY                   *ImageRecordLink;
-  LIST_ENTRY                   *ImageRecordList;
-  UINTN                        Index;
-
-  ImageRecordList = &mImagePropertiesPrivateData.ImageRecordList;
-
-  for (ImageRecordLink = ImageRecordList->ForwardLink, Index= 0;
-       ImageRecordLink != ImageRecordList;
-       ImageRecordLink = ImageRecordLink->ForwardLink, Index++) {
-    ImageRecord = CR (
-                    ImageRecordLink,
-                    IMAGE_PROPERTIES_RECORD,
-                    Link,
-                    IMAGE_PROPERTIES_RECORD_SIGNATURE
-                    );
-    DEBUG ((EFI_D_VERBOSE, "  Image[%d]: 0x%016lx - 0x%016lx\n", Index, ImageRecord->ImageBase, ImageRecord->ImageSize));
-  }
-}
-
 /**
   Insert image record.
 
@@ -1323,29 +1261,6 @@ InstallPropertiesTable (
   )
 {
   mPropertiesTableEndOfDxe = TRUE;
-  if (PcdGetBool (PcdPropertiesTableEnable)) {
-    EFI_STATUS  Status;
-
-    Status = gBS->InstallConfigurationTable (&gEfiPropertiesTableGuid, &mPropertiesTable);
-    ASSERT_EFI_ERROR (Status);
-
-    DEBUG ((EFI_D_INFO, "MemoryProtectionAttribute - 0x%016lx\n", mPropertiesTable.MemoryProtectionAttribute));
-    if ((mPropertiesTable.MemoryProtectionAttribute & EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA) == 0) {
-      DEBUG ((EFI_D_ERROR, "MemoryProtectionAttribute NON_EXECUTABLE_PE_DATA is not set, "));
-      DEBUG ((EFI_D_ERROR, "because Runtime Driver Section Alignment is not %dK.\n", RUNTIME_PAGE_ALLOCATION_GRANULARITY >> 10));
-      return ;
-    }
-
-    gBS->GetMemoryMap = CoreGetMemoryMapWithSeparatedImageSection;
-    gBS->Hdr.CRC32 = 0;
-    gBS->CalculateCrc32 ((UINT8 *)gBS, gBS->Hdr.HeaderSize, &gBS->Hdr.CRC32);
-
-    DEBUG ((EFI_D_VERBOSE, "Total Image Count - 0x%x\n", mImagePropertiesPrivateData.ImageRecordCount));
-    DEBUG ((EFI_D_VERBOSE, "Dump ImageRecord:\n"));
-    DumpImageRecord ();
-
-    mPropertiesTableEnable = TRUE;
-  }
 }
 
 /**
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 91a3c608231c..aeb8b820db1b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1866,30 +1866,6 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   # @Prompt Flag to request system reboot after processing capsule.
   gEfiMdeModulePkgTokenSpaceGuid.PcdSystemRebootAfterCapsuleProcessFlag|0x0001|UINT16|0x0000006d
 
-  ## Publish PropertiesTable or not.
-  #
-  # If this PCD is TRUE, DxeCore publishs PropertiesTable.
-  # DxeCore evaluates if all runtime drivers has 4K aligned PE sections. If all
-  # PE sections in runtime drivers are 4K aligned, DxeCore sets BIT0 in
-  # PropertiesTable. Or DxeCore clears BIT0 in PropertiesTable.
-  # If this PCD is FALSE, DxeCore does not publish PropertiesTable.
-  #
-  # If PropertiesTable has BIT0 set, DxeCore uses below policy in UEFI memory map:
-  #   1) Use EfiRuntimeServicesCode for runtime driver PE image code section and
-  #      use EfiRuntimeServicesData for runtime driver PE image header and other section.
-  #   2) Set EfiRuntimeServicesCode to be EFI_MEMORY_RO.
-  #   3) Set EfiRuntimeServicesData to be EFI_MEMORY_XP.
-  #   4) Set EfiMemoryMappedIO and EfiMemoryMappedIOPortSpace to be EFI_MEMORY_XP.
-  #
-  # NOTE: Platform need gurantee this PCD is set correctly. Platform should set
-  # this PCD to be TURE if and only if all runtime driver has seperated Code/Data
-  # section. If PE code/data sections are merged, the result is unpredictable.
-  #
-  # UEFI 2.6 specification does not recommend to use this BIT0 attribute.
-  #
-  # @Prompt Publish UEFI PropertiesTable.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE|BOOLEAN|0x0000006e
-
   ## Default OEM ID for ACPI table creation, its length must be 0x6 bytes to follow ACPI specification.
   # @Prompt Default OEM ID for ACPI table creation.
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"INTEL "|VOID*|0x30001034
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 2c856ed07333..2007e0596c4f 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -891,27 +891,6 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFrontPageFormSetGuid_HELP  #language en-US "This PCD points to the front page formset GUID\n"
                                                                                          "Compare the FormsetGuid or ClassGuid with this PCD value can detect whether in front page"
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPropertiesTableEnable_PROMPT  #language en-US "Publish UEFI PropertiesTable."
-
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPropertiesTableEnable_HELP  #language en-US "Publish PropertiesTable or not.\n"
-                                                                                          "\n"
-                                                                                          "If this PCD is TRUE, DxeCore publishs PropertiesTable.\n"
-                                                                                          "DxeCore evaluates if all runtime drivers has 4K aligned PE sections. If all\n"
-                                                                                          "PE sections in runtime drivers are 4K aligned, DxeCore sets BIT0 in\n"
-                                                                                          "PropertiesTable. Or DxeCore clears BIT0 in PropertiesTable.\n"
-                                                                                          "If this PCD is FALSE, DxeCore does not publish PropertiesTable.\n"
-                                                                                          "\n"
-                                                                                          "If PropertiesTable has BIT0 set, DxeCore uses below policy in UEFI memory map:\n"
-                                                                                          "1) Use EfiRuntimeServicesCode for runtime driver PE image code section and\n"
-                                                                                          "use EfiRuntimeServicesData for runtime driver PE image header and other section.\n"
-                                                                                          "2) Set EfiRuntimeServicesCode to be EFI_MEMORY_RO.\n"
-                                                                                          "3) Set EfiRuntimeServicesData to be EFI_MEMORY_XP.\n"
-                                                                                          "4) Set EfiMemoryMappedIO and EfiMemoryMappedIOPortSpace to be EFI_MEMORY_XP.\n"
-                                                                                          "\n"
-                                                                                          "NOTE: Platform need gurantee this PCD is set correctly. Platform should set\n"
-                                                                                          "this PCD to be TURE if and only if all runtime driver has seperated Code/Data\n"
-                                                                                          "section. If PE code/data sections are merged, the result is unpredictable.\n"
-
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdShadowPeimOnBoot_HELP  #language en-US "Indicates if to shadow PEIM and PeiCore after memory is ready.<BR><BR>\n"
                                                                                      "This PCD is used on other boot path except for S3 boot.\n"
                                                                                      "TRUE  - Shadow PEIM and PeiCore after memory is ready.<BR>\n"
-- 
2.17.1


  parent reply	other threads:[~2020-03-26 10:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26 10:24 [PATCH 0/4] remove generation of EFI properties table Ard Biesheuvel
2020-03-26 10:24 ` [PATCH 1/4] OvmfPkg: remove handling of " Ard Biesheuvel
2020-03-27 14:01   ` [edk2-devel] " Laszlo Ersek
2020-03-26 10:24 ` Ard Biesheuvel [this message]
2020-03-26 10:24 ` [PATCH 3/4] MdePkg: remove PropertiesTable GUID Ard Biesheuvel
2020-03-27  1:02   ` [EXTERNAL] " Bret Barkelew
2020-03-27  9:16     ` Ard Biesheuvel
2020-03-26 10:24 ` [PATCH 4/4] MdeModulePkg/DxeCore: merge properties table routines into MAT handling Ard Biesheuvel
2020-03-27  5:00 ` [PATCH 0/4] remove generation of EFI properties table Yao, Jiewen
2020-03-30 13:42   ` [edk2-devel] " Liming Gao
2020-03-30 17:57     ` Ard Biesheuvel
2020-04-01 17:17       ` Ard Biesheuvel
2020-04-03  2:22 ` Dandan Bi
2020-04-06 11:42   ` Ard Biesheuvel
2020-04-07  7:27     ` Wang, Jian J
2020-04-07  8:22       ` Ard Biesheuvel

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=20200326102443.748-3-ard.biesheuvel@linaro.org \
    --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