Re: [Xen-devel] [PATCH v8 04/13] x86: properly calculate xen ELF end of image address

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Sep 29, 2016 at 01:34:30AM -0600, Jan Beulich wrote:
> >>> On 29.09.16 at 00:51, <daniel.kiper@xxxxxxxxxx> wrote:
> > Currently xen ELF end of image address is calculated using first line from
> > "nm -nr xen/xen-syms" output. However, potentially it may contain symbol
> > address not related to the end of image in any way. It can happen if a 
> > symbol
> > is introduced with address larger than _end symbol address. Such situation
> > encountered when I linked xen ELF binary with 
> > xen/arch/x86/efi/relocs-dummy.S.
> > Then first line from "nm -nr xen/xen-syms" contained "ffff82d0c0000000 A 
> > ALT_START"
> > and xen ELF image memory size was silently set to 1023 MiB. This issue 
> > happened
> > because there is no check which symbol address is used to calculate end of
> > image address. So, let's fix it and take ELF end of image address by reading
> > _end symbol address from nm output. This way xen ELF image build process is
> > not prone to changes in order of nm output.
> >
> > This patch is prereq for "efi: build xen.gz with EFI code" patch which adds,
> > among others, xen/arch/x86/efi/relocs-dummy.S to xen.gz output.
>
> I still dislike how you put this. It would make a lot more sense if this
> extra paragraph came much earlier (perhaps right after the first
> sentence), and the rest of the description would then be slightly
> re-worded to fit this aspect.

OK.

> > --- 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) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . _end$$/0x\1/p'`
>
> I thought we had agreed to use __2M_rwdata_end here.

Oh, I forgot about that.

> And also note that I had said "such that the command line argument
> here aligns with the first one on the previous line" in my reply to v7
> regarding the indentation change.

$(TARGET)-syms starts in the same column as $(notes_phdrs) does in the
final output as you requested. However, here it looks that it is
misaligned due to tab at the begging of the line.

Daniel

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