Re: [Edk2 Dev] EDK 1 vs. 2 module map

Subject: Re: [Edk2 Dev] EDK 1 vs. 2 module map

From: David Elliott <dfe@tgwbd.org>

To: dev@edk2.tianocore.org

Date: 2008-04-18 10:48:01


On Apr 18, 2008, at 1:25 AM, Lu, Ken wrote:
> Hi, David:
>   I am very appreciating your detail analysis for building issue in
> *inux environment. Currently python tool does not fully support *inux
> building, so many modules maybe contains the issue you just  
> mentioned in
> last email. I think you analysis is good example for other modules.

Yes, when dealing with assembler files there doesn't seem to be much  
recourse but to write it twice, once for Microsoft/Intel assemblers  
and again for GNU (AT&T syntax) style assemblers.  For some things  
(and the CpuIoAccess was a good example) GCC inline assembly is  
preferable to a plain .S file since the interface is already designed  
to be C callable.  You will find that nearly identical code to what I  
wrote is present in nearly all open UNIX-like systems.

For other things it will probably be inevitable that regular assembler  
files will have to be used.  I am somewhat torn as to whether to do  
EnterDxeCore as a pure .S file or using GCC inline assembler.  Multi- 
line asm statements are possible (just separate lines with \n\t) and  
it's also possible to use "memory operands" so that the stack  
parameters can be declared explicitly.  For example, %0 ought to  
result in 0xXX(%ebp) thus a statement like mov %0, %%eax will be  
converted to mov 0xXX(%ebp), %eax in the resulting interim assembler  
file that the compiler generates and passes on to the assembler.  The  
only catch is that EnterDxeCore would ultimately have to depend on  
this since it sets esp.  If the source file were ever compiled with - 
fomit-frame-pointer, all hell would break loose as GCC would replace  
%0 with 0xYY(%esp) since there would be no frame pointer (ebp)  
register to base from.  So I feel that in the EnterDxeCore case, a  
pure assembler file is probably preferable.

>   I have applied your patch to code base, I just change a little for
> patch that I add tool tag for source file in CpuIo.inf as following:
>  Ia32/CpuIoAccess.asm  | MSFT   <== MSFT means Microsoft tool will use
> this file for building
>  Ia32/CpuIoAccessGNU.c | GCC    <== GNU tool chain will use this file
> for building.
>
It's useful to know that I can just add |TOOLCHAIN to a given line to  
do this and not have to create different sections or anything like that.

>  I also add three missing module: FsVariable, EfiLdr and Legacy8259
> into our code base.
Excellent.

>  SecPeiDxeTimerLibCpu library instance is not correct library instance
> for DXE_RUNTIME_DRIVER, I just pick it for building in porting, you  
> are
> correct, it break building, so I replace it as
> BaseTimerLibNullTemplate.inf. As you known, library class and instance
> is new thing introduced by EDK2, so after success building, we need
> review whether the library instances in DSC file is correctly.
>
I think I understand how EDK2 handles this, I haven't quite figured  
out how EDK1 managed to find which particular implementation of a  
library to use.  I guess there used to only ever be one implementation?

>  For the line ending style, I do not catch your idea totally, Do you
> mean window style line ending CRLF will break compiling or your  
> editor?
> I guess many editors whether in window or Unix can be configured for
> this staff. Before python tool is used for edk2, we ever use java  
> tool,
> EdkUnixPkg is developed in linux environment by Tristan Gingold. Could
> you please explain the detail "terribly annoying" when your create FDF
> file?
>  And do you mean our python tool requires CRLF, I also include the
> people who is the right person for answering your this question in  
> this
> email.
>

Some compilers (fortunately modern GCC does not seem to be among them)  
choke on CRLF for certain things.  Consider when you are doing a  
preprocessor macro with line continuation.  The preprocessor sees a  
backslash and then a CR.  Since it treats CR as regular whitespace, it  
just escapes it which does nothing.  Then a LF is present which  
terminates the line.  The backslash is not used as a line continuation  
and so the code fails to compile correctly.  These days, it's not so  
much a concern, but it's not really a great idea to have source files  
with the non-native (LF only) line endings on UNIX in general.

