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.

Sign up for GitHub

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

Jump to bottom

Implement HWM on/off behaviour #99

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Implement HWM on/off behaviour #99

merged 3 commits into from
Apr 11, 2024

Conversation

ajinkyaraj-23
Copy link
Collaborator

@ajinkyaraj-23 ajinkyaraj-23 commented Apr 9, 2024 •
edited
Loading

In order to make HWM setting in NVRAM optional and still keep the HWM tracking , following changes are implemented in this MR.

  1. An independent structure is creating in global RAM to track hwm settings.
  2. This data structure in RAM is only point of contact for HWM data for the application.
  3. In case the HWM setting is enabled, and extra update is done to NVRAM when the RAM HWM data is updated. This is distinctly written in macros of the form UPDATE_NVRAM...
  4. Change of settings HWM (Enabled/disabled) is always executed and updated in NVRAM

@ajinkyaraj-23
Move tracking of HWM to RAM
27f9ae8 
- rename nvram_data to baking_hwm_data
- Move out updating hwm_data out of UPDATE_NVRAM macro
- update single variable at a time in nvram
- enable/disable nvram update in macro based on hwm setting.
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch 2 times, most recently from ca689d5 to 5998000 Compare April 9, 2024 13:00
@ajinkyaraj-23 ajinkyaraj-23 requested a review from spalmer25 April 9, 2024 13:07
spalmer25
spalmer25 reviewed Apr 9, 2024
View reviewed changes
test/test_instructions.py Outdated Show resolved Hide resolved
test/test_instructions.py Outdated Show resolved Hide resolved
src/types.h Outdated Show resolved Hide resolved
src/apdu_reset.c Outdated Show resolved Hide resolved
src/apdu_reset.c Outdated Show resolved Hide resolved
src/apdu_pubkey.c Show resolved Hide resolved
@ajinkyaraj-23
Replace usage of NVRAM hwm_data to RAM hwm_data
9e15b4e 
- Remove HWM struct from apdu.
- Create a global define g_hwm for RAM hwm_data
- Move NVRAM hwm_data to RAM during app init
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch from 5998000 to 7b7740f Compare April 9, 2024 14:48
@ajinkyaraj-23 ajinkyaraj-23 added this to the Baking app upgrade milestone Apr 9, 2024
@ajinkyaraj-23 ajinkyaraj-23 self-assigned this Apr 9, 2024
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch 3 times, most recently from 9e5169f to cb4b232 Compare April 9, 2024 16:05
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch from cb4b232 to 64a532f Compare April 9, 2024 16:10
spalmer25
spalmer25 reviewed Apr 10, 2024
View reviewed changes
Copy link
Collaborator

@spalmer25 spalmer25 left a comment

Choose a reason for hiding this comment

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

Can you add a manual test to show that disabling the HWM parameter stops the HWM recording and enabling it allows the HWM to be recorded?

test/utils/navigator.py Outdated Show resolved Hide resolved
test/test_instructions.py Outdated Show resolved Hide resolved
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch 2 times, most recently from ffe5579 to 04c542e Compare April 10, 2024 10:58
@spalmer25 spalmer25 self-requested a review April 10, 2024 11:12
spalmer25
spalmer25 reviewed Apr 10, 2024
View reviewed changes
test/test_instructions.py Show resolved Hide resolved
test/test_instructions.py Outdated Show resolved Hide resolved
test/test_instructions.py Outdated Show resolved Hide resolved
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch from 04c542e to a0d6524 Compare April 10, 2024 13:44
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch from a0d6524 to 764c94d Compare April 11, 2024 06:58
@ajinkyaraj-23 ajinkyaraj-23 force-pushed the 90-add-hwm-onoff-in-settings branch from 764c94d to 846f39c Compare April 11, 2024 09:58
spalmer25
spalmer25 approved these changes Apr 11, 2024
View reviewed changes
Copy link
Collaborator

@spalmer25 spalmer25 left a comment

Choose a reason for hiding this comment

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

LGTM

@ajinkyaraj-23 ajinkyaraj-23 merged commit a92013c into main Apr 11, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spalmer25 spalmer25 spalmer25 approved these changes

Assignees

@ajinkyaraj-23 ajinkyaraj-23

Labels
new-baking-app
Projects
None yet
Milestone
Baking app upgrade
Development

Successfully merging this pull request may close these issues.
2 participants
@ajinkyaraj-23 @spalmer25