Laszlo: Thanks for your work to make sure this issue be fixed. I agree this change. Reviewed-by: Liming Gao <gaolim...@byosoft.com.cn>
Thanks Liming > -----邮件原件----- > 发件人: Laszlo Ersek <ler...@redhat.com> > 发送时间: 2024年2月25日 5:05 > 收件人: edk2-devel-groups-io <devel@edk2.groups.io> > 抄送: Bob Feng <bob.c.f...@intel.com>; Liming Gao > <gaolim...@byosoft.com.cn>; Michael D Kinney > <michael.d.kin...@intel.com>; Rebecca Cran <rebe...@bsdio.com>; Yuwei > Chen <yuwei.c...@intel.com> > 主题: [PATCH] BaseTools/AutoGen: declare ProcessLibraryConstructorList() > for SEC modules > > Most module types have standardized entry point function prototypes. They > are declared in headers like > > - MdePkg/Include/Library/PeiCoreEntryPoint.h > - MdePkg/Include/Library/PeimEntryPoint.h > - MdePkg/Include/Library/DxeCoreEntryPoint.h > - MdePkg/Include/Library/UefiDriverEntryPoint.h > - MdePkg/Include/Library/UefiApplicationEntryPoint.h > > These header files also declare matching ProcessLibraryConstructorList() > prototypes. > > The SEC module type does not have a standardized entry point prototype > (aka parameter list), therefore no header file like the above ones exists > for SEC. Consequently, no header file *declares* > ProcessLibraryConstructorList() for SEC modules, even though AutoGen > always *defines* ProcessLibraryConstructorList() with the same, empty, > parameter list (i.e., just (VOID)). > > The lack of a central declaration is a problem because in SEC code, > ProcessLibraryConstructorList() needs to be called manually, and those > calls need a prototype. Most SEC modules in edk2 get around this by > declaring ProcessLibraryConstructorList() manually, while some others use > an incorrect (PEIM) prototype. > > Liming suggested in > <https://bugzilla.tianocore.org/show_bug.cgi?id=991#c2> that AutoGen > provide the declaration as well; implement that in this patch. > > Mike suggested that the feature be gated with INF_VERSION, for > compatibility reasons. (INF_VERSION >= 1.30) reflects that the latest > (draft) version of the INF specification, as of this writing, is commit > a31e3c842bee / version 1.29. > > For example, if we modify "OvmfPkg/Sec/SecMain.inf" as follows: > > > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf > > index 3c47a664a95d..dca932a474ee 100644 > > --- a/OvmfPkg/Sec/SecMain.inf > > +++ b/OvmfPkg/Sec/SecMain.inf > > @@ -8,7 +8,7 @@ > > ## > > > > [Defines] > > - INF_VERSION = 0x00010005 > > + INF_VERSION = 1.30 > > BASE_NAME = SecMain > > FILE_GUID = > df1ccef6-f301-4a63-9661-fc6030dcc880 > > MODULE_TYPE = SEC > > then the patch produces the following difference in > "Build/OvmfX64/NOOPT_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGe > n.h": > > > --- AutoGen.h.orig 2024-02-06 23:10:23.469535345 +0100 > > +++ AutoGen.h 2024-02-07 00:00:57.361294055 +0100 > > @@ -220,6 +220,13 @@ > > > > // Definition of PCDs used in libraries is in AutoGen.c > > > > +// ProcessLibraryConstructorList() declared here because SEC has no > standard entry point. > > +VOID > > +EFIAPI > > +ProcessLibraryConstructorList ( > > + VOID > > + ); > > + > > > > #ifdef __cplusplus > > } > > which presently (as of edk2 commit edc6681206c1) triggers the following > build error: > > > In file included from OvmfPkg/Sec/SecMain.c:14: > > MdePkg/Include/Library/PeimEntryPoint.h:74:1: error: conflicting types for > > ‘ProcessLibraryConstructorList’; have ‘void(void *, const > > EFI_PEI_SERVICES **)’ {aka ‘void(void *, const struct _EFI_PEI_SERVICES > > **)’} > > 74 | ProcessLibraryConstructorList ( > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > In file included from <command-line>: > > > Build/OvmfX64/NOOPT_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen. > h:226:1: note: > > previous declaration of ‘ProcessLibraryConstructorList’ with type > > ‘void(void)’ > > 226 | ProcessLibraryConstructorList ( > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > That's a genuine bug in OvmfPkg that needs to be fixed, but we keep > compatibility with existent SEC modules until/unless they upgrade > INF_VERSION to 1.30+. > > Cc: Bob Feng <bob.c.f...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Rebecca Cran <rebe...@bsdio.com> > Cc: Yuwei Chen <yuwei.c...@intel.com> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=991 > Suggested-by: Liming Gao <gaolim...@byosoft.com.cn> > Suggested-by: Michael D Kinney <michael.d.kin...@intel.com> > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > > Notes: > v2: > > - CI run: <https://github.com/tianocore/edk2/pull/5404> > > - port to edk2-basetools: > <https://github.com/tianocore/edk2-basetools/pull/120> > > - depend on INF_VERSION >= 1.30 [Mike] > > - extend commit message with INF_VERSION dependency > > - extend commit message with example build error due to preexistent > broken ProcessLibraryConstructorList() declaration in OvmfPkg, now > caught by the auto-generated prototype > > BaseTools/Source/Python/AutoGen/GenC.py | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/BaseTools/Source/Python/AutoGen/GenC.py > b/BaseTools/Source/Python/AutoGen/GenC.py > index a2053d548521..5ad10cee2898 100755 > --- a/BaseTools/Source/Python/AutoGen/GenC.py > +++ b/BaseTools/Source/Python/AutoGen/GenC.py > @@ -1371,6 +1371,14 @@ def CreateLibraryConstructorCode(Info, > AutoGenC, AutoGenH): > else: > if Info.ModuleType in [SUP_MODULE_BASE, SUP_MODULE_SEC, > SUP_MODULE_USER_DEFINED, SUP_MODULE_HOST_APPLICATION]: > > AutoGenC.Append(gLibraryString[SUP_MODULE_BASE].Replace(Dict)) > + if Info.ModuleType == SUP_MODULE_SEC and > Info.AutoGenVersion >= 0x0001001E: > + AutoGenH.Append(("\n" > + "// ProcessLibraryConstructorList() > declared here because SEC has no standard entry point.\n" > + "VOID\n" > + "EFIAPI\n" > + "ProcessLibraryConstructorList (\n" > + " VOID\n" > + " );\n")) > elif Info.ModuleType in SUP_MODULE_SET_PEI: > AutoGenC.Append(gLibraryString['PEI'].Replace(Dict)) > elif Info.ModuleType in [SUP_MODULE_DXE_CORE, > SUP_MODULE_DXE_DRIVER, SUP_MODULE_DXE_SMM_DRIVER, > SUP_MODULE_DXE_RUNTIME_DRIVER, > > base-commit: edc6681206c1a8791981a2f911d2fb8b3d2f5768 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115920): https://edk2.groups.io/g/devel/message/115920 Mute This Topic: https://groups.io/mt/104558376/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
