Hi Lokesh,
Thanks for the review.
Fabien is in vacation and I will integrate this serie in my next stm32 pull
request.
> From: Lokesh Vutla <lokeshvu...@ti.com>
> Sent: lundi 3 juin 2019 07:31
>
>
>
> 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.palla...@st.com>
> > Signed-off-by: Fabien Dessenne <fabien.desse...@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.
Yes it is a possibility, but for my side I prefer the Fabien proposition.
(we can perhaps reuse the check of ELF for other use case).
I will merge the patch with this version (to have the patch in v2019.10) .
But I let Fabien conclude and potentially sent a minor update.
> Other than that:
>
> Reviewed-by: Lokesh Vutla <lokeshvu...@ti.com>
>
> Thanks and regards,
> Lokesh
Thanks
Patrick
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
