Re: UefiCpuPkg/CpuDxe: Fix boot error

["Laszlo Ersek" <lersek@redhat.com>]

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