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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Mon, Sep 19, 2016 at 08:52:02AM -0600, Jan Beulich wrote:
> >>> On 19.09.16 at 15:56, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Mon, Sep 19, 2016 at 05:14:07AM -0600, Jan Beulich wrote:
> >> >>> On 16.09.16 at 22:43, <konrad.wilk@xxxxxxxxxx> 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
> >>
> >> So what is it ignoring, you think? The -n? Surely not. Are you perhaps
> >> overlooking that -r means "reverse" (and hence that piping through
> >> "sort" inverts all the sorting done by nm)?
> >>
> >> > [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_
> >> >
> >> >> 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.
> >>
> >> So before taking this patch I'd really like to see proof that what gets
> >> done currently does actually go wrong in at least one case. So far I
> >
> > During initial work on this patch series I discovered that p_memsz in xen
> > ELF file is set to unreasonably huge value, IIRC, ~1 GiB. That is why at
> > first I enforced 16 MiB here (just temporary workaround) and later proposed
> > this patch. Sadly, right now I am not able to find commit which introduced
> > this issue. However, it seems that it was "fixed" later by another commit.
> >
> > Anyway, I am not sure why are you against, IMO, more reliable solution.
> > This way we would avoid in the future similar issues which I described
> > above.
>
> I'm not against anything if it gets properly explained. Here,
> however, you present some vague statements which even you
> can't verify right now as it looks.
>
> And then I'm not sure going from one way of using nm to another
> is all that much of an improvement. A true improvement might be
> to do away with the nm use and e.g. also consider the section
> table.

However, it looks that this requires changes in mkelf32.c and
I do not think that we should play with it right now.

> >> can't see things working as "just by lucky coincidence". In particular
> >> I can't see why __2M_rwdata_end (aliased to efi) does not relate to
> >> the intended image end. Once we switch back to using large pages
> >> even when not using xen.efi I'd even question whether preferring
> >> _end over __2M_rwdata_end is actually correct.
> >
> > This is good question. I was thinking about that after posting v6. It seems
> > that from image POV _end is correct. However, taking into account that Xen
> > image mapping covers 16 MiB then I think we should use __2M_rwdata_end (or
> > define __end_of_image__ symbol equal to __2M_rwdata_end). This way boot
> > loader will allocate memory region for Xen image later covered properly by
> > mapping, nothing will be put in memory immediately after the Xen image and
> > later incorrectly mapped as Xen image.

Current xen image p_memsz does not end at 16 MiB. It seems to me that this is
incorrect. Should I fix it? It looks that we just have to move out .pad of
#ifdef EFI. Are you OK with it?

Daniel

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