Re: [PATCH v2 2/2] xen: Populate xen.lds.h and make use of its macros

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.03.2022 13:07, Julien Grall wrote:
> On 29/03/2022 11:37, Jan Beulich wrote:
>> On 29.03.2022 11:54, Julien Grall wrote:
>>> On 22/03/2022 08:02, Michal Orzel wrote:
>>>> --- a/xen/include/xen/xen.lds.h
>>>> +++ b/xen/include/xen/xen.lds.h
>>>> @@ -5,4 +5,104 @@
>>>>     * Common macros to be used in architecture specific linker scripts.
>>>>     */
>>>>    
>>>> +/* Macros to declare debug sections. */
>>>> +#ifdef EFI
>>>
>>> AFAIK, we don't define EFI on Arm (just CONFIG_EFI). Yet we do support
>>> EFI on arm64.
>>>
>>> As this #ifdef is now in generic code, can you explain how this is meant
>>> to be used?
>>
>> The identifier may now be somewhat misleading, yes - it has always meant
>> "linking a native EFI (i.e. PE/COFF) binary". The equivalence "EFI binary"
>> == "EFI support" has long been lost.
> On Arm, we will be generating a EFI binary (or better a Image/EFI). So 
> IIUC the description, we should in theory set EFI.

Well, no - you're mixing up "generating" and "linking". What's of interest
here is what the linker is told to produce, not what may involved further
processing steps. We're talking about a linker script here, after all.

> But I think it would do the wrong thing on Arm. Would you be able to 
> explain why you need to differentiate it on x86?

The differences aren't unique to x86; they all are related to how ELF and
PE/COFF differ from one another.

>>>> +/*
>>>> + * Use the NOLOAD directive, despite currently ignored by (at least) GNU 
>>>> ld
>>>> + * for PE output, in order to record that we'd prefer these sections to 
>>>> not
>>>> + * be loaded into memory.
>>>> + */
>>>> +#define DECL_DEBUG(x, a) #x ALIGN(a) (NOLOAD) : { *(x) }
>>>> +#define DECL_DEBUG2(x, y, a) #x ALIGN(a) (NOLOAD) : { *(x) *(y) }
>>>> +#else
>>>> +#define DECL_DEBUG(x, a) #x 0 : { *(x) }
>>>> +#define DECL_DEBUG2(x, y, a) #x 0 : { *(x) *(y) }
>>>> +#endif
>>>> +
>>>> +/* DWARF debug sections. */
>>>> +#define DWARF_DEBUG_SECTIONS                      \
>>>> +  DECL_DEBUG(.debug_abbrev, 1)                    \
>>>> +  DECL_DEBUG2(.debug_info, .gnu.linkonce.wi.*, 1) \
>>>> +  DECL_DEBUG(.debug_types, 1)                     \
>>>> +  DECL_DEBUG(.debug_str, 1)                       \
>>>> +  DECL_DEBUG2(.debug_line, .debug_line.*, 1)      \
>>>> +  DECL_DEBUG(.debug_line_str, 1)                  \
>>>> +  DECL_DEBUG(.debug_names, 4)                     \
>>>> +  DECL_DEBUG(.debug_frame, 4)                     \
>>>> +  DECL_DEBUG(.debug_loc, 1)                       \
>>>> +  DECL_DEBUG(.debug_loclists, 4)                  \
>>>> +  DECL_DEBUG(.debug_macinfo, 1)                   \
>>>> +  DECL_DEBUG(.debug_macro, 1)                     \
>>>> +  DECL_DEBUG(.debug_ranges, 8)                    \
>>>> +  DECL_DEBUG(.debug_rnglists, 4)                  \
>>>> +  DECL_DEBUG(.debug_addr, 8)                      \
>>>> +  DECL_DEBUG(.debug_aranges, 1)                   \
>>>> +  DECL_DEBUG(.debug_pubnames, 1)                  \
>>>> +  DECL_DEBUG(.debug_pubtypes, 1)
>>>> +
>>>> +/* Stabs debug sections. */
>>>> +#define STABS_DEBUG_SECTIONS                 \
>>>> +  .stab 0 : { *(.stab) }                     \
>>>> +  .stabstr 0 : { *(.stabstr) }               \
>>>> +  .stab.excl 0 : { *(.stab.excl) }           \
>>>> +  .stab.exclstr 0 : { *(.stab.exclstr) }     \
>>>> +  .stab.index 0 : { *(.stab.index) }         \
>>>> +  .stab.indexstr 0 : { *(.stab.indexstr) }
>>>> +
>>>> +/*
>>>> + * Required sections not related to debugging.
>>>> + *
>>>> + * LLVM ld also wants .symtab, .strtab, and .shstrtab placed. These look 
>>>> to
>>>> + * be benign to GNU ld, so we can have them here unconditionally.
>>>> + */
>>>> +#define ELF_DETAILS_SECTIONS     \
>>>> +  .comment 0 : { *(.comment) }   \
>>>
>>> This is a bit confusing. Here you seem to use the section .comment. But...
>>>
>>>> +  .symtab 0 : { *(.symtab) }     \
>>>> +  .strtab 0 : { *(.strtab) }     \
>>>> +  .shstrtab 0 : { *(.shstrtab) }
>>>> +
>>>> +#ifdef EFI
>>>> +#define DISCARD_EFI_SECTIONS \
>>>> +       *(.comment)   \
>>>
>>> ... here you will discard it if EFI is set. Which one take precedence if
>>> the caller use both ELF_DETAILS_SECTIONS and DISCARD_EFI_SECTION?
>>
>> Given the above explanation I think it's clear that only one of the
>> two may be used at a time: ELF_DETAILS_SECTIONS when linking an ELF
>> binary and DISCARD_EFI_SECTIONS when linking a PE/COFF binary.
> 
> I guess this may be obvious on x86. But for Arm, we are generating the 
> ELF first and then extracting the information to generate the binary. 
> The end result will be a binary that is PE/COFF compatible.
> 
> So to me, it would make sense to include DISCARD_EFI_SECTIONS because we 
> going to create an EFI binary and also include EFI_DETAILS_SECTIONS 
> because we are building an ELF.

No - as per above, all we should be concerned about in the linker script
are requirements by the linker for linking a file in the request output
format.

Jan