| Message ID | 1402324092-28707-1-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | Accepted |
| Commit | b0add92bb6553bfa481e84203bbb93b946d61824 |
| Headers | show |
Hi Ian, On 06/09/2014 03:28 PM, Ian Campbell wrote: > Otherwise functions in these sections can end up between .text and .rodata > which is after _etext and therefore gets made non-executable. > > This matches x86 (although it was done there for different reasons). > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Jan Beulich <JBeulich@suse.com> Acked-by: Julien Grall <julien.grall@linaro.org> Regards,
>>> On 09.06.14 at 16:28, <ian.campbell@citrix.com> wrote: > Otherwise functions in these sections can end up between .text and .rodata > which is after _etext and therefore gets made non-executable. > > This matches x86 (although it was done there for different reasons). > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Jan Beulich <JBeulich@suse.com> > --- > Jan, any reason why x86 doesn't just use .text.*? Because we want the .cold and .unlikely all adjacent, rather than intermixed with eventual other ones (imagine mixing in a hypothetical .text.hot). Jan > --- > xen/arch/arm/xen.lds.S | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index e8b4f47..be55dad 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -30,6 +30,8 @@ SECTIONS > .text : /* XXX should be AT ( XEN_PHYS_START ) */ { > _stext = .; /* Text section */ > *(.text) > + *(.text.cold) > + *(.text.unlikely) > *(.fixup) > *(.gnu.warning) > _etext = .; /* End of text section */ > -- > 1.7.10.4
On Tue, 2014-06-10 at 09:50 +0100, Jan Beulich wrote: > >>> On 09.06.14 at 16:28, <ian.campbell@citrix.com> wrote: > > Otherwise functions in these sections can end up between .text and .rodata > > which is after _etext and therefore gets made non-executable. > > > > This matches x86 (although it was done there for different reasons). > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: Jan Beulich <JBeulich@suse.com> > > --- > > Jan, any reason why x86 doesn't just use .text.*? > > Because we want the .cold and .unlikely all adjacent, rather than > intermixed with eventual other ones (imagine mixing in a > hypothetical .text.hot). That makes sense. Could we specify .text.* after those as a fallback or would we rather deal with each new section specifically so we can have a think about the right location? I don't suppose there is a linker option to make it barf on non-explicitly placed sections, is there? That would have turned this silent (and hard to diagnose) failure into something more explicit. Ian. > > Jan > > > --- > > xen/arch/arm/xen.lds.S | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > > index e8b4f47..be55dad 100644 > > --- a/xen/arch/arm/xen.lds.S > > +++ b/xen/arch/arm/xen.lds.S > > @@ -30,6 +30,8 @@ SECTIONS > > .text : /* XXX should be AT ( XEN_PHYS_START ) */ { > > _stext = .; /* Text section */ > > *(.text) > > + *(.text.cold) > > + *(.text.unlikely) > > *(.fixup) > > *(.gnu.warning) > > _etext = .; /* End of text section */ > > -- > > 1.7.10.4 > > >
>>> On 10.06.14 at 11:07, <Ian.Campbell@citrix.com> wrote: > On Tue, 2014-06-10 at 09:50 +0100, Jan Beulich wrote: >> >>> On 09.06.14 at 16:28, <ian.campbell@citrix.com> wrote: >> > Otherwise functions in these sections can end up between .text and .rodata >> > which is after _etext and therefore gets made non-executable. >> > >> > This matches x86 (although it was done there for different reasons). >> > >> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >> > Cc: Jan Beulich <JBeulich@suse.com> >> > --- >> > Jan, any reason why x86 doesn't just use .text.*? >> >> Because we want the .cold and .unlikely all adjacent, rather than >> intermixed with eventual other ones (imagine mixing in a >> hypothetical .text.hot). > > That makes sense. Could we specify .text.* after those as a fallback or > would we rather deal with each new section specifically so we can have a > think about the right location? That's certainly an option - placing them inefficiently is clearly better than placing them wrongly. > I don't suppose there is a linker option to make it barf on > non-explicitly placed sections, is there? That would have turned this > silent (and hard to diagnose) failure into something more explicit. I'm not aware of any such option. Jan
On Mon, 2014-06-09 at 15:50 +0100, Julien Grall wrote: > Hi Ian, > > On 06/09/2014 03:28 PM, Ian Campbell wrote: > > Otherwise functions in these sections can end up between .text and .rodata > > which is after _etext and therefore gets made non-executable. > > > > This matches x86 (although it was done there for different reasons). > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: Jan Beulich <JBeulich@suse.com> > > Acked-by: Julien Grall <julien.grall@linaro.org> Applied, thanks. Ian.
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index e8b4f47..be55dad 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -30,6 +30,8 @@ SECTIONS .text : /* XXX should be AT ( XEN_PHYS_START ) */ { _stext = .; /* Text section */ *(.text) + *(.text.cold) + *(.text.unlikely) *(.fixup) *(.gnu.warning) _etext = .; /* End of text section */
Otherwise functions in these sections can end up between .text and .rodata which is after _etext and therefore gets made non-executable. This matches x86 (although it was done there for different reasons). Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Jan Beulich <JBeulich@suse.com> --- Jan, any reason why x86 doesn't just use .text.*? --- xen/arch/arm/xen.lds.S | 2 ++ 1 file changed, 2 insertions(+)