From: Jian J Wang Date: Mon, 23 Oct 2017 06:49:02 +0000 (+0800) Subject: UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map X-Git-Tag: edk2-stable201903~3020 X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=768bd967847e91aa236c1eed3507e0863547f544 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 Cc: Jiewen Yao Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Tested-by: Laszlo Ersek Reviewed-by: Star Zeng Reviewed-by: Laszlo Ersek --- diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 76f44f9bd1..9658ed74c5 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;