Message ID | 1560975087-25632-3-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xen/link: Fixes and improvements to Xen's linking | expand |
Hi Andrew, On 6/19/19 9:11 PM, Andrew Cooper wrote: > Neither of these should live in .data > > * .data.schedulers is only ever read, so is moved into .rodata > * CONSTRUCTORS is only ever read, and only at boot, so is moved to beside > .init.rodata > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wl@xen.org> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/xen.lds.S | 11 ++++++----- > xen/arch/x86/xen.lds.S | 11 ++++++----- > 2 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index 31d74a8..2b44e5d 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -66,6 +66,11 @@ SECTIONS > *(.data.param) > __param_end = .; > > + . = ALIGN(POINTER_ALIGN); The alignment is going to be different on arm32 now. Please explain in the commit message why this is fine. > + __start_schedulers_array = .; > + *(.data.schedulers) > + __end_schedulers_array = .; > + Cheers,
On Wed, Jun 19, 2019 at 09:11:25PM +0100, Andrew Cooper wrote: > Neither of these should live in .data > > * .data.schedulers is only ever read, so is moved into .rodata > * CONSTRUCTORS is only ever read, and only at boot, so is moved to beside > .init.rodata > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> For x86: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> One comment below: > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wl@xen.org> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/xen.lds.S | 11 ++++++----- > xen/arch/x86/xen.lds.S | 11 ++++++----- > 2 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > index ec37d38..9fa6c99 100644 > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -140,6 +140,11 @@ SECTIONS > *(.data.param) > __param_end = .; > > + . = ALIGN(POINTER_ALIGN); > + __start_schedulers_array = .; > + *(.data.schedulers) > + __end_schedulers_array = .; > + > #if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) > . = ALIGN(POINTER_ALIGN); > __start_vpci_array = .; > @@ -207,6 +212,7 @@ SECTIONS > > *(.init.rodata) > *(.init.rodata.*) > + CONSTRUCTORS According to the ld manual CONSTRUCTORS is only relevant for a.out, ECOFF and XCOFF. I'm unsure whether PE does use CONSTRUCTORS or not, since it's a descendant of COFF. Thanks, Roger.
On 20/06/2019 09:40, Roger Pau Monné wrote: > On Wed, Jun 19, 2019 at 09:11:25PM +0100, Andrew Cooper wrote: >> Neither of these should live in .data >> >> * .data.schedulers is only ever read, so is moved into .rodata >> * CONSTRUCTORS is only ever read, and only at boot, so is moved to beside >> .init.rodata >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > For x86: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > One comment below: > >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Wei Liu <wl@xen.org> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/xen.lds.S | 11 ++++++----- >> xen/arch/x86/xen.lds.S | 11 ++++++----- >> 2 files changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S >> index ec37d38..9fa6c99 100644 >> --- a/xen/arch/x86/xen.lds.S >> +++ b/xen/arch/x86/xen.lds.S >> @@ -140,6 +140,11 @@ SECTIONS >> *(.data.param) >> __param_end = .; >> >> + . = ALIGN(POINTER_ALIGN); >> + __start_schedulers_array = .; >> + *(.data.schedulers) >> + __end_schedulers_array = .; >> + >> #if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) >> . = ALIGN(POINTER_ALIGN); >> __start_vpci_array = .; >> @@ -207,6 +212,7 @@ SECTIONS >> >> *(.init.rodata) >> *(.init.rodata.*) >> + CONSTRUCTORS > According to the ld manual CONSTRUCTORS is only relevant for a.out, > ECOFF and XCOFF. I'm unsure whether PE does use CONSTRUCTORS or not, > since it's a descendant of COFF. CONSTRUCTORS is strictly needed for the GCC coverage build to function (hence its introduction in the first place), and any code which uses __attribute__((constructor)) (which we use it in libxc, but not in Xen). I'd say that the ld manual is out-of-date. ~Andrew
>>> On 19.06.19 at 22:11, <andrew.cooper3@citrix.com> wrote: > Neither of these should live in .data > > * .data.schedulers is only ever read, so is moved into .rodata Which then would better be .rodata.schedulers, wouldn't it? Same would apparently go for .data.param. Jan
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 31d74a8..2b44e5d 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -66,6 +66,11 @@ SECTIONS *(.data.param) __param_end = .; + . = ALIGN(POINTER_ALIGN); + __start_schedulers_array = .; + *(.data.schedulers) + __end_schedulers_array = .; + __proc_info_start = .; *(.proc.info) __proc_info_end = .; @@ -92,12 +97,7 @@ SECTIONS . = ALIGN(PAGE_SIZE); *(.data.page_aligned) *(.data) - . = ALIGN(8); - __start_schedulers_array = .; - *(.data.schedulers) - __end_schedulers_array = .; *(.data.*) - CONSTRUCTORS } :text . = ALIGN(SMP_CACHE_BYTES); @@ -154,6 +154,7 @@ SECTIONS .init.data : { *(.init.rodata) *(.init.rodata.*) + CONSTRUCTORS . = ALIGN(POINTER_ALIGN); __setup_start = .; diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index ec37d38..9fa6c99 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -140,6 +140,11 @@ SECTIONS *(.data.param) __param_end = .; + . = ALIGN(POINTER_ALIGN); + __start_schedulers_array = .; + *(.data.schedulers) + __end_schedulers_array = .; + #if defined(CONFIG_HAS_VPCI) && defined(CONFIG_LATE_HWDOM) . = ALIGN(POINTER_ALIGN); __start_vpci_array = .; @@ -207,6 +212,7 @@ SECTIONS *(.init.rodata) *(.init.rodata.*) + CONSTRUCTORS . = ALIGN(POINTER_ALIGN); __setup_start = .; @@ -261,17 +267,12 @@ SECTIONS . = ALIGN(SMP_CACHE_BYTES); DECL_SECTION(.data.read_mostly) { *(.data.read_mostly) - . = ALIGN(8); - __start_schedulers_array = .; - *(.data.schedulers) - __end_schedulers_array = .; } :text DECL_SECTION(.data) { *(.data.page_aligned) *(.data) *(.data.*) - CONSTRUCTORS } :text DECL_SECTION(.bss) {
Neither of these should live in .data * .data.schedulers is only ever read, so is moved into .rodata * CONSTRUCTORS is only ever read, and only at boot, so is moved to beside .init.rodata Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/xen.lds.S | 11 ++++++----- xen/arch/x86/xen.lds.S | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-)