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

tfm: Add zephyr native ns_interface and logging function #33553

Conversation

oyvindronningstad
Copy link
Collaborator

To allow using TFM NS interface without enabling CMSIS_RTOS support.
And to allow using TFM ns code that uses logging.

Signed-off-by: Øyvind Rønningstad [email protected]

To allow using TFM NS interface without enabling CMSIS_RTOS support.
And to allow using TFM ns code that uses logging.

Signed-off-by: Øyvind Rønningstad <[email protected]>
Comment on lines +259 to +260
zephyr_sources(
${ZEPHYR_BASE}/modules/trusted-firmware-m/src/zephyr_tfm_ns_interface.c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this a zephyr_sources ?
Does it really belong in the zephyr lib.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is part of the nonsecure image, to help interfacing with tfm.

@oyvindronningstad
Copy link
Collaborator Author

Merged as part of #33510.

@tejlmand
Copy link
Collaborator

Merged as part of #33510.

Seems that nullified my comment 😆

@ioannisg
Copy link
Member

Merged as part of #33510.

Seems that nullified my comment 😆

I think this source is part of zephyr lib, @tejlmand. But I guess @oyvindronningstad could still answer, regardless of the change being already merged. :)

@oyvindronningstad
Copy link
Collaborator Author

I just wanted to stop/redirect more comments on this. I will answer your question @tejlmand. It's probably "that's what worked after a couple of hours struggling a to get cmake to give me my symbols" 🙃

@tejlmand
Copy link
Collaborator

I think this source is part of zephyr lib, @tejlmand. But I guess @oyvindronningstad could still answer, regardless of the change being already merged. :)

I took a second look.

Actually, you don't know which library the file will end up in.

At first glance, this looks reasonable, as it will be added to the currently defined library (usually not libzephyr.a)

zephyr_library_sources_ifdef(CONFIG_BUILD_WITH_TFM interface/interface.c)
# Non-Secure interface to request system reboot
zephyr_library_sources_ifdef(CONFIG_TFM_PARTITION_PLATFORM src/reboot.c)

until one realizes there is NO zephyr_library() call in this CMakeLists.txt.
Because this is included as ZEPHYR_<MODULE>_CMAKE_DIR and zephyr is the current library, then you get the code in the zephyr lib, but if there had been another zephyr_library() call prior to this code in the parent scope, then the code would have ended up inside that particular library.

When adding sources to zephyr lib, then zephyr_sources should be used and not zephyr_library_sources.
Also, there should be a good reason for adding sources to zephyr lib, and not just then it works.
See this comment, on how zephyr lib may look:
#8825 (comment)

It's probably "that's what worked after a couple of hours struggling a to get cmake to give me my symbols" upside_down_face

Sound like a bad approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Modules area: Samples Samples area: TF-M ARM Trusted Firmware-M (TF-M)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants