Page MenuHomePhabricator
Create Task
Maniphest T923

PS relies on linking ITS library to get its definition
Open, NormalPublic
Actions

Description

Although ITS & PS are known to be tightly-coupled, the current code seems to be more than that.
Current ITS/PS look more like conjoined twins, instead of working side-by-side and still being two identities.

One of the tricky stuff is that, PS_xxx is defined under ITS scope.
(https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/partitions/internal_trusted_storage/CMakeLists.txt#n56)
Which relies on linking ITS library to get its own definition from ITS.

ps_encrypted_object.c & PS_ENCRYPTION for example:
(https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/partitions/protected_storage/ps_encrypted_object.c#n15)
PS_ENCRYPTION is set to ON by default (config/config_default.cmake)
Source code, in ITS, will *see* the definition because of the $<$<BOOL:${PS_ENCRYPTION}>:PS_ENCRYPTION> statement in CMakeLists.txt

However, PS does not have the same definition in its CMakeLists.txt.
Theoretically, Source code under PS scope would have compiling error when it compiles ps_encrypted_object.c, because the code uses the field encapsulated by PS_ENCRYPTION, which is defined in ps_object_defs.h.
Magically, PS_ENCRYPTION is defined when it compiles ps_encrypted_object.c.

Another example is TFM_PARTITION_PROTECTED_STORAGE
It's being used in both ITS & PS, and the tfm_ps_init() is almost identical to the code in the TFM_PARTITION_PROTECTED_STORAGE scope in tfm_its_init().

Event Timeline

AlamyLiu triaged this task as Normal priority.Apr 29 2021, 7:10 AM
AlamyLiu created this task.
davidhuziji reassigned this task from davidhuziji to Sherryzhang2.Apr 29 2021, 7:12 AM
davidhuziji added a subscriber: davidhuziji.
Sherryzhang2 added a comment.May 7 2021, 9:03 AM

Hi Alamy,

Thanks for your interest in PS/ITS services.

Yes, the PS and ITS service are highly coupled. But not mutually coupled. That means the PS service relies on ITS service. But in turn, ITS service does not rely on PS service. The main reason for PS calling the ITS partition is to reduce code size. It means only one copy of the filesystem/flash code is needed in the ITS partition, with PS able to use it too by calling ITS.

The build time configurations related to the flash operations are passed into the ITS partition as PS operates the flash via ITS.

The file ps_encrypted_object.c is only compiled into the ps partition when PS_ENCRYPTION is ON. See
https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/partitions/protected_storage/CMakeLists.txt#n40

AlamyLiu added a comment.May 19 2021, 6:18 AM

Hi Sherry,

Thank you for explaining the coding structure.
What I want to say is (to make it simple), the source code (ITS/PS) does not service its purpose well anymore, in my opinion.

There are many ways, in writing code, to archive the same purpose. (i.e.: there is not only one way from point A to point B).
One of the purpose, of source code, is to help other people to understand the coding structure, logic, design idea, ...etc easily.
If I write a piece of code, so tricky, that other people could not understand it easily. To me, it's not other people not good enough to understand my code. Contrary, it's that I'm not good enough to explain things, in a simple way, for other people to understand.

PS_ENCRYPTION for example, it's used by PS, but it's defined in ITS scope. (along with many other PS_xxx)
https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/partitions/internal_trusted_storage/CMakeLists.txt#n65
I wonder how many engineers out there could explain why it works. And see if they could answer that why it could not be defined in PS scope.
This tricky coding method is not knowledge that would help engineers to improve coding skill in my opinion.
It very depends on 'cmake', and it might be broken if cmake changed the way how it compiles the code (although unlikely).

I believe you already know that, how the library (structures/algorithms) was invented, to save memory and/or improve performance.
And I do believe there are other ways to organize the ITS/PS code, that they could be more graceful.

I would save my words for now.
Best Regards,

Sherryzhang2 added a comment.May 19 2021, 9:12 AM

Hi Alamy,

I believe the ITS/PS service can always be improved.

As I explained, currently, PS access the file system relying on ITS. That is why some PS build-time configurations are passed into the ITS partition. Are you proposing that the PS partition should not share the file system with ITS as it makes it difficult to understand the code?

Besides that it is hard to understand, is there any other point that you think should be improved? Or do you have specific suggestions on the code structure design to help us improve it?

By the way, TF-M is an open-source project. Feel free to contribute to it.

Regards,
Sherry

• knrdigital7 added a subscriber: • knrdigital7.Aug 18 2023, 7:26 PM

By leveraging the power of data analytics and utilizing the latest tools and technologies, KNR Digital help businesses optimize their digital marketing efforts and achieve measurable results.