Re: [edk2-devel] [PATCH] OvmfPkg/Sec: Use correct prototype of ProcessLibraryConstructorList()

["Laszlo Ersek" <lersek@redhat.com>]

Hello Ard,

On 2/2/24 19:24, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The prototype of ProcessLibraryConstructorList(), which is generated by
> the build tools, depends on the boot phase, and SEC and PEI differ in
> this regard. However, OVMF's SEC implemention includes PeimEntryPoint.h
> and calls the PEI version of the protoype (passing NULL arguments),
> whereas the actual implementation observed by the compiler follows the
> SEC version, which takes VOID.
> 
> The compiler usually doesn't spot the difference, but the LTO linker
> does, and throws an error:
> 
> MdePkg/Include/Library/PeimEntryPoint.h:74:1: error: type of 'ProcessLibraryConstructorList' does not match original declaration [-Werror=lto-type-mismatch]
>    74 | ProcessLibraryConstructorList (
>       | ^
> Build/OvmfX64/RELEASE_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:310:1: note: type mismatch in parameter 1
>   310 | ProcessLibraryConstructorList (
>       | ^
> Build/OvmfX64/RELEASE_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:310:1: note: type 'void' should match type 'void *'
> Build/OvmfX64/RELEASE_GCC5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:310:1: note: 'ProcessLibraryConstructorList' was previously declared here
> 
> Fix this by dropping the #include, and providing a declaration of
> ProcessLibraryConstructorList() in SecMain.c
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4643
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  OvmfPkg/Sec/SecMain.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

This issue was reported 5+ years ago:

  https://bugzilla.tianocore.org/show_bug.cgi?id=990

I asked for a general solution in BaseTools:

  https://bugzilla.tianocore.org/show_bug.cgi?id=991

Liming recommended that BaseTools should generate a proper declaration
in "AutoGen.h":

  https://bugzilla.tianocore.org/show_bug.cgi?id=991#c2

But nothing happened.

> 
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index 31da5d0ace51..7b7d4793984c 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -11,7 +11,6 @@
>  
>  #include <PiPei.h>
>  
> -#include <Library/PeimEntryPoint.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/BaseMemoryLib.h>
> @@ -39,6 +38,12 @@ typedef struct _SEC_IDT_TABLE {
>    IA32_IDT_GATE_DESCRIPTOR    IdtTable[SEC_IDT_ENTRY_COUNT];
>  } SEC_IDT_TABLE;
>  
> +VOID
> +EFIAPI
> +ProcessLibraryConstructorList (
> +  VOID
> +  );
> +

This is my problem right here.

Why do 10+ files have to do this individually? See the above BZs for
details.

Well, whatever.

What a terrible project.

*shrug*

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

>  VOID
>  EFIAPI
>  SecStartupPhase2 (
> @@ -844,7 +849,7 @@ SecCoreStartupWithStack (
>      InitializeCpuExceptionHandlers (NULL);
>    }
>  
> -  ProcessLibraryConstructorList (NULL, NULL);
> +  ProcessLibraryConstructorList ();
>  
>    if (!SevEsIsEnabled ()) {
>      //



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115089): https://edk2.groups.io/g/devel/message/115089
Mute This Topic: https://groups.io/mt/104167522/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-