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

Fix direct access to device key test for small erase/program ratio #9932

Merged

Conversation

davidsaada
Copy link
Contributor

Description

This PR fixes the failure in the "Direct access to device key" test when working with internal flash components, whose erase size to program size ratio is small. In such cases, the last two sectors are not large enough to store the device key.

This fixes the test failure in #9908.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[x] Test update
[ ] Breaking change

@davidsaada davidsaada mentioned this pull request Mar 4, 2019
@offirko
Copy link
Contributor

offirko commented Mar 4, 2019

@mmahadevan108 , @mikisch81 - FYI

@ciarmcom ciarmcom requested review from a team March 4, 2019 18:00
@ciarmcom
Copy link
Member

ciarmcom commented Mar 4, 2019

@davidsaada, thank you for your changes.
@ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@mikisch81
Copy link
Contributor

mikisch81 commented Mar 4, 2019

@ARMmbed/mbed-os-maintainers @maclobdell @amiraloosh this is needed for #9908 and #9910 (new PSA targets) so it needs to have 5.12 label

@cmonr
Copy link
Contributor

cmonr commented Mar 4, 2019

CI started whilst reviews are ongoing

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 4, 2019

This PR fixes the failure in the "Direct access to device key" test when working with internal flash components, whose erase size to program size ratio is small. In such cases, the last two sectors are not large enough to store the device key.

Shall be added to the commit msg?

@davidsaada
Copy link
Contributor Author

This PR fixes the failure in the "Direct access to device key" test when working with internal flash components, whose erase size to program size ratio is small. In such cases, the last two sectors are not large enough to store the device key.

Shall be added to the commit msg?

Indeed should be. Will wait to see if there are any required changes (to not duplicate CI), and if not, would kindly ask maintainers to add when merged.

@mbed-ci
Copy link

mbed-ci commented Mar 4, 2019

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter

@cmonr
Copy link
Contributor

cmonr commented Mar 5, 2019

@davidsaada Go ahead and make the change if you can. Either one of the CI sub-jobs appear to be misconfigured, or something is broken on master (betting on former).

@ghost ghost added the PM_ACCEPTED label Mar 5, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 5, 2019

@davidsaada Go ahead and make the change if you can. Either one of the CI sub-jobs appear to be misconfigured, or something is broken on master (betting on former).

We will investigate. CI will be restarted

@davidsaada davidsaada force-pushed the david_devkey_direct_test_large_page branch from 314642b to d0b01ef Compare March 5, 2019 08:59
@davidsaada
Copy link
Contributor Author

Commit message modified.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 5, 2019

@ARMmbed/mbed-os-storage Review please

@@ -198,6 +198,13 @@ void test_direct_access_to_devicekey_tdb_last_two_sectors()
uint32_t flash_bd_size = flash_bd_end_address - flash_bd_start_address;

FlashIAPBlockDevice *flash_bd = new FlashIAPBlockDevice((bd_addr_t)flash_bd_start_address, (bd_size_t)flash_bd_size);
flash_bd->init();
uint32_t erase_prog_ratio = flash_bd->get_erase_size(flash_bd->size() - 1) / flash_bd->get_program_size();
Copy link
Contributor

Choose a reason for hiding this comment

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

This calculates and checks only the last sector erase_prog_ratio but on a very rare occasion, the second sector may have different erase_prog_ratio due to different sector size. I recommend adding a calculation and checkup also for the second sector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed now. Please review.

This commit fixes the failure in the "Direct access to device key" test,
when working with internal flash components, whose erase size to program
size ratio is small. In such cases, the last two sectors are not large
enough to store the device key.
@davidsaada davidsaada force-pushed the david_devkey_direct_test_large_page branch from d0b01ef to eb29af5 Compare March 5, 2019 09:37
Copy link
Contributor

@yossi2le yossi2le left a comment

Choose a reason for hiding this comment

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

LGTM

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 5, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Mar 5, 2019

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter

@alekla01
Copy link
Contributor

alekla01 commented Mar 5, 2019

rebuilt exporter, error unrelated to this pr, and will be fixed elsewhere.

@0xc0170 0xc0170 merged commit 03bb615 into ARMmbed:master Mar 5, 2019
@davidsaada davidsaada deleted the david_devkey_direct_test_large_page branch March 16, 2019 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants