Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v4] Relax location of ta head #6316

Merged
merged 5 commits into from
Oct 13, 2023
Merged

Conversation

jenswi-linaro
Copy link
Contributor

jenswi-linaro#10 applied to upstream instead of my private fork.

This is part of #6032

Moves the make macro mv-if-changed() into mk/macros.mk to allow use of
it in the TA devkit.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Always generate the file holding the list of dynamic symbols a TA should
provide. This is needed if CFG_FTRACE_SUPPORT should be changed between
two compilations. Use the make macro mv-if-changed() to only update the
used file if it will be changed.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Legacy TAs have their TA header in a .ta_head section of the TA binary.
However, in commits to follow the TA header will instead be located in
the symbol ta_head located somewhere inside the ELF binary. So update
the ts_bin_to_c.py script to support the updated format.

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Legacy TAs has their TA header as at the load address of the TA binary.
However, in commits to follow the TA header will instead be located in
the symbol ta_head located somewhere inside the ELF binary. So update
the ldelf to support the updated format while still supporting legacy
TAs.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
TAs where required to have the ta_head as at the load address of the TA
prior to this patch. This makes the linker script slightly more
complicated and also confuses GDB so that an offset must be applied to
the load address of the TA when using GDB for debugging. So allow that
ta_head symbol to reside anywhere in the ELF binary and also add ta_head
to the .dynsym section to make sure that tools and ldelf can find the
symbol.

This change requires prior updates to tools and ldelf.

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@jforissier
Copy link
Contributor

Could we have a prefix in the PR subject to identify more easily the for-4.0.0 stuff? Such as [v4] maybe?

@jenswi-linaro jenswi-linaro changed the title Relax location of ta head [v4] Relax location of ta head Sep 25, 2023
@jenswi-linaro
Copy link
Contributor Author

Could we have a prefix in the PR subject to identify more easily the for-4.0.0 stuff? Such as [v4] maybe?

Good idea, done.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside comment on commit "ta/link.mk: always generate TA dyn_list",
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> for the series.

@@ -37,17 +37,18 @@ endif
link-ldflags += --as-needed # Do not add dependency on unused shlib
link-ldflags += $(link-ldflags$(sm))

$(link-out-dir$(sm))/dyn_list:
$(link-out-dir$(sm))/dyn_list: FORCE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of FORCE, could there be a dependancy on $(conf-file)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that is set in the TA dev-kit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think it's set in the TA dev-kit, but that's also why we're using mv-if-changed() below so that we don't trigger unnecessary rebuilds.

@jforissier jforissier merged commit 4bdddf2 into OP-TEE:master Oct 13, 2023
7 checks passed
@jenswi-linaro jenswi-linaro deleted the relax_ta_head branch October 14, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants