blob: 97303905fd1b529abfa753985fa041af2bf9fab1 [file] [log] [blame]
Paul Beesley07f0a312019-05-16 13:33:18 +01001Coding Guidelines
2=================
Paul Beesley4e2e1b22019-01-17 15:44:37 +00003
Paul Beesley07f0a312019-05-16 13:33:18 +01004This document provides some additional guidelines to consider when writing
5|TF-A| code. These are not intended to be strictly-enforced rules like the
6contents of the :ref:`Coding Style`.
Paul Beesley4e2e1b22019-01-17 15:44:37 +00007
Paul Beesley07f0a312019-05-16 13:33:18 +01008Automatic Editor Configuration
9------------------------------
Paul Beesley4e2e1b22019-01-17 15:44:37 +000010
Paul Beesley07f0a312019-05-16 13:33:18 +010011Many of the rules given below (such as indentation size, use of tabs, and
12newlines) can be set automatically using the `EditorConfig`_ configuration file
13in the root of the repository: ``.editorconfig``. With a supported editor, the
14rules set out in this file can be automatically applied when you are editing
15files in the |TF-A| repository.
Paul Beesleyfd688342019-01-21 16:11:28 +000016
Paul Beesley07f0a312019-05-16 13:33:18 +010017Several editors include built-in support for EditorConfig files, and many others
18support its functionality through plugins.
Paul Beesleyfd688342019-01-21 16:11:28 +000019
Paul Beesley07f0a312019-05-16 13:33:18 +010020Use of the EditorConfig file is suggested but is not required.
Paul Beesleyfd688342019-01-21 16:11:28 +000021
Sandrine Bailleuxe0cb1892020-08-14 15:58:50 +020022.. _automatic-compliance-checking:
Paul Beesleyfd688342019-01-21 16:11:28 +000023
Paul Beesley07f0a312019-05-16 13:33:18 +010024Automatic Compliance Checking
25-----------------------------
Paul Beesleyfd688342019-01-21 16:11:28 +000026
Paul Beesley07f0a312019-05-16 13:33:18 +010027To assist with coding style compliance, the project Makefile contains two
28targets which both utilise the `checkpatch.pl` script that ships with the Linux
29source tree. The project also defines certain *checkpatch* options in the
30``.checkpatch.conf`` file in the top-level directory.
Paul Beesleyfd688342019-01-21 16:11:28 +000031
Paul Beesley07f0a312019-05-16 13:33:18 +010032.. note::
33 Checkpatch errors will gate upstream merging of pull requests.
34 Checkpatch warnings will not gate merging but should be reviewed and fixed if
35 possible.
Paul Beesley4e2e1b22019-01-17 15:44:37 +000036
Paul Beesley07f0a312019-05-16 13:33:18 +010037To check the entire source tree, you must first download copies of
38``checkpatch.pl``, ``spelling.txt`` and ``const_structs.checkpatch`` available
39in the `Linux master tree`_ *scripts* directory, then set the ``CHECKPATCH``
40environment variable to point to ``checkpatch.pl`` (with the other 2 files in
41the same directory) and build the `checkcodebase` target:
Paul Beesley4e2e1b22019-01-17 15:44:37 +000042
Paul Beesley07f0a312019-05-16 13:33:18 +010043.. code:: shell
Paul Beesley4e2e1b22019-01-17 15:44:37 +000044
Paul Beesley07f0a312019-05-16 13:33:18 +010045 make CHECKPATCH=<path-to-linux>/linux/scripts/checkpatch.pl checkcodebase
Paul Beesley4e2e1b22019-01-17 15:44:37 +000046
Paul Beesley07f0a312019-05-16 13:33:18 +010047To just check the style on the files that differ between your local branch and
48the remote master, use:
Paul Beesley4e2e1b22019-01-17 15:44:37 +000049
Paul Beesley07f0a312019-05-16 13:33:18 +010050.. code:: shell
Paul Beesley4e2e1b22019-01-17 15:44:37 +000051
Paul Beesley07f0a312019-05-16 13:33:18 +010052 make CHECKPATCH=<path-to-linux>/linux/scripts/checkpatch.pl checkpatch
Paul Beesley4e2e1b22019-01-17 15:44:37 +000053
Paul Beesley07f0a312019-05-16 13:33:18 +010054If you wish to check your patch against something other than the remote master,
55set the ``BASE_COMMIT`` variable to your desired branch. By default,
56``BASE_COMMIT`` is set to ``origin/master``.
Paul Beesley4e2e1b22019-01-17 15:44:37 +000057
Paul Beesley07f0a312019-05-16 13:33:18 +010058Ignored Checkpatch Warnings
59^^^^^^^^^^^^^^^^^^^^^^^^^^^
Paul Beesley4e2e1b22019-01-17 15:44:37 +000060
Paul Beesley07f0a312019-05-16 13:33:18 +010061Some checkpatch warnings in the TF codebase are deliberately ignored. These
62include:
Paul Beesley4e2e1b22019-01-17 15:44:37 +000063
Paul Beesley07f0a312019-05-16 13:33:18 +010064- ``**WARNING: line over 80 characters**``: Although the codebase should
65 generally conform to the 80 character limit this is overly restrictive in some
66 cases.
Paul Beesley4e2e1b22019-01-17 15:44:37 +000067
Paul Beesley07f0a312019-05-16 13:33:18 +010068- ``**WARNING: Use of volatile is usually wrong``: see
69 `Why the “volatile” type class should not be used`_ . Although this document
70 contains some very useful information, there are several legimate uses of the
71 volatile keyword within the TF codebase.
Paul Beesley4e2e1b22019-01-17 15:44:37 +000072
Paul Beesley07f0a312019-05-16 13:33:18 +010073Performance considerations
74--------------------------
Paul Beesley4e2e1b22019-01-17 15:44:37 +000075
Paul Beesley07f0a312019-05-16 13:33:18 +010076Avoid printf and use logging macros
77^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Paul Beesley4e2e1b22019-01-17 15:44:37 +000078
Paul Beesley07f0a312019-05-16 13:33:18 +010079``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``)
80which wrap ``tf_log`` and which allow the logging call to be compiled-out
81depending on the ``make`` command. Use these macros to avoid print statements
82being compiled unconditionally into the binary.
Paul Beesley4e2e1b22019-01-17 15:44:37 +000083
Paul Beesley07f0a312019-05-16 13:33:18 +010084Each logging macro has a numerical log level:
Paul Beesley4e2e1b22019-01-17 15:44:37 +000085
86.. code:: c
87
Paul Beesley07f0a312019-05-16 13:33:18 +010088 #define LOG_LEVEL_NONE 0
89 #define LOG_LEVEL_ERROR 10
90 #define LOG_LEVEL_NOTICE 20
91 #define LOG_LEVEL_WARNING 30
92 #define LOG_LEVEL_INFO 40
93 #define LOG_LEVEL_VERBOSE 50
Paul Beesley4e2e1b22019-01-17 15:44:37 +000094
Paul Beesley07f0a312019-05-16 13:33:18 +010095By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will
96be compiled into debug builds and all statements with a log level
97``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be
98overridden from the command line or by the platform makefile (although it may be
Sandrine Bailleux391679f2020-08-20 10:41:36 +020099necessary to clean the build directory first).
100
101For example, to enable ``VERBOSE`` logging on FVP:
102
103.. code:: shell
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000104
Sandrine Bailleux391679f2020-08-20 10:41:36 +0200105 make PLAT=fvp LOG_LEVEL=50 all
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000106
Paul Beesley07f0a312019-05-16 13:33:18 +0100107Use const data where possible
108^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000109
Paul Beesley07f0a312019-05-16 13:33:18 +0100110For example, the following code:
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000111
Paul Beesleyfd688342019-01-21 16:11:28 +0000112.. code:: c
113
Paul Beesley07f0a312019-05-16 13:33:18 +0100114 struct my_struct {
Paul Beesleyfd688342019-01-21 16:11:28 +0000115 int arg1;
116 int arg2;
Paul Beesley07f0a312019-05-16 13:33:18 +0100117 };
Paul Beesleyfd688342019-01-21 16:11:28 +0000118
Paul Beesley07f0a312019-05-16 13:33:18 +0100119 void init(struct my_struct *ptr);
120
121 void main(void)
122 {
123 struct my_struct x;
124 x.arg1 = 1;
125 x.arg2 = 2;
126 init(&x);
127 }
Paul Beesleyfd688342019-01-21 16:11:28 +0000128
129is better written as:
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000130
131.. code:: c
132
Paul Beesleyfd688342019-01-21 16:11:28 +0000133 struct my_struct {
134 int arg1;
135 int arg2;
136 };
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000137
Paul Beesley07f0a312019-05-16 13:33:18 +0100138 void init(const struct my_struct *ptr);
Paul Beesleyfd688342019-01-21 16:11:28 +0000139
Paul Beesley07f0a312019-05-16 13:33:18 +0100140 void main(void)
141 {
142 const struct my_struct x = { 1, 2 };
143 init(&x);
144 }
Paul Beesleyfd688342019-01-21 16:11:28 +0000145
Paul Beesley07f0a312019-05-16 13:33:18 +0100146This allows the linker to put the data in a read-only data section instead of a
147writeable data section, which may result in a smaller and faster binary. Note
148that this may require dependent functions (``init()`` in the above example) to
149have ``const`` arguments, assuming they don't need to modify the data.
Paul Beesleyfd688342019-01-21 16:11:28 +0000150
Soby Mathew497092d2019-06-20 12:46:11 +0100151Libc functions that are banned or to be used with caution
152---------------------------------------------------------
153
154Below is a list of functions that present security risks and either must not be
155used (Banned) or are discouraged from use and must be used with care (Caution).
156
157+------------------------+-----------+--------------------------------------+
158| libc function | Status | Comments |
159+========================+===========+======================================+
John Tsichritzis890f3f02019-07-09 18:09:03 +0100160| ``strcpy, wcscpy``, | Banned | use strlcpy instead |
Soby Mathew497092d2019-06-20 12:46:11 +0100161| ``strncpy`` | | |
162+------------------------+-----------+--------------------------------------+
John Tsichritzis890f3f02019-07-09 18:09:03 +0100163| ``strcat, wcscat``, | Banned | use strlcat instead |
Soby Mathew497092d2019-06-20 12:46:11 +0100164| ``strncat`` | | |
John Tsichritzis890f3f02019-07-09 18:09:03 +0100165+------------------------+-----------+--------------------------------------+
Soby Mathew497092d2019-06-20 12:46:11 +0100166| ``sprintf, vsprintf`` | Banned | use snprintf, vsnprintf |
167| | | instead |
John Tsichritzis890f3f02019-07-09 18:09:03 +0100168+------------------------+-----------+--------------------------------------+
Soby Mathew497092d2019-06-20 12:46:11 +0100169| ``snprintf`` | Caution | ensure result fits in buffer |
170| | | i.e : snprintf(buf,size...) < size |
171+------------------------+-----------+--------------------------------------+
172| ``vsnprintf`` | Caution | inspect va_list match types |
173| | | specified in format string |
174+------------------------+-----------+--------------------------------------+
175| ``strtok`` | Banned | use strtok_r or strsep instead |
176+------------------------+-----------+--------------------------------------+
177| ``strtok_r, strsep`` | Caution | inspect for terminated input buffer |
178+------------------------+-----------+--------------------------------------+
179| ``ato*`` | Banned | use equivalent strto* functions |
180+------------------------+-----------+--------------------------------------+
181| ``*toa`` | Banned | Use snprintf instead |
182+------------------------+-----------+--------------------------------------+
183
184The `libc` component in the codebase will not add support for the banned APIs.
185
Paul Beesleyfd688342019-01-21 16:11:28 +0000186Error handling and robustness
187-----------------------------
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000188
189Using CASSERT to check for compile time data errors
190^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
191
192Where possible, use the ``CASSERT`` macro to check the validity of data known at
193compile time instead of checking validity at runtime, to avoid unnecessary
194runtime code.
195
196For example, this can be used to check that the assembler's and compiler's views
197of the size of an array is the same.
198
199.. code:: c
200
201 #include <cassert.h>
202
203 define MY_STRUCT_SIZE 8 /* Used by assembler source files */
204
205 struct my_struct {
206 uint32_t arg1;
207 uint32_t arg2;
208 };
209
210 CASSERT(MY_STRUCT_SIZE == sizeof(struct my_struct), assert_my_struct_size_mismatch);
211
212
213If ``MY_STRUCT_SIZE`` in the above example were wrong then the compiler would
214emit an error like this:
215
Paul Beesley493e3492019-03-13 15:11:04 +0000216::
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000217
218 my_struct.h:10:1: error: size of array ‘assert_my_struct_size_mismatch’ is negative
219
220
221Using assert() to check for programming errors
222^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
223
224In general, each secure world TF image (BL1, BL2, BL31 and BL32) should be
225treated as a tightly integrated package; the image builder should be aware of
226and responsible for all functionality within the image, even if code within that
227image is provided by multiple entities. This allows us to be more aggressive in
228interpreting invalid state or bad function arguments as programming errors using
229``assert()``, including arguments passed across platform porting interfaces.
230This is in contrast to code in a Linux environment, which is less tightly
231integrated and may attempt to be more defensive by passing the error back up the
232call stack.
233
234Where possible, badly written TF code should fail early using ``assert()``. This
235helps reduce the amount of untested conditional code. By default these
236statements are not compiled into release builds, although this can be overridden
237using the ``ENABLE_ASSERTIONS`` build flag.
238
239Examples:
240
241- Bad argument supplied to library function
242- Bad argument provided by platform porting function
243- Internal secure world image state is inconsistent
244
245
246Handling integration errors
247^^^^^^^^^^^^^^^^^^^^^^^^^^^
248
249Each secure world image may be provided by a different entity (for example, a
250Trusted Boot vendor may provide the BL2 image, a TEE vendor may provide the BL32
251image and the OEM/SoC vendor may provide the other images).
252
253An image may contain bugs that are only visible when the images are integrated.
254The system integrator may not even have access to the debug variants of all the
255images in order to check if asserts are firing. For example, the release variant
256of BL1 may have already been burnt into the SoC. Therefore, TF code that detects
257an integration error should _not_ consider this a programming error, and should
258always take action, even in release builds.
259
260If an integration error is considered non-critical it should be treated as a
261recoverable error. If the error is considered critical it should be treated as
262an unexpected unrecoverable error.
263
264Handling recoverable errors
265^^^^^^^^^^^^^^^^^^^^^^^^^^^
266
267The secure world **must not** crash when supplied with bad data from an external
268source. For example, data from the normal world or a hardware device. Similarly,
269the secure world **must not** crash if it detects a non-critical problem within
270itself or the system. It must make every effort to recover from the problem by
271emitting a ``WARN`` message, performing any necessary error handling and
272continuing.
273
274Examples:
275
276- Secure world receives SMC from normal world with bad arguments.
277- Secure world receives SMC from normal world at an unexpected time.
278- BL31 receives SMC from BL32 with bad arguments.
279- BL31 receives SMC from BL32 at unexpected time.
280- Secure world receives recoverable error from hardware device. Retrying the
281 operation may help here.
282- Non-critical secure world service is not functioning correctly.
283- BL31 SPD discovers minor configuration problem with corresponding SP.
284
285Handling unrecoverable errors
286^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
287
288In some cases it may not be possible for the secure world to recover from an
289error. This situation should be handled in one of the following ways:
290
2911. If the unrecoverable error is unexpected then emit an ``ERROR`` message and
292 call ``panic()``. This will end up calling the platform-specific function
293 ``plat_panic_handler()``.
2942. If the unrecoverable error is expected to occur in certain circumstances,
295 then emit an ``ERROR`` message and call the platform-specific function
296 ``plat_error_handler()``.
297
Paul Beesley07f0a312019-05-16 13:33:18 +0100298Cases 1 and 2 are subtly different. A platform may implement
299``plat_panic_handler`` and ``plat_error_handler`` in the same way (for example,
300by waiting for a secure watchdog to time-out or by invoking an interface on the
301platform's power controller to reset the platform). However,
302``plat_error_handler`` may take additional action for some errors (for example,
303it may set a flag so the platform resets into a different mode). Also,
304``plat_panic_handler()`` may implement additional debug functionality (for
305example, invoking a hardware breakpoint).
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000306
307Examples of unexpected unrecoverable errors:
308
309- BL32 receives an unexpected SMC response from BL31 that it is unable to
310 recover from.
311- BL31 Trusted OS SPD code discovers that BL2 has not loaded the corresponding
312 Trusted OS, which is critical for platform operation.
313- Secure world discovers that a critical hardware device is an unexpected and
314 unrecoverable state.
315- Secure world receives an unexpected and unrecoverable error from a critical
316 hardware device.
317- Secure world discovers that it is running on unsupported hardware.
318
319Examples of expected unrecoverable errors:
320
321- BL1/BL2 fails to load the next image due to missing/corrupt firmware on disk.
322- BL1/BL2 fails to authenticate the next image due to an invalid certificate.
323- Secure world continuously receives recoverable errors from a hardware device
324 but is unable to proceed without a valid response.
325
326Handling critical unresponsiveness
327^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
328
329If the secure world is waiting for a response from an external source (for
330example, the normal world or a hardware device) which is critical for continued
331operation, it must not wait indefinitely. It must have a mechanism (for example,
332a secure watchdog) for resetting itself and/or the external source to prevent
333the system from executing in this state indefinitely.
334
335Examples:
336
337- BL1 is waiting for the normal world to raise an SMC to proceed to the next
338 stage of the secure firmware update process.
339- A Trusted OS is waiting for a response from a proxy in the normal world that
340 is critical for continued operation.
341- Secure world is waiting for a hardware response that is critical for continued
342 operation.
343
Paul Beesley07f0a312019-05-16 13:33:18 +0100344Use of built-in *C* and *libc* data types
345-----------------------------------------
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000346
Paul Beesley07f0a312019-05-16 13:33:18 +0100347The |TF-A| codebase should be kept as portable as possible, especially since
348both 64-bit and 32-bit platforms are supported. To help with this, the following
349data type usage guidelines should be followed:
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000350
Paul Beesley07f0a312019-05-16 13:33:18 +0100351- Where possible, use the built-in *C* data types for variable storage (for
352 example, ``char``, ``int``, ``long long``, etc) instead of the standard *C99*
353 types. Most code is typically only concerned with the minimum size of the
354 data stored, which the built-in *C* types guarantee.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000355
Paul Beesley07f0a312019-05-16 13:33:18 +0100356- Avoid using the exact-size standard *C99* types in general (for example,
357 ``uint16_t``, ``uint32_t``, ``uint64_t``, etc) since they can prevent the
358 compiler from making optimizations. There are legitimate uses for them,
359 for example to represent data of a known structure. When using them in struct
360 definitions, consider how padding in the struct will work across architectures.
361 For example, extra padding may be introduced in |AArch32| systems if a struct
362 member crosses a 32-bit boundary.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000363
Paul Beesley07f0a312019-05-16 13:33:18 +0100364- Use ``int`` as the default integer type - it's likely to be the fastest on all
365 systems. Also this can be assumed to be 32-bit as a consequence of the
366 `Procedure Call Standard for the Arm Architecture`_ and the `Procedure Call
367 Standard for the Arm 64-bit Architecture`_ .
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000368
Paul Beesley07f0a312019-05-16 13:33:18 +0100369- Avoid use of ``short`` as this may end up being slower than ``int`` in some
370 systems. If a variable must be exactly 16-bit, use ``int16_t`` or
371 ``uint16_t``.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000372
Paul Beesley07f0a312019-05-16 13:33:18 +0100373- Avoid use of ``long``. This is guaranteed to be at least 32-bit but, given
374 that `int` is 32-bit on Arm platforms, there is no use for it. For integers of
375 at least 64-bit, use ``long long``.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000376
Paul Beesley07f0a312019-05-16 13:33:18 +0100377- Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000378
Paul Beesley07f0a312019-05-16 13:33:18 +0100379- Use ``unsigned`` for integers that can never be negative (counts,
380 indices, sizes, etc). TF intends to comply with MISRA "essential type" coding
381 rules (10.X), where signed and unsigned types are considered different
382 essential types. Choosing the correct type will aid this. MISRA static
383 analysers will pick up any implicit signed/unsigned conversions that may lead
384 to unexpected behaviour.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000385
Paul Beesley07f0a312019-05-16 13:33:18 +0100386- For pointer types:
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000387
Paul Beesley07f0a312019-05-16 13:33:18 +0100388 - If an argument in a function declaration is pointing to a known type then
389 simply use a pointer to that type (for example: ``struct my_struct *``).
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000390
Paul Beesley07f0a312019-05-16 13:33:18 +0100391 - If a variable (including an argument in a function declaration) is pointing
392 to a general, memory-mapped address, an array of pointers or another
393 structure that is likely to require pointer arithmetic then use
394 ``uintptr_t``. This will reduce the amount of casting required in the code.
395 Avoid using ``unsigned long`` or ``unsigned long long`` for this purpose; it
396 may work but is less portable.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000397
Paul Beesley07f0a312019-05-16 13:33:18 +0100398 - For other pointer arguments in a function declaration, use ``void *``. This
399 includes pointers to types that are abstracted away from the known API and
400 pointers to arbitrary data. This allows the calling function to pass a
401 pointer argument to the function without any explicit casting (the cast to
402 ``void *`` is implicit). The function implementation can then do the
403 appropriate casting to a specific type.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000404
Paul Beesley07f0a312019-05-16 13:33:18 +0100405 - Avoid pointer arithmetic generally (as this violates MISRA C 2012 rule
406 18.4) and especially on void pointers (as this is only supported via
407 language extensions and is considered non-standard). In TF-A, setting the
408 ``W`` build flag to ``W=3`` enables the *-Wpointer-arith* compiler flag and
409 this will emit warnings where pointer arithmetic is used.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000410
Paul Beesley07f0a312019-05-16 13:33:18 +0100411 - Use ``ptrdiff_t`` to compare the difference between 2 pointers.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000412
Paul Beesley07f0a312019-05-16 13:33:18 +0100413- Use ``size_t`` when storing the ``sizeof()`` something.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000414
Paul Beesley07f0a312019-05-16 13:33:18 +0100415- Use ``ssize_t`` when returning the ``sizeof()`` something from a function that
416 can also return an error code; the signed type allows for a negative return
417 code in case of error. This practice should be used sparingly.
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000418
Paul Beesley07f0a312019-05-16 13:33:18 +0100419- Use ``u_register_t`` when it's important to store the contents of a register
420 in its native size (32-bit in |AArch32| and 64-bit in |AArch64|). This is not a
421 standard *C99* type but is widely available in libc implementations,
422 including the FreeBSD version included with the TF codebase. Where possible,
423 cast the variable to a more appropriate type before interpreting the data. For
424 example, the following struct in ``ep_info.h`` could use this type to minimize
425 the storage required for the set of registers:
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000426
427.. code:: c
428
Paul Beesley07f0a312019-05-16 13:33:18 +0100429 typedef struct aapcs64_params {
430 u_register_t arg0;
431 u_register_t arg1;
432 u_register_t arg2;
433 u_register_t arg3;
434 u_register_t arg4;
435 u_register_t arg5;
436 u_register_t arg6;
437 u_register_t arg7;
438 } aapcs64_params_t;
Paul Beesleyfd688342019-01-21 16:11:28 +0000439
Paul Beesley07f0a312019-05-16 13:33:18 +0100440If some code wants to operate on ``arg0`` and knows that it represents a 32-bit
441unsigned integer on all systems, cast it to ``unsigned int``.
Paul Beesleyfd688342019-01-21 16:11:28 +0000442
Paul Beesley07f0a312019-05-16 13:33:18 +0100443These guidelines should be updated if additional types are needed.
Paul Beesleyfd688342019-01-21 16:11:28 +0000444
Sandrine Bailleux22d6abc2020-08-20 11:17:41 +0200445Favor C language over assembly language
446---------------------------------------
447
448Generally, prefer code written in C over assembly. Assembly code is less
449portable, harder to understand, maintain and audit security wise. Also, static
450analysis tools generally don't analyze assembly code.
451
Boyan Karatoteve2e05782023-03-24 13:09:33 +0000452If specific system-level instructions must be used (like cache maintenance
453operations), please consider using inline assembly. The ``arch_helpers.h`` files
454already define inline functions for a lot of these.
Sandrine Bailleux22d6abc2020-08-20 11:17:41 +0200455
Boyan Karatoteve2e05782023-03-24 13:09:33 +0000456There are, however, legitimate uses of assembly language. These are usually
457early boot (eg. cpu reset sequences) and exception handling code before the C
458runtime environment is set up.
Sandrine Bailleux22d6abc2020-08-20 11:17:41 +0200459
Boyan Karatoteve2e05782023-03-24 13:09:33 +0000460When writing assembly please note that a wide variety of common instruction
461sequences have helper macros in ``asm_macros.S`` which are preferred over
462writing them directly. This is especially important for debugging purposes as
463debug symbols must manually be included. Please use the ``func_prologue`` and
464``func_epilogue`` macros if you need to use the stack. Also, obeying the
465Procedure Call Standard (PCS) is generally recommended.
Sandrine Bailleux22d6abc2020-08-20 11:17:41 +0200466
Sandrine Bailleux7a53a912023-02-08 13:55:51 +0100467Do not use weak functions
468-------------------------
469
470.. note::
471
472 The following guideline applies more strongly to common, platform-independent
473 code. For plaform code (under ``plat/`` directory), it is up to each platform
474 maintainer to decide whether this should be striclty enforced or not.
475
476The use of weak functions is highly discouraged in the TF-A codebase. Newly
477introduced platform interfaces should be strongly defined, wherever possible. In
478the rare cases where this is not possible or where weak functions appear as the
479best tool to solve the problem at hand, this should be discussed with the
480project's maintainers and justified in the code.
481
482For the purpose of providing a default implementation of a platform interface,
483an alternative to weak functions is to provide a strongly-defined implementation
484under the ``plat/common/`` directory. Then platforms have two options to pull
485in this implementation:
486
487 - They can include the source file through the platform's makefile. Note that
488 this method is suitable only if the platform wants *all* default
489 implementations defined in this file, else either the file should be
490 refactored or the next approach should be used.
491
492 - They access the platform interface through a **constant** function pointer.
493
494In both cases, what matters is that platforms include the default implementation
495as a conscious decision.
496
497.. rubric:: Rationale
498
499Weak functions may sound useful to simplify the initial porting effort to a
500new platform, such that one can quickly get the firmware to build and link,
501without implementing all platform interfaces from the beginning. For this
502reason, the TF-A project used to make heavy use of weak functions and there
503are still many outstanding usages of them across the code base today. We
504intend to convert them to strongly-defined functions over time.
505
506However, weak functions also have major drawbacks, which we consider
507outweighing their benefits. They can make it hard to identify which
508implementation gets built into the firmware, especially when using multiple
509levels of "weakness". This has resulted in bugs in the past.
510
511Weak functions are also forbidden by MISRA coding guidelines, which TF-A aims to
512comply with.
513
Paul Beesley07f0a312019-05-16 13:33:18 +0100514--------------
Paul Beesleyfd688342019-01-21 16:11:28 +0000515
Sandrine Bailleux7a53a912023-02-08 13:55:51 +0100516*Copyright (c) 2020 - 2023, Arm Limited and Contributors. All rights reserved.*
Paul Beesleyfd688342019-01-21 16:11:28 +0000517
Paul Beesley07f0a312019-05-16 13:33:18 +0100518.. _`Linux master tree`: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
Govindraj Raja0667bb32022-11-10 15:27:35 +0000519.. _`Procedure Call Standard for the Arm Architecture`: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst
520.. _`Procedure Call Standard for the Arm 64-bit Architecture`: https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst
Paul Beesley07f0a312019-05-16 13:33:18 +0100521.. _`EditorConfig`: http://editorconfig.org/
Paul Beesley4e2e1b22019-01-17 15:44:37 +0000522.. _`Why the “volatile” type class should not be used`: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html
Paul Beesley07f0a312019-05-16 13:33:18 +0100523.. _`MISRA C:2012 Guidelines`: https://www.misra.org.uk/Activities/MISRAC/tabid/160/Default.aspx
524.. _`a spreadsheet`: https://developer.trustedfirmware.org/file/download/lamajxif3w7c4mpjeoo5/PHID-FILE-fp7c7acszn6vliqomyhn/MISRA-and-TF-Analysis-v1.3.ods