Merge "doc: Split and expand coding style documentation" into integration
This commit is contained in:
commit
cf96f2ed03
|
@ -2763,6 +2763,19 @@ build system.
|
||||||
to ``no``. If any of the options ``EL3_PAYLOAD_BASE`` or ``PRELOADED_BL33_BASE``
|
to ``no``. If any of the options ``EL3_PAYLOAD_BASE`` or ``PRELOADED_BL33_BASE``
|
||||||
are used, this flag will be set to ``no`` automatically.
|
are used, this flag will be set to ``no`` automatically.
|
||||||
|
|
||||||
|
Platform include paths
|
||||||
|
----------------------
|
||||||
|
|
||||||
|
Platforms are allowed to add more include paths to be passed to the compiler.
|
||||||
|
The ``PLAT_INCLUDES`` variable is used for this purpose. This is needed in
|
||||||
|
particular for the file ``platform_def.h``.
|
||||||
|
|
||||||
|
Example:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
PLAT_INCLUDES += -Iinclude/plat/myplat/include
|
||||||
|
|
||||||
C Library
|
C Library
|
||||||
---------
|
---------
|
||||||
|
|
||||||
|
@ -2844,7 +2857,7 @@ amount of open resources per driver.
|
||||||
|
|
||||||
--------------
|
--------------
|
||||||
|
|
||||||
*Copyright (c) 2013-2019, Arm Limited and Contributors. All rights reserved.*
|
*Copyright (c) 2013-2020, Arm Limited and Contributors. All rights reserved.*
|
||||||
|
|
||||||
.. _PSCI: http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
|
.. _PSCI: http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
|
||||||
.. _Arm Generic Interrupt Controller version 2.0 (GICv2): http://infocenter.arm.com/help/topic/com.arm.doc.ihi0048b/index.html
|
.. _Arm Generic Interrupt Controller version 2.0 (GICv2): http://infocenter.arm.com/help/topic/com.arm.doc.ihi0048b/index.html
|
||||||
|
|
|
@ -1,18 +1,61 @@
|
||||||
Coding Style & Guidelines
|
Coding Guidelines
|
||||||
=========================
|
=================
|
||||||
|
|
||||||
The following sections contain TF coding guidelines. They are continually
|
This document provides some additional guidelines to consider when writing
|
||||||
evolving and should not be considered "set in stone". Feel free to question them
|
|TF-A| code. These are not intended to be strictly-enforced rules like the
|
||||||
and provide feedback.
|
contents of the :ref:`Coding Style`.
|
||||||
|
|
||||||
Some of the guidelines may also apply to other codebases.
|
Automatic Editor Configuration
|
||||||
|
------------------------------
|
||||||
|
|
||||||
|
Many of the rules given below (such as indentation size, use of tabs, and
|
||||||
|
newlines) can be set automatically using the `EditorConfig`_ configuration file
|
||||||
|
in the root of the repository: ``.editorconfig``. With a supported editor, the
|
||||||
|
rules set out in this file can be automatically applied when you are editing
|
||||||
|
files in the |TF-A| repository.
|
||||||
|
|
||||||
|
Several editors include built-in support for EditorConfig files, and many others
|
||||||
|
support its functionality through plugins.
|
||||||
|
|
||||||
|
Use of the EditorConfig file is suggested but is not required.
|
||||||
|
|
||||||
|
|
||||||
|
Automatic Compliance Checking
|
||||||
|
-----------------------------
|
||||||
|
|
||||||
|
To assist with coding style compliance, the project Makefile contains two
|
||||||
|
targets which both utilise the `checkpatch.pl` script that ships with the Linux
|
||||||
|
source tree. The project also defines certain *checkpatch* options in the
|
||||||
|
``.checkpatch.conf`` file in the top-level directory.
|
||||||
|
|
||||||
.. note::
|
.. note::
|
||||||
The existing TF codebase does not necessarily comply with all the
|
Checkpatch errors will gate upstream merging of pull requests.
|
||||||
below guidelines but the intent is for it to do so eventually.
|
Checkpatch warnings will not gate merging but should be reviewed and fixed if
|
||||||
|
possible.
|
||||||
|
|
||||||
Checkpatch overrides
|
To check the entire source tree, you must first download copies of
|
||||||
--------------------
|
``checkpatch.pl``, ``spelling.txt`` and ``const_structs.checkpatch`` available
|
||||||
|
in the `Linux master tree`_ *scripts* directory, then set the ``CHECKPATCH``
|
||||||
|
environment variable to point to ``checkpatch.pl`` (with the other 2 files in
|
||||||
|
the same directory) and build the `checkcodebase` target:
|
||||||
|
|
||||||
|
.. code:: shell
|
||||||
|
|
||||||
|
make CHECKPATCH=<path-to-linux>/linux/scripts/checkpatch.pl checkcodebase
|
||||||
|
|
||||||
|
To just check the style on the files that differ between your local branch and
|
||||||
|
the remote master, use:
|
||||||
|
|
||||||
|
.. code:: shell
|
||||||
|
|
||||||
|
make CHECKPATCH=<path-to-linux>/linux/scripts/checkpatch.pl checkpatch
|
||||||
|
|
||||||
|
If you wish to check your patch against something other than the remote master,
|
||||||
|
set the ``BASE_COMMIT`` variable to your desired branch. By default,
|
||||||
|
``BASE_COMMIT`` is set to ``origin/master``.
|
||||||
|
|
||||||
|
Ignored Checkpatch Warnings
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
Some checkpatch warnings in the TF codebase are deliberately ignored. These
|
Some checkpatch warnings in the TF codebase are deliberately ignored. These
|
||||||
include:
|
include:
|
||||||
|
@ -23,210 +66,61 @@ include:
|
||||||
|
|
||||||
- ``**WARNING: Use of volatile is usually wrong``: see
|
- ``**WARNING: Use of volatile is usually wrong``: see
|
||||||
`Why the “volatile” type class should not be used`_ . Although this document
|
`Why the “volatile” type class should not be used`_ . Although this document
|
||||||
contains some very useful information, there are several legitimate uses of
|
contains some very useful information, there are several legimate uses of the
|
||||||
the volatile keyword within the TF codebase.
|
volatile keyword within the TF codebase.
|
||||||
|
|
||||||
Headers and inclusion
|
Performance considerations
|
||||||
---------------------
|
--------------------------
|
||||||
|
|
||||||
Header guards
|
Avoid printf and use logging macros
|
||||||
^^^^^^^^^^^^^
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
For a header file called "some_driver.h" the style used by the Trusted Firmware
|
``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``)
|
||||||
is:
|
which wrap ``tf_log`` and which allow the logging call to be compiled-out
|
||||||
|
depending on the ``make`` command. Use these macros to avoid print statements
|
||||||
|
being compiled unconditionally into the binary.
|
||||||
|
|
||||||
|
Each logging macro has a numerical log level:
|
||||||
|
|
||||||
.. code:: c
|
.. code:: c
|
||||||
|
|
||||||
#ifndef SOME_DRIVER_H
|
#define LOG_LEVEL_NONE 0
|
||||||
#define SOME_DRIVER_H
|
#define LOG_LEVEL_ERROR 10
|
||||||
|
#define LOG_LEVEL_NOTICE 20
|
||||||
|
#define LOG_LEVEL_WARNING 30
|
||||||
|
#define LOG_LEVEL_INFO 40
|
||||||
|
#define LOG_LEVEL_VERBOSE 50
|
||||||
|
|
||||||
<header content>
|
By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will
|
||||||
|
be compiled into debug builds and all statements with a log level
|
||||||
|
``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be
|
||||||
|
overridden from the command line or by the platform makefile (although it may be
|
||||||
|
necessary to clean the build directory first). For example, to enable
|
||||||
|
``VERBOSE`` logging on FVP:
|
||||||
|
|
||||||
#endif /* SOME_DRIVER_H */
|
``make PLAT=fvp LOG_LEVEL=50 all``
|
||||||
|
|
||||||
Include statement ordering
|
Use const data where possible
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
All header files that are included by a source file must use the following,
|
For example, the following code:
|
||||||
grouped ordering. This is to improve readability (by making it easier to quickly
|
|
||||||
read through the list of headers) and maintainability.
|
|
||||||
|
|
||||||
#. *System* includes: Header files from the standard *C* library, such as
|
|
||||||
``stddef.h`` and ``string.h``.
|
|
||||||
|
|
||||||
#. *Project* includes: Header files under the ``include/`` directory within TF
|
|
||||||
are *project* includes.
|
|
||||||
|
|
||||||
#. *Platform* includes: Header files relating to a single, specific platform,
|
|
||||||
and which are located under the ``plat/<platform_name>`` directory within TF,
|
|
||||||
are *platform* includes.
|
|
||||||
|
|
||||||
Within each group, ``#include`` statements must be in alphabetical order,
|
|
||||||
taking both the file and directory names into account.
|
|
||||||
|
|
||||||
Groups must be separated by a single blank line for clarity.
|
|
||||||
|
|
||||||
The example below illustrates the ordering rules using some contrived header
|
|
||||||
file names; this type of name reuse should be otherwise avoided.
|
|
||||||
|
|
||||||
.. code:: c
|
.. code:: c
|
||||||
|
|
||||||
#include <string.h>
|
struct my_struct {
|
||||||
|
|
||||||
#include <a_dir/example/a_header.h>
|
|
||||||
#include <a_dir/example/b_header.h>
|
|
||||||
#include <a_dir/test/a_header.h>
|
|
||||||
#include <b_dir/example/a_header.h>
|
|
||||||
|
|
||||||
#include "./a_header.h"
|
|
||||||
|
|
||||||
Include statement variants
|
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^
|
|
||||||
|
|
||||||
Two variants of the ``#include`` directive are acceptable in the TF codebase.
|
|
||||||
Correct use of the two styles improves readability by suggesting the location
|
|
||||||
of the included header and reducing ambiguity in cases where generic and
|
|
||||||
platform-specific headers share a name.
|
|
||||||
|
|
||||||
For header files that are in the same directory as the source file that is
|
|
||||||
including them, use the ``"..."`` variant.
|
|
||||||
|
|
||||||
For header files that are **not** in the same directory as the source file that
|
|
||||||
is including them, use the ``<...>`` variant.
|
|
||||||
|
|
||||||
Example (bl1_fwu.c):
|
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
#include <assert.h>
|
|
||||||
#include <errno.h>
|
|
||||||
#include <string.h>
|
|
||||||
|
|
||||||
#include "bl1_private.h"
|
|
||||||
|
|
||||||
Platform include paths
|
|
||||||
^^^^^^^^^^^^^^^^^^^^^^
|
|
||||||
|
|
||||||
Platforms are allowed to add more include paths to be passed to the compiler.
|
|
||||||
The ``PLAT_INCLUDES`` variable is used for this purpose. This is needed in
|
|
||||||
particular for the file ``platform_def.h``.
|
|
||||||
|
|
||||||
Example:
|
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
PLAT_INCLUDES += -Iinclude/plat/myplat/include
|
|
||||||
|
|
||||||
Types and typedefs
|
|
||||||
------------------
|
|
||||||
|
|
||||||
Use of built-in *C* and *libc* data types
|
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
|
||||||
|
|
||||||
The TF codebase should be kept as portable as possible, especially since both
|
|
||||||
64-bit and 32-bit platforms are supported. To help with this, the following data
|
|
||||||
type usage guidelines should be followed:
|
|
||||||
|
|
||||||
- Where possible, use the built-in *C* data types for variable storage (for
|
|
||||||
example, ``char``, ``int``, ``long long``, etc) instead of the standard *C99*
|
|
||||||
types. Most code is typically only concerned with the minimum size of the
|
|
||||||
data stored, which the built-in *C* types guarantee.
|
|
||||||
|
|
||||||
- Avoid using the exact-size standard *C99* types in general (for example,
|
|
||||||
``uint16_t``, ``uint32_t``, ``uint64_t``, etc) since they can prevent the
|
|
||||||
compiler from making optimizations. There are legitimate uses for them,
|
|
||||||
for example to represent data of a known structure. When using them in struct
|
|
||||||
definitions, consider how padding in the struct will work across architectures.
|
|
||||||
For example, extra padding may be introduced in AArch32 systems if a struct
|
|
||||||
member crosses a 32-bit boundary.
|
|
||||||
|
|
||||||
- Use ``int`` as the default integer type - it's likely to be the fastest on all
|
|
||||||
systems. Also this can be assumed to be 32-bit as a consequence of the
|
|
||||||
`Procedure Call Standard for the Arm Architecture`_ and the `Procedure Call
|
|
||||||
Standard for the Arm 64-bit Architecture`_ .
|
|
||||||
|
|
||||||
- Avoid use of ``short`` as this may end up being slower than ``int`` in some
|
|
||||||
systems. If a variable must be exactly 16-bit, use ``int16_t`` or
|
|
||||||
``uint16_t``.
|
|
||||||
|
|
||||||
- Avoid use of ``long``. This is guaranteed to be at least 32-bit but, given
|
|
||||||
that `int` is 32-bit on Arm platforms, there is no use for it. For integers of
|
|
||||||
at least 64-bit, use ``long long``.
|
|
||||||
|
|
||||||
- Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data.
|
|
||||||
|
|
||||||
- Use ``unsigned`` for integers that can never be negative (counts,
|
|
||||||
indices, sizes, etc). TF intends to comply with MISRA "essential type" coding
|
|
||||||
rules (10.X), where signed and unsigned types are considered different
|
|
||||||
essential types. Choosing the correct type will aid this. MISRA static
|
|
||||||
analysers will pick up any implicit signed/unsigned conversions that may lead
|
|
||||||
to unexpected behaviour.
|
|
||||||
|
|
||||||
- For pointer types:
|
|
||||||
|
|
||||||
- If an argument in a function declaration is pointing to a known type then
|
|
||||||
simply use a pointer to that type (for example: ``struct my_struct *``).
|
|
||||||
|
|
||||||
- If a variable (including an argument in a function declaration) is pointing
|
|
||||||
to a general, memory-mapped address, an array of pointers or another
|
|
||||||
structure that is likely to require pointer arithmetic then use
|
|
||||||
``uintptr_t``. This will reduce the amount of casting required in the code.
|
|
||||||
Avoid using ``unsigned long`` or ``unsigned long long`` for this purpose; it
|
|
||||||
may work but is less portable.
|
|
||||||
|
|
||||||
- For other pointer arguments in a function declaration, use ``void *``. This
|
|
||||||
includes pointers to types that are abstracted away from the known API and
|
|
||||||
pointers to arbitrary data. This allows the calling function to pass a
|
|
||||||
pointer argument to the function without any explicit casting (the cast to
|
|
||||||
``void *`` is implicit). The function implementation can then do the
|
|
||||||
appropriate casting to a specific type.
|
|
||||||
|
|
||||||
- Use ``ptrdiff_t`` to compare the difference between 2 pointers.
|
|
||||||
|
|
||||||
- Use ``size_t`` when storing the ``sizeof()`` something.
|
|
||||||
|
|
||||||
- Use ``ssize_t`` when returning the ``sizeof()`` something from a function that
|
|
||||||
can also return an error code; the signed type allows for a negative return
|
|
||||||
code in case of error. This practice should be used sparingly.
|
|
||||||
|
|
||||||
- Use ``u_register_t`` when it's important to store the contents of a register
|
|
||||||
in its native size (32-bit in AArch32 and 64-bit in AArch64). This is not a
|
|
||||||
standard *C99* type but is widely available in libc implementations,
|
|
||||||
including the FreeBSD version included with the TF codebase. Where possible,
|
|
||||||
cast the variable to a more appropriate type before interpreting the data. For
|
|
||||||
example, the following struct in ``ep_info.h`` could use this type to minimize
|
|
||||||
the storage required for the set of registers:
|
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
typedef struct aapcs64_params {
|
|
||||||
u_register_t arg0;
|
|
||||||
u_register_t arg1;
|
|
||||||
u_register_t arg2;
|
|
||||||
u_register_t arg3;
|
|
||||||
u_register_t arg4;
|
|
||||||
u_register_t arg5;
|
|
||||||
u_register_t arg6;
|
|
||||||
u_register_t arg7;
|
|
||||||
} aapcs64_params_t;
|
|
||||||
|
|
||||||
If some code wants to operate on ``arg0`` and knows that it represents a 32-bit
|
|
||||||
unsigned integer on all systems, cast it to ``unsigned int``.
|
|
||||||
|
|
||||||
These guidelines should be updated if additional types are needed.
|
|
||||||
|
|
||||||
Avoid anonymous typedefs of structs/enums in headers
|
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
|
||||||
|
|
||||||
For example, the following definition:
|
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
typedef struct {
|
|
||||||
int arg1;
|
int arg1;
|
||||||
int arg2;
|
int arg2;
|
||||||
} my_struct_t;
|
};
|
||||||
|
|
||||||
|
void init(struct my_struct *ptr);
|
||||||
|
|
||||||
|
void main(void)
|
||||||
|
{
|
||||||
|
struct my_struct x;
|
||||||
|
x.arg1 = 1;
|
||||||
|
x.arg2 = 2;
|
||||||
|
init(&x);
|
||||||
|
}
|
||||||
|
|
||||||
is better written as:
|
is better written as:
|
||||||
|
|
||||||
|
@ -237,31 +131,18 @@ is better written as:
|
||||||
int arg2;
|
int arg2;
|
||||||
};
|
};
|
||||||
|
|
||||||
This allows function declarations in other header files that depend on the
|
void init(const struct my_struct *ptr);
|
||||||
struct/enum to forward declare the struct/enum instead of including the
|
|
||||||
entire header:
|
|
||||||
|
|
||||||
.. code:: c
|
void main(void)
|
||||||
|
{
|
||||||
|
const struct my_struct x = { 1, 2 };
|
||||||
|
init(&x);
|
||||||
|
}
|
||||||
|
|
||||||
#include <my_struct.h>
|
This allows the linker to put the data in a read-only data section instead of a
|
||||||
void my_func(my_struct_t *arg);
|
writeable data section, which may result in a smaller and faster binary. Note
|
||||||
|
that this may require dependent functions (``init()`` in the above example) to
|
||||||
instead of:
|
have ``const`` arguments, assuming they don't need to modify the data.
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
struct my_struct;
|
|
||||||
void my_func(struct my_struct *arg);
|
|
||||||
|
|
||||||
Some TF definitions use both a struct/enum name **and** a typedef name. This
|
|
||||||
is discouraged for new definitions as it makes it difficult for TF to comply
|
|
||||||
with MISRA rule 8.3, which states that "All declarations of an object or
|
|
||||||
function shall use the same names and type qualifiers".
|
|
||||||
|
|
||||||
The Linux coding standards also discourage new typedefs and checkpatch emits
|
|
||||||
a warning for this.
|
|
||||||
|
|
||||||
Existing typedefs will be retained for compatibility.
|
|
||||||
|
|
||||||
Libc functions that are banned or to be used with caution
|
Libc functions that are banned or to be used with caution
|
||||||
---------------------------------------------------------
|
---------------------------------------------------------
|
||||||
|
@ -410,14 +291,14 @@ error. This situation should be handled in one of the following ways:
|
||||||
then emit an ``ERROR`` message and call the platform-specific function
|
then emit an ``ERROR`` message and call the platform-specific function
|
||||||
``plat_error_handler()``.
|
``plat_error_handler()``.
|
||||||
|
|
||||||
Cases 1 and 2 are subtly different. A platform may implement ``plat_panic_handler``
|
Cases 1 and 2 are subtly different. A platform may implement
|
||||||
and ``plat_error_handler`` in the same way (for example, by waiting for a secure
|
``plat_panic_handler`` and ``plat_error_handler`` in the same way (for example,
|
||||||
watchdog to time-out or by invoking an interface on the platform's power
|
by waiting for a secure watchdog to time-out or by invoking an interface on the
|
||||||
controller to reset the platform). However, ``plat_error_handler`` may take
|
platform's power controller to reset the platform). However,
|
||||||
additional action for some errors (for example, it may set a flag so the
|
``plat_error_handler`` may take additional action for some errors (for example,
|
||||||
platform resets into a different mode). Also, ``plat_panic_handler()`` may
|
it may set a flag so the platform resets into a different mode). Also,
|
||||||
implement additional debug functionality (for example, invoking a hardware
|
``plat_panic_handler()`` may implement additional debug functionality (for
|
||||||
breakpoint).
|
example, invoking a hardware breakpoint).
|
||||||
|
|
||||||
Examples of unexpected unrecoverable errors:
|
Examples of unexpected unrecoverable errors:
|
||||||
|
|
||||||
|
@ -456,131 +337,115 @@ Examples:
|
||||||
- Secure world is waiting for a hardware response that is critical for continued
|
- Secure world is waiting for a hardware response that is critical for continued
|
||||||
operation.
|
operation.
|
||||||
|
|
||||||
Security considerations
|
Use of built-in *C* and *libc* data types
|
||||||
-----------------------
|
-----------------------------------------
|
||||||
|
|
||||||
Part of the security of a platform is handling errors correctly, as described in
|
The |TF-A| codebase should be kept as portable as possible, especially since
|
||||||
the previous section. There are several other security considerations covered in
|
both 64-bit and 32-bit platforms are supported. To help with this, the following
|
||||||
this section.
|
data type usage guidelines should be followed:
|
||||||
|
|
||||||
Do not leak secrets to the normal world
|
- Where possible, use the built-in *C* data types for variable storage (for
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
example, ``char``, ``int``, ``long long``, etc) instead of the standard *C99*
|
||||||
|
types. Most code is typically only concerned with the minimum size of the
|
||||||
|
data stored, which the built-in *C* types guarantee.
|
||||||
|
|
||||||
The secure world **must not** leak secrets to the normal world, for example in
|
- Avoid using the exact-size standard *C99* types in general (for example,
|
||||||
response to an SMC.
|
``uint16_t``, ``uint32_t``, ``uint64_t``, etc) since they can prevent the
|
||||||
|
compiler from making optimizations. There are legitimate uses for them,
|
||||||
|
for example to represent data of a known structure. When using them in struct
|
||||||
|
definitions, consider how padding in the struct will work across architectures.
|
||||||
|
For example, extra padding may be introduced in |AArch32| systems if a struct
|
||||||
|
member crosses a 32-bit boundary.
|
||||||
|
|
||||||
Handling Denial of Service attacks
|
- Use ``int`` as the default integer type - it's likely to be the fastest on all
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
systems. Also this can be assumed to be 32-bit as a consequence of the
|
||||||
|
`Procedure Call Standard for the Arm Architecture`_ and the `Procedure Call
|
||||||
|
Standard for the Arm 64-bit Architecture`_ .
|
||||||
|
|
||||||
The secure world **should never** crash or become unusable due to receiving too
|
- Avoid use of ``short`` as this may end up being slower than ``int`` in some
|
||||||
many normal world requests (a *Denial of Service* or *DoS* attack). It should
|
systems. If a variable must be exactly 16-bit, use ``int16_t`` or
|
||||||
have a mechanism for throttling or ignoring normal world requests.
|
``uint16_t``.
|
||||||
|
|
||||||
Performance considerations
|
- Avoid use of ``long``. This is guaranteed to be at least 32-bit but, given
|
||||||
--------------------------
|
that `int` is 32-bit on Arm platforms, there is no use for it. For integers of
|
||||||
|
at least 64-bit, use ``long long``.
|
||||||
|
|
||||||
Avoid printf and use logging macros
|
- Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data.
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
|
||||||
|
|
||||||
``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``)
|
- Use ``unsigned`` for integers that can never be negative (counts,
|
||||||
which wrap ``tf_log`` and which allow the logging call to be compiled-out
|
indices, sizes, etc). TF intends to comply with MISRA "essential type" coding
|
||||||
depending on the ``make`` command. Use these macros to avoid print statements
|
rules (10.X), where signed and unsigned types are considered different
|
||||||
being compiled unconditionally into the binary.
|
essential types. Choosing the correct type will aid this. MISRA static
|
||||||
|
analysers will pick up any implicit signed/unsigned conversions that may lead
|
||||||
|
to unexpected behaviour.
|
||||||
|
|
||||||
Each logging macro has a numerical log level:
|
- For pointer types:
|
||||||
|
|
||||||
|
- If an argument in a function declaration is pointing to a known type then
|
||||||
|
simply use a pointer to that type (for example: ``struct my_struct *``).
|
||||||
|
|
||||||
|
- If a variable (including an argument in a function declaration) is pointing
|
||||||
|
to a general, memory-mapped address, an array of pointers or another
|
||||||
|
structure that is likely to require pointer arithmetic then use
|
||||||
|
``uintptr_t``. This will reduce the amount of casting required in the code.
|
||||||
|
Avoid using ``unsigned long`` or ``unsigned long long`` for this purpose; it
|
||||||
|
may work but is less portable.
|
||||||
|
|
||||||
|
- For other pointer arguments in a function declaration, use ``void *``. This
|
||||||
|
includes pointers to types that are abstracted away from the known API and
|
||||||
|
pointers to arbitrary data. This allows the calling function to pass a
|
||||||
|
pointer argument to the function without any explicit casting (the cast to
|
||||||
|
``void *`` is implicit). The function implementation can then do the
|
||||||
|
appropriate casting to a specific type.
|
||||||
|
|
||||||
|
- Avoid pointer arithmetic generally (as this violates MISRA C 2012 rule
|
||||||
|
18.4) and especially on void pointers (as this is only supported via
|
||||||
|
language extensions and is considered non-standard). In TF-A, setting the
|
||||||
|
``W`` build flag to ``W=3`` enables the *-Wpointer-arith* compiler flag and
|
||||||
|
this will emit warnings where pointer arithmetic is used.
|
||||||
|
|
||||||
|
- Use ``ptrdiff_t`` to compare the difference between 2 pointers.
|
||||||
|
|
||||||
|
- Use ``size_t`` when storing the ``sizeof()`` something.
|
||||||
|
|
||||||
|
- Use ``ssize_t`` when returning the ``sizeof()`` something from a function that
|
||||||
|
can also return an error code; the signed type allows for a negative return
|
||||||
|
code in case of error. This practice should be used sparingly.
|
||||||
|
|
||||||
|
- Use ``u_register_t`` when it's important to store the contents of a register
|
||||||
|
in its native size (32-bit in |AArch32| and 64-bit in |AArch64|). This is not a
|
||||||
|
standard *C99* type but is widely available in libc implementations,
|
||||||
|
including the FreeBSD version included with the TF codebase. Where possible,
|
||||||
|
cast the variable to a more appropriate type before interpreting the data. For
|
||||||
|
example, the following struct in ``ep_info.h`` could use this type to minimize
|
||||||
|
the storage required for the set of registers:
|
||||||
|
|
||||||
.. code:: c
|
.. code:: c
|
||||||
|
|
||||||
#define LOG_LEVEL_NONE 0
|
typedef struct aapcs64_params {
|
||||||
#define LOG_LEVEL_ERROR 10
|
u_register_t arg0;
|
||||||
#define LOG_LEVEL_NOTICE 20
|
u_register_t arg1;
|
||||||
#define LOG_LEVEL_WARNING 30
|
u_register_t arg2;
|
||||||
#define LOG_LEVEL_INFO 40
|
u_register_t arg3;
|
||||||
#define LOG_LEVEL_VERBOSE 50
|
u_register_t arg4;
|
||||||
|
u_register_t arg5;
|
||||||
|
u_register_t arg6;
|
||||||
|
u_register_t arg7;
|
||||||
|
} aapcs64_params_t;
|
||||||
|
|
||||||
|
If some code wants to operate on ``arg0`` and knows that it represents a 32-bit
|
||||||
|
unsigned integer on all systems, cast it to ``unsigned int``.
|
||||||
|
|
||||||
By default, all logging statements with a log level ``<= LOG_LEVEL_INFO`` will
|
These guidelines should be updated if additional types are needed.
|
||||||
be compiled into debug builds and all statements with a log level
|
|
||||||
``<= LOG_LEVEL_NOTICE`` will be compiled into release builds. This can be
|
|
||||||
overridden from the command line or by the platform makefile (although it may be
|
|
||||||
necessary to clean the build directory first). For example, to enable
|
|
||||||
``VERBOSE`` logging on FVP:
|
|
||||||
|
|
||||||
``make PLAT=fvp LOG_LEVEL=50 all``
|
--------------
|
||||||
|
|
||||||
Use const data where possible
|
*Copyright (c) 2020, Arm Limited and Contributors. All rights reserved.*
|
||||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
|
||||||
|
|
||||||
For example, the following code:
|
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
struct my_struct {
|
|
||||||
int arg1;
|
|
||||||
int arg2;
|
|
||||||
};
|
|
||||||
|
|
||||||
void init(struct my_struct *ptr);
|
|
||||||
|
|
||||||
void main(void)
|
|
||||||
{
|
|
||||||
struct my_struct x;
|
|
||||||
x.arg1 = 1;
|
|
||||||
x.arg2 = 2;
|
|
||||||
init(&x);
|
|
||||||
}
|
|
||||||
|
|
||||||
is better written as:
|
|
||||||
|
|
||||||
.. code:: c
|
|
||||||
|
|
||||||
struct my_struct {
|
|
||||||
int arg1;
|
|
||||||
int arg2;
|
|
||||||
};
|
|
||||||
|
|
||||||
void init(const struct my_struct *ptr);
|
|
||||||
|
|
||||||
void main(void)
|
|
||||||
{
|
|
||||||
const struct my_struct x = { 1, 2 };
|
|
||||||
init(&x);
|
|
||||||
}
|
|
||||||
|
|
||||||
This allows the linker to put the data in a read-only data section instead of a
|
|
||||||
writeable data section, which may result in a smaller and faster binary. Note
|
|
||||||
that this may require dependent functions (``init()`` in the above example) to
|
|
||||||
have ``const`` arguments, assuming they don't need to modify the data.
|
|
||||||
|
|
||||||
Library and driver code
|
|
||||||
-----------------------
|
|
||||||
|
|
||||||
TF library code (under ``lib/`` and ``include/lib``) is any code that provides a
|
|
||||||
reusable interface to other code, potentially even to code outside of TF.
|
|
||||||
|
|
||||||
In some systems drivers must conform to a specific driver framework to provide
|
|
||||||
services to the rest of the system. TF has no driver framework and the
|
|
||||||
distinction between a driver and library is somewhat subjective.
|
|
||||||
|
|
||||||
A driver (under ``drivers/`` and ``include/drivers/``) is defined as code that
|
|
||||||
interfaces with hardware via a memory mapped interface.
|
|
||||||
|
|
||||||
Some drivers (for example, the Arm CCI driver in ``include/drivers/arm/cci.h``)
|
|
||||||
provide a general purpose API to that specific hardware. Other drivers (for
|
|
||||||
example, the Arm PL011 console driver in ``drivers/arm/pl011/pl011_console.S``)
|
|
||||||
provide a specific hardware implementation of a more abstract library API. In
|
|
||||||
the latter case there may potentially be multiple drivers for the same hardware
|
|
||||||
device.
|
|
||||||
|
|
||||||
Neither libraries nor drivers should depend on platform-specific code. If they
|
|
||||||
require platform-specific data (for example, a base address) to operate then
|
|
||||||
they should provide an initialization function that takes the platform-specific
|
|
||||||
data as arguments.
|
|
||||||
|
|
||||||
TF common code (under ``common/`` and ``include/common/``) is code that is re-used
|
|
||||||
by other generic (non-platform-specific) TF code. It is effectively internal
|
|
||||||
library code.
|
|
||||||
|
|
||||||
|
.. _`Linux master tree`: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/
|
||||||
|
.. _`Procedure Call Standard for the Arm Architecture`: https://developer.arm.com/docs/ihi0042/latest/
|
||||||
|
.. _`Procedure Call Standard for the Arm 64-bit Architecture`: https://developer.arm.com/docs/ihi0055/latest/
|
||||||
|
.. _`EditorConfig`: http://editorconfig.org/
|
||||||
.. _`Why the “volatile” type class should not be used`: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html
|
.. _`Why the “volatile” type class should not be used`: https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html
|
||||||
.. _`Procedure Call Standard for the Arm Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf
|
.. _`MISRA C:2012 Guidelines`: https://www.misra.org.uk/Activities/MISRAC/tabid/160/Default.aspx
|
||||||
.. _`Procedure Call Standard for the Arm 64-bit Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
|
.. _`a spreadsheet`: https://developer.trustedfirmware.org/file/download/lamajxif3w7c4mpjeoo5/PHID-FILE-fp7c7acszn6vliqomyhn/MISRA-and-TF-Analysis-v1.3.ods
|
||||||
|
|
|
@ -0,0 +1,468 @@
|
||||||
|
Coding Style
|
||||||
|
============
|
||||||
|
|
||||||
|
The following sections outline the |TF-A| coding style for *C* code. The style
|
||||||
|
is based on the `Linux kernel coding style`_, with a few modifications.
|
||||||
|
|
||||||
|
The style should not be considered *set in stone*. Feel free to provide feedback
|
||||||
|
and suggestions.
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
You will almost certainly find code in the |TF-A| repository that does not
|
||||||
|
follow the style. The intent is for all code to do so eventually.
|
||||||
|
|
||||||
|
File Encoding
|
||||||
|
-------------
|
||||||
|
|
||||||
|
The source code must use the **UTF-8** character encoding. Comments and
|
||||||
|
documentation may use non-ASCII characters when required (e.g. Greek letters
|
||||||
|
used for units) but code itself is still limited to ASCII characters.
|
||||||
|
|
||||||
|
Newlines must be in **Unix** style, which means that only the Line Feed (``LF``)
|
||||||
|
character is used to break a line and reset to the first column.
|
||||||
|
|
||||||
|
Language
|
||||||
|
--------
|
||||||
|
|
||||||
|
The primary language for comments and naming must be International English. In
|
||||||
|
cases where there is a conflict between the American English and British English
|
||||||
|
spellings of a word, the American English spelling is used.
|
||||||
|
|
||||||
|
Exceptions are made when referring directly to something that does not use
|
||||||
|
international style, such as the name of a company. In these cases the existing
|
||||||
|
name should be used as-is.
|
||||||
|
|
||||||
|
C Language Standard
|
||||||
|
-------------------
|
||||||
|
|
||||||
|
The C language mode used for TF-A is *GNU99*. This is the "GNU dialect of ISO
|
||||||
|
C99", which implies the *ISO C99* standard with GNU extensions.
|
||||||
|
|
||||||
|
Both GCC and Clang compiler toolchains have support for *GNU99* mode, though
|
||||||
|
Clang does lack support for a small number of GNU extensions. These
|
||||||
|
missing extensions are rarely used, however, and should not pose a problem.
|
||||||
|
|
||||||
|
MISRA Compliance
|
||||||
|
----------------
|
||||||
|
|
||||||
|
TF-A attempts to comply with the `MISRA C:2012 Guidelines`_. Coverity
|
||||||
|
Static Analysis is used to regularly generate a report of current MISRA defects
|
||||||
|
and to prevent the addition of new ones.
|
||||||
|
|
||||||
|
It is not possible for the project to follow all MISRA guidelines. We maintain
|
||||||
|
`a spreadsheet`_ that lists all rules and directives and whether we aim to
|
||||||
|
comply with them or not. A rationale is given for each deviation.
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
Enforcing a rule does not mean that the codebase is free of defects
|
||||||
|
of that rule, only that they would ideally be removed.
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
Third-party libraries are not considered in our MISRA analysis and we do not
|
||||||
|
intend to modify them to make them MISRA compliant.
|
||||||
|
|
||||||
|
Indentation
|
||||||
|
-----------
|
||||||
|
|
||||||
|
Use **tabs** for indentation. The use of spaces for indentation is forbidden
|
||||||
|
except in the case where a term is being indented to a boundary that cannot be
|
||||||
|
achieved using tabs alone.
|
||||||
|
|
||||||
|
Tab spacing should be set to **8 characters**.
|
||||||
|
|
||||||
|
Trailing whitespace is not allowed and must be trimmed.
|
||||||
|
|
||||||
|
Spacing
|
||||||
|
-------
|
||||||
|
|
||||||
|
Single spacing should be used around most operators, including:
|
||||||
|
|
||||||
|
- Arithmetic operators (``+``, ``-``, ``/``, ``*``)
|
||||||
|
- Assignment operators (``=``, ``+=``, etc)
|
||||||
|
- Boolean operators (``&&``, ``||``)
|
||||||
|
- Comparison operators (``<``, ``>``, ``==``, etc)
|
||||||
|
|
||||||
|
A space should also be used to separate parentheses and braces when they are not
|
||||||
|
already separated by a newline, such as for the ``if`` statement in the
|
||||||
|
following example:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
int function_foo(bool bar)
|
||||||
|
{
|
||||||
|
if (bar) {
|
||||||
|
function_baz();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Note that there is no space between the name of a function and the following
|
||||||
|
parentheses.
|
||||||
|
|
||||||
|
Control statements (``if``, ``for``, ``switch``, ``while``, etc) must be
|
||||||
|
separated from the following open paranthesis by a single space. The previous
|
||||||
|
example illustrates this for an ``if`` statement.
|
||||||
|
|
||||||
|
Line Length
|
||||||
|
-----------
|
||||||
|
|
||||||
|
Line length *should* be at most **80 characters**. This limit does not include
|
||||||
|
non-printing characters such as the line feed.
|
||||||
|
|
||||||
|
This rule is a *should*, not a must, and it is acceptable to exceed the limit
|
||||||
|
**slightly** where the readability of the code would otherwise be significantly
|
||||||
|
reduced. Use your judgement in these cases.
|
||||||
|
|
||||||
|
Blank Lines
|
||||||
|
-----------
|
||||||
|
|
||||||
|
Functions are usually separated by a single blank line. In certain cases it is
|
||||||
|
acceptable to use additional blank lines for clarity, if required.
|
||||||
|
|
||||||
|
The file must end with a single newline character. Many editors have the option
|
||||||
|
to insert this automatically and to trim multiple blank lines at the end of the
|
||||||
|
file.
|
||||||
|
|
||||||
|
Braces
|
||||||
|
------
|
||||||
|
|
||||||
|
Opening Brace Placement
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
Braces follow the **Kernighan and Ritchie (K&R)** style, where the opening brace
|
||||||
|
is **not** placed on a new line.
|
||||||
|
|
||||||
|
Example for a ``while`` loop:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
while (condition) {
|
||||||
|
foo();
|
||||||
|
bar();
|
||||||
|
}
|
||||||
|
|
||||||
|
This style applies to all blocks except for functions which, following the Linux
|
||||||
|
style, **do** place the opening brace on a new line.
|
||||||
|
|
||||||
|
Example for a function:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
int my_function(void)
|
||||||
|
{
|
||||||
|
int a;
|
||||||
|
|
||||||
|
a = 1;
|
||||||
|
return a;
|
||||||
|
}
|
||||||
|
|
||||||
|
Conditional Statement Bodies
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
Where conditional statements (such as ``if``, ``for``, ``while`` and ``do``) are
|
||||||
|
used, braces must be placed around the statements that form the body of the
|
||||||
|
conditional. This is the case regardless of the number of statements in the
|
||||||
|
body.
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
This is a notable departure from the Linux coding style that has been
|
||||||
|
adopted to follow MISRA guidelines more closely and to help prevent errors.
|
||||||
|
|
||||||
|
For example, use the following style:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
if (condition) {
|
||||||
|
foo++;
|
||||||
|
}
|
||||||
|
|
||||||
|
instead of omitting the optional braces around a single statement:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
/* This is violating MISRA C 2012: Rule 15.6 */
|
||||||
|
if (condition)
|
||||||
|
foo++;
|
||||||
|
|
||||||
|
The reason for this is to prevent accidental changes to control flow when
|
||||||
|
modifying the body of the conditional. For example, at a quick glance it is easy
|
||||||
|
to think that the value of ``bar`` is only incremented if ``condition``
|
||||||
|
evaluates to ``true`` but this is not the case - ``bar`` will always be
|
||||||
|
incremented regardless of the condition evaluation. If the developer forgets to
|
||||||
|
add braces around the conditional body when adding the ``bar++;`` statement then
|
||||||
|
the program execution will not proceed as intended.
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
/* This is violating MISRA C 2012: Rule 15.6 */
|
||||||
|
if (condition)
|
||||||
|
foo++;
|
||||||
|
bar++;
|
||||||
|
|
||||||
|
Naming
|
||||||
|
------
|
||||||
|
|
||||||
|
Functions
|
||||||
|
^^^^^^^^^
|
||||||
|
|
||||||
|
Use lowercase for function names, separating multiple words with an underscore
|
||||||
|
character (``_``). This is sometimes referred to as *Snake Case*. An example is
|
||||||
|
given below:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
void bl2_arch_setup(void)
|
||||||
|
{
|
||||||
|
...
|
||||||
|
}
|
||||||
|
|
||||||
|
Local Variables and Parameters
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
Local variables and function parameters use the same format as function names:
|
||||||
|
lowercase with underscore separation between multiple words. An example is
|
||||||
|
given below:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
static void set_scr_el3_from_rm(uint32_t type,
|
||||||
|
uint32_t interrupt_type_flags,
|
||||||
|
uint32_t security_state)
|
||||||
|
{
|
||||||
|
uint32_t flag, bit_pos;
|
||||||
|
|
||||||
|
...
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
Preprocessor Macros
|
||||||
|
^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
Identifiers that are defined using preprocessor macros are written in all
|
||||||
|
uppercase text.
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
#define BUFFER_SIZE_BYTES 64
|
||||||
|
|
||||||
|
Function Attributes
|
||||||
|
-------------------
|
||||||
|
|
||||||
|
Place any function attributes after the function type and before the function
|
||||||
|
name.
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
void __init plat_arm_interconnect_init(void);
|
||||||
|
|
||||||
|
Alignment
|
||||||
|
---------
|
||||||
|
|
||||||
|
Alignment should be performed primarily with tabs, adding spaces if required to
|
||||||
|
achieve a granularity that is smaller than the tab size. For example, with a tab
|
||||||
|
size of eight columns it would be necessary to use one tab character and two
|
||||||
|
spaces to indent text by ten columns.
|
||||||
|
|
||||||
|
Switch Statement Alignment
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
When using ``switch`` statements, align each ``case`` statement with the
|
||||||
|
``switch`` so that they are in the same column.
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
switch (condition) {
|
||||||
|
case A:
|
||||||
|
foo();
|
||||||
|
case B:
|
||||||
|
bar();
|
||||||
|
default:
|
||||||
|
baz();
|
||||||
|
}
|
||||||
|
|
||||||
|
Pointer Alignment
|
||||||
|
^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
The reference and dereference operators (ampersand and *pointer star*) must be
|
||||||
|
aligned with the name of the object on which they are operating, as opposed to
|
||||||
|
the type of the object.
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
uint8_t *foo;
|
||||||
|
|
||||||
|
foo = &bar;
|
||||||
|
|
||||||
|
|
||||||
|
Comments
|
||||||
|
--------
|
||||||
|
|
||||||
|
The general rule for comments is that the double-slash style of comment (``//``)
|
||||||
|
is not allowed. Examples of the allowed comment formats are shown below:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
/*
|
||||||
|
* This example illustrates the first allowed style for multi-line comments.
|
||||||
|
*
|
||||||
|
* Blank lines within multi-lines are allowed when they add clarity or when
|
||||||
|
* they separate multiple contexts.
|
||||||
|
*
|
||||||
|
*/
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
/**************************************************************************
|
||||||
|
* This is the second allowed style for multi-line comments.
|
||||||
|
*
|
||||||
|
* In this style, the first and last lines use asterisks that run the full
|
||||||
|
* width of the comment at its widest point.
|
||||||
|
*
|
||||||
|
* This style can be used for additional emphasis.
|
||||||
|
*
|
||||||
|
*************************************************************************/
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
/* Single line comments can use this format */
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
/***************************************************************************
|
||||||
|
* This alternative single-line comment style can also be used for emphasis.
|
||||||
|
**************************************************************************/
|
||||||
|
|
||||||
|
Headers and inclusion
|
||||||
|
---------------------
|
||||||
|
|
||||||
|
Header guards
|
||||||
|
^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
For a header file called "some_driver.h" the style used by |TF-A| is:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
#ifndef SOME_DRIVER_H
|
||||||
|
#define SOME_DRIVER_H
|
||||||
|
|
||||||
|
<header content>
|
||||||
|
|
||||||
|
#endif /* SOME_DRIVER_H */
|
||||||
|
|
||||||
|
Include statement ordering
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
All header files that are included by a source file must use the following,
|
||||||
|
grouped ordering. This is to improve readability (by making it easier to quickly
|
||||||
|
read through the list of headers) and maintainability.
|
||||||
|
|
||||||
|
#. *System* includes: Header files from the standard *C* library, such as
|
||||||
|
``stddef.h`` and ``string.h``.
|
||||||
|
|
||||||
|
#. *Project* includes: Header files under the ``include/`` directory within
|
||||||
|
|TF-A| are *project* includes.
|
||||||
|
|
||||||
|
#. *Platform* includes: Header files relating to a single, specific platform,
|
||||||
|
and which are located under the ``plat/<platform_name>`` directory within
|
||||||
|
|TF-A|, are *platform* includes.
|
||||||
|
|
||||||
|
Within each group, ``#include`` statements must be in alphabetical order,
|
||||||
|
taking both the file and directory names into account.
|
||||||
|
|
||||||
|
Groups must be separated by a single blank line for clarity.
|
||||||
|
|
||||||
|
The example below illustrates the ordering rules using some contrived header
|
||||||
|
file names; this type of name reuse should be otherwise avoided.
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
#include <string.h>
|
||||||
|
|
||||||
|
#include <a_dir/example/a_header.h>
|
||||||
|
#include <a_dir/example/b_header.h>
|
||||||
|
#include <a_dir/test/a_header.h>
|
||||||
|
#include <b_dir/example/a_header.h>
|
||||||
|
|
||||||
|
#include "a_header.h"
|
||||||
|
|
||||||
|
Include statement variants
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
Two variants of the ``#include`` directive are acceptable in the |TF-A|
|
||||||
|
codebase. Correct use of the two styles improves readability by suggesting the
|
||||||
|
location of the included header and reducing ambiguity in cases where generic
|
||||||
|
and platform-specific headers share a name.
|
||||||
|
|
||||||
|
For header files that are in the same directory as the source file that is
|
||||||
|
including them, use the ``"..."`` variant.
|
||||||
|
|
||||||
|
For header files that are **not** in the same directory as the source file that
|
||||||
|
is including them, use the ``<...>`` variant.
|
||||||
|
|
||||||
|
Example (bl1_fwu.c):
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
#include <assert.h>
|
||||||
|
#include <errno.h>
|
||||||
|
#include <string.h>
|
||||||
|
|
||||||
|
#include "bl1_private.h"
|
||||||
|
|
||||||
|
Typedefs
|
||||||
|
--------
|
||||||
|
|
||||||
|
Avoid anonymous typedefs of structs/enums in headers
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
For example, the following definition:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
typedef struct {
|
||||||
|
int arg1;
|
||||||
|
int arg2;
|
||||||
|
} my_struct_t;
|
||||||
|
|
||||||
|
|
||||||
|
is better written as:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
struct my_struct {
|
||||||
|
int arg1;
|
||||||
|
int arg2;
|
||||||
|
};
|
||||||
|
|
||||||
|
This allows function declarations in other header files that depend on the
|
||||||
|
struct/enum to forward declare the struct/enum instead of including the
|
||||||
|
entire header:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
struct my_struct;
|
||||||
|
void my_func(struct my_struct *arg);
|
||||||
|
|
||||||
|
instead of:
|
||||||
|
|
||||||
|
.. code:: c
|
||||||
|
|
||||||
|
#include <my_struct.h>
|
||||||
|
void my_func(my_struct_t *arg);
|
||||||
|
|
||||||
|
Some TF definitions use both a struct/enum name **and** a typedef name. This
|
||||||
|
is discouraged for new definitions as it makes it difficult for TF to comply
|
||||||
|
with MISRA rule 8.3, which states that "All declarations of an object or
|
||||||
|
function shall use the same names and type qualifiers".
|
||||||
|
|
||||||
|
The Linux coding standards also discourage new typedefs and checkpatch emits
|
||||||
|
a warning for this.
|
||||||
|
|
||||||
|
Existing typedefs will be retained for compatibility.
|
||||||
|
|
||||||
|
--------------
|
||||||
|
|
||||||
|
*Copyright (c) 2020, Arm Limited. All rights reserved.*
|
||||||
|
|
||||||
|
.. _`Linux kernel coding style`: https://www.kernel.org/doc/html/latest/process/coding-style.html
|
||||||
|
.. _`MISRA C:2012 Guidelines`: https://www.misra.org.uk/Activities/MISRAC/tabid/160/Default.aspx
|
||||||
|
.. _`a spreadsheet`: https://developer.trustedfirmware.org/file/download/lamajxif3w7c4mpjeoo5/PHID-FILE-fp7c7acszn6vliqomyhn/MISRA-and-TF-Analysis-v1.3.ods
|
|
@ -23,7 +23,7 @@ Making Changes
|
||||||
|
|
||||||
- Make commits of logical units. See these general `Git guidelines`_ for
|
- Make commits of logical units. See these general `Git guidelines`_ for
|
||||||
contributing to a project.
|
contributing to a project.
|
||||||
- Follow the :ref:`Coding Style & Guidelines`.
|
- Follow the :ref:`Coding Style` and :ref:`Coding Guidelines`.
|
||||||
|
|
||||||
- Use the checkpatch.pl script provided with the Linux source tree. A
|
- Use the checkpatch.pl script provided with the Linux source tree. A
|
||||||
Makefile target is provided for convenience.
|
Makefile target is provided for convenience.
|
||||||
|
@ -128,7 +128,7 @@ Binary Components
|
||||||
|
|
||||||
--------------
|
--------------
|
||||||
|
|
||||||
*Copyright (c) 2013-2019, Arm Limited and Contributors. All rights reserved.*
|
*Copyright (c) 2013-2020, Arm Limited and Contributors. All rights reserved.*
|
||||||
|
|
||||||
.. _developer.trustedfirmware.org: https://developer.trustedfirmware.org
|
.. _developer.trustedfirmware.org: https://developer.trustedfirmware.org
|
||||||
.. _issue: https://developer.trustedfirmware.org/project/board/1/
|
.. _issue: https://developer.trustedfirmware.org/project/board/1/
|
||||||
|
|
|
@ -8,6 +8,7 @@ Processes & Policies
|
||||||
|
|
||||||
security
|
security
|
||||||
platform-compatibility-policy
|
platform-compatibility-policy
|
||||||
|
coding-style
|
||||||
coding-guidelines
|
coding-guidelines
|
||||||
contributing
|
contributing
|
||||||
faq
|
faq
|
||||||
|
|
|
@ -1,10 +1,30 @@
|
||||||
Security hardening
|
Secure Development Guidelines
|
||||||
==================
|
=============================
|
||||||
|
|
||||||
This page contains guidance on what to check for additional security measures,
|
This page contains guidance on what to check for additional security measures,
|
||||||
including build options that can be modified to improve security or catch issues
|
including build options that can be modified to improve security or catch issues
|
||||||
early in development.
|
early in development.
|
||||||
|
|
||||||
|
Security considerations
|
||||||
|
-----------------------
|
||||||
|
|
||||||
|
Part of the security of a platform is handling errors correctly, as described in
|
||||||
|
the previous section. There are several other security considerations covered in
|
||||||
|
this section.
|
||||||
|
|
||||||
|
Do not leak secrets to the normal world
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
The secure world **must not** leak secrets to the normal world, for example in
|
||||||
|
response to an SMC.
|
||||||
|
|
||||||
|
Handling Denial of Service attacks
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
|
||||||
|
The secure world **should never** crash or become unusable due to receiving too
|
||||||
|
many normal world requests (a *Denial of Service* or *DoS* attack). It should
|
||||||
|
have a mechanism for throttling or ignoring normal world requests.
|
||||||
|
|
||||||
Build options
|
Build options
|
||||||
-------------
|
-------------
|
||||||
|
|
||||||
|
@ -53,4 +73,4 @@ Several build options can be used to check for security issues. Refer to the
|
||||||
|
|
||||||
--------------
|
--------------
|
||||||
|
|
||||||
*Copyright (c) 2019, Arm Limited. All rights reserved.*
|
*Copyright (c) 2019-2020, Arm Limited. All rights reserved.*
|
||||||
|
|
Loading…
Reference in New Issue