* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 3:10 [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error Guo Dong
@ 2021-01-07 11:56 ` Laszlo Ersek
2021-01-07 22:25 ` James Bottomley
2021-01-07 15:01 ` Lendacky, Thomas
2021-01-07 18:22 ` Michael D Kinney
2 siblings, 1 reply; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-07 11:56 UTC (permalink / raw)
To: Guo Dong, devel
Cc: eric.dong, ray.ni, rahul1.kumar, Tom Lendacky, James Bottomley,
Michael Kinney
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 11:56 ` Laszlo Ersek
@ 2021-01-07 22:25 ` James Bottomley
2021-01-09 2:30 ` Ni, Ray
0 siblings, 1 reply; 8+ messages in thread
From: James Bottomley @ 2021-01-07 22:25 UTC (permalink / raw)
To: devel, lersek, Guo Dong
Cc: eric.dong, ray.ni, rahul1.kumar, Tom Lendacky, Michael Kinney
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 22:25 ` James Bottomley
@ 2021-01-09 2:30 ` Ni, Ray
0 siblings, 0 replies; 8+ messages in thread
From: Ni, Ray @ 2021-01-09 2:30 UTC (permalink / raw)
To: jejb@linux.ibm.com, devel@edk2.groups.io, lersek@redhat.com,
Dong, Guo
Cc: Dong, Eric, Kumar, Rahul1, Tom Lendacky, Kinney, Michael D
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 3:10 [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error Guo Dong
2021-01-07 11:56 ` Laszlo Ersek
@ 2021-01-07 15:01 ` Lendacky, Thomas
2021-01-07 16:14 ` Guo Dong
2021-01-07 18:22 ` Michael D Kinney
2 siblings, 1 reply; 8+ messages in thread
From: Lendacky, Thomas @ 2021-01-07 15:01 UTC (permalink / raw)
To: devel, guo.dong; +Cc: eric.dong, ray.ni, lersek, rahul1.kumar
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
>
> ;------------------------------------------------------------------------------
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 15:01 ` Lendacky, Thomas
@ 2021-01-07 16:14 ` Guo Dong
2021-01-07 17:32 ` Laszlo Ersek
0 siblings, 1 reply; 8+ messages in thread
From: Guo Dong @ 2021-01-07 16:14 UTC (permalink / raw)
To: devel@edk2.groups.io, thomas.lendacky@amd.com
Cc: Dong, Eric, Ni, Ray, lersek@redhat.com, Kumar, Rahul1
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
> >
> > ;------------------------------------------------------------------------------
> >
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 16:14 ` Guo Dong
@ 2021-01-07 17:32 ` Laszlo Ersek
0 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2021-01-07 17:32 UTC (permalink / raw)
To: Dong, Guo, devel@edk2.groups.io, thomas.lendacky@amd.com
Cc: Dong, Eric, Ni, Ray, Kumar, Rahul1
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
>>>
>>> ;------------------------------------------------------------------------------
>>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error
2021-01-07 3:10 [edk2-devel] [PATCH V2] UefiCpuPkg/CpuDxe: Fix boot error Guo Dong
2021-01-07 11:56 ` Laszlo Ersek
2021-01-07 15:01 ` Lendacky, Thomas
@ 2021-01-07 18:22 ` Michael D Kinney
2 siblings, 0 replies; 8+ messages in thread
From: Michael D Kinney @ 2021-01-07 18:22 UTC (permalink / raw)
To: devel@edk2.groups.io, Dong, Guo, Kinney, Michael D
Cc: Dong, Eric, Ni, Ray, lersek@redhat.com, Kumar, Rahul1
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
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread