> -----Original Message----- > From: Kinney, Michael D > Sent: Wednesday, August 28, 2019 1:25 PM > To: Wu, Hao A; Leif Lindholm; Laszlo Ersek; Kinney, Michael D > Cc: devel@edk2.groups.io; Andrew Fish; Baptiste Gerondeau; Wang, Jian J; > Feng, Bob C; Gao, Liming > Subject: RE: [PATCH 1/1] MdeModulePkg: fix !x86 builds (more) > > Hao Wu, > > Please provide a patch to BaseTools to restore the previous behavior. > > We need that to review the complexity of the change to determine > what to do. Without that information today, the release date > of this stable tag is at risk.
Hello Mike, I confirmed with Bob that he is preparing a new BaseTools patch to restore the previous behavior. Best Regards, Hao Wu > > Mike > > > -----Original Message----- > > From: Wu, Hao A > > Sent: Tuesday, August 27, 2019 6:40 PM > > To: Leif Lindholm <leif.lindh...@linaro.org>; Laszlo > > Ersek <ler...@redhat.com>; Kinney, Michael D > > <michael.d.kin...@intel.com> > > Cc: devel@edk2.groups.io; Andrew Fish <af...@apple.com>; > > Baptiste Gerondeau <baptiste.gerond...@linaro.org>; Wang, > > Jian J <jian.j.w...@intel.com>; Feng, Bob C > > <bob.c.f...@intel.com>; Gao, Liming > > <liming....@intel.com> > > Subject: RE: [PATCH 1/1] MdeModulePkg: fix !x86 builds > > (more) > > > > > -----Original Message----- > > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org] > > > Sent: Wednesday, August 28, 2019 4:59 AM > > > To: Laszlo Ersek > > > Cc: devel@edk2.groups.io; Andrew Fish; Kinney, Michael > > D; Baptiste > > > Gerondeau; Wang, Jian J; Wu, Hao A; Feng, Bob C; Gao, > > Liming > > > Subject: Re: [PATCH 1/1] MdeModulePkg: fix !x86 builds > > (more) > > > > > > +Bob, Liming, > > > > > > On Tue, Aug 27, 2019 at 09:26:05PM +0200, Laszlo Ersek > > wrote: > > > > Hi Leif, > > > > > > > > On 08/27/19 14:43, Leif Lindholm wrote: > > > > > Commit 4a1f6b85c184 > > > > > ("MdeModulePkg: add LockBoxNullLib for !IA32/X64 in > > .dsc") added > > > > > an ARM/AARCH64 resolution for LockBoxLib. However, > > this failed to > > > > > address the overrides provided for PEIM, > > DXE_DRIVER, > > > > > DXE_RUNTIME_DRIVER, DXE_SMM_DRIVER and UEFI_DRIVER, > > so any > > > modules > > > > > of those classes still failed to build. > > > > > > > > > > Break these out properly into their own > > LibraryClasses sections. > > > > > > > > > > Resolves BZ: > > https://bugzilla.tianocore.org/show_bug.cgi?id=2134 > > > > > > > > > > Signed-off-by: Leif Lindholm > > <leif.lindh...@linaro.org> > > > > > Reported-by: Baptiste Gerondeau > > <baptiste.gerond...@linaro.org> > > > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > > > > Cc: Hao A Wu <hao.a...@intel.com> > > > > > --- > > > > > > > > > > I don't understand how the above would appear to > > work back when I > > > > > submitted the previous patch but not work now, but > > I haven't dug > > > > > into it deeper. Including the x86-specific > > LockBoxLib in the > > > > > .common section is however clearly not correct. > > > > > > > > I agree with you that the present situation is not > > correct. > > > > > > > > According to: > > > > > > > > https://edk2-docs.gitbooks.io/edk-ii-dsc- > > > > > specification/2_dsc_overview/26_[libraryclasses]_section_ > > processing.ht > > > ml > > > > > > > > the library class resolutions take effect in the > > following order > > > > (entries near the top have higher priority): > > > > > > > > > 1. <LibraryClasses> associated with the INF file in > > the > > > > > [Components] > > > section > > > > > 2. [LibraryClasses.$(Arch).$(MODULE_TYPE), > > > LibraryClasses.$(Arch).$(MODULE_TYPE)] > > > > > 3. [LibraryClasses.$(Arch).$(MODULE_TYPE)] > > > > > 4. [LibraryClasses.common.$(MODULE_TYPE)] > > > > > 5. [LibraryClasses.$(Arch)] > > > > > 6. [LibraryClasses.common] or [LibraryClasses] > > > > > > > > (Side comment 1: levels #2 and #3 look very similar; > > I think the > > > > difference is that #2 is supposed to be a multi-arch > > and/or > > > > multi-module-type section, while #3 is a single-arch > > and > > > > single-module-type section.) > > > > > > > > Commit 4a1f6b85c184 ("MdeModulePkg: add > > LockBoxNullLib for !IA32/X64 > > > in > > > > .dsc", 2019-03-27) provided a LockBoxLib resolution > > at level #5: > > > > > > Yes. > > > > > > > > [LibraryClasses.ARM, LibraryClasses.AARCH64] > > > > > > > > However, the other LockBoxLib resolutions are at > > level #4: > > > > > > > > > [LibraryClasses.common.PEIM] > > > > > [LibraryClasses.common.DXE_DRIVER] > > > > > [LibraryClasses.common.DXE_RUNTIME_DRIVER] > > > > > [LibraryClasses.common.DXE_SMM_DRIVER] > > > > > [LibraryClasses.common.UEFI_DRIVER] > > > > > > > > So the latter taking priority is actually specified > > behavior. > > > > > > Hmm. That's not great. > > > Anyway, I stopped being lazy and did a bisect. > > > > > > The culprit is > > > e8449e1d8e3b ("BaseTools: Decouple AutoGen Objects"), > > marked as > > > resolving > > https://bugzilla.tianocore.org/show_bug.cgi?id=1875. > > > > > > This also affects SignedCapsulePkg/SignedCapsulePkg.dsc > > (although once > > > addressed, AARCH64 also needs a NULL entry added for > > > CompilerIntrinsicsLib. > > > > > > > (Side comment 2: EBC is in the same boat, from commit > > cbcccd2c9d93 > > > > ("Update Code to pass EBC compiler", 2013-05-13): > > > > > > > > > [LibraryClasses.EBC] > > > > > > > > > > > > LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNul > > lLib.inf > > > > ) > > > > > > > > As to why this breakage was not exposed right at > > commit 4a1f6b85c184 > > > > -- I have no idea. Perhaps it was hidden by a > > BaseTools issue that > > > > has been fixed meanwhile. > > > > > > Yes. > > > But it is also a fundamental change in tool behaviour > > introduced on 9 > > > August. I am really uncomfortable about this making it > > into the > > > release this week - but I also believe this is the > > foundation for the > > > multiprocess autogen. > > > > > > Since you have very helpfully analyzed *what* changed > > ... would the > > > better "fix" for 2019.08 be to intentionally break the > > new code to > > > conform to the old behaviour - and then revert that > > patch after the > > > tag? > > > > > > If we do that, this patch could then wait and indeed be > > merged as part > > > of the same set. > > > > > > > On 08/27/19 14:43, Leif Lindholm wrote: > > > > > I think a fix for this issue needs to go into > > 2019.08, > > > > > > > > I agree the problem should be fixed in 2019.08 -- > > taking your word > > > > that commit 4a1f6b85c184 *appeared* to fix the > > MdeModulePkg.dsc > > > > build for ARM/AARCH64, we now have a regression since > > that commit > > > > (dated 2019-03-27). > > > > > > > > > but if someone has a prettier suggestion, I am not > > wedded to this one. > > > > > > > > I think this is good enough. The lib class > > resolutions are raised to > > > > level #2, but they will no longer match ARM / > > AARCH64, so your > > > > level#5 addition from commit 4a1f6b85c184 will take > > effect. > > > > > > > > > > > > > > MdeModulePkg/MdeModulePkg.dsc | 16 +++++++++++++-- > > - > > > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/MdeModulePkg/MdeModulePkg.dsc > > > b/MdeModulePkg/MdeModulePkg.dsc > > > > > index 4320839abfb5..15ba96cecbed 100644 > > > > > --- a/MdeModulePkg/MdeModulePkg.dsc > > > > > +++ b/MdeModulePkg/MdeModulePkg.dsc > > > > > @@ -109,6 +109,8 @@ [LibraryClasses.common.PEIM] > > > > > HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > > > > > > > > > > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib > > /PeiMemory > > > AllocationLib.inf > > > > > > > > > > ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSe > > ctionLib/PeiE > > > ExtractGuidedSectionLib|x > > > tractGuidedSectionLib.inf > > > > > + > > > > > +[LibraryClasses.IA32.PEIM,LibraryClasses.X64.PEIM] > > > > > > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxP > > eiLib.inf > > > > > > > > (1) I suggest replacing "," with ", ". (That's more > > consistent with > > > > preexistent section headers in the DSC file.) Applies > > to the other > > > > new section headers too. > > > > > > Yes, fair point. > > > > > > > > > > > > > [LibraryClasses.common.DXE_CORE] > > > > > @@ -118,18 +120,22 @@ > > [LibraryClasses.common.DXE_CORE] > > > > > > > > > > [LibraryClasses.common.DXE_DRIVER] > > > > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > > > > - > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxD > > xeLib.in > > > f > > > > > > > > > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLi > > b/UefiMemo > > > ryAllocationLib.inf > > > > > > > > > > ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSe > > ctionLib/DxeE > > > xtractGuidedSectionLib.inf > > > > > > > > > > CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsu > > leLib.inf > > > > > > > > > > > > +[LibraryClasses.IA32.DXE_DRIVER,LibraryClasses.X64.DXE_D > > RIVER] > > > > > + > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxD > > xeLib.in > > > f > > > > > + > > > > > [LibraryClasses.common.DXE_RUNTIME_DRIVER] > > > > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > > > > > > > > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLi > > b/UefiMemo > > > ryAllocationLib.inf > > > > > > > > > > DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibCo > > nOut.inf > > > > > - > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxD > > xeLib.in > > > f > > > > > > > > > > CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRunti > > meCapsule > > > Lib.inf > > > > > > > > > > > > > > > +[LibraryClasses.IA32.DXE_RUNTIME_DRIVER,LibraryClasses.X > > 64.DXE_RUNTI > > > ME_DRIVER] > > > > > + > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxD > > xeLib.in > > > f > > > > > + > > > > > [LibraryClasses.common.SMM_CORE] > > > > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > > > > > > > > > MemoryAllocationLib|MdeModulePkg/Library/PiSmmCoreMemoryA > > llocatio > > > nLib/PiSmmCoreMemoryAllocationLib.inf > > > > > @@ -143,13 +149,17 @@ > > [LibraryClasses.common.DXE_SMM_DRIVER] > > > > > > > > > > MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib > > /SmmMe > > > moryAllocationLib.inf > > > > > > > > > > MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmSe > > rvicesTabl > > > eLib.inf > > > > > > > > > > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/Sm > > mServicesT > > > ableLib.inf > > > > > + SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > > > > > + > > > > > > > > > > +[LibraryClasses.IA32.DXE_SMM_DRIVER,LibraryClasses.X64.D > > XE_SMM_DRI > > > VER] > > > > > > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxS > > mmLib.i > > > nf > > > > > - SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > > > > > > > > > > > > > I wonder if this is really necessary. I would assume > > all the > > > > DXE_SMM_DRIVER modules are listed under > > > > > > > > [Components.IA32, Components.X64] > > > > > > > > already. But, this hunk certainly doesn't hurt. > > > > > > Well, this fixes the current issue. I completely agree > > the file could > > > benefit from some overall restructuring. > > > > > > > > [LibraryClasses.common.UEFI_DRIVER] > > > > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > > > > > > > > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLi > > b/UefiMemo > > > ryAllocationLib.inf > > > > > > > > > > DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibCo > > nOut.inf > > > > > + > > > > > > > +[LibraryClasses.IA32.UEFI_DRIVER,LibraryClasses.X64.UEFI > > _DRIVER] > > > > > > > > > > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxD > > xeLib.in > > > f > > > > > > > > > > [LibraryClasses.common.UEFI_APPLICATION] > > > > > > > > > > > > > With (1) fixed (feel free to correct that just before > > pushing): > > > > > > > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > > > > > Thanks! > > > > > > > Do wait for maintainer review, of course. > > > > > > Of course. > > > > > > Per my understanding to the analysis from Leif, Laszlo > > and Mike, the patch will depend on the precedence of the > > below rules: > > > > * [LibraryClasses.common.$(MODULE_TYPE)] > > * [LibraryClasses.$(Arch)] > > > > So for now, we should wait for the final call on the > > above open, right? > > > > Best Regards, > > Hao Wu > > > > > > > > > > Best Regards, > > > > > > Leif -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46527): https://edk2.groups.io/g/devel/message/46527 Mute This Topic: https://groups.io/mt/33045351/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
