File ovmf-bsc1092943-fix-attributes-table.patch of Package ovmf

Overview Repositories Revisions Requests Users Attributes Meta

File ovmf-bsc1092943-fix-attributes-table.patch of Package ovmf (Revision 78fdba678982b7e987d8ff1fd27fa0a1)

Currently displaying revision 78fdba678982b7e987d8ff1fd27fa0a1 , Show latest

From 05d65cc26b15944e1d57c11e63072f1ab71c77b2 Mon Sep 17 00:00:00 2001
From: Jian J Wang <jian.j.wang@intel.com>
Date: Mon, 23 Oct 2017 14:49:02 +0800
Subject: [PATCH 1/3] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in
 memory map

More than one entry of RT_CODE memory might cause boot problem for some
old OSs. This patch will fix this issue to keep OS compatibility as much
as possible.

More detailed information, please refer to
    https://bugzilla.tianocore.org/show_bug.cgi?id=753

Laszlo did a thorough test on OVMF emulated platform. The details can be found
at
    https://bugzilla.tianocore.org/show_bug.cgi?id=753#c10

Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from commit 768bd967847e91aa236c1eed3507e0863547f544)
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 94 +++++++++++++++++++++++++++++++---------
 1 file changed, 73 insertions(+), 21 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 309f448a8357..db41fa401f19 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -769,6 +769,20 @@ AssignMemoryPageAttributes (
   return Status;
 }
 
+/**
+ Check if Execute Disable feature is enabled or not.
+**/
+BOOLEAN
+IsExecuteDisableEnabled (
+  VOID
+  )
+{
+  MSR_CORE_IA32_EFER_REGISTER    MsrEfer;
+
+  MsrEfer.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
+  return (MsrEfer.Bits.NXE == 1);
+}
+
 /**
   Update GCD memory space attributes according to current page table setup.
 **/
