[PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[Ian Campbell]

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(+)

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

Re: [PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[Julien Grall]

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,

-- 
Julien Grall

Re: [PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[Jan Beulich]

>>> 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

Re: [PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[Ian Campbell]

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
> 
> 
> 

Re: [PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[Jan Beulich]

>>> 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

Re: [PATCH] xen: arm: include .text.cold and .text.unlikely in text area

[Ian Campbell]

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.