The strings in usage functions are sent to stdout by printf(),
but some new lines are sent to stderr by fputc(..., stderr).
This inconsistency will break the usage format when users re-direct
stdout or stderr. Just use printf() for consistency.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
These variables store return values of functions. Remove all of
meaningless initializers.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
The ext_new_nvcounter() function calls i2d_ASN1_INTEGER() twice;
the first call to get the return value "sz", and the second one
for writing data into the buffer. This is actually redundant.
We can do both by one function call.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
In the current code, both key_load() and key_create() call key_new()
to allocate a key container (and they do not free it even if they
fail). If a specific key is not given by the command option,
key_load() fails, then key_create() is called. At this point, the
key container that has been allocated in key_load() is still alive,
and it is overwritten by a new key container created by key_create().
Move the key_new() call to the main() function to make sure it is
called just once for each descriptor.
While we are here, let's fix one more bug; the error handling code
ERROR("Malloc error while loading '%s'\n", keys[i].fn);
is wrong because keys[i].fn is NULL pointer unless a specific key is
given by the command option. This code could be run in either case.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
AEMv8-A Model release v8.2 has been made available and Trusted Firmware
has been tested against these versions as part of its CI system. This
patch updates the user guide documentation to reflect the version of AEM
and Cortex Models that Trusted Firmware has been tested against.
Also, the Linaro Release Notes link was broken and this patch updates the
link.
Change-Id: I88729cef909a69fff629036f480fd6168ad7dc9a
Signed-off-by: David Cunado <david.cunado@arm.com>
Replace all use of memset by zeromem when zeroing moderately-sized
structure by applying the following transformation:
memset(x, 0, sizeof(x)) => zeromem(x, sizeof(x))
As the Trusted Firmware is compiled with -ffreestanding, it forbids the
compiler from using __builtin_memset and forces it to generate calls to
the slow memset implementation. Zeromem is a near drop in replacement
for this use case, with a more efficient implementation on both AArch32
and AArch64.
Change-Id: Ia7f3a90e888b96d056881be09f0b4d65b41aa79e
Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>
Introduce zeromem_dczva function on AArch64 that can handle unaligned
addresses and make use of DC ZVA instruction to zero a whole block at a
time. This zeroing takes place directly in the cache to speed it up
without doing external memory access.
Remove the zeromem16 function on AArch64 and replace it with an alias to
zeromem. This zeromem16 function is now deprecated.
Remove the 16-bytes alignment constraint on __BSS_START__ in
firmware-design.md as it is now not mandatory anymore (it used to comply
with zeromem16 requirements).
Change the 16-bytes alignment constraints in SP min's linker script to a
8-bytes alignment constraint as the AArch32 zeromem implementation is now
more efficient on 8-bytes aligned addresses.
Introduce zero_normalmem and zeromem helpers in platform agnostic header
that are implemented this way:
* AArch32:
* zero_normalmem: zero using usual data access
* zeromem: alias for zero_normalmem
* AArch64:
* zero_normalmem: zero normal memory using DC ZVA instruction
(needs MMU enabled)
* zeromem: zero using usual data access
Usage guidelines: in most cases, zero_normalmem should be preferred.
There are 2 scenarios where zeromem (or memset) must be used instead:
* Code that must run with MMU disabled (which means all memory is
considered device memory for data accesses).
* Code that fills device memory with null bytes.
Optionally, the following rule can be applied if performance is
important:
* Code zeroing small areas (few bytes) that are not secrets should use
memset to take advantage of compiler optimizations.
Note: Code zeroing security-related critical information should use
zero_normalmem/zeromem instead of memset to avoid removal by
compilers' optimizations in some cases or misbehaving versions of GCC.
FixesARM-software/tf-issues#408
Change-Id: Iafd9663fc1070413c3e1904e54091cf60effaa82
Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>
RESET_TO_BL31=1 is specified by plat/xilinx/zynqmp/platform.mk with
"override" directive. So, RESET_TO_BL31=1 is guaranteed without any
operation on users' side.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
GNU GLOBAL (https://www.gnu.org/software/global/) is source code
tagging system. It creates 4 tag files (GTAGS, GRTAGS, GSYMS and
GPATH) for the symbol cross-reference. Ignore them.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Currently, the BUILD_STRING is just 7-digits git hash. It is true
we can identify which version is running, but we can not get a quick
idea about how new or old it is.
The command "git describe" provides us a bit more useful information
in the format of:
(tag-name)-(number of commits on top the tag)-g(7 digits hash)
I added some options:
--always
Make "git describe" work without any tag in case the upstream
ATF is cloned, but all the tags are locally dropped.
--tags
Use any tag instead of only annotated tags. In ATF, only some
tags are annotated, actually the last annotated tag is "v0.2",
whereas we are on "v1.3" tag now. This option is needed to get
something like v1.3-233-gbcc2bf0 instead of v0.2-1713-gbcc2bf0.
--dirty
The mark "-dirty" is appended if the source tree is locally
modified.
With this commit, the welcome string
NOTICE: BL1: v1.3(debug):bcc2bf0
will become like follows:
NOTICE: BL1: v1.3(debug):v1.3-233-gbcc2bf0-dirty
While we are here, let's add "2> /dev/null" as well to silently
ignore any error message from git. We should not assume that users
always work in a git repository; the ATF might be released in a
tarball form instead of a git repository. In such a case, the git
command will fail, then the ugly message "fatal: Not a git ..." will
be displayed during the build:
$ make CROSS_COMPILE=aarch64-linux-gnu-
fatal: Not a git repository (or any of the parent directories): .git
Building fvp
CC drivers/io/io_semihosting.c
CC lib/semihosting/semihosting.c
...
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
`PMF_ARM_TIF_IMPL_ID` should be set to 0x41. The code already left
shifts it by 24 bit positions so this was overflowing.
This fixes a build error with GCC 6.2 when
`ENABLE_RUNTIME_INSTRUMENTATION` is set.
Change-Id: I4c99d48ea7ce3d76e9edd1325b1979994db2c0fb
Signed-off-by: dp-arm <dimitris.papastamos@arm.com>
Sharing the same CFLAGS for both the firmware and host tools is not
useful and can lead to compilation failure when CFLAGS is set on the
command line.
This patch make MAKEOVERRIDES special variable empty so make's command
line variable definitions are not implicitly forwarded to sub make
invocations. However, make options are still forwarded like "-s" option
to make the build silent.
Change-Id: Ia921c02beb1fd2041696a26160b9e7974ae000fa
Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>
The errata reporting policy is as follows:
- If an errata workaround is enabled:
- If it applies (i.e. the CPU is affected by the errata), an INFO
message is printed, confirming that the errata workaround has been
applied.
- If it does not apply, a VERBOSE message is printed, confirming
that the errata workaround has been skipped.
- If an errata workaround is not enabled, but would have applied had
it been, a WARN message is printed, alerting that errata workaround
is missing.
The CPU errata messages are printed by both BL1 (primary CPU only) and
runtime firmware on debug builds, once for each CPU/errata combination.
Relevant output from Juno r1 console when ARM Trusted Firmware is built
with PLAT=juno LOG_LEVEL=50 DEBUG=1:
VERBOSE: BL1: cortex_a57: errata workaround for 806969 was not applied
VERBOSE: BL1: cortex_a57: errata workaround for 813420 was not applied
INFO: BL1: cortex_a57: errata workaround for disable_ldnp_overread was applied
WARNING: BL1: cortex_a57: errata workaround for 826974 was missing!
WARNING: BL1: cortex_a57: errata workaround for 826977 was missing!
WARNING: BL1: cortex_a57: errata workaround for 828024 was missing!
WARNING: BL1: cortex_a57: errata workaround for 829520 was missing!
WARNING: BL1: cortex_a57: errata workaround for 833471 was missing!
...
VERBOSE: BL31: cortex_a57: errata workaround for 806969 was not applied
VERBOSE: BL31: cortex_a57: errata workaround for 813420 was not applied
INFO: BL31: cortex_a57: errata workaround for disable_ldnp_overread was applied
WARNING: BL31: cortex_a57: errata workaround for 826974 was missing!
WARNING: BL31: cortex_a57: errata workaround for 826977 was missing!
WARNING: BL31: cortex_a57: errata workaround for 828024 was missing!
WARNING: BL31: cortex_a57: errata workaround for 829520 was missing!
WARNING: BL31: cortex_a57: errata workaround for 833471 was missing!
...
VERBOSE: BL31: cortex_a53: errata workaround for 826319 was not applied
INFO: BL31: cortex_a53: errata workaround for disable_non_temporal_hint was applied
Also update documentation.
Change-Id: Iccf059d3348adb876ca121cdf5207bdbbacf2aba
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>
At present, spin locks can only defined from C files. Add some macros
such that they can be defined from assembly files too.
Change-Id: I64f0c214062f5c15b3c8b412c7f25c908e87d970
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>
Disable the automatic substitution of functions with builtins. The
existing -ffreestanding option should already do this but explicitly
adding -fno-builtin reduces the risk of compiler variation. With this
option, GCC is not supposed to be able to make assumptions on what the
function does, which could otherwise lead to security-sensitive code
removal.
This can lead to potentially less efficient code but improves
predictability of what code is actually compiled into the binary.
Change-Id: I06ad151c61318bd1b00d84976f051d2d94314acc
Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>
The current fiptool packs all the images without any padding between
them. So, the offset to each image has no alignment. This is not
efficient, for example, when the FIP is read from a block-oriented
device.
For example, (e)MMC is accessed by block-addressing. The block size
is 512 byte. So, the best case is each image is aligned by 512 byte
since the DMA engine can transfer the whole of the image to its load
address directly. The worst case is the offset does not have even
DMA-capable alignment (this is where we stand now). In this case,
we need to transfer every block to a bounce buffer, then do memcpy()
from the bounce buffer to our final destination. At least, this
should work with the abstraction by the block I/O layer, but the
CPU-intervention for the whole data transfer makes it really slow.
This commit adds a new option --align to the fiptool. This option,
if given, requests the tool to align each component in the FIP file
by the specified byte. Also, add a new Make option FIP_ALIGN for
easier access to this feature; users can give something like
FIP_ALIGN=512 from the command line, or add "FIP_ALIGN := 512" to
their platform.mk file.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
The struct image has "uuid" and "size" to memorize the field values
they had in the TOC entry. So, parse_fip() copies them from struct
fip_toc_entry to struct image, then pack_images() copies them back
to struct fip_toc_entry.
The next commit (support --align option) will require to save the
"offset" field as well. This makes me realize that struct image
can embed struct fip_toc_entry.
This commit will allow the "flags" field to persevere the "update"
command. At this moment, the "flags" is not used in a useful way.
(Yet, platforms can save their own parameters in the flags field.)
It makes sense to save it unless users explicitly replace the image.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
We need not handle the image_head as a special case. Just use
a double-pointer to simplify the traverse.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
lookup_image(_desc)_from_uuid() traverses the linked list, so it
is not efficient. We just want to make sure *p points to NULL here.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Commit e0f083a09b ("fiptool: Prepare ground for expanding the set
of images at runtime") introduced another side effect; the "update"
command now changes the image order in the FIP.
Let's say you have an FIP with BL2, BL31, BL32, BL33. If you update
for example, BL32 with the "update" command, you will get a new FIP
with BL2, BL31, BL33, BL32, in this order.
It happens like this; remove_image() removes the old image from the
linked list, add_image() adds the new image at the tail of the list,
then images are packed in the new order. Prior to that commit,
images were updated by replace_image(), but it was deleted by the
re-work. Revive replace_image() that is re-implemented to work with
the linked list.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
The conditional
if (desc != NULL)
...
is always true here because we assert it 6 lines above:
assert(desc != NULL);
Remove the if-conditional and concatenate the printf() calls.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
This line should check the existence of the input file, but it is
actually checking the output file. When -o option is given to the
"update" command, the outfile is unlikely to exist, then parse_fip()
is skipped and an empty FIP file is output. This is wrong behavior.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
The current user guide mentions that Foundation model doesn't support
debugger interface. Clarify that all FVPs support --cadi-server option
such that a CADI-compliant debugger can connect to and control model
execution.
Also fix broken URL to FVP home page.
Change-Id: Ia14d618a4e0abb4b228eb1616040f9b51fb3f6f9
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>
With GCC 6.2 compiler, more C undefined behaviour is being flagged as
warnings, which result in build errors in ARM TF build.
The specific issue that this patch resolves is the use of (1 << 31),
which is predominantly used in case statements, where 1 is represented
as a signed int. When shifted to msb the behaviour is undefined.
The resolution is to specify 1 as an unsigned int using a convenience
macro ULL(). A duplicate macro MAKE_ULL() is replaced.
FixesARM-software/tf-issues#438
Change-Id: I08e3053bbcf4c022ee2be33a75bd0056da4073e1
Signed-off-by: David Cunado <david.cunado@arm.com>
To avoid timing side-channel attacks, it is needed to use a constant
time memory comparison function when comparing hashes. The affected
code only cheks for equality so it isn't needed to use any variant of
memcmp(), bcmp() is enough.
Also, timingsafe_bcmp() is as fast as memcmp() when the two compared
regions are equal, so this change incurrs no performance hit in said
case. In case they are unequal, the boot sequence wouldn't continue as
normal, so performance is not an issue.
Change-Id: I1c7c70ddfa4438e6031c8814411fef79fd3bb4df
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
Some side-channel attacks involve an attacker inferring something from
the time taken for a memory compare operation to complete, for example
when comparing hashes during image authentication. To mitigate this,
timingsafe_bcmp() must be used for such operations instead of the
standard memcmp().
This function executes in constant time and so doesn't leak any timing
information to the caller.
Change-Id: I470a723dc3626a0ee6d5e3f7fd48d0a57b8aa5fd
Signed-off-by: dp-arm <dimitris.papastamos@arm.com>
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
This code has been imported and slightly adapted from FreeBSD:
6253393ad8/lib/libc/string/strnlen.c
Change-Id: Ie5ef5f92e6e904adb88f8628077fdf1d27470eb3
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
One nasty part of ATF is some of boolean macros are always defined
as 1 or 0, and the rest of them are only defined under certain
conditions.
For the former group, "#if FOO" or "#if !FOO" must be used because
"#ifdef FOO" is always true. (Options passed by $(call add_define,)
are the cases.)
For the latter, "#ifdef FOO" or "#ifndef FOO" should be used because
checking the value of an undefined macro is strange.
For AARCH32/AARCH64, these macros are defined in the top-level
Makefile as follows:
ifeq (${ARCH},aarch32)
$(eval $(call add_define,AARCH32))
else
$(eval $(call add_define,AARCH64))
endif
This means only one of the two is defined. So, AARCH32/AARCH64
belongs to the latter group where we should use #ifdef or #ifndef.
The conditionals are mostly coded correctly, but I see some mistakes.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Masahiro Yamada
David Cunado
danh-arm
Douglas Raillard
dp-arm
Jeenu Viswambharan
Antonio Nino Diaz
Sandrine Bailleux
Koan-Sin Tan