arch: arc: let zephyr run in normal mode of arc #9341
Conversation
arch/arc/core/fault_s.S
Outdated
There was a problem hiding this comment.
I think it's actually ok to read ERSEC_STAT from Normal mode as well - fields without access will be 0 (IOW/RAZ)
There was a problem hiding this comment.
read is ok, but no write. It will make codes simpler
arch/arc/core/reset.S
Outdated
There was a problem hiding this comment.
I assume this is a temporary, minimal initialization for Secure Mode when Zephyr runs in Normal mode. To be replaced later with a Secure firmware that can be extended to provide more functions as well.
arch/arc/core/reset.S
Outdated
There was a problem hiding this comment.
maybe add comment that these are initialized in Secure mode initialization code instead.
arch/arc/core/swap.S
Outdated
There was a problem hiding this comment.
See earlier remark on ERSEC_STAT, I think it's ok to access from Normal mode as well.
arch/arc/core/thread.c
Outdated
There was a problem hiding this comment.
Is reading the current contents best way to initialize the SEC_STAT in the initial context? Can't it be some constant value?
There was a problem hiding this comment.
It could be a constan value. I will consider to change it later.
arch/arc/core/vector_table.c
Outdated
There was a problem hiding this comment.
Assume this is for the temporary secure initialization code, it's not really 'logical' to declare secure stuff in a 'NORMAL_FIRMWARE' conditional section. (maybe add a 'TODO' note)
arch/arc/include/swap_macros.h
Outdated
There was a problem hiding this comment.
not sure we cover all combination of options, looks like HAS_SECURE implies we also have USERSPACE - no support for Secure but no userspace?
There was a problem hiding this comment.
if no userspace, no need to save and restore USER/KERNEL_SP realted aux regs.
arch/arc/include/swap_macros.h
Outdated
There was a problem hiding this comment.
Missing case for HAS_SECURE and NORMAL_FIRMWARE?
arch/arc/include/v2/irq.h
Outdated
There was a problem hiding this comment.
not sure an sjli call would be required here, this setup is only done once and could be done by Secure firmware before switching to Normal Zephyr.
There was a problem hiding this comment.
copy/paste typo: this should be nsim_sem_normal
There was a problem hiding this comment.
maybe suspend/resume functionality should have different overall implementation, Secure firmware should probably also support suspend/resume for this to work.
include/arch/arc/v2/arcv2_irq_unit.h
Outdated
There was a problem hiding this comment.
All interrupts either Secure or Normal is probably too strong, some should be allocated to Secure and some to Normal depending on application. For now ok, but add a note (or issue) to support configuration/allocation of interrupts to Secure or Normal.
There was a problem hiding this comment.
It requires a mechinism to tell which interrupts are secure, which are normal
vonhust
force-pushed
the
arc_normal_mode
branch
2 times, most recently
from
206078c to
ed62684
Compare
|
@ruuddw are you ok with this? |
|
@nashif still work in progress, I've marked it DNM for now. |
ok |
vonhust
force-pushed
the
arc_normal_mode
branch
from
ed62684 to
ddb1a59
Compare
|
All checks are passing now. Review history of this comment for details about previous failed status. |
There was a problem hiding this comment.
Ping me when there's documentation to review...
vonhust
force-pushed
the
arc_normal_mode
branch
2 times, most recently
from
6c52893 to
dd88959
Compare
vonhust
force-pushed
the
arc_normal_mode
branch
from
4c4fd18 to
9d23ae3
Compare
There was a problem hiding this comment.
Unless we're referring to the repo name, use an upper case Z on Zephyr.
There was a problem hiding this comment.
let's change this to:
... runs together with a normal Zephyr application.
There was a problem hiding this comment.
Start the sentence with an upper-case letter, The secure application ...
There was a problem hiding this comment.
Capitalize acronyms: RAM, ROM
There was a problem hiding this comment.
Change to:
* Memory not allocated to the secure application is allocated to
the normal application.
There was a problem hiding this comment.
Change this to:
Run using the bootloader
The bootloader should load the secure and normal application into the correct place,
then jump to the entry of the secure application.
Run using the debugger (recommended)
Use the gdb debugger to load and run the two applications.
There was a problem hiding this comment.
Change to:
For nsim sem, you need two consoles: one for
application output, and one for the debugger.
There was a problem hiding this comment.
do you need to specify the path of the secure application, or is that on line 120?
There was a problem hiding this comment.
means any zephyr image built in line 73, i will make it more clarified
vonhust
force-pushed
the
arc_normal_mode
branch
4 times, most recently
from
63b4e74 to
7820d92
Compare
vonhust
force-pushed
the
arc_normal_mode
branch
from
7820d92 to
4f588ec
Compare
I made a force push, could you check that again? |
|
Seems to me the fixes were pushed (but might be missing something @dkinder). |
* it's based on ARC SecureShield * add basic secure service in arch/arc/core/secureshield * necesssary changes in arch level * thread switch * irq/exception handling * initialization * add secure time support Signed-off-by: Wayne Ren <[email protected]>
* Non-secure/normal application: em_starterkit_em7d_normal * secure application: em_starterkit_em7d Signed-off-by: Wayne Ren <[email protected]>
normal/non-secure application: nsim_sem_normal Signed-off-by: Wayne Ren <[email protected]>
* this is a simple sample to show how secure applicaiton and non-secure application work together. More details are in README.rst Signed-off-by: Wayne Ren <[email protected]>
vonhust
force-pushed
the
arc_normal_mode
branch
from
4f588ec to
4908e17
Compare
There is still a request-for-changes by @dbkinder. |
vonhust
dismissed
dbkinder’s stale review
all feedbacks are addressed, neeto merge for zephyr 2.0
There is no relation between this PR and #17767. I just dismissed dbkinder's request as i addressed his feedbacks. Let's go for merge |
Yes. @dbkinder if there are still imperfections in documentation, we can definitely address them during the stabilization period. |
7 participants
This PR is initial implementation of some kind of TEE support for ARC.
As the TEE design/framework in zephyr matures, more work will be needed.
It includes necessary commits to let zephyr run in normal mode of
arc processors equipped with SecureShield.
In this PR, zephyr can be built into:
boot, as when processor is reset, it's secure mode.
To show how normal zephyr application works with secure zephyr applicaiton, an example is provided in samples/boards/arc_secure_services, more details, pls go to the README.rst of this example.