1. Patchew
  2. EDK2
  3. View series

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

Guo Dong posted 1 patch 3 years, 3 months ago
Failed in applying to current master (apply log)
UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
[edk2-devel] [PATCH V2] 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 "o64 retf" 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..a8216cd56f 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
+    o64 retf
 setCodeSelectorLongJump:
-    add     rsp, 0x10
     ret
 
 ;------------------------------------------------------------------------------
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69889): https://edk2.groups.io/g/devel/message/69889
Mute This Topic: https://groups.io/mt/79491640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Laszlo Ersek 3 years, 3 months ago 
Adding James, Mike, and Tom:

On 01/07/21 04:10, Guo Dong wrote:
> 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 "o64 retf" 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..a8216cd56f 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
> +    o64 retf
>  setCodeSelectorLongJump:
> -    add     rsp, 0x10
>      ret
>  
>  ;------------------------------------------------------------------------------
> 

I'm asking for:

- a Reviewed-by or Tested-by from James please, and
- an Acked-by or Reviewed-by from Mike please, and
- a Reviewed-by or Tested-by from Tom please.

Personally I'm not going to verify the details of the conversion of the
mode switch, from the far jump to the far ret, but squinting from a
distance, the patch looks plausible.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69916): https://edk2.groups.io/g/devel/message/69916
Mute This Topic: https://groups.io/mt/79491640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
Posted by James Bottomley 3 years, 3 months ago 
On Thu, 2021-01-07 at 12:56 +0100, Laszlo Ersek wrote:
> Adding James, Mike, and Tom:
> 
> On 01/07/21 04:10, Guo Dong wrote:
> > 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 "o64 retf" 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..a8216cd56f 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
> > +    o64 retf
> >  setCodeSelectorLongJump:
> > -    add     rsp, 0x10
> >      ret
> >  
> >  ;-----------------------------------------------------------------
> > -------------
> > 
> 
> I'm asking for:
> James Bottomley <jejb@linux.ibm.com>
> - a Reviewed-by or Tested-by from James please, and

Tested-by: James Bottomley <jejb@linux.ibm.com>

James




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69988): https://edk2.groups.io/g/devel/message/69988
Mute This Topic: https://groups.io/mt/79491640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Ni, Ray 3 years, 3 months ago 
Reviewed-by: Ray Ni <ray.ni@intel.com>
I will remove the additional comma when creating the pull request.

> -----Original Message-----
> From: James Bottomley <jejb@linux.ibm.com>
> Sent: Friday, January 8, 2021 6:25 AM
> To: devel@edk2.groups.io; lersek@redhat.com; Dong, Guo
> <guo.dong@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul1 <rahul1.kumar@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
> 
> On Thu, 2021-01-07 at 12:56 +0100, Laszlo Ersek wrote:
> > Adding James, Mike, and Tom:
> >
> > On 01/07/21 04:10, Guo Dong wrote:
> > > 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 "o64 retf" 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..a8216cd56f 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
> > > +    o64 retf
> > >  setCodeSelectorLongJump:
> > > -    add     rsp, 0x10
> > >      ret
> > >
> > >  ;-----------------------------------------------------------------
> > > -------------
> > >
> >
> > I'm asking for:
> > James Bottomley <jejb@linux.ibm.com>
> > - a Reviewed-by or Tested-by from James please, and
> 
> Tested-by: James Bottomley <jejb@linux.ibm.com>
> 
> James
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70075): https://edk2.groups.io/g/devel/message/70075
Mute This Topic: https://groups.io/mt/79491640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Lendacky, Thomas 3 years, 3 months ago 
On 1/6/21 9:10 PM, Guo Dong via groups.io wrote:
> 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 "o64 retf" instead.
> 
> Signed-off-by: Guo Dong <guo.dong@intel.com>

With one little comment below, I verified this method allows my system to 
boot.

Tested-by: Tom Lendacky <thomas.lendacky@amd.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..a8216cd56f 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,

Extraneous comma after rcx.

Thanks,
Tom

