diff mbox series

[U-Boot,02/25] remoteproc: ops: Add elf section size as input parameter to device_to_virt api

Message ID 20190828125534.29408-3-lokeshvutla@ti.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series remoteproc: Add support for R5F and DSP processors | expand

Commit Message

Lokesh Vutla Aug. 28, 2019, 12:55 p.m. UTC
Introduce a new parameter size that accepts elf section size to remoteproc
ops callback device_to_virt(). This can enforce more checks on the
elf section that is being loaded.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/remoteproc/rproc-elf-loader.c | 3 ++-
 drivers/remoteproc/sandbox_testproc.c | 4 +++-
 drivers/remoteproc/stm32_copro.c      | 4 +++-
 include/remoteproc.h                  | 3 ++-
 4 files changed, 10 insertions(+), 4 deletions(-)

Comments

Fabien DESSENNE Aug. 29, 2019, 8:44 a.m. UTC | #1
Hi Lokesh


On 28/08/2019 2:55 PM, Lokesh Vutla wrote:
> Introduce a new parameter size that accepts elf section size to remoteproc
> ops callback device_to_virt(). This can enforce more checks on the
> elf section that is being loaded.


It would be better to talk about "size" instead of "elf section size" 
since device_to_virt may deal with any memory region.


> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>   drivers/remoteproc/rproc-elf-loader.c | 3 ++-
>   drivers/remoteproc/sandbox_testproc.c | 4 +++-
>   drivers/remoteproc/stm32_copro.c      | 4 +++-
>   include/remoteproc.h                  | 3 ++-
>   4 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
> index 67937a7595..7574ba3fb3 100644
> --- a/drivers/remoteproc/rproc-elf-loader.c
> +++ b/drivers/remoteproc/rproc-elf-loader.c
> @@ -86,7 +86,8 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr)
>   			continue;
>   
>   		if (ops->device_to_virt)
> -			dst = ops->device_to_virt(dev, (ulong)dst);
> +			dst = ops->device_to_virt(dev, (ulong)dst,
> +						  phdr->p_memsz);
>   
>   		dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
>   			i, dst, phdr->p_filesz);
> diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
> index 5f35119ab7..0a8f7871e1 100644
> --- a/drivers/remoteproc/sandbox_testproc.c
> +++ b/drivers/remoteproc/sandbox_testproc.c
> @@ -306,9 +306,11 @@ static int sandbox_testproc_ping(struct udevice *dev)
>    * sandbox_testproc_device_to_virt() - Convert device address to virtual address
>    * @dev:	device to operate upon
>    * @da:		device address
> + * @size:	Size of the section


Size of the memory region @da is pointing to


