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

["Long1 Huang" <long1.huang@intel.com>]

Hi All,

I'm the submitter. 

==================Here's the DSC precedence rule from DSC Spec===================
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):
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]
===========================================================================

Background:
Our system takes DXE and later stages as 64-bit by default, using [LibraryClasses.X64] or appending a more detailed section "MODULE_TYPE"(e.g. [LibraryClasses.X64.DXE_CORE], [LibraryClasses.X64.DXE_SMM_DRIVER]). For SEC or PEI,  use [LibraryClasses.IA32] or [LibraryClasses.IA32.SEC], [LibraryClasses.IA32.PEIM], etc. It works fine for traditional.

With the arrival of pure X64, there will be some linking issues in this way. Because which library to link against cannot be determined simply using ARCH. 
During the development of X64 PEI, we found that the actual behavior of BaseTools for DSC precedence:
[LibraryClasses.common.PEIM]   <   [LibraryClasses.X64]
[LibraryClasses.IA32.PEIM]           >   [LibraryClasses.X64]
[LibraryClasses.X64.PEIM]            >   [LibraryClasses.X64]

We can see that this does not match the order of 4 and 5 in the spec.

So when we tried to use "[LibraryClasses.common.MODULE_TYPE]" such the common ways to support both X64 and IA32 SEC & PEI and avoid duplication, before the fixed patch, modules in the PEI stage will linking DXE and later stage's libraries first(under [LibraryClasses.X64]), it will cause many puzzling linking issues. After the fixed patch downstream, we setup the X64 PEI build target, and works well for several days(both for IA32 and X64 SEC & PEI build).

For Ard Biesheuvel's question:
I think it may be the LockBoxLib under [LibraryClasses.ARM, LibraryClasses.AARCH64] doesn't provide the MODULE_TYPE section, it can work before this patch as I said above. But now it should be replaced with [LibraryClasses.ARM.MODULE_TYPE, LibraryClasses.AARCH64.MODULE_TYPE] to get higher priority than LockBoxLib under [LibraryClasses.comon.MODULE_TYPE].

Conclusion:
From my point of view, I think the DSC precedence rule defined by spec is reasonable and make sense. MinPlatform gives us a good example, using the method of [LibraryClasses.common.MODULE_TYPE]. Do not assume the current arch, use "common" instead, then always append "MODULE_TYPE" section. It's clear, compatible and unambiguous. So we strongly support the idea of the DSC spec: Unless specified under [Components], then more detailed sections, higher priority. I think it will benefit future development.

Short term solution:
we can agree to temporarily revert this patch for stable tag if we can't fix the MdeModulePkg build issues or other potential problems immediately. For us, we will downstream overwrite it to keep this change to support the X64 build target.

Long term solution:
We suggest that we can discuss the rationale of this rule and evaluate if this patch should be reintroduced after the stable tag.

Thanks,
Huang, Long

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: Friday, August 26, 2022 8:56 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; quic_llindhol@quicinc.com; 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>; Wang, Jian J <jian.j.wang@intel.com>; Huang, Long1 <long1.huang@intel.com>
Subject: RE: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04

+ Huang Long, the bug submitter.

I am OK to revert this patch for this stable tag.

Since this patch break the MdeModulePkg build, also suggest to add this case in edk2 CI.

Thanks,
Bob

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
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
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
>