On 22.03.2022 09:02, Michal Orzel wrote:
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -69,10 +69,7 @@ SECTIONS
> __proc_info_end = .;
>
> #ifdef CONFIG_HAS_VPCI
> - . = ALIGN(POINTER_ALIGN);
> - __start_vpci_array = .;
> - *(SORT(.data.vpci.*))
> - __end_vpci_array = .;
> + VPCI_ARRAY
> #endif
Here and elsewhere I think the #ifdef should move as well, or to be
precise VPCI_ARRAY (in the specific case here) should simply expand to
nothing when CONFIG_HAS_VPCI is not defined.
> @@ -222,22 +213,18 @@ SECTIONS
> /* Section for the device tree blob (if any). */
> .dtb : { *(.dtb) } :text
>
> + /*
> + * Explicitly list debug sections, to avoid these sections being viewed as
> + * "orphan" by the linker.
> + */
> + DWARF_DEBUG_SECTIONS
Considering the comment, perhaps better to move ...
> /* Sections to be discarded */
> - /DISCARD/ : {
> - *(.exit.text)
> - *(.exit.data)
> - *(.exitcall.exit)
> - *(.eh_frame)
> - }
> + DISCARD_SECTIONS
>
> /* Stabs debugging 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) }
> - .comment 0 : { *(.comment) }
> + STABS_DEBUG_SECTIONS
... this up there?
> --- 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
> +/*
> + * 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 \
May I suggest to call this DWARF2_DEBUG_SECTIONS, to make clear no
Dwarf1 section is included here (and we also don't mean to support
such debug info)?
> + 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) } \
> + .symtab 0 : { *(.symtab) } \
> + .strtab 0 : { *(.strtab) } \
> + .shstrtab 0 : { *(.shstrtab) }
> +
> +#ifdef EFI
> +#define DISCARD_EFI_SECTIONS \
> + *(.comment) \
> + *(.comment.*) \
> + *(.note.*)
> +#else
> +#define DISCARD_EFI_SECTIONS
> +#endif
> +
> +/* Sections to be discarded. */
> +#define DISCARD_SECTIONS \
> + /DISCARD/ : { \
> + *(.text.exit) \
> + *(.exit.text) \
> + *(.exit.data) \
> + *(.exitcall.exit) \
> + *(.discard) \
> + *(.discard.*) \
> + *(.eh_frame) \
> + *(.dtors) \
> + *(.dtors.*) \
> + *(.fini_array) \
> + *(.fini_array.*) \
> + DISCARD_EFI_SECTIONS \
> + }
> +
> +#define VPCI_ARRAY \
> + . = ALIGN(POINTER_ALIGN); \
> + __start_vpci_array = .; \
> + *(SORT(.data.vpci.*)) \
> + __end_vpci_array = .;
> +
> +#define HYPFS_PARAM \
> + . = ALIGN(8); \
Since you're generalizing this, you mean POINTER_ALIGN here, not 8.
> + __paramhypfs_start = .; \
> + *(.data.paramhypfs) \
> + __paramhypfs_end = .;
> +
> +#define LOCK_PROFILE_DATA \
> + . = ALIGN(POINTER_ALIGN); \
> + __lock_profile_start = .; \
> + *(.lockprofile.data) \
> + __lock_profile_end = .;
While for *_SECTIONS I don't care as much, for these last three items
I think it would be good if we (maybe just informally) established an
ordering rule, such that we can ask further additions here to not occur
randomly. Once we've grown a few more of these, this would also help
quickly locating the perhaps just one construct of interest when
looking up things. Personally I think the only sensible ordering
criteria is the name of the construct being defined. This could be
mentioned in a comment ahead of them, and that comment would then at
the same time serve as separator between *_SECTIONS and any constructs
also defining (enclosing) symbols.
Jan
