Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler
We have some internal feature, required to run the code at the each SMI entrypoint, instead of common code. That is why we write code in previous way. I suggest we keep using the previous way and provide a good example on how to handle absolute address in new way. Can we provide an update for below: > >mov rax, ASM_PFX(CpuSmmDebugEntry) > >callrax Thank you Yao Jiewen > -Original Message- > From: Gao, Liming > Sent: Wednesday, September 12, 2018 9:30 AM > To: Yao, Jiewen ; Laszlo Ersek ; > edk2-devel@lists.01.org > Cc: Dong, Eric ; Gao, Liming > Subject: RE: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove > unnecessary jmp _SmiHandler > > Jiewen: > Because nasm doesn't generate the absolute address, we can change the > below code, but we need to fix up its value at boot time like current > _SmiHandler way. > > Could you let me know why can't use current way? > > 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) > >callrax > > > >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 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 > >> 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 the > >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 > >> callASM_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-boun...@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/Smi
Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler
Jiewen: Because nasm doesn't generate the absolute address, we can change the below code, but we need to fix up its value at boot time like current _SmiHandler way. Could you let me know why can't use current way? 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) >callrax > >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 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 >> 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 the >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 >> callASM_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-boun...@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): >> >> > learax, [ASM_PFX(gSmiHandlerIdtr)] >> >> > learcx, [SmiHandlerIdtrAbsAddr] >> >> > movqword [rcx - 8], rax >> >> > - >> >> > -learax, [_SmiHandler] >> >> > -learcx, [_SmiHandlerAbsAddr] >> >> > -movqword [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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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) callrax 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 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 > 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 the 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 > callASM_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-boun...@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): > >> > learax, [ASM_PFX(gSmiHandlerIdtr)] > >> > learcx, [SmiHandlerIdtrAbsAddr] > >> > movqword [rcx - 8], rax > >> > - > >> > -learax, [_SmiHandler] > >> > -learcx, [_SmiHandlerAbsAddr] > >> > -movqword [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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler
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 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 the 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 callASM_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-boun...@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): >> > learax, [ASM_PFX(gSmiHandlerIdtr)] >> > learcx, [SmiHandlerIdtrAbsAddr] >> > movqword [rcx - 8], rax >> > - >> > -learax, [_SmiHandler] >> > -learcx, [_SmiHandlerAbsAddr] >> > -movqword [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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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-boun...@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): > > learax, [ASM_PFX(gSmiHandlerIdtr)] > > learcx, [SmiHandlerIdtrAbsAddr] > > movqword [rcx - 8], rax > > - > > -learax, [_SmiHandler] > > -learcx, [_SmiHandlerAbsAddr] > > -movqword [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 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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): > learax, [ASM_PFX(gSmiHandlerIdtr)] > learcx, [SmiHandlerIdtrAbsAddr] > movqword [rcx - 8], rax > - > -learax, [_SmiHandler] > -learcx, [_SmiHandlerAbsAddr] > -movqword [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
[edk2] [Patch] UefiCpuPkg PiSmmCpuDxeSmm: Remove unnecessary jmp _SmiHandler
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): learax, [ASM_PFX(gSmiHandlerIdtr)] learcx, [SmiHandlerIdtrAbsAddr] movqword [rcx - 8], rax - -learax, [_SmiHandler] -learcx, [_SmiHandlerAbsAddr] -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
