1 Coding Style & Guidelines
2 =========================
4 The following sections contain TF coding guidelines. They are continually
5 evolving and should not be considered "set in stone". Feel free to question them
8 Some of the guidelines may also apply to other codebases.
11 The existing TF codebase does not necessarily comply with all the
12 below guidelines but the intent is for it to do so eventually.
17 Some checkpatch warnings in the TF codebase are deliberately ignored. These
20 - ``**WARNING: line over 80 characters**``: Although the codebase should
21 generally conform to the 80 character limit this is overly restrictive in some
24 - ``**WARNING: Use of volatile is usually wrong``: see
25 `Why the “volatile” type class should not be used`_ . Although this document
26 contains some very useful information, there are several legitimate uses of
27 the volatile keyword within the TF codebase.
35 For a header file called "some_driver.h" the style used by the Trusted Firmware
45 #endif /* SOME_DRIVER_H */
47 Include statement ordering
48 ^^^^^^^^^^^^^^^^^^^^^^^^^^
50 All header files that are included by a source file must use the following,
51 grouped ordering. This is to improve readability (by making it easier to quickly
52 read through the list of headers) and maintainability.
54 #. *System* includes: Header files from the standard *C* library, such as
55 ``stddef.h`` and ``string.h``.
57 #. *Project* includes: Header files under the ``include/`` directory within TF
58 are *project* includes.
60 #. *Platform* includes: Header files relating to a single, specific platform,
61 and which are located under the ``plat/<platform_name>`` directory within TF,
62 are *platform* includes.
64 Within each group, ``#include`` statements must be in alphabetical order,
65 taking both the file and directory names into account.
67 Groups must be separated by a single blank line for clarity.
69 The example below illustrates the ordering rules using some contrived header
70 file names; this type of name reuse should be otherwise avoided.
76 #include <a_dir/example/a_header.h>
77 #include <a_dir/example/b_header.h>
78 #include <a_dir/test/a_header.h>
79 #include <b_dir/example/a_header.h>
81 #include "./a_header.h"
83 Include statement variants
84 ^^^^^^^^^^^^^^^^^^^^^^^^^^
86 Two variants of the ``#include`` directive are acceptable in the TF codebase.
87 Correct use of the two styles improves readability by suggesting the location
88 of the included header and reducing ambiguity in cases where generic and
89 platform-specific headers share a name.
91 For header files that are in the same directory as the source file that is
92 including them, use the ``"..."`` variant.
94 For header files that are **not** in the same directory as the source file that
95 is including them, use the ``<...>`` variant.
105 #include "bl1_private.h"
107 Platform include paths
108 ^^^^^^^^^^^^^^^^^^^^^^
110 Platforms are allowed to add more include paths to be passed to the compiler.
111 The ``PLAT_INCLUDES`` variable is used for this purpose. This is needed in
112 particular for the file ``platform_def.h``.
118 PLAT_INCLUDES += -Iinclude/plat/myplat/include
123 Use of built-in *C* and *libc* data types
124 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
126 The TF codebase should be kept as portable as possible, especially since both
127 64-bit and 32-bit platforms are supported. To help with this, the following data
128 type usage guidelines should be followed:
130 - Where possible, use the built-in *C* data types for variable storage (for
131 example, ``char``, ``int``, ``long long``, etc) instead of the standard *C99*
132 types. Most code is typically only concerned with the minimum size of the
133 data stored, which the built-in *C* types guarantee.
135 - Avoid using the exact-size standard *C99* types in general (for example,
136 ``uint16_t``, ``uint32_t``, ``uint64_t``, etc) since they can prevent the
137 compiler from making optimizations. There are legitimate uses for them,
138 for example to represent data of a known structure. When using them in struct
139 definitions, consider how padding in the struct will work across architectures.
140 For example, extra padding may be introduced in AArch32 systems if a struct
141 member crosses a 32-bit boundary.
143 - Use ``int`` as the default integer type - it's likely to be the fastest on all
144 systems. Also this can be assumed to be 32-bit as a consequence of the
145 `Procedure Call Standard for the Arm Architecture`_ and the `Procedure Call
146 Standard for the Arm 64-bit Architecture`_ .
148 - Avoid use of ``short`` as this may end up being slower than ``int`` in some
149 systems. If a variable must be exactly 16-bit, use ``int16_t`` or
152 - Avoid use of ``long``. This is guaranteed to be at least 32-bit but, given
153 that `int` is 32-bit on Arm platforms, there is no use for it. For integers of
154 at least 64-bit, use ``long long``.
156 - Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data.
158 - Use ``unsigned`` for integers that can never be negative (counts,
159 indices, sizes, etc). TF intends to comply with MISRA "essential type" coding
160 rules (10.X), where signed and unsigned types are considered different
161 essential types. Choosing the correct type will aid this. MISRA static
162 analysers will pick up any implicit signed/unsigned conversions that may lead
163 to unexpected behaviour.
167 - If an argument in a function declaration is pointing to a known type then
168 simply use a pointer to that type (for example: ``struct my_struct *``).
170 - If a variable (including an argument in a function declaration) is pointing
171 to a general, memory-mapped address, an array of pointers or another
172 structure that is likely to require pointer arithmetic then use
173 ``uintptr_t``. This will reduce the amount of casting required in the code.
174 Avoid using ``unsigned long`` or ``unsigned long long`` for this purpose; it
175 may work but is less portable.
177 - For other pointer arguments in a function declaration, use ``void *``. This
178 includes pointers to types that are abstracted away from the known API and
179 pointers to arbitrary data. This allows the calling function to pass a
180 pointer argument to the function without any explicit casting (the cast to
181 ``void *`` is implicit). The function implementation can then do the
182 appropriate casting to a specific type.
184 - Use ``ptrdiff_t`` to compare the difference between 2 pointers.
186 - Use ``size_t`` when storing the ``sizeof()`` something.
188 - Use ``ssize_t`` when returning the ``sizeof()`` something from a function that
189 can also return an error code; the signed type allows for a negative return
190 code in case of error. This practice should be used sparingly.
192 - Use ``u_register_t`` when it's important to store the contents of a register
193 in its native size (32-bit in AArch32 and 64-bit in AArch64). This is not a
194 standard *C99* type but is widely available in libc implementations,
195 including the FreeBSD version included with the TF codebase. Where possible,
196 cast the variable to a more appropriate type before interpreting the data. For
197 example, the following struct in ``ep_info.h`` could use this type to minimize
198 the storage required for the set of registers:
202 typedef struct aapcs64_params {
213 If some code wants to operate on ``arg0`` and knows that it represents a 32-bit
214 unsigned integer on all systems, cast it to ``unsigned int``.
216 These guidelines should be updated if additional types are needed.
218 Avoid anonymous typedefs of structs/enums in headers
219 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
221 For example, the following definition:
231 is better written as:
240 This allows function declarations in other header files that depend on the
241 struct/enum to forward declare the struct/enum instead of including the
246 #include <my_struct.h>
247 void my_func(my_struct_t *arg);
254 void my_func(struct my_struct *arg);
256 Some TF definitions use both a struct/enum name **and** a typedef name. This
257 is discouraged for new definitions as it makes it difficult for TF to comply
258 with MISRA rule 8.3, which states that "All declarations of an object or
259 function shall use the same names and type qualifiers".
261 The Linux coding standards also discourage new typedefs and checkpatch emits
264 Existing typedefs will be retained for compatibility.
266 Libc functions that are banned or to be used with caution
267 ---------------------------------------------------------
269 Below is a list of functions that present security risks and either must not be
270 used (Banned) or are discouraged from use and must be used with care (Caution).
272 +------------------------+-----------+--------------------------------------+
273 | libc function | Status | Comments |
274 +========================+===========+======================================+
275 | ``strcpy, wcscpy``, | Banned | use strlcpy instead |
277 +------------------------+-----------+--------------------------------------+
278 | ``strcat, wcscat``, | Banned | use strlcat instead |
280 +------------------------+-----------+--------------------------------------+
281 | ``sprintf, vsprintf`` | Banned | use snprintf, vsnprintf |
283 +------------------------+-----------+--------------------------------------+
284 | ``snprintf`` | Caution | ensure result fits in buffer |
285 | | | i.e : snprintf(buf,size...) < size |
286 +------------------------+-----------+--------------------------------------+
287 | ``vsnprintf`` | Caution | inspect va_list match types |
288 | | | specified in format string |
289 +------------------------+-----------+--------------------------------------+
290 | ``strtok`` | Banned | use strtok_r or strsep instead |
291 +------------------------+-----------+--------------------------------------+
292 | ``strtok_r, strsep`` | Caution | inspect for terminated input buffer |
293 +------------------------+-----------+--------------------------------------+
294 | ``ato*`` | Banned | use equivalent strto* functions |
295 +------------------------+-----------+--------------------------------------+
296 | ``*toa`` | Banned | Use snprintf instead |
297 +------------------------+-----------+--------------------------------------+
299 The `libc` component in the codebase will not add support for the banned APIs.
301 Error handling and robustness
302 -----------------------------
304 Using CASSERT to check for compile time data errors
305 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
307 Where possible, use the ``CASSERT`` macro to check the validity of data known at
308 compile time instead of checking validity at runtime, to avoid unnecessary
311 For example, this can be used to check that the assembler's and compiler's views
312 of the size of an array is the same.
318 define MY_STRUCT_SIZE 8 /* Used by assembler source files */
325 CASSERT(MY_STRUCT_SIZE == sizeof(struct my_struct), assert_my_struct_size_mismatch);
328 If ``MY_STRUCT_SIZE`` in the above example were wrong then the compiler would
329 emit an error like this:
333 my_struct.h:10:1: error: size of array ‘assert_my_struct_size_mismatch’ is negative
336 Using assert() to check for programming errors
337 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
339 In general, each secure world TF image (BL1, BL2, BL31 and BL32) should be
340 treated as a tightly integrated package; the image builder should be aware of
341 and responsible for all functionality within the image, even if code within that
342 image is provided by multiple entities. This allows us to be more aggressive in
343 interpreting invalid state or bad function arguments as programming errors using
344 ``assert()``, including arguments passed across platform porting interfaces.
345 This is in contrast to code in a Linux environment, which is less tightly
346 integrated and may attempt to be more defensive by passing the error back up the
349 Where possible, badly written TF code should fail early using ``assert()``. This
350 helps reduce the amount of untested conditional code. By default these
351 statements are not compiled into release builds, although this can be overridden
352 using the ``ENABLE_ASSERTIONS`` build flag.
356 - Bad argument supplied to library function
357 - Bad argument provided by platform porting function
358 - Internal secure world image state is inconsistent
361 Handling integration errors
362 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
364 Each secure world image may be provided by a different entity (for example, a
365 Trusted Boot vendor may provide the BL2 image, a TEE vendor may provide the BL32
366 image and the OEM/SoC vendor may provide the other images).
368 An image may contain bugs that are only visible when the images are integrated.
369 The system integrator may not even have access to the debug variants of all the
370 images in order to check if asserts are firing. For example, the release variant
371 of BL1 may have already been burnt into the SoC. Therefore, TF code that detects
372 an integration error should _not_ consider this a programming error, and should
373 always take action, even in release builds.
375 If an integration error is considered non-critical it should be treated as a
376 recoverable error. If the error is considered critical it should be treated as
377 an unexpected unrecoverable error.
379 Handling recoverable errors
380 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
382 The secure world **must not** crash when supplied with bad data from an external
383 source. For example, data from the normal world or a hardware device. Similarly,
384 the secure world **must not** crash if it detects a non-critical problem within
385 itself or the system. It must make every effort to recover from the problem by
386 emitting a ``WARN`` message, performing any necessary error handling and
391 - Secure world receives SMC from normal world with bad arguments.
392 - Secure world receives SMC from normal world at an unexpected time.
393 - BL31 receives SMC from BL32 with bad arguments.
394 - BL31 receives SMC from BL32 at unexpected time.
395 - Secure world receives recoverable error from hardware device. Retrying the
396 operation may help here.
397 - Non-critical secure world service is not functioning correctly.
398 - BL31 SPD discovers minor configuration problem with corresponding SP.
400 Handling unrecoverable errors
401 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
403 In some cases it may not be possible for the secure world to recover from an
404 error. This situation should be handled in one of the following ways:
406 1. If the unrecoverable error is unexpected then emit an ``ERROR`` message and
407 call ``panic()``. This will end up calling the platform-specific function
408 ``plat_panic_handler()``.
409 2. If the unrecoverable error is expected to occur in certain circumstances,
410 then emit an ``ERROR`` message and call the platform-specific function
411 ``plat_error_handler()``.
413 Cases 1 and 2 are subtly different. A platform may implement ``plat_panic_handler``
414 and ``plat_error_handler`` in the same way (for example, by waiting for a secure
415 watchdog to time-out or by invoking an interface on the platform's power
416 controller to reset the platform). However, ``plat_error_handler`` may take
417 additional action for some errors (for example, it may set a flag so the
418 platform resets into a different mode). Also, ``plat_panic_handler()`` may
419 implement additional debug functionality (for example, invoking a hardware
422 Examples of unexpected unrecoverable errors:
424 - BL32 receives an unexpected SMC response from BL31 that it is unable to
426 - BL31 Trusted OS SPD code discovers that BL2 has not loaded the corresponding
427 Trusted OS, which is critical for platform operation.
428 - Secure world discovers that a critical hardware device is an unexpected and
430 - Secure world receives an unexpected and unrecoverable error from a critical
432 - Secure world discovers that it is running on unsupported hardware.
434 Examples of expected unrecoverable errors:
436 - BL1/BL2 fails to load the next image due to missing/corrupt firmware on disk.
437 - BL1/BL2 fails to authenticate the next image due to an invalid certificate.
438 - Secure world continuously receives recoverable errors from a hardware device
439 but is unable to proceed without a valid response.
441 Handling critical unresponsiveness
442 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
444 If the secure world is waiting for a response from an external source (for
445 example, the normal world or a hardware device) which is critical for continued
446 operation, it must not wait indefinitely. It must have a mechanism (for example,
447 a secure watchdog) for resetting itself and/or the external source to prevent
448 the system from executing in this state indefinitely.
452 - BL1 is waiting for the normal world to raise an SMC to proceed to the next
453 stage of the secure firmware update process.
454 - A Trusted OS is waiting for a response from a proxy in the normal world that
455 is critical for continued operation.
456 - Secure world is waiting for a hardware response that is critical for continued
459 Security considerations
460 -----------------------
462 Part of the security of a platform is handling errors correctly, as described in
463 the previous section. There are several other security considerations covered in
466 Do not leak secrets to the normal world
467 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
469 The secure world **must not** leak secrets to the normal world, for example in
472 Handling Denial of Service attacks
473 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
475 The secure world **should never** crash or become unusable due to receiving too
476 many normal world requests (a *Denial of Service* or *DoS* attack). It should
477 have a mechanism for throttling or ignoring normal world requests.
479 Performance considerations
480 --------------------------
482 Avoid printf and use logging macros
483 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
485 ``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``)
486 which wrap ``tf_log`` and which allow the logging call to be compiled-out
487 depending on the ``make`` command. Use these macros to avoid print statements
488 being compiled unconditionally into the binary.
490 Each logging macro has a numerical log level:
494 #define LOG_LEVEL_NONE 0
495 #define LOG_LEVEL_ERROR 10
496 #define LOG_LEVEL_NOTICE 20
497 #define LOG_LEVEL_WARNING 30
498 #define LOG_LEVEL_INFO 40
499 #define LOG_LEVEL_VERBOSE 50
502 By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will
503 be compiled into debug builds and all statements with a log level
504 ``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be
505 overridden from the command line or by the platform makefile (although it may be
506 necessary to clean the build directory first). For example, to enable
507 ``VERBOSE`` logging on FVP:
509 ``make PLAT=fvp LOG_LEVEL=50 all``
511 Use const data where possible
512 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
514 For example, the following code:
523 void init(struct my_struct *ptr);
533 is better written as:
542 void init(const struct my_struct *ptr);
546 const struct my_struct x = { 1, 2 };
550 This allows the linker to put the data in a read-only data section instead of a
551 writeable data section, which may result in a smaller and faster binary. Note
552 that this may require dependent functions (``init()`` in the above example) to
553 have ``const`` arguments, assuming they don't need to modify the data.
555 Library and driver code
556 -----------------------
558 TF library code (under ``lib/`` and ``include/lib``) is any code that provides a
559 reusable interface to other code, potentially even to code outside of TF.
561 In some systems drivers must conform to a specific driver framework to provide
562 services to the rest of the system. TF has no driver framework and the
563 distinction between a driver and library is somewhat subjective.
565 A driver (under ``drivers/`` and ``include/drivers/``) is defined as code that
566 interfaces with hardware via a memory mapped interface.
568 Some drivers (for example, the Arm CCI driver in ``include/drivers/arm/cci.h``)
569 provide a general purpose API to that specific hardware. Other drivers (for
570 example, the Arm PL011 console driver in ``drivers/arm/pl011/pl011_console.S``)
571 provide a specific hardware implementation of a more abstract library API. In
572 the latter case there may potentially be multiple drivers for the same hardware
575 Neither libraries nor drivers should depend on platform-specific code. If they
576 require platform-specific data (for example, a base address) to operate then
577 they should provide an initialization function that takes the platform-specific
580 TF common code (under ``common/`` and ``include/common/``) is code that is re-used
581 by other generic (non-platform-specific) TF code. It is effectively internal
584 .. _`Why the “volatile” type class should not be used`: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html
585 .. _`Procedure Call Standard for the Arm Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf
586 .. _`Procedure Call Standard for the Arm 64-bit Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf