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

2022-04-11 Thread Michal Orzel
Hi Julien,

On 08.04.2022 19:58, Julien Grall wrote:
> Hi Michal,
> 
> On 05/04/2022 10:16, Michal Orzel wrote:
>>   #if defined(BUILD_ID)
>> @@ -109,12 +104,7 @@ SECTIONS
>>  *(.data.schedulers)
>>  __end_schedulers_array = .;
>>   -#ifdef CONFIG_HYPFS
>> -   . = ALIGN(8);
> 
> This will be replaced with POINTER_ALIGN which is 4-byte on Arm32. AFAICT, 
> there are no 64-bit value used in struct param_hypfs. So it should be fine.
> 
> That said, I think this is worth mentioning in the commit message.
> 
> The rest of this patch looks good to me.
> 
> Cheers,
> 
Ok, I will update the commit and repush the series.

Thanks,
Michal



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

2022-04-08 Thread Julien Grall

Hi Michal,

On 05/04/2022 10:16, Michal Orzel wrote:

  #if defined(BUILD_ID)
@@ -109,12 +104,7 @@ SECTIONS
 *(.data.schedulers)
 __end_schedulers_array = .;
  
-#ifdef CONFIG_HYPFS

-   . = ALIGN(8);


This will be replaced with POINTER_ALIGN which is 4-byte on Arm32. 
AFAICT, there are no 64-bit value used in struct param_hypfs. So it 
should be fine.


That said, I think this is worth mentioning in the commit message.

The rest of this patch looks good to me.

Cheers,

--
Julien Grall



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

2022-04-05 Thread Jan Beulich
On 05.04.2022 11:16, Michal Orzel wrote:
> Populate header file xen.lds.h with the first portion of macros storing
> constructs common to x86 and arm linker scripts. Replace the original
> constructs with these helpers.
> 
> No functional improvements to x86 linker script.
> 
> Making use of common macros improves arm linker script with:
> - explicit list of debug sections that otherwise are seen as "orphans"
>   by the linker. This will allow to fix issues after enabling linker
>   option --orphan-handling one day,
> - extended list of discarded section to include: .discard, destructors
>   related sections, .fini_array which can reference .text.exit,
> - sections not related to debugging that are placed by ld.lld. Even
>   though we do not support linking with LLD on Arm, these sections do
>   not cause problem to GNU ld.
> 
> Please note that this patch does not aim to perform the full sync up
> between the linker scripts. It creates a base for further work.
> 
> Signed-off-by: Michal Orzel 

Reviewed-by: Jan Beulich 




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

2022-04-05 Thread Michal Orzel
Populate header file xen.lds.h with the first portion of macros storing
constructs common to x86 and arm linker scripts. Replace the original
constructs with these helpers.

No functional improvements to x86 linker script.

Making use of common macros improves arm linker script with:
- explicit list of debug sections that otherwise are seen as "orphans"
  by the linker. This will allow to fix issues after enabling linker
  option --orphan-handling one day,
- extended list of discarded section to include: .discard, destructors
  related sections, .fini_array which can reference .text.exit,
- sections not related to debugging that are placed by ld.lld. Even
  though we do not support linking with LLD on Arm, these sections do
  not cause problem to GNU ld.

Please note that this patch does not aim to perform the full sync up
between the linker scripts. It creates a base for further work.

Signed-off-by: Michal Orzel 
---
Changes since v3:
-use POINTER_ALIGN in debug sections when needed
-modify comment about ELF_DETAILS_SECTIONS
Changes since v2:
-refactor commit msg
-move constructs together with surrounding ifdefery
-list constructs other than *_SECTIONS in alphabetical order
-add comment about EFI vs EFI support
Changes since v1:
-merge x86 and arm changes into single patch
-do not propagate issues by generalizing CTORS
-extract sections not related to debugging into separate macro
-get rid of _SECTION suffix in favor of using more meaningful suffixes
---
 xen/arch/arm/xen.lds.S|  44 +++--
 xen/arch/x86/xen.lds.S|  96 +++-
 xen/include/xen/xen.lds.h | 129 ++
 3 files changed, 147 insertions(+), 122 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index c666fc3e69..649aa04f7f 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -68,12 +68,7 @@ SECTIONS
