1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
|
#####################
Code Review Guideline
#####################
The purpose of this document is to clarify design items to be reviewed during
the code review process.
Please contact :doc:`maintainers </contributing/maintainers>` or write an e-mail
thread on the `TF-M mailing list <mailto:tf-m@lists.trustedfirmware.org>`_ for
any questions.
**********************
List of the guidelines
**********************
The prerequisites before going to the review stage:
- Read the :doc:`Contributing Process </contributing/contributing_process>`
to know basic concepts.
- Read the :ref:`source_structure`
for structure related reference.
The review guidelines consist of these items:
- `Exceptions`_.
- `Non-source items`_.
- `Namespace`_.
- `Assembler code`_.
- `Include`_.
- `Auto-doc`_.
- `Commit Message`_.
.. note::
Each guideline item is assigned with a unique symbol in the format ``Rx.y``,
while ``x`` and ``y`` are numeric symbols. These symbols are created for easy
referencing during the code review process.
Exceptions
==========
Files under listed folders are fully or partially imported from 3rd party
projects, these files follow the original project defined guidelines and
styles:
.. important::
- R1.1 ``ext`` and its subfolders.
Non-source items
================
For all non-source files such as build system files or configuration files:
.. important::
- R2.1 Follow the existing style while making changes.
Namespace
=========
TF-M assign the namespace to files and symbols for an easier code reading. The
symbol here includes functions, variables, types and MACROs. The prerequisites:
.. important::
- R3.1 Follow the documents or specifications if they propose namespaces
('psa\_' for PSA defined items E.g.,). Ask the contributor to tell the
documents or specifications if the reviewer is not sure about a namespace.
- R3.2 Assign TF-M specific namespace 'tfm\_' or 'TFM\_' for symbols
implementing TF-M specific features. Ask the contributor to clarify the
purpose of the patch if the reviewer is not sure about the namespace.
For the sources out of the above prerequisites (R3.1 and R3.2):
.. important::
- R3.3 Do not assign a namespace for source files.
- R3.4 Assigning a namespace for interface symbols is recommended. The
namespace needs to be expressed in one word to be followed by other words
into a phrase can represent the functionalities implemented. One word
naming is allowed if the namespace can represent the functionality
perfectly.
- R3.5 Assign a namespace for private symbols is NOT recommended.
- R3.6 Do not assign characters out of alphabets as the leading character.
Examples:
.. code-block:: c
/* R3.1 FILE: s/spm/core/psa_client.c */
/* R3.2 FILE: s/spm/core/tfm_secure_context.c */
/* R3.4 FILE: s/spm/core/spm.c, 'spm\_' as the namespace */
void spm_init(void);
/* R3.5 FILE: s/spm/core/main.c */
static void init_functions(void);
/* R3.6 Not permitted: */
/* static uint32_t __count; */
Assembler code
==============
.. important::
- R4.1 Pure assembler sources or inline assembler code are required to be put
under the platform-independent or architecture-independent folders.
The logic folders should not contain any assembler code, referring to
external MACRO wrapped assembler code is allowed. Here is one example of the
logic folder:
- 'secure_fw/spm'.
Examples:
.. code-block:: c
/*
* R4.1 The following MACRO is allowed to be referenced under
* 'secure_fw/spm'
*/
#define SVC(code) __asm volatile("svc %0", ::"I"(code))
Include
=======
This chapter describes the placement of the headers and including. There are
two types of headers: The ``interface`` headers contain symbols to be shared
between modules and the ``private`` headers contain symbols only for internal
usage.
.. important::
- R5.1 Put the ``interface header`` of one module in the ``include`` folder
under the root of this module. Deeper sub-folders can not have ``include``
folders, which means only one ``include`` is allowed for one module.
- R5.2 Creating sub-folders under ``include`` to represent the more granular
scope of the interfaces is allowed.
- R5.3 ``private header`` can be put at the same place with the implementation
sources for the private symbols contained in the header. It also can be put
at the place where the sources need it. The latter is useful when some
"private header" contains abstracted interfaces, but these interfaces are
not public interfaces so it won't be put under "include" folder.
- R5.4 Use <> when including public headers.
- R5.5 Use "" when including private headers.
- R5.6 The module's ``include`` folder needs to be added into referencing
module's header searching path.
- R5.7 The module's ``include`` folder and the root folder needs to be added
into its own header searching path and apply a hierarchy including with
folder name.
- R5.8 Path hierarchy including is allowed since there are sub-folders under
``include`` folder and the module folder.
- R5.9 The including statement group order: the beginning group contains
toolchain headers, then follows the public headers group and finally the
private headers group.
- R5.10 The including statement order inside a group: Compare the include
statement as strings and sort by the string comparison result.
- R5.11 The header for the referenced symbol or definition must be included
even this header is included inside one of the existing included headers.
This improves portability in case the existing header implementation
changed.
Examples:
.. code-block:: c
/*
* The structure:
* module_a/include/func1.h
* module_a/include/func2/fmain.h
* module_a/func1.c
* module_a/func2/fmain.c
* module_b/include/funcx.h
* module_b/include/funcy/fmain.h
* module_b/funcx.c
* module_b/funcxi.h
* module_b/funcy/fmain.c
* module_b/funcy/fsub.c
* module_b/funcy/fsub.h
* Here takes module_b/funcx.c as example:
*/
#include <func1.h>
#include <func2/fmain.h>
#include <funcx.h>
#include "funcxi.h"
#include "funcy/fsub.h"
Auto-doc
========
Auto document system such as doxygen is useful for describing interfaces. While
it would be a development burden since the details are described in the design
documents already. The guidelines for auto-doc:
.. important::
- R6.1 Headers and sources under these folders need to apply auto-doc style
comments: ``*include``.
- R6.2 Developers decide the comment style for sources out of listed folders.
Commit Message
==============
TF-M has the requirements on commit message:
.. important::
- R7.1 Assign correct topic for a patch. Check the following table.
============== ====================================================
Topic Justification
============== ====================================================
Boot `bl2/*` or `bl1/*`
Build For build system related purpose.
Docs All \*.rst changes.
Dualcpu Dual-cpu related changes.
HAL Generic HAL interface/implementation changes.
Interface Interface changes, either Non-source and secure.
Pack For packing purpose.
Platform Generic platform related changes under `platform/*`.
Platform Name Specific platform changes.
Partition Multiple partition related changes.
Partition Name Specific partition related changes.
Service Multiple service related changes.
Service Name Specific service related changes.
SPM `secure_fw/spm/*`
SPRTL `secure-fw/partitions/lib/runtime/*`
Tool `tools` folder or `tf-m-tools` repo
============== ====================================================
.. note::
Ideally, one topic should cover one specific type of changes. For crossing
topic changes, check the main part of the change and use the main part
related topic as patch topic. If there is no suitable topics to cover the
change, contact the community for an update.
--------------
*Copyright (c) 2020-2022, Arm Limited. All rights reserved.*
|