>    * @return converted virtual address
>    */
> -static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da)
> +static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da,
> +					     ulong size)
>   {
>   	u64 paddr;
>   
> diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c
> index de3b9729f3..ff82313bca 100644
> --- a/drivers/remoteproc/stm32_copro.c
> +++ b/drivers/remoteproc/stm32_copro.c
> @@ -107,9 +107,11 @@ static int stm32_copro_set_hold_boot(struct udevice *dev, bool hold)
>    * stm32_copro_device_to_virt() - Convert device address to virtual address
>    * @dev:	corresponding STM32 remote processor device
>    * @da:		device address
> + * @size:	Size of the section
>    * @return converted virtual address
>    */
> -static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da)
> +static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da,
> +					ulong size)
>   {


I guess it would be better than this function also checks that the end 
address of the region is in the range of valid region

Something like:

end_addr = cpu_to_be32(da + size - 1)   + check success of 
dev_translate_dma_address(dev, &end_addr);


>   	fdt32_t in_addr = cpu_to_be32(da);
>   	u64 paddr;
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> index 4987194905..81f616a1f9 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -122,9 +122,10 @@ struct dm_rproc_ops {
>   	 *
>   	 * @dev:	Remote proc device
>   	 * @da:		Device address
> +	 * @size:	Size of the section


Size of the memory region @da is pointing to


>   	 * @return virtual address.
>   	 */
> -	void * (*device_to_virt)(struct udevice *dev, ulong da);
> +	void * (*device_to_virt)(struct udevice *dev, ulong da, ulong size);
>   };
>   
>   /* Accessor */
Lokesh Vutla Aug. 30, 2019, 9:54 a.m. UTC | #2
On 29/08/19 2:14 PM, Fabien DESSENNE wrote:
> Hi Lokesh
> 
> 
> On 28/08/2019 2:55 PM, Lokesh Vutla wrote:
>> Introduce a new parameter size that accepts elf section size to remoteproc
>> ops callback device_to_virt(). This can enforce more checks on the
>> elf section that is being loaded.
> 
> 
> It would be better to talk about "size" instead of "elf section size" 
> since device_to_virt may deal with any memory region.
> 
> 
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>   drivers/remoteproc/rproc-elf-loader.c | 3 ++-
>>   drivers/remoteproc/sandbox_testproc.c | 4 +++-
>>   drivers/remoteproc/stm32_copro.c      | 4 +++-
>>   include/remoteproc.h                  | 3 ++-
>>   4 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
>> index 67937a7595..7574ba3fb3 100644
>> --- a/drivers/remoteproc/rproc-elf-loader.c
>> +++ b/drivers/remoteproc/rproc-elf-loader.c
>> @@ -86,7 +86,8 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr)
>>   			continue;
>>   
>>   		if (ops->device_to_virt)
>> -			dst = ops->device_to_virt(dev, (ulong)dst);
>> +			dst = ops->device_to_virt(dev, (ulong)dst,
>> +						  phdr->p_memsz);
>>   
>>   		dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
>>   			i, dst, phdr->p_filesz);
>> diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
>> index 5f35119ab7..0a8f7871e1 100644
>> --- a/drivers/remoteproc/sandbox_testproc.c
>> +++ b/drivers/remoteproc/sandbox_testproc.c
>> @@ -306,9 +306,11 @@ static int sandbox_testproc_ping(struct udevice *dev)
>>    * sandbox_testproc_device_to_virt() - Convert device address to virtual address
>>    * @dev:	device to operate upon
>>    * @da:		device address
>> + * @size:	Size of the section
> 
> 
> Size of the memory region @da is pointing to
> 
> 
>>    * @return converted virtual address
>>    */
>> -static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da)
>> +static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da,
>> +					     ulong size)
>>   {
>>   	u64 paddr;
>>   
>> diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c
>> index de3b9729f3..ff82313bca 100644
>> --- a/drivers/remoteproc/stm32_copro.c
>> +++ b/drivers/remoteproc/stm32_copro.c
>> @@ -107,9 +107,11 @@ static int stm32_copro_set_hold_boot(struct udevice *dev, bool hold)
>>    * stm32_copro_device_to_virt() - Convert device address to virtual address
>>    * @dev:	corresponding STM32 remote processor device
>>    * @da:		device address
>> + * @size:	Size of the section
>>    * @return converted virtual address
>>    */
>> -static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da)
>> +static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da,
>> +					ulong size)
>>   {
> 
> 
> I guess it would be better than this function also checks that the end 
> address of the region is in the range of valid region
> 
> Something like:
> 
> end_addr = cpu_to_be32(da + size - 1)   + check success of 
> dev_translate_dma_address(dev, &end_addr);

Sure Ill fix in v2 but I cannot test stm32 coproc. Please make sure that v2 will
not break your platform.

Thanks and regards,
Lokesh

> 
> 
>>   	fdt32_t in_addr = cpu_to_be32(da);
>>   	u64 paddr;
>> diff --git a/include/remoteproc.h b/include/remoteproc.h
>> index 4987194905..81f616a1f9 100644
>> --- a/include/remoteproc.h
>> +++ b/include/remoteproc.h
>> @@ -122,9 +122,10 @@ struct dm_rproc_ops {
>>   	 *
>>   	 * @dev:	Remote proc device
>>   	 * @da:		Device address
>> +	 * @size:	Size of the section
> 
> 
> Size of the memory region @da is pointing to
> 
> 
>>   	 * @return virtual address.
>>   	 */
>> -	void * (*device_to_virt)(struct udevice *dev, ulong da);
>> +	void * (*device_to_virt)(struct udevice *dev, ulong da, ulong size);
>>   };
>>   
>>   /* Accessor */
diff mbox series

Patch

diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c
index 67937a7595..7574ba3fb3 100644
--- a/drivers/remoteproc/rproc-elf-loader.c
+++ b/drivers/remoteproc/rproc-elf-loader.c
@@ -86,7 +86,8 @@  int rproc_elf32_load_image(struct udevice *dev, unsigned long addr)
 			continue;
 
 		if (ops->device_to_virt)
-			dst = ops->device_to_virt(dev, (ulong)dst);
+			dst = ops->device_to_virt(dev, (ulong)dst,
+						  phdr->p_memsz);
 
 		dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
 			i, dst, phdr->p_filesz);
diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
index 5f35119ab7..0a8f7871e1 100644
--- a/drivers/remoteproc/sandbox_testproc.c
+++ b/drivers/remoteproc/sandbox_testproc.c
@@ -306,9 +306,11 @@  static int sandbox_testproc_ping(struct udevice *dev)
  * sandbox_testproc_device_to_virt() - Convert device address to virtual address
  * @dev:	device to operate upon
  * @da:		device address
+ * @size:	Size of the section
  * @return converted virtual address
  */
-static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da)
+static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da,
+					     ulong size)
 {
 	u64 paddr;
 
diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c
index de3b9729f3..ff82313bca 100644
--- a/drivers/remoteproc/stm32_copro.c
+++ b/drivers/remoteproc/stm32_copro.c
@@ -107,9 +107,11 @@  static int stm32_copro_set_hold_boot(struct udevice *dev, bool hold)
  * stm32_copro_device_to_virt() - Convert device address to virtual address
  * @dev:	corresponding STM32 remote processor device
  * @da:		device address
+ * @size:	Size of the section
  * @return converted virtual address
  */
-static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da)
+static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da,
+					ulong size)
 {
 	fdt32_t in_addr = cpu_to_be32(da);
 	u64 paddr;
diff --git a/include/remoteproc.h b/include/remoteproc.h
index 4987194905..81f616a1f9 100644
--- a/include/remoteproc.h
+++ b/include/remoteproc.h
@@ -122,9 +122,10 @@  struct dm_rproc_ops {
 	 *
 	 * @dev:	Remote proc device
 	 * @da:		Device address
+	 * @size:	Size of the section
 	 * @return virtual address.
 	 */
-	void * (*device_to_virt)(struct udevice *dev, ulong da);
+	void * (*device_to_virt)(struct udevice *dev, ulong da, ulong size);
 };
 
 /* Accessor */