[PATCH v2 0/2] Update section header name check

[PATCH v2 0/2] Update section header name check

[Srinivasa Rao Mandadapu]

Update section header name check and corresponding documentation.

Changes since v1:
    -- Update the commit message.
	-- Use strstarts instead of strstr.
	-- Update documentation file.
	
Srinivasa Rao Mandadapu (2):
  remoteproc: elf_loader: Update resource table name check
  docs: remoteproc: Update section header name requirement

 Documentation/staging/remoteproc.rst       | 2 ++
 drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v2 1/2] remoteproc: elf_loader: Update resource table name check

[Srinivasa Rao Mandadapu]

Update the way of checking resource table name with prefix
substring search instead of complete string search.
In general Qualcomm DSP binary is prepared by combining different elfs,
hence section header name (e.g. .resource_table), appended with elf name
to differentiate with same section of different elfs.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
Changes since v1:
    -- Update the commit message.
	-- Use strstarts instead of strstr.

 drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 5a412d7..77330d6 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -272,7 +272,7 @@ find_table(struct device *dev, const struct firmware *fw)
 		u64 offset = elf_shdr_get_sh_offset(class, shdr);
 		u32 name = elf_shdr_get_sh_name(class, shdr);
 
-		if (strcmp(name_table + name, ".resource_table"))
+		if (!strstarts(name_table + name, ".resource_table"))
 			continue;
 
 		table = (struct resource_table *)(elf_data + offset);
-- 
2.7.4

[PATCH v2 2/2] docs: remoteproc: Update section header name requirement

[Srinivasa Rao Mandadapu]

Add section header name requirement specification in elf segments.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
 Documentation/staging/remoteproc.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
index 348ee7e..3125030 100644
--- a/Documentation/staging/remoteproc.rst
+++ b/Documentation/staging/remoteproc.rst
@@ -245,6 +245,8 @@ if the remote processor is accessing memory directly).
 
 In addition to the standard ELF segments, most remote processors would
 also include a special section which we call "the resource table".
+This resource table section name may have anything appended after it,
+but it must start with ".resource_table"
 
 The resource table contains system resources that the remote processor
 requires before it should be powered on, such as allocation of physically
-- 
2.7.4

Re: [PATCH v2 1/2] remoteproc: elf_loader: Update resource table name check

[Philippe Mathieu-Daudé]

On 13/12/22 10:48, Srinivasa Rao Mandadapu wrote:
> Update the way of checking resource table name with prefix
> substring search instead of complete string search.
> In general Qualcomm DSP binary is prepared by combining different elfs,

Maybe 'ELFs'? (twice).

> hence section header name (e.g. .resource_table), appended with elf name
> to differentiate with same section of different elfs.

Please include here the section info parsed by readelf as an example,
as suggested by Stephen in your v1:
https://lore.kernel.org/linux-remoteproc/CAE-0n52cNite8-4HDoQcsZ+UvZFkJU8c5oUjxPB5ag5WP6E9=g@mail.gmail.com/

> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
> Changes since v1:
>      -- Update the commit message.
> 	-- Use strstarts instead of strstr.
> 
>   drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> index 5a412d7..77330d6 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -272,7 +272,7 @@ find_table(struct device *dev, const struct firmware *fw)
>   		u64 offset = elf_shdr_get_sh_offset(class, shdr);
>   		u32 name = elf_shdr_get_sh_name(class, shdr);
>   
> -		if (strcmp(name_table + name, ".resource_table"))
> +		if (!strstarts(name_table + name, ".resource_table"))
>   			continue;
>   
>   		table = (struct resource_table *)(elf_data + offset);

Re: [PATCH v2 2/2] docs: remoteproc: Update section header name requirement

[Philippe Mathieu-Daudé]

On 13/12/22 10:48, Srinivasa Rao Mandadapu wrote:
> Add section header name requirement specification in elf segments.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
>   Documentation/staging/remoteproc.rst | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
> index 348ee7e..3125030 100644
> --- a/Documentation/staging/remoteproc.rst
> +++ b/Documentation/staging/remoteproc.rst
> @@ -245,6 +245,8 @@ if the remote processor is accessing memory directly).
>   
>   In addition to the standard ELF segments, most remote processors would
>   also include a special section which we call "the resource table".
> +This resource table section name may have anything appended after it,
> +but it must start with ".resource_table"

What do you think of:

    In addition to the standard ELF segments, most remote processors would
    also include a special section which we call the "resource table".
    A "resource table" section name must start with the ".resource_table"
    prefix, optionally having a more descriptive string appended. For
    example, ".resource_table.my_rproc" is a valid section name.

Regards,

Phil.

Re: [PATCH v2 2/2] docs: remoteproc: Update section header name requirement

[Srinivasa Rao Mandadapu]

On 12/15/2022 12:55 PM, Philippe Mathieu-Daudé wrote:
Thanks for your time Phil!!!
> On 13/12/22 10:48, Srinivasa Rao Mandadapu wrote:
>> Add section header name requirement specification in elf segments.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>>   Documentation/staging/remoteproc.rst | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/staging/remoteproc.rst 
>> b/Documentation/staging/remoteproc.rst
>> index 348ee7e..3125030 100644
>> --- a/Documentation/staging/remoteproc.rst
>> +++ b/Documentation/staging/remoteproc.rst
>> @@ -245,6 +245,8 @@ if the remote processor is accessing memory 
>> directly).
>>     In addition to the standard ELF segments, most remote processors 
>> would
>>   also include a special section which we call "the resource table".
>> +This resource table section name may have anything appended after it,
>> +but it must start with ".resource_table"
>
> What do you think of:
>
>    In addition to the standard ELF segments, most remote processors would
>    also include a special section which we call the "resource table".
>    A "resource table" section name must start with the ".resource_table"
>    prefix, optionally having a more descriptive string appended. For
>    example, ".resource_table.my_rproc" is a valid section name.
Okay. Will change accordingly and re-spin the patch set.
>
> Regards,
>
> Phil.

Re: [PATCH v2 1/2] remoteproc: elf_loader: Update resource table name check

[Srinivasa Rao Mandadapu]

On 12/15/2022 12:36 PM, Philippe Mathieu-Daudé wrote:
Thanks for your time Phil!!!
> On 13/12/22 10:48, Srinivasa Rao Mandadapu wrote:
>> Update the way of checking resource table name with prefix
>> substring search instead of complete string search.
>> In general Qualcomm DSP binary is prepared by combining different elfs,
>
> Maybe 'ELFs'? (twice).
Okay. Will update accordingly.
>
>> hence section header name (e.g. .resource_table), appended with elf name
>> to differentiate with same section of different elfs.
>
> Please include here the section info parsed by readelf as an example,
> as suggested by Stephen in your v1:
> https://lore.kernel.org/linux-remoteproc/CAE-0n52cNite8-4HDoQcsZ+UvZFkJU8c5oUjxPB5ag5WP6E9=g@mail.gmail.com/ 
>
Okay. Will include example and re-spin the patch set.
>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>> Changes since v1:
>>      -- Update the commit message.
>>     -- Use strstarts instead of strstr.
>>
>>   drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c 
>> b/drivers/remoteproc/remoteproc_elf_loader.c
>> index 5a412d7..77330d6 100644
>> --- a/drivers/remoteproc/remoteproc_elf_loader.c
>> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
>> @@ -272,7 +272,7 @@ find_table(struct device *dev, const struct 
>> firmware *fw)
>>           u64 offset = elf_shdr_get_sh_offset(class, shdr);
>>           u32 name = elf_shdr_get_sh_name(class, shdr);
>>   -        if (strcmp(name_table + name, ".resource_table"))
>> +        if (!strstarts(name_table + name, ".resource_table"))
>>               continue;
>>             table = (struct resource_table *)(elf_data + offset);
>