Merge "doc: Split and expand coding style documentation" into integration
diff --git a/docs/getting_started/porting-guide.rst b/docs/getting_started/porting-guide.rst
index bb14717..e8357b3 100644
--- a/docs/getting_started/porting-guide.rst
+++ b/docs/getting_started/porting-guide.rst
@@ -2763,6 +2763,19 @@
to ``no``. If any of the options ``EL3_PAYLOAD_BASE`` or ``PRELOADED_BL33_BASE``
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
---------
@@ -2844,7 +2857,7 @@
--------------
-*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
.. _Arm Generic Interrupt Controller version 2.0 (GICv2): http://infocenter.arm.com/help/topic/com.arm.doc.ihi0048b/index.html
diff --git a/docs/process/coding-guidelines.rst b/docs/process/coding-guidelines.rst
index cb8b892..f7d53a9 100644
--- a/docs/process/coding-guidelines.rst
+++ b/docs/process/coding-guidelines.rst
@@ -1,232 +1,126 @@
-Coding Style & Guidelines
-=========================
+Coding Guidelines
+=================
-The following sections contain TF coding guidelines. They are continually
-evolving and should not be considered "set in stone". Feel free to question them
-and provide feedback.
+This document provides some additional guidelines to consider when writing
+|TF-A| code. These are not intended to be strictly-enforced rules like the
+contents of the :ref:`Coding Style`.
-Some of the guidelines may also apply to other codebases.
+Automatic Editor Configuration
+------------------------------
-.. note::
- The existing TF codebase does not necessarily comply with all the
- below guidelines but the intent is for it to do so eventually.
-
-Checkpatch overrides
---------------------
-
-Some checkpatch warnings in the TF codebase are deliberately ignored. These
-include:
-
-- ``**WARNING: line over 80 characters**``: Although the codebase should
- generally conform to the 80 character limit this is overly restrictive in some
- cases.
-
-- ``**WARNING: Use of volatile is usually wrong``: see
- `Why the “volatile” type class should not be used`_ . Although this document
- contains some very useful information, there are several legitimate uses of
- the volatile keyword within the TF codebase.
-
-Headers and inclusion
----------------------
-
-Header guards
-^^^^^^^^^^^^^
-
-For a header file called "some_driver.h" the style used by the Trusted Firmware
-is:
-
-.. code:: c
-
- #ifndef SOME_DRIVER_H
- #define SOME_DRIVER_H
-
- <header content>
-
- #endif /* SOME_DRIVER_H */
+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.
-Include statement ordering
-^^^^^^^^^^^^^^^^^^^^^^^^^^
+Several editors include built-in support for EditorConfig files, and many others
+support its functionality through plugins.
-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.
+Use of the EditorConfig file is suggested but is not required.
-#. *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
-
- #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 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
+Automatic Compliance Checking
+-----------------------------
-Types and typedefs
-------------------
+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.
-Use of built-in *C* and *libc* data types
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+.. note::
+ Checkpatch errors will gate upstream merging of pull requests.
+ Checkpatch warnings will not gate merging but should be reviewed and fixed if
+ possible.
-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:
+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:
-- 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.
+.. code:: shell
-- 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`_ .
+ make CHECKPATCH=<path-to-linux>/linux/scripts/checkpatch.pl checkcodebase
-- 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``.
+To just check the style on the files that differ between your local branch and
+the remote master, use:
-- 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``.
+.. code:: shell
-- Use ``char`` for storing text. Use ``uint8_t`` for storing other 8-bit data.
+ make CHECKPATCH=<path-to-linux>/linux/scripts/checkpatch.pl checkpatch
-- 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.
+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``.
-- For pointer types:
+Ignored Checkpatch Warnings
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
- - 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 *``).
+Some checkpatch warnings in the TF codebase are deliberately ignored. These
+include:
- - 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.
+- ``**WARNING: line over 80 characters**``: Although the codebase should
+ generally conform to the 80 character limit this is overly restrictive in some
+ cases.
- - 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.
+- ``**WARNING: Use of volatile is usually wrong``: see
+ `Why the “volatile” type class should not be used`_ . Although this document
+ contains some very useful information, there are several legimate uses of the
+ volatile keyword within the TF codebase.
- - Use ``ptrdiff_t`` to compare the difference between 2 pointers.
+Performance considerations
+--------------------------
-- Use ``size_t`` when storing the ``sizeof()`` something.
+Avoid printf and use logging macros
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-- 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.
+``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``)
+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.
-- 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:
+Each logging macro has a numerical log level:
.. 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;
+ #define LOG_LEVEL_NONE 0
+ #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
-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
+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:
-These guidelines should be updated if additional types are needed.
+``make PLAT=fvp LOG_LEVEL=50 all``
-Avoid anonymous typedefs of structs/enums in headers
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Use const data where possible
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-For example, the following definition:
+For example, the following code:
.. code:: c
- typedef struct {
+ struct my_struct {
int arg1;
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:
@@ -237,31 +131,18 @@
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
-
- #include <my_struct.h>
- void my_func(my_struct_t *arg);
-
-instead of:
-
-.. 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".
+ void init(const struct my_struct *ptr);
-The Linux coding standards also discourage new typedefs and checkpatch emits
-a warning for this.
+ void main(void)
+ {
+ const struct my_struct x = { 1, 2 };
+ init(&x);
+ }
-Existing typedefs will be retained for compatibility.
+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.
Libc functions that are banned or to be used with caution
---------------------------------------------------------
@@ -410,14 +291,14 @@
then emit an ``ERROR`` message and call the platform-specific function
``plat_error_handler()``.
-Cases 1 and 2 are subtly different. A platform may implement ``plat_panic_handler``
-and ``plat_error_handler`` in the same way (for example, by waiting for a secure
-watchdog to time-out or by invoking an interface on the platform's power
-controller to reset the platform). However, ``plat_error_handler`` may take
-additional action for some errors (for example, it may set a flag so the
-platform resets into a different mode). Also, ``plat_panic_handler()`` may
-implement additional debug functionality (for example, invoking a hardware
-breakpoint).
+Cases 1 and 2 are subtly different. A platform may implement
+``plat_panic_handler`` and ``plat_error_handler`` in the same way (for example,
+by waiting for a secure watchdog to time-out or by invoking an interface on the
+platform's power controller to reset the platform). However,
+``plat_error_handler`` may take additional action for some errors (for example,
+it may set a flag so the platform resets into a different mode). Also,
+``plat_panic_handler()`` may implement additional debug functionality (for
+example, invoking a hardware breakpoint).
Examples of unexpected unrecoverable errors:
@@ -456,131 +337,115 @@
- Secure world is waiting for a hardware response that is critical for continued
operation.
-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
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Use of built-in *C* and *libc* data types
+-----------------------------------------
-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.
+The |TF-A| 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:
-Performance considerations
---------------------------
+- 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 printf and use logging macros
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- 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.
-``debug.h`` provides logging macros (for example, ``WARN`` and ``ERROR``)
-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.
+- 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`_ .
-Each logging macro has a numerical log level:
+- 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``.
-.. code:: c
+- 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``.
- #define LOG_LEVEL_NONE 0
- #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
+- 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.
-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:
+- For pointer types:
-``make PLAT=fvp LOG_LEVEL=50 all``
+ - 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 *``).
-Use const data where possible
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ - 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 example, the following code:
+ - 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.
-.. code:: c
+ - 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.
- struct my_struct {
- int arg1;
- int arg2;
- };
+ - Use ``ptrdiff_t`` to compare the difference between 2 pointers.
- void init(struct my_struct *ptr);
+- Use ``size_t`` when storing the ``sizeof()`` something.
- void main(void)
- {
- struct my_struct x;
- x.arg1 = 1;
- x.arg2 = 2;
- init(&x);
- }
+- 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.
-is better written as:
+- 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
- 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.
+ 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;
-A driver (under ``drivers/`` and ``include/drivers/``) is defined as code that
-interfaces with hardware via a memory mapped interface.
+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``.
-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.
+These guidelines should be updated if additional types are needed.
-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.
+*Copyright (c) 2020, Arm Limited and Contributors. All rights reserved.*
+.. _`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
-.. _`Procedure Call Standard for the Arm Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf
-.. _`Procedure Call Standard for the Arm 64-bit Architecture`: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
+.. _`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
diff --git a/docs/process/coding-style.rst b/docs/process/coding-style.rst
new file mode 100644
index 0000000..fd1984d
--- /dev/null
+++ b/docs/process/coding-style.rst
@@ -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
diff --git a/docs/process/contributing.rst b/docs/process/contributing.rst
index f569fcb..68c494b 100644
--- a/docs/process/contributing.rst
+++ b/docs/process/contributing.rst
@@ -23,7 +23,7 @@
- Make commits of logical units. See these general `Git guidelines`_ for
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
Makefile target is provided for convenience.
@@ -128,7 +128,7 @@
--------------
-*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
.. _issue: https://developer.trustedfirmware.org/project/board/1/
diff --git a/docs/process/index.rst b/docs/process/index.rst
index 9c12de8..1cb5354 100644
--- a/docs/process/index.rst
+++ b/docs/process/index.rst
@@ -8,6 +8,7 @@
security
platform-compatibility-policy
+ coding-style
coding-guidelines
contributing
faq
diff --git a/docs/process/security-hardening.rst b/docs/process/security-hardening.rst
index a18a792..43a5721 100644
--- a/docs/process/security-hardening.rst
+++ b/docs/process/security-hardening.rst
@@ -1,10 +1,30 @@
-Security hardening
-==================
+Secure Development Guidelines
+=============================
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
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
-------------
@@ -53,4 +73,4 @@
--------------
-*Copyright (c) 2019, Arm Limited. All rights reserved.*
+*Copyright (c) 2019-2020, Arm Limited. All rights reserved.*