From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 662A120349DBB for ; Tue, 14 Nov 2017 22:48:33 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Nov 2017 22:52:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,398,1505804400"; d="scan'208";a="149641908" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 14 Nov 2017 22:52:41 -0800 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 14 Nov 2017 22:52:40 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 14 Nov 2017 22:52:40 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Wed, 15 Nov 2017 14:52:32 +0800 From: "Zeng, Star" To: "Wang, Jian J" , Laszlo Ersek CC: Matt Fleming , "edk2-devel@lists.01.org" , "Yao, Jiewen" , "Dong, Eric" , Ard Biesheuvel , "Zeng, Star" Thread-Topic: [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map Thread-Index: AQHTXVX5SdZFZz6mMUu4JO7MnG7CHaMU/00g Date: Wed, 15 Nov 2017 06:52:32 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9B5921@shsmsx102.ccr.corp.intel.com> References: <20171110010223.12696-1-jian.j.wang@intel.com> <9f53346f-c82c-c0ee-bca8-f53116227926@redhat.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZWJmYzQ2YzAtZTlhMC00MzJlLWI2ZmMtOTAzMDdlZjUyOWE4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJyZWxiclYyTThyQ3N4bjVwcVJBZE9GSTI2dW04WFwvNGV1Uk85MXFWOFNcL3dqeHlOMHlPNnFlelRqZHVJbk01RFoifQ== 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 v5] 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, 15 Nov 2017 06:48:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable How about the code to filter out paging capabilities from UEFI memory map i= n Page.c CoreGetMemoryMap(), then with maximum compatibility, the UEFI memo= ry map could be same with before 14dde9e903bb9a719ebb8f3381da72b19509bc36 [= MdeModulePkg/Core: Fix out-of-sync issue in GCD]. Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wang= , Jian J Sent: Tuesday, November 14, 2017 10:36 PM To: Laszlo Ersek Cc: Matt Fleming ; edk2-devel@lists.01.org; Yao, = Jiewen ; Dong, Eric ; Ard Bieshe= uvel Subject: Re: [edk2] [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of R= T_CODE in memory map Hi Laszlo, I did some investigation works on this issue. I think I found the root caus= e and tend to believe this is a Fedora kernel issue. Here're proves: memmap output patterns in which Fedora 26 failed to boot: 1) BS_Data 00000000AE200000-00000000AE20EFFF 000000000000000F 0000000000= 02600F RT_Data 00000000AE20F000-00000000AE38EFFF 0000000000000180 800000000= 002600F RT_Code 00000000AE38F000-00000000AE48EFFF 0000000000000100 800000000= 002600F Reserved 00000000AE48F000-00000000AE58EFFF 0000000000000100 000000000= 002600F 2) BS_Data 00000000AE200000-00000000AE20EFFF 000000000000000F 0000000000= 00000F RT_Data 00000000AE20F000-00000000AE38EFFF 0000000000000180 800000000= 000000F RT_Code 00000000AE38F000-00000000AE467FFF 00000000000000D9 800000000= 000000F RT_Code 00000000AE468000-00000000AE469FFF 0000000000000002 800000000= 002600F RT_Code 00000000AE46A000-00000000AE46EFFF 0000000000000005 800000000= 000000F RT_Code 00000000AE46F000-00000000AE470FFF 0000000000000002 800000000= 002600F RT_Code 00000000AE471000-00000000AE475FFF 0000000000000005 800000000= 000000F RT_Code 00000000AE476000-00000000AE479FFF 0000000000000004 800000000= 002600F RT_Code 00000000AE47A000-00000000AE47FFFF 0000000000000006 800000000= 000000F RT_Code 00000000AE480000-00000000AE481FFF 0000000000000002 800000000= 002600F RT_Code 00000000AE482000-00000000AE487FFF 0000000000000006 800000000= 000000F RT_Code 00000000AE488000-00000000AE489FFF 0000000000000002 800000000= 002600F RT_Code 00000000AE48A000-00000000AE48EFFF 0000000000000005 800000000= 000000F Reserved 00000000AE48F000-00000000AE58EFFF 0000000000000100 000000000= 000000F 3) RT_Data 00000000AE20F000-00000000AE38EFFF 0000000000000180 8000000000= 00000F RT_Code 00000000AE38F000-00000000AE418FFF 000000000000008A 800000000= 000000F RT_Code 00000000AE419000-00000000AE48EFFF 0000000000000076 800000000= 002600F Reserved 00000000AE48F000-00000000AE58EFFF 0000000000000100 000000000= 000000F 4) BS_Data 00000000AE200000-00000000AE20EFFF 000000000000000F 0000000000= 02400F RT_Data 00000000AE20F000-00000000AE38EFFF 0000000000000180 800000000= 002400F RT_Code 00000000AE38F000-00000000AE48EFFF 0000000000000100 800000000= 002400F Reserved 00000000AE48F000-00000000AE58EFFF 0000000000000100 000000000= 002400F memmap output pattern in which Fedora 26 booted successfully 5) BS_Data 00000000AE200000-00000000AE20EFFF 000000000000000F 0000000000= 00000F RT_Data 00000000AE20F000-00000000AE38EFFF 0000000000000180 800000000= 000000F RT_Code 00000000AE38F000-00000000AE48EFFF 0000000000000100 800000000= 000000F Reserved 00000000AE48F000-00000000AE58EFFF 0000000000000100 000000000= 000000F 6) BS_Data 00000000AE200000-00000000AE20EFFF 000000000000000F 0000000000= 00000F RT_Data 00000000AE20F000-00000000AE38EFFF 0000000000000180 800000000= 000000F RT_Code 00000000AE38F000-00000000AE418FFF 000000000000008A 800000000= 000000F RT_Code 00000000AE419000-00000000AE419FFF 0000000000000001 800000000= 000400F RT_Code 00000000AE41A000-00000000AE41BFFF 0000000000000002 800000000= 002000F RT_Code 00000000AE41C000-00000000AE420FFF 0000000000000005 800000000= 000400F RT_Code 00000000AE421000-00000000AE422FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE423000-00000000AE427FFF 0000000000000005 800000000= 000400F RT_Code 00000000AE428000-00000000AE42AFFF 0000000000000003 800000000= 002000F RT_Code 00000000AE42B000-00000000AE42FFFF 0000000000000005 800000000= 000400F RT_Code 00000000AE430000-00000000AE432FFF 0000000000000003 800000000= 002000F RT_Code 00000000AE433000-00000000AE439FFF 0000000000000007 800000000= 000400F RT_Code 00000000AE43A000-00000000AE43DFFF 0000000000000004 800000000= 002000F RT_Code 00000000AE43E000-00000000AE444FFF 0000000000000007 800000000= 000400F RT_Code 00000000AE445000-00000000AE448FFF 0000000000000004 800000000= 002000F RT_Code 00000000AE449000-00000000AE44EFFF 0000000000000006 800000000= 000400F RT_Code 00000000AE44F000-00000000AE450FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE451000-00000000AE455FFF 0000000000000005 800000000= 000400F RT_Code 00000000AE456000-00000000AE458FFF 0000000000000003 800000000= 002000F RT_Code 00000000AE459000-00000000AE45FFFF 0000000000000007 800000000= 000400F RT_Code 00000000AE460000-00000000AE461FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE462000-00000000AE467FFF 0000000000000006 800000000= 000400F RT_Code 00000000AE468000-00000000AE469FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE46A000-00000000AE46EFFF 0000000000000005 800000000= 000400F RT_Code 00000000AE46F000-00000000AE470FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE471000-00000000AE475FFF 0000000000000005 800000000= 000400F RT_Code 00000000AE476000-00000000AE479FFF 0000000000000004 800000000= 002000F RT_Code 00000000AE47A000-00000000AE47FFFF 0000000000000006 800000000= 000400F RT_Code 00000000AE480000-00000000AE481FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE482000-00000000AE487FFF 0000000000000006 800000000= 000400F RT_Code 00000000AE488000-00000000AE489FFF 0000000000000002 800000000= 002000F RT_Code 00000000AE48A000-00000000AE48EFFF 0000000000000005 800000000= 000400F Reserved 00000000AE48F000-00000000AE58EFFF 0000000000000100 000000000= 000000F You may notice that 4) will fail but 6) will succeed. Taking into account t= he fact that failure always happened in the runtime service api (set_variab= le), I think it must be that the kernel will mark the memory block to be RO= /XP/RP memory according to its capabilities but not its current attributes.= =20 This can explain why 4) will fail but 6) will succeed. Although memmap show= s all runtime driver image memory as RT_Code, but they're not all code segm= ent. Instead some of them are actually data segment. It's normal to mark code segment to be RO and data segment to be XP. But ma= rk data segment to be RO will cause runtime services failure. 4) shows all = RT_Code to be XXX24XXX, which means Fedora kernel will mark all code segmen= t and data segment to be RO and XP mistakenly, based on my previous hypothe= sis. This can also explain why 1), 2), 3) will fail the boot because XXX26X= XX will let Fedora kernel to mark RT_Code memory block to be not-present. I haven't got time to look into the Linux kernel source so I can't confirm = above analysis yet. I think you're more familiar with kernel source than us. Maybe you could he= lp to take a look. Thanks, Jian > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Friday, November 10, 2017 8:24 PM > To: Wang, Jian J > Cc: edk2-devel@lists.01.org; Dong, Eric ; Yao,=20 > Jiewen ; Ard Biesheuvel=20 > ; Matt Fleming > Subject: Re: [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of=20 > RT_CODE in memory map >=20 > Hi Jian, >=20 > I'm CC'ing Ard and Matt, and commenting at the bottom. >=20 > On 11/10/17 02:02, Jian J Wang wrote: > >> 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.=20 > >> Fix logic hole in updating attributes c. Instead of checking=20 > >> illegal memory space address and size, use return > >> status of gDS->SetMemorySpaceCapabilities() to skip memory block wh= ich > >> 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=20 > > much as possible. > > > > More detailed information, please refer to > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D753 > > > > Cc: Eric Dong > > Cc: Jiewen Yao > > Cc: Laszlo Ersek > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > --- > > UefiCpuPkg/CpuDxe/CpuPageTable.c | 69 > +++++++++++++++++++++++++++++----------- > > 1 file changed, 50 insertions(+), 19 deletions(-) > > > > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c > b/UefiCpuPkg/CpuDxe/CpuPageTable.c > > index d312eb66f8..61537838b7 100644 > > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c > > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c > > @@ -789,8 +789,7 @@ RefreshGcdMemoryAttributesFromPaging ( > > UINT64 BaseAddress; > > UINT64 PageStartAddress; > > UINT64 Attributes; > > - UINT64 Capabilities; > > - BOOLEAN DoUpdate; > > + UINT64 NewAttributes; > > UINTN Index; > > > > // > > @@ -802,9 +801,8 @@ RefreshGcdMemoryAttributesFromPaging ( > > > > GetCurrentPagingContext (&PagingContext); > > > > - DoUpdate =3D FALSE; > > - Capabilities =3D 0; > > Attributes =3D 0; > > + NewAttributes =3D 0; > > BaseAddress =3D 0; > > PageLength =3D 0; > > > > @@ -813,6 +811,34 @@ RefreshGcdMemoryAttributesFromPaging ( > > continue; > > } > > > > + // > > + // Sync the actual paging related capabilities back to GCD service= first. > > + // As a side effect (good one), this can also help to avoid unnece= ssary > > + // memory map entries due to the different capabilities of the sam= e type > > + // memory, such as multiple RT_CODE and RT_DATA entries in=20 > > + 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 | > > + EFI_MEMORY_PAGETYPE_MASK > > + ); > > + if (EFI_ERROR (Status)) { > > + // > > + // If we cannot udpate the capabilities, we cannot update its > > + // attributes either. So just simply skip current block of memor= y. > > + // > > + DEBUG (( > > + DEBUG_WARN, > > + "Failed to update capability: [%lu] %016lx - %016lx (%016lx=20 > > + - > > %016lx)\r\n", > > + (UINT64)Index, BaseAddress, BaseAddress + Length - 1, > > + MemorySpaceMap[Index].Capabilities, > > + MemorySpaceMap[Index].Capabilities | EFI_MEMORY_PAGETYPE_MASK > > + )); > > + continue; > > + } > > + > > if (MemorySpaceMap[Index].BaseAddress >=3D (BaseAddress + PageLeng= th)) { > > // > > // Current memory space starts at a new page. Resetting=20 > > PageLength will @@ -826,7 +852,9 @@ RefreshGcdMemoryAttributesFromPagin= g ( > > PageLength -=3D (MemorySpaceMap[Index].BaseAddress - BaseAddress= ); > > } > > > > - // 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 +870,26 @@=20 > > RefreshGcdMemoryAttributesFromPaging ( > > PageStartAddress =3D (*PageEntry) & > (UINT64)PageAttributeToMask(PageAttribute); > > PageLength =3D PageAttributeToLength (PageAttribute) - = (BaseAddress - > 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].Capabili= ties; > > - } else { > > - DoUpdate =3D FALSE; > > - } > > } > > > > Length =3D MIN (PageLength, MemorySpaceLength); > > - if (DoUpdate) { > > - gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabili= ties); > > - gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes= ); > > - DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %01= 6lx > - %016lx (%08lx -> %08lx)\r\n", > > - Index, BaseAddress, BaseAddress + Length = - 1, > > - MemorySpaceMap[Index].Attributes, Attribu= tes)); > > + 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=20 > > + (%016lx - > > %016lx)\r\n", > > + (UINT64)Index, BaseAddress, BaseAddress + Length - 1, > > + MemorySpaceMap[Index].Attributes, > > + NewAttributes > > + )); > > } > > > > PageLength -=3D Length; > > >=20 > So, I was ready to give my R-b for this patch, but then I also wanted=20 > to test it. I applied the patch on current edk2 master (7e2a8dfe8a9a, > "ArmPlatformPkg/PrePeiCore: seed temporary stack before entering PEI=20 > core", 2017-10-20), and built OVMF like this: >=20 > $ build \ > -a IA32 \ > -a X64 \ > -p OvmfPkg/OvmfPkgIa32X64.dsc \ > -t GCC48 \ > -b NOOPT \ > -D SMM_REQUIRE \ > -D SECURE_BOOT_ENABLE \ > -D E1000_ENABLE \ > -D HTTP_BOOT_ENABLE >=20 > For testing I used a recent-ish upstream QEMU development build=20 > (ae49fbbcd8e4, "Merge remote-tracking branch=20 > 'remotes/rth/tags/pull-tcg-20171025' into staging", 2017-10-25), with=20 > the Q35 machine type (which is required by SMM anyway). >=20 > The results vary across guest OSes: >=20 > (1) Up-to-date Fedora 26 guest crashes during boot, with the following=20 > call stack: >=20 > BUG: unable to handle kernel paging request at fffffffefe893018 Call=20 > Trace: > ? __change_page_attr_set_clr+0xaa6/0xd70 > ? kernel_map_pages_in_pgd+0xbc/0xd0 > ? efi_call+0x58/0x90 > ? virt_efi_set_variable.part.7+0x66/0x120 > ? virt_efi_set_variable+0x4f/0x60 > ? efi_delete_dummy_variable+0x62/0x90 > ? efi_enter_virtual_mode+0x4d4/0x4e8 > ? efi_enter_virtual_mode+0x4d4/0x4e8 > ? start_kernel+0x442/0x4e6 > ? early_idt_handler_array+0x120/0x120 > ? x86_64_start_reservations+0x24/0x26 > ? x86_64_start_kernel+0x13e/0x161 > ? secondary_startup_64+0x9f/0x9f >=20 > (2) The following Windows OSes all boot successfully: >=20 > - Windows 7 > - Windows Server 2008 R2 > - Windows 8.1 > - Windows Server 2012 R2 > - Windows 10 >=20 > (3) Windows Server 2016 crashes with a BSOD; reporting "ATTEMPTED=20 > WRITE TO READONLY MEMORY". >=20 > (Without the patch, all OSes boot OK.) >=20 >=20 > I'm attaching a ZIP file with the following contents (note that I'll=20 > attach the same file to TianoCore BZ#753 as well, because the mailing=20 > list archive(s) don't seem to preserve attachments): >=20 > - "ovmf.pre.txt", "shell.memmap.pre.txt", "kernel.pre.txt": OVMF log,=20 > MEMMAP command output in the UEFI shell, and Fedora 26 kernel boot log > (successful) *before* applying your patch. The kernel log is detailed=20 > (the cmdline had "ignore_loglevel" and "efi=3Ddebug"). >=20 > - "ovmf.post.txt", "shell.memmap.post.txt", "kernel.post.txt": same=20 > files as above, but saved *after* applying your patch. This is when=20 > the > F26 kernel crashes. >=20 > - "win2016.post.png": screenshot of the Windows Server 2016 boot=20 > failure (after the patch was applied). >=20 > Thanks, > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel