[Skiboot] [PATCH v2] Allow ELF executables with function descriptor entrypoint to be skiboot payloads

[Benjamin Herrenschmidt]

On Tue, 2015-01-27 at 17:00 -0800, Nathan Whitehorn wrote:
> Big-endian ELF64 ELF executables normally (the Linux kernel is an 
> exception) have their entry point refer to a function descriptor instead 
> of the first instruction. Distinguish between the Linux case and the 
> function descriptor case, which is used for the FreeBSD kernel, by 
> checking whether the entry point points into an executable section or 
> not. This allows use of the FreeBSD kernel as a skiboot payload.

Thanks !

I did a closer review and all I found was a minor style nit,
Stewart can probably fix it up while applying (see below).

Note also that in *theory* there can be ABI v1 LE kernels but
I wouldn't make a fuss here, I doubt that will concern freebsd.

> Signed-off-by: Nathan Whitehorn <nwhitehorn at freebsd.org>
> ---
>   core/init.c   | 22 ++++++++++++++++++++++
>   include/elf.h | 17 +++++++++++++++++
>   2 files changed, 39 insertions(+)
> 
> 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;

I would reverse the test polarity and break instead of continuing, thus
avoiding the line below:

> +		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;
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot