[v3,2/3] xen/link: handle .init.rodata.cst* sections in the linker script
diff mbox series

Message ID 20190627093335.56355-2-roger.pau@citrix.com
State New, archived
Headers show
Series
  • [v3,1/3] x86/linker: add a reloc section to ELF linker script
Related show

Commit Message

Roger Pau Monne June 27, 2019, 9:33 a.m. UTC
Note that those sections when not prefixed with .init are already
handled by the more general .rodata.* matching pattern in the .rodata
output section.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
---
 xen/arch/arm/xen.lds.S | 1 +
 xen/arch/x86/xen.lds.S | 1 +
 2 files changed, 2 insertions(+)

Comments

Andrew Cooper June 27, 2019, 10:53 a.m. UTC | #1
On 27/06/2019 10:33, Roger Pau Monne wrote:
> Note that those sections when not prefixed with .init are already
> handled by the more general .rodata.* matching pattern in the .rodata
> output section.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

So, in hindsight, we'll never get .cst in .data, because of the
"constant" in the name, which rules out a lot of my first attemt.

As this does resolve the problem, Acked-by: Andrew Cooper
<andrew.cooper3@citrix.com>

However ...

> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> ---
>  xen/arch/arm/xen.lds.S | 1 +
>  xen/arch/x86/xen.lds.S | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index e664c4441a..b636d9f223 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -156,6 +156,7 @@ SECTIONS
>         *(.init.rodata)
>         *(.init.rodata.rel)
>         *(.init.rodata.str*)
> +       *(.init.rodata.cst*)

... .init is just a grouping prefix, so I'd recommend that we treat
.init.rodata in exactly the same way as we treat .rodata, so I'd suggest
turning this into

*(.init.rodata)
*(.init.rodata.*)

to match the regular .rodata.

This can easily be adjusted on commit, to save yet-another-round.

~Andrew
Jan Beulich June 27, 2019, 11:25 a.m. UTC | #2
>>> On 27.06.19 at 12:53, <andrew.cooper3@citrix.com> wrote:
> On 27/06/2019 10:33, Roger Pau Monne wrote:
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -156,6 +156,7 @@ SECTIONS
>>         *(.init.rodata)
>>         *(.init.rodata.rel)
>>         *(.init.rodata.str*)
>> +       *(.init.rodata.cst*)
> 
> ... .init is just a grouping prefix, so I'd recommend that we treat
> .init.rodata in exactly the same way as we treat .rodata, so I'd suggest
> turning this into
> 
> *(.init.rodata)
> *(.init.rodata.*)
> 
> to match the regular .rodata.

Or, as suggested elsewhere, make .rodata use less wide matching,
like we do for .init.rodata.

Jan
Roger Pau Monne June 27, 2019, 1:26 p.m. UTC | #3
On Thu, Jun 27, 2019 at 05:25:06AM -0600, Jan Beulich wrote:
> >>> On 27.06.19 at 12:53, <andrew.cooper3@citrix.com> wrote:
> > On 27/06/2019 10:33, Roger Pau Monne wrote:
> >> --- a/xen/arch/arm/xen.lds.S
> >> +++ b/xen/arch/arm/xen.lds.S
> >> @@ -156,6 +156,7 @@ SECTIONS
> >>         *(.init.rodata)
> >>         *(.init.rodata.rel)
> >>         *(.init.rodata.str*)
> >> +       *(.init.rodata.cst*)
> > 
> > ... .init is just a grouping prefix, so I'd recommend that we treat
> > .init.rodata in exactly the same way as we treat .rodata, so I'd suggest
> > turning this into
> > 
> > *(.init.rodata)
> > *(.init.rodata.*)
> > 
> > to match the regular .rodata.
> 
> Or, as suggested elsewhere, make .rodata use less wide matching,
> like we do for .init.rodata.

I'm happy to handle .init.rodata subsections as Xen currently handles
.rodata subsections. There are no custom subsections explicitly added
to either .rodata or .init.rodata, and if we start adding such
subsections explicitly the linker script will likely need
modifications anyway to mark the start and end of those subsections in
the final binary, or the default placement of the wider wildcard will
be already fine.

Thanks, Roger.

Patch
diff mbox series

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index e664c4441a..b636d9f223 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -156,6 +156,7 @@  SECTIONS
        *(.init.rodata)
        *(.init.rodata.rel)
        *(.init.rodata.str*)
+       *(.init.rodata.cst*)
 
        . = ALIGN(POINTER_ALIGN);
        __setup_start = .;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 19aa4332cf..d0c7fbc37b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -216,6 +216,7 @@  SECTIONS
        *(.init.rodata)
        *(.init.rodata.rel)
        *(.init.rodata.str*)
+       *(.init.rodata.cst*)
 
        . = ALIGN(POINTER_ALIGN);
        __setup_start = .;