From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7254D220757F5 for ; Tue, 21 Nov 2017 23:50:04 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Nov 2017 23:54:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,436,1505804400"; d="scan'208";a="152049076" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga004.jf.intel.com with ESMTP; 21 Nov 2017 23:54:19 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 21 Nov 2017 23:54:18 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Wed, 22 Nov 2017 15:54:14 +0800 From: "Zeng, Star" To: "Wang, Jian J" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , "Yao, Jiewen" , Laszlo Ersek , Ard Biesheuvel , "Zeng, Star" Thread-Topic: [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map Thread-Index: AQHTXqxT5z7tY8QHjkmTB9nbs9kYYaMgEL8w Date: Wed, 22 Nov 2017 07:54:14 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9BA573@shsmsx102.ccr.corp.intel.com> References: <20171116072700.11456-1-jian.j.wang@intel.com> <20171116072700.11456-3-jian.j.wang@intel.com> In-Reply-To: <20171116072700.11456-3-jian.j.wang@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjA4MDY3NTItNDJiMC00YThiLTk3ZDEtYmUzOWNjNDNhMzUxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJPT1FtclRqSFwvNE1RRWZ0NU15a0tsXC9TMkJKMDdMNjNzekYyTnc4UUo1VHVGK3JFSGQ3U21BN3l5VVR5Wk1xTU4ifQ== dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Nov 2017 07:50:04 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Star Zeng -----Original Message----- From: Wang, Jian J=20 Sent: Thursday, November 16, 2017 3:27 PM To: edk2-devel@lists.01.org Cc: Dong, Eric ; Yao, Jiewen ; Z= eng, Star ; Laszlo Ersek ; Ard Bies= heuvel Subject: [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in = memory map > 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=20 > variable to hold new attributes to make code cleaner > v3: > a. Add comment to explain more on updating memory capabilities b. Fix=20 > logic hole in updating attributes c. Instead of checking illegal=20 > 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 sm= aller > 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 po= ssible. More detailed information, please refer to https://bugzilla.tianocore.org/show_bug.cgi?id=3D753 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 --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 94 +++++++++++++++++++++++++++++++-----= ---- 1 file changed, 73 insertions(+), 21 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTa= ble.c index d312eb66f8..3297c1900b 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -769,6 +769,20 @@ AssignMemoryPageAttributes ( return Status; } =20 +/** + Check if Execute Disable feature is enabled or not. +**/ +BOOLEAN +IsExecuteDisableEnabled ( + VOID + ) +{ + MSR_CORE_IA32_EFER_REGISTER MsrEfer; + + MsrEfer.Uint64 =3D AsmReadMsr64 (MSR_IA32_EFER); + return (MsrEfer.Bits.NXE =3D=3D 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; =20 // @@ -802,17 +816,50 @@ RefreshGcdMemoryAttributesFromPaging ( =20 GetCurrentPagingContext (&PagingContext); =20 - DoUpdate =3D FALSE; - Capabilities =3D 0; - Attributes =3D 0; - BaseAddress =3D 0; - PageLength =3D 0; + Attributes =3D 0; + NewAttributes =3D 0; + BaseAddress =3D 0; + PageLength =3D 0; + + if (IsExecuteDisableEnabled ()) { + Capabilities =3D EFI_MEMORY_RO | EFI_MEMORY_RP | EFI_MEMORY_XP; }=20 + else { + Capabilities =3D EFI_MEMORY_RO | EFI_MEMORY_RP; } =20 for (Index =3D 0; Index < NumberOfDescriptors; Index++) { if (MemorySpaceMap[Index].GcdMemoryType =3D=3D EfiGcdMemoryTypeNonExis= tent) { continue; } =20 + // + // Sync the actual paging related capabilities back to GCD service fir= st. + // As a side effect (good one), this can also help to avoid unnecessar= y + // memory map entries due to the different capabilities of the same ty= pe + // 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 =3D 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 -> %01= 6lx)\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 >=3D (BaseAddress + PageLength))= { // // Current memory space starts at a new page. Resetting PageLength w= ill @@ -826,7 +873,9 @@ RefreshGcdMemoryAttributesFromPaging ( PageLength -=3D (MemorySpaceMap[Index].BaseAddress - BaseAddress); } =20 - // Sync real page attributes to GCD + // + // Sync actual page attributes to GCD + // BaseAddress =3D MemorySpaceMap[Index].BaseAddress; MemorySpaceLength =3D MemorySpaceMap[Index].Length; while (MemorySpaceLength > 0) { @@ -842,23 +891,26 @@ RefreshGcdMemoryAttributesFromPaging ( PageStartAddress =3D (*PageEntry) & (UINT64)PageAttributeToMask(P= ageAttribute); PageLength =3D PageAttributeToLength (PageAttribute) - (Bas= eAddress - PageStartAddress); Attributes =3D GetAttributesFromPageEntry (PageEntry); - - if (Attributes !=3D (MemorySpaceMap[Index].Attributes & EFI_MEMORY= _PAGETYPE_MASK)) { - DoUpdate =3D TRUE; - Attributes |=3D (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_= PAGETYPE_MASK); - Capabilities =3D Attributes | MemorySpaceMap[Index].Capabilities= ; - } else { - DoUpdate =3D FALSE; - } } =20 Length =3D 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 !=3D (MemorySpaceMap[Index].Attributes & + EFI_MEMORY_PAGETYPE_MASK)) { + NewAttributes =3D (MemorySpaceMap[Index].Attributes & + ~EFI_MEMORY_PAGETYPE_MASK) | Attributes; + Status =3D 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 + )); } =20 PageLength -=3D Length; -- 2.14.1.windows.1