| Message ID | 54C7D22C.5040202@freebsd.org |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, 2015-01-27 at 10:00 -0800, Nathan Whitehorn wrote: > On 01/27/15 09:59, Nathan Whitehorn wrote: > > On 01/22/15 09:02, Benjamin Herrenschmidt wrote: > >> On Wed, 2015-01-21 at 08:45 -0800, Nathan Whitehorn wrote: > >>> The attached patch fixes the big-endian ELF64 loader in skiboot to > >>> handle the fact that the ELF entry point is specified to point to a > >>> function descriptor describing the entry point rather than the entry > >>> point itself. (I haven't set it up to load the TOC base pointer though) > >>> This is required to load the FreeBSD kernel as a skiboot payload. The > >>> patch does not touch the little-endian loader since I'm not sure if the > >>> ELFv2 spec still has function descriptors or not. > >> An additional problem is that the Linux 64-bit BE kernel vmlinux doesn't > >> have a correct function descriptor as an entry point :-( So that patch > >> breaks loading a raw BE vmlinux... We probably need a quirk to recognize > >> such an image, possibly via the fact that the value that would be in the > >> function descriptor cannot possibly be a valid entry point as part of > >> the image. > > > > Here's a second version of the patch that can load both Linux and > > FreeBSD. It iterates through the ELF section table and, if the entry > > point points to an executable section, treats the entry point as the > > first instruction to run. Otherwise, it treats it as a function > > descriptor. > > -Nathan > > Sorry, missed a file in the patch. Here's the right one. I would have gone for something even simpler such as checking the boundary of the descriptor :-) But what you did is probably cleaner. Can you re-submit in the appropriate format ? I'm not sure we've documented it yet but we follow the same practices as Linux here that is a subject, a changeset comment, a Signed-off-by line, then the patch (the important thing for us is the S-O-B without which we cannot apply it). Cheers, Ben. > -Nathan
Nathan Whitehorn <nwhitehorn@freebsd.org> writes: > On 01/27/15 09:59, Nathan Whitehorn wrote: >> On 01/22/15 09:02, Benjamin Herrenschmidt wrote: >>> On Wed, 2015-01-21 at 08:45 -0800, Nathan Whitehorn wrote: >>>> The attached patch fixes the big-endian ELF64 loader in skiboot to >>>> handle the fact that the ELF entry point is specified to point to a >>>> function descriptor describing the entry point rather than the entry >>>> point itself. (I haven't set it up to load the TOC base pointer though) >>>> This is required to load the FreeBSD kernel as a skiboot payload. The >>>> patch does not touch the little-endian loader since I'm not sure if the >>>> ELFv2 spec still has function descriptors or not. >>> An additional problem is that the Linux 64-bit BE kernel vmlinux doesn't >>> have a correct function descriptor as an entry point :-( So that patch >>> breaks loading a raw BE vmlinux... We probably need a quirk to recognize >>> such an image, possibly via the fact that the value that would be in the >>> function descriptor cannot possibly be a valid entry point as part of >>> the image. >> >> Here's a second version of the patch that can load both Linux and >> FreeBSD. It iterates through the ELF section table and, if the entry >> point points to an executable section, treats the entry point as the >> first instruction to run. Otherwise, it treats it as a function >> descriptor. Cool. I'll let Ben do some ack/review on it. It'd be great if you could resend with a Signed-off-by line (we follow Signed-off-by etc just like linux kernel does), that'd be great!
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > Can you re-submit in the appropriate format ? I'm not sure we've > documented it yet but we follow the same practices as Linux here that > is a subject, a changeset comment, a Signed-off-by line, then the patch > (the important thing for us is the S-O-B without which we cannot apply > it). I now mention it in the README and I just point to the linux Documentation/SubmittingPatches rather than having spent any time documenting it separately. We possibly should... and I wonder if we should co-ordinate somewhat with other OpenPower related projects so that we get to a consistent way of working.
diff --git a/core/init.c b/core/init.c index 2c7e30c..e75b5a3 100644 --- a/core/init.c +++ b/core/init.c @@ -111,6 +111,7 @@ static bool try_load_elf64(struct elf_hdr *header) struct elf64_hdr *kh = (struct elf64_hdr *)header; uint64_t load_base = (uint64_t)kh; struct elf64_phdr *ph; + struct elf64_shdr *sh; unsigned int i; /* Check it's a ppc64 LE ELF */ @@ -152,6 +153,27 @@ static bool try_load_elf64(struct elf_hdr *header) prerror("INIT: Failed to find kernel entry !\n"); return false; } + + /* For the normal big-endian ELF ABI, the kernel entry points + * to a function descriptor in the data section. Linux instead + * has it point directly to code. Test whether it is pointing + * into an executable section or not to figure this out. Default + * to assuming it obeys the ABI. + */ + sh = (struct elf64_shdr *)(load_base + kh->e_shoff); + for (i = 0; i < kh->e_shnum; i++, sh++) { + if (sh->sh_addr > kh->e_entry || + (sh->sh_addr + sh->sh_size) <= kh->e_entry) + continue; + + break; + } + + if (i == kh->e_shnum || !(sh->sh_flags & ELF_SFLAGS_X)) { + kernel_entry = *(uint64_t *)(kernel_entry + load_base); + kernel_entry = kernel_entry - ph->p_vaddr + ph->p_offset; + } + kernel_entry += load_base; kernel_32bit = false; diff --git a/include/elf.h b/include/elf.h index 0a52f3e..c600f7f 100644 --- a/include/elf.h +++ b/include/elf.h @@ -76,6 +76,23 @@ struct elf64_phdr { uint64_t p_align; }; +/* 64-bit ELF section header */ +struct elf64_shdr { + uint32_t sh_name; + uint32_t sh_type; + uint64_t sh_flags; +#define ELF_SFLAGS_X 0x4 +#define ELF_SFLAGS_A 0x2 +#define ELF_SFLAGS_W 0x1 + uint64_t sh_addr; + uint64_t sh_offset; + uint64_t sh_size; + uint32_t sh_link; + uint32_t sh_info; + uint64_t sh_addralign; + uint64_t sh_entsize; +}; + /* Some relocation related stuff used in relocate.c */ struct elf64_dyn { int64_t d_tag;