xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Kiper <daniel.kiper@oracle.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "jgross@suse.com" <jgross@suse.com>,
	"grub-devel@gnu.org" <grub-devel@gnu.org>,
	"eric.snowberg@oracle.com" <eric.snowberg@oracle.com>,
	"arvidjaar@gmail.com" <arvidjaar@gmail.com>,
	Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>,
	"stefano.stabellini@eu.citrix.com"
	<stefano.stabellini@eu.citrix.com>,
	"cardoe@cardoe.com" <cardoe@cardoe.com>,
	"pgnet.dev@gmail.com" <pgnet.dev@gmail.com>,
	"roy.franz@linaro.org" <roy.franz@linaro.org>,
	"ning.sun@intel.com" <ning.sun@intel.com>,
	"david.vrabel@citrix.com" <david.vrabel@citrix.com>,
	"jbeulich@suse.com" <jbeulich@suse.com>,
	"andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"qiaowei.ren@intel.com" <qiaowei.ren@intel.com>,
	"richard.l.maliszewski@intel.com"
	<richard.l.maliszewski@intel.com>,
	"gang.wei@intel.com" <gang.wei@intel.com>"fu.wei@linaro.org" <f>
Subject: Re: [GRUB2 PATCH v4 4/4] multiboot2: Add support for relocatable images
Date: Wed, 16 Mar 2016 11:34:19 +0100	[thread overview]
Message-ID: <20160316103419.GG31771__41742.7493645393$1458124555$gmane$org@olila.local.net-space.pl> (raw)
In-Reply-To: <20160315235408.GF29495@char.us.oracle.com>

