Re: [edk2] [PATCH 0/8] NASM: Object files; Thunk16; Require for non-MSVC

Subject: Re: [edk2] [PATCH 0/8] NASM: Object files; Thunk16; Require for non-MSVC

From: Sergey Isakov <isakov-sl@bk.ru>

To: edk2-devel@lists.sourceforge.net

Date: 2014-08-28 21:47:23

I checked only for X64 but I think ia32 will be the same. 
OK, will test now.

On 28.08.2014, at 12:53, Gao, Liming wrote:

Sergey:
   Your change adds ASM_PFX() for the external variable to let it pass XLCANG tool chain. Is it for IA32 Thunk16.nasm or X64 Thunk16.nasm or both?
 
Thanks
Liming
From: Sergey Isakov [mailto:isakov-sl@bk.ru] 
Sent: Thursday, August 28, 2014 3:16 PM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [PATCH 0/8] NASM: Object files; Thunk16; Require for non-MSVC
 
Comments below
 
On 28.08.2014, at 7:54, Andrew Fish wrote:


 
On Aug 27, 2014, at 8:43 PM, Sergey Isakov <isakov-sl@bk.ru> wrote:


Jordan,
There was no issue with gcc49 toolchain in x86_64.
I made an attempt to use nasm with XCLANG toolset but failed on ld:
I seems have to wait while Andrew make XCODE5 fully working.
 
 __USER_LABEL_PREFIX__ is set to _ on clang (XCODE5). 
This patch works for both GCC49 and XCLANG toolchain (compilation, not concerning relocation problem)
---
--- Thunk16.nasm   2014-08-26 22:45:26.000000000 +0400
+++ New/Thunk16.nasm         2014-08-28 11:01:17.000000000 +0400
@@ -56,11 +56,11 @@ SECTION .data
 ;
 ; These are global constant to convey information to C code.
 ;
-ASM_PFX(m16Size)         DW      InternalAsmThunk16 - m16Start
-ASM_PFX(mThunk16Attr)    DW      _BackFromUserCode.ThunkAttrEnd - 4 - m16Start
-ASM_PFX(m16Gdt)          DW      _NullSeg - m16Start
-ASM_PFX(m16GdtrBase)     DW      _16GdtrBase - m16Start
-ASM_PFX(mTransition)     DW      _EntryPoint - m16Start
+ASM_PFX(m16Size)         DW      ASM_PFX(InternalAsmThunk16) - ASM_PFX(m16Start)
+ASM_PFX(mThunk16Attr)    DW      _BackFromUserCode.ThunkAttrEnd - 4 - ASM_PFX(m16Start)
+ASM_PFX(m16Gdt)          DW      _NullSeg - ASM_PFX(m16Start)
+ASM_PFX(m16GdtrBase)     DW      _16GdtrBase - ASM_PFX(m16Start)
+ASM_PFX(mTransition)     DW      _EntryPoint - ASM_PFX(m16Start)
 
 SECTION .text
 
@@ -140,7 +140,7 @@ BITS    64
     ret
 
 _EntryPoint:
-        DD      _ToUserCode - m16Start
+        DD      _ToUserCode - ASM_PFX(m16Start)
         DW      CODE16
 _16Gdtr:
         DW      GDT_SIZE - 1
@@ -256,16 +256,16 @@ BITS    64
     add     edi, eax                    ; edi <- linear address of 16-bit stack
     pop     rcx
     rep     movsd                       ; copy RegSet
-    lea     ecx, [rdx + (_BackFromUserCode.SavedCr4End - m16Start)]
+    lea     ecx, [rdx + (_BackFromUserCode.SavedCr4End - ASM_PFX(m16Start))]
     mov     eax, edx                    ; eax <- transition code address
     and     edx, 0fh
     shl     eax, 12                     ; segment address in high order 16 bits
-    lea     ax, [rdx + (_BackFromUserCode - m16Start)]  ; offset address
+    lea     ax, [rdx + (_BackFromUserCode - ASM_PFX(m16Start))]  ; offset address
     stosd                               ; [edi] <- return address of user code
   
     sgdt    [rsp + 60h]       ; save GDT stack in argument space
     movzx   r10, word [rsp + 60h]   ; r10 <- GDT limit 
