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.

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

Integrate OPTIGA Trust M #3186

Merged
merged 1 commit into from
Aug 1, 2023
Merged

Integrate OPTIGA Trust M #3186

merged 1 commit into from
Aug 1, 2023

Conversation

andrewkozlik
Copy link
Contributor

@andrewkozlik andrewkozlik commented Jul 28, 2023

Further work in this PR:

  • I am probably doing the initialization of the I2C interface wrong. There has been some refactoring recently around I2C, so that's not reflected in this PR. @TychoVrahe (Edit: Done)
  • Maybe move the code to a subdirectory in core/embed/trezorhal such as se or optiga. (Edit: Done)

Further work, that can be split into separate issues/PRs:

  • Not sure why I can't use the full capacity of the data register. See OPTIGA_DATA_REG_LEN in transport.h. @TychoVrahe (Edit: Solved)
  • We will need to agree on how to integrate this with the emulator. My plan is to add a new file optiga.h, which will implement some higher level operations, such as optiga_get_device_cert() or optiga_check_pin(). Only these would be used by the firmware directly and only they would need to be implemented for the emulator.
  • Improve management of timeouts. Right now there is just a for-loop with some arbitrary maximum retry count.
  • Improve error handling in general, e.g. retransmission in case of CRC failures.
  • Add the files to the build process only for the right firmwares. How will we determine that? Using TREZOR_MODEL, or something else? (Edit: Solved)

@andrewkozlik andrewkozlik added the core Trezor Core firmware. Runs on Trezor Model T and T2B1. label Jul 28, 2023
@andrewkozlik andrewkozlik self-assigned this Jul 28, 2023
@TychoVrahe
Copy link
Contributor

Added a commit 3c3dec0

It handles

  • the I2C initialization, which is done in the MCU-specific i2c driver, so its removed from optiga impl. We can probably simplify a bit by
  • moves the optiga code to subdirectory as proposed (trezorhal/optiga), these files should now be(or at least aim to be) independent on underlying MCU so they stand above
  • the files are added to build for T2B1, revision 10, firmware for now
  • adds optiga_hal.c file, which initializes the reset pin and exposes hw reset funciton.

Tested the optiga_open_application command on T2B1 HW, returns success.

Agree with adding single interface file optiga.h and exposing only higher level operations if emulator support is required.

@andrewkozlik andrewkozlik requested review from matejcik and removed request for prusnak July 28, 2023 16:37
@TychoVrahe TychoVrahe mentioned this pull request Jul 28, 2023
@TychoVrahe
Copy link
Contributor

As for the OPTIGA_DATA_REG_LEN limitation: it seems to me that the I2C is timeouting. e.g if you send 225 bytes, thats 1800 bits and with 200kHz i2c that is some 9ms. Timeout in optiga_transport is set to 10ms. So with some more bytes to send this should timeout. Try increasing the timeout. We could alternatively increase the I2C speed to 400kHz (which is the limit), just be aware that we don't want this speed for every i2c, specifically touch layer comm, and the driver is shared.

@andrewkozlik
Copy link
Contributor Author

As for the OPTIGA_DATA_REG_LEN limitation: it seems to me that the I2C is timeouting.

Thanks! That was the problem. I increased the timeout to 15 ms and it works fine now. dbca703

@Hannsek Hannsek requested a review from hiviah July 31, 2023 12:04
Copy link
Contributor

@hiviah hiviah left a comment

Choose a reason for hiding this comment

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

Awesome.

@andrewkozlik andrewkozlik merged commit 7475931 into master Aug 1, 2023
@andrewkozlik andrewkozlik deleted the andrewkozlik/optiga branch August 1, 2023 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants