Re: [edk2] [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
Laszlo: Thanks for your comments. I have corrected the patch and pushed it into edk2. Thanks Liming >-Original Message- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Friday, September 21, 2018 6:49 PM >To: Gao, Liming ; edk2-devel@lists.01.org >Cc: Dong, Eric ; Yao, Jiewen >Subject: Re: [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry >function run the same position > >Hi Liming, > >On 09/21/18 02:56, Liming Gao wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191 >> >> Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp >_SmiHandler >> is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved >into rax, >> then call it. But, this code doesn't work in XCODE5 tool chain. Because >XCODE5 >> doesn't generated the absolute address in the EFI image. So, rax stores the >> relative address. Once this logic is moved to another place, it will not >> work. >> ; jmp _SmiHandler ; instruction is not needed >> ... >> mov rax, ASM_PFX(CpuSmmDebugEntry) >> callrax >> >> Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support XCODE5. >> One tricky way is selected to fix it. Although SmiEntry logic is copied to >> another place and run, but here jmp _SmiHandler is enabled to jmp the >original >> code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative >address. >> mov rax, strict qword 0 ; mov rax, _SmiHandler >> _SmiHandlerAbsAddr: >> jmp rax >> ... >> callASM_PFX(CpuSmmDebugEntry) >> >> Now, BZ 1191 raises the issue that SmiHandler should run in the copied >address, >> can't run in the common address. So, jmp _SmiHandler is required to be >removed, >> the code is kept to run in copied address. And, the relative address is >> requried to be fixed up to the absolute address. The necessary changes >should >> not affect the behavior of platforms that already consume >PiSmmCpuDxeSmm. >> OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has >been verified. >> ... >> mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) >> CpuSmmDebugEntryAbsAddr: >> callrax >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Liming Gao >> Cc: Laszlo Ersek >> Cc: Eric Dong >> Cc: Jiewen Yao >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42 >++--- >> 1 file changed, 33 insertions(+), 9 deletions(-) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> index 315d0f8..815f95b 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm >> @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr: >> mov gs, eax >> mov ax, [rbx + DSC_SS] >> mov ss, eax >> -mov rax, strict qword 0 ; mov rax, _SmiHandler >> -_SmiHandlerAbsAddr: >> -jmp rax >> >> _SmiHandler: >> mov rbx, [rsp + 0x8] ; rcx <- CpuIndex >> @@ -189,13 +186,19 @@ _SmiHandler: >> add rsp, -0x20 >> >> mov rcx, rbx >> -callASM_PFX(CpuSmmDebugEntry) >> +mov rax, strict qword 0 ; call >> ASM_PFX(CpuSmmDebugEntry) >> +CpuSmmDebugEntryAbsAddr: >> +callrax >> >> mov rcx, rbx >> -callASM_PFX(SmiRendezvous) >> +mov rax, strict qword 0 ; callASM_PFX(SmiRendezvous) >> +SmiRendezvousAbsAddr: >> +callrax >> >> mov rcx, rbx >> -callASM_PFX(CpuSmmDebugExit) >> +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugExit) >> +CpuSmmDebugExitAbsAddr: >> +callrax >> >> add rsp, 0x20 >> >> @@ -206,7 +209,8 @@ _SmiHandler: >> >> add rsp, 0x200 >> >> -lea rax, [ASM_PFX(mXdSupported)] >> +mov rax, strict qword 0 ; lea rax, >> [ASM_PFX(mXdSupported)] >> +mXdSupportedAbsAddr: >> mov al, [rax] >> cmp al, 0 >> jz .1 >> @@ -224,13 +228,33 @@ _SmiHandler: >> >> ASM_PFX(gcSmiHandlerSize)DW $ - _SmiEntryPoint >> >> +; >> +; Retrieve the address and fill it into mov opcode. >> +; >> +; It is called in the driver entry point first. >> +; It is used to fix up the real address in mov opcode. >> +; Then, after the code logic is copied to the different location, > >The "git am" command complained that the line above added a whitespace >error. Can you please strip the trailing space character when you push >the patch? > >Reviewed-by: Laszlo Ersek >Tested-by: Laszlo Ersek > >Thanks, >Laszlo > > >> +; the code can also run. >> +; >> global ASM_PFX(PiSmmCpuSmiEntryFixupAddress) >> ASM_PFX(PiSmmCpuSmiEntryFixupAddress): >> learax, [ASM_PFX(gSmiHandlerIdtr)] >> learcx, [SmiHandlerIdtrAbsAddr] >> movqword [rcx - 8], rax >> >> -learax, [_SmiHandler] >> -learcx, [_SmiHandlerAbsAddr] >> +learax, [ASM_PFX(CpuSmmDebugEntry)] >>
Re: [edk2] [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
Hi Liming, On 09/21/18 02:56, Liming Gao wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191 > > Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp _SmiHandler > is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved into rax, > then call it. But, this code doesn't work in XCODE5 tool chain. Because XCODE5 > doesn't generated the absolute address in the EFI image. So, rax stores the > relative address. Once this logic is moved to another place, it will not work. > ; jmp _SmiHandler ; instruction is not needed > ... > mov rax, ASM_PFX(CpuSmmDebugEntry) > callrax > > Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support XCODE5. > One tricky way is selected to fix it. Although SmiEntry logic is copied to > another place and run, but here jmp _SmiHandler is enabled to jmp the original > code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative address. > mov rax, strict qword 0 ; mov rax, _SmiHandler > _SmiHandlerAbsAddr: > jmp rax > ... > callASM_PFX(CpuSmmDebugEntry) > > Now, BZ 1191 raises the issue that SmiHandler should run in the copied > address, > can't run in the common address. So, jmp _SmiHandler is required to be > removed, > the code is kept to run in copied address. And, the relative address is > requried to be fixed up to the absolute address. The necessary changes should > not affect the behavior of platforms that already consume PiSmmCpuDxeSmm. > OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has been > verified. > ... > mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) > CpuSmmDebugEntryAbsAddr: > callrax > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Liming Gao > Cc: Laszlo Ersek > Cc: Eric Dong > Cc: Jiewen Yao > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42 > ++--- > 1 file changed, 33 insertions(+), 9 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index 315d0f8..815f95b 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr: > mov gs, eax > mov ax, [rbx + DSC_SS] > mov ss, eax > -mov rax, strict qword 0 ; mov rax, _SmiHandler > -_SmiHandlerAbsAddr: > -jmp rax > > _SmiHandler: > mov rbx, [rsp + 0x8] ; rcx <- CpuIndex > @@ -189,13 +186,19 @@ _SmiHandler: > add rsp, -0x20 > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugEntry) > +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) > +CpuSmmDebugEntryAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(SmiRendezvous) > +mov rax, strict qword 0 ; callASM_PFX(SmiRendezvous) > +SmiRendezvousAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugExit) > +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugExit) > +CpuSmmDebugExitAbsAddr: > +callrax > > add rsp, 0x20 > > @@ -206,7 +209,8 @@ _SmiHandler: > > add rsp, 0x200 > > -lea rax, [ASM_PFX(mXdSupported)] > +mov rax, strict qword 0 ; lea rax, > [ASM_PFX(mXdSupported)] > +mXdSupportedAbsAddr: > mov al, [rax] > cmp al, 0 > jz .1 > @@ -224,13 +228,33 @@ _SmiHandler: > > ASM_PFX(gcSmiHandlerSize)DW $ - _SmiEntryPoint > > +; > +; Retrieve the address and fill it into mov opcode. > +; > +; It is called in the driver entry point first. > +; It is used to fix up the real address in mov opcode. > +; Then, after the code logic is copied to the different location, The "git am" command complained that the line above added a whitespace error. Can you please strip the trailing space character when you push the patch? Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek Thanks, Laszlo > +; the code can also run. > +; > global ASM_PFX(PiSmmCpuSmiEntryFixupAddress) > ASM_PFX(PiSmmCpuSmiEntryFixupAddress): > learax, [ASM_PFX(gSmiHandlerIdtr)] > learcx, [SmiHandlerIdtrAbsAddr] > movqword [rcx - 8], rax > > -learax, [_SmiHandler] > -learcx, [_SmiHandlerAbsAddr] > +learax, [ASM_PFX(CpuSmmDebugEntry)] > +learcx, [CpuSmmDebugEntryAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(SmiRendezvous)] > +learcx, [SmiRendezvousAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(CpuSmmDebugExit)] > +learcx, [CpuSmmDebugExitAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(mXdSupported)] > +learcx, [mXdSupportedAbsAddr] > movqword [rcx - 8], rax > ret >
Re: [edk2] [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
Reviewed-by: jiewen@intel.com > -Original Message- > From: Gao, Liming > Sent: Friday, September 21, 2018 8:56 AM > To: edk2-devel@lists.01.org > Cc: Laszlo Ersek ; Dong, Eric ; > Yao, Jiewen > Subject: [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry > function run the same position > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191 > > Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp > _SmiHandler > is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved > into rax, > then call it. But, this code doesn't work in XCODE5 tool chain. Because > XCODE5 > doesn't generated the absolute address in the EFI image. So, rax stores the > relative address. Once this logic is moved to another place, it will not work. > ; jmp _SmiHandler ; instruction is not needed > ... > mov rax, ASM_PFX(CpuSmmDebugEntry) > callrax > > Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support > XCODE5. > One tricky way is selected to fix it. Although SmiEntry logic is copied to > another place and run, but here jmp _SmiHandler is enabled to jmp the > original > code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative > address. > mov rax, strict qword 0 ; mov rax, _SmiHandler > _SmiHandlerAbsAddr: > jmp rax > ... > callASM_PFX(CpuSmmDebugEntry) > > Now, BZ 1191 raises the issue that SmiHandler should run in the copied > address, > can't run in the common address. So, jmp _SmiHandler is required to be > removed, > the code is kept to run in copied address. And, the relative address is > requried to be fixed up to the absolute address. The necessary changes > should > not affect the behavior of platforms that already consume > PiSmmCpuDxeSmm. > OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has > been verified. > ... > mov rax, strict qword 0 ; call > ASM_PFX(CpuSmmDebugEntry) > CpuSmmDebugEntryAbsAddr: > callrax > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Liming Gao > Cc: Laszlo Ersek > Cc: Eric Dong > Cc: Jiewen Yao > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42 > ++--- > 1 file changed, 33 insertions(+), 9 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index 315d0f8..815f95b 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr: > mov gs, eax > mov ax, [rbx + DSC_SS] > mov ss, eax > -mov rax, strict qword 0 ; mov rax, > _SmiHandler > -_SmiHandlerAbsAddr: > -jmp rax > > _SmiHandler: > mov rbx, [rsp + 0x8] ; rcx <- CpuIndex > @@ -189,13 +186,19 @@ _SmiHandler: > add rsp, -0x20 > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugEntry) > +mov rax, strict qword 0 ; call > ASM_PFX(CpuSmmDebugEntry) > +CpuSmmDebugEntryAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(SmiRendezvous) > +mov rax, strict qword 0 ; call > ASM_PFX(SmiRendezvous) > +SmiRendezvousAbsAddr: > +callrax > > mov rcx, rbx > -callASM_PFX(CpuSmmDebugExit) > +mov rax, strict qword 0 ; call > ASM_PFX(CpuSmmDebugExit) > +CpuSmmDebugExitAbsAddr: > +callrax > > add rsp, 0x20 > > @@ -206,7 +209,8 @@ _SmiHandler: > > add rsp, 0x200 > > -lea rax, [ASM_PFX(mXdSupported)] > +mov rax, strict qword 0 ; lea rax, > [ASM_PFX(mXdSupported)] > +mXdSupportedAbsAddr: > mov al, [rax] > cmp al, 0 > jz .1 > @@ -224,13 +228,33 @@ _SmiHandler: > > ASM_PFX(gcSmiHandlerSize)DW $ - _SmiEntryPoint > > +; > +; Retrieve the address and fill it into mov opcode. > +; > +; It is called in the driver entry point first. > +; It is used to fix up the real address in mov opcode. > +; Then, after the code logic is copied to the different location, > +; the code can also run. > +; > global ASM_PFX(PiSmmCpuSmiEntryFixupAddress) > ASM_PFX(PiSmmCpuSmiEntryFixupAddress): > learax, [ASM_PFX(gSmiHandlerIdtr)] > learcx, [SmiHandlerIdtrAbsAddr] > movqword [rcx - 8], rax > > -learax, [_SmiHandler] > -learcx, [_SmiHandlerAbsAddr] > +learax, [ASM_PFX(CpuSmmDebugEntry)] > +learcx, [CpuSmmDebugEntryAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(SmiRendezvous)] > +learcx, [SmiRendezvousAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(CpuSmmDebugExit)] > +learcx, [CpuSmmDebugExitAbsAddr] > +movqword [rcx - 8], rax > + > +learax, [ASM_PFX(mXdSupported)] > +learcx, [mXdSupportedAbsAddr] > movqword [rcx - 8], rax > ret > -- >
[edk2] [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191 Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp _SmiHandler is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved into rax, then call it. But, this code doesn't work in XCODE5 tool chain. Because XCODE5 doesn't generated the absolute address in the EFI image. So, rax stores the relative address. Once this logic is moved to another place, it will not work. ; jmp _SmiHandler ; instruction is not needed ... mov rax, ASM_PFX(CpuSmmDebugEntry) callrax Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support XCODE5. One tricky way is selected to fix it. Although SmiEntry logic is copied to another place and run, but here jmp _SmiHandler is enabled to jmp the original code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative address. mov rax, strict qword 0 ; mov rax, _SmiHandler _SmiHandlerAbsAddr: jmp rax ... callASM_PFX(CpuSmmDebugEntry) Now, BZ 1191 raises the issue that SmiHandler should run in the copied address, can't run in the common address. So, jmp _SmiHandler is required to be removed, the code is kept to run in copied address. And, the relative address is requried to be fixed up to the absolute address. The necessary changes should not affect the behavior of platforms that already consume PiSmmCpuDxeSmm. OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has been verified. ... mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) CpuSmmDebugEntryAbsAddr: callrax Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Liming Gao Cc: Laszlo Ersek Cc: Eric Dong Cc: Jiewen Yao --- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42 ++--- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm index 315d0f8..815f95b 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr: mov gs, eax mov ax, [rbx + DSC_SS] mov ss, eax -mov rax, strict qword 0 ; mov rax, _SmiHandler -_SmiHandlerAbsAddr: -jmp rax _SmiHandler: mov rbx, [rsp + 0x8] ; rcx <- CpuIndex @@ -189,13 +186,19 @@ _SmiHandler: add rsp, -0x20 mov rcx, rbx -callASM_PFX(CpuSmmDebugEntry) +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugEntry) +CpuSmmDebugEntryAbsAddr: +callrax mov rcx, rbx -callASM_PFX(SmiRendezvous) +mov rax, strict qword 0 ; callASM_PFX(SmiRendezvous) +SmiRendezvousAbsAddr: +callrax mov rcx, rbx -callASM_PFX(CpuSmmDebugExit) +mov rax, strict qword 0 ; callASM_PFX(CpuSmmDebugExit) +CpuSmmDebugExitAbsAddr: +callrax add rsp, 0x20 @@ -206,7 +209,8 @@ _SmiHandler: add rsp, 0x200 -lea rax, [ASM_PFX(mXdSupported)] +mov rax, strict qword 0 ; lea rax, [ASM_PFX(mXdSupported)] +mXdSupportedAbsAddr: mov al, [rax] cmp al, 0 jz .1 @@ -224,13 +228,33 @@ _SmiHandler: ASM_PFX(gcSmiHandlerSize)DW $ - _SmiEntryPoint +; +; Retrieve the address and fill it into mov opcode. +; +; It is called in the driver entry point first. +; It is used to fix up the real address in mov opcode. +; Then, after the code logic is copied to the different location, +; the code can also run. +; global ASM_PFX(PiSmmCpuSmiEntryFixupAddress) ASM_PFX(PiSmmCpuSmiEntryFixupAddress): learax, [ASM_PFX(gSmiHandlerIdtr)] learcx, [SmiHandlerIdtrAbsAddr] movqword [rcx - 8], rax -learax, [_SmiHandler] -learcx, [_SmiHandlerAbsAddr] +learax, [ASM_PFX(CpuSmmDebugEntry)] +learcx, [CpuSmmDebugEntryAbsAddr] +movqword [rcx - 8], rax + +learax, [ASM_PFX(SmiRendezvous)] +learcx, [SmiRendezvousAbsAddr] +movqword [rcx - 8], rax + +learax, [ASM_PFX(CpuSmmDebugExit)] +learcx, [CpuSmmDebugExitAbsAddr] +movqword [rcx - 8], rax + +learax, [ASM_PFX(mXdSupported)] +learcx, [mXdSupportedAbsAddr] movqword [rcx - 8], rax ret -- 2.10.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
