On 2016-06-20 16:28:04, Laszlo Ersek wrote: > On 06/20/16 10:07, Gao, Liming wrote: > > Laszlo: > > New changes: > > > > 1. Yes. nasm is listed first in BUILDRULEORDER so nasm source file > > will be used. > > > > 3. The files in MdePkg are auto converted. Most of the files in other > > packages manually are updated. They are updated based on the auto converted > > version. > > As I suspected, the manual conversion of the mode switches / mixed mode > code turned out to be a bit risky. I ran the following tests with OVMF > (all VMs used 4 or 8 VCPUs): > > Fedora guest: > > * Ia32 build with SMM_REQUIRE: S3 resume is broken. I'll follow up with > a patch for this. (It can be squashed into your corresponding patch, > but some credit in the commit message would be nice :) -- it wasn't > easy to find and fix this issue.) >
Since we didn't compare the .S output, I guess this means that the .S and .asm were producing different results. This must mean that a visual studio based build would not have worked with SMM_REQUIRE previously. Another reason to move to one set of source code for these functions. -Jordan > * Ia32 build without SMM_REQUIRE: S3 works fine. > > * Ia32X64 build with SMM_REQUIRE: S3 resume is broken. It has the same > error as the Ia32 build, but in two places. I'll follow up with > another patch for this. > > * Ia32X64 build without SMM_REQUIRE: S3 works fine. > > * X64 build with SMM_REQUIRE: in this case, S3Resume2Pei doesn't > support S3, so I disabled it on the QEMU command line, and tested > only cold-boot. It works fine. > > * X64 build without SMM_REQUIRE: S3 works fine. > > For the rest of the patches in the series: it seems they are fine (my > testing doesn't seem to have turned up any more errors), but I don't > think I'll figure out what patches exactly (from the 351 :)) I should > respond to with my Tested-by. > > Instead, I collected a list of object files that got built for my most > common tests. I'm attaching that list. For those patches that modify > files *only* on this list, if you guys are willing to track them down > (maybe with a script?), I'm okay to add my T-b, I guess. > > After applying my two fixup patches mentioned above, I did some more > tests with the Ia32X64 + SMM_REQUIRE build: > > - Tested key enrollment and SB enablement with the > "EnrollDefaultKeys.efi" app. (This used to trigger an SMM stack > overflow earlier, until we fixed it in 509f8425b75d^..0d0c245dfb14.) > It works fine. > > - Tested an S3 cycle with a Windows 8.1 guest. It's a PASS. > > Thanks > Laszlo > > > Verification > > > > 1. I compile .asm and .nasm source, and use VS dumpbin tool to dump > > the obj file to compare the disassembly code. > > > > Yes. I give the example here for window users. Most linux user doesn't > > need to configure NASM_PREFIX, because nasm is in system PATH. > > > > Thanks > > Liming > > From: Laszlo Ersek [mailto:ler...@redhat.com] > > Sent: Saturday, June 18, 2016 5:51 AM > > To: Gao, Liming <liming....@intel.com>; edk2-de...@ml01.01.org > > Cc: Justen, Jordan L <jordan.l.jus...@intel.com> > > Subject: Re: [edk2] [Patch 000/351] Convert EDK II core packages to NASM > > for IA32/X64 > > > > On 06/16/16 03:46, Liming Gao wrote: > >> These patches are available in https://github.com/lgao4/edk2.git nasm-v1. > >> The nasm-v1 branch sha1 is 42bec457c575b6cb7c9fc1730cbea263ffce7b1c > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Liming Gao > >> Signed-off-by: Jordan Justen > >> > >> These patches base on previous patches in > >> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/10977. > >> New changes in these patches includes: > >> 1. Keep ASM, GAS and NASM all. User can configure BUILDRULEORDER to specify > >> which one will be used. > > > > The current catch-all rule seems to be > > > > Conf/tools_def.template:*_*_*_*_BUILDRULEORDER = nasm asm Asm ASM S s > > > > I guess that should continue working for me, right? > > > >> 2. Add NASM in new modules from UefiCpuPkg, IntelFsp2Pkg and > >> IntelFsp2WrapperPkg. > >> 3. Use 16bit and 32bit assembly code to replace hard code db. > > > > This last step looks pretty error prone -- was this automated, or > > converted manually? > > > >> The done test includes: > >> 1. Verify the output obj file from ASM and NASM to make sure the output obj > >> have the same disassembly code. > > > > OTOH, binary comparison should make it pretty safe. > > > >> 2. Verify OvmfIa32X64 boot to Shell on VS2015x86. > >> 3. Verify OVMF X64 when booting to arch linux, and with S3 suspend/resume. > >> > >> These patches convert these package to use NASM for IA32 & X64 > >> * MdePkg > >> * MdeModulePkg > >> * IntelFrameworkModulePkg > >> * UefiCpuPkg > >> * SourceLevelDebugPkg > >> * PcAtChipsetPkg > >> * IntelFsp2Pkg > >> * IntelFsp2WrapperPkg > >> > >> Package maintainers: Could you help review and verify your package? If you > >> find > >> any issues, please let us know. > > > > I'm adding this to my TODO list. Hopefully I can play a little with this > > next week. > > > > Just to make sure, the above BUILDRULEORDER should ensure that the new > > NASM files will be utilized in my build, right? > > > >> Notes: these patches will cause NASM compiler to be required for all IA32 > >> and X64 > >> toolchains with the default tool configuration. > >> NASM compiler can be downloaded from http://www.nasm.us/ > >> After download it, please configure NASM_PREFIX env to point to its > >> directory. > >> For example, I place nasm.exe in C:\nasm directory, then set > >> NASM_PREFIX=C:\nasm\ > > > > (I think this advice is Windows-specific; for me the "nasm" binary is > > simply on PATH -- /usr/bin/nasm.) > > > > Thanks! > > Laszlo > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
