From: Daniel Kiper <daniel.kiper@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Juergen Gross <JGross@suse.com>,
sstabellini@kernel.org, andrew.cooper3@citrix.com,
cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
qiaowei.ren@intel.com, gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v6 01/15] x86: properly calculate ELF end of image address
Date: Tue, 20 Sep 2016 20:00:01 +0200 [thread overview]
Message-ID: <20160920180001.GD28753@olila.local.net-space.pl> (raw)
In-Reply-To: <57E017B20200007800110279@prv-mh.provo.novell.com>
On Mon, Sep 19, 2016 at 08:52:02AM -0600, Jan Beulich wrote:
> >>> On 19.09.16 at 15:56, <daniel.kiper@oracle.com> wrote:
> > On Mon, Sep 19, 2016 at 05:14:07AM -0600, Jan Beulich wrote:
[...]
> >> 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.
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
[...]
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.
Daniel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-09-20 18:00 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-12 20:18 [PATCH v6 00/15] x86: multiboot2 protocol support Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 01/15] x86: properly calculate ELF end of image address Daniel Kiper
2016-09-16 20:43 ` Konrad Rzeszutek Wilk
2016-09-19 11:14 ` Jan Beulich
2016-09-19 13:56 ` Daniel Kiper
2016-09-19 14:52 ` Jan Beulich
2016-09-20 11:43 ` Daniel Kiper
2016-09-20 13:28 ` Jan Beulich
2016-09-20 18:00 ` Daniel Kiper [this message]
2016-09-21 9:37 ` Jan Beulich
2016-09-22 11:45 ` Daniel Kiper
2016-09-22 12:02 ` Jan Beulich
2016-09-19 11:18 ` Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 02/15] x86/boot/reloc: create generic alloc and copy functions Daniel Kiper
2016-09-19 11:23 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 03/15] x86/boot/reloc: rename some variables and rearrange code a bit Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 04/15] x86: add multiboot2 protocol support Daniel Kiper
2016-09-19 11:42 ` Jan Beulich
2016-09-21 8:56 ` Daniel Kiper
2016-09-21 9:41 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 05/15] efi: create efi_enabled() Daniel Kiper
2016-09-19 11:58 ` Jan Beulich
2016-09-19 14:27 ` Daniel Kiper
2016-09-19 14:31 ` Wei Liu
2016-09-19 14:57 ` Jan Beulich
2016-09-19 15:38 ` Daniel Kiper
2016-09-19 16:00 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 06/15] x86: allow EFI reboot method neither on EFI platforms Daniel Kiper
2016-09-19 12:01 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 07/15] efi: build xen.gz with EFI code Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 08/15] x86/efi: create new early memory allocator Daniel Kiper
2016-09-19 12:12 ` Jan Beulich
2016-09-19 15:04 ` Daniel Kiper
2016-09-19 15:17 ` Jan Beulich
2016-09-20 9:45 ` Daniel Kiper
2016-09-20 9:57 ` Jan Beulich
2016-09-20 10:52 ` Daniel Kiper
2016-09-20 13:46 ` Jan Beulich
2016-09-20 18:45 ` Daniel Kiper
2016-09-21 9:42 ` Jan Beulich
2016-09-22 10:52 ` Daniel Kiper
2016-09-22 11:25 ` Jan Beulich
2016-09-22 12:07 ` Daniel Kiper
2016-09-22 13:12 ` Jan Beulich
2016-09-22 17:07 ` Julien Grall
2016-09-23 10:50 ` Daniel Kiper
2016-09-23 11:07 ` Julien Grall
2016-09-23 11:35 ` Daniel Kiper
2016-09-23 11:42 ` Julien Grall
2016-09-23 11:53 ` Daniel Kiper
2016-09-23 14:10 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 09/15] x86: add multiboot2 protocol support for EFI platforms Daniel Kiper
2016-09-19 12:29 ` Jan Beulich
2016-09-19 15:18 ` Daniel Kiper
2016-09-19 15:28 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 10/15] x86/boot: implement early command line parser in C Daniel Kiper
2016-09-19 12:47 ` Jan Beulich
2016-09-19 15:27 ` Daniel Kiper
2016-09-19 16:03 ` Jan Beulich
2016-09-20 9:49 ` Daniel Kiper
2016-09-20 10:05 ` Jan Beulich
2016-09-20 11:31 ` Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 11/15] x86: change default load address from 1 MiB to 2 MiB Daniel Kiper
2016-09-19 12:54 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 12/15] x86/setup: use XEN_IMG_OFFSET instead of Daniel Kiper
2016-09-19 13:00 ` Jan Beulich
2016-09-12 20:18 ` [PATCH v6 13/15] x86: make Xen early boot code relocatable Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 14/15] x86/boot: rename sym_phys() to sym_offs() Daniel Kiper
2016-09-12 20:18 ` [PATCH v6 15/15] x86: add multiboot2 protocol support for relocatable images Daniel Kiper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160920180001.GD28753@olila.local.net-space.pl \
--to=daniel.kiper@oracle.com \
--cc=JBeulich@suse.com \
--cc=JGross@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=cardoe@cardoe.com \
--cc=david.vrabel@citrix.com \
--cc=fu.wei@linaro.org \
--cc=gang.wei@intel.com \
--cc=ning.sun@intel.com \
--cc=pgnet.dev@gmail.com \
--cc=qiaowei.ren@intel.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).