On Tue, Mar 15, 2016 at 07:54:08PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 15, 2016 at 10:42:21PM +0100, Daniel Kiper wrote:
> > On Tue, Mar 15, 2016 at 05:30:20PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> > > On Tuesday, March 15, 2016, Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
> > > wrote:
> > >
> > > >
> > > >> +           if (mld->relocatable)
> > > >> +             err = grub_relocator_alloc_chunk_align
> > > >> (grub_multiboot_relocator, &ch,
> > > >> +                                                     mld->min_addr,
> > > >> mld->max_addr - phdr(i)->p_memsz,
> > > >> +                                                     phdr(i)->p_memsz,
> > > >> mld->align ? mld->align : 1,
> > > >> +                                                     mld->preference,
> > > >> mld->avoid_efi_boot_services);
> > > >> +           else
> > > >> +             err = grub_relocator_alloc_chunk_addr
> > > >> (grub_multiboot_relocator,
> > > >> +                                                    &ch,
> > > >> phdr(i)->p_paddr,
> > > >> +                                                    phdr(i)->p_memsz);
> > > >>
> > > > I believe this is faulty if you have more than one PHDR. You load every
> >
> > Argh... You are right!
> >
> > > > PHDR individually to essentially random address. Pieces have no reasonable
> > > > way to find each other. Moreover entry point calculation is also faulty.
> > > > Imagine sth like this:
> > > > PHDR 1M-2M
> > > > PHDR 2M-5M
> > > > Entry point 2.5M (in second PHDR)
> > > > then if first PHDR is loaded to 1M and second to 10M then base and link
> > > > addr are both 1M, so entry point will be calculated as 2.5M, which points
> > > > to no segment. I see 2 solutions:
> > > > 1) Look where entry falls in original layout, then adjust it in accordance
> > > > with where this phdr will be loaded. This requires least efforts. Finding
> > > > different PHDRs is still impossible but it will be possible in the future
> > > > with relocations.
> >
> > It looks that we should store somewhere and export to image via relevant tags
> > link addresses and load addresses. Hmmm... Maybe we should just provide load
> > addresses to image. Image can have link addresses in its data. And this
> > probably does not require huge changes.
> >
> > > > 2) Allocate a buffer of size highest - lowest and load everything into
> > > > this buffer keeping relative offsets. If we do this, then we need to
> > > > document if it's required for boorloader to behave this way or not. If it
> > > > is, we can in future provide a tag to say that image is fine with
> > > > rearrangement of PHDR, if it ever becomes relevant (I heavily doubt it).
> > > > I guess that xen is a single phdr image and so essentially any code will
> > > > work with it.
>
> Won't be in Xen 4.7.
> > > > This problem appears in couple of other places, I'll skip commenting on
> > > > them explicitly.
> > > >
> > > I take back the part "requires least effort" for solution 1. Solution 2 is
> > > probably simpler and less error-prone as developper doesn't control if
> > > binutils decode to put several phdrs.
> >
> > #2 looks promising but what if PHDR_1 is at 1 MiB - 2 MiB and PHDR_2 is at
> > 808 MiB - 809 MiB? Then we will allocate more than 800 MiB just for an
> > unusable hole. So, I think that we should go that way if solution #1
> > is too complicated.
>
> Daniel, my xSplice patches make the Xen have two ELF PHDRS: 1)the PT_LOAD
> and 2) PT_NOTE (which points to smack in the .text section) so you can try
> that as an example payload.
>
> (If you want to put your patches on top of mine:
> git://xenbits.xen.org/people/konradwilk/xen.git #xsplice.v4)

Thanks, but multiboot2 loads just PT_LOAD segments.

Daniel

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

  reply	other threads:[~2016-03-16 10:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-15 15:25 [GRUB2 PATCH v4 0/4] multiboot2: Add two extensions Daniel Kiper
2016-03-15 15:25 ` [GRUB2 PATCH v4 1/4] i386/relocator: Add grub_relocator64_efi relocator Daniel Kiper
2016-03-15 16:00   ` Vladimir 'phcoder' Serbinenko
     [not found]   ` <CAEaD8JO23MkxBVF7ZqL8pDDz_85Nof1kbMo-RtxMam8S2KOeOg@mail.gmail.com>
2016-03-15 19:59     ` Daniel Kiper
2016-03-15 15:25 ` [GRUB2 PATCH v4 2/4] multiboot2: Add tags used to pass ImageHandle to loaded image Daniel Kiper
2016-03-15 16:03   ` Vladimir 'phcoder' Serbinenko
2016-03-15 23:39   ` Konrad Rzeszutek Wilk
2016-03-15 15:26 ` [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled Daniel Kiper
2016-03-15 23:46   ` Konrad Rzeszutek Wilk
     [not found]   ` <20160315234646.GE29495@char.us.oracle.com>
2016-03-16 10:02     ` Daniel Kiper
     [not found]     ` <20160316100214.GF31771@olila.local.net-space.pl>
2016-03-16 10:14       ` Toomas Soome
     [not found]       ` <D5F25A44-6157-46FB-B717-A7ED06FD01C8@me.com>
2016-03-16 10:39         ` Vladimir 'phcoder' Serbinenko
2016-03-16 13:06           ` Konrad Rzeszutek Wilk
2016-03-15 15:26 ` [GRUB2 PATCH v4 3/4 - FOR COMMIT] " Daniel Kiper
2016-03-15 16:07   ` [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] " Vladimir 'phcoder' Serbinenko
     [not found]   ` <CAEaD8JP6BJQkrLSN+GgPDqYtiXMgq=A3CnhbVHv9xZ57x4NH6Q@mail.gmail.com>
2016-03-15 18:06     ` Andrei Borzenkov
     [not found]     ` <56E84F10.5080804@gmail.com>
2016-03-15 18:10       ` Vladimir 'phcoder' Serbinenko
     [not found]       ` <CAEaD8JMgHphk31781sFpZVcT5+Q+Sj1LGpNfvhr3cbT=+sU1Lg@mail.gmail.com>
2016-03-15 20:59         ` Daniel Kiper
2016-03-15 20:01     ` Daniel Kiper
2016-03-15 15:26 ` [GRUB2 PATCH v4 4/4] multiboot2: Add support for relocatable images Daniel Kiper
2016-03-15 16:27   ` Vladimir 'phcoder' Serbinenko
     [not found]   ` <CAEaD8JOin-GSP8+kqC3bnS-_boKzvFgV-WoByLrkDOaeNThMGg@mail.gmail.com>
2016-03-15 16:30     ` Vladimir 'phcoder' Serbinenko
     [not found]     ` <CAEaD8JPS9cbmmS+a0pjCMboGTd-jkwCZp59KYpnEhOgpGva6zw@mail.gmail.com>
2016-03-15 21:42       ` Daniel Kiper
     [not found]       ` <20160315214221.GE31771@olila.local.net-space.pl>
2016-03-15 23:54         ` Konrad Rzeszutek Wilk
2016-03-16 10:34           ` Daniel Kiper [this message]
2016-03-16 10:41         ` Vladimir 'phcoder' Serbinenko

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='20160316103419.GG31771__41742.7493645393$1458124555$gmane$org@olila.local.net-space.pl' \
    --to=daniel.kiper@oracle.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=arvidjaar@gmail.com \
    --cc=cardoe@cardoe.com \
    --cc=david.vrabel@citrix.com \
    --cc=eric.snowberg@oracle.com \
    --cc=gang.wei@intel.com \
    --cc=grub-devel@gnu.org \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=ning.sun@intel.com \
    --cc=pgnet.dev@gmail.com \
    --cc=phcoder@gmail.com \
    --cc=qiaowei.ren@intel.com \
    --cc=richard.l.maliszewski@intel.com \
    --cc=roy.franz@linaro.org \
    --cc=stefano.stabellini@eu.citrix.com \
    --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).