Hi Kevin,
Yes my email was meant to be on the mailing list. Added back on.
Thanks
Abhishek
-----Original Message-----
From: Kevin Peng (Arm Technology China) <Kevin.Peng(a)arm.com>
Sent: 21 November 2019 09:13
To: Abhishek Pandit <Abhishek.Pandit(a)arm.com>
Subject: RE: Call for review on header include update
Hi Abhishek,
[I guess you are trying to reply to mailing list as well?] I think it's mostly about coding style.
Most of the headers are included without any relative path in TF-M.
So it's wired that lots of headers are included by a relative path, and relative to the project root make it even more unreasonable.
The reason I didn't explain it in detail is that I didn't find any documents about this code style.
Of course I had discussions with some team members and they are feeling the same.
So I'd like to hear more feedbacks first.
Best Regards,
Kevin
-----Original Message-----
From: Abhishek Pandit <Abhishek.Pandit(a)arm.com>
Sent: Thursday, November 21, 2019 4:54 PM
To: Kevin Peng (Arm Technology China) <Kevin.Peng(a)arm.com>
Cc: nd <nd(a)arm.com>
Subject: RE: Call for review on header include update
Hi Kevin,
Is there something that explains why this is required?
Thanks
Abhishek
-----Original Message-----
From: TF-M <tf-m-bounces(a)lists.trustedfirmware.org> On Behalf Of Kevin Peng (Arm Technology China) via TF-M
Sent: 21 November 2019 08:38
To: tf-m(a)lists.trustedfirmware.org
Cc: nd <nd(a)arm.com>
Subject: [TF-M] Call for review on header include update
Hi,
I've proposed some patches to update the header includes in TF-M:
https://review.trustedfirmware.org/q/topic:%22include_path_update%22+(statu…
The idea is to remove the relative paths to TF-M root and add the necessary paths to include search list.
Any kinds of comments are welcome.
Best Regards,
Kevin
--
TF-M mailing list
TF-M(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-m
Hi,
I've proposed some patches to update the header includes in TF-M:
https://review.trustedfirmware.org/q/topic:%22include_path_update%22+(statu…
The idea is to remove the relative paths to TF-M root and add the necessary paths to include search list.
Any kinds of comments are welcome.
Best Regards,
Kevin
Hi David,
Let's go further. The main goal of the proposal should be to have the all TFM portable code in one place as much as possible - so the top folder is named as "port".
New proposed structure:
port
|-- platform
| |
| |-- Partner ABC
| | |-- common
| | |-- target_abc_1
| | |-- target_abc_2
| |
| |-- Partner XYZ
| | |-- common
| | |-- target_xyz_1
| | |-- target_xyz_2
| |-- ...
| -- arch
| -- include
Thanks,
Andrej
-----Original Message-----
From: TF-M <tf-m-bounces(a)lists.trustedfirmware.org> On Behalf Of David Hu (Arm Technology China) via TF-M
Sent: Wednesday, November 20, 2019 4:20 AM
To: tf-m(a)lists.trustedfirmware.org
Cc: nd <nd(a)arm.com>
Subject: [TF-M] [RFC] Re-organize the TF-M platform folder structure
Hi all,
I'd like to propose to adjust the platform folder structure. Would you please check the details below and share your opinions? Any suggestion or feedback will be very helpful.
In current platform folder structure, all the targets are listed together at the same level under `target` folder.
Platform
|-- ext
|-- .
|-- target_xxx.cmake
|-- .
|-- target_zzz.cmake
|-- target
|-- mps2
|-- musca_a
|-- .
A drawback is that, there are many duplicated files in those targets folders. It may become difficult to maintain the target support when targets, platforms, drivers and features continue growing.
- A platform may have a series of variants. If each variant target folder has to contain a copy of platform drivers or library, it is difficult to align and maintain the duplicated drivers.
- All targets belonging to the same platform contain the duplicated common operation files or configurations, such as HUK, NV counter and platform configurations. A small change has to be applied to multiple identical files.
To solve the duplicated files management issue, I'd propose to insert partner folders under `target` and move the targets under the partner folder. The proposed structure looks like the following:
Platform
|-- ext
|-- target
|
|-- Partner ABC
| |-- *common drivers and files*
| |-- target_abc_1
| | |-- target_abc_1.cmake
| |-- target_abc_2
|
|-- Partner XYZ
| |-- *common drivers and files*
| |-- target_xyz_1
| | |-- target_xyz_1.cmake
| |-- target_xyz_2
|-- ...
The ideas behind the proposal:
- It is more close to the platform support structure in popular RTOSs. It can be more convenient for partners to port platform support to TF-M from existing projects.
- Common drivers and operations implementations can be gathered into partner/platform specific folder. Thus each partner/platform folder doesn't have to hold a duplicated copy.
- It is easier for partners to fully organize and maintain their platform folders with full permission.
More layers can be added under a specific partner folder, such as diverse platforms.
- Put the target specific cmake config file back into the target folder. It can make the `platform` folder much more clear.
Best regards,
Hu Ziji
--
TF-M mailing list
TF-M(a)lists.trustedfirmware.org
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.tru…
Hi David,
The concept itself looks ok for me. So that the scope of vendor would be much clearer.
And I think some structure change out of platform folder needs to happen to cooperate with this change, hope to see the detailed analysis result of the changes to be made.
Thanks.
/Ken
-----Original Message-----
From: TF-M <tf-m-bounces(a)lists.trustedfirmware.org> On Behalf Of David Hu (Arm Technology China) via TF-M
Sent: Wednesday, November 20, 2019 11:20 AM
To: tf-m(a)lists.trustedfirmware.org
Cc: nd <nd(a)arm.com>
Subject: [TF-M] [RFC] Re-organize the TF-M platform folder structure
Hi all,
I'd like to propose to adjust the platform folder structure. Would you please check the details below and share your opinions? Any suggestion or feedback will be very helpful.
In current platform folder structure, all the targets are listed together at the same level under `target` folder.
Platform
|-- ext
|-- .
|-- target_xxx.cmake
|-- .
|-- target_zzz.cmake
|-- target
|-- mps2
|-- musca_a
|-- .
A drawback is that, there are many duplicated files in those targets folders. It may become difficult to maintain the target support when targets, platforms, drivers and features continue growing.
- A platform may have a series of variants. If each variant target folder has to contain a copy of platform drivers or library, it is difficult to align and maintain the duplicated drivers.
- All targets belonging to the same platform contain the duplicated common operation files or configurations, such as HUK, NV counter and platform configurations. A small change has to be applied to multiple identical files.
To solve the duplicated files management issue, I'd propose to insert partner folders under `target` and move the targets under the partner folder. The proposed structure looks like the following:
Platform
|-- ext
|-- target
|
|-- Partner ABC
| |-- *common drivers and files*
| |-- target_abc_1
| | |-- target_abc_1.cmake
| |-- target_abc_2
|
|-- Partner XYZ
| |-- *common drivers and files*
| |-- target_xyz_1
| | |-- target_xyz_1.cmake
| |-- target_xyz_2
|-- ...
The ideas behind the proposal:
- It is more close to the platform support structure in popular RTOSs. It can be more convenient for partners to port platform support to TF-M from existing projects.
- Common drivers and operations implementations can be gathered into partner/platform specific folder. Thus each partner/platform folder doesn't have to hold a duplicated copy.
- It is easier for partners to fully organize and maintain their platform folders with full permission.
More layers can be added under a specific partner folder, such as diverse platforms.
- Put the target specific cmake config file back into the target folder. It can make the `platform` folder much more clear.
Best regards,
Hu Ziji
--
TF-M mailing list
TF-M(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-m
Hi all,
I'd like to propose to adjust the platform folder structure. Would you please check the details below and share your opinions? Any suggestion or feedback will be very helpful.
In current platform folder structure, all the targets are listed together at the same level under `target` folder.
Platform
|-- ext
|-- .
|-- target_xxx.cmake
|-- .
|-- target_zzz.cmake
|-- target
|-- mps2
|-- musca_a
|-- .
A drawback is that, there are many duplicated files in those targets folders. It may become difficult to maintain the target support when targets, platforms, drivers and features continue growing.
- A platform may have a series of variants. If each variant target folder has to contain a copy of platform drivers or library, it is difficult to align and maintain the duplicated drivers.
- All targets belonging to the same platform contain the duplicated common operation files or configurations, such as HUK, NV counter and platform configurations. A small change has to be applied to multiple identical files.
To solve the duplicated files management issue, I'd propose to insert partner folders under `target` and move the targets under the partner folder. The proposed structure looks like the following:
Platform
|-- ext
|-- target
|
|-- Partner ABC
| |-- *common drivers and files*
| |-- target_abc_1
| | |-- target_abc_1.cmake
| |-- target_abc_2
|
|-- Partner XYZ
| |-- *common drivers and files*
| |-- target_xyz_1
| | |-- target_xyz_1.cmake
| |-- target_xyz_2
|-- ...
The ideas behind the proposal:
- It is more close to the platform support structure in popular RTOSs. It can be more convenient for partners to port platform support to TF-M from existing projects.
- Common drivers and operations implementations can be gathered into partner/platform specific folder. Thus each partner/platform folder doesn't have to hold a duplicated copy.
- It is easier for partners to fully organize and maintain their platform folders with full permission.
More layers can be added under a specific partner folder, such as diverse platforms.
- Put the target specific cmake config file back into the target folder. It can make the `platform` folder much more clear.
Best regards,
Hu Ziji
Hi Tamas,
The guidelines look good.
I would suggest distinguishing these rules between `original tfm created sources` and `3rd party` sources because tfm is now involving some 3rd party projects and we should keep their original taste as much as we could.
/Ken
-----Original Message-----
From: TF-M <tf-m-bounces(a)lists.trustedfirmware.org> On Behalf Of Kumar Gala via TF-M
Sent: Tuesday, November 19, 2019 11:22 PM
To: Kevin Townsend <kevin.townsend(a)linaro.org>
Cc: tf-m(a)lists.trustedfirmware.org
Subject: Re: [TF-M] Coding guideline
I’ve been curious if something like https://lgtm.com/ could be used to have checks for code guidelines.
- k
> On Nov 19, 2019, at 8:22 AM, Kevin Townsend via TF-M <tf-m(a)lists.trustedfirmware.org> wrote:
>
> Hi Tamas,
>
> Just to add to the conversation, something that the Zephyr project
> added to make enforcing the coding guidelines easier was a config file for uncrustify.
>
> https://docs.zephyrproject.org/latest/contribute/index.html#uncrustify
>
> The Zephyr config file itself can be seen here:
> https://github.com/zephyrproject-rtos/zephyr/blob/master/.uncrustify.c
> fg
>
> The advantage here is that most text editors can be configured to
> automatically apply these rules and avoid common white space or other issues.
>
> I've generally found this very useful generating PRs and just ensuring
> I didn't make any obvious errors when I commit files or changes.
>
> Best regards,
> Kevin
>
>
> On Mon, 18 Nov 2019 at 17:31, Tamas Ban via TF-M
> <tf-m(a)lists.trustedfirmware.org> wrote:
>>
>> Hi,
>>
>> I would like to open a conversation about TF-M coding guideline<https://git.trustedfirmware.org/trusted-firmware-m.git/tree/docs/coding_gui…>. I have these proposals:
>>
>> * Change the rule of the 80 character column width:
>> * Max 80 characters length.
>> * Column length is max 80 character in first place, but there are exceptions when length could be MAX 120 character:
>> * Degrades code understandability: short, obscured variable names; awkward line breaks
>> * Maximum 1% of the lines can exceed 80 charter length.
>> * Might remove this rule, because in many cases we are not complaint with it. For example PSA Crypto API:
>>
>> o Order function parameters so that input params are before output params.
>>
>> o https://git.trustedfirmware.org/trusted-firmware-m.git/tree/interface/inclu…
>>
>> I'm interested in your opinions! Other rules also can be revisited!
>>
>> Tamas
>> --
>> TF-M mailing list
>> TF-M(a)lists.trustedfirmware.org
>> https://lists.trustedfirmware.org/mailman/listinfo/tf-m
> --
> TF-M mailing list
> TF-M(a)lists.trustedfirmware.org
> https://lists.trustedfirmware.org/mailman/listinfo/tf-m
--
TF-M mailing list
TF-M(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-m
Hi,
Musca-B1 board is the target of PSA Level 2 certification. As part of this process we integrated the CC312 crypto HW accelerator with TF-M. The following major changes was done:
* Replacement of mbedTLS library to mbed-crypto in MCUBoot.
* Integration of CryptoCell-runtime-library to TF-M build system.
* Default behaviour of Musca-B1 is unchanged, it use SW only crypto and relying only on mbed-crypto. HW accelerated crypto can be turned on with CMake command line switch: -DCRYPTO_HW_ACCELERATOR=ON
* When HW crypto acceleration is enabled then CryptoCell-runtime-library and CC312 HW perform the crypto operations instead of mbed-crypto (mbedTLS). The library provides an mbedTLS compliant API, therefore it requires minimal modification in the code base.
* Creating a crypto key provisioning tool to program the TF-M related crypto keys to OTP memory (HUK, ROTPK, attest priv. key). Here we took a sort-cut and currently linked this tool to MCUBoot on demand. CMake command line switch: -DCRYPTO_HW_ACCELERATOR_OTP_STATE=PROVISIONING
* Porting of platform layer in Musca-B1 folder to rely on CC312 and OTP memory. CMake command line switch: -DCRYPTO_HW_ACCELERATOR_OTP_STATE=ENABLED
* Extends initial attestation API to provide a public API to retrieve the public part of the attestation key.
The top of the patch-set:
https://review.trustedfirmware.org/#/c/trusted-firmware-m/+/2539/
We would like to review and merge the changes by 27th of November!
Reviewers are welcome!
Tamas Ban
I’ve been curious if something like https://lgtm.com/ could be used to have checks for code guidelines.
- k
> On Nov 19, 2019, at 8:22 AM, Kevin Townsend via TF-M <tf-m(a)lists.trustedfirmware.org> wrote:
>
> Hi Tamas,
>
> Just to add to the conversation, something that the Zephyr project added to make
> enforcing the coding guidelines easier was a config file for uncrustify.
>
> https://docs.zephyrproject.org/latest/contribute/index.html#uncrustify
>
> The Zephyr config file itself can be seen here:
> https://github.com/zephyrproject-rtos/zephyr/blob/master/.uncrustify.cfg
>
> The advantage here is that most text editors can be configured to automatically
> apply these rules and avoid common white space or other issues.
>
> I've generally found this very useful generating PRs and just ensuring
> I didn't make
> any obvious errors when I commit files or changes.
>
> Best regards,
> Kevin
>
>
> On Mon, 18 Nov 2019 at 17:31, Tamas Ban via TF-M
> <tf-m(a)lists.trustedfirmware.org> wrote:
>>
>> Hi,
>>
>> I would like to open a conversation about TF-M coding guideline<https://git.trustedfirmware.org/trusted-firmware-m.git/tree/docs/coding_gui…>. I have these proposals:
>>
>> * Change the rule of the 80 character column width:
>> * Max 80 characters length.
>> * Column length is max 80 character in first place, but there are exceptions when length could be MAX 120 character:
>> * Degrades code understandability: short, obscured variable names; awkward line breaks
>> * Maximum 1% of the lines can exceed 80 charter length.
>> * Might remove this rule, because in many cases we are not complaint with it. For example PSA Crypto API:
>>
>> o Order function parameters so that input params are before output params.
>>
>> o https://git.trustedfirmware.org/trusted-firmware-m.git/tree/interface/inclu…
>>
>> I'm interested in your opinions! Other rules also can be revisited!
>>
>> Tamas
>> --
>> TF-M mailing list
>> TF-M(a)lists.trustedfirmware.org
>> https://lists.trustedfirmware.org/mailman/listinfo/tf-m
> --
> TF-M mailing list
> TF-M(a)lists.trustedfirmware.org
> https://lists.trustedfirmware.org/mailman/listinfo/tf-m
Hi Tamas,
Just to add to the conversation, something that the Zephyr project added to make
enforcing the coding guidelines easier was a config file for uncrustify.
https://docs.zephyrproject.org/latest/contribute/index.html#uncrustify
The Zephyr config file itself can be seen here:
https://github.com/zephyrproject-rtos/zephyr/blob/master/.uncrustify.cfg
The advantage here is that most text editors can be configured to automatically
apply these rules and avoid common white space or other issues.
I've generally found this very useful generating PRs and just ensuring
I didn't make
any obvious errors when I commit files or changes.
Best regards,
Kevin
On Mon, 18 Nov 2019 at 17:31, Tamas Ban via TF-M
<tf-m(a)lists.trustedfirmware.org> wrote:
>
> Hi,
>
> I would like to open a conversation about TF-M coding guideline<https://git.trustedfirmware.org/trusted-firmware-m.git/tree/docs/coding_gui…>. I have these proposals:
>
> * Change the rule of the 80 character column width:
> * Max 80 characters length.
> * Column length is max 80 character in first place, but there are exceptions when length could be MAX 120 character:
> * Degrades code understandability: short, obscured variable names; awkward line breaks
> * Maximum 1% of the lines can exceed 80 charter length.
> * Might remove this rule, because in many cases we are not complaint with it. For example PSA Crypto API:
>
> o Order function parameters so that input params are before output params.
>
> o https://git.trustedfirmware.org/trusted-firmware-m.git/tree/interface/inclu…
>
> I'm interested in your opinions! Other rules also can be revisited!
>
> Tamas
> --
> TF-M mailing list
> TF-M(a)lists.trustedfirmware.org
> https://lists.trustedfirmware.org/mailman/listinfo/tf-m