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=jiewen.yao@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 BBE6C21129455 for ; Tue, 11 Sep 2018 18:03:54 -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 orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Sep 2018 18:03:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,362,1531810800"; d="scan'208";a="87961069" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga004.fm.intel.com with ESMTP; 11 Sep 2018 18:03:37 -0700 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 11 Sep 2018 18:03:37 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 11 Sep 2018 18:03:36 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.226]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.205]) with mapi id 14.03.0319.002; Wed, 12 Sep 2018 09:03:34 +0800 From: "Yao, Jiewen" To: "Gao, Liming" , Laszlo Ersek , "edk2-devel@lists.01.org" CC: "Dong, Eric" Thread-Topic: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler Thread-Index: AQHUSN8xD8V1gRuKEEm+8A26kUclC6Tqqh2AgAD6PwD//6trgIAAhu7w Date: Wed, 12 Sep 2018 01:03:33 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503AD52D8F@shsmsx102.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> In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E2F5D28@SHSMSX104.ccr.corp.intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMmQyOTAyZDUtZjYxNy00Y2U3LWEzMzgtYTg3NjM2YjU4ZjgzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiVUdTb3FaOWtlcVdsYUhteGhxcWpPQWVjWlBBRDJRdHpEeDZURGV6OFJ6c1A1MEg5RGFKZUROd1hadjEycElSaSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action 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:03:54 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 >=20 > Jiewen: > After do more verification, I recall this change. Current code is reall= y > 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 func= tion > CpuSmmDebugEntry(). nasm compiler generates the function call with the > relative address. After _SmiEntryPoint() function is copied and run in th= e new > address, its external function call will not work. To fix it, I add jmp i= nstruction > to the original address, then process function all and works. >=20 > mov rax, strict qword 0 ; mov rax, _SmiHandler > _SmiHandlerAbsAddr: > jmp rax >=20 > ... > mov rcx, rbx > call ASM_PFX(CpuSmmDebugEntry) >=20 > 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