Use a list of pointers to simplify the handling of 32- vs 64-bit.
Also on ARM the section name is ".init_array" and not
".ctors".
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Frediano Ziglio <frediano.ziglio@citrix.com>
Cc: keir@xen.org
---
This applies independently of my 64-bit ARM series.
---
xen/arch/arm/xen.lds.S | 10 ++++------
xen/arch/x86/xen.lds.S | 6 ++----
xen/common/lib.c | 13 +++++--------
3 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 9043994..fd755d7 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -91,12 +91,10 @@ SECTIONS
*(.init.data.rel)
*(.init.data.rel.*)
- . = ALIGN(4);
- __CTOR_LIST__ = .;
- LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2)
- *(.ctors)
- LONG(0)
- __CTOR_END__ = .;
+ . = ALIGN(8);
+ __ctors_start = .;
+ *(.init_array)
+ __ctors_end = .;
} :text
. = ALIGN(32);
.init.setup : {
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 5570389..d959941 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -110,11 +110,9 @@ SECTIONS
__trampoline_seg_stop = .;
. = ALIGN(8);
- __CTOR_LIST__ = .;
- QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2)
+ __ctors_start = .;
*(.ctors)
- QUAD(0)
- __CTOR_END__ = .;
+ __ctors_end = .;
} :text
. = ALIGN(32);
.init.setup : {
diff --git a/xen/common/lib.c b/xen/common/lib.c
index e0c65cf..6b80601 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -479,17 +479,14 @@ unsigned long long parse_size_and_unit(const char *s,
const char **ps)
return ret;
}
-extern const struct
-{
- unsigned long count;
- void (*funcs[1])(void);
-} __CTOR_LIST__;
+typedef void (*ctor_func_t)(void);
+extern const ctor_func_t __ctors_start[], __ctors_end[];
void __init init_constructors(void)
{
- unsigned long n;
- for ( n = 0; n < __CTOR_LIST__.count; ++n )
- __CTOR_LIST__.funcs[n]();
+ const ctor_func_t *f;
+ for (f = __ctors_start; f < __ctors_end; ++f )
+ (*f)();
}
/*
--
1.7.2.5
>>> On 22.02.13 at 11:57, Ian Campbell <ian.campbell@citrix.com> wrote: > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -91,12 +91,10 @@ SECTIONS > *(.init.data.rel) > *(.init.data.rel.*) > > - . = ALIGN(4); > - __CTOR_LIST__ = .; > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) > - *(.ctors) > - LONG(0) > - __CTOR_END__ = .; > + . = ALIGN(8); > + __ctors_start = .; > + *(.init_array)Is this renaming from .ctors to .init_array really intended (i.e. was using .ctors here wrong)? Jan
On Fri, 2013-02-22 at 11:16 +0000, Jan Beulich wrote:> >>> On 22.02.13 at 11:57, Ian Campbell <ian.campbell@citrix.com> wrote: > > --- a/xen/arch/arm/xen.lds.S > > +++ b/xen/arch/arm/xen.lds.S > > @@ -91,12 +91,10 @@ SECTIONS > > *(.init.data.rel) > > *(.init.data.rel.*) > > > > - . = ALIGN(4); > > - __CTOR_LIST__ = .; > > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) > > - *(.ctors) > > - LONG(0) > > - __CTOR_END__ = .; > > + . = ALIGN(8); > > + __ctors_start = .; > > + *(.init_array) > > Is this renaming from .ctors to .init_array really intended (i.e. was > using .ctors here wrong)?Yes, I mentioned it in the commit message, it is called .init_array on ARM for whatever reason. Probably either historical accident or compatibility eg. with ARM''s compilers. Ian.
>>> On 22.02.13 at 12:24, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Fri, 2013-02-22 at 11:16 +0000, Jan Beulich wrote: >> >>> On 22.02.13 at 11:57, Ian Campbell <ian.campbell@citrix.com> wrote: >> > --- a/xen/arch/arm/xen.lds.S >> > +++ b/xen/arch/arm/xen.lds.S >> > @@ -91,12 +91,10 @@ SECTIONS >> > *(.init.data.rel) >> > *(.init.data.rel.*) >> > >> > - . = ALIGN(4); >> > - __CTOR_LIST__ = .; >> > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) >> > - *(.ctors) >> > - LONG(0) >> > - __CTOR_END__ = .; >> > + . = ALIGN(8); >> > + __ctors_start = .; >> > + *(.init_array) >> >> Is this renaming from .ctors to .init_array really intended (i.e. was >> using .ctors here wrong)? > > Yes, I mentioned it in the commit message,Oops, sorry, managed to skip that line somehow.> it is called .init_array on > ARM for whatever reason. Probably either historical accident or > compatibility eg. with ARM''s compilers.Or rather the other way around, because .init_array is a (newer) ELF concept, whereas .ctors isn''t. Jan
On Fri, 2013-02-22 at 11:41 +0000, Jan Beulich wrote:> >>> On 22.02.13 at 12:24, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Fri, 2013-02-22 at 11:16 +0000, Jan Beulich wrote: > >> >>> On 22.02.13 at 11:57, Ian Campbell <ian.campbell@citrix.com> wrote: > >> > --- a/xen/arch/arm/xen.lds.S > >> > +++ b/xen/arch/arm/xen.lds.S > >> > @@ -91,12 +91,10 @@ SECTIONS > >> > *(.init.data.rel) > >> > *(.init.data.rel.*) > >> > > >> > - . = ALIGN(4); > >> > - __CTOR_LIST__ = .; > >> > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) > >> > - *(.ctors) > >> > - LONG(0) > >> > - __CTOR_END__ = .; > >> > + . = ALIGN(8); > >> > + __ctors_start = .; > >> > + *(.init_array) > >> > >> Is this renaming from .ctors to .init_array really intended (i.e. was > >> using .ctors here wrong)? > > > > Yes, I mentioned it in the commit message, > > Oops, sorry, managed to skip that line somehow. > > > it is called .init_array on > > ARM for whatever reason. Probably either historical accident or > > compatibility eg. with ARM''s compilers. > > Or rather the other way around, because .init_array is a (newer) > ELF concept, whereas .ctors isn''t.Ah, that explains it, thanks! Ian.
On Fri, 2013-02-22 at 10:57 +0000, Ian Campbell wrote:> Use a list of pointers to simplify the handling of 32- vs 64-bit. > > Also on ARM the section name is ".init_array" and not ".ctors".Any comments/ack/nacks?> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Frediano Ziglio <frediano.ziglio@citrix.com> > Cc: keir@xen.org > --- > This applies independently of my 64-bit ARM series. > --- > xen/arch/arm/xen.lds.S | 10 ++++------ > xen/arch/x86/xen.lds.S | 6 ++---- > xen/common/lib.c | 13 +++++-------- > 3 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index 9043994..fd755d7 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -91,12 +91,10 @@ SECTIONS > *(.init.data.rel) > *(.init.data.rel.*) > > - . = ALIGN(4); > - __CTOR_LIST__ = .; > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) > - *(.ctors) > - LONG(0) > - __CTOR_END__ = .; > + . = ALIGN(8); > + __ctors_start = .; > + *(.init_array) > + __ctors_end = .; > } :text > . = ALIGN(32); > .init.setup : { > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > index 5570389..d959941 100644 > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -110,11 +110,9 @@ SECTIONS > __trampoline_seg_stop = .; > > . = ALIGN(8); > - __CTOR_LIST__ = .; > - QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2) > + __ctors_start = .; > *(.ctors) > - QUAD(0) > - __CTOR_END__ = .; > + __ctors_end = .; > } :text > . = ALIGN(32); > .init.setup : { > diff --git a/xen/common/lib.c b/xen/common/lib.c > index e0c65cf..6b80601 100644 > --- a/xen/common/lib.c > +++ b/xen/common/lib.c > @@ -479,17 +479,14 @@ unsigned long long parse_size_and_unit(const char *s, const char **ps) > return ret; > } > > -extern const struct > -{ > - unsigned long count; > - void (*funcs[1])(void); > -} __CTOR_LIST__; > +typedef void (*ctor_func_t)(void); > +extern const ctor_func_t __ctors_start[], __ctors_end[]; > > void __init init_constructors(void) > { > - unsigned long n; > - for ( n = 0; n < __CTOR_LIST__.count; ++n ) > - __CTOR_LIST__.funcs[n](); > + const ctor_func_t *f; > + for (f = __ctors_start; f < __ctors_end; ++f ) > + (*f)(); > } > > /*
On 27/02/2013 13:17, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:> On Fri, 2013-02-22 at 10:57 +0000, Ian Campbell wrote: >> Use a list of pointers to simplify the handling of 32- vs 64-bit. >> >> Also on ARM the section name is ".init_array" and not ".ctors". > > Any comments/ack/nacks?If it works, it''s good by me ;) -- Keir>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >> Cc: Frediano Ziglio <frediano.ziglio@citrix.com> >> Cc: keir@xen.org >> --- >> This applies independently of my 64-bit ARM series. >> --- >> xen/arch/arm/xen.lds.S | 10 ++++------ >> xen/arch/x86/xen.lds.S | 6 ++---- >> xen/common/lib.c | 13 +++++-------- >> 3 files changed, 11 insertions(+), 18 deletions(-) >> >> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S >> index 9043994..fd755d7 100644 >> --- a/xen/arch/arm/xen.lds.S >> +++ b/xen/arch/arm/xen.lds.S >> @@ -91,12 +91,10 @@ SECTIONS >> *(.init.data.rel) >> *(.init.data.rel.*) >> >> - . = ALIGN(4); >> - __CTOR_LIST__ = .; >> - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) >> - *(.ctors) >> - LONG(0) >> - __CTOR_END__ = .; >> + . = ALIGN(8); >> + __ctors_start = .; >> + *(.init_array) >> + __ctors_end = .; >> } :text >> . = ALIGN(32); >> .init.setup : { >> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S >> index 5570389..d959941 100644 >> --- a/xen/arch/x86/xen.lds.S >> +++ b/xen/arch/x86/xen.lds.S >> @@ -110,11 +110,9 @@ SECTIONS >> __trampoline_seg_stop = .; >> >> . = ALIGN(8); >> - __CTOR_LIST__ = .; >> - QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2) >> + __ctors_start = .; >> *(.ctors) >> - QUAD(0) >> - __CTOR_END__ = .; >> + __ctors_end = .; >> } :text >> . = ALIGN(32); >> .init.setup : { >> diff --git a/xen/common/lib.c b/xen/common/lib.c >> index e0c65cf..6b80601 100644 >> --- a/xen/common/lib.c >> +++ b/xen/common/lib.c >> @@ -479,17 +479,14 @@ unsigned long long parse_size_and_unit(const char *s, >> const char **ps) >> return ret; >> } >> >> -extern const struct >> -{ >> - unsigned long count; >> - void (*funcs[1])(void); >> -} __CTOR_LIST__; >> +typedef void (*ctor_func_t)(void); >> +extern const ctor_func_t __ctors_start[], __ctors_end[]; >> >> void __init init_constructors(void) >> { >> - unsigned long n; >> - for ( n = 0; n < __CTOR_LIST__.count; ++n ) >> - __CTOR_LIST__.funcs[n](); >> + const ctor_func_t *f; >> + for (f = __ctors_start; f < __ctors_end; ++f ) >> + (*f)(); >> } >> >> /* > >
On Wed, 2013-02-27 at 13:17 +0000, Ian Campbell wrote:> On Fri, 2013-02-22 at 10:57 +0000, Ian Campbell wrote: > > Use a list of pointers to simplify the handling of 32- vs 64-bit. > > > > Also on ARM the section name is ".init_array" and not ".ctors". > > Any comments/ack/nacks? >Looks fine to me. Did you test it? Actually the way I test is: - compile with coverage - run xencov to extract information (more or less for x64 is 500k) - check file content to see if counters increase while taking multiple snapshot (xencov read - | hexdump -C | tail -100, counters are after 0CTX string which means "Xen Test Coverage count 0") - reset counters (but this has nothing to do with constructors) I actually cannot test for Arm. Oh.. there is a missing space after a "for(" Frediano> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: Frediano Ziglio <frediano.ziglio@citrix.com> > > Cc: keir@xen.org > > --- > > This applies independently of my 64-bit ARM series. > > --- > > xen/arch/arm/xen.lds.S | 10 ++++------ > > xen/arch/x86/xen.lds.S | 6 ++---- > > xen/common/lib.c | 13 +++++-------- > > 3 files changed, 11 insertions(+), 18 deletions(-) > > > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > > index 9043994..fd755d7 100644 > > --- a/xen/arch/arm/xen.lds.S > > +++ b/xen/arch/arm/xen.lds.S > > @@ -91,12 +91,10 @@ SECTIONS > > *(.init.data.rel) > > *(.init.data.rel.*) > > > > - . = ALIGN(4); > > - __CTOR_LIST__ = .; > > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) > > - *(.ctors) > > - LONG(0) > > - __CTOR_END__ = .; > > + . = ALIGN(8); > > + __ctors_start = .; > > + *(.init_array) > > + __ctors_end = .; > > } :text > > . = ALIGN(32); > > .init.setup : { > > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > > index 5570389..d959941 100644 > > --- a/xen/arch/x86/xen.lds.S > > +++ b/xen/arch/x86/xen.lds.S > > @@ -110,11 +110,9 @@ SECTIONS > > __trampoline_seg_stop = .; > > > > . = ALIGN(8); > > - __CTOR_LIST__ = .; > > - QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2) > > + __ctors_start = .; > > *(.ctors) > > - QUAD(0) > > - __CTOR_END__ = .; > > + __ctors_end = .; > > } :text > > . = ALIGN(32); > > .init.setup : { > > diff --git a/xen/common/lib.c b/xen/common/lib.c > > index e0c65cf..6b80601 100644 > > --- a/xen/common/lib.c > > +++ b/xen/common/lib.c > > @@ -479,17 +479,14 @@ unsigned long long parse_size_and_unit(const char *s, const char **ps) > > return ret; > > } > > > > -extern const struct > > -{ > > - unsigned long count; > > - void (*funcs[1])(void); > > -} __CTOR_LIST__; > > +typedef void (*ctor_func_t)(void); > > +extern const ctor_func_t __ctors_start[], __ctors_end[]; > > > > void __init init_constructors(void) > > { > > - unsigned long n; > > - for ( n = 0; n < __CTOR_LIST__.count; ++n ) > > - __CTOR_LIST__.funcs[n](); > > + const ctor_func_t *f; > > + for (f = __ctors_start; f < __ctors_end; ++f ) > > + (*f)(); > > } > > > > /* > >
On Thu, 2013-02-28 at 10:00 +0000, Frediano Ziglio wrote:> On Wed, 2013-02-27 at 13:17 +0000, Ian Campbell wrote: > > On Fri, 2013-02-22 at 10:57 +0000, Ian Campbell wrote: > > > Use a list of pointers to simplify the handling of 32- vs 64-bit. > > > > > > Also on ARM the section name is ".init_array" and not ".ctors". > > > > Any comments/ack/nacks? > > > > Looks fine to me. Did you test it?I tested on x86_64 and arm32/64 by annotating the init function to print the function names as it called them and observing that a) a bunch of functions were called (on x86 it looked similar before and after this change) and b) it didn''t crash.> Actually the way I test is: > - compile with coverage > - run xencov to extract information (more or less for x64 is 500k) > - check file content to see if counters increase while taking multiple > snapshot (xencov read - | hexdump -C | tail -100, counters are after > 0CTX string which means "Xen Test Coverage count 0") > - reset counters (but this has nothing to do with constructors) > > I actually cannot test for Arm. > > Oh.. there is a missing space after a "for("So there is, thanks for spotting. A resend will have to wait until after my flight to Hong Kong or else whoever applies, which may end up being me, can fix it as they go. Cheers, Ian.> > Frediano > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > Cc: Frediano Ziglio <frediano.ziglio@citrix.com> > > > Cc: keir@xen.org > > > --- > > > This applies independently of my 64-bit ARM series. > > > --- > > > xen/arch/arm/xen.lds.S | 10 ++++------ > > > xen/arch/x86/xen.lds.S | 6 ++---- > > > xen/common/lib.c | 13 +++++-------- > > > 3 files changed, 11 insertions(+), 18 deletions(-) > > > > > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > > > index 9043994..fd755d7 100644 > > > --- a/xen/arch/arm/xen.lds.S > > > +++ b/xen/arch/arm/xen.lds.S > > > @@ -91,12 +91,10 @@ SECTIONS > > > *(.init.data.rel) > > > *(.init.data.rel.*) > > > > > > - . = ALIGN(4); > > > - __CTOR_LIST__ = .; > > > - LONG((__CTOR_END__ - __CTOR_LIST__) / 4 - 2) > > > - *(.ctors) > > > - LONG(0) > > > - __CTOR_END__ = .; > > > + . = ALIGN(8); > > > + __ctors_start = .; > > > + *(.init_array) > > > + __ctors_end = .; > > > } :text > > > . = ALIGN(32); > > > .init.setup : { > > > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > > > index 5570389..d959941 100644 > > > --- a/xen/arch/x86/xen.lds.S > > > +++ b/xen/arch/x86/xen.lds.S > > > @@ -110,11 +110,9 @@ SECTIONS > > > __trampoline_seg_stop = .; > > > > > > . = ALIGN(8); > > > - __CTOR_LIST__ = .; > > > - QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2) > > > + __ctors_start = .; > > > *(.ctors) > > > - QUAD(0) > > > - __CTOR_END__ = .; > > > + __ctors_end = .; > > > } :text > > > . = ALIGN(32); > > > .init.setup : { > > > diff --git a/xen/common/lib.c b/xen/common/lib.c > > > index e0c65cf..6b80601 100644 > > > --- a/xen/common/lib.c > > > +++ b/xen/common/lib.c > > > @@ -479,17 +479,14 @@ unsigned long long parse_size_and_unit(const char *s, const char **ps) > > > return ret; > > > } > > > > > > -extern const struct > > > -{ > > > - unsigned long count; > > > - void (*funcs[1])(void); > > > -} __CTOR_LIST__; > > > +typedef void (*ctor_func_t)(void); > > > +extern const ctor_func_t __ctors_start[], __ctors_end[]; > > > > > > void __init init_constructors(void) > > > { > > > - unsigned long n; > > > - for ( n = 0; n < __CTOR_LIST__.count; ++n ) > > > - __CTOR_LIST__.funcs[n](); > > > + const ctor_func_t *f; > > > + for (f = __ctors_start; f < __ctors_end; ++f ) > > > + (*f)(); > > > } > > > > > > /* > > > > >