-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
tfm: Configuration changes #19868
tfm: Configuration changes #19868
Conversation
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: 550d3b079c10142127b53465f81e17d3c78d1a97 more detailssdk-nrf:
Github labels
List of changed files detected by CI (3)
Outputs:ToolchainVersion: 11349092be Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
You can find the documentation preview for this PR at this link. Note: This comment is automatically posted by the Documentation Publish GitHub Action. |
- Allow CONFIG_TFM_PARTITION_PLATFORM without other partitions. - Kconfig changes for better usability of PS. - Kconfig information for ITS. Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
ca0b49a
to
550d3b0
Compare
@@ -179,17 +181,32 @@ config TFM_ITS_VALIDATE_METADATA_FROM_FLASH | |||
Validate filesystem metadata every time it is read from flash | |||
|
|||
config TFM_ITS_MAX_ASSET_SIZE | |||
range 512 4096 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it makes sense to use the NRF_TRUSTZONE_FLASH_REGION_SIZE as the maximum range here (both here and in the PS)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Although, if we ever get something else than 4096, then other values will need to revisit other values as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, actually, does not make sense in PS, as it is 4024, not 4096, but in here it can be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spoke too soon:
- The maximum size would actually be the flash erase page size (4096).
- Range in Kconfig only deals with numerics, so it is not configurable.
|
||
config TFM_PS_NUM_ASSETS | ||
int "Maximum stored assets number" | ||
range 1 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How was the 120 selected here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The table which stores the files needs to fit in TFM_PS_MAX_ASSET_SIZE. Maximum value of files stored with 4024 was (I think) 123.
area | ||
The maximum number of assets to be stored in the Protected Storage area. | ||
(TF-M does not guarantee that the Protected Storage area is large | ||
enough to store the maximum amount of assets with the maximum size.) Set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enough to store the maximum amount of assets with the maximum size.) Set | |
enough to store the maximum amount of assets with the maximum size) Set |
nit
Or you can even just remove the parenthesis all together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, both suggested edits introduce punctuation errors. Keep the parenthesis and the period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but make sure not to implement this change: https://github.com/nrfconnect/sdk-nrf/pull/19868/files#r1916850492
test_crypto: PR-742