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.65; helo=mga03.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (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 71D9021128700 for ; Tue, 11 Sep 2018 17:59:14 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Sep 2018 17:59:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,362,1531810800"; d="scan'208";a="87960008" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga004.fm.intel.com with ESMTP; 11 Sep 2018 17:59:13 -0700 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 11 Sep 2018 17:59:13 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 11 Sep 2018 17:59:13 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.143]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.16]) with mapi id 14.03.0319.002; Wed, 12 Sep 2018 08:59:12 +0800 From: "Gao, Liming" To: "Yao, Jiewen" , Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Dong, Eric" Thread-Topic: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler Thread-Index: AQHUSN83WpVD15/hAkKOT1YAlAEWHqTqqh2AgAB0V4CAALFEgA== Date: Wed, 12 Sep 2018 00:59:11 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E2F5D28@SHSMSX104.ccr.corp.intel.com> References: <1536567625-9540-1-git-send-email-liming.gao@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AD51D73@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AD51D73@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 00:59:14 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jiewen: After do more verification, I recall this change. Current code is really = 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 me= mory location and run. But, _ SmiEntryPoint() calls the external C function= CpuSmmDebugEntry(). nasm compiler generates the function call with the rel= ative address. After _SmiEntryPoint() function is copied and run in the new= address, its external function call will not work. To fix it, I add jmp in= struction to the original address, then process function all and works.=20 =20 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