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

From: Andrew Fish <afish@apple.com>
To: Zhichao Gao <zhichao.gao@intel.com>,
	Mike Kinney <michael.d.kinney@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>
Cc: edk2-devel <edk2-devel@lists.01.org>,
	Bret Barkelew <Bret.Barkelew@microsoft.com>,
	Hao Wu <hao.a.wu@intel.com>,
	Michael Turner <Michael.Turner@microsoft.com>,
	Star Zeng <star.zeng@intel.com>
Subject: Re: [PATCH V2 17/17] MdeModulePkg: Add PEIM and lib to dsc file
Date: Thu, 14 Mar 2019 22:54:24 -0700	[thread overview]
Message-ID: <46E887C8-DAAB-4855-BB70-69066F6B45FD@apple.com> (raw)
In-Reply-To: <20190315051749.6564-18-zhichao.gao@intel.com>

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. 

Thanks,

Andrew Fish

> On Mar 14, 2019, at 10:17 PM, Zhichao Gao <zhichao.gao@intel.com> wrote:
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> 
> Add the new PEIM DebugServicePei and library instance
> PeiDebugLibDebugPpi to dsc file to verify it can build
> correctly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
> MdeModulePkg/MdeModulePkg.dsc | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> index 6cd1727a0d..dec441e23e 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -297,6 +297,7 @@
>   MdeModulePkg/Library/PlatformHookLibSerialPortPpi/PlatformHookLibSerialPortPpi.inf
>   MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
>   MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> +  MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDebugPpi.inf
>   MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>   MdeModulePkg/Library/PlatformBootManagerLibNull/PlatformBootManagerLibNull.inf
>   MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
> @@ -423,6 +424,8 @@
>   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
>   MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
> 
> +  MdeModulePkg/Universal/DebugServicePei/DebugServicePei.inf
> +
>   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
>   MdeModulePkg/Library/FmpAuthenticationLibNull/FmpAuthenticationLibNull.inf
>   MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> -- 
> 2.16.2.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel