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=liming.gao@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 A67ED21129455 for ; Tue, 11 Sep 2018 18:30:43 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Sep 2018 18:30:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,362,1531810800"; d="scan'208";a="79740240" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by FMSMGA003.fm.intel.com with ESMTP; 11 Sep 2018 18:29:56 -0700 Received: from fmsmsx121.amr.corp.intel.com (10.18.125.36) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 11 Sep 2018 18:29:56 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx121.amr.corp.intel.com (10.18.125.36) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 11 Sep 2018 18:29:56 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.143]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.205]) with mapi id 14.03.0319.002; Wed, 12 Sep 2018 09:29:53 +0800 From: "Gao, Liming" To: "Yao, Jiewen" , Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Dong, Eric" , "Gao, Liming" Thread-Topic: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler Thread-Index: AQHUSN83WpVD15/hAkKOT1YAlAEWHqTqqh2AgAB0V4CAALFEgP//gUeAgACHdQA= Date: Wed, 12 Sep 2018 01:29:52 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E2F5F38@SHSMSX104.ccr.corp.intel.com> References: <1536567625-9540-1-git-send-email-liming.gao@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AD51D73@shsmsx102.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E2F5D28@SHSMSX104.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AD52D8F@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AD52D8F@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Sep 2018 01:30:43 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jiewen: Because nasm doesn't generate the absolute address, we can change the bel= ow code, but we need to fix up its value at boot time like current _SmiHand= ler way.=20 Could you let me know why can't use current way?=20 Thanks Liming >-----Original Message----- >From: Yao, Jiewen >Sent: Wednesday, September 12, 2018 9:04 AM >To: Gao, Liming ; Laszlo Ersek ; >edk2-devel@lists.01.org >Cc: Dong, Eric >Subject: RE: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove >unnecessary jmp _SmiHandler > >The original code is below. Can we rollback to the indirect call? > > mov rax, ASM_PFX(CpuSmmDebugEntry) > call rax > >Thank you >Yao Jiewen > >> -----Original Message----- >> From: Gao, Liming >> Sent: Wednesday, September 12, 2018 8:59 AM >> To: Yao, Jiewen ; Laszlo Ersek >; >> edk2-devel@lists.01.org >> Cc: Dong, Eric >> Subject: RE: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove >> unnecessary jmp _SmiHandler >> >> Jiewen: >> After do more verification, I recall this change. Current code is real= ly >> required. Without it, OVMF SMM can't boot. So, below code can't be >> removed. >> The reason is that nasm _SmiEntryPoint() function is copied to another >> memory location and run. But, _ SmiEntryPoint() calls the external C >function >> CpuSmmDebugEntry(). nasm compiler generates the function call with the >> relative address. After _SmiEntryPoint() function is copied and run in t= he >new >> address, its external function call will not work. To fix it, I add jmp = instruction >> to the original address, then process function all and works. >> >> mov rax, strict qword 0 ; mov rax, _SmiHandler >> _SmiHandlerAbsAddr: >> jmp rax >> >> ... >> mov rcx, rbx >> call ASM_PFX(CpuSmmDebugEntry) >> >> Thanks >> Liming >> >-----Original Message----- >> >From: Yao, Jiewen >> >Sent: Wednesday, September 12, 2018 6:03 AM >> >To: Laszlo Ersek ; Gao, Liming >> ; >> >edk2-devel@lists.01.org >> >Cc: Dong, Eric >> >Subject: RE: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove >> >unnecessary jmp _SmiHandler >> > >> >HI >> >Would you please add info on what unit test has been done? >> > >> >Thank you >> >Yao Jiewen >> > >> > >> >> -----Original Message----- >> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf >Of >> >> Laszlo Ersek >> >> Sent: Tuesday, September 11, 2018 11:06 PM >> >> To: Gao, Liming ; edk2-devel@lists.01.org >> >> Cc: Yao, Jiewen ; Dong, Eric >> >> >> Subject: Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove >> >> unnecessary jmp _SmiHandler >> >> >> >> On 09/10/18 10:20, Liming Gao wrote: >> >> > This change is wrong introduced by >> >> e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 >> >> > It is not required. So, revert it. >> >> > >> >> > Contributed-under: TianoCore Contribution Agreement 1.1 >> >> > Signed-off-by: Liming Gao >> >> > Cc: Jiewen Yao >> >> > --- >> >> > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 9 ++------- >> >> > 1 file changed, 2 insertions(+), 7 deletions(-) >> >> > >> >> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> >> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> >> > index 315d0f8..7b1b3ca 100644 >> >> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> >> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> >> > @@ -173,9 +173,8 @@ SmiHandlerIdtrAbsAddr: >> >> > mov gs, eax >> >> > mov ax, [rbx + DSC_SS] >> >> > mov ss, eax >> >> > - mov rax, strict qword 0 ; mov rax, >> >> _SmiHandler >> >> > -_SmiHandlerAbsAddr: >> >> > - jmp rax >> >> > + >> >> > +; jmp _SmiHandler ; instruction is not >> needed >> >> > >> >> > _SmiHandler: >> >> > mov rbx, [rsp + 0x8] ; rcx <- CpuIndex >> >> > @@ -229,8 +228,4 @@ ASM_PFX(PiSmmCpuSmiEntryFixupAddress): >> >> > lea rax, [ASM_PFX(gSmiHandlerIdtr)] >> >> > lea rcx, [SmiHandlerIdtrAbsAddr] >> >> > mov qword [rcx - 8], rax >> >> > - >> >> > - lea rax, [_SmiHandler] >> >> > - lea rcx, [_SmiHandlerAbsAddr] >> >> > - mov qword [rcx - 8], rax >> >> > ret >> >> > >> >> >> >> Please remember to CC package maintainers / reviewers directly. >> >> >> >> The patch makes sense to me, and indeed it restores the original code= . >> >> >> >> Reviewed-by: Laszlo Ersek >> >> >> >> Can you perhaps add another sentence to the commit message, before >> you >> >> push the patch, such as "the original code already uses RIP-relative >> >> addressing"? >> >> >> >> Thanks >> >> Laszlo >> >> _______________________________________________ >> >> edk2-devel mailing list >> >> edk2-devel@lists.01.org >> >> https://lists.01.org/mailman/listinfo/edk2-devel