* FW: UefiCpuPkg/CpuDxe: Fix boot error [not found] <BYAPR11MB36223CE7DD008AE912112CD59ED00@BYAPR11MB3622.namprd11.prod.outlook.com> @ 2021-01-06 22:07 ` Guo Dong 2021-01-07 1:00 ` Michael D Kinney 0 siblings, 1 reply; 3+ messages in thread From: Guo Dong @ 2021-01-06 22:07 UTC (permalink / raw) To: devel@edk2.groups.io, Kinney, Michael D Cc: Dong, Eric, Ni, Ray, lersek@redhat.com, Kumar, Rahul1 [-- Attachment #1: Type: text/plain, Size: 4583 bytes --] Hi Mike, I just did a test with retfq, it looks the compiler will take 'retfq' as a label with a warnig: "CpuAsm.iii:26: warning: label alone on a line without a colon might be in error" CpuAsm.nasm is the source file. Both CpuAsm.iii and CpuAsm.obj are from the build folder. CpuAsm_dasm.txt is the dasm file using objdump. I am using "NASM version 2.12.01rc2 compiled on Mar 7 2016". BTW, if using "O64 retf" instead of "retfq", the generated obj matches with DB one. Do you prefer using " O64 retf" comparing with "DB 0x48, 0xcb"? Thanks, Guo > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Tuesday, January 5, 2021 7:01 PM > To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io; Kinney, Michael > D <michael.d.kinney@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > lersek@redhat.com; Kumar, Rahul1 <rahul1.kumar@intel.com> > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > Hi Guo, > > Could be the port from MASM to NASM did not check to see if NASM supported > the instruction. > > You can verify the NASM disassembly to make sure it matches the DB bytes. > > Mike > > > -----Original Message----- > > From: Dong, Guo <guo.dong@intel.com> > > Sent: Tuesday, January 5, 2021 4:51 PM > > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > lersek@redhat.com; Kumar, Rahul1 > > <rahul1.kumar@intel.com> > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > > > Hi Mike, > > > > Thanks for the comments. I will remove DB and submit a new patch. > > I used DB because retfq is used in EDK2 only in > OvmfPkg\Library\LoadLinuxLib\X64\JumpToKernel.nasm and it used DB. > > Not sure if there is any BKM why they use it. > > > > Thanks, > > Guo > > > > > -----Original Message----- > > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > > Sent: Monday, January 4, 2021 9:31 PM > > > To: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; Kinney, > Michael > > > D <michael.d.kinney@intel.com> > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > > > lersek@redhat.com; Kumar, Rahul1 <rahul1.kumar@intel.com> > > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > > > Hi Guo, > > > > > > NASM has good support for instructions. Can the DB be removed and > replaced > > > with the equivalent instruction? > > > > > > Thanks, > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo > Dong > > > > Sent: Thursday, December 24, 2020 12:04 PM > > > > To: devel@edk2.groups.io > > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > > > lersek@redhat.com; Kumar, Rahul1 > > > > <rahul1.kumar@intel.com> > > > > Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3084 > > > > > > > > When DXE drivers are dispatched above 4GB memory in 64bit > > > > mode, the address setCodeSelectorLongJump in stack will > > > > be override by parameter. Jump to Qword is not supported > > > > by some processors. So use retfq instead. > > > > > > > > Signed-off-by: Guo Dong <guo.dong@intel.com> > > > > --- > > > > UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm | 8 +++----- > > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > b/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > index c3489bcc3e..e33ddb2784 100644 > > > > --- a/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > +++ b/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > @@ -20,13 +20,11 @@ > > > > ;------------------------------------------------------------------------------ > > > > global ASM_PFX(SetCodeSelector) > > > > ASM_PFX(SetCodeSelector): > > > > - sub rsp, 0x10 > > > > + push rcx, > > > > lea rax, [setCodeSelectorLongJump] > > > > - mov [rsp], rax > > > > - mov [rsp+4], cx > > > > - jmp dword far [rsp] > > > > + push rax > > > > + DB 0x48, 0xcb ; retfq > > > > setCodeSelectorLongJump: > > > > - add rsp, 0x10 > > > > ret > > > > > > > > ;------------------------------------------------------------------------------ > > > > -- > > > > 2.16.2.windows.1 > > > > > > > > > > > > > > > > > > > > [-- Attachment #2: CpuAsm.iii --] [-- Type: application/x-iphone, Size: 1139 bytes --] [-- Attachment #3: CpuAsm.obj --] [-- Type: application/octet-stream, Size: 1329 bytes --] [-- Attachment #4: CpuAsm_dasm.txt --] [-- Type: text/plain, Size: 734 bytes --] /mnt/c/projects/sources/Gitlab/uefi/Edk2Open/Build/UefiPayloadPkgX64/DEBUG_VS2015x86/X64/UefiCpuPkg/CpuDxe/CpuDxe/OUTPUT/X64/CpuAsm.obj: file format pe-x86-64 Disassembly of section .text: 0000000000000000 <SetCodeSelector>: 0: 51 push %rcx 1: 48 8d 05 01 00 00 00 lea 0x1(%rip),%rax # 9 <retfq> 8: 50 push %rax 0000000000000009 <retfq>: 9: c3 retq 000000000000000a <SetDataSelectors>: a: 66 8e d1 mov %cx,%ss d: 66 8e d9 mov %cx,%ds 10: 66 8e c1 mov %cx,%es 13: 66 8e e1 mov %cx,%fs 16: 66 8e e9 mov %cx,%gs 19: c3 retq [-- Attachment #5: CpuAsm.nasm --] [-- Type: application/octet-stream, Size: 1175 bytes --] ;------------------------------------------------------------------------------ ;* ;* Copyright (c) 2016, Intel Corporation. All rights reserved.<BR> ;* SPDX-License-Identifier: BSD-2-Clause-Patent ;* ;* CpuAsm.nasm ;* ;* Abstract: ;* ;------------------------------------------------------------------------------ DEFAULT REL SECTION .text ;------------------------------------------------------------------------------ ; VOID ; SetCodeSelector ( ; UINT16 Selector ; ); ;------------------------------------------------------------------------------ global ASM_PFX(SetCodeSelector) ASM_PFX(SetCodeSelector): push rcx, lea rax, [setCodeSelectorLongJump] push rax retfq setCodeSelectorLongJump: ret ;------------------------------------------------------------------------------ ; VOID ; SetDataSelectors ( ; UINT16 Selector ; ); ;------------------------------------------------------------------------------ global ASM_PFX(SetDataSelectors) ASM_PFX(SetDataSelectors): o16 mov ss, cx o16 mov ds, cx o16 mov es, cx o16 mov fs, cx o16 mov gs, cx ret ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: UefiCpuPkg/CpuDxe: Fix boot error 2021-01-06 22:07 ` FW: UefiCpuPkg/CpuDxe: Fix boot error Guo Dong @ 2021-01-07 1:00 ` Michael D Kinney 2021-01-07 9:54 ` Laszlo Ersek 0 siblings, 1 reply; 3+ messages in thread From: Michael D Kinney @ 2021-01-07 1:00 UTC (permalink / raw) To: Dong, Guo, devel@edk2.groups.io, Kinney, Michael D Cc: Dong, Eric, Ni, Ray, lersek@redhat.com, Kumar, Rahul1 Guo, Yes. Using the following looks like the correct solution: o64 retf Thanks, Mike > -----Original Message----- > From: Dong, Guo <guo.dong@intel.com> > Sent: Wednesday, January 6, 2021 2:07 PM > To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; lersek@redhat.com; Kumar, Rahul1 > <rahul1.kumar@intel.com> > Subject: FW: UefiCpuPkg/CpuDxe: Fix boot error > > > Hi Mike, > > I just did a test with retfq, it looks the compiler will take 'retfq' as a label with a warnig: > "CpuAsm.iii:26: warning: label alone on a line without a colon might be in error" > > CpuAsm.nasm is the source file. > Both CpuAsm.iii and CpuAsm.obj are from the build folder. > CpuAsm_dasm.txt is the dasm file using objdump. > > I am using "NASM version 2.12.01rc2 compiled on Mar 7 2016". > BTW, if using "O64 retf" instead of "retfq", the generated obj matches with DB one. > Do you prefer using " O64 retf" comparing with "DB 0x48, 0xcb"? > > Thanks, > Guo > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > Sent: Tuesday, January 5, 2021 7:01 PM > > To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io; Kinney, Michael > > D <michael.d.kinney@intel.com> > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > > lersek@redhat.com; Kumar, Rahul1 <rahul1.kumar@intel.com> > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > Hi Guo, > > > > Could be the port from MASM to NASM did not check to see if NASM supported > > the instruction. > > > > You can verify the NASM disassembly to make sure it matches the DB bytes. > > > > Mike > > > > > -----Original Message----- > > > From: Dong, Guo <guo.dong@intel.com> > > > Sent: Tuesday, January 5, 2021 4:51 PM > > > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > > lersek@redhat.com; Kumar, Rahul1 > > > <rahul1.kumar@intel.com> > > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > > > > > > Hi Mike, > > > > > > Thanks for the comments. I will remove DB and submit a new patch. > > > I used DB because retfq is used in EDK2 only in > > OvmfPkg\Library\LoadLinuxLib\X64\JumpToKernel.nasm and it used DB. > > > Not sure if there is any BKM why they use it. > > > > > > Thanks, > > > Guo > > > > > > > -----Original Message----- > > > > From: Kinney, Michael D <michael.d.kinney@intel.com> > > > > Sent: Monday, January 4, 2021 9:31 PM > > > > To: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; Kinney, > > Michael > > > > D <michael.d.kinney@intel.com> > > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > > > > lersek@redhat.com; Kumar, Rahul1 <rahul1.kumar@intel.com> > > > > Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > > > > > Hi Guo, > > > > > > > > NASM has good support for instructions. Can the DB be removed and > > replaced > > > > with the equivalent instruction? > > > > > > > > Thanks, > > > > > > > > Mike > > > > > > > > > -----Original Message----- > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo > > Dong > > > > > Sent: Thursday, December 24, 2020 12:04 PM > > > > > To: devel@edk2.groups.io > > > > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; > > > > lersek@redhat.com; Kumar, Rahul1 > > > > > <rahul1.kumar@intel.com> > > > > > Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error > > > > > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3084 > > > > > > > > > > When DXE drivers are dispatched above 4GB memory in 64bit > > > > > mode, the address setCodeSelectorLongJump in stack will > > > > > be override by parameter. Jump to Qword is not supported > > > > > by some processors. So use retfq instead. > > > > > > > > > > Signed-off-by: Guo Dong <guo.dong@intel.com> > > > > > --- > > > > > UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm | 8 +++----- > > > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > b/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > > index c3489bcc3e..e33ddb2784 100644 > > > > > --- a/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > > +++ b/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm > > > > > @@ -20,13 +20,11 @@ > > > > > ;------------------------------------------------------------------------------ > > > > > global ASM_PFX(SetCodeSelector) > > > > > ASM_PFX(SetCodeSelector): > > > > > - sub rsp, 0x10 > > > > > + push rcx, > > > > > lea rax, [setCodeSelectorLongJump] > > > > > - mov [rsp], rax > > > > > - mov [rsp+4], cx > > > > > - jmp dword far [rsp] > > > > > + push rax > > > > > + DB 0x48, 0xcb ; retfq > > > > > setCodeSelectorLongJump: > > > > > - add rsp, 0x10 > > > > > ret > > > > > > > > > > ;------------------------------------------------------------------------------ > > > > > -- > > > > > 2.16.2.windows.1 > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: UefiCpuPkg/CpuDxe: Fix boot error 2021-01-07 1:00 ` Michael D Kinney @ 2021-01-07 9:54 ` Laszlo Ersek 0 siblings, 0 replies; 3+ messages in thread From: Laszlo Ersek @ 2021-01-07 9:54 UTC (permalink / raw) To: Kinney, Michael D, Dong, Guo, devel@edk2.groups.io Cc: Dong, Eric, Ni, Ray, Kumar, Rahul1 On 01/07/21 02:00, Kinney, Michael D wrote: > Guo, > > Yes. Using the following looks like the correct solution: > > o64 retf I agree; these operand size prefixes are one of the strengths of NASM. Thanks! Laszlo > > Thanks, > > Mike > >> -----Original Message----- >> From: Dong, Guo <guo.dong@intel.com> >> Sent: Wednesday, January 6, 2021 2:07 PM >> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com> >> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; lersek@redhat.com; Kumar, Rahul1 >> <rahul1.kumar@intel.com> >> Subject: FW: UefiCpuPkg/CpuDxe: Fix boot error >> >> >> Hi Mike, >> >> I just did a test with retfq, it looks the compiler will take 'retfq' as a label with a warnig: >> "CpuAsm.iii:26: warning: label alone on a line without a colon might be in error" >> >> CpuAsm.nasm is the source file. >> Both CpuAsm.iii and CpuAsm.obj are from the build folder. >> CpuAsm_dasm.txt is the dasm file using objdump. >> >> I am using "NASM version 2.12.01rc2 compiled on Mar 7 2016". >> BTW, if using "O64 retf" instead of "retfq", the generated obj matches with DB one. >> Do you prefer using " O64 retf" comparing with "DB 0x48, 0xcb"? >> >> Thanks, >> Guo >> >>> -----Original Message----- >>> From: Kinney, Michael D <michael.d.kinney@intel.com> >>> Sent: Tuesday, January 5, 2021 7:01 PM >>> To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io; Kinney, Michael >>> D <michael.d.kinney@intel.com> >>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; >>> lersek@redhat.com; Kumar, Rahul1 <rahul1.kumar@intel.com> >>> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error >>> >>> Hi Guo, >>> >>> Could be the port from MASM to NASM did not check to see if NASM supported >>> the instruction. >>> >>> You can verify the NASM disassembly to make sure it matches the DB bytes. >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: Dong, Guo <guo.dong@intel.com> >>>> Sent: Tuesday, January 5, 2021 4:51 PM >>>> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io >>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; >>> lersek@redhat.com; Kumar, Rahul1 >>>> <rahul1.kumar@intel.com> >>>> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error >>>> >>>> >>>> Hi Mike, >>>> >>>> Thanks for the comments. I will remove DB and submit a new patch. >>>> I used DB because retfq is used in EDK2 only in >>> OvmfPkg\Library\LoadLinuxLib\X64\JumpToKernel.nasm and it used DB. >>>> Not sure if there is any BKM why they use it. >>>> >>>> Thanks, >>>> Guo >>>> >>>>> -----Original Message----- >>>>> From: Kinney, Michael D <michael.d.kinney@intel.com> >>>>> Sent: Monday, January 4, 2021 9:31 PM >>>>> To: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; Kinney, >>> Michael >>>>> D <michael.d.kinney@intel.com> >>>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; >>>>> lersek@redhat.com; Kumar, Rahul1 <rahul1.kumar@intel.com> >>>>> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error >>>>> >>>>> Hi Guo, >>>>> >>>>> NASM has good support for instructions. Can the DB be removed and >>> replaced >>>>> with the equivalent instruction? >>>>> >>>>> Thanks, >>>>> >>>>> Mike >>>>> >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo >>> Dong >>>>>> Sent: Thursday, December 24, 2020 12:04 PM >>>>>> To: devel@edk2.groups.io >>>>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; >>>>> lersek@redhat.com; Kumar, Rahul1 >>>>>> <rahul1.kumar@intel.com> >>>>>> Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error >>>>>> >>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3084 >>>>>> >>>>>> When DXE drivers are dispatched above 4GB memory in 64bit >>>>>> mode, the address setCodeSelectorLongJump in stack will >>>>>> be override by parameter. Jump to Qword is not supported >>>>>> by some processors. So use retfq instead. >>>>>> >>>>>> Signed-off-by: Guo Dong <guo.dong@intel.com> >>>>>> --- >>>>>> UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm | 8 +++----- >>>>>> 1 file changed, 3 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm >>>>> b/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm >>>>>> index c3489bcc3e..e33ddb2784 100644 >>>>>> --- a/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm >>>>>> +++ b/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm >>>>>> @@ -20,13 +20,11 @@ >>>>>> ;------------------------------------------------------------------------------ >>>>>> global ASM_PFX(SetCodeSelector) >>>>>> ASM_PFX(SetCodeSelector): >>>>>> - sub rsp, 0x10 >>>>>> + push rcx, >>>>>> lea rax, [setCodeSelectorLongJump] >>>>>> - mov [rsp], rax >>>>>> - mov [rsp+4], cx >>>>>> - jmp dword far [rsp] >>>>>> + push rax >>>>>> + DB 0x48, 0xcb ; retfq >>>>>> setCodeSelectorLongJump: >>>>>> - add rsp, 0x10 >>>>>> ret >>>>>> >>>>>> ;------------------------------------------------------------------------------ >>>>>> -- >>>>>> 2.16.2.windows.1 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-07 9:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <BYAPR11MB36223CE7DD008AE912112CD59ED00@BYAPR11MB3622.namprd11.prod.outlook.com>
2021-01-06 22:07 ` FW: UefiCpuPkg/CpuDxe: Fix boot error Guo Dong
2021-01-07 1:00 ` Michael D Kinney
2021-01-07 9:54 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox