[U-Boot] [PATCH v3 4/7] remoteproc: add elf file load support

[Lokesh Vutla]

On 31/05/19 6:41 PM, Fabien Dessenne wrote:
> The current implementation supports only binary file load.
> Add helpers to support ELF32 format (sanity check, and load).
> Note that since an ELF32 image is built for the remote processor, the
> load function uses the device_to_virt ops to translate the addresses.
> Implement a basic translation for sandbox_testproc.
> 
> Add related tests. Test result:
> => ut dm remoteproc_elf
> Test: dm_test_remoteproc_elf: remoteproc.c
> Test: dm_test_remoteproc_elf: remoteproc.c (flat tree)
> Failures: 0
> 
> Signed-off-by: Loic Pallardy <loic.pallardy at st.com>
> Signed-off-by: Fabien Dessenne <fabien.dessenne at st.com>
> ---

[...snip...]

> +/* Basic function to verify ELF32 image format */
> +int rproc_elf32_sanity_check(ulong addr, ulong size)
> +{
> +	Elf32_Ehdr *ehdr;
> +	char class;
> +
> +	if (!addr) {
> +		pr_debug("Invalid fw address?\n");
> +		return -EFAULT;
> +	}
> +
> +	if (size < sizeof(Elf32_Ehdr)) {
> +		pr_debug("Image is too small\n");
> +		return -ENOSPC;
> +	}
> +
> +	ehdr = (Elf32_Ehdr *)addr;
> +	class = ehdr->e_ident[EI_CLASS];
> +
> +	if (!IS_ELF(*ehdr) || ehdr->e_type != ET_EXEC || class != ELFCLASS32) {
> +		pr_debug("Not an executable ELF32 image\n");
> +		return -EPROTONOSUPPORT;
> +	}
> +
> +	/* We assume the firmware has the same endianness as the host */
> +# ifdef __LITTLE_ENDIAN
> +	if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
> +# else /* BIG ENDIAN */
> +	if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) {
> +# endif
> +		pr_debug("Unsupported firmware endianness\n");
> +		return -EILSEQ;
> +	}
> +
> +	if (size < ehdr->e_shoff + sizeof(Elf32_Shdr)) {
> +		pr_debug("Image is too small\n");
> +		return -ENOSPC;
> +	}
> +
> +	if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) {
> +		pr_debug("Image is corrupted (bad magic)\n");
> +		return -EBADF;
> +	}
> +
> +	if (ehdr->e_phnum == 0) {
> +		pr_debug("No loadable segments\n");
> +		return -ENOEXEC;
> +	}
> +
> +	if (ehdr->e_phoff > size) {
> +		pr_debug("Firmware size is too small\n");
> +		return -ENOSPC;
> +	}
> +
> +	return 0;
> +}
> +
> +/* A very simple elf loader, assumes the image is valid */
> +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr)
> +{
> +	Elf32_Ehdr *ehdr; /* Elf header structure pointer */
> +	Elf32_Phdr *phdr; /* Program header structure pointer */
> +	const struct dm_rproc_ops *ops;
> +	unsigned int i;
> +

I would prefer to call  rproc_elf32_sanity_check() here and reduce the burden on
user. It's my preference and no strong objections.

Other than that:

Reviewed-by: Lokesh Vutla <lokeshvutla at ti.com>

Thanks and regards,
Lokesh