From: "Laszlo Ersek" <lersek@redhat.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>,
edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Abdul Lateef Attar <abdattar@amd.com>,
Abner Chang <abner.chang@amd.com>,
"Warkentin, Andrei" <andrei.warkentin@intel.com>,
Andrew Fish <afish@apple.com>,
Ard Biesheuvel <ardb+tianocore@kernel.org>,
"S, Ashraf Ali" <ashraf.ali.s@intel.com>,
Bibo Mao <maobibo@loongson.cn>,
"Feng, Bob C" <bob.c.feng@intel.com>,
"West, Catharine" <catharine.west@intel.com>,
Chao Li <lichao@loongson.cn>,
"Chiu, Chasel" <chasel.chiu@intel.com>,
"Duggapu, Chinni B" <chinni.b.duggapu@intel.com>,
Duke Zhai <duke.zhai@amd.com>,
"Aktas, Erdem" <erdemaktas@google.com>,
Eric Xing <eric.xing@amd.com>, Gerd Hoffmann <kraxel@redhat.com>,
"Guo, Gua" <gua.guo@intel.com>, "Dong, Guo" <guo.dong@intel.com>,
Igniculus Fu <igniculus.fu@amd.com>,
"Lu, James" <james.lu@intel.com>,
"Yao, Jiewen" <jiewen.yao@intel.com>,
Kelly Steele <kelly.steele@intel.com>, Ken Yao <ken.yao@amd.com>,
Leif Lindholm <quic_llindhol@quicinc.com>,
Liming Gao <gaoliming@byosoft.com.cn>,
Michael Roth <michael.roth@amd.com>,
"Xu, Min M" <min.m.xu@intel.com>,
"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>,
Paul Grimes <paul.grimes@amd.com>,
"Kumar, Rahul R" <rahul.r.kumar@intel.com>,
"Ni, Ray" <ray.ni@intel.com>, Rebecca Cran <rebecca@bsdio.com>,
"Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>,
Sami Mujawar <sami.mujawar@arm.com>,
"Rhodes, Sean" <sean@starlabs.systems>,
"Zeng, Star" <star.zeng@intel.com>,
Sunil V L <sunilvl@ventanamicro.com>,
"Mohapatra, Susovan" <susovan.mohapatra@intel.com>,
"Kuo, Ted" <ted.kuo@intel.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
USER0FISH <libing1202@outlook.com>,
Xianglai li <lixianglai@loongson.cn>,
"Chen, Christine" <yuwei.chen@intel.com>,
caiyuqing379 <caiyuqing_hz@outlook.com>,
dahogn <dahogn@hotmail.com>, meng-cz <mengcz1126@gmail.com>
Subject: Re: [edk2-devel] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules
Date: Wed, 7 Feb 2024 16:40:00 +0100 [thread overview]
Message-ID: <d66e5275-b876-51b7-4b68-804760496e0b@redhat.com> (raw)
In-Reply-To: <CO1PR11MB492903833D232E7BCD7FEC3DD2452@CO1PR11MB4929.namprd11.prod.outlook.com>
On 2/7/24 02:16, Kinney, Michael D wrote:
> Hi Laszlo,
>
> The main issue is that a non-backwards compatible change is being
> proposed to BaseTools.
>
> In order for this to work, BaseTools needs to support both the
> old style and new style SEC autogen.
>
> The guarantee of backwards compatibility is something that is required
> for BaseTools. Similar to constrains on compilers/linkers. New features
> can be added, but the old behavior has to always be supported. Removing
> old behavior takes a very long time.
This change consists of three parts:
(1) generate the SEC ProcessLibraryConstructorList() declaration in
"AutoGen.h"
(2) in those modules that declare SEC ProcessLibraryConstructorList()
themselves *properly*, remove the (now superfluous, but certainly *not
conflicting*) manual declarations
(3) in those modules that declare SEC ProcessLibraryConstructorList()
themselves *incorrectly*, either manually, or by way of including
<Library/PeimEntryPoint.h>, remove the *now-conflicting* manual
declarations / inclusions.
For modules that fall in category (2), the AutoGen change (1) is
compatible. The AutoGen change (1) is incompatible only with modules
that fall into category (3) -- that is, with modules that already
declare SEC ProcessLibraryConstructorList() incorrectly.
In other words, the compatibility we're talking about here is *bug
compatibility*.
If bug compat is a goal, then the AutoGen patch cannot be merged at all.
If bug compat is not a goal, then:
(a) the AutoGen change could be / should be merged into the separate
BaseTools repository at first,
(b) for edk2, I should rework this series such that:
- modules from category (3) be brought to category (2) at first (i.e.,
replacing the bogus calls / manual declarations with proper calls /
manual declarations),
- the edk2-basetools dependency in "pip-requirements.txt" be bumped from
0.1.48 to the new version,
- finally, the manual (but otherwise, now-correct) declarations in the
modules of category (2) be eliminated.
Therefore, my question is: do we intend to preserve bug compatibility?
Thanks!
Laszlo
>
> Mike
>
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, February 6, 2024 4:51 PM
>> To: edk2-devel-groups-io <devel@edk2.groups.io>
>> Cc: Abdul Lateef Attar <abdattar@amd.com>; Abner Chang
>> <abner.chang@amd.com>; Warkentin, Andrei <andrei.warkentin@intel.com>;
>> Andrew Fish <afish@apple.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>; S, Ashraf Ali <ashraf.ali.s@intel.com>;
>> Bibo Mao <maobibo@loongson.cn>; Feng, Bob C <bob.c.feng@intel.com>;
>> West, Catharine <catharine.west@intel.com>; Chao Li
>> <lichao@loongson.cn>; Chiu, Chasel <chasel.chiu@intel.com>; Duggapu,
>> Chinni B <chinni.b.duggapu@intel.com>; Duke Zhai <duke.zhai@amd.com>;
>> Aktas, Erdem <erdemaktas@google.com>; Eric Xing <eric.xing@amd.com>;
>> Gerd Hoffmann <kraxel@redhat.com>; Guo, Gua <gua.guo@intel.com>; Dong,
>> Guo <guo.dong@intel.com>; Igniculus Fu <igniculus.fu@amd.com>; Lu,
>> James <james.lu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kelly
>> Steele <kelly.steele@intel.com>; Ken Yao <ken.yao@amd.com>; Leif
>> Lindholm <quic_llindhol@quicinc.com>; Liming Gao
>> <gaoliming@byosoft.com.cn>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Michael Roth <michael.roth@amd.com>; Xu,
>> Min M <min.m.xu@intel.com>; Desimone, Nathaniel L
>> <nathaniel.l.desimone@intel.com>; Paul Grimes <paul.grimes@amd.com>;
>> Kumar, Rahul R <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>;
>> Rebecca Cran <rebecca@bsdio.com>; Chaganty, Rangasai V
>> <rangasai.v.chaganty@intel.com>; Sami Mujawar <sami.mujawar@arm.com>;
>> Rhodes, Sean <sean@starlabs.systems>; Zeng, Star <star.zeng@intel.com>;
>> Sunil V L <sunilvl@ventanamicro.com>; Mohapatra, Susovan
>> <susovan.mohapatra@intel.com>; Kuo, Ted <ted.kuo@intel.com>; Tom
>> Lendacky <thomas.lendacky@amd.com>; USER0FISH <libing1202@outlook.com>;
>> Xianglai li <lixianglai@loongson.cn>; Chen, Christine
>> <yuwei.chen@intel.com>; caiyuqing379 <caiyuqing_hz@outlook.com>; dahogn
>> <dahogn@hotmail.com>; meng-cz <mengcz1126@gmail.com>
>> Subject: BaseTools/AutoGen: declare ProcessLibraryConstructorList() for
>> SEC modules
>>
>> Bugzillas:
>> - https://bugzilla.tianocore.org/show_bug.cgi?id=990
>> - https://bugzilla.tianocore.org/show_bug.cgi?id=991
>>
>> Personal build:
>> - https://github.com/tianocore/edk2/pull/5354
>>
>> I'll follow up with two series, in response to this message. One of
>> those sets makes AutoGen declare ProcessLibraryConstructorList() for
>> SEC
>> modules (as Liming suggested), and cleans up the buggy or superfluous
>> manual declarations in edk2. The other set cleans up edk2-platforms
>> similarly. The edk2 set is intended to be bisectable. The edk2-
>> platforms
>> set can't be like that (separate repo anyway). I build-tested
>> everything
>> that I could, but the coverage is not complete in edk2-platforms.
>>
>> Note that the personal build (PR#5354) failed. The reason is that CI
>> uses an out-of-tree BaseTools installation / package (version 0.1.48 at
>> the moment). This just goes on to show why it's a bad idea to develop
>> BaseTools in separation from edk2: even though the edk2 series *is*
>> fully bisectable, it's impossible to get it through CI (because CI
>> ignores the BaseTools update inside the series); and if we get the
>> central BaseTools patch from the edk2 series into "that other"
>> (separate) project *first*, then the series will not be bisectable any
>> longer (because the BaseTools/AutoGen update at the front, without
>> further patches in the series applied, will trigger build errors).
>>
>> Laszlo
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115238): https://edk2.groups.io/g/devel/message/115238
Mute This Topic: https://groups.io/mt/104210524/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-02-07 15:40 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-07 0:50 [edk2-devel] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 00/11] " Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 01/11] OvmfPkg/Sec: Use correct prototype of ProcessLibraryConstructorList() Laszlo Ersek
2024-02-14 14:27 ` Ard Biesheuvel
2024-02-07 1:04 ` [edk2-devel] [PATCH 02/11] OvmfPkg/RiscVVirt/Sec: don't #include <Library/PeimEntryPoint.h> Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 03/11] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 04/11] ArmPlatformPkg: remove SEC ProcessLibraryConstructorList() declarations Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 05/11] ArmVirtPkg: remove SEC ProcessLibraryConstructorList() declaration Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 06/11] EmulatorPkg: " Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 07/11] IntelFsp2Pkg: " Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 08/11] OvmfPkg/RiscVVirt: " Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 09/11] OvmfPkg: " Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 10/11] UefiCpuPkg: " Laszlo Ersek
2024-02-07 1:04 ` [edk2-devel] [PATCH 11/11] UefiPayloadPkg: " Laszlo Ersek
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 0/6] clean up SEC ProcessLibraryConstructorList() declarations Laszlo Ersek
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 1/6] ChachaniBoardPkg: remove SEC ProcessLibraryConstructorList() declaration Laszlo Ersek
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 2/6] BeagleBoardPkg: " Laszlo Ersek
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 3/6] QuarkPlatformPkg: " Laszlo Ersek
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 4/6] SimicsOpenBoardPkg: fix SEC ProcessLibraryConstructorList() prototype Laszlo Ersek
2024-02-09 0:07 ` Nate DeSimone
2024-02-09 9:24 ` Laszlo Ersek
2024-02-09 22:53 ` Nate DeSimone
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 5/6] LoongArchQemuPkg: " Laszlo Ersek
2024-02-19 2:21 ` Chao Li
2024-02-19 20:08 ` Laszlo Ersek
2024-02-07 1:09 ` [edk2-devel] [edk2-platforms PATCH 6/6] SG2042Pkg: clean up SEC ProcessLibraryConstructorList() declaration Laszlo Ersek
2024-02-14 14:25 ` [edk2-devel] [edk2-platforms PATCH 0/6] clean up SEC ProcessLibraryConstructorList() declarations Ard Biesheuvel
2024-02-07 1:16 ` [edk2-devel] BaseTools/AutoGen: declare ProcessLibraryConstructorList() for SEC modules Michael D Kinney
2024-02-07 15:40 ` Laszlo Ersek [this message]
2024-02-08 16:40 ` Michael D Kinney
2024-02-15 7:57 ` Laszlo Ersek
2024-02-15 17:29 ` Michael D Kinney
2024-02-16 14:16 ` Laszlo Ersek
2024-02-16 17:21 ` Michael D Kinney
2024-02-24 18:02 ` Laszlo Ersek
2024-02-24 18:37 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d66e5275-b876-51b7-4b68-804760496e0b@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox