From: hpa@zytor.com
To: Andrew Fish <afish@apple.com>, "Fan, Jeff" <jeff.fan@intel.com>
Cc: Mike Kinney <michael.d.kinney@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@ml01.01.org>
Subject: Re: [Patch V2] UefiCpuPkg/MpInitLib: Fix X64 XCODE5/NASM compatibility issues
Date: Tue, 06 Jun 2017 14:05:10 -0700 [thread overview]
Message-ID: <EE48FAD1-95F6-499D-AD4E-EE689A4DB81A@zytor.com> (raw)
In-Reply-To: <7A0046FF-FF4E-44FA-9A6F-D2765E07DAA1@apple.com>
On June 6, 2017 1:49:34 PM PDT, Andrew Fish <afish@apple.com> wrote:
>
>> On Jun 6, 2017, at 12:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On 05/22/17 19:08, Fan, Jeff wrote:
>>>
>>> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>> index fa54d01..0b14a53 100644
>>> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>>> @@ -1,5 +1,5 @@
>>>
>;------------------------------------------------------------------------------
>; -; Copyright (c) 2015 - 2016, Intel Corporation. All rights
>reserved.<BR>
>>> +; Copyright (c) 2015 - 2017, Intel Corporation. All rights
>>> +reserved.<BR>
>>> ; This program and the accompanying materials ; are licensed and
>made available under the terms and conditions of the BSD License ;
>which accompanies this distribution. The full text of the license may
>be found at @@ -201,7 +201,7 @@ CProcedureInvoke:
>>> push rbp
>>> mov rbp, rsp
>>>
>>> - mov rax, ASM_PFX(InitializeFloatingPointUnits)
>>> + mov rax, qword [esi +
>InitializeFloatingPointUnitsAddress]
>
>Does nasm remove the need for the ASM_PFX() macro? That macro hides if
>C is decorating with a _ prefix.
>
>Also given it is a #define (equ) why do we use camel case vs. all caps?
>
>Thanks,
>
>Andrew Fish
>
>>> sub rsp, 20h
>>> call rax ; Call assembly function to
>initialize FPU per UEFI spec
>>> add rsp, 20h
>>
>> FYI, the qword specifier is unnecessary since you are already
>specifying
>> rax.
>>
>> However, why not simply drop the use of rax entirely and do:
>>
>> call [esi + InitializeFloatingPointUnitsAddress]
>>
>> (Also: is this *really* supposed to be esi and not rsi? The former
>> means a 32-bit address.)
>>
>> -hpa
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
The NASM command-line option --prefix _ should do exactly that.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
next prev parent reply other threads:[~2017-06-06 21:06 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-22 17:12 [Patch V2] UefiCpuPkg/MpInitLib: Fix X64 XCODE5/NASM compatibility issues Michael Kinney
2017-05-22 17:14 ` Andrew Fish
2017-05-23 2:08 ` Fan, Jeff
2017-06-06 19:41 ` H. Peter Anvin
2017-06-06 20:49 ` Andrew Fish
2017-06-06 21:05 ` hpa [this message]
2017-06-06 21:15 ` Andrew Fish
2017-06-06 21:21 ` hpa
2017-06-13 23:15 ` Kinney, Michael D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=EE48FAD1-95F6-499D-AD4E-EE689A4DB81A@zytor.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox