public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
@ 2022-08-21  4:34 Rebecca Cran
  2022-08-22  9:11 ` [edk2-devel] " Ard Biesheuvel
  0 siblings, 1 reply; 17+ messages in thread
From: Rebecca Cran @ 2022-08-21  4:34 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Jian J Wang,
	Liming Gao

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/SmmLockBoxPeiLib.inf(42): 
error 3000: PCD 
[gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode] in 
[/home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf] 
is not found in dependent packages:
                 /home/bcran/src/tmp/edk2/MdePkg/MdePkg.dec
         /home/bcran/src/tmp/edk2/MdeModulePkg/MdeModulePkg.dec


- Failed -
Build end time: 22:29:19, Aug.20 2022
Build total time: 00:00:01


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-21  4:34 MdeModulePkg build fails for AARCH64 on Ubuntu 22.04 Rebecca Cran
@ 2022-08-22  9:11 ` Ard Biesheuvel
  2022-08-22  9:30   ` Ard Biesheuvel
  0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2022-08-22  9:11 UTC (permalink / raw)
  To: devel, rebecca, Feng, Bob C, Liming Gao
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Jian J Wang

(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/SmmLockBoxPeiLib.inf(42):
> error 3000: PCD
> [gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode] in
> [/home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-22  9:11 ` [edk2-devel] " Ard Biesheuvel
@ 2022-08-22  9:30   ` Ard Biesheuvel
  2022-08-22  9:34     ` Ard Biesheuvel
  0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2022-08-22  9:30 UTC (permalink / raw)
  To: devel, rebecca, Feng, Bob C, Liming Gao
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Jian J Wang

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/SmmLockBoxPeiLib.inf(42):
> > error 3000: PCD
> > [gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode] in
> > [/home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.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
work.




[0] https://edk2-docs.gitbook.io/edk-ii-dsc-specification/2_dsc_overview/26_-libraryclasses-_section_processing

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-22  9:30   ` Ard Biesheuvel
@ 2022-08-22  9:34     ` Ard Biesheuvel
  2022-08-23  1:54       ` 回复: " gaoliming
  0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2022-08-22  9:34 UTC (permalink / raw)
  To: devel, rebecca, Feng, Bob C, Liming Gao
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Jian J Wang

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/SmmLockBoxPeiLib.inf(42):
> > > error 3000: PCD
> > > [gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode] in
> > > [/home/bcran/src/tmp/edk2/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* 回复: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-22  9:34     ` Ard Biesheuvel
@ 2022-08-23  1:54       ` gaoliming
  2022-08-23  7:40         ` Bob Feng
  0 siblings, 1 reply; 17+ messages in thread
From: gaoliming @ 2022-08-23  1:54 UTC (permalink / raw)
  To: 'Ard Biesheuvel', devel, rebecca, 'Feng, Bob C',
	'Yuwei Chen'
  Cc: 'Andrew Fish', 'Leif Lindholm',
	'Michael D Kinney', 'Jian J Wang'

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-23  1:54       ` 回复: " gaoliming
@ 2022-08-23  7:40         ` Bob Feng
  2022-08-24  7:16           ` 回复: " gaoliming
  2022-08-24  7:49           ` Ard Biesheuvel
  0 siblings, 2 replies; 17+ messages in thread
From: Bob Feng @ 2022-08-23  7:40 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming, 'Ard Biesheuvel',
	rebecca@bsdio.com, Chen, Christine
  Cc: 'Andrew Fish', 'Leif Lindholm', Kinney, Michael D,
	Wang, Jian J

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








^ permalink raw reply	[flat|nested] 17+ messages in thread

* 回复: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-23  7:40         ` Bob Feng
@ 2022-08-24  7:16           ` gaoliming
  2022-08-24 17:17             ` Bob Feng
  2022-08-24  7:49           ` Ard Biesheuvel
  1 sibling, 1 reply; 17+ messages in thread
From: gaoliming @ 2022-08-24  7:16 UTC (permalink / raw)
  To: 'Feng, Bob C', devel, 'Ard Biesheuvel', rebecca,
	'Chen, Christine'
  Cc: 'Andrew Fish', 'Leif Lindholm',
	'Kinney, Michael D', 'Wang, Jian J'

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
> 
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-23  7:40         ` Bob Feng
  2022-08-24  7:16           ` 回复: " gaoliming
@ 2022-08-24  7:49           ` Ard Biesheuvel
  2022-08-24 10:14             ` Ard Biesheuvel
  1 sibling, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2022-08-24  7:49 UTC (permalink / raw)
  To: devel, bob.c.feng
  Cc: Gao, Liming, rebecca@bsdio.com, Chen, Christine, Andrew Fish,
	Leif Lindholm, Kinney, Michael D, Wang, Jian J

On Tue, 23 Aug 2022 at 09:41, Bob Feng <bob.c.feng@intel.com> wrote:
>
> 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.

Platforms implemented against the DSC spec would have never worked if
they relied on behavior that BaseTools currently does not implement.

> This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 make the Basetools behavior be consistent with DSC spec so I don't think it's a regression bug.
>

Platforms built against the current behavior that used to work might
break after this change. MdeModulePkg.dsc for ARM/AARCH64 no longer
builds due to this change.

Also, as I pointed out, the DSC spec is far from ambiguous. This means
that platforms implemented against the spec will rely on one of
several possible interpretations of that ambiguous set of rules.

So let's revert this change now. Let's fix the DSC spec next, And only
then, let's fix the code (assuming it still needs to be fixed)

-- 
Ard.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-24  7:49           ` Ard Biesheuvel
@ 2022-08-24 10:14             ` Ard Biesheuvel
  2022-08-25  2:04               ` 回复: [edk2-devel][edk2-stable202208] " gaoliming
  0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2022-08-24 10:14 UTC (permalink / raw)
  To: devel, bob.c.feng
  Cc: Gao, Liming, rebecca@bsdio.com, Chen, Christine, Andrew Fish,
	Leif Lindholm, Kinney, Michael D, Wang, Jian J

On Wed, 24 Aug 2022 at 09:49, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 23 Aug 2022 at 09:41, Bob Feng <bob.c.feng@intel.com> wrote:
> >
> > 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.
>
> Platforms implemented against the DSC spec would have never worked if
> they relied on behavior that BaseTools currently does not implement.
>
> > This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 make the Basetools behavior be consistent with DSC spec so I don't think it's a regression bug.
> >
>
> Platforms built against the current behavior that used to work might
> break after this change. MdeModulePkg.dsc for ARM/AARCH64 no longer
> builds due to this change.
>
> Also, as I pointed out, the DSC spec is far from ambiguous.

Ugh I did it again ...

"far from *un*ambiguous"

Will use less difficult words from now on - I obviously cannot be
trusted with them.

> This means
> that platforms implemented against the spec will rely on one of
> several possible interpretations of that ambiguous set of rules.
>
> So let's revert this change now. Let's fix the DSC spec next, And only
> then, let's fix the code (assuming it still needs to be fixed)
>
> --
> Ard.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-24  7:16           ` 回复: " gaoliming
@ 2022-08-24 17:17             ` Bob Feng
  2022-08-25  8:48               ` Leif Lindholm
  0 siblings, 1 reply; 17+ messages in thread
From: Bob Feng @ 2022-08-24 17:17 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming, 'Ard Biesheuvel',
	rebecca@bsdio.com, Chen, Christine
  Cc: 'Andrew Fish', 'Leif Lindholm', Kinney, Michael D,
	Wang, Jian J

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
> 
> 
> 
> 
> 
> 









^ permalink raw reply	[flat|nested] 17+ messages in thread

* 回复: [edk2-devel][edk2-stable202208] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-24 10:14             ` Ard Biesheuvel
@ 2022-08-25  2:04               ` gaoliming
  0 siblings, 0 replies; 17+ messages in thread
From: gaoliming @ 2022-08-25  2:04 UTC (permalink / raw)
  To: devel, ardb, bob.c.feng, 'Andrew Fish', quic_llindhol,
	'Kinney, Michael D'
  Cc: rebecca, 'Chen, Christine', 'Wang, Jian J'

Bob:
  I mean this change brings the behavior change, but this patch doesn't highlight its impact and give the suggestion to modify the impacted DSC file. Otherwise, this impact can be raised and discussed early. Now, we are near to create edk2-stable202208. There is no enough time to discuss this change. So, I suggest to revert this change for stable tag 202208. 

Andrew, Lefi and Mike:
  The commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 (BaseTools: Fix DSC LibraryClass precedence rule) introduces this issue. This commit was merged into Edk2 on Last month (2022 July 17th). But, its impact was reported on last week. Because we are near for the stable tag edk2-stable202208, we need to make the decision to keep this change or revert this change for this stable tag. Can you give your comments on this change?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2022年8月24日 18:14
> 收件人: devel@edk2.groups.io; bob.c.feng@intel.com
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>; 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
> 
> On Wed, 24 Aug 2022 at 09:49, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 23 Aug 2022 at 09:41, Bob Feng <bob.c.feng@intel.com> wrote:
> > >
> > > 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.
> >
> > Platforms implemented against the DSC spec would have never worked if
> > they relied on behavior that BaseTools currently does not implement.
> >
> > > This commit 039bdb4d3e96f9c9264abf135b8a0eef2e2b4860 make the
> Basetools behavior be consistent with DSC spec so I don't think it's a
> regression bug.
> > >
> >
> > Platforms built against the current behavior that used to work might
> > break after this change. MdeModulePkg.dsc for ARM/AARCH64 no longer
> > builds due to this change.
> >
> > Also, as I pointed out, the DSC spec is far from ambiguous.
> 
> Ugh I did it again ...
> 
> "far from *un*ambiguous"
> 
> Will use less difficult words from now on - I obviously cannot be
> trusted with them.
> 
> > This means
> > that platforms implemented against the spec will rely on one of
> > several possible interpretations of that ambiguous set of rules.
> >
> > So let's revert this change now. Let's fix the DSC spec next, And only
> > then, let's fix the code (assuming it still needs to be fixed)
> >
> > --
> > Ard.
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-24 17:17             ` Bob Feng
@ 2022-08-25  8:48               ` Leif Lindholm
  2022-08-25 15:43                 ` Michael D Kinney
  0 siblings, 1 reply; 17+ messages in thread
From: Leif Lindholm @ 2022-08-25  8:48 UTC (permalink / raw)
  To: Feng, Bob C, devel@edk2.groups.io, Gao, Liming,
	'Ard Biesheuvel', rebecca@bsdio.com, Chen, Christine
  Cc: 'Andrew Fish', Kinney, Michael D, Wang, Jian J

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
>>
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-25  8:48               ` Leif Lindholm
@ 2022-08-25 15:43                 ` Michael D Kinney
  2022-08-26  0:28                   ` Ni, Ray
  2022-08-26  0:55                   ` Bob Feng
  0 siblings, 2 replies; 17+ messages in thread
From: Michael D Kinney @ 2022-08-25 15:43 UTC (permalink / raw)
  To: devel@edk2.groups.io, quic_llindhol@quicinc.com, Feng, Bob C,
	Gao, Liming, 'Ard Biesheuvel', rebecca@bsdio.com,
	Chen, Christine, Kinney, Michael D
  Cc: 'Andrew Fish', Wang, Jian J

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
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-25 15:43                 ` Michael D Kinney
@ 2022-08-26  0:28                   ` Ni, Ray
  2022-08-26  0:55                   ` Bob Feng
  1 sibling, 0 replies; 17+ messages in thread
From: Ni, Ray @ 2022-08-26  0:28 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D,
	quic_llindhol@quicinc.com, Feng, Bob C, Gao, Liming,
	'Ard Biesheuvel', rebecca@bsdio.com, Chen, Christine
  Cc: 'Andrew Fish', Wang, Jian J

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
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-25 15:43                 ` Michael D Kinney
  2022-08-26  0:28                   ` Ni, Ray
@ 2022-08-26  0:55                   ` Bob Feng
  2022-08-26  3:28                     ` Long1 Huang
  1 sibling, 1 reply; 17+ messages in thread
From: Bob Feng @ 2022-08-26  0:55 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io,
	quic_llindhol@quicinc.com, Gao, Liming, 'Ard Biesheuvel',
	rebecca@bsdio.com, Chen, Christine
  Cc: 'Andrew Fish', Wang, Jian J, Huang, Long1

+ 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
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-26  0:55                   ` Bob Feng
@ 2022-08-26  3:28                     ` Long1 Huang
  2022-08-26 10:12                       ` Ard Biesheuvel
  0 siblings, 1 reply; 17+ messages in thread
From: Long1 Huang @ 2022-08-26  3:28 UTC (permalink / raw)
  To: Feng, Bob C, Kinney, Michael D, devel@edk2.groups.io,
	quic_llindhol@quicinc.com, Gao, Liming, 'Ard Biesheuvel',
	rebecca@bsdio.com, Chen, Christine
  Cc: 'Andrew Fish', Wang, Jian J

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
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [edk2-devel] MdeModulePkg build fails for AARCH64 on Ubuntu 22.04
  2022-08-26  3:28                     ` Long1 Huang
@ 2022-08-26 10:12                       ` Ard Biesheuvel
  0 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2022-08-26 10:12 UTC (permalink / raw)
  To: Huang, Long1
  Cc: Feng, Bob C, Kinney, Michael D, devel@edk2.groups.io,
	quic_llindhol@quicinc.com, Gao, Liming, rebecca@bsdio.com,
	Chen, Christine, Andrew Fish, Wang, Jian J

comments inline

On Fri, 26 Aug 2022 at 05:28, Huang, Long1 <long1.huang@intel.com> wrote:
>
> Hi All,
>
> I'm the submitter.
>

Hello Huang, Long,

Thanks for providing this background - it is very helpful. Some of
this should have been recorded in the bugzilla entry or the patch
commit log, though, as now, nobody understood the problem or why the
fix was necessary to begin with.

> ==================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]
> ===========================================================================
>

This is incomplete. Before that, the DSC spec says

"""
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.
"""

Should we simply drop that part from the DSC spec?

Also, do you happen to have any idea for the rationale of having 2.
supersede 3. explicltly?

> 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).
>

OK. The problem, however, is that many platform definitions may exist
that actually rely on this behavior. And the symptom could be
something that only manifests itself at runtime.

> 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].
>

Thanks for clarifying that. But fixing the MdeModulePkg build is not
the priority here. The MdeModulePkg breakage is the canary in the
coalmine here, and tells us that making changes to the build tools
like this is dangerous, and may affect many users of this code base.

> 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.
>

Doesn't this mean that you should not be using [LibraryClasses.X64] to
begin with? Or am I misunderstanding the point you are making? Because
in that case, the problem would not exist either.

> 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.
>

Fixing MdeModulePkg is easy. Fixing all the DSCs out in the world that
may silently break because of this change is much harder.

> 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.
>

I agree. But we should clarify the DSC spec first, and at least
consider aligning the spec with the existing code (which has been in
wide use for many years) instead of the other way around.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-08-26 10:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-21  4:34 MdeModulePkg build fails for AARCH64 on Ubuntu 22.04 Rebecca Cran
2022-08-22  9:11 ` [edk2-devel] " Ard Biesheuvel
2022-08-22  9:30   ` Ard Biesheuvel
2022-08-22  9:34     ` Ard Biesheuvel
2022-08-23  1:54       ` 回复: " gaoliming
2022-08-23  7:40         ` Bob Feng
2022-08-24  7:16           ` 回复: " gaoliming
2022-08-24 17:17             ` Bob Feng
2022-08-25  8:48               ` Leif Lindholm
2022-08-25 15:43                 ` Michael D Kinney
2022-08-26  0:28                   ` Ni, Ray
2022-08-26  0:55                   ` Bob Feng
2022-08-26  3:28                     ` Long1 Huang
2022-08-26 10:12                       ` Ard Biesheuvel
2022-08-24  7:49           ` Ard Biesheuvel
2022-08-24 10:14             ` Ard Biesheuvel
2022-08-25  2:04               ` 回复: [edk2-devel][edk2-stable202208] " gaoliming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox