2023-02-23 15:07:17

by Mohammad Rafi Shaik

[permalink] [raw]
Subject: [PATCH v5 0/2] Update section header name check

Update section header name check and corresponding documentation.
Changes since v4:
-- Rephrase commit message.
Changes since v3:
-- Rephrase commit message.
Changes since v2:
-- Update the commit message with example.
-- Update the documentation text appropriately.
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 | 5 ++++-
drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)

--
2.25.1



2023-02-23 15:07:21

by Mohammad Rafi Shaik

[permalink] [raw]
Subject: [PATCH v5 1/2] remoteproc: elf_loader: Update resource table name check

From: Srinivasa Rao Mandadapu <[email protected]>

Qualcomm DSP binary is prepared by combining different ELFs'.
This patch differentiates the section header name of ELF within
the same existing section headers of the same ELF.

Example readelf output of DSP binary:
[60] .start.ac_bin_process PROGBITS
[61] .resource_table.ac_bin_process PROGBITS
[62] .comment.ac_bin_process PROGBITS

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Signed-off-by: Mohammad Rafi Shaik <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Reviewed-by: Mukesh Ojha <[email protected]>
---
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 5a412d7b6e0b..77330d6f43d0 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.25.1


2023-02-23 15:07:30

by Mohammad Rafi Shaik

[permalink] [raw]
Subject: [PATCH v5 2/2] docs: remoteproc: Update section header name requirement

From: Srinivasa Rao Mandadapu <[email protected]>

Add section header name requirement specification in elf segments.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Signed-off-by: Mohammad Rafi Shaik <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Reviewed-by: Mukesh Ojha <[email protected]>
---
Documentation/staging/remoteproc.rst | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/staging/remoteproc.rst b/Documentation/staging/remoteproc.rst
index 348ee7e508ac..0c9c10a30c3d 100644
--- a/Documentation/staging/remoteproc.rst
+++ b/Documentation/staging/remoteproc.rst
@@ -244,7 +244,10 @@ according to the specified device address (might be a physical address
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".
+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.

The resource table contains system resources that the remote processor
requires before it should be powered on, such as allocation of physically
--
2.25.1


2023-03-02 22:06:36

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] Update section header name check

On Thu, Feb 23, 2023 at 08:35:57PM +0530, Mohammad Rafi Shaik wrote:
> Update section header name check and corresponding documentation.
> Changes since v4:
> -- Rephrase commit message.

Asked for clarifications on V4 that were never given to me. This patchset will
not move forward until those have been resolved.

> Changes since v3:
> -- Rephrase commit message.
> Changes since v2:
> -- Update the commit message with example.
> -- Update the documentation text appropriately.
> 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 | 5 ++++-
> drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> --
> 2.25.1
>

2023-05-25 15:49:03

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] Update section header name check

On Thu, May 11, 2023 at 10:09:45PM +0530, Mohammad Rafi Shaik wrote:
>
> On 3/3/2023 3:27 AM, Mathieu Poirier wrote:
> > On Thu, Feb 23, 2023 at 08:35:57PM +0530, Mohammad Rafi Shaik wrote:
> > > Update section header name check and corresponding documentation.
> > > Changes since v4:
> > > -- Rephrase commit message.
> > Asked for clarifications on V4 that were never given to me. This patchset will
> > not move forward until those have been resolved.
> The present Qualcomm DSP binary contains resource table name as
> ".resource_table.ac_bin_process.",

My questions still haven't been answered:

1. Why do we have to change the kernel because of the way a company specific
tool generates an ELF?

2. Why is the "ac_bin_process" part needed at all?

> so the current logic with strcmp will fail with present comparision ?as the
> binary name is not only .resource_table but resource_table.ac_bin_process So
> to overcome this issue we modified the way of checking the resource table
> name to make it generic.
>
> strstarts(name_table + name, ".resource_table");
>
> This logic will perform a string comparison with name ".resouce_table"as our
> binary name is .resource_table.ac_bin_process it succeeds
>
> > > Changes since v3:
> > > -- Rephrase commit message.
> > > Changes since v2:
> > > -- Update the commit message with example.
> > > -- Update the documentation text appropriately.
> > > 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 | 5 ++++-
> > > drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
> > > 2 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.25.1
> > >