search

tillitis / tillitis-key1

Board designs, FPGA verilog, firmware for TKey, the flexible and open USB security key 🔑
https://www.tillitis.se
382 stars 24 forks source link

Remove possible redundant RAM clearing functionality #224

Closed secworks closed 3 weeks ago

secworks commented 1 month ago

(The purpose of this issue is primarily to document the observed behavior.)

In FW start.S there is functionality to clear the RAM by writing zeros, see: https://github.com/tillitis/tillitis-key1/blob/0454e023cd5c05cf8c84df358c45a05f291e153e/hw/application_fpga/fw/tk1/start.S#L41

In FW main.c there is functionality to fill the RAM with (pseudo) random data, see: https://github.com/tillitis/tillitis-key1/blob/0454e023cd5c05cf8c84df358c45a05f291e153e/hw/application_fpga/fw/tk1/main.c#L366

This functionality is called fairly early in main, see: https://github.com/tillitis/tillitis-key1/blob/0454e023cd5c05cf8c84df358c45a05f291e153e/hw/application_fpga/fw/tk1/main.c#L408

The zero fill in start.S was probably added before RAM scrambling was added. But do we still need to fill the RAM with zeros first, or is this now redundant? Removing it would reduce the time before the FW is ready to receive a device app. It would also reduce the size of the FW with a few bytes. If we remove the zero fill we could probably move the random fill to before the call to print_hw_version(), see:

https://github.com/tillitis/tillitis-key1/blob/0454e023cd5c05cf8c84df358c45a05f291e153e/hw/application_fpga/fw/tk1/main.c#L395

This would make the difference in cycles after reset to when the memory is filled the with random data a few cycles. Basically the number of cycles it takes to jump to main().

secworks commented 1 month ago

A related issue to this is https://github.com/tillitis/tillitis-key1/issues/162

secworks commented 1 month ago

I would suggest that we instead of removing the zeroisation loop change it to fill FW_RAM instead of the RAM. This would have three benifits:

  1. We would shorten the time until the FW is ready to receive an app to load
  2. It would close a hole in regards to possibly have assets in FW_RAM between resets
  3. It would separate the purpose of the setup in start.S vs main.c. In start.S, we setup the execution of the FW in main.c. In main.c we set up the execution of device apps.