-    lea     r11, [rcx + (InternalAsmThunk16 - _BackFromUserCode.SavedCr4End) + 0xf]
+    lea     r11, [rcx + (ASM_PFX(InternalAsmThunk16) - _BackFromUserCode.SavedCr4End) + 0xf]
     and     r11, ~0xf            ; r11 <- 16-byte aligned shadowed GDT table in real mode buffer
     
     mov     [rcx + (SavedGdt - _BackFromUserCode.SavedCr4End)], r10w      ; save the limit of shadowed GDT table
 
---


 
Actually I just noticed you are getting an illegal text-relocation error. This is usually caused by an absolute relocation, and clang wants it to be IP relative. Basically Xcode/clang only supports a subset of the relocations that gcc uses for X64. 
If I correctly understand you, compilation for Thunk16 is not possible in XCLANG toolset because of idea of this codes?


 
Thanks,
 
Andrew Fish


Sergey
 
On 28 . 2014 ., at 7:32, Andrew Fish <afish@apple.com> wrote:



On Aug 27, 2014, at 8:08 PM, Gao, Liming <liming.gao@intel.com> wrote:


Hi,
  What change do you make? In previous mail, you mention x86_64 there is no issue.
 
 
Liming,
 
On the various GNU (including clang) toolchains __USER_LABEL_PREFIX__ is the prefix applied to user labels (symbols visible to C) in assembly. Some versions set it to '', and some version have it set to _'
 
#define ASM_PFX(name) _CONCATENATE (__USER_LABEL_PREFIX__, name)
 
So you can have a bug that passes on one version and fails on another. 
 
Thanks,
 
Andrew Fish


Thanks
Liming
From: Sergey Isakov [mailto:isakov-sl@bk.ru] 
Sent: Thursday, August 28, 2014 4:42 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [PATCH 0/8] NASM: Object files; Thunk16; Require for non-MSVC
 
The error disappeared but I still have other mistakes
ld: illegal text-relocation to '_NullSeg' in   BaseLib.lib(Thunk16.obj) from '_16GdtrBase in BaseLib.lib(Thunk16.obj) for architecture x86_64
Still in work
 
On 28 . 2014 ., at 0:33, Jordan Justen <jljusten@gmail.com> wrote:



On Wed, Aug 27, 2014 at 1:08 PM, Sergey Isakov <isakov-sl@bk.ru> wrote:


And one new question. Why _16GdtrBase with underscore while m16Start without
it?

Does replacing m16Start with ASM_PFX(m16Start) help?

It looks like I didn't convert this up properly. The MASM code didn't
use ASM_PFX...

-Jordan




;
; These are global constant to convey information to C code.
;
ASM_PFX(m16Size)         DW      InternalAsmThunk16 - m16Start
ASM_PFX(mThunk16Attr)    DW      _BackFromUserCode.ThunkAttrEnd - 4 -
m16Start
ASM_PFX(m16Gdt)          DW      _NullSeg - m16Start
ASM_PFX(m16GdtrBase)     DW      _16GdtrBase - m16Start
ASM_PFX(mTransition)     DW      _EntryPoint - m16Start


