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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.06.2025 23:28, Andrew Cooper wrote:
> On 10/06/2025 9:01 am, Jan Beulich wrote:
>> On 06.06.2025 17:01, Andrew Cooper wrote:
>>> On 06/06/2025 8:22 am, Jan Beulich wrote:
>>>> And then, based on your reasoning above, why don't you also drop the
>>>> #ifdef CONFIG_X86?
>>> Because that's the one non-buggy way of excluding an impossible case.
>>>
>>> x86 is the only architecture possibly linking with pep emulation, and
>>> therefore the only architecture to possibly have a CodeView record.
>> And how's the, say, Arm case different from the x86 case with no such
>> record built in? 
> 
> Because its currently impossible for ARM to have a codeview record.

There must be some pretty fundamental misunderstanding then: I can only
ask again - how's this different from xen.gz? There's not going to be
CodeView record there, yet still you enable the logic there. Hence the
body of the conditional is unreachable in that case.

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.

> Remember that ARM writes a MZ/PE header by hand in a flat binary.  It
> does not use a PEP linker.

Of course. Much like for xen.gz there's no PEP linking involved.

Jan