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

["Bob Feng" <bob.c.feng@intel.com>]

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