@@ -790,7 +804,7 @@ RefreshGcdMemoryAttributesFromPaging (
   UINT64                              PageStartAddress;
   UINT64                              Attributes;
   UINT64                              Capabilities;
-  BOOLEAN                             DoUpdate;
+  UINT64                              NewAttributes;
   UINTN                               Index;
 
   //
@@ -802,17 +816,50 @@ RefreshGcdMemoryAttributesFromPaging (
 
   GetCurrentPagingContext (&PagingContext);
 
-  DoUpdate      = FALSE;
-  Capabilities  = 0;
-  Attributes    = 0;
-  BaseAddress   = 0;
-  PageLength    = 0;
+  Attributes      = 0;
+  NewAttributes   = 0;
+  BaseAddress     = 0;
+  PageLength      = 0;
+
+  if (IsExecuteDisableEnabled ()) {
+    Capabilities = EFI_MEMORY_RO | EFI_MEMORY_RP | EFI_MEMORY_XP;
+  } else {
+    Capabilities = EFI_MEMORY_RO | EFI_MEMORY_RP;
+  }
 
   for (Index = 0; Index < NumberOfDescriptors; Index++) {
     if (MemorySpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
       continue;
     }
 
+    //
+    // Sync the actual paging related capabilities back to GCD service first.
+    // As a side effect (good one), this can also help to avoid unnecessary
+    // memory map entries due to the different capabilities of the same type
+    // memory, such as multiple RT_CODE and RT_DATA entries in memory map,
+    // which could cause boot failure of some old Linux distro (before v4.3).
+    //
+    Status = gDS->SetMemorySpaceCapabilities (
+                    MemorySpaceMap[Index].BaseAddress,
+                    MemorySpaceMap[Index].Length,
+                    MemorySpaceMap[Index].Capabilities | Capabilities
+                    );
+    if (EFI_ERROR (Status)) {
+      //
+      // If we cannot udpate the capabilities, we cannot update its
+      // attributes either. So just simply skip current block of memory.
+      //
+      DEBUG ((
+        DEBUG_WARN,
+        "Failed to update capability: [%lu] %016lx - %016lx (%016lx -> %016lx)\r\n",
+        (UINT64)Index, MemorySpaceMap[Index].BaseAddress,
+        MemorySpaceMap[Index].BaseAddress + MemorySpaceMap[Index].Length - 1,
+        MemorySpaceMap[Index].Capabilities,
+        MemorySpaceMap[Index].Capabilities | Capabilities
+        ));
+      continue;
+    }
+
     if (MemorySpaceMap[Index].BaseAddress >= (BaseAddress + PageLength)) {
       //
       // Current memory space starts at a new page. Resetting PageLength will
@@ -826,7 +873,9 @@ RefreshGcdMemoryAttributesFromPaging (
       PageLength -= (MemorySpaceMap[Index].BaseAddress - BaseAddress);
     }
 
-    // Sync real page attributes to GCD
+    //
+    // Sync actual page attributes to GCD
+    //
     BaseAddress       = MemorySpaceMap[Index].BaseAddress;
     MemorySpaceLength = MemorySpaceMap[Index].Length;
     while (MemorySpaceLength > 0) {
@@ -842,23 +891,26 @@ RefreshGcdMemoryAttributesFromPaging (
         PageStartAddress  = (*PageEntry) & (UINT64)PageAttributeToMask(PageAttribute);
         PageLength        = PageAttributeToLength (PageAttribute) - (BaseAddress - PageStartAddress);
         Attributes        = GetAttributesFromPageEntry (PageEntry);
-
-        if (Attributes != (MemorySpaceMap[Index].Attributes & EFI_MEMORY_PAGETYPE_MASK)) {
-          DoUpdate = TRUE;
-          Attributes |= (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_PAGETYPE_MASK);
-          Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
-        } else {
-          DoUpdate = FALSE;
-        }
       }
 
       Length = MIN (PageLength, MemorySpaceLength);
-      if (DoUpdate) {
-        gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
-        gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
-        DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx - %016lx (%08lx -> %08lx)\r\n",
-                             Index, BaseAddress, BaseAddress + Length - 1,
-                             MemorySpaceMap[Index].Attributes, Attributes));
+      if (Attributes != (MemorySpaceMap[Index].Attributes &
+                         EFI_MEMORY_PAGETYPE_MASK)) {
+        NewAttributes = (MemorySpaceMap[Index].Attributes &
+                         ~EFI_MEMORY_PAGETYPE_MASK) | Attributes;
+        Status = gDS->SetMemorySpaceAttributes (
+                        BaseAddress,
+                        Length,
+                        NewAttributes
+                        );
+        ASSERT_EFI_ERROR (Status);
+        DEBUG ((
+          DEBUG_INFO,
+          "Updated memory space attribute: [%lu] %016lx - %016lx (%016lx -> %016lx)\r\n",
+          (UINT64)Index, BaseAddress, BaseAddress + Length - 1,
+          MemorySpaceMap[Index].Attributes,
+          NewAttributes
+          ));
       }
 
       PageLength        -= Length;
-- 
2.16.3

From 7478b4efda85470b9507b8c693a955763c9355bd Mon Sep 17 00:00:00 2001
From: Jian J Wang <jian.j.wang@intel.com>
Date: Thu, 23 Nov 2017 09:48:33 +0800
Subject: [PATCH 2/3] MdeModulePkg/DxeCore: Filter out all paging capabilities

Some OSs will treat EFI_MEMORY_DESCRIPTOR.Attribute as really
set attributes and change memory paging attribute accordingly.
But current EFI_MEMORY_DESCRIPTOR.Attribute is assigned by
value from Capabilities in GCD memory map. This might cause
boot problems. Clearing all paging related capabilities can
workaround it. The code added in this patch is supposed to
be removed once the usage of EFI_MEMORY_DESCRIPTOR.Attribute
is clarified in UEFI spec and adopted by both EDK-II Core and
all supported OSs.

Laszlo did a thorough test on OVMF emulated platform. The details
can be found at
    https://bugzilla.tianocore.org/show_bug.cgi?id=753#c10

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from commit e38451cd9a87f86d9c699281252d381e10dcc98e)
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index f1e4a37f2af6..5636d3bf421d 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1687,6 +1687,7 @@ CoreGetMemoryMap (
   EFI_GCD_MAP_ENTRY                 MergeGcdMapEntry;
   EFI_MEMORY_TYPE                   Type;
   EFI_MEMORY_DESCRIPTOR             *MemoryMapStart;
+  EFI_MEMORY_DESCRIPTOR             *MemoryMapEnd;
 
   //
   // Make sure the parameters are valid
@@ -1896,6 +1897,25 @@ CoreGetMemoryMap (
   //
   BufferSize = ((UINT8 *)MemoryMap - (UINT8 *)MemoryMapStart);
 
+  //
+  // Note: Some OSs will treat EFI_MEMORY_DESCRIPTOR.Attribute as really
+  //       set attributes and change memory paging attribute accordingly.
+  //       But current EFI_MEMORY_DESCRIPTOR.Attribute is assigned by
+  //       value from Capabilities in GCD memory map. This might cause
+  //       boot problems. Clearing all paging related capabilities can
+  //       workaround it. Following code is supposed to be removed once
+  //       the usage of EFI_MEMORY_DESCRIPTOR.Attribute is clarified in
+  //       UEFI spec and adopted by both EDK-II Core and all supported
+  //       OSs.
+  //
+  MemoryMapEnd = MemoryMap;
+  MemoryMap = MemoryMapStart;
+  while (MemoryMap < MemoryMapEnd) {
+    MemoryMap->Attribute &= ~(UINT64)(EFI_MEMORY_RP | EFI_MEMORY_RO |
+                                      EFI_MEMORY_XP);
+    MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
+  }
+
   Status = EFI_SUCCESS;
 
 Done:
-- 
2.16.3

From 0e383df1cb58429d82b158edb7ee9cece022975f Mon Sep 17 00:00:00 2001
From: Jian J Wang <jian.j.wang@intel.com>
Date: Thu, 23 Nov 2017 09:57:01 +0800
Subject: [PATCH 3/3] MdeModulePkg/Core: Merge memory map after filtering
 paging capability

Once the paging capabilities were filtered out, there might be some adjacent entries
sharing the same capabilities. It's recommended to merge those entries for the OS
compatibility purpose.

This patch makes use of existing method MergeMemoryMap() to do it. This is done by
simply turning this method from static to extern, and call it after filter code.

This patch is related to an issue described at
   https://bugzilla.tianocore.org/show_bug.cgi?id=753

This patch is also passed test of booting follow OSs:
    Windows 10
    Windows Server 2016
    Fedora 26
    Fedora 25

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from commit 646127c1929a606a1a5709d78f06f2dd5494b397)
---
 MdeModulePkg/Core/Dxe/DxeMain.h              | 18 ++++++++++++++++++
 MdeModulePkg/Core/Dxe/Mem/Page.c             |  2 ++
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c |  1 -
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 1a0babba713a..07b86ba696da 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy (
   IN  UINT64                Length
   );
 
+/**
+  Merge continous memory map entries whose have same attributes.
+
+  @param  MemoryMap       A pointer to the buffer in which firmware places
+                          the current memory map.
+  @param  MemoryMapSize   A pointer to the size, in bytes, of the
+                          MemoryMap buffer. On input, this is the size of
+                          the current memory map.  On output,
+                          it is the size of new memory map after merge.
+  @param  DescriptorSize  Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+**/
+VOID
+MergeMemoryMap (
+  IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
+  IN OUT UINTN                  *MemoryMapSize,
+  IN UINTN                      DescriptorSize
+  );
+
 #endif
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 5636d3bf421d..ff5c3108ccca 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1915,6 +1915,8 @@ CoreGetMemoryMap (
                                       EFI_MEMORY_XP);
     MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
   }
+  MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
+  MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMapStart + BufferSize);
 
   Status = EFI_SUCCESS;
 
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 6cf5edcbe5d9..75d9b14c1f87 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -182,7 +182,6 @@ SortMemoryMap (
                                  it is the size of new memory map after merge.
   @param  DescriptorSize         Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
 **/
-STATIC
 VOID
 MergeMemoryMap (
   IN OUT EFI_MEMORY_DESCRIPTOR  *MemoryMap,
-- 
2.16.3

Places

File ovmf-bsc1092943-fix-attributes-table.patch of Package ovmf (Revision 78fdba678982b7e987d8ff1fd27fa0a1)

Places