All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.