Message ID | 20190828125534.29408-4-lokeshvutla@ti.com |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
Series | remoteproc: Add support for R5F and DSP processors | expand |
Hi Lokesh I would prefer you keep the rproc_elf32_sanity_check() API available (not static). Some u-boot drivers (eg ti_power_proc.c) handle binary (raw) firmwares while some other handle ELF format ones. We may think about drivers that can handle both formats. rproc_elf32_sanity_check() can then be used to check if the firmware shall be ELF-decoded or simply memcpy'ed. Anyway I agree with your proposal to call rproc_elf32_sanity_check() from rproc_elf32_load_image() BR Fabien On 28/08/2019 2:55 PM, Lokesh Vutla wrote: > rproc_elf32_load_image() rely on user to send a valid address for elf loading. > Instead do a sanity check on the address passed by user. This will help > all rproc elf users to not call sanity_check explicitly before calling > elf_loading. > > Now that rproc_elf32_sanity_check() is used only in rproc-elf-loader.c > make it static. > > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > drivers/remoteproc/rproc-elf-loader.c | 13 +++++++++---- > drivers/remoteproc/stm32_copro.c | 9 +-------- > include/remoteproc.h | 20 ++++---------------- > test/dm/remoteproc.c | 7 ++----- > 4 files changed, 16 insertions(+), 33 deletions(-) > > diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c > index 7574ba3fb3..149aeafb85 100644 > --- a/drivers/remoteproc/rproc-elf-loader.c > +++ b/drivers/remoteproc/rproc-elf-loader.c > @@ -8,7 +8,7 @@ > #include <remoteproc.h> > > /* Basic function to verify ELF32 image format */ > -int rproc_elf32_sanity_check(ulong addr, ulong size) > +static int rproc_elf32_sanity_check(ulong addr, ulong size) > { > Elf32_Ehdr *ehdr; > char class; > @@ -64,13 +64,18 @@ int rproc_elf32_sanity_check(ulong addr, ulong size) > return 0; > } > > -/* A very simple elf loader, assumes the image is valid */ > -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr) > +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size) > { > Elf32_Ehdr *ehdr; /* Elf header structure pointer */ > Elf32_Phdr *phdr; /* Program header structure pointer */ > const struct dm_rproc_ops *ops; > - unsigned int i; > + unsigned int i, ret; > + > + ret = rproc_elf32_sanity_check(addr, size); > + if (ret) { > + dev_err(dev, "Invalid ELF32 Image %d\n", ret); > + return ret; > + } > > ehdr = (Elf32_Ehdr *)addr; > phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff); > diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c > index ff82313bca..1760dc5aba 100644 > --- a/drivers/remoteproc/stm32_copro.c > +++ b/drivers/remoteproc/stm32_copro.c > @@ -149,14 +149,7 @@ static int stm32_copro_load(struct udevice *dev, ulong addr, ulong size) > return ret; > } > > - /* Support only ELF32 image */ > - ret = rproc_elf32_sanity_check(addr, size); > - if (ret) { > - dev_err(dev, "Invalid ELF32 image (%d)\n", ret); > - return ret; > - } > - > - return rproc_elf32_load_image(dev, addr); > + return rproc_elf32_load_image(dev, addr, size); > } > > /** > diff --git a/include/remoteproc.h b/include/remoteproc.h > index 81f616a1f9..6657b39723 100644 > --- a/include/remoteproc.h > +++ b/include/remoteproc.h > @@ -202,25 +202,14 @@ int rproc_ping(int id); > */ > int rproc_is_running(int id); > > -/** > - * rproc_elf32_sanity_check() - Verify if an image is a valid ELF32 one > - * > - * Check if a valid ELF32 image exists at the given memory location. Verify > - * basic ELF32 format requirements like magic number and sections size. > - * > - * @addr: address of the image to verify > - * @size: size of the image > - * @return 0 if the image looks good, else appropriate error value. > - */ > -int rproc_elf32_sanity_check(ulong addr, ulong size); > - > /** > * rproc_elf32_load_image() - load an ELF32 image > * @dev: device loading the ELF32 image > * @addr: valid ELF32 image address > + * @size: size of the image > * @return 0 if the image is successfully loaded, else appropriate error value. > */ > -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr); > +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size); > #else > static inline int rproc_init(void) { return -ENOSYS; } > static inline int rproc_dev_init(int id) { return -ENOSYS; } > @@ -231,10 +220,9 @@ static inline int rproc_stop(int id) { return -ENOSYS; } > static inline int rproc_reset(int id) { return -ENOSYS; } > static inline int rproc_ping(int id) { return -ENOSYS; } > static inline int rproc_is_running(int id) { return -ENOSYS; } > -static inline int rproc_elf32_sanity_check(ulong addr, > - ulong size) { return -ENOSYS; } > static inline int rproc_elf32_load_image(struct udevice *dev, > - unsigned long addr) { return -ENOSYS; } > + unsigned long addr, ulong size) > +{ return -ENOSYS; } > #endif > > #endif /* _RPROC_H_ */ > diff --git a/test/dm/remoteproc.c b/test/dm/remoteproc.c > index a2c4be7c27..fb3b615fc3 100644 > --- a/test/dm/remoteproc.c > +++ b/test/dm/remoteproc.c > @@ -171,18 +171,15 @@ static int dm_test_remoteproc_elf(struct unit_test_state *uts) > ut_assertnonnull(loaded_firmware); > memset(loaded_firmware, 0, loaded_firmware_size); > > - /* Verify valid ELF format */ > - ut_assertok(rproc_elf32_sanity_check((ulong)valid_elf32, size)); > - > /* Load firmware in loaded_firmware, and verify it */ > - ut_assertok(rproc_elf32_load_image(dev, (unsigned long)valid_elf32)); > + ut_assertok(rproc_elf32_load_image(dev, (ulong)valid_elf32, size)); > ut_assertok(memcmp(loaded_firmware, valid_elf32, loaded_firmware_size)); > unmap_physmem(loaded_firmware, MAP_NOCACHE); > > /* Invalid ELF Magic */ > valid_elf32[0] = 0; > ut_asserteq(-EPROTONOSUPPORT, > - rproc_elf32_sanity_check((ulong)valid_elf32, size)); > + rproc_elf32_load_image(dev, (ulong)valid_elf32, size)); > > return 0; > }
diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c index 7574ba3fb3..149aeafb85 100644 --- a/drivers/remoteproc/rproc-elf-loader.c +++ b/drivers/remoteproc/rproc-elf-loader.c @@ -8,7 +8,7 @@ #include <remoteproc.h> /* Basic function to verify ELF32 image format */ -int rproc_elf32_sanity_check(ulong addr, ulong size) +static int rproc_elf32_sanity_check(ulong addr, ulong size) { Elf32_Ehdr *ehdr; char class; @@ -64,13 +64,18 @@ int rproc_elf32_sanity_check(ulong addr, ulong size) return 0; } -/* A very simple elf loader, assumes the image is valid */ -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr) +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size) { Elf32_Ehdr *ehdr; /* Elf header structure pointer */ Elf32_Phdr *phdr; /* Program header structure pointer */ const struct dm_rproc_ops *ops; - unsigned int i; + unsigned int i, ret; + + ret = rproc_elf32_sanity_check(addr, size); + if (ret) { + dev_err(dev, "Invalid ELF32 Image %d\n", ret); + return ret; + } ehdr = (Elf32_Ehdr *)addr; phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff); diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c index ff82313bca..1760dc5aba 100644 --- a/drivers/remoteproc/stm32_copro.c +++ b/drivers/remoteproc/stm32_copro.c @@ -149,14 +149,7 @@ static int stm32_copro_load(struct udevice *dev, ulong addr, ulong size) return ret; } - /* Support only ELF32 image */ - ret = rproc_elf32_sanity_check(addr, size); - if (ret) { - dev_err(dev, "Invalid ELF32 image (%d)\n", ret); - return ret; - } - - return rproc_elf32_load_image(dev, addr); + return rproc_elf32_load_image(dev, addr, size); } /** diff --git a/include/remoteproc.h b/include/remoteproc.h index 81f616a1f9..6657b39723 100644 --- a/include/remoteproc.h +++ b/include/remoteproc.h @@ -202,25 +202,14 @@ int rproc_ping(int id); */ int rproc_is_running(int id); -/** - * rproc_elf32_sanity_check() - Verify if an image is a valid ELF32 one - * - * Check if a valid ELF32 image exists at the given memory location. Verify - * basic ELF32 format requirements like magic number and sections size. - * - * @addr: address of the image to verify - * @size: size of the image - * @return 0 if the image looks good, else appropriate error value. - */ -int rproc_elf32_sanity_check(ulong addr, ulong size); - /** * rproc_elf32_load_image() - load an ELF32 image * @dev: device loading the ELF32 image * @addr: valid ELF32 image address + * @size: size of the image * @return 0 if the image is successfully loaded, else appropriate error value. */ -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr); +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong size); #else static inline int rproc_init(void) { return -ENOSYS; } static inline int rproc_dev_init(int id) { return -ENOSYS; } @@ -231,10 +220,9 @@ static inline int rproc_stop(int id) { return -ENOSYS; } static inline int rproc_reset(int id) { return -ENOSYS; } static inline int rproc_ping(int id) { return -ENOSYS; } static inline int rproc_is_running(int id) { return -ENOSYS; } -static inline int rproc_elf32_sanity_check(ulong addr, - ulong size) { return -ENOSYS; } static inline int rproc_elf32_load_image(struct udevice *dev, - unsigned long addr) { return -ENOSYS; } + unsigned long addr, ulong size) +{ return -ENOSYS; } #endif #endif /* _RPROC_H_ */ diff --git a/test/dm/remoteproc.c b/test/dm/remoteproc.c index a2c4be7c27..fb3b615fc3 100644 --- a/test/dm/remoteproc.c +++ b/test/dm/remoteproc.c @@ -171,18 +171,15 @@ static int dm_test_remoteproc_elf(struct unit_test_state *uts) ut_assertnonnull(loaded_firmware); memset(loaded_firmware, 0, loaded_firmware_size); - /* Verify valid ELF format */ - ut_assertok(rproc_elf32_sanity_check((ulong)valid_elf32, size)); - /* Load firmware in loaded_firmware, and verify it */ - ut_assertok(rproc_elf32_load_image(dev, (unsigned long)valid_elf32)); + ut_assertok(rproc_elf32_load_image(dev, (ulong)valid_elf32, size)); ut_assertok(memcmp(loaded_firmware, valid_elf32, loaded_firmware_size)); unmap_physmem(loaded_firmware, MAP_NOCACHE); /* Invalid ELF Magic */ valid_elf32[0] = 0; ut_asserteq(-EPROTONOSUPPORT, - rproc_elf32_sanity_check((ulong)valid_elf32, size)); + rproc_elf32_load_image(dev, (ulong)valid_elf32, size)); return 0; }
rproc_elf32_load_image() rely on user to send a valid address for elf loading. Instead do a sanity check on the address passed by user. This will help all rproc elf users to not call sanity_check explicitly before calling elf_loading. Now that rproc_elf32_sanity_check() is used only in rproc-elf-loader.c make it static. Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> --- drivers/remoteproc/rproc-elf-loader.c | 13 +++++++++---- drivers/remoteproc/stm32_copro.c | 9 +-------- include/remoteproc.h | 20 ++++---------------- test/dm/remoteproc.c | 7 ++----- 4 files changed, 16 insertions(+), 33 deletions(-)