I got an error with this
Thunk16.iii:60: error: symbol `m16Start' undefined


On 27 . 2014 ., at 22:04, Jordan Justen <jljusten@gmail.com> wrote:

On Wed, Aug 27, 2014 at 5:18 AM, Sergey Isakov <isakov-sl@bk.ru> wrote:

Hi sirs,
I tested Thunk16.nasm too. For x86_64 there is no issue.
For ia32 compilation disasm shows not good codes
---------
00000000 <m16Start>:
 0: 00 00                 add    %al,(%eax)
 2: 00 00                 add    %al,(%eax)
...

00000006 <_BackFromUserCode>:
 6: 16                   push   %ss
 7: 0e                   push   %cs
 8: 67 e8 00 00 66 9c     addr16 call 9c66000e
<InternalAsmThunk16+0x9c65ff36>

0000000c <_BackFromUserCode.Base>:
 c: 66 9c                 pushfw
 e: fa                   cli

---------
to compare gcc-4.9.1 cvompilation
---------
00000000 <m16Start>:
 0: 00 00                 add    %al,(%eax)
 2: 00 00                 add    %al,(%eax)
...

00000006 <BackFromUserCode>:
 6: 16                   push   %ss
 7: 0e                   push   %cs
 8: 66 e8 00 00           callw  c <BackFromUserCode+0x6>
...

0000000e <L_Base1>:
 e: 66 9c                 pushfw
10: fa                   cli

----------
May be change a32 prefix to o32 in the line:
"a32 call    .Base                       ; push eip"
?


In the X64 file, I used:
call    dword .Base

I notice this produced the same code as:
o32 call    dword .Base

But,
o32 call    .Base
seems to produce invalid code. (OVMF hangs.)

It does seem like:
a32 call    .Base
is functional as well, but I know we are attempting to make the new
nasm code produce similar code to the old asm code where reasonable.

I think I only changed it in the X64 file to try to match the old 'db' code.

-Jordan

On 27.08.2014, at 9:50, Jordan Justen wrote:

On Tue, Aug 19, 2014 at 4:57 PM, Jordan Justen
<jordan.l.justen@intel.com> wrote:

This series:

* Adds support for creating object files from .nasm source files to

allow NASM source files to be linked into libraries and images

* Adds NASM source files for Thunk16 on IA32 and X64

* Convert BaseLib to use NASM Source Files for Thunk16 rather than

GNU Assembler (.S) files

- This will make NASM a build requirement for MdePkg with IA32 &

  X64 on all toolchains except MSVC and ICC.


Thanks for Liming Gao for the tools_def changes for MSVC. (But, these

have not been tested very well yet.)


This series is available in this git branch:

https://github.com/jljusten/edk2.git nasm-v1


I update my nasm branch based on the feedback.

The differences vs. v1 are in the attached patch.

Is anyone interested in seeing a v2 posted to the list?

Liming, in patch 1, I mentioned I only tested with GCC49. Did you have
any testing to add to that?

-Jordan

Jordan Justen (8):

BaseTools/tools_def: Add NASM_FLAGS

BaseTools/build_rule: Add .nasm => .obj build rule

MdePkg/Base.h: Always define ASM_PFX

MdePkg/BaseLib Thunk16: Replace IA32 GAS Thunk16 with NASM version

MdePkg/BaseLib Thunk16: Replace X64 GAS Thunk16 with NASM version

MdePkg/BaseLib NASM Thunk16: Use NASM local labels

MdePkg/BaseLib NASM Thunk16: Use bits 16 for 16-bit code

MdePkg/BaseLib NASM Thunk16: Remove remaining 'DB' code


BaseTools/Conf/build_rule.template       |  15 ++

BaseTools/Conf/tools_def.template        | 287 ++++++++++++++++++++++++++-

MdePkg/Include/AArch64/ProcessorBind.h   |   4 +

MdePkg/Include/Arm/ProcessorBind.h       |   4 +

MdePkg/Include/Base.h                    |  25 ++-

MdePkg/Include/Ebc/ProcessorBind.h       |   4 +

MdePkg/Include/Ia32/ProcessorBind.h      |   4 +

MdePkg/Include/Ipf/ProcessorBind.h       |   4 +

MdePkg/Include/X64/ProcessorBind.h       |   4 +

MdePkg/Library/BaseLib/BaseLib.inf       |   4 +-

MdePkg/Library/BaseLib/Ia32/Thunk16.nasm | 260 +++++++++++++++++++++++++

MdePkg/Library/BaseLib/X64/Thunk16.nasm  | 321
+++++++++++++++++++++++++++++++

12 files changed, 914 insertions(+), 22 deletions(-)

create mode 100644 MdePkg/Library/BaseLib/Ia32/Thunk16.nasm

create mode 100644 MdePkg/Library/BaseLib/X64/Thunk16.nasm


--

2.1.0



------------------------------------------------------------------------------

Slashdot TV.

Video for Nerds.  Stuff that matters.

http://tv.slashdot.org/

_______________________________________________

edk2-devel mailing list

edk2-devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/edk2-devel

<compare-nasm-to-nasm-v1.patch>------------------------------------------------------------------------------

Slashdot TV.
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel



------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel



------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
 
------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
 
------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
 
------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
 
------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel