From the files listed by Jordan, it seems that rax is most popular in EDKII code base now. That means only few files need to be updated to make the code consistence in EDKII, if using %rax.
After discussed with Feng, I could help to change the following file to use %rax to make the code pass GNC and make the code consistence, besides Jordan's patch. SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/X64/AsmFuncs.S UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.S Is there anything missing? Jeff -----Original Message----- From: Jordan Justen [mailto:[email protected]] Sent: Monday, December 15, 2014 4:45 PM To: Tian, Feng; John Smith; [email protected] Subject: Re: [edk2] [PATCH] MdeModulePkg DebugSupportDxe: Fix build error with GNU assembler On 2014-12-14 23:31:28, Tian, Feng wrote: > Hi, John > > Thanks for raising this issue again. > > Here is the original patch proposed by Jordan, > - movw %es, %rax > + mov %es, %rax > > Here is the explanation in IA32 manual. > 1) IF Instruction is MOVW, THEN OperandSize is 16; > 2) MOV r/m64,Sreg Move zero extended 16-bit segment register to r/m64. > > That’s why I think the proposed patch may be inconsistent on behavior > (The former will not change the value of high 48bits. But the latter > will set them to 0). If I made some mistakes, please let me know. > > There was no progress because I didn’t get feedback from the original > patch owner. I was opting for a simple change that was more consistent with other similar code in EDK II as I mentioned: http://permalink.gmane.org/gmane.comp.bios.tianocore.devel/10502 > For me, I prefer to use movw %es, ax if it could pass build. Any idea > on this, John & Jordan? I don't use DebugSupportDxe (nor would I recommend it to anyone at this time), so I don't think I'm in a good position to make a different change. If you want to fix the code differently than my patch, then I think you are in a better position to test it. I would only hope you would also make the code consistent across EDK II. $ git grep -E 'mov\s+rax,\s*[defg]s' EdkCompatibilityPkg/Compatibility/BootScriptSaveOnS3SaveStateThunk/X64/AsmDispatchExecute.asm: mov rax, ds IntelFspWrapperPkg/Library/BaseFspApiLib/X64/Thunk64To32.asm: mov rax, ds MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm: mov rax, ds MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm: mov rax, es MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm: mov rax, fs MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm: mov rax, gs UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.asm: mov rax, ds UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.asm: mov rax, es UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.asm: mov rax, fs UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.asm: mov rax, gs $ git grep -E 'movw?\s+%[defg]s,\s*%rax' MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S: mov %ds, %rax MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S: movw %es, %rax MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S: mov %fs, %rax MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S: mov %gs, %rax SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/X64/AsmFuncs.S: movw %ds, %rax SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/X64/AsmFuncs.S: movw %es, %rax SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/X64/AsmFuncs.S: movw %fs, %rax SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/X64/AsmFuncs.S: movw %gs, %rax -Jordan > From: John Smith [mailto:[email protected]] > Sent: Sunday, December 14, 2014 08:26 > To: [email protected] > Cc: Tian, Feng > Subject: Re: [edk2] [PATCH] MdeModulePkg DebugSupportDxe: Fix build > error with GNU assembler > > > > Oops, and I meant Feng, not Jiang. Was reading some other email from a > Jiang... > > V/R > > JRS > > > > On Sat, Dec 13, 2014 at 7:25 PM, John Smith <[email protected]> > wrote: > > I don't know a way to reply to a thread that predates my being on the list > but for this thread (which I've copied the last message, circa Oct 22 > below) it's still relevant. Because I just got this error in an Ubuntu > 14.10 VM, and the easy fix was just to change the asm. I don't know why > Jiang thinks the patch could change the upper 48 bits. He's probably so > used to looking at instructions in Intel syntax that he got confused over > which direction the move was happening. > > Anyway, point is, you can't build UDK2014 SP1 on Ubuntu 14.10 (with gcc > 4.9.1) without changing this single instruction. I chose to make it an ax > instead of rax. 6 of one, half dozen of the other. > > > > V/R > > > > JRS > > > > From: Tian, Feng <feng.tian@in...> - 2014-10-22 08:55:08 > > Jordan, > > > > Your patch is: > > > > - movw %es, %rax > > > > + mov %es, %rax > > > > The disassemble code through https://defuse.ca/online-x86-assembler.htm > is (I change it from AT&T syntax to Masm): > > 66 8c c0 movw rax, es > > 48 8c c0 mov rax, es > > > > I am not sure if it will cause behavior change, such as RAX's high 48 bit > is cleaned to 0? At least the binary of instruction is changed. Could you > help confirm this? > > > > Thanks > > Feng > > > > From: Fan, Jeff > > Sent: Wednesday, October 22, 2014 3:54 PM > > To: Andrew Fish; edk2-devel@... > > Cc: Tian, Feng > > Subject: RE: [edk2] [PATCH] MdeModulePkg DebugSupportDxe: Fix > build error with GNU assembler > > > > I agree eax is better (0 extended and small size) > > > > Jordan, I could clean up the incorrect usage in other packages.:-) > > > > Jeff > > From: Andrew Fish [mailto:afish@...] > > Sent: Wednesday, October 22, 2014 2:55 PM > > To: edk2-devel@...<mailto:edk2-devel@...> > > Cc: Fan, Jeff; Tian, Feng > > Subject: Re: [edk2] [PATCH] MdeModulePkg DebugSupportDxe: Fix > build error with GNU assembler > > > > > > On Oct 21, 2014, at 11:27 PM, Jordan Justen > <jljusten@...<mailto:jljusten@...>> wrote: > > > > On Sun, Oct 19, 2014 at 7:09 PM, Fan, Jeff > <jeff.fan@...<mailto:jeff.fan@...>> wrote: > > SourceLevelDebugPkg/DebugAgentLib x64 also used movw %es, %rax. > > We need to fix it also. > > > > And UefiCpuPkg/Library/CpuExceptionLib x64 used movl %es, %eax. > > We could clean it to make a consistence in code base. > > > > More than that, it appears that the MASM code does a mov rax, es too. > > > > I'm not sure that mov ax, es is really much of an improvement, and > I'm > > not interested in cleaning all that code up. I'd just like > > MdeModulePkg to build. :) Would one of you be interested in > pursuing > > this cleanup? > > > > I'm also not sure, but it could be that the mov rax, es actually > does > > a zero extended move, so maybe the mov ax, es would be different. > > Would that impact the code that looks at this value later? > > > > Actually it looks like eax is the correct form for X64 code ... > > > > On the MASM side for X64 > https://defuse.ca/online-x86-assembler.htm > > mov rax, es > > mov eax ,es > > mov ax, es > > mov es, rax > > mov es, eax > > mov es, ax > > 0: 48 8c c0 mov rax,es > > 3: 8c c0 mov eax,es > > 5: 66 8c c0 mov ax,es > > 8: 48 8e c0 mov es,rax > > b: 8e c0 mov es,eax > > d: 8e c0 mov es,eax > > ax generates 0x66 - operand-size override prefix > > rax generates 0x48 - REX use 64-bit operand size. > > > > Anyway, I think that my original patch makes the code more > consistent > > with the current other GNU asm and MASM code. So, maybe it is okay > as > > a small next step? > ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
