From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (NAM02-SN1-obe.outbound.protection.outlook.com [40.107.77.80]) by mx.groups.io with SMTP id smtpd.web10.1581.1588775705471450210 for ; Wed, 06 May 2020 07:35:05 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=D1T7Pc+Q; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.77.80, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NHh3phkmb45RdLBJ9IbE7ObF3uSpxVtfsConjmtW7ASsU0yZ3sYVQIbM6Mhemy3zBVGiZOsdSDH3N4qo8MY7XXhY9H6jJvJ11YvcM7Y6LZrSQh+GXLhJ/N+heb2MeDkaWT/JKIHwuK6swvFYLWFOIS2m28hPo1+xfIwSPxNlxmvbNWlYaHOhwDiW33OJSE0R0E/w9ctntjMchzwgjoDt9yVFZM4qFA9yedxZO5UbEU/Ke1rVxMsqmUFV8pxwDdN3wBpMeykdb4cQJp3hcROXePDianLK/rrFqCBOigyvAqNoz3BAHJh+l/NGreVSEJM5+pbv1GzVcJYuuBm+RW7uVw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=L+D7eq+JUIXIUg6mHOF7zMpuM19HKkCVFHvmdzUDtHY=; b=c9xVD5jm8Dl1xGBq5UDqQQEICBm9Z/jnjhTEbePp+PuwpaKhdq0egJKOKjeDm6GRIOe+ppZwvwuC+5LED6b/yvbVM4TomGaJ5o0V3D4gjn6LBqArs59Ogydhwn3kVT1VdqqsMaPkSOzAE1U9Qko1geYbaL7sbtMbesdXC6hZc7qh5jp+E97InidZdx62S1OyobXj+kNu0X13QAcIhSAS7DEtO3GPW+RcT0V4I4FekjTsLcSyBTDkJPLWcN+vgYJIx1klR3Ua3Prhi9MpZEzNh/+GOzIq0AJjrmWVtJ/QcDVrnOwIWMTWkfwL0DjtLwh804dUSXBKNuy0rXDuuf9xog== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=L+D7eq+JUIXIUg6mHOF7zMpuM19HKkCVFHvmdzUDtHY=; b=D1T7Pc+QutPADLSMiFUt0k5lF44w6qEtJyDLsrvhHUhypVS24Y1gnJn2POU474MUl0CRvZjaFAFfoXSY5Yxvr1pF6X8e11wxZHuGxuCE6367ZU9u2IrwdxJl0tSZlpmt/j8G6G5tbBI8dmsQ4Jxe3+I6cfX1kqrlfZZdEkpsNJQ= Authentication-Results: apple.com; dkim=none (message not signed) header.d=none;apple.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM5PR12MB2374.namprd12.prod.outlook.com (2603:10b6:4:b4::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2979.26; Wed, 6 May 2020 14:35:04 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4ce1:9947:9681:c8b1]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4ce1:9947:9681:c8b1%10]) with mapi id 15.20.2958.030; Wed, 6 May 2020 14:35:03 +0000 Subject: Re: [edk2-devel] [PATCH 4/4] UefiCpuPkg/CpuExceptionHandler: Revert binary patching in standard CpuExceptionHandlerLib To: devel@edk2.groups.io, lersek@redhat.com Cc: Jordan Justen , Ard Biesheuvel , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh , Anthony Perard , Benjamin You , Guo Dong , Julien Grall , Maurice Ma , Andrew Fish References: From: "Lendacky, Thomas" Message-ID: <26e51d24-fe19-4fa1-6bac-6936041af39d@amd.com> Date: Wed, 6 May 2020 09:35:01 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 In-Reply-To: X-ClientProxiedBy: SN4PR0201CA0038.namprd02.prod.outlook.com (2603:10b6:803:2e::24) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN4PR0201CA0038.namprd02.prod.outlook.com (2603:10b6:803:2e::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2958.30 via Frontend Transport; Wed, 6 May 2020 14:35:02 +0000 X-Originating-IP: [67.79.209.213] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: d73c0995-1310-44ae-1d83-08d7f1caaa0f X-MS-TrafficTypeDiagnostic: DM5PR12MB2374:|DM5PR12MB2374: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-Forefront-PRVS: 03950F25EC X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: yqgHjQO9woJ0pVmv8ZGyMJZ4jmsxgr1awXODjCRYrFLRDzA4Aubl/lvoMSk204P1nhQbYIrmYD0n+SeFIwrI0780aPSUIubsphVo0/XOVkQhlUHu6vUQq4I0c1R8PntYyx6O+6qM90aUIYL8E88Dm2HDuVJjooRYIZ2XPQLekD4eNVM3upxQ29g+Nc6V8bgERN2w+dX9YXAHTRXK7OmpNhw9p7VBF1xIfOneRV6GP0dKCSOI/QxcO6yLK+cXDplhQkF5Jt7ZLxDjcCqbaPqDXajuyLr+1yMlAWsIOL7sN7XtpOBRYcq2kmrK12yy5mdPxd5sBhglpdjUjT4UQItXVKAS3wXmhmHW+b2g0eF7KhLhm4Qj32f6zqTsKyBg1c6SyluCvrnWf9a1gqNfEFlc/XAmGAEH9fARRbT3waDGkkwYaXlpA+KO7u1Wrhm4UNRSXTbQHuRA+xdl5106oZst3WAfEqIQVKL2t5pJz7bUi5EMDjBQUbZl/iLmVhR5eGu2+8k7yKPioBhWYPY6vr3IanniwgJRDS7kZqL0Vw03fQJt05cd/GF+RH9/iphRDKdPrldtHoZXXnFkBh7O7nZ6WuT2YaOJuJUL1/6DtqUgJIIYRmbLtxMKwH/8hOitAPka X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(376002)(396003)(366004)(39860400002)(346002)(136003)(33430700001)(186003)(8936002)(19627235002)(316002)(16526019)(6506007)(966005)(5660300002)(31696002)(8676002)(6512007)(478600001)(2906002)(16799955002)(53546011)(54906003)(26005)(6486002)(4326008)(52116002)(45080400002)(33440700001)(66946007)(86362001)(31686004)(66556008)(66476007)(2616005)(956004)(36756003)(7416002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: wo8r1Hq3antrMaAJHgJm9ea4hflwTNWnudK5E+wemCoCpLh11z5QWNdjnsnfE84dVWvmDmFRaL3fGzMwwFrTtSvHQdj3H6N20i89yhmtB/Wrig/S/IA439SIG4V3oJFUFc11Bj1kUDGAP3d8bddw8fIc75l+/XZBZLUoghfQoPYVtOs1GE6U0tenBkl6ugMVv83zTWNWGOA9Q+AzSrG1ukzgPdGL76HzbEqACIFOQ2hSppB/mRUwecMovB6jqEtIoFvWoi2sjYdIY9jMoLnPHh4/tuIDLMh0hNKZmPjrL5ZOsYZ8UVuPwHzsa1n6qXWpdikq/ZgFlblqi92xcPdq504DghDOL5xYF4A47HqObZ8v1h1nBttSc07+Pk/oOPy5s73Zch7EYGOT3vknctK9ieyweVruWteGEn6ojlTE2wCcbjmnU0lzg4JFCf4ABqA87gpuGooCS8mYNKY7fr1rlgt+a6ZNL/H9FLu/BT5Z+hbJLqlNMvS59eYP2MWY253/w5VqtmTUgtos6eymubMsK0s6TkxSnbUwFnifYM9Cz9kBhviIS3ehFwlVNDhe0uEvZm2PuRMGzpKduyv3lMeEQuGktYtRx73ZgDZ3CsEgyiyL6kzxfl7xnNoLVXyLHGq8NcYgFECKzCoF65NJ4Sr/dww3UBOcb+wbaR/gH0gPRnxFKCm0/9+XKvOPPaosDu3u3UBW5Q2OI91Kcb+JCsIWliT2O5n0sAGG2glOWX1KXbQcsfDWPw2BnypNpNMHyg67tdw55p5uZe/V7LzaXhFJ62rKFdW0oLav7QrGVItsfRI= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: d73c0995-1310-44ae-1d83-08d7f1caaa0f X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2020 14:35:03.8937 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: e022Ib7M83sz+wVSGSqPvHiQ1H5pQJMz7J99RdqkNCfW06x7KhSnfG/dWCJfHJrlGbwAUjCbgeR1t/Bx9K19Vg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB2374 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 5/5/20 5:15 PM, Laszlo Ersek via groups.io wrote: > On 05/01/20 22:17, Lendacky, Thomas wrote: >> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2340&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cd2ec699d2c644a55724008d7f141d96f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243137431443098&sdata=oTTju7144KZc8VCmQqu74UilIOzQyji9jlO%2BMJeZYyU%3D&reserved=0 >> >> Now that an XCODE5 specific CpuExceptionHandlerLib library is in place, >> revert the changes made to the ExceptionHandlerAsm.nasm in commit >> 2db0ccc2d7fe ("UefiCpuPkg: Update CpuExceptionHandlerLib pass XCODE5 tool >> chain") so that binary patching of flash code is not performed. >> >> Cc: Eric Dong >> Cc: Ray Ni >> Cc: Laszlo Ersek >> Cc: Liming Gao >> Signed-off-by: Tom Lendacky >> --- >> .../X64/ExceptionHandlerAsm.nasm | 25 +++++-------------- >> 1 file changed, 6 insertions(+), 19 deletions(-) >> >> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm >> index 19198f273137..3814f9de3703 100644 >> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm >> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm >> @@ -34,7 +34,7 @@ AsmIdtVectorBegin: >> db 0x6a ; push #VectorNum >> db ($ - AsmIdtVectorBegin) / ((AsmIdtVectorEnd - AsmIdtVectorBegin) / 32) ; VectorNum >> push rax >> - mov rax, strict qword 0 ; mov rax, ASM_PFX(CommonInterruptEntry) >> + mov rax, ASM_PFX(CommonInterruptEntry) >> jmp rax >> %endrep >> AsmIdtVectorEnd: >> @@ -44,8 +44,7 @@ HookAfterStubHeaderBegin: >> @VectorNum: >> db 0 ; 0 will be fixed >> push rax >> - mov rax, strict qword 0 ; mov rax, HookAfterStubHeaderEnd >> -JmpAbsoluteAddress: >> + mov rax, HookAfterStubHeaderEnd >> jmp rax >> HookAfterStubHeaderEnd: >> mov rax, rsp >> @@ -257,7 +256,8 @@ HasErrorCode: >> ; and make sure RSP is 16-byte aligned >> ; >> sub rsp, 4 * 8 + 8 >> - call ASM_PFX(CommonExceptionHandler) >> + mov rax, ASM_PFX(CommonExceptionHandler) >> + call rax >> add rsp, 4 * 8 + 8 >> >> cli >> @@ -365,24 +365,11 @@ DoIret: >> ; comments here for definition of address map >> global ASM_PFX(AsmGetTemplateAddressMap) >> ASM_PFX(AsmGetTemplateAddressMap): >> - lea rax, [AsmIdtVectorBegin] >> + mov rax, AsmIdtVectorBegin >> mov qword [rcx], rax >> mov qword [rcx + 0x8], (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32 >> - lea rax, [HookAfterStubHeaderBegin] >> + mov rax, HookAfterStubHeaderBegin >> mov qword [rcx + 0x10], rax >> - >> -; Fix up CommonInterruptEntry address >> - lea rax, [ASM_PFX(CommonInterruptEntry)] >> - lea rcx, [AsmIdtVectorBegin] >> -%rep 32 >> - mov qword [rcx + (JmpAbsoluteAddress - 8 - HookAfterStubHeaderBegin)], rax >> - add rcx, (AsmIdtVectorEnd - AsmIdtVectorBegin) / 32 >> -%endrep >> -; Fix up HookAfterStubHeaderEnd >> - lea rax, [HookAfterStubHeaderEnd] >> - lea rcx, [JmpAbsoluteAddress] >> - mov qword [rcx - 8], rax >> - >> ret >> >> ;------------------------------------------------------------------------------------- >> > > With this patch applied, the differences with the "original" remain: > > $ git diff 2db0ccc2d7fe^..HEAD -- \ > UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm > >> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm >> index ba8993d84b0b..3814f9de3703 100644 >> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm >> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm >> @@ -1,12 +1,6 @@ >> ;------------------------------------------------------------------------------ ; >> -; Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.
>> -; This program and the accompanying materials >> -; are licensed and made available under the terms and conditions of the BSD License >> -; which accompanies this distribution. The full text of the license may be found at >> -; https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopensource.org%2Flicenses%2Fbsd-license.php&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cd2ec699d2c644a55724008d7f141d96f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637243137431443098&sdata=SZAc83Y%2BwZauGcj47EDgc10fnxSucy2ljeI9PcaJSvE%3D&reserved=0. >> -; >> -; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> -; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> +; Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
>> +; SPDX-License-Identifier: BSD-2-Clause-Patent >> ; >> ; Module Name: >> ; > > This is expected. > >> @@ -189,17 +183,19 @@ HasErrorCode: >> push rax >> push rax >> sidt [rsp] >> - xchg rax, [rsp + 2] >> - xchg rax, [rsp] >> - xchg rax, [rsp + 8] >> + mov bx, word [rsp] >> + mov rax, qword [rsp + 2] >> + mov qword [rsp], rax >> + mov word [rsp + 8], bx >> >> xor rax, rax >> push rax >> push rax >> sgdt [rsp] >> - xchg rax, [rsp + 2] >> - xchg rax, [rsp] >> - xchg rax, [rsp + 8] >> + mov bx, word [rsp] >> + mov rax, qword [rsp + 2] >> + mov qword [rsp], rax >> + mov word [rsp + 8], bx >> >> ;; UINT64 Ldtr, Tr; >> xor rax, rax >> > > Also expected, from commit f4c898f2b2db > ("UefiCpuPkg/CpuExceptionHandlerLib: Fix split lock", 2019-09-20). > > Therefore, for this patch: > > Reviewed-by: Laszlo Ersek > > *However*, this revert must be restricted to the original > "SecPeiCpuExceptionHandlerLib.inf" instance, i.e. where binary patching > is not acceptable. (Otherwise, in combination with my request (1) under > patch#1, we'd needlessly break the PEI / DXE / SMM lib instances under > XCODE5.) > > (1) Therefore, please insert a new patch between patches #1 and #2, such > that the new patch flip > > - PeiCpuExceptionHandlerLib.inf > - DxeCpuExceptionHandlerLib.inf > - SmmCpuExceptionHandlerLib.inf > > to using "Xcode5ExceptionHandlerAsm.nasm". > > (If you wish, you can squash these modifications into the updated > patch#1, rather than inserting them as a separate patch between #1 and > #2.) > > > In summary, I suggest the following end-state: > > - we should have a self-patching NASM file, and one without > self-patching, > > - the self-patching variant should be called > "Xcode5ExceptionHandlerAsm.nasm" (because the *only* reason for the > self-patching is xcode5), > > - we should have 5 INF files in total, > > - "PeiCpuExceptionHandlerLib.inf", "DxeCpuExceptionHandlerLib.inf", > "SmmCpuExceptionHandlerLib.inf" should use > "Xcode5ExceptionHandlerAsm.nasm" (the self-patching is harmless), > > - "SecPeiCpuExceptionHandlerLib.inf" should use > "ExceptionHandlerAsm.nasm" (self-patching is invalid, so don't do it), > > - "Xcode5SecPeiCpuExceptionHandlerLib.inf" should use > "Xcode5ExceptionHandlerAsm.nasm" (the self-patching is invalid, but we > can't avoid it when building with XCODE5), > > - platforms should resolve the CpuExceptionHandlerLib class to > "Xcode5SecPeiCpuExceptionHandlerLib.inf" only for the XCODE5 toolchain > *and* for the SEC phase. Ok, I have v2 ready to go, but when I ran it through the integration tests using a pull request I received some errors (see https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=6516&view=results). The error is the same in all cases and the error message is: CRITICAL - UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf not in UefiCpuPkg/UefiCpuPkg.dsc Any idea about the reason for this message? Is it due to the [Components] section of the UefiCpuPkg/UefiCpuPkg.dsc file? Should that section not use the !if check and just list both .inf files (SecPeiCpuExceptionHandlerLib.inf and Xcode5SecPeiCpuExceptionHandlerLib.inf)? Thanks, Tom > > Thanks, > Laszlo > > > >