From: Jian J Wang <jian.j.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Eric Dong <eric.dong@intel.com>,
Jiewen Yao <jiewen.yao@intel.com>,
Star Zeng <star.zeng@intel.com>, Laszlo Ersek <lersek@redhat.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH v7 2/2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map
Date: Tue, 21 Nov 2017 14:17:25 +0800 [thread overview]
Message-ID: <20171121061725.11028-3-jian.j.wang@intel.com> (raw)
In-Reply-To: <20171121061725.11028-1-jian.j.wang@intel.com>
> v7:
> No change.
> v6:
> Add ExecuteDisable feature check to include/exclude EFI_MEMORY_XP
> v5:
> Coding style clean-up
> v4:
> a. Remove DoUpdate and check attributes mismatch all the time to avoid
> a logic hole
> b. Add warning message if failed to update capability
> c. Add local variable to hold new attributes to make code cleaner
> v3:
> a. Add comment to explain more on updating memory capabilities
> b. Fix logic hole in updating attributes
> c. Instead of checking illegal memory space address and size, use return
> status of gDS->SetMemorySpaceCapabilities() to skip memory block which
> cannot be updated with new capabilities.
> v2
> a. Fix an issue which will cause setting capability failure if size is smaller
> than a page.
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
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: Laszlo Ersek <lersek@redhat.com>
---
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 309f448a83..db41fa401f 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.14.1.windows.1
next prev parent reply other threads:[~2017-11-21 6:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-21 6:17 [PATCH v7 0/2] Fix multiple entries of RT_CODE in memory map Jian J Wang
2017-11-21 6:17 ` [PATCH v7 1/2] MdeModulePkg/DxeCore: Filter out all paging capabilities Jian J Wang
2017-11-21 7:31 ` Zeng, Star
2017-11-21 6:17 ` Jian J Wang [this message]
2017-11-21 13:38 ` [PATCH v7 0/2] Fix multiple entries of RT_CODE in memory map Laszlo Ersek
2017-11-22 7:56 ` Zeng, Star
2017-11-22 7:57 ` Yao, Jiewen
2017-11-22 9:05 ` Laszlo Ersek
2017-11-22 9:09 ` Wang, Jian J
2017-11-22 9:07 ` Wang, Jian J
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=20171121061725.11028-3-jian.j.wang@intel.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