Re: [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file

["Gao, Zhichao" <zhichao.gao@intel.com>]

Thanks for your clear check. I didn't reply all the comments.
The patch set is update to V7 base on the comments from the community.

Thanks,
Zhichao

> -----Original Message-----
> From: Justen, Jordan L
> Sent: Monday, April 1, 2019 2:33 PM
> To: Gao, Liming <liming.gao@intel.com>; Andrew Fish <afish@apple.com>;
> Andrew Fish via edk2-devel <edk2-devel@lists.01.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel <edk2-
> devel@lists.01.org>; Bret Barkelew <Bret.Barkelew@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>; Zeng, Star
> <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: Re: [edk2] [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to
> dsc file
> 
> Whoops. It looks like this was specifically addressed in v3 of the series. I just
> didn't see a follow up email discussion to Andrew's email.
> 
> Please disregard.
> 
> -Jordan
> 
> On 2019-03-31 23:17:12, Jordan Justen wrote:
> > On 2019-03-14 22:54:24, Andrew Fish via edk2-devel wrote:
> > > I understand the motivation for this change as I've done something
> > > much less portable that looks a lot like this to save the PEI XIP
> > > space....
> > >
> > > I seem to remember a long time ago we add a public VA_LIST to an API
> > > and we ran into an issue due to the marker format being compiler
> > > specific. You don't tend to see this on VC++ as it mostly uses a
> > > pointer to the frame, but GCC and clang can use a data structure
> > > especially not on IA32 (i386) for a VA_LIST (this is part of the
> > > reason there are so many #ifdefs in the Base.h VA_LIST section).
> > >
> > > In the past to fix this Mike Kinney added BASE_LIST. I seem to
> > > remember MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode
> passes
> > > a BASE_LIST into ReportStatusCode vs. a VA_LIST for this reason. Was
> > > this VA_LIST portability concern taken into consideration for this
> > > new API?
> > >
> > > In general VA_LIST is not an issue as long as all the code is
> > > compiled with the same compiler but if binaries are in play then you
> > > can have issues. So things like FSB, Option ROMs, OS Loaders, EFI
> > > Shell, EFI Apps. are when you can see the issue.
> > >
> >
> > I didn't see a reply to Andrew's concern. Was it considered, and
> > addressed?
> >
> > I remember having several issues in the past with var-args. I haven't
> > seen it show up in quite a while.
> >
> > I know we have to make sure to consistently apply EFIAPI to such
> > function declarations.
> >
> > I did think we still considered it forbidden to use var-args across a
> > binary interface. (PPI, Protocol, Core functions)
> >
> > I'm not sure if EFIAPI is enough to make it safe as long as all code
> > is compiled by the same compiler, or if it truely makes it safe for a
> > binary compiled with one compiler to be called from a binary compiled
> > with any other compiler.
> >
> > I'll add Ard and Laszlo to the Cc, as I seem to recall they may have
> > looked at it before. (Although, I think they've been Cc'd on the
> > series, so maybe they've already considered this, and don't expect
> > there is a problem.)
> >
> > -Jordan