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 lokeshvutla@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.pallardy@st.com Signed-off-by: Fabien Dessenne fabien.dessenne@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 lokeshvutla@ti.com
Thanks and regards, Lokesh
Thanks Patrick