Re: [PATCH] Elf loader fixes

[Gerd Hoffmann <kraxel@suse.de>]

  Hi,

> I have two issues with your latest patch:
> - the heuristics you use to distinguish between "old" and "new" kernel
> images, especially the case where a valid "new" kernel image could be
> mistaken for an "old" image.

I don't think it can ever happen, at least not with linux.  The
(correct) paddr addresses must be relatively small, otherwise loading
the kernel on machines with a small amount of memory will not work.
Especially they must be much smaller than the usual linux kernel VIRT_BASE.

Not sure about other OS'es, maybe there are some which use a small
VIRT_BASE, then it could be problematic, yes.

> - the change to the entry point, now entry point and the elf header
> paddr fields are in different address modes.

xen/include/xen/elf.h says entry point is virtual.  IMHO the elf headers
should be correctly filled.

> - we want to be able to specify where the virtual address space mapped
> by the initial pagetables starts, this is why we have VIRT_BASE

Sure.

> - since we always run in virtual address mode, we thought that the
> loader should also do so, thus it will load the image to a virtual
> address - this seems to be in line with how other loaders would read
> the elf header paddr fields, i.e. use them as load addresses.

Well, loadelfimage() in tools/libxc/xc_load_elf.c copyes the image
page-by-page to the _physical_ addresses.  And this is how it should be
IMHO.  virt_base is only used to create the initial virtual mappings as
expected by the guest kernel (and for bug-compatibility with old kernels).

The dom0 builder can take a shortcut and simply copy the big blobs to
virtual addresses (paddr + virt_base).  That works because the initial
virtual mapping is a simple offset to the physical address and the
mappings are already created and active at that point.

> If we really need to get rid of how we change LOAD_OFFSET in Linux,
> how about this:
> we add another entry to the __xen_guest header, PHYS_OFFSET=0 and then
> substract PHYS_OFFSET from all elf header paddr fiels to turn them
> into physical addresses, respectively, resp. add VIRT_BASE-PHYS_OFFSET
> to turn them into virtual addresses.  We default PHYS_OFFSET to
> VIRT_BASE if it's not present in the __xen_guest header.

I don't think the new PHYS_OFFSET is needed.

cheers,

  Gerd

-- 
Gerd 'just married' Hoffmann <kraxel@suse.de>
I'm the hacker formerly known as Gerd Knorr.
http://www.suse.de/~kraxel/just-married.jpeg