Laszlo:
Thanks for your test and finding. I notice this difference between asm and
nasm output file. I read IA32 manual to try understanding 66 and 67 prefix
operand. I wrongly think 66 has the same functionality to 66 67. So, I think
they are same. I will go through all patches to make sure there is no other 67
missing.
ASM dump:
00000032: 66 67 EA 00 00 00 jmp _RendezvousFunnelProc
00
NASM dump:
00000032: 66 EA 00 00 00 00 jmp _RendezvousFunnelProc
Thanks
Liming
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Tuesday, June 21, 2016 9:21 AM
> To: Laszlo Ersek <ler...@redhat.com>; Justen, Jordan L
> <jordan.l.jus...@intel.com>; Gao, Liming <liming....@intel.com>; Kinney,
> Michael D <michael.d.kin...@intel.com>
> Cc: edk2-de...@ml01.01.org
> Subject: RE: [edk2] [Patch 000/351] Convert EDK II core packages to NASM
> for IA32/X64
>
> Liming,
>
> Where these 2 files missed in your binary compare of output?
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> > Sent: Monday, June 20, 2016 5:59 PM
> > To: Justen, Jordan L <jordan.l.jus...@intel.com>; Gao, Liming
> <liming....@intel.com>
> > Cc: edk2-de...@ml01.01.org
> > Subject: Re: [edk2] [Patch 000/351] Convert EDK II core packages to NASM
> for IA32/X64
> >
> > On 06/21/16 02:31, Jordan Justen wrote:
> > > 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.
> >
> > The 0x67 address-size prefixes that were missing from the NASM-
> generated
> > machine code, and that I had to fix up in the manually converted NASM
> > source, are open-coded in *both* .S and .asm.
> >
> > When I found these errors, I was really surprised, because a binary
> > comparison against the machine code built from the original, unconverted
> > source should have found them. Therefore I assumed that the binary
> > comparison that you guys did didn't cover these specific files.
> >
> > If that's not the case, then there is only one possible explanation:
> > from the same NASM source code, my nasm (nasm-2.10.07-7.el7.x86_64)
> > generates different machine code from yours.
> >
> > Thanks
> > Laszlo
> >
> > > 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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
