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

2019-05-29 Thread Fabien DESSENNE
Hi Lokesh


On 29/05/2019 6:21 AM, Lokesh Vutla wrote:
>
> On 27/05/19 5:53 PM, Fabien Dessenne wrote:
>> The current implementation supports only binary file load.
>> Add helpers to support ELF format (sanity check, and load).
>> Note that since an ELF 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 
>> Signed-off-by: Fabien Dessenne 
> Can you create a new file(rproc-elf-loader.c) or something similar for elf
> loading support.


It sounds good, I'll do that.


>   Ill be sending 64bit elf loading support soon. Instead of
> cluttering rproc-uclass, it is better to separate out elf loading support.


Since you plan support for elf64, I may consider renaming the elf 
functions in elf32. I'll check this too.

BR

Fabien


>
> Thanks and regards,
> Lokesh
>
>> ---
>>   drivers/remoteproc/rproc-uclass.c |  99 +++
>>   drivers/remoteproc/sandbox_testproc.c |  19 ++
>>   include/remoteproc.h  |  30 -
>>   test/dm/remoteproc.c  | 122 
>> ++
>>   4 files changed, 267 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/rproc-uclass.c 
>> b/drivers/remoteproc/rproc-uclass.c
>> index c8a41a6..4d85732 100644
>> --- a/drivers/remoteproc/rproc-uclass.c
>> +++ b/drivers/remoteproc/rproc-uclass.c
>> @@ -5,6 +5,7 @@
>>*/
>>   #define pr_fmt(fmt) "%s: " fmt, __func__
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>   #include 
>> @@ -291,6 +292,104 @@ int rproc_dev_init(int id)
>>  return ret;
>>   }
>>   
>> +/* Basic function to verify ELF image format */
>> +int rproc_elf_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_elf_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;
>> +
>> +ehdr = (Elf32_Ehdr *)addr;
>> +phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
>> +
>> +ops = rproc_get_ops(dev);
>> +
>> +/* Load each program header */
>> +for (i = 0; i < ehdr->e_phnum; ++i) {
>> +void *dst = (void *)(uintptr_t)phdr->p_paddr;
>> +void *src = (void *)addr + phdr->p_offset;
>> +
>> +if (phdr->p_type != PT_LOAD)
>> +continue;
>> +
>> +if (ops->device_to_virt)
>> +dst = ops->device_to_virt(dev, (ulong)dst);
>> +
>> +dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
>> +i, dst, phdr->p_filesz);
>> +if (phdr->p_filesz)
>> +memcpy(dst, src, phdr->p_filesz);
>> +if (phdr->p_filesz != phdr->p_memsz)
>> +memset(dst + phdr->p_filesz, 0x00,
>> +   phdr->p_memsz - phdr->p_filesz);
>> +flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
>> +roundup((unsigned long)dst + phdr->p_filesz,
>> +   

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

2019-05-28 Thread Lokesh Vutla


On 27/05/19 5:53 PM, Fabien Dessenne wrote:
> The current implementation supports only binary file load.
> Add helpers to support ELF format (sanity check, and load).
> Note that since an ELF 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 
> Signed-off-by: Fabien Dessenne 

Can you create a new file(rproc-elf-loader.c) or something similar for elf
loading support. Ill be sending 64bit elf loading support soon. Instead of
cluttering rproc-uclass, it is better to separate out elf loading support.

Thanks and regards,
Lokesh

> ---
>  drivers/remoteproc/rproc-uclass.c |  99 +++
>  drivers/remoteproc/sandbox_testproc.c |  19 ++
>  include/remoteproc.h  |  30 -
>  test/dm/remoteproc.c  | 122 
> ++
>  4 files changed, 267 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/rproc-uclass.c 
> b/drivers/remoteproc/rproc-uclass.c
> index c8a41a6..4d85732 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -5,6 +5,7 @@
>   */
>  #define pr_fmt(fmt) "%s: " fmt, __func__
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -291,6 +292,104 @@ int rproc_dev_init(int id)
>   return ret;
>  }
>  
> +/* Basic function to verify ELF image format */
> +int rproc_elf_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_elf_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;
> +
> + ehdr = (Elf32_Ehdr *)addr;
> + phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
> +
> + ops = rproc_get_ops(dev);
> +
> + /* Load each program header */
> + for (i = 0; i < ehdr->e_phnum; ++i) {
> + void *dst = (void *)(uintptr_t)phdr->p_paddr;
> + void *src = (void *)addr + phdr->p_offset;
> +
> + if (phdr->p_type != PT_LOAD)
> + continue;
> +
> + if (ops->device_to_virt)
> + dst = ops->device_to_virt(dev, (ulong)dst);
> +
> + dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
> + i, dst, phdr->p_filesz);
> + if (phdr->p_filesz)
> + memcpy(dst, src, phdr->p_filesz);
> + if (phdr->p_filesz != phdr->p_memsz)
> + memset(dst + phdr->p_filesz, 0x00,
> +phdr->p_memsz - phdr->p_filesz);
> + flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> + roundup((unsigned long)dst + phdr->p_filesz,
> + ARCH_DMA_MINALIGN) -
> + rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> + ++phdr;
> + }
> +
> + return 0;
> +}
> +
>  int rproc_load(int id, ulong addr, ulong size)
>  {
>   struct udevice *dev = NULL;
> diff --git 

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

2019-05-27 Thread Fabien Dessenne
The current implementation supports only binary file load.
Add helpers to support ELF format (sanity check, and load).
Note that since an ELF 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 
Signed-off-by: Fabien Dessenne 
---
 drivers/remoteproc/rproc-uclass.c |  99 +++
 drivers/remoteproc/sandbox_testproc.c |  19 ++
 include/remoteproc.h  |  30 -
 test/dm/remoteproc.c  | 122 ++
 4 files changed, 267 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/rproc-uclass.c 
b/drivers/remoteproc/rproc-uclass.c
index c8a41a6..4d85732 100644
--- a/drivers/remoteproc/rproc-uclass.c
+++ b/drivers/remoteproc/rproc-uclass.c
@@ -5,6 +5,7 @@
  */
 #define pr_fmt(fmt) "%s: " fmt, __func__
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -291,6 +292,104 @@ int rproc_dev_init(int id)
return ret;
 }
 
