Re: [PATCH 1/3] x86/EFI: Fix detection of buildid

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.06.2025 22:18, Andrew Cooper wrote:
> On 16/06/2025 7:27 am, Jan Beulich wrote:
>> To expand on my earlier suggestion (ab)using the "efi" global: With
>> the linker script having this
>>
>> #ifdef EFI
>>   .reloc ALIGN(4) : {
>>     __base_relocs_start = .;
>>     *(.reloc)
>>     __base_relocs_end = .;
>>   }
>> #elif defined(XEN_BUILD_EFI)
>>   /*
>>    * Due to the way EFI support is currently implemented, these two symbols
>>    * need to be defined.  Their precise values shouldn't matter (the 
>> consuming
>>    * function doesn't get called), but to be on the safe side both values 
>> would
>>    * better match.  Of course the need to be reachable by the relocations
>>    * referencing them.
>>    */
>>   PROVIDE(__base_relocs_start = .);
>>   PROVIDE(__base_relocs_end = .);
>> #else
>>   efi = .;
>> #endif
>>
>> where only the #if applies to xen.efi, can't we (ab)use the combination of 
>> the
>> other two symbols here to decide between xen.efi vs xen.gz?
>> __base_relocs_{start,efi} won't possibly be equal for xen.efi, except in an
>> extremely theoretical situation (and we could cover for that case by an 
>> ASSERT
>> in the linker script). Pseudo code:
>>
>> #ifdef XEN_BUILD_EFI
>>     if ( __base_relocs_start != __base_relocs_end )
>>     {
>>         ...
>>     }
>> #endif
>>
>> IOW that #if could simply replace the CONFIG_X86 one that's there right now.
> 
> That's horrifying.  Also you can't include efi-boot.h to get the
> declarations.

The declarations could be moved, but ...

> But given that you are adamant that the (...) in there containing a
> CodeView check is unacceptable to have, why does wrapping it in yet
> another conditional make it ok?

... hmm, yes, there'll still be unreachable code there. I guess I'll shut
up here and leave this to the EFI maintainers. Just as long as there's not
going to be any Eclair / Misra fallout.

Jan