Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04

["Ni, Ray" <ray.ni@intel.com>]

I didn't check with Bob about that patch.
I guess that was to fix a bug I found in BaseTools which didn't take care of the library override properly.
Or the rule of the library override was in a chaos that didn't seem to be an understood rule.

If some platforms happen to rely on this rule, I agree this is a risk.

So, I think a better solution is to evaluate the library instance mapping before and after the patch.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
> Sent: Thursday, August 25, 2022 11:44 PM
> To: devel@edk2.groups.io; quic_llindhol@quicinc.com; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; 'Ard Biesheuvel' <ardb@kernel.org>; rebecca@bsdio.com; Chen, Christine
> <yuwei.chen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: 'Andrew Fish' <afish@apple.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
> 
> Hi Bob,
> 
> There was feedback in the Bugzilla on July 5 asking if this is
> a DSC spec issue or a BaseTools implementation issue.  There was
> no response to that question.
> 
> Why was a spec change not considered for this case to match the
> BaseTools behavior?
> 
> Why were no additional details added to the BZ?
> 
> I agree with Leif.  If this change introduced regressions, we should
> consider a revert and evaluate again after the release.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
> > Sent: Thursday, August 25, 2022 1:49 AM
> > To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; 'Ard
> Biesheuvel'
> > <ardb@kernel.org>; rebecca@bsdio.com; Chen, Christine <yuwei.chen@intel.com>
> > Cc: 'Andrew Fish' <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> > Subject: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
> >
> > Hi Bob,
> >
> > You are suggesting to knowingly break builds between one stable tag and
> > another on a technicality, for one of the most common (and up to date)
> > build operating systems.
> >
> > The purpose of the stable tag is to avoid unplanned breakages. Catching
> > this sort of situation is literally what it is for.
> >
> > /
> >      Leif
> >
> > On 2022-08-24 18:17, Feng, Bob C wrote:
> > > Hi Liming,
> > >
> > > This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 was to fix the bug that the basetools implementation dose
> not follow the
> > dsc spec and pass the edk2 CI.
> > > The patch was send on June 30, reviewed on July 11 and pushed on July 17. I think there was enough time to give
> feedback to
> > this change.
> > >
> > > Why not consider to fix the DSC file?
> > >
> > > Thanks,
> > > Bob
> > >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming via groups.io
> > > Sent: Wednesday, August 24, 2022 3:16 PM
> > > To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io; 'Ard Biesheuvel' <ardb@kernel.org>;
> rebecca@bsdio.com; Chen,
> > Christine <yuwei.chen@intel.com>
> > > Cc: 'Andrew Fish' <afish@apple.com>; 'Leif Lindholm' <quic_llindhol@quicinc.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > > Subject: 回复: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
> > >
> > > Bob:
> > >   The commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 was merged into Edk2 on Last month (2022 July 17th). This
> is very new
> > change. It brings the behavior change. But, it doesn't highlight its impact to collect feedback.
> > >
> > >   I suggest to revert it, because I think we need more time to discuss this behavior change. Even if we decide to make
> this
> > change, we also need to give other people enough time to update DSC file.
> > >
> > > Thanks
> > > Liming
> > >> -----邮件原件-----
> > >> 发件人: Feng, Bob C <bob.c.feng@intel.com>
> > >> 发送时间: 2022年8月23日 15:40
> > >> 收件人: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>;
> > >> 'Ard Biesheuvel' <ardb@kernel.org>; rebecca@bsdio.com; Chen, Christine
> > >> <yuwei.chen@intel.com>
> > >> 抄送: 'Andrew Fish' <afish@apple.com>; 'Leif Lindholm'
> > >> <quic_llindhol@quicinc.com>; Kinney, Michael D
> > >> <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > >> 主题: RE: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu
> > >> 22.04
> > >>
> > >> Hi Liming,
> > >>
> > >> Reverting patch may not a good idea, some platforms have done the
> > >> implementation based on the DSC spec, if revert, those platforms build
> > >> will break.
> > >> This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 make the
> > >> Basetools behavior be consistent with DSC spec so I don't think it's a
> > >> regression bug.
> > >>
> > >> Thanks,
> > >> Bob
> > >>
> > >> -----Original Message-----
> > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > >> gaoliming via groups.io
> > >> Sent: Tuesday, August 23, 2022 9:55 AM
> > >> To: 'Ard Biesheuvel' <ardb@kernel.org>; devel@edk2.groups.io;
> > >> rebecca@bsdio.com; Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
> > >> <yuwei.chen@intel.com>
> > >> Cc: 'Andrew Fish' <afish@apple.com>; 'Leif Lindholm'
> > >> <quic_llindhol@quicinc.com>; Kinney, Michael D
> > >> <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> > >> Subject: 回复: [edk2-devel] MdeModulePkg build fails for AARCH64 on
> > >> Ubuntu 22.04
> > >> Importance: High
> > >>
> > >> Yuwei:
> > >>    The commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 (BaseTools:
> > >> Fix DSC LibraryClass precedence rule) introduces this issue. I agree
> > >> with Ard suggestion to revert this change first for this stable tag
> > >> 202208. Then, we can discuss this change in the detail.
> > >>
> > >> Thanks
> > >> Liming
> > >>> -----邮件原件-----
> > >>> 发件人: Ard Biesheuvel <ardb@kernel.org>
> > >>> 发送时间: 2022年8月22日 17:34
> > >>> 收件人: devel@edk2.groups.io; rebecca@bsdio.com; Feng, Bob C
> > >>> <bob.c.feng@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> > >>> 抄送: Andrew Fish <afish@apple.com>; Leif Lindholm
> > >>> <quic_llindhol@quicinc.com>; Michael D Kinney
> > >>> <michael.d.kinney@intel.com>; Jian J Wang <jian.j.wang@intel.com>
> > >>> 主题: Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu
> > >>> 22.04
> > >>>
> > >>> On Mon, 22 Aug 2022 at 11:30, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>>>
> > >>>> On Mon, 22 Aug 2022 at 11:11, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>>>>
> > >>>>> (cc Bob)
> > >>>>>
> > >>>>> NOTE this affects the stable tag - please see below
> > >>>>>
> > >>>>>
> > >>>>> On Sun, 21 Aug 2022 at 06:34, Rebecca Cran <rebecca@bsdio.com>
> > >>> wrote:
> > >>>>>>
> > >>>>>> I noticed that MdeModulePkg fails to build on my Ubuntu 22.04
> > >>>>>> system (and previously on the Ubuntu 20.04 installation). Is
> > >>>>>> this a
> > >> known bug?
> > >>>>>>
> > >>>>>> It shouldn't be relevant, but I'm using gcc version 11.2.0
> > >>>>>> (Ubuntu 11.2.0-17ubuntu1). But perhaps more relevant, I'm
> > >>>>>> using
> > >> Python 3.10.4.
> > >>>>>>
> > >>>>>> I'm trying to build commit
> > >>> e2ac68a23b4954d5c0399913a1df3dd9fd90315d.
> > >>>>>>
> > >>>>>>
> > >>>>>> bcran@photon:~/src/tmp/edk2$ build -p
> > >>> MdeModulePkg/MdeModulePkg.dsc -a
> > >>>>>> AARCH64 -t GCC5 -b RELEASE
> > >>>>>> Build environment:
> > >>>>>> Linux-5.15.0-46-generic-x86_64-with-glibc2.35
> > >>>>>> Build start time: 22:29:18, Aug.20 2022
> > >>>>>>
> > >>>>>> WORKSPACE        = /home/bcran/src/tmp/edk2
> > >>>>>> EDK_TOOLS_PATH   = /home/bcran/src/tmp/edk2/BaseTools
> > >>>>>> CONF_PATH        = /home/bcran/src/tmp/edk2/Conf
> > >>>>>> PYTHON_COMMAND   = /usr/bin/python3
> > >>>>>>
> > >>>>>>
> > >>>>>> Processing meta-data
> > >>>>>> .Architecture(s)  = AARCH64
> > >>>>>> Build target     = RELEASE
> > >>>>>> Toolchain        = GCC5
> > >>>>>>
> > >>>>>> Active Platform          =
> > >>>>>> /home/bcran/src/tmp/edk2/MdeModulePkg/MdeModulePkg.dsc
> > >>>>>>
> > >>>>>>
> > >>>>>> build.py...
> > >>>>>>
> > >>>
> > >> /home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmL
> > >>> ockBoxPeiLib.inf(42):
> > >>>>>> error 3000: PCD
> > >>>>>> [gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode] in
> > >>>>>>
> > >>>
> > >> [/home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmL
> > >>> ockBoxPeiLib.inf]
> > >>>>>> is not found in dependent packages:
> > >>>>>>
> > >> /home/bcran/src/tmp/edk2/MdePkg/MdePkg.dec
> > >>>>>>
> > >>> /home/bcran/src/tmp/edk2/MdeModulePkg/MdeModulePkg.dec
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>> This looks like a BaseTools regression to me - afaik, these
> > >>>>> packages all used to build for AARCH64 in the past.
> > >>>>>
> > >>>>> The following LockBoxLib resolutions appear in MdeModulePkg.dsc
> > >>>>> (with line numbers)
> > >>>>>
> > >>>>> 115-[LibraryClasses.common.PEIM]
> > >>>>> 119:
> > >>>
> > >> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
> > >>>>>
> > >>>>> 178-[LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >>>>> 181:
> > >>> LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
> > >>>>>
> > >>>>> No other [LlibraryClasses] references to SmmLockBoxPeiLib.inf
> > >>>>> exist in that file, and the [Components] reference does not
> > >>>>> apply to
> > >> AARCH64.
> > >>>>>
> > >>>>> This means the DSC parser ends up using the earlier definition,
> > >>>>> which I think is a bug, or at least a change in behavior. Note
> > >>>>> that doing this silently could potentially break many platforms
> > >>>>> in subtle ways so I think this should be root caused and
> > >>>>> preferably fixed before creating the stable tag.
> > >>>>
> > >>>> Seems to be deliberate:
> > >>>>
> > >>>> commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860
> > >>>> Author: Chen, Christine <Yuwei.Chen@intel.com>
> > >>>> Date:   Thu Jun 30 17:04:05 2022 +0800
> > >>>>
> > >>>>      BaseTools: Fix DSC LibraryClass precedence rule
> > >>>>
> > >>>> but I don't think the impact on other platforms and drivers has
> > >>>> been taken into account here. Also, the document [0] seems ambiguous to me:
> > >>>>
> > >>>> """
> > >>>> The first globally defined library instance, defined in a DSC
> > >>>> file, that satisfies a module's requirement for a Library Class,
> > >>>> unless specifically overridden by the module in the [Components]
> > >>>> section, will be used.
> > >>>> """
> > >>>>
> > >>>> This says that the first occurrence of a library instance
> > >>>> definition that satisfies a INF dependency will be selected. Then,
> > >>>> there is
> > >>>>
> > >>>> """
> > >>>> The Library Instances will be selected using the following rules
> > >>>> to satisfy a library class for each module listed in the
> > >>>> [Components] section (in order of highest precedence):
> > >>>> """
> > >>>>
> > >>>> which says that not the *first* occurrence, but the occurrence
> > >>>> *with the highest precedence* will be selected.
> > >>>>
> > >>>> Then, the precedence list has these items:
> > >>>>
> > >>>> """
> > >>>> 2. [LibraryClasses.$(Arch).$(MODULE_TYPE),
> > >>>> LibraryClasses.$(Arch).$(MODULE_TYPE)]
> > >>>> 3. [LibraryClasses.$(Arch).$(MODULE_TYPE)]
> > >>>> """
> > >>>>
> > >>>> which seem to suggest that a definition that applies to multiple
> > >>>> arch/module_type combinations automatically takes precedence over
> > >>>> one that targets a single arch/module_type combination, which
> > >>>> makes no sense at all.
> > >>>>
> > >>>> So I strongly suggest we revert the patch, fix the document so it
> > >>>> is clear and ambiguous, and preferably, align it with how it used
> > >>>> to
> > >>>
> > >>> *un*ambiguous ....
> > >>>
> > >>> sigh
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
>