1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error

Guo Dong posted 1 patch 3 years, 3 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
[edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Guo Dong 3 years, 3 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69457): https://edk2.groups.io/g/devel/message/69457
Mute This Topic: https://groups.io/mt/79209121/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Michael D Kinney 3 years, 3 months ago 
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
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69669): https://edk2.groups.io/g/devel/message/69669
Mute This Topic: https://groups.io/mt/79209121/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Guo Dong 3 years, 3 months ago 
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
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69764): https://edk2.groups.io/g/devel/message/69764
Mute This Topic: https://groups.io/mt/79209121/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Michael D Kinney 3 years, 3 months ago 
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
> > >
> > >
> > >
> > > 
> > >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69770): https://edk2.groups.io/g/devel/message/69770
Mute This Topic: https://groups.io/mt/79209121/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Laszlo Ersek 3 years, 3 months ago 
On 01/06/21 03:00, Kinney, Michael D wrote:
> Hi Guo,
> 
> Could be the port from MASM to NASM did not check to see if NASM supported the instruction.

I agree: ad8ae98d2fa2 ("OvmfPkg LoadLinuxLib: Convert
X64/JumpToKernel.asm to NASM", 2014-10-31). I seem to remember that
Jordan implemented that large MASM->NASM conversion series mostly
mechanically (which was the right approach, of course, given the number
of assembly files, and the regression risks).

> You can verify the NASM disassembly to make sure it matches the DB bytes.

Thanks!
Laszlo

> 
> 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
>>>>
>>>>
>>>>
>>>> 
>>>>
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69836): https://edk2.groups.io/g/devel/message/69836
Mute This Topic: https://groups.io/mt/79209121/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Laszlo Ersek 3 years, 3 months ago 
On 01/05/21 05:31, Michael D Kinney wrote:
> Hi Guo,
> 
> NASM has good support for instructions.  Can the DB be removed and replaced with the equivalent instruction?

Indeed. Thanks!
Laszlo

> 
> 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
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69827): https://edk2.groups.io/g/devel/message/69827
Mute This Topic: https://groups.io/mt/79209121/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.