sancus-tee / sancus-core

Minimal OpenMSP430 hardware extensions for isolation and attestation
BSD 3-Clause "New" or "Revised" License
20 stars 13 forks source link

Improve IRQ real-time compliance #24

Closed jovanbulck closed 2 years ago

jovanbulck commented 4 years ago

This PR adds support for interruptible crypto operations (in an abandon-and-restart fashion) + use SSA memory pointer for enclave register content instead of enclave stack.

Still needs to be done before merging:

fritzalder commented 4 years ago

Added https://github.com/sancus-tee/sancus-compiler/pull/33 which add the SSA compatibility for linker and trusted runtime. Those work flawlessly now with maybe 50% of the sancus examples (the rest probably failing because of sm_support and confidential loading)

jovanbulck commented 4 years ago

okay, added some comments on the trusted runtime in the compiler repo. I suspect those may be the cause of the failing tests rather than the interruptible crypto. Normally the sancus-examples don't have a lot of (random) IRQs and confidential loading is still enabled in the core, so I don't expect those to fail the tests.

jovanbulck commented 3 years ago

I think this concludes the changes to sancus-core :-) All todos above have been addressed and all unit tests are normally passing now w/ and w/o the ATOMICITY_MONITOR, but make sure to do a careful review @fritzalder esp for the atomicity monitor changes..

Some notes:

Last but not least, I only tested the core under the custom assembly unit test framework. So no guarantees everything still works with the sancus-compiler and this should be checked and ran against sancus-examples. I did push a small commit in sancus-tee/sancus-compiler#33 that should normally add transparent compiler support for interrupt-restart crypto, but again I didn't test this at the level of the compiler/sancus-examples.

jovanbulck commented 2 years ago

@fritzalder why did you need to add an extra wait cycle in the " Adding wait cycle after violations " commit c624d84 above?

Not sure why exactly, but this extra cycle breaks some test cases. Probably a timign issue in the stimulus files, but I would like to make sure the extra wait cycle is really needed before I figure out how to fix the test cases?

fritzalder commented 2 years ago

The wait cycles are indeed necessary after loading the SSA address. At least in verilator, the read SSA address is otherwise not writeable into the mab in the next cycle which means that on an SM IRQ, the SSA will not be correctly written and the SSA remains empty. Whenever an SM then wants to restore itself from an SSA, it will only read zero bytes and crash.

jovanbulck commented 2 years ago

for reference, after digging deeper with @fritzalder, we established that the wait cycle is needed (at least in verilator simulations) since the result of mdb_in is used in the same cycle for mab when writing the PC to the SSA frame (somehow this doesn't manifest as an issue when using iverilog for the test bench sm_irq).

I'll update the failing unit tests before this commit can be merged.

fritzalder commented 2 years ago

As discussed, I merged this to speed things up and opened #28 to address the last tests.