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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Wed, Sep 21, 2016 at 03:37:52AM -0600, Jan Beulich wrote:
> >>> On 20.09.16 at 20:00, <daniel.kiper@xxxxxxxxxx> wrote:
> > OK, I did some more tests and found out that after patch "efi: build
> > xen.gz with EFI code" we have following xen ELF file:
> >
> > Program Headers:
> >   Type           Offset   VirtAddr   PhysAddr   FileSiz  MemSiz     Flg 
> > Align
> >   LOAD           0x000080 0x00100000 0x00100000 0x20c120 0x3ff00000 RWE 0x40
> >                                                          ^^^^^^^^^^
> >
> > because "nm -nr xen/xen-syms" emits:
> >
> > ffff82d0c0000000 A ALT_START
> > ffff82d08034d000 A __2M_rwdata_end
> > ffff82d08034cc00 A _end
> > ffff82d08034cc00 B __per_cpu_data_end
> > ffff82d08034cc00 B __bss_end
>
> So it is your own change that breaks this.

Yes, it is but it happens because current method calculating end of
image address is weak. IMO, we should not blindly assume that first
line from "nm -nr" contains proper end of image address. However,
I can agree that maybe ALT_START at consortes are not needed here.
Though I think this is separate issue.

> > ALT_START lives in xen/arch/x86/efi/relocs-dummy.S. relocs-dummy.S
> > provides __base_relocs_start and __base_relocs_end symbols which
> > are referenced in xen/arch/x86/efi/efi-boot.h:efi_arch_relocate_image().
> > Of course one can argue that maybe we should do some changes in
> > efi_arch_relocate_image() and/or xen/arch/x86/efi/relocs-dummy.S.
> > This is true. However, this is separate issue and we should consider
> > it as such.
> >
> > "efi: build xen.gz with EFI code" patch clearly shows that current
> > method used to calculate <final-exec-addr> mkelf32 argument is based
> > on bogus assumptions and very fragile. So, IMO, it should be changed
> > to something which is more reliable. And my proposal looks quite good.
>
> What about the alternative of simply stripping ALT_START (and then
> also VIRT_START) from the image? They're getting screwed up in
> xen.efi already anyway (due to not being representable in a COFF
> symbol table entry), and hence can't possibly be useful to retain.

Do you think about "strip -N ALT_START xen/xen-syms"? I can add that.
However, I am still not sure why do not you want change currently
existing solution used to calculate end of image address. I showed
that it is easy to break. So, why we must live with it?

> But no matter what route we go: Such a change needs to have a
> description that properly explains the issue. Vague statements are
> not enough.

Could you be more specific? Where are my description(s) vague? What
should be added or removed?

> And btw, vaguely recalling earlier issues (long ago) with awk usage
> (on non-Linux platforms) I'd also prefer if such an adjustment would
> get away without introducing another use of awk. I think what you
> want could easily be achieved by using sed.

OK, I can use sed instead of awk if you wish.

Daniel

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