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

Implementation of COMPONENT_SECUREF with JEDEC TG424_3 for secure flash block device driver #15436

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlvinZhou97
Copy link

@AlvinZhou97 AlvinZhou97 commented Jul 13, 2023

Summary of changes

Add Secure Flash block device driver in COMPONENT_SECUREF

  • Support for Macronix ArmorFlash™ MX78 series.
  • Support for JEDEC Serial NOR Security HAL(TG424_3) in COMPONENT_SECUREF/TG424_3
    • Please read README in TG424_3 for detail

Impact of changes

Migration actions required

Documentation


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

This driver is tested on STM32 NUCLEO-L4R5ZI-P with Mxcronix ArmorFlash MX78U64A00F

Since COMPONENT_SECUREF does not yet support test_multi_thread, test_program_read_small_data_sizes, and test_unaligned_erase_blocks, so they will be ignored in the following tests.

target platform_name test suite test case passed failed result elapsed_time (sec)
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device DEFAULT Testing get type functionality 1 0 OK 0.1
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing BlockDevice erase functionality 1 0 OK 0.46
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing Deinit block device 1 0 OK 0.09
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing Init block device 1 0 OK 0.09
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing contiguous erase, write and read 1 0 OK 0.69
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing multi threads erase program read 1 0 OK 4.03
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing program read small data sizes 1 0 OK 0.28
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing read write random blocks 1 0 OK 1.0
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing unaligned erase blocks 1 0 OK 0.14
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device FLASHIAP Testing write -> deinit -> init -> read 1 0 OK 0.14
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing BlockDevice erase functionality 1 0 OK 0.83
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing Deinit block device 1 0 OK 0.09
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing Init block device 1 0 OK 3.73
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing contiguous erase, write and read 1 0 OK 2.67
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing multi threads erase program read 1 0 OK 0.2
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing program read small data sizes 1 0 OK 0.21
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing read write random blocks 1 0 OK 3.34
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing unaligned erase blocks 1 0 OK 0.19
NUCLEO_L4R5ZI_P-GCC_ARM NUCLEO_L4R5ZI_P storage-blockdevice-tests-tests-blockdevice-general_block_device SECUREF Testing write -> deinit -> init -> read 1 0 OK 11.16
[] No Tests required for this change (E.g docs only update) [x] Covered by existing mbed-os tests (Greentea or Unittest) [] Tests / results supplied as part of this PR

Reviewers


Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

There's lot of changes, some should be separated (own commit or even separate pull request). Please review

@@ -4,7 +4,7 @@
"present": 1,
"main-thread-stack-size": {
"help": "The size of the main thread's stack",
"value": 4096
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be separate commit but the main thread should be changed in the application, this would not fit smaller devices.

Copy link
Author

Choose a reason for hiding this comment

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

Got it, I will move to the appropriate place.

PUBLIC
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/include
)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a new line at the end of the file, following our guidelines

Copy link
Author

Choose a reason for hiding this comment

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

I will make the correction, thanks.


int32_t spi_nor_read(uint8_t *buffer, uint32_t addr, uint32_t size)
{
//TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

why there's lot of TODO in this file?

@@ -139,6 +139,27 @@ class BlockDevice {
return 0;
}

virtual int secure_read(void *buffer, mbed::bd_addr_t addr, mbed::bd_size_t size, int app_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be separate commit (adding new functionality).

Why it is required?

Copy link
Author

Choose a reason for hiding this comment

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

These functions are related to the Read/Program/Erase operations used by Secure Flash. The "app_id" parameter is intended to restrict the related application to only operate within a specific zone in Secure Flash.

@mergify mergify bot added the needs: work label Jul 18, 2023
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.

2 participants