Then there is the issue of diffs.  You probably did not notice but the  
diff-supplied lines (e.g. Index: foo, --- foo ..., +++ foo ...) all  
terminated with pure LF because that is what UNIX diff does.  But then  
the actual files being diffed contained carriage returns.  So the  
effect here is that the diff command considers that CR to be a literal  
part of the text which isn't really appropriate.  When viewing it in  
(g)vim you see a bright-blue ^M control character at the end of each  
source line.

As for the python tools, yes, they only handle CRLF.  Creating a new  
file with a normal UNIX text editor will typically result in a file  
with LF-only line endings.  The Python tools (or at least the fdf  
parser, since this is the one I tested) cannot handle this and give  
very strange diagnostics.  Changing the file to CRLF line endings  
fixes the problem.  I consider this to be a bug as the fdf (and other)  
files ought to have native line endings, not CRLF on every platform.   
That said, the tools should also be smart enough to handle both CRLF  
and LF-only. Python has various functions and libraries to make this  
easier.

It should also be noted that EDK1 does have svn:eol-style set to  
native on most files, but not ones that are platform-specific  
(e.g. .bat files or Microsoft nmake Makefile).  What svn:eol-style  
native does is tell the svn server and client that when the file is  
checked out it should take on the platform-native (on the client side  
obviously) line endings.  On the server side the file is stored with  
svn normal line endings (LF-only).  When you don't set the property  
the file is stored as-is and so when UNIX users check out a file  
committed by a Windows user, the UNIX user sees CRLF line endings  
which while not the end of the world (well, it is with some compilers  
as mentioned above) is still annoying.

Anyway, in the grand scheme of things there are bigger fish to fry but  
it'd be nice if a UNIX checkout of EDK2 actually had UNIX source files  
and not Windows source files.

-Dave

