Change associated to this is here: https://review.trustedfirmware.org/c/trusted-firmware-m/+/1260

Hi Thomas, I see you mention a failure with GNUARM 7.3.1 with some build config. Can you please provide instructions on how to reproduce the build failure? As you say the failure happens with the mainline as well, that shouldn't happen and I am not seeing the failure either. Would like to try to reproduce it here if it's a genuine issue.

Related change:
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1256/

Further patch for this after document migration
https://review.trustedfirmware.org/c/trusted-firmware-m/+/1249/

Further patch for this after document migration
https://review.trustedfirmware.org/c/trusted-firmware-m/+/1248

Patches link about change the context of the manifest file to align with PSA:
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1240/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1241/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1242/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1243/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1244/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1245/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1246/
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1247/

Patch:
https://review.trustedfirmware.org/c/trusted-firmware-m/+/1234

The secure partition init function cannot cover all use cases. The requirement of pre-rtos secure call actually comes from my mbed-os/tf-m port on Nuvoton's M2351 chip. For example, on mbed-os, the CMSIS API SystemCoreClockUpdate(...) is called to update SystemCoreClock in pre-rtos stage on NS side. On Nuvoton's M2351, SystemCoreClockUpdate(...)'s implementation needs to access CLK space registers which are hardwired to secure. That's where secure call in pre-rtos stage is necessary. I've also checked SystemCoreClockUpdate(...)'s implementation on Arm's Musca A1. It has SystemCoreClock fixed in macro, and so it needn't.

I don't really know the full context of this, so maybe I am way off here, but if there is some secure code that needs to be executed before the NS RTOS is started, is it not best executed as part of secure init? The secure partition containing the secure function (the one that must be called before the RTOS is started) will have an init function, so could that be used to execute the required code?

Pushed the changes: https://review.trustedfirmware.org/c/trusted-firmware-m/+/1233

In T376#4490, @ccli8 wrote:

This call involves a Thread -> Handler mode request on every service call to check if we are in pre-RTOS stage. I think this will introduce a non-negligible penalty; in most of the cases, we expect this call to happen when the RTOS has been loaded.

For the osKernelGetState() overhead in tfm_ns_lock_dispatch(...), I think it can be replaced by just checking ns_lock.init.

This call involves a Thread -> Handler mode request on every service call to check if we are in pre-RTOS stage. I think this will introduce a non-negligible penalty; in most of the cases, we expect this call to happen when the RTOS has been loaded.

Upstream change 1231 to support secure call in pre-rtos stage in tfm_ns_lock_dispatch(...). I think some audience would benefit from it. Without it, I need to make an extra check for pre-rtos scenario before making a secure call.

Strictly speaking, the files in interface/src are a possible implementation of the interface described in interface/include. Your integration can provide a different implementation of tfm_ns_lock_dispatch(...) based on your requirements, without the need to upstream your change. But if you think that your change can be useful for a wider audience, yes, please create a change where you modify tfm_ns_lock_dispatch(...) to check for pre-rtos stage and we'll get that reviewed.

After dropping 1123, create another change which adds support for pre-retos dispatch in tfm_ns_lock_dispatch by checking kernel state with osKernelGetState, right?

Thanks for summarising the three options.

1123 is for NS secure call at pre-rtos stage and 1124 for in interrupt-disabled condition. They are different and so separate changes. For 1123, since osKernelGetState can substitute for get_init_state. I have three choices:

1. Abandon 1123 (and also get_init_state)
2. Re-implement get_init_state with osKernelGetState
3. Abandon 1123 (and also get_init_state) and integrate pre-rtos NS secure call into tfm_ns_lock_dispatch

Just to be clear, as there has been some confusion between get_init_state() and get_lock_state (particularly on my side :) ), I think that the get_init_state(...) doesn't need to be exported as probably the same result can be obtained by proper usage of CMSIS-RTOS2 API's (or equivalent API's, based on the NS side scenario). Regarding the get_lock_state(...), I will comment on the other thread. T378

I agree in principle with the idea, but I have a comment regarding the implementation.

The NS lock is initialized at a point in time when the scheduler is not yet started, therefore there is a single thread of execution on the NS side.
I agree it is safe to assume that in such a scenario, the only actor on the NS side is privileged and therefore is assumed to be in full control of execution, there are no separate protection domains within NSPE.
Secure lock is already set up so there's no risk of introducing new exploits with this change.

Change for this ticket is available here: https://review.trustedfirmware.org/c/trusted-firmware-m/+/1223

Related change:
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1220/

Update integration guide:
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1155/

https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/951/ reviewed, approved, upstreamed

Could you open a review on https://review.trustedfirmware.org and push the code there for review?

Patch for this task https://review.trustedfirmware.org/c/trusted-firmware-m/+/1212

Due to increased interest in this feature and no objections to the implementation concept, I'm raising the priority and will rebase my proposal change and do some polishing to make it a good candidate for upstreaming.
The suggested change of naming convention was discussed offline but was deemed unnecessary as there's limited risk of the feature being misunderstood and that is planned to be mitigated by improved documentation, while the design pattern evoked by the current name is hopefully a reasonable point of reference.

Fix for second issue:
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/1197/

Additional patch is required for Musca A to make sure that the quantity of RAM assigned to the SPE in the region_defs.h header is increased for IPC mode build even if the tests are not build, as RAM requirements for IPC are higher than the current default limit of 64 KB. Patch for this is available here: https://review.trustedfirmware.org/c/trusted-firmware-m/+/1184

Upstreamed

Follow-on task to remove deprecated test case: https://developer.trustedfirmware.org/T387

Patch for this task
https://review.trustedfirmware.org/c/trusted-firmware-m/+/1181