>       lea     rax, [setCodeSelectorLongJump]
> -    mov     [rsp], rax
> -    mov     [rsp+4], cx
> -    jmp     dword far [rsp]
> +    push    rax
> +    o64 retf
>   setCodeSelectorLongJump:
> -    add     rsp, 0x10
>       ret
>   
>   ;------------------------------------------------------------------------------
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69927): https://edk2.groups.io/g/devel/message/69927
Mute This Topic: https://groups.io/mt/79491640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Guo Dong 3 years, 3 months ago 
Thanks Tom to help verify this change and point out an extra comma after rcx.
I will remove that extra comma when pushing this patch.
I could send V3 patch if required to remove this extra comma.

Thanks,
Guo

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lendacky,
> Thomas
> Sent: Thursday, January 7, 2021 8:02 AM
> To: devel@edk2.groups.io; Dong, Guo <guo.dong@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 V2] UefiCpuPkg/CpuDxe: Fix boot error
> 
> On 1/6/21 9:10 PM, Guo Dong via groups.io wrote:
> > 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 "o64 retf" instead.
> >
> > Signed-off-by: Guo Dong <guo.dong@intel.com>
> 
> With one little comment below, I verified this method allows my system to
> boot.
> 
> Tested-by: Tom Lendacky <thomas.lendacky@amd.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..a8216cd56f 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,
> 
> Extraneous comma after rcx.
> 
> Thanks,
> Tom
> 
> >       lea     rax, [setCodeSelectorLongJump]
> > -    mov     [rsp], rax
> > -    mov     [rsp+4], cx
> > -    jmp     dword far [rsp]
> > +    push    rax
> > +    o64 retf
> >   setCodeSelectorLongJump:
> > -    add     rsp, 0x10
> >       ret
> >
> >   ;------------------------------------------------------------------------------
> >
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69931): https://edk2.groups.io/g/devel/message/69931
Mute This Topic: https://groups.io/mt/79491640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
Posted by Laszlo Ersek 3 years, 3 months ago 
On 01/07/21 17:14, Dong, Guo wrote:
> 
> Thanks Tom to help verify this change and point out an extra comma after rcx.
> I will remove that extra comma when pushing this patch.
> I could send V3 patch if required to remove this extra comma.

I think a v3 is unneeded just for that.

Thanks
Laszlo

> 
> Thanks,
> Guo
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lendacky,
>> Thomas
>> Sent: Thursday, January 7, 2021 8:02 AM
>> To: devel@edk2.groups.io; Dong, Guo <guo.dong@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 V2] UefiCpuPkg/CpuDxe: Fix boot error
>>
>> On 1/6/21 9:10 PM, Guo Dong via groups.io wrote:
>>> 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 "o64 retf" instead.
>>>
>>> Signed-off-by: Guo Dong <guo.dong@intel.com>
>>
>> With one little comment below, I verified this method allows my system to
>> boot.
>>
>> Tested-by: Tom Lendacky <thomas.lendacky@amd.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..a8216cd56f 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,
>>
>> Extraneous comma after rcx.
>>
>> Thanks,
>> Tom
>>
>>>       lea     rax, [setCodeSelectorLongJump]
>>> -    mov     [rsp], rax
>>> -    mov     [rsp+4], cx
>>> -    jmp     dword far [rsp]
>>> +    push    rax
>>> +    o64 retf
>>>   setCodeSelectorLongJump:
>>> -    add     rsp, 0x10
>>>       ret
>>>
>>>   ;------------------------------------------------------------------------------
>>>
>>
>>
>> 
>>
> 



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

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo Dong
> Sent: Wednesday, January 6, 2021 7:11 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 V2] 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 "o64 retf" 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..a8216cd56f 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
> +    o64 retf
>  setCodeSelectorLongJump:
> -    add     rsp, 0x10
>      ret
> 
>  ;------------------------------------------------------------------------------
> --
> 2.16.2.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69947): https://edk2.groups.io/g/devel/message/69947
Mute This Topic: https://groups.io/mt/79491640/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.