+/* Basic function to verify ELF image format */
+int rproc_elf_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_elf_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;
+
+   ehdr = (Elf32_Ehdr *)addr;
+   phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
+
+   ops = rproc_get_ops(dev);
+
+   /* Load each program header */
+   for (i = 0; i < ehdr->e_phnum; ++i) {
+   void *dst = (void *)(uintptr_t)phdr->p_paddr;
+   void *src = (void *)addr + phdr->p_offset;
+
+   if (phdr->p_type != PT_LOAD)
+   continue;
+
+   if (ops->device_to_virt)
+   dst = ops->device_to_virt(dev, (ulong)dst);
+
+   dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
+   i, dst, phdr->p_filesz);
+   if (phdr->p_filesz)
+   memcpy(dst, src, phdr->p_filesz);
+   if (phdr->p_filesz != phdr->p_memsz)
+   memset(dst + phdr->p_filesz, 0x00,
+  phdr->p_memsz - phdr->p_filesz);
+   flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
+   roundup((unsigned long)dst + phdr->p_filesz,
+   ARCH_DMA_MINALIGN) -
+   rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
+   ++phdr;
+   }
+
+   return 0;
+}
+
 int rproc_load(int id, ulong addr, ulong size)
 {
struct udevice *dev = NULL;
diff --git a/drivers/remoteproc/sandbox_testproc.c 
b/drivers/remoteproc/sandbox_testproc.c
index 51a67e6..5f35119 100644
--- a/drivers/remoteproc/sandbox_testproc.c
+++ b/drivers/remoteproc/sandbox_testproc.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * enum sandbox_state - different device states
@@ -300,6 +301,23 @@ static int sandbox_testproc_ping(struct udevice *dev)
return ret;
 }
 
+#define