> Hi, Wang, Jean:
>   Do you notice the CRLF issue mentioned by David?
>
>   Thanks.
>
>
> - Ken
>
> Best Regard!
>
> ------------------------------------------------------------------------
> ------------
>
> Hear and you forget; see and you remember; do and you understand.
>
> "The content of this message is my personal opinion only and  
> although I
> am an employee of Intel, the statements I make here in no way  
> represent
> Intel's position on the issue, nor am I authorized to speak on  
> behalf of
> Intel on this matter"
>
> -----Original Message-----
> From: David Elliott [mailto:dfe@tgwbd.org]
> Sent: Thursday, April 17, 2008 4:55 PM
> To: dev@edk2.tianocore.org
> Subject: Re: [Edk2 Dev] EDK 1 vs. 2 module map
>
> Hi Ken,
>
> Great!  I have a few minor issues just running a build:
>
> Unix specific:
> 1. Several paths use \ instead of / as a path separator.
>   DuetPkg/CpuIoDxe/CpuIo.inf
>   DuetPkg/DxeIpl/DxeIpl.inf
>   DuetPkg/KbcResetDxe/Reset.inf
>
> 2. A few paths have bad case (IA32 in inf vs. Ia32 on FS).
>
>
> GCC warnings (which are errors since -Werror is used):
> 1. PrintString and DebugSerialPrint in the DxeIpl module take a UINT8*
> instead of a CHAR8*.  This means that you cannot pass a simple literal
> string (e.g. "foobar") to them without using a cast or you will get a
> warning.  I assume these should simply take CHAR8* or better yet CHAR8
> CONST* (or CONST CHAR8* which is the same thing but more commonly  
> seen).
>
> 2. Several constants have values that will only fit in a 64-bit type.
> It seems that IA32 EFI uses the normal ILP32 data model so long long
> and hence the LL constant suffix must be used for these.  Is there
> some macro to handle this like a CONST64(0x0123456789abcdef) which
> will apply the LL suffix in 32-bit compilations to force a 64-bit
> constant?
>   ACPI_RSD_PTR in DuetPkg/DxeIpl/LegacyTable.c
>   Hardcoded 4GB (0x100000000LL) in DuetPkg/DxeIpl/HobGeneration.c
>
> 3. DxeIpl.h does not end with a newline.  GCC warns about this..
>
> 4. The LocateProtocol boot service takes a VOID** output.  SomeType**
> does not implicitly cast to VOID** without a warning from GCC.  So
> instead of &gDataHub or &gHii you must use (VOID**)&gDataHub and
> (VOID**)&gHii.
>   DuetPkg/DataHubGenDxe/DataHubGen.c
>
> 5. In MdeModulePkg/Universal/DevicePathDxe/DevicePathToText.c the
> UnpackDevicePath static function should take a CONST pointer to
> DevPath.  Likewise its Src automatic variable should be a CONST
> pointer.  There's a few other of these in the Hii code as well.
>
>
> DUET specific:
> 1. The following modules do not exist in svn:
>   DuetPkg/FSVariable/FSVariable.inf
>   DuetPkg/FvbRuntimeService/DUETFwh.inf
>   DuetPkg/EfiLdr/EfiLdr.inf
>   IntelFrameworkModulePkg/Universal/Legacy8259Dxe/8259.inf
>
> 2. SecPeiDxeTimerLibCpu does not declare itself as being able to be
> used from a DXE_RUNTIME_DRIVER but you now have DxeIpl being compiled
> as such.  Is DXE_RUNTIME_DRIVER really appropriate?  Would DXE_DRIVER
> be more appropriate?  I am still trying to figure out exactly how
> these types affect the build system.
>
> 2. None of the .asm files are compiled at all since they can't be
> assembled by the GNU assembler.  The most obvious omission is IO port
> read/write.  I have attached a simple GNU C file that can be added to
> the build of DuetPkg/CpuIoDxe.  It's wrapped in a simple #ifdef
> __GNUC__ and since the build tools automatically fail to compile .asm
> files it simply gets used in lieu of that.  Presumably it will simply
> compile to an empty object with MSVC or you can add it to a GCC-
> specific section of the INF.
>
> 3. DxeIpl has its own CPU I/O routines.  You can fix this by copying
> the new .c file and the CpuIoAccess.h file into DxeIpl/Ia32  and
> adding it to the build.  It's possibly just not worth separating this
> into a library.
>
> 4. Along the same lines, DxeIpl has a EnterDxeCore.asm I'll have to
> rewrite in GNU assembler.  No biggie, just haven't gotten there yet.
>
> Other notes:
> It is extremely annoying that none of the files in the SVN repository
> have svn:eol-style set.  Typically you'll want to set svn:eol-style to
> native for at least any .c and .h files.  It would probably also be a
> wise idea to do the same for .dsc, .inf, .fdf, and other build system
> files.  However, BEFORE you do that, be sure to fix the Python build
> tools to handle it.  Right now, they require CRLF line endings even on
> UNIX systems (Mac OS X in my case) which is terribly annoying when you
> go to create a new file (e.g. the temporary .fdf I'd created).
>
> How exactly to properly convert the svn repository is unclear to me.
> But I believe you want to consider that the files currently have CRLF
> line endings and thus you probably want to apply the property from a
> Windows system with a Windows, and not Cygwin, svn.  That said, I do
> not know if that will cause all lines to change or if you still have
> to do something more to get that to happen.
>
> Also note you should not convert any files that are natively CRLF.
> For instance, Microsoft makefiles are not cross-platform so there's no
> advantage to making them use the native line endings when checked out
> on other platforms.  In fact, there's a huge disadvantage since it
> prevents you from checking out using a UNIX svn client and then using
> the working copy from a Windows system (e.g. over a Samba share).
> It's not a problem for C files though since Microsoft's compilers
> handle LF just as well as CRLF.
>
> Perhaps I should put that in a separate list mail.  Has anyone thought
> of this or is it just that most (all?) of the Intel guys are using
> Win32 so they haven't noticed?
>
> -Dave
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@edk2.tianocore.org
> For additional commands, e-mail: dev-help@edk2.tianocore.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@edk2.tianocore.org
For additional commands, e-mail: dev-help@edk2.tianocore.org