*(.proc.info)
__proc_info_end = .;
 
-#ifdef CONFIG_HAS_VPCI
-   . = ALIGN(POINTER_ALIGN);
-   __start_vpci_array = .;
-   *(SORT(.data.vpci.*))
-   __end_vpci_array = .;
-#endif
+   VPCI_ARRAY
   } :text
 
 #if defined(BUILD_ID)
@@ -109,12 +104,7 @@ SECTIONS
*(.data.schedulers)
__end_schedulers_array = .;
 
-#ifdef CONFIG_HYPFS
-   . = ALIGN(8);
-   __paramhypfs_start = .;
-   *(.data.paramhypfs)
-   __paramhypfs_end = .;
-#endif
+   HYPFS_PARAM
 
*(.data .data.*)
CONSTRUCTORS
@@ -178,12 +168,7 @@ SECTIONS
*(.altinstructions)
__alt_instructions_end = .;
 
-#ifdef CONFIG_DEBUG_LOCK_PROFILE
-   . = ALIGN(POINTER_ALIGN);
-   __lock_profile_start = .;
-   *(.lockprofile.data)
-   __lock_profile_end = .;
-#endif
+   LOCK_PROFILE_DATA
 
*(.init.data)
*(.init.data.rel)
@@ -222,22 +207,13 @@ SECTIONS
   /* Section for the device tree blob (if any). */
   .dtb : { *(.dtb) } :text
 
-  /* Sections to be discarded */
-  /DISCARD/ : {
-   *(.exit.text)
-   *(.exit.data)
-   *(.exitcall.exit)
-   *(.eh_frame)
-  }
-
-  /* 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) }
+  DWARF2_DEBUG_SECTIONS
+
+  DISCARD_SECTIONS
+
+  STABS_DEBUG_SECTIONS
+
+  ELF_DETAILS_SECTIONS
 }
 
 /*
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 3e65c09bb3..65cc4c9231 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -13,13 +13,6 @@
 #undef __XEN_VIRT_START
 #define __XEN_VIRT_START __image_base__
 #define DECL_SECTION(x) x :
-/*
- * 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) }
 
 ENTRY(efi_start)
 
@@ -27,8 +20,6 @@ ENTRY(efi_start)
 
 #define FORMAT "elf64-x86-64"
 #define DECL_SECTION(x) #x : AT(ADDR(#x) - __XEN_VIRT_START)
-#define DECL_DEBUG(x, a) #x 0 : { *(x) }
-#define DECL_DEBUG2(x, y, a) #x 0 : { *(x) *(y) }
 
 ENTRY(start_pa)
 
@@ -159,12 +150,7 @@ SECTIONS
*(.note.gnu.build-id)
__note_gnu_build_id_end = .;
 #endif
-#ifdef CONFIG_HAS_VPCI
-   . = ALIGN(POINTER_ALIGN);
-   __start_vpci_array = .;
-   *(SORT(.data.vpci.*))
-   __end_vpci_array = .;
-#endif
+   VPCI_ARRAY
   } PHDR(text)
 
 #if defined(CONFIG_PVH_GUEST) && !defined(EFI)
@@ -278,12 +264,7 @@ SECTIONS
 *(.altinstructions)
 __alt_instructions_end = .;
 
-#ifdef CONFIG_DEBUG_LOCK_PROFILE
-   . = ALIGN(POINTER_ALIGN);
-   __lock_profile_start = .;
-   *(.lockprofile.data)
-   __lock_profile_end = .;
-#endif
+   LOCK_PROFILE_DATA
 
. = ALIGN(8);