On Tuesday, March 15, 2016, Daniel Kiper <daniel.ki...@oracle.com> wrote:
> On Tue, Mar 15, 2016 at 05:30:20PM +0100, Vladimir 'phcoder' Serbinenko > wrote: > > On Tuesday, March 15, 2016, Vladimir 'phcoder' Serbinenko < > phco...@gmail.com <javascript:;>> > > 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. > > > 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. > If 'RELOCATABLE' means 'image is fine with any of its phdr going anywhere' then why would it declare such a layout if it's fine with second phdr going right after the first one? Why not describe the same thing as 1-2M and 2-3M? > > > > + if (mld.relocatable) > > >> + { > > >> + if (mld.load_base_addr >= mld.link_base_addr) > > >> + grub_multiboot_payload_eip += mld.load_base_addr - > > >> mld.link_base_addr; > > >> + else > > >> + grub_multiboot_payload_eip -= mld.link_base_addr - > > >> mld.load_base_addr; > > >> + } > > >> > > > Both branches are mathematically equivalent. Any reason to have if at > all? > > Yep, you are right. However, it looks that real life (C?) is more > complicated. > I am trying to avoid wrap around here if mld.load_base_addr < > mld.link_base_addr. > If you look at C operator precedence then everything should work. However, > I am not 100% sure that a given compiler will not optimize/break my stuff. > So, maybe we should use signed 64-bit int here. > > Daniel > -- Regards Vladimir 'phcoder' Serbinenko
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel