Jordan: I add my comments on 3, 4, 5. -----Original Message----- From: Jordan Justen [mailto:jljus...@gmail.com] Sent: Wednesday, May 21, 2014 1:55 AM To: Gao, Liming Cc: edk2-devel@lists.sourceforge.net; edk2-buildtools-de...@lists.sourceforge.net Subject: Re: [edk2-buildtools] [PATCH 0/7] Build ResetVector/VTF with NASM during EDK II build
On Tue, May 20, 2014 at 1:54 AM, Gao, Liming <liming....@intel.com> wrote: > Jordan: > Thanks for your great contribution. I have some comments. > 1. Which NASM version is used to verify this patch? Good point. That should be documented. For now, I recommend we choose "Nasm 2.03 or newer" which matches UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt. The OldVtf source file might be compatible with earlier versions of NASM. > 2. Nasm-to-Binary-Code-File Rule is added to compile nasm to the > binary ResetVector file. It can replace current ASM16 build rule after > ASM16 source is converted to nasmbin. Right? Yes. One goal would be to replace asm16 with a solution that works on all supported build OSs. > 3. We don't suggest to use MACRO switch in source code. > How about separate nasmbin source file for the different ARCH and DEBUG? > If so, we will use the different nasmbin source files, -D ARCH_IA32, > -D ARCH_X64, -D DEBUG_NONE, -D DEBUG_SERIAL is not required. Do you mean you prefer not to use the -D in the .inf? Or, do you mean NASM macros? We might be able to use MDEPKG_NDEBUG after this change. Maybe it is best to just disable VTF0 debug by default, but let users modify the source to enable debug for those rare cases where it is needed. Then again, for real platforms, they might like to have the port 0x80 debug support enabled by default. (This will output 16/32/64/b0/b1/f0/f1 to the post card.) What do you think? [Liming] I suggest to remove ARCH_IA32, ARCH_X64, DEBUG_NONE, DEBUG_SERIAL macro from NASM and ASM source file. The different source files can be created for the different ARCH or DEBUG. UefiCpuPkg can provide two VTF0. One is generic one without DEBUG. Another is sample one with PORT80 DEBUG. The platform developer can add their one in their platform package. > 4. I see no difference between > UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmbin and > OvmfPkg/ResetVector/ResetVector.nasmbin except for their INF files. So, how > about keep one copy Vtf0.nasmbin, and add two VTF INF files in UefiCpuPkg. > One is DEBUG NONE, another is to support DEBUG on Serial. If so, OVMF > can directly use one from UefiCpuPkg. Hmm, I think I meant DEBUG_NONE in both places. One difference is that by having OvmfPkg/ResetVector/ResetVector.nasmbin, OvmfPkg/ResetVector/Ia32/PageTables64.asm overrides UefiCpuPkg/ResetVector/Vtf0/Ia32/PageTables64.asm. For OVMF we then assume RAM is initialized, and build page-tables in RAM, rather than in the flash device. [Liming] Here, I concern -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/ BuildOption in ResetVector.inf. EDKII design doesn't allow to append -I include option to refer other module internal files, which may bring the maintain effort. If OVMF requires source files in UefiCpuPkg, I prefer to duplicate them in OvmfPkg. > 5. For OldVtf, I don't think it is good name. Normally, we use it > within SecCore driver. How about name it as BaseVtf? One of the problems with the old VTF is that as far as I know it is not spec'd. Therefore, it is challenging to give it a name. BaseVtf doesn't seem quite right. What about FixupVtf or MinimalVtf? I still think "the old" VTF is not a bad name for it, since it never has had any sort of name before and it has been around for the longest. [Liming] I prefer FixupVtf. > 6. For the binary VTF files, some platform may use them. > So, I suggest to keep them for a while, then remove them. That seems reasonable. I kind of suspected we might choose to do that. > 7. I don't see any changes in source ASM files. > Does it mean they also follow NASM syntax? This change only uses the VTF0 source files, and the new OldVtf assembly file. VTF0 has always used NASM syntax, so no change was needed there. :) Thanks, -Jordan > -----Original Message----- > From: Jordan Justen [mailto:jordan.l.jus...@intel.com] > Sent: Tuesday, May 20, 2014 5:28 AM > To: edk2-devel@lists.sourceforge.net > Cc: edk2-buildtools-de...@lists.sourceforge.net > Subject: [edk2-buildtools] [PATCH 0/7] Build ResetVector/VTF with NASM > during EDK II build > > The first patch obviously needs to be made to edk2-buildtools/BaseTools > first, and then sync'd to EDK II. > > The first 6 patches change OVMF to build VTF0 during the EDK II build process > by running NASM. > > The UefiCpuPkg VTF0 patches are not ready yet, since the X64 page > tables are not being built. (They previously were built by > FixupForRawSection.py.) > > The last patch provides an implementation of the old style VTF which can be > built during the EDK II build process using NASM. > > These patches are available in the 'nasm-vtf' branch of the git repo at > https://github.com/jljusten/edk2. > > The web page for this branch also provides a .zip download: > https://github.com/jljusten/edk2/commits/nasm-vtf > > Jordan Justen (7): > BaseTools: Add rules to build NASM into a binary > UefiCpuPkg: Support building VTF0 ResetVector during the EDK II build > OvmfPkg: Support building OVMF's ResetVector during the EDK II build > OvmfPkg: Build OVMF ResetVector during EDK II build process > UefiCpuPkg VTF0: Remove pre-built binaries > OvmfPkg/ResetVector: Remove pre-built binaries > UefiCpuPkg: Add ResetVector/OldVtf > > BaseTools/Conf/build_rule.template | 19 +++- > BaseTools/Conf/tools_def.template | 8 +- > OvmfPkg/OvmfPkgIa32.dsc | 2 + > OvmfPkg/OvmfPkgIa32.fdf | 6 +- > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgIa32X64.fdf | 6 +- > OvmfPkg/OvmfPkgX64.dsc | 2 + > OvmfPkg/OvmfPkgX64.fdf | 6 +- > OvmfPkg/ResetVector/Bin/ResetVector.inf | 29 ------ > OvmfPkg/ResetVector/Bin/ResetVector.x64.raw | Bin 628 -> 0 bytes > OvmfPkg/ResetVector/Build.py | 58 ----------- > OvmfPkg/ResetVector/ResetVector.inf | 37 +++++++ > OvmfPkg/ResetVector/ResetVector.nasmbin | 53 ++++++++++ > OvmfPkg/ResetVector/Tools/FixupForRawSection.py | 26 ----- > UefiCpuPkg/ResetVector/OldVtf/Vtf.inf | 32 ++++++ > UefiCpuPkg/ResetVector/OldVtf/Vtf.nasmbin | 60 +++++++++++ > .../Vtf0/Bin/ResetVector.ia32.port80.raw | Bin 516 -> 0 bytes > .../ResetVector/Vtf0/Bin/ResetVector.ia32.raw | Bin 484 -> 0 bytes > .../Vtf0/Bin/ResetVector.ia32.serial.raw | Bin 884 -> 0 bytes > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf | 33 ------- > .../Vtf0/Bin/ResetVector.x64.port80.raw | Bin 28676 -> 0 bytes > .../ResetVector/Vtf0/Bin/ResetVector.x64.raw | Bin 28676 -> 0 bytes > .../Vtf0/Bin/ResetVector.x64.serial.raw | Bin 28676 -> 0 bytes > UefiCpuPkg/ResetVector/Vtf0/Build.py | 53 ---------- > .../ResetVector/Vtf0/Tools/FixupForRawSection.py | 110 > --------------------- > UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf | 36 +++++++ > UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmbin | 53 ++++++++++ > 27 files changed, 311 insertions(+), 320 deletions(-) delete mode > 100644 OvmfPkg/ResetVector/Bin/ResetVector.inf > delete mode 100644 OvmfPkg/ResetVector/Bin/ResetVector.x64.raw > delete mode 100644 OvmfPkg/ResetVector/Build.py create mode 100644 > OvmfPkg/ResetVector/ResetVector.inf > create mode 100644 OvmfPkg/ResetVector/ResetVector.nasmbin > delete mode 100644 OvmfPkg/ResetVector/Tools/FixupForRawSection.py > create mode 100644 UefiCpuPkg/ResetVector/OldVtf/Vtf.inf > create mode 100644 UefiCpuPkg/ResetVector/OldVtf/Vtf.nasmbin > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.port80.raw > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.raw > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.ia32.serial.raw > delete mode 100644 UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.x64.port80.raw > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.x64.raw > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.x64.serial.raw > delete mode 100644 UefiCpuPkg/ResetVector/Vtf0/Build.py > delete mode 100644 > UefiCpuPkg/ResetVector/Vtf0/Tools/FixupForRawSection.py > create mode 100644 UefiCpuPkg/ResetVector/Vtf0/Vtf0.inf > create mode 100644 UefiCpuPkg/ResetVector/Vtf0/Vtf0.nasmbin > > -- > 2.0.0.rc2 > > > ---------------------------------------------------------------------- > -------- "Accelerate Dev Cycles with Automated Cross-Browser Testing - > For FREE Instantly run your Selenium tests across 300+ browser/OS > combos. > Get unparalleled scalability from the best Selenium testing platform > available Simple to use. Nothing to install. Get started now for free." > http://p.sf.net/sfu/SauceLabs > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-de...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel > > ---------------------------------------------------------------------- > -------- "Accelerate Dev Cycles with Automated Cross-Browser Testing - > For FREE Instantly run your Selenium tests across 300+ browser/OS > combos. > Get unparalleled scalability from the best Selenium testing platform > available Simple to use. Nothing to install. Get started now for free." > http://p.sf.net/sfu/SauceLabs > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-de...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
