Re: [Xen-devel] [PATCH v6 01/15] x86: properly calculate ELF end of image address

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Fri, Sep 16, 2016 at 04:43:21PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Sep 12, 2016 at 10:18:16PM +0200, Daniel Kiper wrote:
> > Currently ELF end of image address is calculated using first line from
> > "nm -nr xen/xen-syms" output. However, today usually it contains random
> > symbol address not related to end of image in any way. So, it looks
>
> Weird. The -n says:
>
> "  -n
>        -v
>        --numeric-sort
>            Sort symbols numerically by their addresses, rather than 
> alphabetically by their names.
> "
>
> And you are right. It is ignoring it:
>
> [konrad@char xen]$ nm -nr xen/xen-syms| sort | head -1
> ffff82d080000000 T __image_base__
> [konrad@char xen]$ nm -nr xen/xen-syms | head -1
> ffff82d08033d000 B efi
>
> [konrad@char xen]$ nm -nr xen/xen-syms| sort | tail -5
> ffff82d08033cb00 B _end
> ffff82d08033cb00 B __per_cpu_data_end
> ffff82d08033d000 B __2M_rwdata_end
> ffff82d08033d000 B efi
>                  U _GLOBAL_OFFSET_TABLE_

Well, TBH, I have never checked what "-nr" means. However, it looks
that it works at least with nm from binutils 2.22. Please look below:

ffff82d080345000 A efi
ffff82d080345000 A __2M_rwdata_end
ffff82d080344b80 A _end
ffff82d080344b80 B __per_cpu_data_end
ffff82d080344b80 B __bss_end
ffff82d080344b28 b per_cpu__vmxon_region
ffff82d080344b20 b per_cpu__root_vmcb
ffff82d080344b18 b per_cpu__hsa

[...]

Anyway, I think that we should apply my fix. Though I can agree that we
do not need "-nr" for nm here any more.

> > that for years that stuff have been working just by lucky coincidence.
> > Hence, it have to be changed to something more reliable. So, let's take
> > ELF end of image address by reading _end symbol address from nm output.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/Makefile |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > index d3875c5..a4fe740 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -91,7 +91,7 @@ endif
> >
> >  $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
> >     ./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) 0x100000 \
> > -   `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
> > +           `$(NM) -nr $(TARGET)-syms | awk '$$3 == "_end" {print "0x"$$1}'`
> >
>
> Something is off with your tabs/spaces.

I think that it is OK. I added second tab to mark that it is a continuation.

> I would also modify the arch/x86/xen.lds.S and put a comment
> around _end = .; to mention this dependency - in case somebody adds some
> extra things after _end.

I am not sure it is needed. However, if Andrew and Jan do not object I can add 
that.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel