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.

Sign up for GitHub

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

Jump to bottom

Non-Secure Callable improvements #16055

Merged
merged 2 commits into from
May 30, 2019
Merged

Non-Secure Callable improvements #16055

merged 2 commits into from
May 30, 2019

Conversation

oyvindronningstad
Copy link
Collaborator

@oyvindronningstad oyvindronningstad commented May 10, 2019 •
edited by ioannisg
Loading

In brief, this patch does the following:

  • Makes it "easier" for Non-Secure ARM firmware (CONFIG_TRUSTED_EXECUTION_NONSECURE=y) to include a Secure Entry function veneer library into the non-secure firmware build. This is done by Kconfig symbol additions in arch/arm/core/Kconfig
  • Fixes the automatic Non-Secure-Callable (NSC) region placement (in the ARM linker script) for nRF SoCs, respecting their constrains on region alignment. The patch forces the region to be placed at the end of the Firmware flash area. The patch does not alter the default way the NSC region is generated and placed (i.e. for non-nRF SoCs)

@oyvindronningstad oyvindronningstad changed the title Non-Secure Callable improvements [RFC] [WIP] Non-Secure Callable improvements May 10, 2019
@oyvindronningstad
Copy link
Collaborator Author

cc @ioannisg @hakonfam @frkv

@oyvindronningstad oyvindronningstad mentioned this pull request May 10, 2019
Merged
@zephyrbot
Copy link
Collaborator

zephyrbot commented May 10, 2019 •
edited
Loading

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@ioannisg ioannisg added area: ARM ARM (32-bit) Architecture area: Trusted Execution Trusted Execution RFC Request For Comments: want input from the community labels May 10, 2019
ioannisg
ioannisg reviewed May 10, 2019
View reviewed changes
arch/arm/core/Kconfig Outdated Show resolved Hide resolved
ioannisg
ioannisg reviewed May 10, 2019
View reviewed changes
arch/arm/core/Kconfig Outdated Show resolved Hide resolved
ioannisg
ioannisg reviewed May 10, 2019
View reviewed changes
Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @oyvindronningstad, i left some comments in the PR and invited @SebastianBoe to take a look, as well. I believe, after addressing the issues, this should be turned to a normal PR

@ioannisg ioannisg requested a review from SebastianBoe May 10, 2019 12:19
@ioannisg
Copy link
Member

@carlescufi FYI

SebastianBoe
SebastianBoe reviewed May 10, 2019
View reviewed changes
arch/arm/core/Kconfig Outdated Show resolved Hide resolved
@ioannisg ioannisg requested a review from ulfalizer May 10, 2019 12:49
SebastianBoe
SebastianBoe approved these changes May 10, 2019
View reviewed changes
Copy link
Collaborator

@SebastianBoe SebastianBoe left a comment

Choose a reason for hiding this comment

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

Build system related changes are OK.

Needs to be rebased on top of it's dependent PR #16059

@oyvindronningstad oyvindronningstad force-pushed the nsc-linker branch 2 times, most recently from 2483751 to 640c2f0 Compare May 13, 2019 06:11
@ioannisg
Copy link
Member

ioannisg commented May 13, 2019 •
edited
Loading

#16059

This is, now, meged, @oyvindronningstad ; you might want to rebase it and update the PR description.
Also, you could change this to a standard PR.

@oyvindronningstad
Copy link
Collaborator Author

#16059

This is, now, meged, @oyvindronningstad ; you might want to rebase it and update the PR description.
Also, you could change this to a standard PR.

Done

@oyvindronningstad oyvindronningstad marked this pull request as ready for review May 13, 2019 07:31
@SebastianBoe
Copy link
Collaborator

LGTM

@ioannisg
Copy link
Member

@galak can you look at this one, please?

@ioannisg ioannisg requested review from ulfalizer and removed request for ulfalizer May 13, 2019 20:09
ulfalizer
ulfalizer approved these changes May 13, 2019
View reviewed changes
Copy link
Collaborator

@ulfalizer ulfalizer left a comment

Choose a reason for hiding this comment

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

Not an expert on ARM security stuff, but looks reasonable at a glance.

arch/arm/core/Kconfig Outdated Show resolved Hide resolved
@ioannisg ioannisg mentioned this pull request May 14, 2019
Closed
26 tasks
@ioannisg ioannisg added the Enhancement Changes/Updates/Additions to existing features label May 14, 2019
@oyvindronningstad oyvindronningstad force-pushed the nsc-linker branch 2 times, most recently from 671d169 to d2e0b0a Compare May 16, 2019 07:24
@ioannisg ioannisg self-assigned this May 16, 2019
@ioannisg
Copy link
Member

@galak , this patch looks merge-able now. Please, take a look as well :)

We tested that it works as expected for nrf9160_pca10090.
In addition, we have tested that the NSC section generation still works for musca_a and musca_b1.

@ioannisg
Copy link
Member

recheck

@ioannisg ioannisg requested a review from wentongwu May 23, 2019 07:32
@ioannisg
Copy link
Member

@wentongwu could you take a look?

@ioannisg ioannisg requested a review from wentongwu May 24, 2019 09:08
@oyvindronningstad
linker: Move sgstubs section to the very end and fix for nrf91
7100086 
Add ifdefs to handle the nrf91 case. This change will dynamically
place and size the NSC region according to nrf91 HW limitations.

Add Cmake check of NSC offset if manually set.

Signed-off-by: Øyvind Rønningstad <oyvind.ronningstad@nordicsemi.no>
@oyvindronningstad
arch: arm: core: tz: Linking of secure entry functions into NS firmware
638e041 
Add Kconfig options:
 - ARM_FIRMWARE_USES_SECURE_ENTRY_FUNCS
 - ARM_ENTRY_VENEERS_LIB_NAME

Use these to link the veneers lib into the Non-Secure Firmware when
needed.

Also, make the path passed to the linker absolute to make it work with
makefiles.

Signed-off-by: Øyvind Rønningstad <oyvind.ronningstad@nordicsemi.no>
@ioannisg
Copy link
Member

@MaureenHelm would you like to take a look at this one, as well?

@carlescufi carlescufi merged commit f032729 into zephyrproject-rtos:master May 30, 2019
@oyvindronningstad oyvindronningstad deleted the nsc-linker branch May 31, 2019 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ioannisg ioannisg ioannisg approved these changes

@SebastianBoe SebastianBoe SebastianBoe approved these changes

@wentongwu wentongwu wentongwu approved these changes

@carlescufi carlescufi carlescufi approved these changes

@ulfalizer ulfalizer ulfalizer approved these changes

@galak galak Awaiting requested review from galak

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@karl-zh karl-zh Awaiting requested review from karl-zh

Assignees

@ioannisg ioannisg

Labels
area: ARM ARM (32-bit) Architecture area: Trusted Execution Trusted Execution Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@oyvindronningstad @zephyrbot @ioannisg @SebastianBoe @ulfalizer @carlescufi @wentongwu